Repository: coreos/dex
Pull request #562 (2016-08-20 00:26:55)

dev branch: support the implicit flow
Friday hack.

cc @sym3tri @chancez @dghubble 

Am going to update this PR with an integration test later.

sym3tri: Given that implicit flow is a lot less secure I still thing we should have it off by default and only enable it if opted-in by a flag.
ericchiang: Updated to make optional and integration test added.

Patch:
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 {


Test patch:
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()