Merge remote-tracking branch 'origin/token-refresh' into token-exchange-endpoint

Signed-off-by: Matt Moyer <moyerm@vmware.com>

cmd/pinniped/cmd/login_oidc.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/coreos/go-oidc"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1"
@@ -49,7 +50,7 @@ func oidcLoginCommand(loginFunc func(issuer string, clientID string, opts ...oid
 	cmd.Flags().StringVar(&issuer, "issuer", "", "OpenID Connect issuer URL.")
 	cmd.Flags().StringVar(&clientID, "client-id", "", "OpenID Connect client ID.")
 	cmd.Flags().Uint16Var(&listenPort, "listen-port", 0, "TCP port for localhost listener (authorization code flow only).")
-	cmd.Flags().StringSliceVar(&scopes, "scopes", []string{"offline_access", "openid"}, "OIDC scopes to request during login.")
+	cmd.Flags().StringSliceVar(&scopes, "scopes", []string{oidc.ScopeOfflineAccess, oidc.ScopeOpenID}, "OIDC scopes to request during login.")
 	cmd.Flags().BoolVar(&skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL).")
 	cmd.Flags().StringVar(&sessionCachePath, "session-cache", filepath.Join(mustGetConfigDir(), "sessions.yaml"), "Path to session cache file.")
 	cmd.Flags().StringSliceVar(&caBundlePaths, "ca-bundle", nil, "Path to TLS certificate authority bundle (PEM format, optional, can be repeated).")

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 {

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,

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.
+		// Automatically grant the openid and offline_access scopes, but only if they were requested.
-		grantOpenIDScopeIfRequested(authorizeRequester)
+		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{},

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.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes())
-		require.Contains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid")
-	} else {
-		require.NotContains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid")
-	}
 
 	// Check all the other fields of the stored request.
 	require.NotEmpty(t, storedRequestFromAuthcode.ID)

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) {
+func (k KubeStorage) GetRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, session fosite.Session) (request fosite.Requester, err error) {
-	return k.refreshTokenStorage.GetRefreshTokenSession(ctx, signature, session)
+	return k.refreshTokenStorage.GetRefreshTokenSession(ctx, signatureOfRefreshToken, session)
 }
 
-func (k KubeStorage) DeleteRefreshTokenSession(ctx context.Context, signature string) (err error) {
+func (k KubeStorage) DeleteRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string) (err error) {
-	return k.refreshTokenStorage.DeleteRefreshTokenSession(ctx, signature)
+	return k.refreshTokenStorage.DeleteRefreshTokenSession(ctx, signatureOfRefreshToken)
 }
 
-func (k KubeStorage) CreateAccessTokenSession(ctx context.Context, signature string, requester fosite.Requester) (err error) {
+func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) error {
-	return k.accessTokenStorage.CreateAccessTokenSession(ctx, signature, requester)
+	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)
+// 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
-func (k KubeStorage) DeleteAccessTokenSession(ctx context.Context, signature string) (err error) {
+// storage mechanism to fetch them.
-	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)
-}
 
 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
 }

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"},
+				GrantTypes:    []string{"authorization_code", "refresh_token"},
-				Scopes:        []string{"openid", "profile", "email"},
+				Scopes:        []string{"openid", "offline_access", "profile", "email"},
 			},
 			TokenEndpointAuthMethod: "none",
 		},

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"
 
@@ -84,7 +85,7 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient {
 			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{"openid", "profile", "email"},
+			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
+		RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess}, // as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
-		MinParameterEntropy: 32,  // 256 bits seems about right
+
+		MinParameterEntropy: 32, // 256 bits seems about right
 	}
 
 	return compose.Compose(
@@ -157,3 +159,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)
+		}
+	}
+}

internal/oidc/token/token_handler_test.go

@@ -211,8 +211,9 @@ func TestTokenEndpoint(t *testing.T) {
 	tests := []struct {
 		name string
 
-		authRequest func(authRequest *http.Request)
+		modifyAuthRequest  func(authRequest *http.Request)
-		storage     func(
+		modifyTokenRequest func(r *http.Request, authCode string)
+		modifyStorage      func(
 			t *testing.T,
 			s interface {
 				oauth2.TokenRevocationStorage
@@ -223,7 +224,6 @@ func TestTokenEndpoint(t *testing.T) {
 			},
 			authCode string,
 		)
-		request        func(r *http.Request, authCode string)
 		makeOathHelper func(
 			t *testing.T,
 			authRequest *http.Request,
@@ -236,73 +236,94 @@ func TestTokenEndpoint(t *testing.T) {
 			},
 		) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey)
 
-		wantStatus     int
+		wantStatus          int
-		wantBodyFields []string
+		wantBodyFields      []string
-		wantExactBody  string
+		wantExactBody       string
+		wantRequestedScopes []string
 	}{
 		// happy path
 		{
-			name:           "request is valid and tokens are issued",
+			name:                "request is valid and tokens are issued",
-			wantStatus:     http.StatusOK,
+			wantStatus:          http.StatusOK,
-			wantBodyFields: []string{"id_token", "access_token", "token_type", "scope", "expires_in"},
+			wantBodyFields:      []string{"id_token", "access_token", "token_type", "scope", "expires_in"}, // no refresh token
+			wantRequestedScopes: []string{"openid", "profile", "email"},
 		},
 		{
 			name: "openid scope was not requested from authorize endpoint",
-			authRequest: func(authRequest *http.Request) {
+			modifyAuthRequest: func(authRequest *http.Request) {
 				authRequest.Form.Set("scope", "profile email")
 			},
-			wantStatus:     http.StatusOK,
+			wantStatus:          http.StatusOK,
-			wantBodyFields: []string{"access_token", "token_type", "scope", "expires_in"},
+			wantBodyFields:      []string{"access_token", "token_type", "scope", "expires_in"}, // no id or refresh tokens
+			wantRequestedScopes: []string{"profile", "email"},
+		},
+		{
+			name: "offline_access and openid scopes were requested and granted from authorize endpoint",
+			modifyAuthRequest: func(authRequest *http.Request) {
+				authRequest.Form.Set("scope", "openid offline_access")
+			},
+			wantStatus:          http.StatusOK,
+			wantBodyFields:      []string{"id_token", "access_token", "token_type", "scope", "expires_in", "refresh_token"}, // all possible tokens
+			wantRequestedScopes: []string{"openid", "offline_access"},
+		},
+		{
+			name: "offline_access (without openid scope) was requested and granted from authorize endpoint",
+			modifyAuthRequest: func(authRequest *http.Request) {
+				authRequest.Form.Set("scope", "offline_access")
+			},
+			wantStatus:          http.StatusOK,
+			wantBodyFields:      []string{"access_token", "token_type", "scope", "expires_in", "refresh_token"}, // no id token
+			wantRequestedScopes: []string{"offline_access"},
 		},
 
 		// sad path
 		{
-			name:          "GET method is wrong",
+			name:               "GET method is wrong",
-			request:       func(r *http.Request, authCode string) { r.Method = http.MethodGet },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodGet },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeInvalidMethodErrorBody("GET"),
+			wantExactBody:      fositeInvalidMethodErrorBody("GET"),
 		},
 		{
-			name:          "PUT method is wrong",
+			name:               "PUT method is wrong",
-			request:       func(r *http.Request, authCode string) { r.Method = http.MethodPut },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodPut },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeInvalidMethodErrorBody("PUT"),
+			wantExactBody:      fositeInvalidMethodErrorBody("PUT"),
 		},
 		{
-			name:          "PATCH method is wrong",
+			name:               "PATCH method is wrong",
-			request:       func(r *http.Request, authCode string) { r.Method = http.MethodPatch },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodPatch },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeInvalidMethodErrorBody("PATCH"),
+			wantExactBody:      fositeInvalidMethodErrorBody("PATCH"),
 		},
 		{
-			name:          "DELETE method is wrong",
+			name:               "DELETE method is wrong",
-			request:       func(r *http.Request, authCode string) { r.Method = http.MethodDelete },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Method = http.MethodDelete },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeInvalidMethodErrorBody("DELETE"),
+			wantExactBody:      fositeInvalidMethodErrorBody("DELETE"),
 		},
 		{
-			name:          "content type is invalid",
+			name:               "content type is invalid",
-			request:       func(r *http.Request, authCode string) { r.Header.Set("Content-Type", "text/plain") },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Header.Set("Content-Type", "text/plain") },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeEmptyPayloadErrorBody,
+			wantExactBody:      fositeEmptyPayloadErrorBody,
 		},
 		{
 			name: "payload is not valid form serialization",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = ioutil.NopCloser(strings.NewReader("this newline character is not allowed in a form serialization: \n"))
 			},
 			wantStatus:    http.StatusBadRequest,
 			wantExactBody: fositeMissingGrantTypeErrorBody,
 		},
 		{
-			name:          "payload is empty",
+			name:               "payload is empty",
-			request:       func(r *http.Request, authCode string) { r.Body = nil },
+			modifyTokenRequest: func(r *http.Request, authCode string) { r.Body = nil },
-			wantStatus:    http.StatusBadRequest,
+			wantStatus:         http.StatusBadRequest,
-			wantExactBody: fositeInvalidPayloadErrorBody,
+			wantExactBody:      fositeInvalidPayloadErrorBody,
 		},
 		{
 			name: "grant type is missing in request",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithGrantType("").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -310,7 +331,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "grant type is not authorization_code",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithGrantType("bogus").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -318,7 +339,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "client id is missing in request",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithClientID("").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -326,7 +347,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "client id is wrong",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithClientID("bogus").ReadCloser()
 			},
 			wantStatus:    http.StatusUnauthorized,
@@ -334,7 +355,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "auth code is missing in request",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithAuthCode("").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -342,7 +363,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "auth code has never been valid",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithAuthCode("bogus").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -350,7 +371,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "auth code is invalidated",
-			storage: func(
+			modifyStorage: func(
 				t *testing.T,
 				s interface {
 					oauth2.TokenRevocationStorage
@@ -369,7 +390,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "redirect uri is missing in request",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithRedirectURI("").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -377,7 +398,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "redirect uri is wrong",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithRedirectURI("bogus").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -385,7 +406,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "pkce is missing in request",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: func(r *http.Request, authCode string) {
 				r.Body = happyBody(authCode).WithPKCE("").ReadCloser()
 			},
 			wantStatus:    http.StatusBadRequest,
@@ -393,7 +414,7 @@ func TestTokenEndpoint(t *testing.T) {
 		},
 		{
 			name: "pkce is wrong",
-			request: func(r *http.Request, authCode string) {
+			modifyTokenRequest: 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()
@@ -412,8 +433,8 @@ func TestTokenEndpoint(t *testing.T) {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
 			authRequest := deepCopyRequestForm(happyAuthRequest)
-			if test.authRequest != nil {
+			if test.modifyAuthRequest != nil {
-				test.authRequest(authRequest)
+				test.modifyAuthRequest(authRequest)
 			}
 
 			client := fake.NewSimpleClientset()
@@ -430,24 +451,26 @@ func TestTokenEndpoint(t *testing.T) {
 				oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore)
 			}
 
-			if test.storage != nil {
+			if test.modifyStorage != nil {
-				test.storage(t, oauthStore, authCode)
+				test.modifyStorage(t, oauthStore, authCode)
 			}
 			subject := NewHandler(oauthHelper)
 
+			authorizeEndpointGrantedOpenIDScope := strings.Contains(authRequest.Form.Get("scope"), "openid")
+			expectedNumberOfIDSessionsStored := 0
+			if authorizeEndpointGrantedOpenIDScope {
+				expectedNumberOfIDSessionsStored = 1
+			}
+
 			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}, expectedNumberOfIDSessionsStored)
-				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+			testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2+expectedNumberOfIDSessionsStored)
-				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())
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-			if test.request != nil {
+			if test.modifyTokenRequest != nil {
-				test.request(req, authCode)
+				test.modifyTokenRequest(req, authCode)
 			}
 			rsp := httptest.NewRecorder()
 
@@ -458,29 +481,38 @@ func TestTokenEndpoint(t *testing.T) {
 			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{}
+				var parsedResponseBody map[string]interface{}
-				require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &m))
+				require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody))
-				require.ElementsMatch(t, test.wantBodyFields, getMapKeys(m))
+				require.ElementsMatch(t, test.wantBodyFields, getMapKeys(parsedResponseBody))
+
+				wantIDToken := contains(test.wantBodyFields, "id_token")
+				wantRefreshToken := contains(test.wantBodyFields, "refresh_token")
 
 				code := req.PostForm.Get("code")
-				wantOpenidScope := contains(test.wantBodyFields, "id_token")
 				requireInvalidAuthCodeStorage(t, code, oauthStore)
-				requireValidAccessTokenStorage(t, m, oauthStore, wantOpenidScope)
+				requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, wantIDToken, wantRefreshToken)
 				requireInvalidPKCEStorage(t, code, oauthStore)
-				requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
+				requireValidOIDCStorage(t, parsedResponseBody, code, oauthStore, test.wantRequestedScopes, wantIDToken, wantRefreshToken)
+
+				expectedNumberOfRefreshTokenSessionsStored := 0
+				if wantRefreshToken {
+					expectedNumberOfRefreshTokenSessionsStored = 1
+				}
+				expectedNumberOfIDSessionsStored = 0
+				if wantIDToken {
+					expectedNumberOfIDSessionsStored = 1
+					requireValidIDToken(t, parsedResponseBody, jwtSigningKey)
+				}
+				if wantRefreshToken {
+					requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, wantIDToken, wantRefreshToken)
+				}
 
 				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: refreshtoken.TypeLabelValue}, expectedNumberOfRefreshTokenSessionsStored)
-				if wantOpenidScope {
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, expectedNumberOfIDSessionsStored)
-					requireValidIDToken(t, m, jwtSigningKey)
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2+expectedNumberOfRefreshTokenSessionsStored+expectedNumberOfIDSessionsStored)
-					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)
-				}
 			} else {
 				require.JSONEq(t, test.wantExactBody, rsp.Body.String())
 			}
@@ -489,6 +521,12 @@ func TestTokenEndpoint(t *testing.T) {
 
 	t.Run("auth code is used twice", func(t *testing.T) {
 		authRequest := deepCopyRequestForm(happyAuthRequest)
+		authRequest.Form.Set("scope", "openid offline_access profile email")
+
+		wantRequestedScopes := []string{"openid", "offline_access", "profile", "email"}
+		wantBodyFields := []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}
+		wantGrantedOpenidScope := true
+		wantGrantedOfflineAccessScope := true
 
 		client := fake.NewSimpleClientset()
 		secrets := client.CoreV1().Secrets("some-namespace")
@@ -513,26 +551,25 @@ func TestTokenEndpoint(t *testing.T) {
 		testutil.RequireEqualContentType(t, rsp0.Header().Get("Content-Type"), "application/json")
 		require.Equal(t, http.StatusOK, rsp0.Code)
 
-		var m map[string]interface{}
+		var parsedResponseBody map[string]interface{}
-		require.NoError(t, json.Unmarshal(rsp0.Body.Bytes(), &m))
+		require.NoError(t, json.Unmarshal(rsp0.Body.Bytes(), &parsedResponseBody))
 
-		wantBodyFields := []string{"id_token", "access_token", "token_type", "expires_in", "scope"}
+		require.ElementsMatch(t, wantBodyFields, getMapKeys(parsedResponseBody))
-		require.ElementsMatch(t, wantBodyFields, getMapKeys(m))
+
+		requireValidIDToken(t, parsedResponseBody, jwtSigningKey)
 
 		code := req.PostForm.Get("code")
-		wantOpenidScope := true
 		requireInvalidAuthCodeStorage(t, code, oauthStore)
-		requireValidAccessTokenStorage(t, m, oauthStore, wantOpenidScope)
+		requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, wantRequestedScopes, wantGrantedOpenidScope, wantGrantedOfflineAccessScope)
 		requireInvalidPKCEStorage(t, code, oauthStore)
-		requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
+		requireValidOIDCStorage(t, parsedResponseBody, code, oauthStore, wantRequestedScopes, wantGrantedOpenidScope, wantGrantedOfflineAccessScope)
-		requireValidIDToken(t, m, jwtSigningKey)
 
 		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: accesstoken.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 4)
 
 		// Second call - should be unsuccessful since auth code was already used.
 		//
@@ -546,16 +583,21 @@ func TestTokenEndpoint(t *testing.T) {
 		testutil.RequireEqualContentType(t, rsp1.Header().Get("Content-Type"), "application/json")
 		require.JSONEq(t, fositeReusedAuthCodeErrorBody, rsp1.Body.String())
 
+		// This was previously invalidated by the first request, so it remains invalidated
 		requireInvalidAuthCodeStorage(t, code, oauthStore)
-		requireInvalidAccessTokenStorage(t, m, 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, code, oauthStore)
-		requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
+		// Fosite never cleans up OpenID Connect session storage, so it is still there
+		requireValidOIDCStorage(t, parsedResponseBody, code, oauthStore, wantRequestedScopes, wantGrantedOpenidScope, wantGrantedOfflineAccessScope)
 
+		// 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{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
 		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2)
 	})
 }
@@ -671,6 +713,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 +750,44 @@ func requireInvalidAuthCodeStorage(
 	require.True(t, errors.Is(err, fosite.ErrInvalidatedAuthorizeCode))
 }
 
+func requireValidRefreshTokenStorage(
+	t *testing.T,
+	body map[string]interface{},
+	storage oauth2.CoreStorage,
+	wantRequestedScopes []string,
+	wantGrantedOpenidScope bool,
+	wantGrantedOfflineAccessScope bool,
+) {
+	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,
+		wantGrantedOpenidScope,
+		wantGrantedOfflineAccessScope,
+		true,
+	)
+}
+
 func requireValidAccessTokenStorage(
 	t *testing.T,
 	body map[string]interface{},
 	storage oauth2.CoreStorage,
+	wantRequestedScopes []string,
 	wantGrantedOpenidScope bool,
+	wantGrantedOfflineAccessScope bool,
 ) {
 	t.Helper()
 
@@ -718,7 +796,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.
@@ -736,24 +815,33 @@ func requireValidAccessTokenStorage(
 
 	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 := ""
+	require.Equal(t, strings.Join(wantGrantedScopes(wantGrantedOpenidScope, wantGrantedOfflineAccessScope), " "), actualGrantedScopesString)
-	if wantGrantedOpenidScope {
-		wantScopes += "openid"
-	}
-	require.Equal(t, wantScopes, scopesString)
 
-	// Fosite stores access tokens without any of the original request form pararmeters.
+	// Fosite stores access tokens without any of the original request form parameters.
-	requireValidAuthRequest(
+	requireValidStoredRequest(
 		t,
-		authRequest,
+		storedRequest,
-		authRequest.Sanitize([]string{}).GetRequestForm(),
+		storedRequest.Sanitize([]string{}).GetRequestForm(),
+		wantRequestedScopes,
 		wantGrantedOpenidScope,
+		wantGrantedOfflineAccessScope,
 		true,
 	)
 }
 
+func wantGrantedScopes(wantGrantedOpenidScope, wantGrantedOfflineAccessScope bool) []string {
+	scopesWanted := []string{}
+	if wantGrantedOpenidScope {
+		scopesWanted = append(scopesWanted, "openid")
+	}
+	if wantGrantedOfflineAccessScope {
+		scopesWanted = append(scopesWanted, "offline_access")
+	}
+	return scopesWanted
+}
+
 func requireInvalidAccessTokenStorage(
 	t *testing.T,
 	body map[string]interface{},
@@ -788,13 +876,15 @@ func requireValidOIDCStorage(
 	body map[string]interface{},
 	code string,
 	storage openid.OpenIDConnectRequestStorage,
+	wantRequestedScopes []string,
 	wantGrantedOpenidScope bool,
+	wantGrantedOfflineAccessScope bool,
 ) {
 	t.Helper()
 
 	if wantGrantedOpenidScope {
 		// 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 +894,13 @@ 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,
+			storedRequest,
-			authRequest.Sanitize([]string{"nonce"}).GetRequestForm(),
+			storedRequest.Sanitize([]string{"nonce"}).GetRequestForm(),
-			true,
+			wantRequestedScopes,
+			wantGrantedOpenidScope,
+			wantGrantedOfflineAccessScope,
 			false,
 		)
 	} else {
@@ -817,34 +909,30 @@ func requireValidOIDCStorage(
 	}
 }
 
-func requireValidAuthRequest(
+func requireValidStoredRequest(
 	t *testing.T,
-	authRequest fosite.Requester,
+	request fosite.Requester,
 	wantRequestForm url.Values,
+	wantRequestedScopes []string,
 	wantGrantedOpenidScope bool,
+	wantGrantedOfflineAccessScope bool,
 	wantAccessTokenExpiresAt bool,
 ) {
 	t.Helper()
 
-	// Assert that the getters on the authRequest return what we think they should.
+	// Assert that the getters on the request return what we think they should.
-	wantRequestedScopes := []string{"profile", "email"}
+	require.NotEmpty(t, request.GetID())
-	wantGrantedScopes := []string{}
+	testutil.RequireTimeInDelta(t, request.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second)
-	if wantGrantedOpenidScope {
+	require.Equal(t, goodClient, request.GetClient().GetID())
-		wantRequestedScopes = append([]string{"openid"}, wantRequestedScopes...)
+	require.Equal(t, fosite.Arguments(wantRequestedScopes), request.GetRequestedScopes())
-		wantGrantedScopes = append([]string{"openid"}, wantGrantedScopes...)
+	require.Equal(t, fosite.Arguments(wantGrantedScopes(wantGrantedOpenidScope, wantGrantedOfflineAccessScope)), request.GetGrantedScopes())
-	}
+	require.Empty(t, request.GetRequestedAudience())
-	require.NotEmpty(t, authRequest.GetID())
+	require.Empty(t, request.GetGrantedAudience())
-	testutil.RequireTimeInDelta(t, authRequest.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second)
+	require.Equal(t, wantRequestForm, request.GetRequestForm()) // Fosite stores access token request without form
-	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
 
 	// Cast session to the type we think it should be.
-	session, ok := authRequest.GetSession().(*openid.DefaultSession)
+	session, ok := request.GetSession().(*openid.DefaultSession)
-	require.Truef(t, ok, "could not cast %T to %T", authRequest.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 {

pkg/oidcclient/login.go

@@ -174,7 +174,7 @@ func Login(issuer string, clientID string, opts ...Option) (*oidctypes.Token, er
 		issuer:       issuer,
 		clientID:     clientID,
 		listenAddr:   "localhost:0",
-		scopes:       []string{"offline_access", "openid", "email", "profile"},
+		scopes:       []string{oidc.ScopeOfflineAccess, oidc.ScopeOpenID, "email", "profile"},
 		cache:        &nopCache{},
 		callbackPath: "/callback",
 		ctx:          context.Background(),