From c113df961aa2fc6af6c4d43eb375c277f8ee45d8 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Fri, 19 Aug 2016 17:24:49 -0700 Subject: [PATCH 1/2] *: support the implicit flow --- cmd/dex/config.go | 6 ++++ server/handlers.go | 68 ++++++++++++++++++++++++++++++---------------- server/oauth2.go | 36 +++++++++++++++++------- server/server.go | 28 ++++++++++++++++--- 4 files changed, 100 insertions(+), 38 deletions(-) diff --git a/cmd/dex/config.go b/cmd/dex/config.go index 1d849980..ea764156 100644 --- a/cmd/dex/config.go +++ b/cmd/dex/config.go @@ -19,10 +19,16 @@ type Config struct { Storage Storage `yaml:"storage"` Connectors []Connector `yaml:"connectors"` Web Web `yaml:"web"` + OAuth2 OAuth2 `yaml:"oauth2"` StaticClients []storage.Client `yaml:"staticClients"` } +// OAuth2 describes enabled OAuth2 extensions. +type OAuth2 struct { + ResponseTypes []string `yaml:"responseTypes"` +} + // Web is the config format for the HTTP server. type Web struct { HTTP string `yaml:"http"` diff --git a/server/handlers.go b/server/handlers.go index 4a0c0305..7c9bba58 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -102,7 +102,7 @@ func (s *Server) handleDiscovery(w http.ResponseWriter, r *http.Request) { // handleAuthorization handles the OAuth2 auth endpoint. func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { - authReq, err := parseAuthorizationRequest(s.storage, r) + authReq, err := parseAuthorizationRequest(s.storage, s.supportedResponseTypes, r) if err != nil { s.renderError(w, http.StatusInternalServerError, err.Type, err.Description) return @@ -318,35 +318,55 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe } return } - code := storage.AuthCode{ - ID: storage.NewID(), - ClientID: authReq.ClientID, - ConnectorID: authReq.ConnectorID, - Nonce: authReq.Nonce, - Scopes: authReq.Scopes, - Claims: *authReq.Claims, - Expiry: s.now().Add(time.Minute * 5), - RedirectURI: authReq.RedirectURI, - } - if err := s.storage.CreateAuthCode(code); err != nil { - log.Printf("Failed to create auth code: %v", err) - s.renderError(w, http.StatusInternalServerError, errServerError, "") - return - } - - if authReq.RedirectURI == "urn:ietf:wg:oauth:2.0:oob" { - // TODO(ericchiang): Add a proper template. - fmt.Fprintf(w, "Code: %s", code.ID) - return - } - u, err := url.Parse(authReq.RedirectURI) if err != nil { s.renderError(w, http.StatusInternalServerError, errServerError, "Invalid redirect URI.") return } q := u.Query() - q.Set("code", code.ID) + + for _, responseType := range authReq.ResponseTypes { + switch responseType { + case responseTypeCode: + code := storage.AuthCode{ + ID: storage.NewID(), + ClientID: authReq.ClientID, + ConnectorID: authReq.ConnectorID, + Nonce: authReq.Nonce, + Scopes: authReq.Scopes, + Claims: *authReq.Claims, + Expiry: s.now().Add(time.Minute * 5), + RedirectURI: authReq.RedirectURI, + } + if err := s.storage.CreateAuthCode(code); err != nil { + log.Printf("Failed to create auth code: %v", err) + s.renderError(w, http.StatusInternalServerError, errServerError, "") + return + } + + if authReq.RedirectURI == redirectURIOOB { + // TODO(ericchiang): Add a proper template. + fmt.Fprintf(w, "Code: %s", code.ID) + return + } + q.Set("code", code.ID) + case responseTypeToken: + idToken, expiry, err := s.newIDToken(authReq.ClientID, *authReq.Claims, authReq.Scopes, authReq.Nonce) + if err != nil { + log.Printf("failed to create ID token: %v", err) + tokenErr(w, errServerError, "", http.StatusInternalServerError) + return + } + v := url.Values{} + v.Set("access_token", storage.NewID()) + v.Set("token_type", "bearer") + v.Set("id_token", idToken) + v.Set("state", authReq.State) + v.Set("expires_in", strconv.Itoa(int(expiry.Sub(s.now())))) + u.Fragment = v.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 1b9ec73a..e236e0c4 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -77,6 +77,10 @@ const ( scopeCrossClientPrefix = "oauth2:server:client_id:" ) +const ( + redirectURIOOB = "urn:ietf:wg:oauth:2.0:oob" +) + const ( grantTypeAuthorizationCode = "authorization_code" grantTypeRefreshToken = "refresh_token" @@ -88,12 +92,6 @@ const ( responseTypeIDToken = "id_token" // ID Token in url fragment ) -var validResponseTypes = map[string]bool{ - "code": true, - "token": true, - "id_token": true, -} - type audience []string func (a audience) MarshalJSON() ([]byte, error) { @@ -182,7 +180,7 @@ 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 parseAuthorizationRequest(s storage.Storage, r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) { +func parseAuthorizationRequest(s storage.Storage, 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."} } @@ -252,9 +250,27 @@ func parseAuthorizationRequest(s storage.Storage, r *http.Request) (req storage. 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 !validResponseTypes[responseType] { + if !supportedResponseTypes[responseType] { + return req, newErr("invalid_request", "Invalid response type %q", responseType) + } + + switch responseType { + case responseTypeCode: + 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) + } + default: return req, newErr("invalid_request", "Invalid response type %q", responseType) } } @@ -263,7 +279,7 @@ func parseAuthorizationRequest(s storage.Storage, r *http.Request) (req storage. ID: storage.NewID(), ClientID: client.ID, State: r.Form.Get("state"), - Nonce: r.Form.Get("nonce"), + Nonce: nonce, ForceApprovalPrompt: r.Form.Get("approval_prompt") == "force", Scopes: scopes, RedirectURI: redirectURI, @@ -308,7 +324,7 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool { return false } - if redirectURI == "urn:ietf:wg:oauth:2.0:oob" { + if redirectURI == redirectURIOOB { return true } if !strings.HasPrefix(redirectURI, "http://localhost:") { diff --git a/server/server.go b/server/server.go index 3c8c2e7c..bb488f54 100644 --- a/server/server.go +++ b/server/server.go @@ -23,6 +23,8 @@ type Connector struct { } // Config holds the server's configuration options. +// +// Multiple servers using the same storage are expected to be configured identically. type Config struct { Issuer string @@ -32,8 +34,10 @@ type Config struct { // Strategies for federated identity. Connectors []Connector - // NOTE: Multiple servers using the same storage are expected to set rotation and - // validity periods to the same values. + // Valid values are "code" to enable the code flow and "token" to enable the implicit + // flow. If no response types are supplied this value defaults to "code". + SupportedResponseTypes []string + RotateKeysAfter time.Duration // Defaults to 6 hours. IDTokensValidFor time.Duration // Defaults to 24 hours @@ -63,6 +67,8 @@ type Server struct { // No package level API to set this, only used in tests. skipApproval bool + supportedResponseTypes map[string]bool + now func() time.Time idTokensValidFor time.Duration @@ -87,6 +93,19 @@ func newServer(c Config, rotationStrategy rotationStrategy) (*Server, error) { if c.Storage == nil { return nil, errors.New("server: storage cannot be nil") } + if len(c.SupportedResponseTypes) == 0 { + c.SupportedResponseTypes = []string{responseTypeCode} + } + + supported := make(map[string]bool) + for _, respType := range c.SupportedResponseTypes { + switch respType { + case responseTypeCode, responseTypeToken: + default: + return nil, fmt.Errorf("unsupported response_type %q", respType) + } + supported[respType] = true + } now := c.Now if now == nil { @@ -102,8 +121,9 @@ func newServer(c Config, rotationStrategy rotationStrategy) (*Server, error) { ), now, ), - idTokensValidFor: value(c.IDTokensValidFor, 24*time.Hour), - now: now, + supportedResponseTypes: supported, + idTokensValidFor: value(c.IDTokensValidFor, 24*time.Hour), + now: now, } for _, conn := range c.Connectors { From 02dd18483df5fdc5b12d85b1a6de63d32d5b90dc Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Wed, 24 Aug 2016 10:00:04 -0700 Subject: [PATCH 2/2] server: add integration test for the implicit flow --- server/server_test.go | 172 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 166 insertions(+), 6 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 381465f1..4865ab1f 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -4,9 +4,14 @@ import ( "crypto/rsa" "crypto/x509" "encoding/pem" + "errors" + "fmt" "net/http" "net/http/httptest" "net/http/httputil" + "net/url" + "strings" + "sync" "testing" "time" @@ -59,12 +64,11 @@ FDWV28nTP9sqbtsmU8Tem2jzMvZ7C/Q0AuDoKELFUpux8shm8wfIhyaPnXUGZoAZ Np4vUwMSYV5mopESLWOg3loBxKyLGFtgGKVCjGiQvy6zISQ4fQo= -----END RSA PRIVATE KEY-----`) -func newTestServer(path string) (*httptest.Server, *Server) { +func newTestServer(updateConfig func(c *Config)) (*httptest.Server, *Server) { var server *Server s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { server.ServeHTTP(w, r) })) - s.URL = s.URL + path config := Config{ Issuer: s.URL, Storage: memory.New(), @@ -76,6 +80,11 @@ func newTestServer(path string) (*httptest.Server, *Server) { }, }, } + if updateConfig != nil { + updateConfig(&config) + } + s.URL = config.Issuer + var err error if server, err = newServer(config, staticRotationStrategy(testKey)); err != nil { panic(err) @@ -85,14 +94,16 @@ func newTestServer(path string) (*httptest.Server, *Server) { } func TestNewTestServer(t *testing.T) { - newTestServer("") + newTestServer(nil) } func TestDiscovery(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - httpServer, _ := newTestServer("/nonrootpath") + httpServer, _ := newTestServer(func(c *Config) { + c.Issuer = c.Issuer + "/non-root-path" + }) defer httpServer.Close() p, err := oidc.NewProvider(ctx, httpServer.URL) @@ -114,11 +125,13 @@ func TestDiscovery(t *testing.T) { } } -func TestOAuth2Flow(t *testing.T) { +func TestOAuth2CodeFlow(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - httpServer, s := newTestServer("/nonrootpath") + httpServer, s := newTestServer(func(c *Config) { + c.Issuer = c.Issuer + "/non-root-path" + }) defer httpServer.Close() p, err := oidc.NewProvider(ctx, httpServer.URL) @@ -221,6 +234,153 @@ func TestOAuth2Flow(t *testing.T) { } } +type nonceSource struct { + nonce string + once sync.Once +} + +func (n *nonceSource) ClaimNonce(nonce string) error { + if n.nonce != nonce { + return errors.New("invalid nonce") + } + ok := false + n.once.Do(func() { ok = true }) + if !ok { + return errors.New("invalid nonce") + } + return nil +} + +func TestOAuth2ImplicitFlow(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + httpServer, s := newTestServer(func(c *Config) { + // Enable support for the implicit flow. + c.SupportedResponseTypes = []string{"code", "token"} + }) + defer httpServer.Close() + + p, err := oidc.NewProvider(ctx, httpServer.URL) + if err != nil { + t.Fatalf("failed to get provider: %v", err) + } + + var ( + reqDump, respDump []byte + gotIDToken bool + state = "a_state" + nonce = "a_nonce" + ) + defer func() { + if !gotIDToken { + t.Errorf("never got a id token in fragment\n%s\n%s", reqDump, respDump) + } + }() + + var oauth2Config *oauth2.Config + oauth2Server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/callback" { + q := r.URL.Query() + if errType := q.Get("error"); errType != "" { + if desc := q.Get("error_description"); desc != "" { + t.Errorf("got error from server %s: %s", errType, desc) + } else { + t.Errorf("got error from server %s", errType) + } + w.WriteHeader(http.StatusInternalServerError) + return + } + // Fragment is checked by the client since net/http servers don't preserve URL fragments. + // E.g. + // + // r.URL.Fragment + // + // Will always be empty. + w.WriteHeader(http.StatusOK) + return + } + u := oauth2Config.AuthCodeURL(state, oauth2.SetAuthURLParam("response_type", "token"), oidc.Nonce(nonce)) + http.Redirect(w, r, u, http.StatusSeeOther) + })) + + defer oauth2Server.Close() + + redirectURL := oauth2Server.URL + "/callback" + client := storage.Client{ + ID: "testclient", + Secret: "testclientsecret", + RedirectURIs: []string{redirectURL}, + } + if err := s.storage.CreateClient(client); err != nil { + t.Fatalf("failed to create client: %v", err) + } + + src := &nonceSource{nonce: nonce} + + idTokenVerifier := p.NewVerifier(ctx, oidc.VerifyAudience(client.ID), oidc.VerifyNonce(src)) + + oauth2Config = &oauth2.Config{ + ClientID: client.ID, + ClientSecret: client.Secret, + Endpoint: p.Endpoint(), + Scopes: []string{oidc.ScopeOpenID, "profile", "email", "offline_access"}, + RedirectURL: redirectURL, + } + + checkIDToken := func(u *url.URL) error { + if u.Fragment == "" { + return fmt.Errorf("url has no fragment: %s", u) + } + v, err := url.ParseQuery(u.Fragment) + if err != nil { + return fmt.Errorf("failed to parse fragment: %v", err) + } + idToken := v.Get("id_token") + if idToken == "" { + return errors.New("no id_token in fragment") + } + if _, err := idTokenVerifier.Verify(idToken); err != nil { + return fmt.Errorf("failed to verify id_token: %v", err) + } + return nil + } + + httpClient := &http.Client{ + // net/http servers don't preserve URL fragments when passing the request to + // handlers. The only way to get at that values is to check the redirect on + // the client side. + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) > 10 { + return errors.New("too many redirects") + } + + // If we're being redirected back to the client server, inspect the URL fragment + // for an ID Token. + u := req.URL.String() + if strings.HasPrefix(u, oauth2Server.URL) { + if err := checkIDToken(req.URL); err == nil { + gotIDToken = true + } else { + t.Error(err) + } + } + return nil + }, + } + + resp, err := httpClient.Get(oauth2Server.URL + "/login") + if err != nil { + t.Fatalf("get failed: %v", err) + } + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { + t.Fatal(err) + } + if respDump, err = httputil.DumpResponse(resp, true); err != nil { + t.Fatal(err) + } +} + type storageWithKeysTrigger struct { storage.Storage f func()