Finish TestTokenExchange unit tests and add missing scope check.

Signed-off-by: Margo Crawford <margaretc@vmware.com>

internal/oidc/token/token_handler_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/square/go-jose.v2"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/kubernetes/fake"
 	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -569,14 +570,16 @@ func TestTokenExchange(t *testing.T) {
 	tests := []struct {
 		name string
 
-		authcodeExchange           authcodeExchangeInputs
+		authcodeExchange    authcodeExchangeInputs
-		modifyTokenExchangeRequest func(r *http.Request)
+		modifyRequestParams func(t *testing.T, params url.Values)
-		requestedAudience          string
+		modifyStorage       func(t *testing.T, storage *oidc.KubeStorage, pendingRequest *http.Request)
+		requestedAudience   string
 
-		wantStatus int
+		wantStatus               int
+		wantResponseBodyContains string
 	}{
 		{
-			name: "token exchange happy path",
+			name: "happy path",
 			authcodeExchange: authcodeExchangeInputs{
 				modifyAuthRequest: func(authRequest *http.Request) {
 					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
@@ -589,37 +592,226 @@ func TestTokenExchange(t *testing.T) {
 			requestedAudience: "some-workload-cluster",
 			wantStatus:        http.StatusOK,
 		},
-		// TODO add the unhappy path table entries: wrong client id, invalid access token, access token does not have pinniped.sts.unrestricted, etc.
+		{
-		//  Can use modifyAuthRequest to change the request params for the original authorize request (e.g. don't request pinniped.sts.unrestricted).
+			name: "missing audience",
-		//  Can use modifyTokenExchangeRequest to change to request params for the token exchange call (e.g. bad access token or wrong http verb).
+			authcodeExchange: authcodeExchangeInputs{
-		//  Will need to add some more fields to the test table to declare the expected values of the error response, etc.
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience:        "",
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: "missing audience parameter",
+		},
+		{
+			name: "missing subject_token",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyRequestParams: func(t *testing.T, params url.Values) {
+				params.Del("subject_token")
+			},
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: "missing subject_token parameter",
+		},
+		{
+			name: "wrong subject_token_type",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyRequestParams: func(t *testing.T, params url.Values) {
+				params.Set("subject_token_type", "invalid")
+			},
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: `unsupported subject_token_type parameter value`,
+		},
+		{
+			name: "wrong requested_token_type",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyRequestParams: func(t *testing.T, params url.Values) {
+				params.Set("requested_token_type", "invalid")
+			},
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: `unsupported requested_token_type parameter value`,
+		},
+		{
+			name: "unsupported RFC8693 parameter",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyRequestParams: func(t *testing.T, params url.Values) {
+				params.Set("resource", "some-resource-parameter-value")
+			},
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: `unsupported parameter resource`,
+		},
+		{
+			name: "bogus access token",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyRequestParams: func(t *testing.T, params url.Values) {
+				params.Set("subject_token", "some-bogus-value")
+			},
+			wantStatus:               http.StatusBadRequest,
+			wantResponseBodyContains: `Invalid token format`,
+		},
+		{
+			name: "valid access token, but deleted from storage",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience: "some-workload-cluster",
+			modifyStorage: func(t *testing.T, storage *oidc.KubeStorage, pendingRequest *http.Request) {
+				parts := strings.Split(pendingRequest.Form.Get("subject_token"), ".")
+				require.Len(t, parts, 2)
+				require.NoError(t, storage.DeleteAccessTokenSession(context.Background(), parts[1]))
+			},
+			wantStatus:               http.StatusUnauthorized,
+			wantResponseBodyContains: `invalid subject_token`,
+		},
+		{
+			name: "access token missing required scopes",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid")
+				},
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid"},
+				wantGrantedScopes:     []string{"openid"},
+			},
+			requestedAudience:        "some-workload-cluster",
+			wantStatus:               http.StatusForbidden,
+			wantResponseBodyContains: `missing the \"pinniped.sts.unrestricted\" scope`,
+		},
+		{
+			name: "token minting failure",
+			authcodeExchange: authcodeExchangeInputs{
+				modifyAuthRequest: func(authRequest *http.Request) {
+					authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted")
+				},
+				// Fail to fetch a JWK signing key after the authcode exchange has happened.
+				makeOathHelper:        makeOauthHelperWithJWTKeyThatWorksOnlyOnce,
+				wantStatus:            http.StatusOK,
+				wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"},
+				wantRequestedScopes:   []string{"openid", "pinniped.sts.unrestricted"},
+				wantGrantedScopes:     []string{"openid", "pinniped.sts.unrestricted"},
+			},
+			requestedAudience:        "some-workload-cluster",
+			wantStatus:               http.StatusServiceUnavailable,
+			wantResponseBodyContains: `The authorization server is currently unable to handle the request`,
+		},
 	}
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			subject, rsp, _, _, _ := exchangeAuthcodeForTokens(t, test.authcodeExchange)
+			subject, rsp, _, secrets, storage := exchangeAuthcodeForTokens(t, test.authcodeExchange)
 			var parsedResponseBody map[string]interface{}
 			require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody))
 
 			request := happyTokenExchangeRequest(test.requestedAudience, parsedResponseBody["access_token"].(string))
+			if test.modifyStorage != nil {
+				test.modifyStorage(t, storage, request)
+			}
+			if test.modifyRequestParams != nil {
+				test.modifyRequestParams(t, request.Form)
+			}
 
 			req := httptest.NewRequest("POST", "/path/shouldn't/matter", body(request.Form).ReadCloser())
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-			if test.modifyTokenExchangeRequest != nil {
-				test.modifyTokenExchangeRequest(req)
-			}
 			rsp = httptest.NewRecorder()
 
+			// Measure the secrets in storage after the auth code flow.
+			existingSecrets, err := secrets.List(context.Background(), metav1.ListOptions{})
+			require.NoError(t, err)
+
 			subject.ServeHTTP(rsp, req)
 			t.Logf("response: %#v", rsp)
 			t.Logf("response body: %q", rsp.Body.String())
 
 			require.Equal(t, test.wantStatus, rsp.Code)
 			testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), "application/json")
+			if test.wantResponseBodyContains != "" {
+				require.Contains(t, rsp.Body.String(), test.wantResponseBodyContains)
+			}
 
-			// TODO write lots more assertions about the happy path
+			// The remaining assertions apply only the the happy path.
-			//  e.g. that nothing was changed in the fosite k8s storage
+			if rsp.Code != http.StatusOK {
-			//  e.g. that the response body is correct, and the decoded token has the right claims
+				return
+			}
+
+			var responseBody map[string]interface{}
+			require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &responseBody))
+
+			require.Contains(t, responseBody, "access_token")
+			require.Equal(t, "N_A", responseBody["token_type"])
+			require.Equal(t, "urn:ietf:params:oauth:token-type:jwt", responseBody["issued_token_type"])
+
+			// Assert that the returned token has expected claims.
+			parsedJWT, err := jose.ParseSigned(responseBody["access_token"].(string))
+			require.NoError(t, err)
+			var tokenClaims map[string]interface{}
+			require.NoError(t, json.Unmarshal(parsedJWT.UnsafePayloadWithoutVerification(), &tokenClaims))
+			require.Contains(t, tokenClaims, "iat")
+			require.Contains(t, tokenClaims, "rat")
+			require.Contains(t, tokenClaims, "jti")
+			require.Len(t, tokenClaims["aud"], 1)
+			require.Contains(t, tokenClaims["aud"], test.requestedAudience)
+			require.Equal(t, goodSubject, tokenClaims["sub"])
+			require.Equal(t, goodIssuer, tokenClaims["iss"])
+
+			// Assert that nothing in storage has been modified.
+			newSecrets, err := secrets.List(context.Background(), metav1.ListOptions{})
+			require.NoError(t, err)
+			require.ElementsMatch(t, existingSecrets.Items, newSecrets.Items)
 		})
 	}
 }
@@ -795,6 +987,38 @@ func makeHappyOauthHelper(
 	return oauthHelper, authResponder.GetCode(), jwtSigningKey
 }
 
+type singleUseJWKProvider struct {
+	jwks.DynamicJWKSProvider
+	calls int
+}
+
+func (s *singleUseJWKProvider) GetJWKS(issuerName string) (jwks *jose.JSONWebKeySet, activeJWK *jose.JSONWebKey) {
+	s.calls += 1
+	if s.calls > 1 {
+		return nil, nil
+	}
+	return s.DynamicJWKSProvider.GetJWKS(issuerName)
+}
+
+func makeOauthHelperWithJWTKeyThatWorksOnlyOnce(
+	t *testing.T,
+	authRequest *http.Request,
+	store interface {
+		oauth2.TokenRevocationStorage
+		oauth2.CoreStorage
+		openid.OpenIDConnectRequestStorage
+		pkce.PKCERequestStorage
+		fosite.ClientManager
+	},
+) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) {
+	t.Helper()
+
+	jwtSigningKey, jwkProvider := generateJWTSigningKeyAndJWKSProvider(t, goodIssuer)
+	oauthHelper := oidc.FositeOauth2Helper(store, goodIssuer, []byte(hmacSecret), &singleUseJWKProvider{DynamicJWKSProvider: jwkProvider})
+	authResponder := simulateAuthEndpointHavingAlreadyRun(t, authRequest, oauthHelper)
+	return oauthHelper, authResponder.GetCode(), jwtSigningKey
+}
+
 func makeOauthHelperWithNilPrivateJWTSigningKey(
 	t *testing.T,
 	authRequest *http.Request,

internal/oidc/token_exchange.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"net/url"
 
+	"github.com/coreos/go-oidc"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
 	"github.com/ory/fosite/handler/oauth2"
@@ -15,8 +16,9 @@ import (
 )
 
 const (
-	tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" //nolint: gosec
+	tokenTypeAccessToken       = "urn:ietf:params:oauth:token-type:access_token" //nolint: gosec
-	tokenTypeJWT         = "urn:ietf:params:oauth:token-type:jwt"          //nolint: gosec
+	tokenTypeJWT               = "urn:ietf:params:oauth:token-type:jwt"          //nolint: gosec
+	pinnipedTokenExchangeScope = "pinniped.sts.unrestricted"                     //nolint: gosec
 )
 
 type stsParams struct {
@@ -63,6 +65,11 @@ func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context
 		return errors.WithStack(err)
 	}
 
+	// Require that the incoming access token has the STS and OpenID scopes .
+	if !originalRequester.GetGrantedScopes().Has(pinnipedTokenExchangeScope, oidc.ScopeOpenID) {
+		return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", pinnipedTokenExchangeScope))
+	}
+
 	// Use the original authorize request information, along with the requested audience, to mint a new JWT.
 	responseToken, err := t.mintJWT(ctx, originalRequester, params.requestedAudience)
 	if err != nil {
@@ -72,7 +79,7 @@ func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context
 	// Format the response parameters according to RFC8693.
 	responder.SetAccessToken(responseToken)
 	responder.SetTokenType("N_A")
-	responder.SetExtra("issued_token_type", "urn:ietf:params:oauth:token-type:jwt")
+	responder.SetExtra("issued_token_type", tokenTypeJWT)
 	return nil
 }
 
@@ -88,19 +95,19 @@ func (t *TokenExchangeHandler) validateParams(params url.Values) (*stsParams, er
 	// Validate some required parameters.
 	result.requestedAudience = params.Get("audience")
 	if result.requestedAudience == "" {
-		return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "missing audience parameter")
+		return nil, fosite.ErrInvalidRequest.WithHint("missing audience parameter")
 	}
 	result.subjectAccessToken = params.Get("subject_token")
 	if result.subjectAccessToken == "" {
-		return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "missing subject_token parameter")
+		return nil, fosite.ErrInvalidRequest.WithHint("missing subject_token parameter")
 	}
 
 	// Validate some parameters with hardcoded values we support.
 	if params.Get("subject_token_type") != tokenTypeAccessToken {
-		return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "unsupported subject_token_type parameter value, must be %q", tokenTypeAccessToken)
+		return nil, fosite.ErrInvalidRequest.WithHintf("unsupported subject_token_type parameter value, must be %q", tokenTypeAccessToken)
 	}
 	if params.Get("requested_token_type") != tokenTypeJWT {
-		return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "unsupported requested_token_type parameter value, must be %q", tokenTypeJWT)
+		return nil, fosite.ErrInvalidRequest.WithHintf("unsupported requested_token_type parameter value, must be %q", tokenTypeJWT)
 	}
 
 	// Validate that none of these unsupported parameters were sent. These are optional and we do not currently support them.
@@ -111,7 +118,7 @@ func (t *TokenExchangeHandler) validateParams(params url.Values) (*stsParams, er
 		"actor_token_type",
 	} {
 		if params.Get(param) != "" {
-			return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "unsupported parameter %s", param)
+			return nil, fosite.ErrInvalidRequest.WithHintf("unsupported parameter %s", param)
 		}
 	}
 
@@ -125,7 +132,7 @@ func (t *TokenExchangeHandler) validateAccessToken(ctx context.Context, requeste
 	signature := t.accessTokenStrategy.AccessTokenSignature(accessToken)
 	originalRequester, err := t.accessTokenStorage.GetAccessTokenSession(ctx, signature, requester.GetSession())
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return nil, fosite.ErrRequestUnauthorized.WithCause(err).WithHint("invalid subject_token")
 	}
 	return originalRequester, nil
 }