Supervisor token endpoint returns refresh tokens when requested

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

internal/oidc/oidc.go

@@ -84,7 +84,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", "refresh_token"},
 			Scopes:        []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email"},
 		},
 		TokenEndpointAuthMethod: "none",
@@ -112,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(

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())
 			}
@@ -488,7 +520,9 @@ func TestTokenEndpoint(t *testing.T) {
 	}
 
 	t.Run("auth code is used twice", func(t *testing.T) {
+		// TODO upgrade this test to use offline_access so we can be sure that the refresh token was also revoked
 		authRequest := deepCopyRequestForm(happyAuthRequest)
+		wantRequestedScopes := []string{"openid", "profile", "email"}
 
 		client := fake.NewSimpleClientset()
 		secrets := client.CoreV1().Secrets("some-namespace")
@@ -513,25 +547,27 @@ 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(m))
+		require.ElementsMatch(t, wantBodyFields, getMapKeys(parsedResponseBody))
+
+		requireValidIDToken(t, parsedResponseBody, jwtSigningKey)
 
 		code := req.PostForm.Get("code")
-		wantOpenidScope := true
+		wantGrantedOpenidScope := true
+		wantGrantedOfflineAccessScope := false
 		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: openidconnect.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)
 
 		// Second call - should be unsuccessful since auth code was already used.
@@ -546,16 +582,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)
+		// 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 these up, so it is still there
+		requireValidOIDCStorage(t, parsedResponseBody, code, oauthStore, wantRequestedScopes, wantGrantedOpenidScope, wantGrantedOfflineAccessScope)
 
+		// Check that the access token storage was 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 +712,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 +749,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 +795,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 +814,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 +875,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 +893,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 +908,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 {