diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 966bfb54..1998b72a 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -9,6 +9,7 @@ import ( "net/http" "time" + coreosoidc "github.com/coreos/go-oidc" "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" @@ -57,7 +58,10 @@ func NewHandler( } // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. - grantOpenIDScopeIfRequested(authorizeRequester) + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) + // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope + // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) now := time.Now() _, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{ @@ -148,14 +152,6 @@ func readCSRFCookie(r *http.Request, codec oidc.Codec) csrftoken.CSRFToken { return csrfFromCookie } -func grantOpenIDScopeIfRequested(authorizeRequester fosite.AuthorizeRequester) { - for _, scope := range authorizeRequester.GetRequestedScopes() { - if scope == "openid" { - authorizeRequester.GrantScope(scope) - } - } -} - func chooseUpstreamIDP(idpListGetter oidc.IDPListGetter) (provider.UpstreamOIDCIdentityProviderI, error) { allUpstreamIDPs := idpListGetter.GetIDPList() if len(allUpstreamIDPs) == 0 { diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 30a8dc5d..0175c64c 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -119,7 +119,7 @@ func TestAuthorizationEndpoint(t *testing.T) { Name: "some-idp", ClientID: "some-client-id", AuthorizationURL: *upstreamAuthURL, - Scopes: []string{"scope1", "scope2"}, + Scopes: []string{"scope1", "scope2"}, // the scopes to request when starting the upstream authorization flow } // Configure fosite the same way that the production code would, using NullStorage to turn off storage. @@ -372,6 +372,26 @@ func TestAuthorizationEndpoint(t *testing.T) { wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, + { + name: "happy path when downstream requested scopes include offline_access", + issuer: downstreamIssuer, + idpListGetter: oidctestutil.NewIDPListGetter(&upstreamOIDCIdentityProvider), + generateCSRF: happyCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid offline_access"}), + wantStatus: http.StatusFound, + wantContentType: "text/html; charset=utf-8", + wantCSRFValueInCookieHeader: happyCSRF, + wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{ + "scope": "openid offline_access", + }, "", "")), + wantUpstreamStateParamInLocationHeader: true, + wantBodyStringWithLocationInHref: true, + }, { name: "downstream redirect uri does not match what is configured for client", issuer: downstreamIssuer, diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index e102dc62..3c85ee4c 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -11,6 +11,7 @@ import ( "net/url" "time" + coreosoidc "github.com/coreos/go-oidc" "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" @@ -70,8 +71,9 @@ func NewHandler( return httperr.New(http.StatusBadRequest, "error using state downstream auth params") } - // Grant the openid scope only if it was requested. - grantOpenIDScopeIfRequested(authorizeRequester) + // Automatically grant the openid and offline_access scopes, but only if they were requested. + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens( r.Context(), @@ -189,14 +191,6 @@ func readState(r *http.Request, stateDecoder oidc.Decoder) (*oidc.UpstreamStateP return &state, nil } -func grantOpenIDScopeIfRequested(authorizeRequester fosite.AuthorizeRequester) { - for _, scope := range authorizeRequester.GetRequestedScopes() { - if scope == "openid" { - authorizeRequester.GrantScope(scope) - } - } -} - func getUsernameFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index ed4c6b01..67857f6a 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -66,7 +66,8 @@ const ( var ( upstreamGroupMembership = []string{"test-pinniped-group-0", "test-pinniped-group-1"} - happyDownstreamScopesRequested = []string{"openid", "profile", "email"} + happyDownstreamScopesRequested = []string{"openid"} + happyDownstreamScopesGranted = []string{"openid"} happyDownstreamRequestParamsQuery = url.Values{ "response_type": []string{"code"}, @@ -127,7 +128,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus int wantBody string wantRedirectLocationRegexp string - wantGrantedOpenidScope bool + wantDownstreamGrantedScopes []string wantDownstreamIDTokenSubject string wantDownstreamIDTokenGroups []string wantDownstreamRequestedScopes []string @@ -145,11 +146,11 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, - wantGrantedOpenidScope: true, wantBody: "", wantDownstreamIDTokenSubject: upstreamUsername, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, @@ -163,11 +164,11 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, - wantGrantedOpenidScope: true, wantBody: "", wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, wantDownstreamIDTokenGroups: nil, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, @@ -181,11 +182,11 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, - wantGrantedOpenidScope: true, wantBody: "", wantDownstreamIDTokenSubject: upstreamSubject, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, @@ -316,6 +317,28 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, + { + name: "state's downstream auth params also included offline_access scope", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath(). + WithState( + happyUpstreamStateParam(). + WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access"}).Encode()). + Build(t, happyStateCodec), + ).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid%20offline_access&state=` + happyDownstreamState, + wantDownstreamIDTokenSubject: upstreamUsername, + wantDownstreamRequestedScopes: []string{"openid", "offline_access"}, + wantDownstreamGrantedScopes: []string{"openid", "offline_access"}, + wantDownstreamIDTokenGroups: upstreamGroupMembership, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, { name: "the UpstreamOIDCProvider CRD has been deleted", idp: otherUpstreamOIDCIdentityProvider, @@ -481,7 +504,7 @@ func TestCallbackEndpoint(t *testing.T) { // Several Secrets should have been created expectedNumberOfCreatedSecrets := 2 - if test.wantGrantedOpenidScope { + if includesOpenIDScope(test.wantDownstreamGrantedScopes) { expectedNumberOfCreatedSecrets++ } require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets) @@ -493,7 +516,7 @@ func TestCallbackEndpoint(t *testing.T) { t, oauthStore, authcodeDataAndSignature[1], // Authcode store key is authcode signature - test.wantGrantedOpenidScope, + test.wantDownstreamGrantedScopes, test.wantDownstreamIDTokenSubject, test.wantDownstreamIDTokenGroups, test.wantDownstreamRequestedScopes, @@ -513,7 +536,7 @@ func TestCallbackEndpoint(t *testing.T) { ) // One IDSession should have been stored, if the downstream actually requested the "openid" scope - if test.wantGrantedOpenidScope { + if includesOpenIDScope(test.wantDownstreamGrantedScopes) { testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) validateIDSessionStorage( @@ -530,6 +553,15 @@ func TestCallbackEndpoint(t *testing.T) { } } +func includesOpenIDScope(scopes []string) bool { + for _, scope := range scopes { + if scope == "openid" { + return true + } + } + return false +} + type requestPath struct { code, state *string } @@ -704,7 +736,7 @@ func validateAuthcodeStorage( t *testing.T, oauthStore *oidc.KubeStorage, storeKey string, - wantGrantedOpenidScope bool, + wantDownstreamGrantedScopes []string, wantDownstreamIDTokenSubject string, wantDownstreamIDTokenGroups []string, wantDownstreamRequestedScopes []string, @@ -719,11 +751,7 @@ func validateAuthcodeStorage( storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode) // Check which scopes were granted. - if wantGrantedOpenidScope { - require.Contains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid") - } else { - require.NotContains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid") - } + require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes()) // Check all the other fields of the stored request. require.NotEmpty(t, storedRequestFromAuthcode.ID) diff --git a/internal/oidc/kube_storage.go b/internal/oidc/kube_storage.go index 874e3164..b4329722 100644 --- a/internal/oidc/kube_storage.go +++ b/internal/oidc/kube_storage.go @@ -41,73 +41,147 @@ func NewKubeStorage(secrets corev1client.SecretInterface) *KubeStorage { } } -func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) error { - return k.refreshTokenStorage.RevokeRefreshToken(ctx, requestID) +// +// Authorization Code sessions: +// +// These are keyed by the signature of the authcode. +// +// Fosite will create these in the authorize endpoint. +// +// Fosite will never delete them. Instead, it wants to mark them as invalidated once the authcode is used to redeem tokens. +// That way, it can later detect the case where an authcode that was already redeemed gets used again. +// + +func (k KubeStorage) CreateAuthorizeCodeSession(ctx context.Context, signatureOfAuthcode string, r fosite.Requester) (err error) { + return k.authorizationCodeStorage.CreateAuthorizeCodeSession(ctx, signatureOfAuthcode, r) +} + +func (k KubeStorage) GetAuthorizeCodeSession(ctx context.Context, signatureOfAuthcode string, s fosite.Session) (request fosite.Requester, err error) { + return k.authorizationCodeStorage.GetAuthorizeCodeSession(ctx, signatureOfAuthcode, s) +} + +func (k KubeStorage) InvalidateAuthorizeCodeSession(ctx context.Context, signatureOfAuthcode string) (err error) { + return k.authorizationCodeStorage.InvalidateAuthorizeCodeSession(ctx, signatureOfAuthcode) +} + +// +// PKCE sessions: +// +// These are keyed by the signature of the authcode. +// +// Fosite will create these in the authorize endpoint at the same time that it is creating an authcode. +// +// Fosite will delete these in the token endpoint during authcode redemption since they are no longer needed after that. +// If the user chooses to never redeem their authcode, then fosite will never delete these. +// + +func (k KubeStorage) CreatePKCERequestSession(ctx context.Context, signatureOfAuthcode string, requester fosite.Requester) error { + return k.pkceStorage.CreatePKCERequestSession(ctx, signatureOfAuthcode, requester) +} + +func (k KubeStorage) GetPKCERequestSession(ctx context.Context, signatureOfAuthcode string, session fosite.Session) (fosite.Requester, error) { + return k.pkceStorage.GetPKCERequestSession(ctx, signatureOfAuthcode, session) +} + +func (k KubeStorage) DeletePKCERequestSession(ctx context.Context, signatureOfAuthcode string) error { + return k.pkceStorage.DeletePKCERequestSession(ctx, signatureOfAuthcode) +} + +// +// OpenID Connect sessions: +// +// These are keyed by the full value of the authcode (not just the signature). +// +// Fosite will create these in the authorize endpoint when it creates an authcode, but only if the user +// requested the openid scope. +// +// Fosite will never delete these, which is likely a bug in fosite. Although there is a delete method below, fosite +// never calls it. Used during authcode redemption, they will never be accessed again after a successful authcode +// redemption. Although that implies that they should probably follow a lifecycle similar the the PKCE storage, they +// are, in fact, not deleted. +// + +func (k KubeStorage) CreateOpenIDConnectSession(ctx context.Context, fullAuthcode string, requester fosite.Requester) error { + return k.oidcStorage.CreateOpenIDConnectSession(ctx, fullAuthcode, requester) +} + +func (k KubeStorage) GetOpenIDConnectSession(ctx context.Context, fullAuthcode string, requester fosite.Requester) (fosite.Requester, error) { + return k.oidcStorage.GetOpenIDConnectSession(ctx, fullAuthcode, requester) +} + +func (k KubeStorage) DeleteOpenIDConnectSession(ctx context.Context, fullAuthcode string) error { + return k.oidcStorage.DeleteOpenIDConnectSession(ctx, fullAuthcode) +} + +// +// Access token sessions: +// +// These are keyed by the signature of the access token. +// +// Fosite will create these in the token endpoint whenever it wants to hand out an access token, including the original +// authcode redemption and also during refresh. +// +// Fosite will not use the delete method. Instead, it will use the revoke method to delete them. +// During a refresh in the token endpoint, the old access token is revoked just before the new access token is created. +// Also, if the token endpoint receives an authcode that was already used successfully, then it revokes the access token +// that was previously handed out for that authcode. If a user stops coming back to refresh their tokens, then that +// access token will never be deleted. +// + +func (k KubeStorage) CreateAccessTokenSession(ctx context.Context, signatureOfAccessToken string, requester fosite.Requester) (err error) { + return k.accessTokenStorage.CreateAccessTokenSession(ctx, signatureOfAccessToken, requester) +} + +func (k KubeStorage) GetAccessTokenSession(ctx context.Context, signatureOfAccessToken string, session fosite.Session) (request fosite.Requester, err error) { + return k.accessTokenStorage.GetAccessTokenSession(ctx, signatureOfAccessToken, session) +} + +func (k KubeStorage) DeleteAccessTokenSession(ctx context.Context, signatureOfAccessToken string) (err error) { + return k.accessTokenStorage.DeleteAccessTokenSession(ctx, signatureOfAccessToken) } func (k KubeStorage) RevokeAccessToken(ctx context.Context, requestID string) error { return k.accessTokenStorage.RevokeAccessToken(ctx, requestID) } -func (k KubeStorage) CreateRefreshTokenSession(ctx context.Context, signature string, request fosite.Requester) (err error) { - return k.refreshTokenStorage.CreateRefreshTokenSession(ctx, signature, request) +// +// Refresh token sessions: +// +// These are keyed by the signature of the refresh token. +// +// Fosite will create these in the token endpoint whenever it wants to hand out an refresh token, including the original +// authcode redemption and also during refresh. Refresh tokens are only handed out when the user requested the +// offline_access scope on the original authorization request. +// +// Fosite will not use the delete method. Instead, it will use the revoke method to delete them. +// During a refresh in the token endpoint, the old refresh token is revoked just before the new refresh token is created. +// Also, if the token endpoint receives an authcode that was already used successfully, then it revokes the refresh token +// that was previously handed out for that authcode. If a user stops coming back to refresh their tokens, then that +// refresh token will never be deleted. +// + +func (k KubeStorage) CreateRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, request fosite.Requester) (err error) { + return k.refreshTokenStorage.CreateRefreshTokenSession(ctx, signatureOfRefreshToken, request) } -func (k KubeStorage) GetRefreshTokenSession(ctx context.Context, signature string, session fosite.Session) (request fosite.Requester, err error) { - return k.refreshTokenStorage.GetRefreshTokenSession(ctx, signature, session) +func (k KubeStorage) GetRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, session fosite.Session) (request fosite.Requester, err error) { + return k.refreshTokenStorage.GetRefreshTokenSession(ctx, signatureOfRefreshToken, session) } -func (k KubeStorage) DeleteRefreshTokenSession(ctx context.Context, signature string) (err error) { - return k.refreshTokenStorage.DeleteRefreshTokenSession(ctx, signature) +func (k KubeStorage) DeleteRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string) (err error) { + return k.refreshTokenStorage.DeleteRefreshTokenSession(ctx, signatureOfRefreshToken) } -func (k KubeStorage) CreateAccessTokenSession(ctx context.Context, signature string, requester fosite.Requester) (err error) { - return k.accessTokenStorage.CreateAccessTokenSession(ctx, signature, requester) +func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) error { + return k.refreshTokenStorage.RevokeRefreshToken(ctx, requestID) } -func (k KubeStorage) GetAccessTokenSession(ctx context.Context, signature string, session fosite.Session) (request fosite.Requester, err error) { - return k.accessTokenStorage.GetAccessTokenSession(ctx, signature, session) -} - -func (k KubeStorage) DeleteAccessTokenSession(ctx context.Context, signature string) (err error) { - return k.accessTokenStorage.DeleteAccessTokenSession(ctx, signature) -} - -func (k KubeStorage) CreateOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) error { - return k.oidcStorage.CreateOpenIDConnectSession(ctx, authcode, requester) -} - -func (k KubeStorage) GetOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) (fosite.Requester, error) { - return k.oidcStorage.GetOpenIDConnectSession(ctx, authcode, requester) -} - -func (k KubeStorage) DeleteOpenIDConnectSession(ctx context.Context, authcode string) error { - return k.oidcStorage.DeleteOpenIDConnectSession(ctx, authcode) -} - -func (k KubeStorage) GetPKCERequestSession(ctx context.Context, signature string, session fosite.Session) (fosite.Requester, error) { - return k.pkceStorage.GetPKCERequestSession(ctx, signature, session) -} - -func (k KubeStorage) CreatePKCERequestSession(ctx context.Context, signature string, requester fosite.Requester) error { - return k.pkceStorage.CreatePKCERequestSession(ctx, signature, requester) -} - -func (k KubeStorage) DeletePKCERequestSession(ctx context.Context, signature string) error { - return k.pkceStorage.DeletePKCERequestSession(ctx, signature) -} - -func (k KubeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, r fosite.Requester) (err error) { - return k.authorizationCodeStorage.CreateAuthorizeCodeSession(ctx, signature, r) -} - -func (k KubeStorage) GetAuthorizeCodeSession(ctx context.Context, signature string, s fosite.Session) (request fosite.Requester, err error) { - return k.authorizationCodeStorage.GetAuthorizeCodeSession(ctx, signature, s) -} - -func (k KubeStorage) InvalidateAuthorizeCodeSession(ctx context.Context, signature string) (err error) { - return k.authorizationCodeStorage.InvalidateAuthorizeCodeSession(ctx, signature) -} +// +// OAuth client definitions: +// +// For the time being, we only allow a single pre-defined client, so we do not need to interact with any underlying +// storage mechanism to fetch them. +// func (KubeStorage) GetClient(_ context.Context, id string) (fosite.Client, error) { client := PinnipedCLIOIDCClient() @@ -117,6 +191,10 @@ func (KubeStorage) GetClient(_ context.Context, id string) (fosite.Client, error return nil, fosite.ErrNotFound } +// +// Unused interface methods. +// + func (KubeStorage) ClientAssertionJWTValid(_ context.Context, _ string) error { return errKubeStorageNotImplemented } diff --git a/internal/oidc/nullstorage_test.go b/internal/oidc/nullstorage_test.go index 2983a800..4adbe807 100644 --- a/internal/oidc/nullstorage_test.go +++ b/internal/oidc/nullstorage_test.go @@ -27,8 +27,8 @@ func TestNullStorage_GetClient(t *testing.T) { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code"}, - Scopes: []string{"openid", "profile", "email"}, + GrantTypes: []string{"authorization_code", "refresh_token"}, + Scopes: []string{"openid", "offline_access", "profile", "email"}, }, TokenEndpointAuthMethod: "none", }, diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 7c550751..96c5e72d 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -7,6 +7,7 @@ package oidc import ( "time" + coreosoidc "github.com/coreos/go-oidc" "github.com/ory/fosite" "github.com/ory/fosite/compose" @@ -83,8 +84,8 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code"}, - Scopes: []string{"openid", "profile", "email"}, + GrantTypes: []string{"authorization_code", "refresh_token"}, + Scopes: []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email"}, }, TokenEndpointAuthMethod: "none", } @@ -111,8 +112,9 @@ func FositeOauth2Helper( EnforcePKCE: true, // follow current set of best practices and always require PKCE AllowedPromptValues: []string{"none"}, // TODO unclear what we should set here - RefreshTokenScopes: nil, // TODO decide what makes sense when we add refresh token support - MinParameterEntropy: 32, // 256 bits seems about right + RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess}, // as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess + + MinParameterEntropy: 32, // 256 bits seems about right } return compose.Compose( @@ -125,9 +127,9 @@ func FositeOauth2Helper( }, nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. compose.OAuth2AuthorizeExplicitFactory, - // compose.OAuth2RefreshTokenGrantFactory, + compose.OAuth2RefreshTokenGrantFactory, compose.OpenIDConnectExplicitFactory, - // compose.OpenIDConnectRefreshFactory, + compose.OpenIDConnectRefreshFactory, compose.OAuth2PKCEFactory, ) } @@ -156,3 +158,11 @@ func FositeErrorForLog(err error) []interface{} { type IDPListGetter interface { GetIDPList() []provider.UpstreamOIDCIdentityProviderI } + +func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) { + for _, scope := range authorizeRequester.GetRequestedScopes() { + if scope == scopeName { + authorizeRequester.GrantScope(scope) + } + } +} diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 7a3e2ffe..cd2d0d28 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -8,6 +8,8 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "crypto/sha256" + "encoding/base64" "encoding/json" "io" "io/ioutil" @@ -26,8 +28,10 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" + josejwt "gopkg.in/square/go-jose.v2/jwt" "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes/fake" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage/accesstoken" @@ -116,6 +120,16 @@ var ( } `) + fositeInvalidRequestMissingGrantTypeErrorBody = here.Doc(` + { + "error": "invalid_request", + "error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nRequest parameter \"grant_type\"\" is missing", + "error_hint": "Request parameter \"grant_type\"\" is missing", + "error_verbose": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed", + "status_code": 400 + } + `) + fositeMissingClientErrorBody = here.Doc(` { "error": "invalid_request", @@ -192,10 +206,8 @@ var ( "status_code": 503 } `) -) -func TestTokenEndpoint(t *testing.T) { - happyAuthRequest := &http.Request{ + happyAuthRequest = &http.Request{ Form: url.Values{ "response_type": {"code"}, "scope": {"openid profile email"}, @@ -207,362 +219,822 @@ func TestTokenEndpoint(t *testing.T) { "redirect_uri": {goodRedirectURI}, }, } +) +type tokenEndpointResponseExpectedValues struct { + wantStatus int + wantSuccessBodyFields []string + wantErrorResponseBody string + wantRequestedScopes []string + wantGrantedScopes []string +} + +type authcodeExchangeInputs struct { + modifyAuthRequest func(authRequest *http.Request) + modifyTokenRequest func(tokenRequest *http.Request, authCode string) + modifyStorage func( + t *testing.T, + s interface { + oauth2.TokenRevocationStorage + oauth2.CoreStorage + openid.OpenIDConnectRequestStorage + pkce.PKCERequestStorage + fosite.ClientManager + }, + authCode string, + ) + makeOathHelper func( + t *testing.T, + authRequest *http.Request, + store interface { + oauth2.TokenRevocationStorage + oauth2.CoreStorage + openid.OpenIDConnectRequestStorage + pkce.PKCERequestStorage + fosite.ClientManager + }, + ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) + + want tokenEndpointResponseExpectedValues +} + +func TestTokenEndpoint(t *testing.T) { tests := []struct { - name string - - authRequest func(authRequest *http.Request) - storage func( - t *testing.T, - s interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, - authCode string, - ) - request func(r *http.Request, authCode string) - makeOathHelper func( - t *testing.T, - authRequest *http.Request, - store interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, - ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) - - wantStatus int - wantBodyFields []string - wantExactBody string + name string + authcodeExchange authcodeExchangeInputs }{ // happy path { - name: "request is valid and tokens are issued", - wantStatus: http.StatusOK, - wantBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, + name: "request is valid and tokens are issued", + authcodeExchange: authcodeExchangeInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, // no refresh token + wantRequestedScopes: []string{"openid", "profile", "email"}, + wantGrantedScopes: []string{"openid"}, + }, + }, }, { name: "openid scope was not requested from authorize endpoint", - authRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "profile email") + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "profile email") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"access_token", "token_type", "scope", "expires_in"}, // no id or refresh tokens + wantRequestedScopes: []string{"profile", "email"}, + wantGrantedScopes: []string{}, + }, + }, + }, + { + name: "offline_access and openid scopes were requested and granted from authorize endpoint", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in", "refresh_token"}, // all possible tokens + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + }, + { + name: "offline_access (without openid scope) was requested and granted from authorize endpoint", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"access_token", "token_type", "scope", "expires_in", "refresh_token"}, // no id token + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }, }, - wantStatus: http.StatusOK, - wantBodyFields: []string{"access_token", "token_type", "scope", "expires_in"}, }, // sad path { - name: "GET method is wrong", - request: func(r *http.Request, authCode string) { r.Method = http.MethodGet }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidMethodErrorBody("GET"), + name: "GET method is wrong", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodGet }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidMethodErrorBody("GET"), + }, + }, }, { - name: "PUT method is wrong", - request: func(r *http.Request, authCode string) { r.Method = http.MethodPut }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidMethodErrorBody("PUT"), + name: "PUT method is wrong", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodPut }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidMethodErrorBody("PUT"), + }, + }, }, { - name: "PATCH method is wrong", - request: func(r *http.Request, authCode string) { r.Method = http.MethodPatch }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidMethodErrorBody("PATCH"), + name: "PATCH method is wrong", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodPatch }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidMethodErrorBody("PATCH"), + }, + }, }, { - name: "DELETE method is wrong", - request: func(r *http.Request, authCode string) { r.Method = http.MethodDelete }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidMethodErrorBody("DELETE"), + name: "DELETE method is wrong", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodDelete }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidMethodErrorBody("DELETE"), + }, + }, }, { - name: "content type is invalid", - request: func(r *http.Request, authCode string) { r.Header.Set("Content-Type", "text/plain") }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeEmptyPayloadErrorBody, + name: "content type is invalid", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Header.Set("Content-Type", "text/plain") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeEmptyPayloadErrorBody, + }, + }, }, { name: "payload is not valid form serialization", - request: func(r *http.Request, authCode string) { - r.Body = ioutil.NopCloser(strings.NewReader("this newline character is not allowed in a form serialization: \n")) + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = ioutil.NopCloser(strings.NewReader("this newline character is not allowed in a form serialization: \n")) + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeMissingGrantTypeErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeMissingGrantTypeErrorBody, }, { - name: "payload is empty", - request: func(r *http.Request, authCode string) { r.Body = nil }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidPayloadErrorBody, + name: "payload is empty", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { r.Body = nil }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidPayloadErrorBody, + }, + }, }, { name: "grant type is missing in request", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithGrantType("").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithGrantType("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeMissingGrantTypeErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeMissingGrantTypeErrorBody, }, { name: "grant type is not authorization_code", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithGrantType("bogus").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithGrantType("bogus").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidRequestErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidRequestErrorBody, }, { name: "client id is missing in request", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithClientID("").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithClientID("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeMissingClientErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeMissingClientErrorBody, }, { name: "client id is wrong", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithClientID("bogus").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithClientID("bogus").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: fositeInvalidClientErrorBody, + }, + }, + }, + { + name: "grant type is missing", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithGrantType("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidRequestMissingGrantTypeErrorBody, + }, + }, + }, + { + name: "grant type is wrong", + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithGrantType("bogus").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidRequestErrorBody, + }, }, - wantStatus: http.StatusUnauthorized, - wantExactBody: fositeInvalidClientErrorBody, }, { name: "auth code is missing in request", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithAuthCode("").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithAuthCode("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidAuthCodeErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidAuthCodeErrorBody, }, { name: "auth code has never been valid", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithAuthCode("bogus").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithAuthCode("bogus").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidAuthCodeErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidAuthCodeErrorBody, }, { name: "auth code is invalidated", - storage: func( - t *testing.T, - s interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager + authcodeExchange: authcodeExchangeInputs{ + modifyStorage: func( + t *testing.T, + s interface { + oauth2.TokenRevocationStorage + oauth2.CoreStorage + openid.OpenIDConnectRequestStorage + pkce.PKCERequestStorage + fosite.ClientManager + }, + authCode string, + ) { + err := s.InvalidateAuthorizeCodeSession(context.Background(), getFositeDataSignature(t, authCode)) + require.NoError(t, err) + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeReusedAuthCodeErrorBody, }, - authCode string, - ) { - err := s.InvalidateAuthorizeCodeSession(context.Background(), getFositeDataSignature(t, authCode)) - require.NoError(t, err) }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeReusedAuthCodeErrorBody, }, { name: "redirect uri is missing in request", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithRedirectURI("").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithRedirectURI("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidRedirectURIErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidRedirectURIErrorBody, }, { name: "redirect uri is wrong", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithRedirectURI("bogus").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithRedirectURI("bogus").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidRedirectURIErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeInvalidRedirectURIErrorBody, }, { name: "pkce is missing in request", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithPKCE("").ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithPKCE("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeMissingPKCEVerifierErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeMissingPKCEVerifierErrorBody, }, { name: "pkce is wrong", - request: func(r *http.Request, authCode string) { - r.Body = happyBody(authCode).WithPKCE( - "bogus-verifier-that-is-at-least-43-characters-for-the-sake-of-entropy", - ).ReadCloser() + authcodeExchange: authcodeExchangeInputs{ + modifyTokenRequest: func(r *http.Request, authCode string) { + r.Body = happyAuthcodeRequestBody(authCode).WithPKCE( + "bogus-verifier-that-is-at-least-43-characters-for-the-sake-of-entropy", + ).ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeWrongPKCEVerifierErrorBody, + }, }, - wantStatus: http.StatusBadRequest, - wantExactBody: fositeWrongPKCEVerifierErrorBody, }, { - name: "private signing key for JWTs has not yet been provided by the controller who is responsible for dynamically providing it", - makeOathHelper: makeOauthHelperWithNilPrivateJWTSigningKey, - wantStatus: http.StatusServiceUnavailable, - wantExactBody: fositeTemporarilyUnavailableErrorBody, + name: "private signing key for JWTs has not yet been provided by the controller who is responsible for dynamically providing it", + authcodeExchange: authcodeExchangeInputs{ + makeOathHelper: makeOauthHelperWithNilPrivateJWTSigningKey, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusServiceUnavailable, + wantErrorResponseBody: fositeTemporarilyUnavailableErrorBody, + }, + }, }, } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - authRequest := deepCopyRequestForm(happyAuthRequest) - if test.authRequest != nil { - test.authRequest(authRequest) - } + t.Parallel() - client := fake.NewSimpleClientset() - secrets := client.CoreV1().Secrets("some-namespace") + exchangeAuthcodeForTokens(t, test.authcodeExchange) + }) + } +} - var oauthHelper fosite.OAuth2Provider - var authCode string - var jwtSigningKey *ecdsa.PrivateKey +func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { + tests := []struct { + name string + authcodeExchange authcodeExchangeInputs + }{ + { + name: "authcode exchange succeeds once and then fails when the same authcode is used again", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access profile email") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access", "profile", "email"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() - oauthStore := oidc.NewKubeStorage(secrets) - if test.makeOathHelper != nil { - oauthHelper, authCode, jwtSigningKey = test.makeOathHelper(t, authRequest, oauthStore) - } else { - oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore) - } + // First call - should be successful. + subject, rsp, authCode, _, secrets, oauthStore := exchangeAuthcodeForTokens(t, test.authcodeExchange) + var parsedResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody)) - if test.storage != nil { - test.storage(t, oauthStore, authCode) - } - subject := NewHandler(oauthHelper) - - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 1) - if strings.Contains(authRequest.Form.Get("scope"), "openid") { - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3) - } else { - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2) - } - - req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyBody(authCode).ReadCloser()) + // Second call - should be unsuccessful since auth code was already used. + // + // Fosite will also revoke the access token as is recommended by the OIDC spec. Currently, we don't + // delete the OIDC storage...but we probably should. + req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyAuthcodeRequestBody(authCode).ReadCloser()) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - if test.request != nil { - test.request(req, authCode) + reusedAuthcodeResponse := httptest.NewRecorder() + subject.ServeHTTP(reusedAuthcodeResponse, req) + t.Logf("second response: %#v", reusedAuthcodeResponse) + t.Logf("second response body: %q", reusedAuthcodeResponse.Body.String()) + require.Equal(t, http.StatusBadRequest, reusedAuthcodeResponse.Code) + testutil.RequireEqualContentType(t, reusedAuthcodeResponse.Header().Get("Content-Type"), "application/json") + require.JSONEq(t, fositeReusedAuthCodeErrorBody, reusedAuthcodeResponse.Body.String()) + + // This was previously invalidated by the first request, so it remains invalidated + requireInvalidAuthCodeStorage(t, authCode, oauthStore) + // Has now invalidated the access token that was previously handed out by the first request + requireInvalidAccessTokenStorage(t, parsedResponseBody, oauthStore) + // This was previously invalidated by the first request, so it remains invalidated + requireInvalidPKCEStorage(t, authCode, oauthStore) + // Fosite never cleans up OpenID Connect session storage, so it is still there + requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, + test.authcodeExchange.want.wantRequestedScopes, test.authcodeExchange.want.wantGrantedScopes) + + // Check that the access token and refresh token storage were both deleted, and the number of other storage objects did not change. + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 0) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2) + }) + } +} + +type refreshRequestInputs struct { + modifyTokenRequest func(tokenRequest *http.Request, refreshToken string, accessToken string) + want tokenEndpointResponseExpectedValues +} + +func TestRefreshGrant(t *testing.T) { + tests := []struct { + name string + authcodeExchange authcodeExchangeInputs + refreshRequest refreshRequestInputs + }{ + { + name: "happy path refresh grant with ID token", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }}, + }, + { + name: "happy path refresh grant without ID token", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }}, + }, + { + name: "when the refresh request adds a new scope to the list of requested scopes then it is ignored", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("openid some-other-scope-not-from-auth-request").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }}, + }, + { + name: "when the refresh request removes a scope which was originally granted from the list of requested scopes then it is ignored", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("").ReadCloser() // TODO FIX ME. WE NEED ANOTHER VALID SCOPE ON THIS CLIENT TO WRITE THIS TEST. + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }}, + }, + { + name: "when the refresh request does not include a scope param then it gets all the same scopes as the original authorization request", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithScope("").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + }}, + }, + { + name: "when a bad refresh token is sent in the refresh request", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithRefreshToken("bad refresh token").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidAuthCodeErrorBody, + }}, + }, + { + name: "when the access token is sent as if it were a refresh token", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithRefreshToken(accessToken).ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusBadRequest, + wantErrorResponseBody: fositeInvalidAuthCodeErrorBody, + }}, + }, + { + name: "when the wrong client ID is included in the refresh request", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"offline_access"}, + wantGrantedScopes: []string{"offline_access"}, + }, + }, + refreshRequest: refreshRequestInputs{ + modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { + r.Body = happyRefreshRequestBody(refreshToken).WithClientID("wrong-client-id").ReadCloser() + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: fositeInvalidClientErrorBody, + }}, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // First exchange the authcode for tokens, including a refresh token. + subject, rsp, authCode, jwtSigningKey, secrets, oauthStore := exchangeAuthcodeForTokens(t, test.authcodeExchange) + var parsedAuthcodeExchangeResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedAuthcodeExchangeResponseBody)) + + // Wait one second before performing the refresh so we can see that the refreshed ID token has new issued + // at and expires at dates which are newer than the old tokens. + // If this gets too annoying in terms of making our test suite slower then we can remove it and adjust + // the expectations about the ID token that are made at the end of this test accordingly. + time.Sleep(1 * time.Second) + + // Send the refresh token back and preform a refresh. + firstRefreshToken := parsedAuthcodeExchangeResponseBody["refresh_token"].(string) + req := httptest.NewRequest("POST", "/path/shouldn't/matter", + happyRefreshRequestBody(firstRefreshToken).ReadCloser()) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if test.refreshRequest.modifyTokenRequest != nil { + test.refreshRequest.modifyTokenRequest(req, firstRefreshToken, parsedAuthcodeExchangeResponseBody["access_token"].(string)) } - rsp := httptest.NewRecorder() - subject.ServeHTTP(rsp, req) - t.Logf("response: %#v", rsp) - t.Logf("response body: %q", rsp.Body.String()) + refreshResponse := httptest.NewRecorder() + subject.ServeHTTP(refreshResponse, req) + t.Logf("second response: %#v", refreshResponse) + t.Logf("second response body: %q", refreshResponse.Body.String()) - require.Equal(t, test.wantStatus, rsp.Code) - testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), "application/json") - if test.wantBodyFields != nil { - var m map[string]interface{} - require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &m)) - require.ElementsMatch(t, test.wantBodyFields, getMapKeys(m)) + // The bug in fosite that prevents at_hash from appearing in the initial ID token does not impact the refreshed ID token + wantAtHashClaimInIDToken := true + // Refreshed ID tokens do not include the nonce from the original auth request + wantNonceValueInIDToken := false + requireTokenEndpointBehavior(t, test.refreshRequest.want, wantAtHashClaimInIDToken, wantNonceValueInIDToken, refreshResponse, authCode, oauthStore, jwtSigningKey, secrets) - code := req.PostForm.Get("code") - wantOpenidScope := contains(test.wantBodyFields, "id_token") - requireInvalidAuthCodeStorage(t, code, oauthStore) - requireValidAccessTokenStorage(t, m, oauthStore, wantOpenidScope) - requireInvalidPKCEStorage(t, code, oauthStore) - requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope) + if test.refreshRequest.want.wantStatus == http.StatusOK { + wantIDToken := contains(test.refreshRequest.want.wantSuccessBodyFields, "id_token") - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0) + var parsedRefreshResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(refreshResponse.Body.Bytes(), &parsedRefreshResponseBody)) - if wantOpenidScope { - requireValidIDToken(t, m, jwtSigningKey) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3) - } else { - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2) + // Check that we got back new tokens. + require.NotEqual(t, parsedAuthcodeExchangeResponseBody["access_token"].(string), parsedRefreshResponseBody["access_token"].(string)) + require.NotEqual(t, parsedAuthcodeExchangeResponseBody["refresh_token"].(string), parsedRefreshResponseBody["refresh_token"].(string)) + if wantIDToken { + require.NotEqual(t, parsedAuthcodeExchangeResponseBody["id_token"].(string), parsedRefreshResponseBody["id_token"].(string)) + } + + // The other fields of the response should be the same as the original response. Note that expires_in is a number of seconds from now. + require.Equal(t, parsedAuthcodeExchangeResponseBody["token_type"].(string), parsedRefreshResponseBody["token_type"].(string)) + require.InDelta(t, parsedAuthcodeExchangeResponseBody["expires_in"].(float64), parsedRefreshResponseBody["expires_in"].(float64), 2) + require.Equal(t, parsedAuthcodeExchangeResponseBody["scope"].(string), parsedRefreshResponseBody["scope"].(string)) + + if wantIDToken { + var claimsOfFirstIDToken map[string]interface{} + firstIDTokenDecoded, _ := josejwt.ParseSigned(parsedAuthcodeExchangeResponseBody["id_token"].(string)) + err := firstIDTokenDecoded.UnsafeClaimsWithoutVerification(&claimsOfFirstIDToken) + require.NoError(t, err) + + var claimsOfSecondIDToken map[string]interface{} + secondIDTokenDecoded, _ := josejwt.ParseSigned(parsedRefreshResponseBody["id_token"].(string)) + err = secondIDTokenDecoded.UnsafeClaimsWithoutVerification(&claimsOfSecondIDToken) + require.NoError(t, err) + + requireClaimsAreNotEqual(t, "jti", claimsOfFirstIDToken, claimsOfSecondIDToken) // JWT ID + requireClaimsAreNotEqual(t, "exp", claimsOfFirstIDToken, claimsOfSecondIDToken) // expires at + require.Greater(t, claimsOfSecondIDToken["exp"], claimsOfFirstIDToken["exp"]) + requireClaimsAreNotEqual(t, "iat", claimsOfFirstIDToken, claimsOfSecondIDToken) // issued at + require.Greater(t, claimsOfSecondIDToken["iat"], claimsOfFirstIDToken["iat"]) + + requireClaimsAreEqual(t, "iss", claimsOfFirstIDToken, claimsOfSecondIDToken) // issuer + requireClaimsAreEqual(t, "aud", claimsOfFirstIDToken, claimsOfSecondIDToken) // audience + requireClaimsAreEqual(t, "sub", claimsOfFirstIDToken, claimsOfSecondIDToken) // subject + requireClaimsAreEqual(t, "rat", claimsOfFirstIDToken, claimsOfSecondIDToken) // requested at + requireClaimsAreEqual(t, "auth_time", claimsOfFirstIDToken, claimsOfSecondIDToken) // auth time } - } else { - require.JSONEq(t, test.wantExactBody, rsp.Body.String()) } }) } +} - t.Run("auth code is used twice", func(t *testing.T) { - authRequest := deepCopyRequestForm(happyAuthRequest) +func requireClaimsAreNotEqual(t *testing.T, claimName string, claimsOfTokenA map[string]interface{}, claimsOfTokenB map[string]interface{}) { + require.NotEmpty(t, claimsOfTokenA[claimName]) + require.NotEmpty(t, claimsOfTokenB[claimName]) + require.NotEqual(t, claimsOfTokenA[claimName], claimsOfTokenB[claimName]) +} - client := fake.NewSimpleClientset() - secrets := client.CoreV1().Secrets("some-namespace") +func requireClaimsAreEqual(t *testing.T, claimName string, claimsOfTokenA map[string]interface{}, claimsOfTokenB map[string]interface{}) { + require.NotEmpty(t, claimsOfTokenA[claimName]) + require.NotEmpty(t, claimsOfTokenB[claimName]) + require.Equal(t, claimsOfTokenA[claimName], claimsOfTokenB[claimName]) +} - oauthStore := oidc.NewKubeStorage(secrets) - oauthHelper, authCode, jwtSigningKey := makeHappyOauthHelper(t, authRequest, oauthStore) - subject := NewHandler(oauthHelper) +func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs) ( + subject http.Handler, + rsp *httptest.ResponseRecorder, + authCode string, + jwtSigningKey *ecdsa.PrivateKey, + secrets v1.SecretInterface, + oauthStore *oidc.KubeStorage, +) { + authRequest := deepCopyRequestForm(happyAuthRequest) + if test.modifyAuthRequest != nil { + test.modifyAuthRequest(authRequest) + } - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3) + client := fake.NewSimpleClientset() + secrets = client.CoreV1().Secrets("some-namespace") - req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyBody(authCode).ReadCloser()) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + var oauthHelper fosite.OAuth2Provider - // First call - should be successful. - rsp0 := httptest.NewRecorder() - subject.ServeHTTP(rsp0, req) - t.Logf("response 0: %#v", rsp0) - t.Logf("response 0 body: %q", rsp0.Body.String()) - testutil.RequireEqualContentType(t, rsp0.Header().Get("Content-Type"), "application/json") - require.Equal(t, http.StatusOK, rsp0.Code) + oauthStore = oidc.NewKubeStorage(secrets) + if test.makeOathHelper != nil { + oauthHelper, authCode, jwtSigningKey = test.makeOathHelper(t, authRequest, oauthStore) + } else { + oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore) + } - var m map[string]interface{} - require.NoError(t, json.Unmarshal(rsp0.Body.Bytes(), &m)) + if test.modifyStorage != nil { + test.modifyStorage(t, oauthStore, authCode) + } + subject = NewHandler(oauthHelper) - wantBodyFields := []string{"id_token", "access_token", "token_type", "expires_in", "scope"} - require.ElementsMatch(t, wantBodyFields, getMapKeys(m)) + authorizeEndpointGrantedOpenIDScope := strings.Contains(authRequest.Form.Get("scope"), "openid") + expectedNumberOfIDSessionsStored := 0 + if authorizeEndpointGrantedOpenIDScope { + expectedNumberOfIDSessionsStored = 1 + } - code := req.PostForm.Get("code") - wantOpenidScope := true - requireInvalidAuthCodeStorage(t, code, oauthStore) - requireValidAccessTokenStorage(t, m, oauthStore, wantOpenidScope) - requireInvalidPKCEStorage(t, code, oauthStore) - requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope) - requireValidIDToken(t, m, jwtSigningKey) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 1) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, expectedNumberOfIDSessionsStored) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2+expectedNumberOfIDSessionsStored) + + req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyAuthcodeRequestBody(authCode).ReadCloser()) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if test.modifyTokenRequest != nil { + test.modifyTokenRequest(req, authCode) + } + rsp = httptest.NewRecorder() + + subject.ServeHTTP(rsp, req) + t.Logf("response: %#v", rsp) + t.Logf("response body: %q", rsp.Body.String()) + + wantAtHashClaimInIDToken := false // due to a bug in fosite, the at_hash claim is not filled in during authcode exchange + wantNonceValueInIDToken := true // ID tokens returned by the authcode exchange must include the nonce from the auth request (unliked refreshed ID tokens) + requireTokenEndpointBehavior(t, test.want, wantAtHashClaimInIDToken, wantNonceValueInIDToken, rsp, authCode, oauthStore, jwtSigningKey, secrets) + + return subject, rsp, authCode, jwtSigningKey, secrets, oauthStore +} + +func requireTokenEndpointBehavior( + t *testing.T, + test tokenEndpointResponseExpectedValues, + wantAtHashClaimInIDToken bool, + wantNonceValueInIDToken bool, + tokenEndpointResponse *httptest.ResponseRecorder, + authCode string, + oauthStore *oidc.KubeStorage, + jwtSigningKey *ecdsa.PrivateKey, + secrets v1.SecretInterface, +) { + testutil.RequireEqualContentType(t, tokenEndpointResponse.Header().Get("Content-Type"), "application/json") + require.Equal(t, test.wantStatus, tokenEndpointResponse.Code) + + if test.wantStatus == http.StatusOK { + require.NotNil(t, test.wantSuccessBodyFields, "problem with test table setup: wanted success but did not specify expected response body") + + var parsedResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(tokenEndpointResponse.Body.Bytes(), &parsedResponseBody)) + require.ElementsMatch(t, test.wantSuccessBodyFields, getMapKeys(parsedResponseBody)) + + wantIDToken := contains(test.wantSuccessBodyFields, "id_token") + wantRefreshToken := contains(test.wantSuccessBodyFields, "refresh_token") + + requireInvalidAuthCodeStorage(t, authCode, oauthStore) + requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) + requireInvalidPKCEStorage(t, authCode, oauthStore) + requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) + + expectedNumberOfRefreshTokenSessionsStored := 0 + if wantRefreshToken { + expectedNumberOfRefreshTokenSessionsStored = 1 + } + expectedNumberOfIDSessionsStored := 0 + if wantIDToken { + expectedNumberOfIDSessionsStored = 1 + requireValidIDToken(t, parsedResponseBody, jwtSigningKey, wantAtHashClaimInIDToken, wantNonceValueInIDToken, parsedResponseBody["access_token"].(string)) + } + if wantRefreshToken { + requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) + } testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0) testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, expectedNumberOfRefreshTokenSessionsStored) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, expectedNumberOfIDSessionsStored) + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2+expectedNumberOfRefreshTokenSessionsStored+expectedNumberOfIDSessionsStored) + } else { + require.NotNil(t, test.wantErrorResponseBody, "problem with test table setup: wanted failure but did not specify failure response body") - // Second call - should be unsuccessful since auth code was already used. - // - // Fosite will also revoke the access token as is recommended by the OIDC spec. Currently, we don't - // delete the OIDC storage...but we probably should. - rsp1 := httptest.NewRecorder() - subject.ServeHTTP(rsp1, req) - t.Logf("response 1: %#v", rsp1) - t.Logf("response 1 body: %q", rsp1.Body.String()) - require.Equal(t, http.StatusBadRequest, rsp1.Code) - testutil.RequireEqualContentType(t, rsp1.Header().Get("Content-Type"), "application/json") - require.JSONEq(t, fositeReusedAuthCodeErrorBody, rsp1.Body.String()) + require.JSONEq(t, test.wantErrorResponseBody, tokenEndpointResponse.Body.String()) + } +} - requireInvalidAuthCodeStorage(t, code, oauthStore) - requireInvalidAccessTokenStorage(t, m, oauthStore) - requireInvalidPKCEStorage(t, code, oauthStore) - requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope) - - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 0) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2) - }) +func hashAccessToken(accessToken string) string { + // See https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken. + // "Access Token hash value. Its value is the base64url encoding of the left-most half of + // the hash of the octets of the ASCII representation of the access_token value, where the + // hash algorithm used is the hash algorithm used in the alg Header Parameter of the ID + // Token's JOSE Header." + b := sha256.Sum256([]byte(accessToken)) + return base64.RawURLEncoding.EncodeToString(b[:len(b)/2]) } type body url.Values -func happyBody(happyAuthCode string) body { +func happyAuthcodeRequestBody(happyAuthCode string) body { return map[string][]string{ "grant_type": {"authorization_code"}, "code": {happyAuthCode}, @@ -572,10 +1044,23 @@ func happyBody(happyAuthCode string) body { } } +func happyRefreshRequestBody(refreshToken string) body { + return map[string][]string{ + "grant_type": {"refresh_token"}, + "scope": {"openid"}, + "client_id": {goodClient}, + "refresh_token": {refreshToken}, + } +} + func (b body) WithGrantType(grantType string) body { return b.with("grant_type", grantType) } +func (b body) WithRefreshToken(refreshToken string) body { + return b.with("refresh_token", refreshToken) +} + func (b body) WithClientID(clientID string) body { return b.with("client_id", clientID) } @@ -584,6 +1069,10 @@ func (b body) WithAuthCode(code string) body { return b.with("code", code) } +func (b body) WithScope(scope string) body { + return b.with("scope", scope) +} + func (b body) WithRedirectURI(redirectURI string) body { return b.with("redirect_uri", redirectURI) } @@ -652,9 +1141,8 @@ func makeOauthHelperWithNilPrivateJWTSigningKey( return oauthHelper, authResponder.GetCode(), nil } +// Simulate the auth endpoint running so Fosite code will fill the store with realistic values. func simulateAuthEndpointHavingAlreadyRun(t *testing.T, authRequest *http.Request, oauthHelper fosite.OAuth2Provider) fosite.AuthorizeResponder { - // Simulate the auth endpoint running so Fosite code will fill the store with realistic values. - // // We only set the fields in the session that Fosite wants us to set. ctx := context.Background() session := &openid.DefaultSession{ @@ -671,6 +1159,9 @@ func simulateAuthEndpointHavingAlreadyRun(t *testing.T, authRequest *http.Reques if strings.Contains(authRequest.Form.Get("scope"), "openid") { authRequester.GrantScope("openid") } + if strings.Contains(authRequest.Form.Get("scope"), "offline_access") { + authRequester.GrantScope("offline_access") + } authResponder, err := oauthHelper.NewAuthorizeResponse(ctx, authRequester, session) require.NoError(t, err) return authResponder @@ -705,11 +1196,41 @@ func requireInvalidAuthCodeStorage( require.True(t, errors.Is(err, fosite.ErrInvalidatedAuthorizeCode)) } +func requireValidRefreshTokenStorage( + t *testing.T, + body map[string]interface{}, + storage oauth2.CoreStorage, + wantRequestedScopes []string, + wantGrantedScopes []string, +) { + t.Helper() + + // Get the refresh token, and make sure we can use it to perform a lookup on the storage. + refreshToken, ok := body["refresh_token"] + require.True(t, ok) + refreshTokenString, ok := refreshToken.(string) + require.Truef(t, ok, "wanted refresh_token to be a string, but got %T", refreshToken) + require.NotEmpty(t, refreshTokenString) + storedRequest, err := storage.GetRefreshTokenSession(context.Background(), getFositeDataSignature(t, refreshTokenString), nil) + require.NoError(t, err) + + // Fosite stores refresh tokens without any of the original request form parameters. + requireValidStoredRequest( + t, + storedRequest, + storedRequest.Sanitize([]string{}).GetRequestForm(), + wantRequestedScopes, + wantGrantedScopes, + true, + ) +} + func requireValidAccessTokenStorage( t *testing.T, body map[string]interface{}, storage oauth2.CoreStorage, - wantGrantedOpenidScope bool, + wantRequestedScopes []string, + wantGrantedScopes []string, ) { t.Helper() @@ -718,7 +1239,8 @@ func requireValidAccessTokenStorage( require.True(t, ok) accessTokenString, ok := accessToken.(string) require.Truef(t, ok, "wanted access_token to be a string, but got %T", accessToken) - authRequest, err := storage.GetAccessTokenSession(context.Background(), getFositeDataSignature(t, accessTokenString), nil) + require.NotEmpty(t, accessTokenString) + storedRequest, err := storage.GetAccessTokenSession(context.Background(), getFositeDataSignature(t, accessTokenString), nil) require.NoError(t, err) // Make sure the other body fields are valid. @@ -732,24 +1254,21 @@ func requireValidAccessTokenStorage( require.True(t, ok) expiresInNumber, ok := expiresIn.(float64) // Go unmarshals JSON numbers to float64, see `go doc encoding/json` require.Truef(t, ok, "wanted expires_in to be an float64, but got %T", expiresIn) - require.InDelta(t, accessTokenExpirationSeconds, expiresInNumber, timeComparisonFudgeSeconds) + require.InDelta(t, accessTokenExpirationSeconds, expiresInNumber, 2) // "expires_in" is a number of seconds, not a timestamp scopes, ok := body["scope"] require.True(t, ok) - scopesString, ok := scopes.(string) + actualGrantedScopesString, ok := scopes.(string) require.Truef(t, ok, "wanted scopes to be an string, but got %T", scopes) - wantScopes := "" - if wantGrantedOpenidScope { - wantScopes += "openid" - } - require.Equal(t, wantScopes, scopesString) + require.Equal(t, strings.Join(wantGrantedScopes, " "), actualGrantedScopesString) - // Fosite stores access tokens without any of the original request form pararmeters. - requireValidAuthRequest( + // Fosite stores access tokens without any of the original request form parameters. + requireValidStoredRequest( t, - authRequest, - authRequest.Sanitize([]string{}).GetRequestForm(), - wantGrantedOpenidScope, + storedRequest, + storedRequest.Sanitize([]string{}).GetRequestForm(), + wantRequestedScopes, + wantGrantedScopes, true, ) } @@ -788,13 +1307,14 @@ func requireValidOIDCStorage( body map[string]interface{}, code string, storage openid.OpenIDConnectRequestStorage, - wantGrantedOpenidScope bool, + wantRequestedScopes []string, + wantGrantedScopes []string, ) { t.Helper() - if wantGrantedOpenidScope { + if contains(wantGrantedScopes, "openid") { // Make sure the OIDC session is still there. Note that Fosite stores OIDC sessions using the full auth code as a key. - authRequest, err := storage.GetOpenIDConnectSession(context.Background(), code, nil) + storedRequest, err := storage.GetOpenIDConnectSession(context.Background(), code, nil) require.NoError(t, err) // Fosite stores OIDC sessions with only the nonce in the original request form. @@ -804,11 +1324,12 @@ func requireValidOIDCStorage( require.Truef(t, ok, "wanted access_token to be a string, but got %T", accessToken) require.NotEmpty(t, accessTokenString) - requireValidAuthRequest( + requireValidStoredRequest( t, - authRequest, - authRequest.Sanitize([]string{"nonce"}).GetRequestForm(), - true, + storedRequest, + storedRequest.Sanitize([]string{"nonce"}).GetRequestForm(), + wantRequestedScopes, + wantGrantedScopes, false, ) } else { @@ -817,37 +1338,32 @@ func requireValidOIDCStorage( } } -func requireValidAuthRequest( +func requireValidStoredRequest( t *testing.T, - authRequest fosite.Requester, + request fosite.Requester, wantRequestForm url.Values, - wantGrantedOpenidScope bool, + wantRequestedScopes []string, + wantGrantedScopes []string, wantAccessTokenExpiresAt bool, ) { t.Helper() - // Assert that the getters on the authRequest return what we think they should. - wantRequestedScopes := []string{"profile", "email"} - wantGrantedScopes := []string{} - if wantGrantedOpenidScope { - wantRequestedScopes = append([]string{"openid"}, wantRequestedScopes...) - wantGrantedScopes = append([]string{"openid"}, wantGrantedScopes...) - } - require.NotEmpty(t, authRequest.GetID()) - testutil.RequireTimeInDelta(t, authRequest.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second) - require.Equal(t, goodClient, authRequest.GetClient().GetID()) - require.Equal(t, fosite.Arguments(wantRequestedScopes), authRequest.GetRequestedScopes()) - require.Equal(t, fosite.Arguments(wantGrantedScopes), authRequest.GetGrantedScopes()) - require.Empty(t, authRequest.GetRequestedAudience()) - require.Empty(t, authRequest.GetGrantedAudience()) - require.Equal(t, wantRequestForm, authRequest.GetRequestForm()) // Fosite stores access token request without form + // Assert that the getters on the request return what we think they should. + require.NotEmpty(t, request.GetID()) + testutil.RequireTimeInDelta(t, request.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second) + require.Equal(t, goodClient, request.GetClient().GetID()) + require.Equal(t, fosite.Arguments(wantRequestedScopes), request.GetRequestedScopes()) + require.Equal(t, fosite.Arguments(wantGrantedScopes), request.GetGrantedScopes()) + require.Empty(t, request.GetRequestedAudience()) + require.Empty(t, request.GetGrantedAudience()) + require.Equal(t, wantRequestForm, request.GetRequestForm()) // Fosite stores access token request without form // Cast session to the type we think it should be. - session, ok := authRequest.GetSession().(*openid.DefaultSession) - require.Truef(t, ok, "could not cast %T to %T", authRequest.GetSession(), &openid.DefaultSession{}) + session, ok := request.GetSession().(*openid.DefaultSession) + require.Truef(t, ok, "could not cast %T to %T", request.GetSession(), &openid.DefaultSession{}) // Assert that the session claims are what we think they should be, but only if we are doing OIDC. - if wantGrantedOpenidScope { + if contains(wantGrantedScopes, "openid") { claims := session.Claims require.Empty(t, claims.JTI) // When claims.JTI is empty, Fosite will generate a UUID for this field. require.Equal(t, goodSubject, claims.Subject) @@ -912,7 +1428,14 @@ func requireValidAuthRequest( require.Equal(t, goodSubject, session.Subject) } -func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKey *ecdsa.PrivateKey) { +func requireValidIDToken( + t *testing.T, + body map[string]interface{}, + jwtSigningKey *ecdsa.PrivateKey, + wantAtHashClaimInIDToken bool, + wantNonceValueInIDToken bool, + actualAccessToken string, +) { t.Helper() idToken, ok := body["id_token"] @@ -936,9 +1459,13 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe AuthTime int64 `json:"auth_time"` } - // Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token. + // Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token + // during the initial authcode exchange, but does not prevent `at_hash` from appearing in the refreshed ID token. // We can add a workaround for this later. idTokenFields := []string{"sub", "aud", "iss", "jti", "nonce", "auth_time", "exp", "iat", "rat"} + if wantAtHashClaimInIDToken { + idTokenFields = append(idTokenFields, "at_hash") + } // make sure that these are the only fields in the token var m map[string]interface{} @@ -953,7 +1480,12 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe require.Equal(t, goodClient, claims.Audience[0]) require.Equal(t, goodIssuer, claims.Issuer) require.NotEmpty(t, claims.JTI) - require.Equal(t, goodNonce, claims.Nonce) + + if wantNonceValueInIDToken { + require.Equal(t, goodNonce, claims.Nonce) + } else { + require.Empty(t, claims.Nonce) + } expiresAt := time.Unix(claims.ExpiresAt, 0) issuedAt := time.Unix(claims.IssuedAt, 0) @@ -963,6 +1495,13 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe testutil.RequireTimeInDelta(t, time.Now().UTC(), issuedAt, timeComparisonFudgeSeconds*time.Second) testutil.RequireTimeInDelta(t, goodRequestedAtTime, requestedAt, timeComparisonFudgeSeconds*time.Second) testutil.RequireTimeInDelta(t, goodAuthTime, authTime, timeComparisonFudgeSeconds*time.Second) + + if wantAtHashClaimInIDToken { + require.NotEmpty(t, actualAccessToken) + require.Equal(t, hashAccessToken(actualAccessToken), claims.AccessTokenHash) + } else { + require.Empty(t, claims.AccessTokenHash) + } } func deepCopyRequestForm(r *http.Request) *http.Request {