diff --git a/server/handlers.go b/server/handlers.go
index c962265fe7325d9165b69821a37b4fff8e73def1..cda845f8b69d52995092c4828bcf7d3661fbb62d 100644
--- a/server/handlers.go
+++ b/server/handlers.go
@@ -144,12 +144,23 @@ func (s *Server) discoveryHandler() (http.Handler, error) {
// handleAuthorization handles the OAuth2 auth endpoint.
func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
- authReq, err := s.parseAuthorizationRequest(s.supportedResponseTypes, r)
+ authReq, err := s.parseAuthorizationRequest(r)
if err != nil {
s.logger.Errorf("Failed to parse authorization request: %v", err)
- s.renderError(w, http.StatusInternalServerError, "Failed to connect to the database.")
+ if handler, ok := err.Handle(); ok {
+ // client_id and redirect_uri checked out and we can redirect back to
+ // the client with the error.
+ handler.ServeHTTP(w, r)
+ return
+ }
+
+ // Otherwise render the error to the user.
+ //
+ // TODO(ericchiang): Should we just always render the error?
+ s.renderError(w, err.Status(), err.Error())
return
}
+
authReq.Expiry = s.now().Add(time.Minute * 30)
if err := s.storage.CreateAuthRequest(authReq); err != nil {
s.logger.Errorf("Failed to create authorization request: %v", err)
@@ -397,12 +408,25 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe
s.renderError(w, http.StatusInternalServerError, "Invalid redirect URI.")
return
}
- q := u.Query()
+
+ var (
+ // Was the initial request using the implicit or hybrid flow instead of
+ // the "normal" code flow?
+ implicitOrHybrid = false
+
+ // Only present in hybrid or code flow. code.ID == "" if this is not set.
+ code storage.AuthCode
+
+ // ID token returned immediately if the response_type includes "id_token".
+ // Only valid for implicit and hybrid flows.
+ idToken string
+ idTokenExpiry time.Time
+ )
for _, responseType := range authReq.ResponseTypes {
switch responseType {
case responseTypeCode:
- code := storage.AuthCode{
+ code = storage.AuthCode{
ID: storage.NewID(),
ClientID: authReq.ClientID,
ConnectorID: authReq.ConnectorID,
@@ -419,32 +443,73 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe
return
}
+ // Implicit and hybrid flows that try to use the OOB redirect URI are
+ // rejected earlier. If we got here we're using the code flow.
if authReq.RedirectURI == redirectURIOOB {
if err := s.templates.oob(w, code.ID); err != nil {
s.logger.Errorf("Server template error: %v", err)
}
return
}
- q.Set("code", code.ID)
case responseTypeToken:
- idToken, expiry, err := s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce)
+ implicitOrHybrid = true
+ case responseTypeIDToken:
+ implicitOrHybrid = true
+ var err error
+ idToken, idTokenExpiry, err = s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce)
if err != nil {
s.logger.Errorf("failed to create ID token: %v", err)
s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
return
}
- v := url.Values{}
- v.Set("access_token", storage.NewID())
- v.Set("token_type", "bearer")
+ }
+ }
+
+ if implicitOrHybrid {
+ v := url.Values{}
+ v.Set("access_token", storage.NewID())
+ v.Set("token_type", "bearer")
+ v.Set("state", authReq.State)
+ if idToken != "" {
v.Set("id_token", idToken)
- v.Set("state", authReq.State)
- v.Set("expires_in", strconv.Itoa(int(expiry.Sub(s.now()).Seconds())))
- u.Fragment = v.Encode()
+ // The hybrid flow with only "code token" or "code id_token" doesn't return an
+ // "expires_in" value. If "code" wasn't provided, indicating the implicit flow,
+ // don't add it.
+ //
+ // https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthResponse
+ if code.ID == "" {
+ v.Set("expires_in", strconv.Itoa(int(idTokenExpiry.Sub(s.now()).Seconds())))
+ }
}
+ if code.ID != "" {
+ v.Set("code", code.ID)
+ }
+
+ // Implicit and hybrid flows return their values as part of the fragment.
+ //
+ // HTTP/1.1 303 See Other
+ // Location: https://client.example.org/cb#
+ // access_token=SlAV32hkKG
+ // &token_type=bearer
+ // &id_token=eyJ0 ... NiJ9.eyJ1c ... I6IjIifX0.DeWt4Qu ... ZXso
+ // &expires_in=3600
+ // &state=af0ifjsldkj
+ //
+ u.Fragment = v.Encode()
+ } else {
+ // The code flow add values to the URL query.
+ //
+ // HTTP/1.1 303 See Other
+ // Location: https://client.example.org/cb?
+ // code=SplxlOBeZQQYbYS6WxSbIA
+ // &state=af0ifjsldkj
+ //
+ q := u.Query()
+ q.Set("code", code.ID)
+ q.Set("state", authReq.State)
+ u.RawQuery = q.Encode()
}
- q.Set("state", authReq.State)
- u.RawQuery = q.Encode()
http.Redirect(w, r, u.String(), http.StatusSeeOther)
}
diff --git a/server/oauth2.go b/server/oauth2.go
index d8211e7313aabb03f9f199a6c3b94a3d36f30032..adbf1eedabddee8cb8f7a7d53a96b5553d596a93 100644
--- a/server/oauth2.go
+++ b/server/oauth2.go
@@ -24,20 +24,39 @@ type authErr struct {
Description string
}
-func (err *authErr) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- v := url.Values{}
- v.Add("state", err.State)
- v.Add("error", err.Type)
- if err.Description != "" {
- v.Add("error_description", err.Description)
- }
- var redirectURI string
- if strings.Contains(err.RedirectURI, "?") {
- redirectURI = err.RedirectURI + "&" + v.Encode()
- } else {
- redirectURI = err.RedirectURI + "?" + v.Encode()
+func (err *authErr) Status() int {
+ if err.State == errServerError {
+ return http.StatusInternalServerError
+ }
+ return http.StatusBadRequest
+}
+
+func (err *authErr) Error() string {
+ return err.Description
+}
+
+func (err *authErr) Handle() (http.Handler, bool) {
+ // Didn't get a valid redirect URI.
+ if err.RedirectURI == "" {
+ return nil, false
+ }
+
+ hf := func(w http.ResponseWriter, r *http.Request) {
+ v := url.Values{}
+ v.Add("state", err.State)
+ v.Add("error", err.Type)
+ if err.Description != "" {
+ v.Add("error_description", err.Description)
+ }
+ var redirectURI string
+ if strings.Contains(err.RedirectURI, "?") {
+ redirectURI = err.RedirectURI + "&" + v.Encode()
+ } else {
+ redirectURI = err.RedirectURI + "?" + v.Encode()
+ }
+ http.Redirect(w, r, redirectURI, http.StatusSeeOther)
}
- http.Redirect(w, r, redirectURI, http.StatusSeeOther)
+ return http.HandlerFunc(hf), true
}
func tokenErr(w http.ResponseWriter, typ, description string, statusCode int) error {
@@ -192,20 +211,19 @@ func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []str
}
// parse the initial request from the OAuth2 client.
-//
-// For correctness the logic is largely copied from https://github.com/RangelReale/osin.
-func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]bool, r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) {
- if err := r.ParseForm(); err != nil {
- return req, &authErr{"", "", errInvalidRequest, "Failed to parse request."}
- }
-
- redirectURI, err := url.QueryUnescape(r.Form.Get("redirect_uri"))
+func (s *Server) parseAuthorizationRequest(r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) {
+ q := r.URL.Query()
+ redirectURI, err := url.QueryUnescape(q.Get("redirect_uri"))
if err != nil {
return req, &authErr{"", "", errInvalidRequest, "No redirect_uri provided."}
}
- state := r.FormValue("state")
- clientID := r.Form.Get("client_id")
+ clientID := q.Get("client_id")
+ state := q.Get("state")
+ nonce := q.Get("nonce")
+ // Some clients, like the old go-oidc, provide extra whitespace. Tolerate this.
+ scopes := strings.Fields(q.Get("scope"))
+ responseTypes := strings.Fields(q.Get("response_type"))
client, err := s.storage.GetClient(clientID)
if err != nil {
@@ -222,12 +240,11 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
return req, &authErr{"", "", errInvalidRequest, description}
}
+ // From here on out, we want to redirect back to the client with an error.
newErr := func(typ, format string, a ...interface{}) *authErr {
return &authErr{state, redirectURI, typ, fmt.Sprintf(format, a...)}
}
- scopes := strings.Fields(r.Form.Get("scope"))
-
var (
unrecognized []string
invalidScopes []string
@@ -247,7 +264,7 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
isTrusted, err := s.validateCrossClientTrust(clientID, peerID)
if err != nil {
- return req, newErr(errServerError, "")
+ return req, newErr(errServerError, "Internal server error.")
}
if !isTrusted {
invalidScopes = append(invalidScopes, scope)
@@ -264,37 +281,61 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
return req, newErr("invalid_scope", "Client can't request scope(s) %q", invalidScopes)
}
- nonce := r.Form.Get("nonce")
- responseTypes := strings.Split(r.Form.Get("response_type"), " ")
- for _, responseType := range responseTypes {
- if !supportedResponseTypes[responseType] {
- return req, newErr("invalid_request", "Invalid response type %q", responseType)
- }
+ var rt struct {
+ code bool
+ idToken bool
+ token bool
+ }
+ for _, responseType := range responseTypes {
switch responseType {
case responseTypeCode:
+ rt.code = true
+ case responseTypeIDToken:
+ rt.idToken = true
case responseTypeToken:
- // Implicit flow requires a nonce value.
- // https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
- if nonce == "" {
- return req, newErr("invalid_request", "Response type 'token' requires a 'nonce' value.")
- }
-
- if redirectURI == redirectURIOOB {
- err := fmt.Sprintf("Cannot use response type 'token' with redirect_uri '%s'.", redirectURIOOB)
- return req, newErr("invalid_request", err)
- }
+ rt.token = true
default:
return req, newErr("invalid_request", "Invalid response type %q", responseType)
}
+
+ if !s.supportedResponseTypes[responseType] {
+ return req, newErr(errUnsupportedResponseType, "Unsupported response type %q", responseType)
+ }
+ }
+
+ if len(responseTypes) == 0 {
+ return req, newErr("invalid_requests", "No response_type provided")
+ }
+
+ if rt.token && !rt.code && !rt.idToken {
+ // "token" can't be provided by its own.
+ //
+ // https://openid.net/specs/openid-connect-core-1_0.html#Authentication
+ return req, newErr("invalid_request", "Response type 'token' must be provided with type 'id_token' and/or 'code'")
+ }
+ if !rt.code {
+ // Either "id_token code" or "id_token" has been provided which implies the
+ // implicit flow. Implicit flow requires a nonce value.
+ //
+ // https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
+ if nonce == "" {
+ return req, newErr("invalid_request", "Response type 'token' requires a 'nonce' value.")
+ }
+ }
+ if rt.token {
+ if redirectURI == redirectURIOOB {
+ err := fmt.Sprintf("Cannot use response type 'token' with redirect_uri '%s'.", redirectURIOOB)
+ return req, newErr("invalid_request", err)
+ }
}
return storage.AuthRequest{
ID: storage.NewID(),
ClientID: client.ID,
- State: r.Form.Get("state"),
+ State: state,
Nonce: nonce,
- ForceApprovalPrompt: r.Form.Get("approval_prompt") == "force",
+ ForceApprovalPrompt: q.Get("approval_prompt") == "force",
Scopes: scopes,
RedirectURI: redirectURI,
ResponseTypes: responseTypes,
diff --git a/server/oauth2_test.go b/server/oauth2_test.go
index abb4e431abd516750a5a1e5e2b77073c236b8f9e..807de8366c1f9305130c118bb920a2809464bd7b 100644
--- a/server/oauth2_test.go
+++ b/server/oauth2_test.go
@@ -1 +1,150 @@
package server
+
+import (
+ "context"
+ "net/http/httptest"
+ "net/url"
+ "testing"
+
+ "github.com/coreos/dex/storage"
+)
+
+func TestParseAuthorizationRequest(t *testing.T) {
+ tests := []struct {
+ name string
+ clients []storage.Client
+ supportedResponseTypes []string
+
+ queryParams map[string]string
+
+ wantErr bool
+ }{
+ {
+ name: "normal request",
+ clients: []storage.Client{
+ {
+ ID: "foo",
+ RedirectURIs: []string{"https://example.com/foo"},
+ },
+ },
+ supportedResponseTypes: []string{"code"},
+ queryParams: map[string]string{
+ "client_id": "foo",
+ "redirect_uri": "https://example.com/foo",
+ "response_type": "code",
+ "scope": "openid email profile",
+ },
+ },
+ {
+ name: "invalid client id",
+ clients: []storage.Client{
+ {
+ ID: "foo",
+ RedirectURIs: []string{"https://example.com/foo"},
+ },
+ },
+ supportedResponseTypes: []string{"code"},
+ queryParams: map[string]string{
+ "client_id": "bar",
+ "redirect_uri": "https://example.com/foo",
+ "response_type": "code",
+ "scope": "openid email profile",
+ },
+ wantErr: true,
+ },
+ {
+ name: "invalid redirect uri",
+ clients: []storage.Client{
+ {
+ ID: "bar",
+ RedirectURIs: []string{"https://example.com/bar"},
+ },
+ },
+ supportedResponseTypes: []string{"code"},
+ queryParams: map[string]string{
+ "client_id": "bar",
+ "redirect_uri": "https://example.com/foo",
+ "response_type": "code",
+ "scope": "openid email profile",
+ },
+ wantErr: true,
+ },
+ {
+ name: "implicit flow",
+ clients: []storage.Client{
+ {
+ ID: "bar",
+ RedirectURIs: []string{"https://example.com/bar"},
+ },
+ },
+ supportedResponseTypes: []string{"code", "id_token", "token"},
+ queryParams: map[string]string{
+ "client_id": "bar",
+ "redirect_uri": "https://example.com/bar",
+ "response_type": "code id_token",
+ "scope": "openid email profile",
+ },
+ },
+ {
+ name: "unsupported response type",
+ clients: []storage.Client{
+ {
+ ID: "bar",
+ RedirectURIs: []string{"https://example.com/bar"},
+ },
+ },
+ supportedResponseTypes: []string{"code"},
+ queryParams: map[string]string{
+ "client_id": "bar",
+ "redirect_uri": "https://example.com/bar",
+ "response_type": "code id_token",
+ "scope": "openid email profile",
+ },
+ wantErr: true,
+ },
+ {
+ name: "only token response type",
+ clients: []storage.Client{
+ {
+ ID: "bar",
+ RedirectURIs: []string{"https://example.com/bar"},
+ },
+ },
+ supportedResponseTypes: []string{"code", "id_token", "token"},
+ queryParams: map[string]string{
+ "client_id": "bar",
+ "redirect_uri": "https://example.com/bar",
+ "response_type": "token",
+ "scope": "openid email profile",
+ },
+ wantErr: true,
+ },
+ }
+
+ for _, tc := range tests {
+ func() {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ httpServer, server := newTestServer(ctx, t, func(c *Config) {
+ c.SupportedResponseTypes = tc.supportedResponseTypes
+ c.Storage = storage.WithStaticClients(c.Storage, tc.clients)
+ })
+ defer httpServer.Close()
+
+ params := url.Values{}
+ for k, v := range tc.queryParams {
+ params.Set(k, v)
+ }
+
+ req := httptest.NewRequest("GET", httpServer.URL+"/auth?"+params.Encode(), nil)
+ _, err := server.parseAuthorizationRequest(req)
+ if err != nil && !tc.wantErr {
+ t.Errorf("%s: %v", tc.name, err)
+ }
+ if err == nil && tc.wantErr {
+ t.Errorf("%s: expected error", tc.name)
+ }
+ }()
+ }
+}
diff --git a/server/rotation.go b/server/rotation.go
index 326e75973985861e3ae2a3df647461debc7e2981..8516e9bd8897d041648135237e1a7fd680237539 100644
--- a/server/rotation.go
+++ b/server/rotation.go
@@ -130,6 +130,7 @@ func (k keyRotater) rotate() error {
// Remove expired verification keys.
i := 0
+
for _, key := range keys.VerificationKeys {
if !key.Expiry.After(tNow) {
keys.VerificationKeys[i] = key
diff --git a/server/server.go b/server/server.go
index 535e21be6b8a1b8edcd9703de7d477632a7dc235..c0b194a673fb016c74681c048058ee928be11eba 100644
--- a/server/server.go
+++ b/server/server.go
@@ -159,7 +159,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
supported := make(map[string]bool)
for _, respType := range c.SupportedResponseTypes {
switch respType {
- case responseTypeCode, responseTypeToken:
+ case responseTypeCode, responseTypeIDToken, responseTypeToken:
default:
return nil, fmt.Errorf("unsupported response_type %q", respType)
}
diff --git a/server/server_test.go b/server/server_test.go
index 96786fc8c9d2e68a20ebe99b1ca32ba6f570f26b..7c499c1580a070c4b02c02267863a503a9921cfd 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -510,7 +510,7 @@ func TestOAuth2ImplicitFlow(t *testing.T) {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
// Enable support for the implicit flow.
- c.SupportedResponseTypes = []string{"code", "token"}
+ c.SupportedResponseTypes = []string{"code", "token", "id_token"}
})
defer httpServer.Close()
@@ -553,7 +553,7 @@ func TestOAuth2ImplicitFlow(t *testing.T) {
w.WriteHeader(http.StatusOK)
return
}
- u := oauth2Config.AuthCodeURL(state, oauth2.SetAuthURLParam("response_type", "token"), oidc.Nonce(nonce))
+ u := oauth2Config.AuthCodeURL(state, oauth2.SetAuthURLParam("response_type", "id_token token"), oidc.Nonce(nonce))
http.Redirect(w, r, u, http.StatusSeeOther)
}))