From e5ecaf01a0179c431c6a56bfb2b923e503a02610 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 7 Dec 2020 17:28:51 -0800 Subject: [PATCH 01/17] WIP stubbing out tokenexchangehandler --- internal/oidc/oidc.go | 2 +- internal/oidc/token_exchange.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 internal/oidc/token_exchange.go diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 7c550751a..81c7d54c0 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -83,7 +83,7 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code"}, + GrantTypes: []string{"authorization_code", "token_exchange"}, Scopes: []string{"openid", "profile", "email"}, }, TokenEndpointAuthMethod: "none", diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go new file mode 100644 index 000000000..2f3f1ee15 --- /dev/null +++ b/internal/oidc/token_exchange.go @@ -0,0 +1,23 @@ +package oidc + +import ( + "context" + + "github.com/ory/fosite" + "github.com/ory/fosite/compose" +) + +func TokenExchangeFactory(config *compose.Config, storage interface{}, strategy interface{}) interface{} { + return &TokenExchangeHandler{} +} + +type TokenExchangeHandler struct { +} + +func (t *TokenExchangeHandler) HandleTokenEndpointRequest(ctx context.Context, requester fosite.AccessRequester) error { + return nil +} + +func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error { + return nil +} From afbef23a5137cfd26a5986b990536f213fb55d0a Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 8 Dec 2020 10:17:03 -0800 Subject: [PATCH 02/17] WIP implementing TokenExchangeHandler methods Signed-off-by: Margo Crawford --- internal/oidc/oidc.go | 3 ++- internal/oidc/token_exchange.go | 41 ++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 81c7d54c0..607abff37 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -83,7 +83,7 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code", "token_exchange"}, + GrantTypes: []string{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"}, Scopes: []string{"openid", "profile", "email"}, }, TokenEndpointAuthMethod: "none", @@ -124,6 +124,7 @@ func FositeOauth2Helper( OpenIDConnectTokenStrategy: newDynamicOpenIDConnectECDSAStrategy(oauthConfig, jwksProvider), }, nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. + TokenExchangeFactory, compose.OAuth2AuthorizeExplicitFactory, // compose.OAuth2RefreshTokenGrantFactory, compose.OpenIDConnectExplicitFactory, diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index 2f3f1ee15..c4fbc7088 100644 --- a/internal/oidc/token_exchange.go +++ b/internal/oidc/token_exchange.go @@ -3,21 +3,60 @@ package oidc import ( "context" + "github.com/ory/fosite/handler/oauth2" + + "github.com/ory/fosite/handler/openid" + + "github.com/pkg/errors" + "github.com/ory/fosite" "github.com/ory/fosite/compose" ) func TokenExchangeFactory(config *compose.Config, storage interface{}, strategy interface{}) interface{} { - return &TokenExchangeHandler{} + return &TokenExchangeHandler{ + strategy.(openid.OpenIDConnectTokenStrategy), + strategy.(oauth2.AccessTokenStrategy), + storage.(oauth2.AccessTokenStorage), + } } type TokenExchangeHandler struct { + idTokenStrategy openid.OpenIDConnectTokenStrategy + accessTokenStrategy oauth2.AccessTokenStrategy + accessTokenStorage oauth2.AccessTokenStorage } func (t *TokenExchangeHandler) HandleTokenEndpointRequest(ctx context.Context, requester fosite.AccessRequester) error { + if !(requester.GetGrantTypes().ExactOne("urn:ietf:params:oauth:grant-type:token-exchange")) { + return errors.WithStack(fosite.ErrUnknownRequest) + } return nil } func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error { + params := requester.GetRequestForm() + accessToken := params.Get("subject_token") + if err := t.accessTokenStrategy.ValidateAccessToken(ctx, requester, accessToken); err != nil { + return errors.WithStack(err) + } + signature := t.accessTokenStrategy.AccessTokenSignature(accessToken) + accessTokenSession, err := t.accessTokenStorage.GetAccessTokenSession(ctx, signature, requester.GetSession()) + if err != nil { + return errors.WithStack(err) + } + if !accessTokenSession.GetGrantedScopes().Has("pinniped.sts.unrestricted") { + return errors.WithStack(fosite.ErrScopeNotGranted) + } + // TODO check the other requester fields + scopedDownRequester := fosite.NewAccessRequest(requester.GetSession()) + scopedDownRequester.GrantedAudience = []string{params.Get("audience")} + newToken, err := t.idTokenStrategy.GenerateIDToken(ctx, scopedDownRequester) + if err != nil { + return errors.WithStack(err) + } + responder.SetAccessToken(newToken) + responder.SetTokenType("N_A") + responder.SetExtra("issued_token_type", "urn:ietf:params:oauth:token-type:jwt") return nil } From f103c024080b65ef36a3a5e932fc8828814bd048 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 8 Dec 2020 17:33:08 -0800 Subject: [PATCH 03/17] Add check for grant type in tokenexchangehandler, - also started writing a test for the tokenexchangehandler, skipping for now Signed-off-by: Ryan Richard --- internal/oidc/nullstorage_test.go | 2 +- internal/oidc/oidc.go | 6 +-- internal/oidc/token/token_handler_test.go | 65 +++++++++++++++++++++++ internal/oidc/token_exchange.go | 8 ++- 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/internal/oidc/nullstorage_test.go b/internal/oidc/nullstorage_test.go index 4adbe8072..62f25cab3 100644 --- a/internal/oidc/nullstorage_test.go +++ b/internal/oidc/nullstorage_test.go @@ -27,7 +27,7 @@ func TestNullStorage_GetClient(t *testing.T) { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code", "refresh_token"}, + GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, Scopes: []string{"openid", "offline_access", "profile", "email"}, }, TokenEndpointAuthMethod: "none", diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 0a239be59..b5a55a098 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -84,8 +84,8 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { Public: true, RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"}, - Scopes: []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email"}, + GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, + Scopes: []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email", "pinniped.sts.unrestricted"}, }, TokenEndpointAuthMethod: "none", } @@ -126,12 +126,12 @@ func FositeOauth2Helper( OpenIDConnectTokenStrategy: newDynamicOpenIDConnectECDSAStrategy(oauthConfig, jwksProvider), }, nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. - TokenExchangeFactory, compose.OAuth2AuthorizeExplicitFactory, // compose.OAuth2RefreshTokenGrantFactory, compose.OpenIDConnectExplicitFactory, // compose.OpenIDConnectRefreshFactory, compose.OAuth2PKCEFactory, + TokenExchangeFactory, ) } diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index beb976608..4fa834b3c 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -206,6 +206,19 @@ var ( "redirect_uri": {goodRedirectURI}, }, } + + happyTokenExchangeRequest = func(audience string, subjectToken string) *http.Request { + return &http.Request{ + Form: url.Values{ + "grant_type": {"urn:ietf:params:oauth:grant-type:token-exchange"}, + "audience": {audience}, + "subject_token": {subjectToken}, + "subject_token_type": {"urn:ietf:params:oauth:token-type:access_token"}, + "requested_token_type": {"urn:ietf:params:oauth:token-type:jwt"}, + "client_id": {goodClient}, + }, + } + } ) type authcodeExchangeInputs struct { @@ -550,6 +563,55 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { } } +func TestTokenExchange(t *testing.T) { + // TODO write this test + t.Skip() + tests := []struct { + name string + authcodeExchange authcodeExchangeInputs + wantStatus int + requestedAudience string + modifyTokenExchangeRequest func(r *http.Request) + }{ + { + name: "token exchange happy path", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(authRequest *http.Request) { + authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") + }, + wantStatus: http.StatusOK, + wantBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "pinniped.sts.unrestricted"}, + }, + wantStatus: http.StatusOK, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + subject, rsp, _, _, _ := exchangeAuthcodeForTokens(t, test.authcodeExchange) + var parsedResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody)) + + request := happyTokenExchangeRequest("foo-cluster", parsedResponseBody["access_token"].(string)) + + 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() + + 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") + }) + } +} + func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs) ( subject http.Handler, rsp *httptest.ResponseRecorder, @@ -758,6 +820,9 @@ func simulateAuthEndpointHavingAlreadyRun(t *testing.T, authRequest *http.Reques if strings.Contains(authRequest.Form.Get("scope"), "offline_access") { authRequester.GrantScope("offline_access") } + if strings.Contains(authRequest.Form.Get("scope"), "pinniped.sts.unrestricted") { + authRequester.GrantScope("pinniped.sts.unrestricted") + } authResponder, err := oauthHelper.NewAuthorizeResponse(ctx, authRequester, session) require.NoError(t, err) return authResponder diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index c4fbc7088..92b07efed 100644 --- a/internal/oidc/token_exchange.go +++ b/internal/oidc/token_exchange.go @@ -1,3 +1,6 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package oidc import ( @@ -35,6 +38,9 @@ func (t *TokenExchangeHandler) HandleTokenEndpointRequest(ctx context.Context, r } func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error { + if !(requester.GetGrantTypes().ExactOne("urn:ietf:params:oauth:grant-type:token-exchange")) { + return errors.WithStack(fosite.ErrUnknownRequest) + } params := requester.GetRequestForm() accessToken := params.Get("subject_token") if err := t.accessTokenStrategy.ValidateAccessToken(ctx, requester, accessToken); err != nil { @@ -49,7 +55,7 @@ func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context return errors.WithStack(fosite.ErrScopeNotGranted) } // TODO check the other requester fields - scopedDownRequester := fosite.NewAccessRequest(requester.GetSession()) + scopedDownRequester := fosite.NewAccessRequest(accessTokenSession.GetSession()) scopedDownRequester.GrantedAudience = []string{params.Get("audience")} newToken, err := t.idTokenStrategy.GenerateIDToken(ctx, scopedDownRequester) if err != nil { From 6420caca9457b3a37126909f70b0a09a25c80b50 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 8 Dec 2020 18:25:01 -0800 Subject: [PATCH 04/17] Bring back the test that was skipped by the previous commit - This test is still a work in progress. Some TODO comments have been added to give hints for next steps. --- internal/oidc/token/token_handler_test.go | 30 +++++++++++++++-------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index a37395c08..a134410e4 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -566,14 +566,14 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { } func TestTokenExchange(t *testing.T) { - // TODO write this test - t.Skip() tests := []struct { - name string + name string + authcodeExchange authcodeExchangeInputs - wantStatus int - requestedAudience string modifyTokenExchangeRequest func(r *http.Request) + requestedAudience string + + wantStatus int }{ { name: "token exchange happy path", @@ -581,12 +581,18 @@ func TestTokenExchange(t *testing.T) { modifyAuthRequest: func(authRequest *http.Request) { authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") }, - wantStatus: http.StatusOK, - wantBodyFields: []string{"id_token", "access_token", "token_type", "expires_in", "scope"}, - wantRequestedScopes: []string{"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"}, }, - wantStatus: http.StatusOK, + 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). + // Can use modifyTokenExchangeRequest to change to request params for the token exchange call (e.g. bad access token or wrong http verb). + // Will need to add some more fields to the test table to declare the expected values of the error response, etc. } for _, test := range tests { test := test @@ -595,7 +601,7 @@ func TestTokenExchange(t *testing.T) { var parsedResponseBody map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody)) - request := happyTokenExchangeRequest("foo-cluster", parsedResponseBody["access_token"].(string)) + request := happyTokenExchangeRequest(test.requestedAudience, parsedResponseBody["access_token"].(string)) req := httptest.NewRequest("POST", "/path/shouldn't/matter", body(request.Form).ReadCloser()) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") @@ -610,6 +616,10 @@ func TestTokenExchange(t *testing.T) { require.Equal(t, test.wantStatus, rsp.Code) testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), "application/json") + + // TODO write lots more assertions about the happy path + // e.g. that nothing was changed in the fosite k8s storage + // e.g. that the response body is correct, and the decoded token has the right claims }) } } From 5f1bd5ec31b4d96d34417d5d3c7b39f008f1d819 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 09:12:32 -0600 Subject: [PATCH 05/17] Update TestNullStorage_GetClient with adjusted pinniped-cli scopes. Signed-off-by: Matt Moyer --- internal/oidc/nullstorage_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/oidc/nullstorage_test.go b/internal/oidc/nullstorage_test.go index 62f25cab3..523aafb5f 100644 --- a/internal/oidc/nullstorage_test.go +++ b/internal/oidc/nullstorage_test.go @@ -28,7 +28,7 @@ func TestNullStorage_GetClient(t *testing.T) { RedirectURIs: []string{"http://127.0.0.1/callback"}, ResponseTypes: []string{"code"}, GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, - Scopes: []string{"openid", "offline_access", "profile", "email"}, + Scopes: []string{"openid", "offline_access", "profile", "email", "pinniped.sts.unrestricted"}, }, TokenEndpointAuthMethod: "none", }, From 644cb687b97a9dd0192502c5f4951c3795c8b799 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 09:36:45 -0600 Subject: [PATCH 06/17] Grant the Pinniped STS scope in authorize/callback handlers. Signed-off-by: Matt Moyer --- internal/oidc/auth/auth_handler.go | 3 +++ internal/oidc/callback/callback_handler.go | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 1998b72a7..61ab75294 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -63,6 +63,9 @@ func NewHandler( // 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) + // Grant the Pinniped STS scope if requested. + oidc.GrantScopeIfRequested(authorizeRequester, "pinniped.sts.unrestricted") + now := time.Now() _, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{ Claims: &jwt.IDTokenClaims{ diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 3c85ee4c9..2cea6c2e4 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -71,9 +71,10 @@ func NewHandler( return httperr.New(http.StatusBadRequest, "error using state downstream auth params") } - // Automatically grant the openid and offline_access scopes, but only if they were requested. + // Automatically grant the openid, offline_access, and Pinniped STS scopes, but only if they were requested. oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) + oidc.GrantScopeIfRequested(authorizeRequester, "pinniped.sts.unrestricted") token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens( r.Context(), From 1db2ae3a457745f42f7a2eab63a87a9ec59bea91 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 10:04:58 -0600 Subject: [PATCH 07/17] Add more parameter validations and refactor internal/oidc/token_exchange.go. Signed-off-by: Matt Moyer --- internal/oidc/token_exchange.go | 113 +++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 25 deletions(-) diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index 92b07efed..a344ed773 100644 --- a/internal/oidc/token_exchange.go +++ b/internal/oidc/token_exchange.go @@ -5,22 +5,30 @@ package oidc import ( "context" - - "github.com/ory/fosite/handler/oauth2" - - "github.com/ory/fosite/handler/openid" - - "github.com/pkg/errors" + "net/url" "github.com/ory/fosite" "github.com/ory/fosite/compose" + "github.com/ory/fosite/handler/oauth2" + "github.com/ory/fosite/handler/openid" + "github.com/pkg/errors" ) +const ( + tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" + tokenTypeJWT = "urn:ietf:params:oauth:token-type:jwt" +) + +type stsParams struct { + subjectAccessToken string + requestedAudience string +} + func TokenExchangeFactory(config *compose.Config, storage interface{}, strategy interface{}) interface{} { return &TokenExchangeHandler{ - strategy.(openid.OpenIDConnectTokenStrategy), - strategy.(oauth2.AccessTokenStrategy), - storage.(oauth2.AccessTokenStorage), + idTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy), + accessTokenStrategy: strategy.(oauth2.AccessTokenStrategy), + accessTokenStorage: storage.(oauth2.AccessTokenStorage), } } @@ -38,31 +46,86 @@ func (t *TokenExchangeHandler) HandleTokenEndpointRequest(ctx context.Context, r } func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error { - if !(requester.GetGrantTypes().ExactOne("urn:ietf:params:oauth:grant-type:token-exchange")) { - return errors.WithStack(fosite.ErrUnknownRequest) - } - params := requester.GetRequestForm() - accessToken := params.Get("subject_token") - if err := t.accessTokenStrategy.ValidateAccessToken(ctx, requester, accessToken); err != nil { + // Skip this request if it's for a different grant type. + if err := t.HandleTokenEndpointRequest(ctx, requester); err != nil { return errors.WithStack(err) } - signature := t.accessTokenStrategy.AccessTokenSignature(accessToken) - accessTokenSession, err := t.accessTokenStorage.GetAccessTokenSession(ctx, signature, requester.GetSession()) + + // Validate the basic RFC8693 parameters we support. + params, err := t.validateParams(requester.GetRequestForm()) if err != nil { return errors.WithStack(err) } - if !accessTokenSession.GetGrantedScopes().Has("pinniped.sts.unrestricted") { - return errors.WithStack(fosite.ErrScopeNotGranted) - } - // TODO check the other requester fields - scopedDownRequester := fosite.NewAccessRequest(accessTokenSession.GetSession()) - scopedDownRequester.GrantedAudience = []string{params.Get("audience")} - newToken, err := t.idTokenStrategy.GenerateIDToken(ctx, scopedDownRequester) + + // Validate the incoming access token and lookup the information about the original authorize request. + originalRequester, err := t.validateAccessToken(ctx, requester, params.subjectAccessToken) if err != nil { return errors.WithStack(err) } - responder.SetAccessToken(newToken) + + // 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 { + return errors.WithStack(err) + } + + // 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") return nil } + +func (t *TokenExchangeHandler) mintJWT(ctx context.Context, requester fosite.Requester, audience string) (string, error) { + downscoped := fosite.NewAccessRequest(requester.GetSession()) + downscoped.Client.(*fosite.DefaultClient).ID = audience + return t.idTokenStrategy.GenerateIDToken(ctx, downscoped) +} + +func (t *TokenExchangeHandler) validateParams(params url.Values) (*stsParams, error) { + var result stsParams + + // Validate some required parameters. + result.requestedAudience = params.Get("audience") + if result.requestedAudience == "" { + return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "missing audience parameter") + } + result.subjectAccessToken = params.Get("subject_token") + if result.subjectAccessToken == "" { + return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "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) + } + if params.Get("requested_token_type") != tokenTypeJWT { + return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "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. + for _, param := range []string{ + "resource", + "scope", + "actor_token", + "actor_token_type", + } { + if params.Get(param) != "" { + return nil, errors.WithMessagef(fosite.ErrInvalidRequest, "unsupported parameter %s", param) + } + } + + return &result, nil +} + +func (t *TokenExchangeHandler) validateAccessToken(ctx context.Context, requester fosite.AccessRequester, accessToken string) (fosite.Requester, error) { + if err := t.accessTokenStrategy.ValidateAccessToken(ctx, requester, accessToken); err != nil { + return nil, errors.WithStack(err) + } + signature := t.accessTokenStrategy.AccessTokenSignature(accessToken) + originalRequester, err := t.accessTokenStorage.GetAccessTokenSession(ctx, signature, requester.GetSession()) + if err != nil { + return nil, errors.WithStack(err) + } + return originalRequester, nil +} From b1542be7b1a1fa391e2927850ad13fce75eb17c5 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 10:08:41 -0600 Subject: [PATCH 08/17] In oidcclient token exchange request, pass client_id but don't bother with authorization header. I think this should be more correct. In the server we're authenticating the request primarily via the `subject_token` parameter anyway, and Fosite needs the `client_id` to be set. Signed-off-by: Matt Moyer --- pkg/oidcclient/login.go | 6 ++---- pkg/oidcclient/login_test.go | 5 +++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 62c380c97..98146fa52 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -338,11 +338,9 @@ func (h *handlerState) tokenExchangeRFC8693(baseToken *oidctypes.Token) (*oidcty return nil, err } - // Use the base access token to authenticate our request. This will populate the "authorization" header. - client := oauth2.NewClient(h.ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: baseToken.AccessToken.Token})) - // Form the HTTP POST request with the parameters specified by RFC8693. reqBody := strings.NewReader(url.Values{ + "client_id": []string{h.clientID}, "grant_type": []string{"urn:ietf:params:oauth:grant-type:token-exchange"}, "audience": []string{h.requestedAudience}, "subject_token": []string{baseToken.AccessToken.Token}, @@ -356,7 +354,7 @@ func (h *handlerState) tokenExchangeRFC8693(baseToken *oidctypes.Token) (*oidcty req.Header.Set("content-type", "application/x-www-form-urlencoded") // Perform the request. - resp, err := client.Do(req) + resp, err := h.httpClient.Do(req) if err != nil { return nil, err } diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index 04c0b4b2c..10daf6ab6 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -152,6 +152,11 @@ func TestLogin(t *testing.T) { } case "urn:ietf:params:oauth:grant-type:token-exchange": + if r.Form.Get("client_id") != "test-client-id" { + http.Error(w, "bad client_id", http.StatusBadRequest) + return + } + switch r.Form.Get("audience") { case "test-audience-produce-invalid-http-response": http.Redirect(w, r, "%", http.StatusTemporaryRedirect) From f1aff2faab293d28cc0080ce16c88b172cdb174b Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 10:23:10 -0600 Subject: [PATCH 09/17] Start extending TestSupervisorLogin to test the token exchange flow (WIP). Signed-off-by: Matt Moyer --- test/integration/supervisor_login_test.go | 46 ++++++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 65e907914..5c8827786 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "crypto/x509/pkix" "encoding/base64" + "encoding/json" "net/http" "net/http/httptest" "net/url" @@ -125,7 +126,7 @@ func TestSupervisorLogin(t *testing.T) { ClientID: "pinniped-cli", Endpoint: discovery.Endpoint(), RedirectURL: localCallbackServer.URL, - Scopes: []string{"openid"}, + Scopes: []string{"openid", "pinniped.sts.unrestricted"}, } // Build a valid downstream authorize URL for the supervisor. @@ -159,7 +160,7 @@ func TestSupervisorLogin(t *testing.T) { callback := localCallbackServer.waitForCallback(10 * time.Second) t.Logf("got callback request: %s", library.MaskTokens(callback.URL.String())) require.Equal(t, stateParam.String(), callback.URL.Query().Get("state")) - require.Equal(t, "openid", callback.URL.Query().Get("scope")) + require.Equal(t, "openid pinniped.sts.unrestricted", callback.URL.Query().Get("scope")) authcode := callback.URL.Query().Get("code") require.NotEmpty(t, authcode) @@ -197,6 +198,8 @@ func TestSupervisorLogin(t *testing.T) { testutil.RequireTimeInDelta(t, time.Now().UTC().Add(time.Minute*5), tokenResponse.Expiry, time.Second*30) require.Empty(t, tokenResponse.RefreshToken) // for now, until the next user story :) + + wipDoTokenExchange(t, &downstreamOAuth2Config, tokenResponse, httpClient, discovery) // WIP while we work on the token exchange server. } func startLocalCallbackServer(t *testing.T) *localCallbackServer { @@ -226,3 +229,42 @@ func (s *localCallbackServer) waitForCallback(timeout time.Duration) *http.Reque return nil } } + +// WIP test code for the token exchange flow. +func wipDoTokenExchange(t *testing.T, config *oauth2.Config, tokenResponse *oauth2.Token, httpClient *http.Client, provider *oidc.Provider) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + + // Form the HTTP POST request with the parameters specified by RFC8693. + reqBody := strings.NewReader(url.Values{ + "grant_type": []string{"urn:ietf:params:oauth:grant-type:token-exchange"}, + "audience": []string{"cluster-1234"}, + "client_id": []string{config.ClientID}, + "subject_token": []string{tokenResponse.AccessToken}, + "subject_token_type": []string{"urn:ietf:params:oauth:token-type:access_token"}, + "requested_token_type": []string{"urn:ietf:params:oauth:token-type:jwt"}, + }.Encode()) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, config.Endpoint.TokenURL, reqBody) + require.NoError(t, err) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + + resp, err := httpClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + var respBody struct { + AccessToken string `json:"access_token"` + IssuedTokenType string `json:"issued_token_type"` + TokenType string `json:"token_type"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&respBody)) + + var clusterVerifier = provider.Verifier(&oidc.Config{ClientID: "cluster-1234"}) + exchangedToken, err := clusterVerifier.Verify(ctx, respBody.AccessToken) + require.NoError(t, err) + + var claims map[string]interface{} + require.NoError(t, exchangedToken.Claims(&claims)) + indentedClaims, err := json.MarshalIndent(claims, " ", " ") + require.NoError(t, err) + t.Logf("exchanged token claims:\n%s", string(indentedClaims)) +} From b04db6ad2b6d1149a98bfada79e119c3c1ba98b8 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 10:42:37 -0600 Subject: [PATCH 10/17] Fix some false positive gosec warnings. Signed-off-by: Matt Moyer --- internal/oidc/token_exchange.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index a344ed773..bc97bfe1c 100644 --- a/internal/oidc/token_exchange.go +++ b/internal/oidc/token_exchange.go @@ -15,8 +15,8 @@ import ( ) const ( - tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" - tokenTypeJWT = "urn:ietf:params:oauth:token-type:jwt" + tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" //nolint: gosec + tokenTypeJWT = "urn:ietf:params:oauth:token-type:jwt" //nolint: gosec ) type stsParams struct { From 02d96d731fb1906cc381ad6c3c68956df13230c7 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 13:56:53 -0600 Subject: [PATCH 11/17] Finish TestTokenExchange unit tests and add missing scope check. Signed-off-by: Margo Crawford --- internal/oidc/token/token_handler_test.go | 256 ++++++++++++++++++++-- internal/oidc/token_exchange.go | 25 ++- 2 files changed, 256 insertions(+), 25 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index a134410e4..0cf67f663 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/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 - modifyTokenExchangeRequest func(r *http.Request) - requestedAudience string + authcodeExchange authcodeExchangeInputs + modifyRequestParams func(t *testing.T, params url.Values) + 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). - // Can use modifyTokenExchangeRequest to change to request params for the token exchange call (e.g. bad access token or wrong http verb). - // Will need to add some more fields to the test table to declare the expected values of the error response, etc. + { + name: "missing audience", + 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: "", + 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 - // e.g. that nothing was changed in the fosite k8s storage - // e.g. that the response body is correct, and the decoded token has the right claims + // The remaining assertions apply only the the happy path. + if rsp.Code != http.StatusOK { + 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, diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index bc97bfe1c..a47d9cd6e 100644 --- a/internal/oidc/token_exchange.go +++ b/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 - tokenTypeJWT = "urn:ietf:params:oauth:token-type:jwt" //nolint: gosec + tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" //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 } From 016b0e9a8ec3cbab16163c7a3f78e373b6f1eace Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 14:41:27 -0600 Subject: [PATCH 12/17] =?UTF-8?q?Satisfy=20the=20pedantic=20linter=20confi?= =?UTF-8?q?g=20=F0=9F=99=83.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Matt Moyer --- internal/oidc/token/token_handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 0cf67f663..daab27f78 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -993,7 +993,7 @@ type singleUseJWKProvider struct { } func (s *singleUseJWKProvider) GetJWKS(issuerName string) (jwks *jose.JSONWebKeySet, activeJWK *jose.JSONWebKey) { - s.calls += 1 + s.calls++ if s.calls > 1 { return nil, nil } From 3e6ebab389f94af97705e0f79f753e511377210f Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 14:49:44 -0600 Subject: [PATCH 13/17] Clean up TestTokenExchange a bit. Signed-off-by: Matt Moyer --- internal/oidc/token/token_handler_test.go | 132 +++++----------------- 1 file changed, 30 insertions(+), 102 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 87a68ec66..1de65f0a9 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -667,6 +667,19 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { } func TestTokenExchange(t *testing.T) { + successfulAuthCodeExchange := tokenEndpointResponseExpectedValues{ + 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"}, + } + + doValidAuthCodeExchange := authcodeExchangeInputs{ + modifyAuthRequest: func(authRequest *http.Request) { + authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") + }, + want: successfulAuthCodeExchange, + } tests := []struct { name string @@ -679,51 +692,21 @@ func TestTokenExchange(t *testing.T) { wantResponseBodyContains string }{ { - name: "happy path", - authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "happy path", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", wantStatus: http.StatusOK, }, { - name: "missing audience", - authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "missing audience", + authcodeExchange: doValidAuthCodeExchange, 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") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "missing subject_token", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyRequestParams: func(t *testing.T, params url.Values) { params.Del("subject_token") @@ -732,18 +715,8 @@ func TestTokenExchange(t *testing.T) { 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") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "wrong subject_token_type", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyRequestParams: func(t *testing.T, params url.Values) { params.Set("subject_token_type", "invalid") @@ -752,18 +725,8 @@ func TestTokenExchange(t *testing.T) { 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") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "wrong requested_token_type", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyRequestParams: func(t *testing.T, params url.Values) { params.Set("requested_token_type", "invalid") @@ -772,18 +735,8 @@ func TestTokenExchange(t *testing.T) { 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") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "unsupported RFC8693 parameter", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyRequestParams: func(t *testing.T, params url.Values) { params.Set("resource", "some-resource-parameter-value") @@ -792,18 +745,8 @@ func TestTokenExchange(t *testing.T) { wantResponseBodyContains: `unsupported parameter resource`, }, { - name: "bogus access token", - authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(authRequest *http.Request) { - authRequest.Form.Set("scope", "openid pinniped.sts.unrestricted") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "bogus access token", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyRequestParams: func(t *testing.T, params url.Values) { params.Set("subject_token", "some-bogus-value") @@ -812,18 +755,8 @@ func TestTokenExchange(t *testing.T) { 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") - }, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, - }, + name: "valid access token, but deleted from storage", + authcodeExchange: doValidAuthCodeExchange, requestedAudience: "some-workload-cluster", modifyStorage: func(t *testing.T, storage *oidc.KubeStorage, pendingRequest *http.Request) { parts := strings.Split(pendingRequest.Form.Get("subject_token"), ".") @@ -858,12 +791,7 @@ func TestTokenExchange(t *testing.T) { }, // Fail to fetch a JWK signing key after the authcode exchange has happened. makeOathHelper: makeOauthHelperWithJWTKeyThatWorksOnlyOnce, - want: tokenEndpointResponseExpectedValues{ - 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"}, - }, + want: successfulAuthCodeExchange, }, requestedAudience: "some-workload-cluster", wantStatus: http.StatusServiceUnavailable, From 167d440b6590d8654e83fb3874a05624a23eabd9 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 9 Dec 2020 14:51:27 -0600 Subject: [PATCH 14/17] Remove this unneccesary go113 `nolint` directives. We disabled this linter across the project. Signed-off-by: Matt Moyer --- internal/crud/crud.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/crud/crud.go b/internal/crud/crud.go index 77160e52f..dee3f49b2 100644 --- a/internal/crud/crud.go +++ b/internal/crud/crud.go @@ -127,14 +127,12 @@ func (s *secretsStorage) DeleteByLabel(ctx context.Context, labelName string, la }.String(), }) if err != nil { - //nolint:err113 // there's nothing wrong with this error return fmt.Errorf(`failed to list secrets for resource "%s" matching label "%s=%s": %w`, s.resource, labelName, labelValue, err) } // TODO try to delete all of the items and consolidate all of the errors and return them all for _, secret := range list.Items { err = s.secrets.Delete(ctx, secret.Name, metav1.DeleteOptions{}) if err != nil { - //nolint:err113 // there's nothing wrong with this error return fmt.Errorf(`failed to delete secrets for resource "%s" matching label "%s=%s" with name %s: %w`, s.resource, labelName, labelValue, secret.Name, err) } } From 0abadddb1affce0a94edb7b848aa3dc9e3888bb0 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Dec 2020 15:03:52 -0800 Subject: [PATCH 15/17] token_handler_test.go: modify a test about refresh request scopes param Signed-off-by: Margo Crawford --- internal/oidc/token/token_handler_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 3f124ff6a..d642ad110 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -922,25 +922,25 @@ func TestRefreshGrant(t *testing.T) { }}, }, { - name: "when the refresh request removes a scope which was originally granted from the list of requested scopes then it is ignored", + name: "when the refresh request removes a scope which was originally granted from the list of requested scopes then it is granted anyway", authcodeExchange: authcodeExchangeInputs{ - modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access pinniped.sts.unrestricted") }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "pinniped.sts.unrestricted"}, + wantGrantedScopes: []string{"openid", "offline_access", "pinniped.sts.unrestricted"}, }, }, 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. + r.Body = happyRefreshRequestBody(refreshToken).WithScope("openid").ReadCloser() // do not ask for "pinniped.sts.unrestricted" again }, 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"}, + wantRequestedScopes: []string{"openid", "offline_access", "pinniped.sts.unrestricted"}, + wantGrantedScopes: []string{"openid", "offline_access", "pinniped.sts.unrestricted"}, }}, }, { From 5b7c5105777a30cd6ad4eb7336865a5ec3ba790d Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Dec 2020 15:15:50 -0800 Subject: [PATCH 16/17] Fixed error handling for token exchange when openid scope missing Signed-off-by: Margo Crawford --- internal/oidc/token/token_handler_test.go | 19 ++++++++++++++++++- internal/oidc/token_exchange.go | 7 +++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index d642ad110..b5e510188 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -753,7 +753,7 @@ func TestTokenExchange(t *testing.T) { wantResponseBodyContains: `invalid subject_token`, }, { - name: "access token missing required scopes", + name: "access token missing pinniped.sts.unrestricted scope", authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(authRequest *http.Request) { authRequest.Form.Set("scope", "openid") @@ -769,6 +769,23 @@ func TestTokenExchange(t *testing.T) { wantStatus: http.StatusForbidden, wantResponseBodyContains: `missing the \"pinniped.sts.unrestricted\" scope`, }, + { + name: "access token missing openid scope", + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(authRequest *http.Request) { + authRequest.Form.Set("scope", "pinniped.sts.unrestricted") + }, + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"pinniped.sts.unrestricted"}, + wantGrantedScopes: []string{"pinniped.sts.unrestricted"}, + }, + }, + requestedAudience: "some-workload-cluster", + wantStatus: http.StatusForbidden, + wantResponseBodyContains: `missing the \"openid\" scope`, + }, { name: "token minting failure", authcodeExchange: authcodeExchangeInputs{ diff --git a/internal/oidc/token_exchange.go b/internal/oidc/token_exchange.go index a47d9cd6e..36ddfb83b 100644 --- a/internal/oidc/token_exchange.go +++ b/internal/oidc/token_exchange.go @@ -65,10 +65,13 @@ 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) { + // Require that the incoming access token has the STS and OpenID scopes. + if !originalRequester.GetGrantedScopes().Has(pinnipedTokenExchangeScope) { return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", pinnipedTokenExchangeScope)) } + if !originalRequester.GetGrantedScopes().Has(oidc.ScopeOpenID) { + return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", oidc.ScopeOpenID)) + } // Use the original authorize request information, along with the requested audience, to mint a new JWT. responseToken, err := t.mintJWT(ctx, originalRequester, params.requestedAudience) From 218f27306cc373748889ce9021f1e31c65877665 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 9 Dec 2020 17:07:37 -0800 Subject: [PATCH 17/17] Integration test for refresh grant Signed-off-by: Ryan Richard --- test/integration/supervisor_login_test.go | 49 ++++++++++++++++++----- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 5c8827786..221978d47 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -126,7 +126,7 @@ func TestSupervisorLogin(t *testing.T) { ClientID: "pinniped-cli", Endpoint: discovery.Endpoint(), RedirectURL: localCallbackServer.URL, - Scopes: []string{"openid", "pinniped.sts.unrestricted"}, + Scopes: []string{"openid", "pinniped.sts.unrestricted", "offline_access"}, } // Build a valid downstream authorize URL for the supervisor. @@ -160,7 +160,7 @@ func TestSupervisorLogin(t *testing.T) { callback := localCallbackServer.waitForCallback(10 * time.Second) t.Logf("got callback request: %s", library.MaskTokens(callback.URL.String())) require.Equal(t, stateParam.String(), callback.URL.Query().Get("state")) - require.Equal(t, "openid pinniped.sts.unrestricted", callback.URL.Query().Get("scope")) + require.ElementsMatch(t, []string{"openid", "pinniped.sts.unrestricted", "offline_access"}, strings.Split(callback.URL.Query().Get("scope"), " ")) authcode := callback.URL.Query().Get("code") require.NotEmpty(t, authcode) @@ -168,6 +168,40 @@ func TestSupervisorLogin(t *testing.T) { tokenResponse, err := downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier()) require.NoError(t, err) + expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat"} + verifyTokenResponse(t, tokenResponse, discovery, downstreamOAuth2Config, env.SupervisorTestUpstream.Issuer, nonceParam, expectedIDTokenClaims) + + // token exchange on the original token + doTokenExchange(t, &downstreamOAuth2Config, tokenResponse, httpClient, discovery) + + // Use the refresh token to get new tokens + refreshSource := downstreamOAuth2Config.TokenSource(oidcHTTPClientContext, &oauth2.Token{RefreshToken: tokenResponse.RefreshToken}) + refreshedTokenResponse, err := refreshSource.Token() + require.NoError(t, err) + + expectedIDTokenClaims = append(expectedIDTokenClaims, "at_hash") + verifyTokenResponse(t, refreshedTokenResponse, discovery, downstreamOAuth2Config, env.SupervisorTestUpstream.Issuer, "", expectedIDTokenClaims) + + require.NotEqual(t, tokenResponse.AccessToken, refreshedTokenResponse.AccessToken) + require.NotEqual(t, tokenResponse.RefreshToken, refreshedTokenResponse.RefreshToken) + require.NotEqual(t, tokenResponse.Extra("id_token"), refreshedTokenResponse.Extra("id_token")) + + // token exchange on the refreshed token + doTokenExchange(t, &downstreamOAuth2Config, refreshedTokenResponse, httpClient, discovery) +} + +func verifyTokenResponse( + t *testing.T, + tokenResponse *oauth2.Token, + discovery *oidc.Provider, + downstreamOAuth2Config oauth2.Config, + upstreamIssuerName string, + nonceParam nonce.Nonce, + expectedIDTokenClaims []string, +) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + // Verify the ID Token. rawIDToken, ok := tokenResponse.Extra("id_token").(string) require.True(t, ok, "expected to get an ID token but did not") @@ -176,7 +210,7 @@ func TestSupervisorLogin(t *testing.T) { require.NoError(t, err) // Check the claims of the ID token. - expectedSubjectPrefix := env.SupervisorTestUpstream.Issuer + "?sub=" + expectedSubjectPrefix := upstreamIssuerName + "?sub=" require.True(t, strings.HasPrefix(idToken.Subject, expectedSubjectPrefix)) require.Greater(t, len(idToken.Subject), len(expectedSubjectPrefix), "the ID token Subject should include the upstream user ID after the upstream issuer name") @@ -189,7 +223,7 @@ func TestSupervisorLogin(t *testing.T) { for k := range idTokenClaims { idTokenClaimNames = append(idTokenClaimNames, k) } - require.ElementsMatch(t, []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat"}, idTokenClaimNames) + require.ElementsMatch(t, expectedIDTokenClaims, idTokenClaimNames) // Some light verification of the other tokens that were returned. require.NotEmpty(t, tokenResponse.AccessToken) @@ -197,9 +231,7 @@ func TestSupervisorLogin(t *testing.T) { require.NotZero(t, tokenResponse.Expiry) testutil.RequireTimeInDelta(t, time.Now().UTC().Add(time.Minute*5), tokenResponse.Expiry, time.Second*30) - require.Empty(t, tokenResponse.RefreshToken) // for now, until the next user story :) - - wipDoTokenExchange(t, &downstreamOAuth2Config, tokenResponse, httpClient, discovery) // WIP while we work on the token exchange server. + require.NotEmpty(t, tokenResponse.RefreshToken) } func startLocalCallbackServer(t *testing.T) *localCallbackServer { @@ -230,8 +262,7 @@ func (s *localCallbackServer) waitForCallback(timeout time.Duration) *http.Reque } } -// WIP test code for the token exchange flow. -func wipDoTokenExchange(t *testing.T, config *oauth2.Config, tokenResponse *oauth2.Token, httpClient *http.Client, provider *oidc.Provider) { +func doTokenExchange(t *testing.T, config *oauth2.Config, tokenResponse *oauth2.Token, httpClient *http.Client, provider *oidc.Provider) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel()