From 5b7c5105777a30cd6ad4eb7336865a5ec3ba790d Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Dec 2020 15:15:50 -0800 Subject: [PATCH] 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 d642ad11..b5e51018 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 a47d9cd6..36ddfb83 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)