server: CodeToken now does Cross-Client auth

scope/scope.go

@@ -0,0 +1,34 @@
+package scope
+
+import "strings"
+
+const (
+	// Scope prefix which indicates initiation of a cross-client authentication flow.
+	// See https://developers.google.com/identity/protocols/CrossClientAuth
+	ScopeGoogleCrossClient = "audience:server:client_id:"
+)
+
+type Scopes []string
+
+func (s Scopes) OfflineAccess() bool {
+	return s.HasScope("offline_access")
+}
+
+func (s Scopes) HasScope(scope string) bool {
+	for _, curScope := range s {
+		if curScope == scope {
+			return true
+		}
+	}
+	return false
+}
+
+func (s Scopes) CrossClientIDs() []string {
+	clients := []string{}
+	for _, scope := range s {
+		if strings.HasPrefix(scope, ScopeGoogleCrossClient) {
+			clients = append(clients, scope[len(ScopeGoogleCrossClient):])
+		}
+	}
+	return clients
+}

server/cross_client_test.go

@@ -1,17 +1,22 @@
 package server
 
 import (
+	"encoding/base64"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"sort"
 	"strings"
 	"testing"
 
 	"github.com/coreos/go-oidc/oidc"
+	"github.com/kylelemons/godebug/pretty"
 
 	"github.com/coreos/dex/client"
+	clientmanager "github.com/coreos/dex/client/manager"
 	"github.com/coreos/dex/connector"
+	"github.com/coreos/dex/scope"
 )
 
 func makeCrossClientTestFixtures() (*testFixtures, error) {
@@ -20,7 +25,6 @@ func makeCrossClientTestFixtures() (*testFixtures, error) {
 		return nil, fmt.Errorf("couldn't make test fixtures: %v", err)
 	}
 
-	creds := map[string]oidc.ClientCredentials{}
 	for _, cliData := range []struct {
 		id         string
 		authorized []string
@@ -38,24 +42,22 @@ func makeCrossClientTestFixtures() (*testFixtures, error) {
 		u := url.URL{
 			Scheme: "https://",
 			Path:   cliData.id,
-			Host:   "auth.example.com",
+			Host:   cliData.id,
 		}
-		cliCreds, err := f.clientRepo.New(client.Client{
+		cliCreds, err := f.clientManager.New(client.Client{
 			Credentials: oidc.ClientCredentials{
 				ID: cliData.id,
 			},
 			Metadata: oidc.ClientMetadata{
 				RedirectURIs: []url.URL{u},
 			},
+		}, &clientmanager.ClientOptions{
+			TrustedPeers: cliData.authorized,
 		})
 		if err != nil {
 			return nil, fmt.Errorf("Unexpected error creating clients: %v", err)
 		}
-		creds[cliData.id] = *cliCreds
+		f.clientCreds[cliData.id] = *cliCreds
-		err = f.clientRepo.SetTrustedPeers(cliData.id, cliData.authorized)
-		if err != nil {
-			return nil, fmt.Errorf("Unexpected error setting cross-client authorizers: %v", err)
-		}
 	}
 	return f, nil
 }
@@ -132,30 +134,30 @@ func TestHandleAuthCrossClient(t *testing.T) {
 		wantCode int
 	}{
 		{
-			scopes:   []string{ScopeGoogleCrossClient + "client_a"},
+			scopes:   []string{scope.ScopeGoogleCrossClient + "client_a"},
 			clientID: "client_b",
 			wantCode: http.StatusBadRequest,
 		},
 		{
-			scopes:   []string{ScopeGoogleCrossClient + "client_b"},
+			scopes:   []string{scope.ScopeGoogleCrossClient + "client_b"},
 			clientID: "client_a",
 			wantCode: http.StatusFound,
 		},
 		{
-			scopes:   []string{ScopeGoogleCrossClient + "client_b"},
+			scopes:   []string{scope.ScopeGoogleCrossClient + "client_b"},
 			clientID: "client_a",
 			wantCode: http.StatusFound,
 		},
 		{
-			scopes:   []string{ScopeGoogleCrossClient + "client_c"},
+			scopes:   []string{scope.ScopeGoogleCrossClient + "client_c"},
 			clientID: "client_a",
 			wantCode: http.StatusFound,
 		},
 		{
 			// Two clients that client_a is authorized to mint tokens for.
 			scopes: []string{
-				ScopeGoogleCrossClient + "client_c",
+				scope.ScopeGoogleCrossClient + "client_c",
-				ScopeGoogleCrossClient + "client_b",
+				scope.ScopeGoogleCrossClient + "client_b",
 			},
 			clientID: "client_a",
 			wantCode: http.StatusFound,
@@ -163,8 +165,8 @@ func TestHandleAuthCrossClient(t *testing.T) {
 		{
 			// Two clients that client_a is authorized to mint tokens for.
 			scopes: []string{
-				ScopeGoogleCrossClient + "client_c",
+				scope.ScopeGoogleCrossClient + "client_c",
-				ScopeGoogleCrossClient + "client_a",
+				scope.ScopeGoogleCrossClient + "client_a",
 			},
 			clientID: "client_b",
 			wantCode: http.StatusBadRequest,
@@ -200,3 +202,129 @@ func TestHandleAuthCrossClient(t *testing.T) {
 	}
 
 }
+
+func TestServerCodeTokenCrossClient(t *testing.T) {
+	f, err := makeCrossClientTestFixtures()
+	if err != nil {
+		t.Fatalf("Error creating test fixtures: %v", err)
+	}
+	sm := f.sessionManager
+
+	tests := []struct {
+		clientID     string
+		offline      bool
+		refreshToken string
+		crossClients []string
+
+		wantErr bool
+		wantAUD []string
+		wantAZP string
+	}{
+		// First test the non-cross-client cases, make sure they're undisturbed:
+		{
+			// No 'offline_access' in scope, should get empty refresh token.
+			clientID:     testClientID,
+			refreshToken: "",
+
+			wantAUD: []string{testClientID},
+		},
+		{
+			// Have 'offline_access' in scope, should get non-empty refresh token.
+			clientID:     testClientID,
+			offline:      true,
+			refreshToken: fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))),
+
+			wantAUD: []string{testClientID},
+		},
+		// Now test cross-client cases:
+		{
+			clientID:     "client_a",
+			crossClients: []string{"client_b"},
+
+			wantAUD: []string{"client_b"},
+			wantAZP: "client_a",
+		},
+		{
+			clientID:     "client_a",
+			crossClients: []string{"client_b", "client_a"},
+
+			wantAUD: []string{"client_a", "client_b"},
+			wantAZP: "client_a",
+		},
+	}
+
+	for i, tt := range tests {
+		scopes := []string{"openid"}
+		if tt.offline {
+			scopes = append(scopes, "offline_access")
+		}
+		for _, client := range tt.crossClients {
+			scopes = append(scopes, scope.ScopeGoogleCrossClient+client)
+		}
+
+		sessionID, err := sm.NewSession("bogus_idpc", tt.clientID, "bogus", url.URL{}, "", false, scopes)
+		if err != nil {
+			t.Fatalf("case %d: unexpected error: %v", i, err)
+		}
+		_, err = sm.AttachRemoteIdentity(sessionID, oidc.Identity{})
+		if err != nil {
+			t.Fatalf("case %d: unexpected error: %v", i, err)
+		}
+
+		_, err = sm.AttachUser(sessionID, "ID-1")
+		if err != nil {
+			t.Fatalf("case %d: unexpected error: %v", i, err)
+		}
+
+		key, err := sm.NewSessionKey(sessionID)
+		if err != nil {
+			t.Fatalf("case %d: unexpected error: %v", i, err)
+		}
+
+		jwt, token, err := f.srv.CodeToken(f.clientCreds[tt.clientID], key)
+		if err != nil {
+			t.Fatalf("case %d: unexpected error: %v", i, err)
+		}
+		if jwt == nil {
+			t.Fatalf("case %d: expect non-nil jwt", i)
+		}
+		if token != tt.refreshToken {
+			t.Errorf("case %d: expect refresh token %q, got %q", i, tt.refreshToken, token)
+		}
+
+		claims, err := jwt.Claims()
+		if err != nil {
+			t.Fatalf("case %d: unexpected error getting claims: %v", i, err)
+		}
+
+		var gotAUD []string
+		if len(tt.wantAUD) < 2 {
+			aud, _, err := claims.StringClaim("aud")
+			if err != nil {
+				t.Fatalf("case %d: unexpected error getting 'aud': %q: raw: %v", i, err, claims["aud"])
+			}
+			gotAUD = []string{aud}
+		} else {
+			gotAUD, _, err = claims.StringsClaim("aud")
+			if err != nil {
+				t.Fatalf("case %d: unexpected error getting 'aud': %v", i, err)
+			}
+		}
+
+		sort.Strings(gotAUD)
+		if diff := pretty.Compare(tt.wantAUD, gotAUD); diff != "" {
+			t.Fatalf("case %d: pretty.Compare(tt.wantAUD, gotAUD): %v", i, diff)
+		}
+
+		gotAZP, _, err := claims.StringClaim("azp")
+		if err != nil {
+			if err != nil {
+				t.Fatalf("case %d: unexpected error getting 'aud': %v", i, err)
+			}
+		}
+
+		if gotAZP != tt.wantAZP {
+			t.Errorf("case %d: wantAZP=%v, gotAZP=%v", i, tt.wantAZP, gotAZP)
+		}
+	}
+}

server/http.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"sort"
 	"strings"
 	"time"
 
@@ -22,6 +21,7 @@ import (
 	"github.com/coreos/dex/connector"
 	phttp "github.com/coreos/dex/pkg/http"
 	"github.com/coreos/dex/pkg/log"
+	"github.com/coreos/dex/scope"
 )
 
 const (
@@ -392,20 +392,18 @@ func handleAuthFunc(srv DexServer, idpcs []connector.Connector, tpl *template.Te
 
 func validateScopes(srv DexServer, clientID string, scopes []string) error {
 	foundOpenIDScope := false
-	sort.Strings(scopes)
+	for i, curScope := range scopes {
-	for i, scope := range scopes {
+		if i > 0 && curScope == scopes[i-1] {
-		if i > 0 && scope == scopes[i-1] {
 			err := oauth2.NewError(oauth2.ErrorInvalidRequest)
 			err.Description = fmt.Sprintf(
 				"Duplicate scopes are not allowed: %q",
-				scope)
+				curScope)
 			return err
 		}
 
 		switch {
-		case strings.HasPrefix(scope, ScopeGoogleCrossClient):
+		case strings.HasPrefix(curScope, scope.ScopeGoogleCrossClient):
-			otherClient := scope[len(ScopeGoogleCrossClient):]
+			otherClient := curScope[len(scope.ScopeGoogleCrossClient):]
-
 			var allowed bool
 			var err error
 			if otherClient == clientID {
@@ -424,11 +422,11 @@ func validateScopes(srv DexServer, clientID string, scopes []string) error {
 					clientID, otherClient)
 				return err
 			}
-		case scope == "openid":
+		case curScope == "openid":
 			foundOpenIDScope = true
-		case scope == "profile":
+		case curScope == "profile":
-		case scope == "email":
+		case curScope == "email":
-		case scope == "offline_access":
+		case curScope == "offline_access":
 			// According to the spec, for offline_access scope, the client must
 			// use a response_type value that would result in an Authorization
 			// Code.  Currently oauth2.ResponseTypeCode is the only supported
@@ -439,7 +437,7 @@ func validateScopes(srv DexServer, clientID string, scopes []string) error {
 		default:
 			// Reject all other scopes.
 			err := oauth2.NewError(oauth2.ErrorInvalidRequest)
-			err.Description = fmt.Sprintf("%q is not a recognized scope", scope)
+			err.Description = fmt.Sprintf("%q is not a recognized scope", curScope)
 			return err
 		}
 	}

server/http_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/coreos/dex/client"
 	"github.com/coreos/dex/connector"
+	"github.com/coreos/dex/scope"
 	"github.com/coreos/go-oidc/jose"
 	"github.com/coreos/go-oidc/oauth2"
 	"github.com/coreos/go-oidc/oidc"
@@ -346,21 +347,21 @@ func TestValidateScopes(t *testing.T) {
 		{
 			// ERR: invalid cross client auth
 			clientID: "XXX",
-			scopes:   []string{"openid", ScopeGoogleCrossClient + "client_a"},
+			scopes:   []string{"openid", scope.ScopeGoogleCrossClient + "client_a"},
 			wantErr:  true,
 		},
 		{
 			// OK: valid cross client auth (though perverse - a client
 			// requesting cross-client auth for itself)
 			clientID: "client_a",
-			scopes:   []string{"openid", ScopeGoogleCrossClient + "client_a"},
+			scopes:   []string{"openid", scope.ScopeGoogleCrossClient + "client_a"},
 			wantErr:  false,
 		},
 		{
 
 			// OK: valid cross client auth
 			clientID: "client_a",
-			scopes:   []string{"openid", ScopeGoogleCrossClient + "client_b"},
+			scopes:   []string{"openid", scope.ScopeGoogleCrossClient + "client_b"},
 			wantErr:  false,
 		},
 		{
@@ -368,8 +369,8 @@ func TestValidateScopes(t *testing.T) {
 			// ERR: valid cross client auth...but duplicated scope.
 			clientID: "client_a",
 			scopes: []string{"openid",
-				ScopeGoogleCrossClient + "client_b",
+				scope.ScopeGoogleCrossClient + "client_b",
-				ScopeGoogleCrossClient + "client_b",
+				scope.ScopeGoogleCrossClient + "client_b",
 			},
 			wantErr: true,
 		},
@@ -378,9 +379,9 @@ func TestValidateScopes(t *testing.T) {
 			clientID: "client_a",
 			scopes: []string{
 				"openid",
-				ScopeGoogleCrossClient + "client_a",
+				scope.ScopeGoogleCrossClient + "client_a",
-				ScopeGoogleCrossClient + "client_b",
+				scope.ScopeGoogleCrossClient + "client_b",
-				ScopeGoogleCrossClient + "client_c",
+				scope.ScopeGoogleCrossClient + "client_c",
 			},
 			wantErr: false,
 		},
@@ -388,9 +389,9 @@ func TestValidateScopes(t *testing.T) {
 			// ERR: valid cross client auth with >1 clients including itself...but no openid!
 			clientID: "client_a",
 			scopes: []string{
-				ScopeGoogleCrossClient + "client_a",
+				scope.ScopeGoogleCrossClient + "client_a",
-				ScopeGoogleCrossClient + "client_b",
+				scope.ScopeGoogleCrossClient + "client_b",
-				ScopeGoogleCrossClient + "client_c",
+				scope.ScopeGoogleCrossClient + "client_c",
 			},
 			wantErr: true,
 		},

server/server.go

@@ -39,10 +39,6 @@ const (
 	ResetPasswordTemplateName          = "reset-password.html"
 
 	APIVersion = "v1"
-
-	// Scope prefix which indicates initiation of a cross-client authentication flow.
-	// See https://developers.google.com/identity/protocols/CrossClientAuth
-	ScopeGoogleCrossClient = "audience:server:client_id:"
 )
 
 type OIDCServer interface {
@@ -454,6 +450,36 @@ func (s *Server) CodeToken(creds oidc.ClientCredentials, sessionKey string) (*jo
 	claims := ses.Claims(s.IssuerURL.String())
 	user.AddToClaims(claims)
 
+	crossClientIDs := ses.Scope.CrossClientIDs()
+	if len(crossClientIDs) > 0 {
+		var aud []string
+		for _, id := range crossClientIDs {
+			if ses.ClientID == id {
+				aud = append(aud, id)
+				continue
+			}
+			allowed, err := s.CrossClientAuthAllowed(ses.ClientID, id)
+			if err != nil {
+				log.Errorf("Failed to check cross client auth. reqClientID %v; authClient:ID %v; err: %v", ses.ClientID, id, err)
+				return nil, "", oauth2.NewError(oauth2.ErrorServerError)
+			}
+			if !allowed {
+				err := oauth2.NewError(oauth2.ErrorInvalidRequest)
+				err.Description = fmt.Sprintf(
+					"%q is not authorized to perform cross-client requests for %q",
+					ses.ClientID, id)
+				return nil, "", err
+			}
+			aud = append(aud, id)
+		}
+		if len(aud) == 1 {
+			claims.Add("aud", aud[0])
+		} else {
+			claims.Add("aud", aud)
+		}
+		claims.Add("azp", ses.ClientID)
+	}
+
 	jwt, err := jose.NewSignedJWT(claims, signer)
 	if err != nil {
 		log.Errorf("Failed to generate ID token: %v", err)
@@ -538,7 +564,7 @@ func (s *Server) RefreshToken(creds oidc.ClientCredentials, token string) (*jose
 }
 
 func (s *Server) CrossClientAuthAllowed(requestingClientID, authorizingClientID string) (bool, error) {
-	alloweds, err := s.ClientRepo.GetTrustedPeers(authorizingClientID)
+	alloweds, err := s.ClientRepo.GetTrustedPeers(nil, authorizingClientID)
 	if err != nil {
 		return false, err
 	}

server/server_test.go

@@ -9,16 +9,17 @@ import (
 	"testing"
 	"time"
 
-	"github.com/coreos/dex/client"
-	"github.com/coreos/dex/db"
-	"github.com/coreos/dex/refresh/refreshtest"
-	"github.com/coreos/dex/session/manager"
-	"github.com/coreos/dex/user"
 	"github.com/coreos/go-oidc/jose"
 	"github.com/coreos/go-oidc/key"
 	"github.com/coreos/go-oidc/oauth2"
 	"github.com/coreos/go-oidc/oidc"
 	"github.com/kylelemons/godebug/pretty"
+
+	"github.com/coreos/dex/client"
+	"github.com/coreos/dex/db"
+	"github.com/coreos/dex/refresh/refreshtest"
+	"github.com/coreos/dex/session/manager"
+	"github.com/coreos/dex/user"
 )
 
 var validRedirURL = url.URL{
@@ -266,6 +267,12 @@ func TestServerLoginDisabledUser(t *testing.T) {
 }
 
 func TestServerCodeToken(t *testing.T) {
+	f, err := makeTestFixtures()
+	if err != nil {
+		t.Fatalf("Error creating test fixtures: %v", err)
+	}
+	sm := f.sessionManager
+
 	tests := []struct {
 		scope        []string
 		refreshToken string
@@ -277,21 +284,14 @@ func TestServerCodeToken(t *testing.T) {
 		},
 		// Have 'offline_access' in scope, should get non-empty refresh token.
 		{
-			// NOTE(ericchiang): This test assumes that the database ID of the first
+			// NOTE(ericchiang): This test assumes that the database ID of the
-			// refresh token will be "1".
+			// first refresh token will be "1".
 			scope:        []string{"openid", "offline_access"},
 			refreshToken: fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))),
 		},
 	}
 
 	for i, tt := range tests {
-		f, err := makeTestFixtures()
-		if err != nil {
-			t.Fatalf("error making test fixtures: %v", err)
-		}
-		f.srv.RefreshTokenRepo = refreshtest.NewTestRefreshTokenRepo()
-
-		sm := f.sessionManager
 		sessionID, err := sm.NewSession("bogus_idpc", testClientID, "bogus", url.URL{}, "", false, tt.scope)
 		if err != nil {
 			t.Fatalf("case %d: unexpected error: %v", i, err)
@@ -311,11 +311,9 @@ func TestServerCodeToken(t *testing.T) {
 			t.Fatalf("case %d: unexpected error: %v", i, err)
 		}
 
-		jwt, token, err := f.srv.CodeToken(
+		jwt, token, err := f.srv.CodeToken(oidc.ClientCredentials{
-			oidc.ClientCredentials{
 			ID:     testClientID,
-				Secret: clientTestSecret,
+			Secret: clientTestSecret}, key)
-			}, key)
 		if err != nil {
 			t.Fatalf("case %d: unexpected error: %v", i, err)
 		}

server/testutil.go

@@ -14,6 +14,7 @@ import (
 	"github.com/coreos/dex/connector"
 	"github.com/coreos/dex/db"
 	"github.com/coreos/dex/email"
+	"github.com/coreos/dex/refresh/refreshtest"
 	sessionmanager "github.com/coreos/dex/session/manager"
 	"github.com/coreos/dex/user"
 	useremail "github.com/coreos/dex/user/email"
@@ -83,6 +84,11 @@ var (
 	}
 
 	testPrivKey, _ = key.GeneratePrivateKey()
+
+	testClientCreds = oidc.ClientCredentials{
+		ID:     testClientID,
+		Secret: base64.URLEncoding.EncodeToString([]byte("secret")),
+	}
 )
 
 type testFixtures struct {
@@ -93,6 +99,7 @@ type testFixtures struct {
 	redirectURL    url.URL
 	clientRepo     client.ClientRepo
 	clientManager  *clientmanager.ClientManager
+	clientCreds    map[string]oidc.ClientCredentials
 }
 
 type testFixtureOptions struct {
@@ -150,6 +157,8 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 	sessionManager := sessionmanager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB()))
 	sessionManager.GenerateCode = sequentialGenerateCodeFunc()
 
+	refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo()
+
 	emailer, err := email.NewTemplatizedEmailerFromGlobs(
 		emailTemplatesLocation+"/*.txt",
 		emailTemplatesLocation+"/*.html",
@@ -210,6 +219,7 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 		UserManager:      userManager,
 		ClientManager:    clientManager,
 		KeyManager:       km,
+		RefreshTokenRepo: refreshTokenRepo,
 	}
 
 	err = setTemplates(srv, tpl)
@@ -243,5 +253,8 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 		emailer:        emailer,
 		clientRepo:     clientRepo,
 		clientManager:  clientManager,
+		clientCreds: map[string]oidc.ClientCredentials{
+			testClientID: testClientCreds,
+		},
 	}, nil
 }

session/session.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/coreos/go-oidc/jose"
 	"github.com/coreos/go-oidc/oidc"
+
+	"github.com/coreos/dex/scope"
 )
 
 const (
@@ -46,11 +48,13 @@ type Session struct {
 	// Regsiter indicates that this session is a registration flow.
 	Register bool
 
-	// Nonce is optionally provided in the initial authorization request, and propogated in such cases to the generated claims.
+	// Nonce is optionally provided in the initial authorization request, and
+	// propogated in such cases to the generated claims.
 	Nonce string
 
-	// Scope is the 'scope' field in the authentication request. Example scopes are 'openid', 'email', 'offline', etc.
+	// Scope is the 'scope' field in the authentication request. Example scopes
-	Scope []string
+	// are 'openid', 'email', 'offline', etc.
+	Scope scope.Scopes
 }
 
 // Claims returns a new set of Claims for the current session.