From dff53b81442604bee1f4ff6adefc1e7f2787e14b Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 19 Jan 2022 13:57:01 -0800 Subject: [PATCH] Changes for Fosite's new RevokeRefreshTokenMaybeGracePeriod() interface Fosite v0.42.0 introduced a new RevokeRefreshTokenMaybeGracePeriod() interface function. Updated our code to support this change. We didn't support grace periods on refresh tokens before, so implemented it by making the new RevokeRefreshTokenMaybeGracePeriod() method just call the old RevokeRefreshToken() method, therefore keeping our old behavior. --- .../refreshtoken/refreshtoken.go | 8 ++- .../refreshtoken/refreshtoken_test.go | 56 +++++++++++++++++++ internal/oidc/kube_storage.go | 4 ++ internal/oidc/nullstorage.go | 6 +- 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/internal/fositestorage/refreshtoken/refreshtoken.go b/internal/fositestorage/refreshtoken/refreshtoken.go index 8faddd5bc..a2a2fe89a 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken.go +++ b/internal/fositestorage/refreshtoken/refreshtoken.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package refreshtoken @@ -35,6 +35,7 @@ const ( type RevocationStorage interface { oauth2.RefreshTokenStorage RevokeRefreshToken(ctx context.Context, requestID string) error + RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error } var _ RevocationStorage = &refreshTokenStorage{} @@ -73,6 +74,11 @@ func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID return a.storage.DeleteByLabel(ctx, fositestorage.StorageRequestIDLabelName, requestID) } +func (a *refreshTokenStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error { + // We don't support a grace period, so always call the regular RevokeRefreshToken(). + return a.RevokeRefreshToken(ctx, requestID) +} + func (a *refreshTokenStorage) CreateRefreshTokenSession(ctx context.Context, signature string, requester fosite.Requester) error { request, err := fositestorage.ValidateAndExtractAuthorizeRequest(requester) if err != nil { diff --git a/internal/fositestorage/refreshtoken/refreshtoken_test.go b/internal/fositestorage/refreshtoken/refreshtoken_test.go index 36af0559c..0ec849a52 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken_test.go +++ b/internal/fositestorage/refreshtoken/refreshtoken_test.go @@ -162,6 +162,62 @@ func TestRefreshTokenStorageRevocation(t *testing.T) { require.Equal(t, wantActions, client.Actions()) } +func TestRefreshTokenStorageRevokeRefreshTokenMaybeGracePeriod(t *testing.T) { + wantActions := []coretesting.Action{ + coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev/type": "refresh-token", + "storage.pinniped.dev/request-id": "abcd-1", + }, + Annotations: map[string]string{ + "storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString, + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/refresh-token", + }), + coretesting.NewListAction(secretsGVR, schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Secret"}, namespace, metav1.ListOptions{ + LabelSelector: "storage.pinniped.dev/type=refresh-token,storage.pinniped.dev/request-id=abcd-1", + }), + coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"), + } + + ctx, client, _, storage := makeTestSubject() + + request := &fosite.Request{ + ID: "abcd-1", + RequestedAt: time.Time{}, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Public: true, + }, + JSONWebKeysURI: "where", + TokenEndpointAuthMethod: "something", + }, + }, + Form: url.Values{"key": []string{"val"}}, + Session: testutil.NewFakePinnipedSession(), + } + err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", request) + require.NoError(t, err) + + // Revoke the request ID of the session that we just created. We don't support grace periods, so this + // should work exactly like the regular RevokeRefreshToken() function. + err = storage.RevokeRefreshTokenMaybeGracePeriod(ctx, "abcd-1", "fancy-signature") + require.NoError(t, err) + + testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed + require.Equal(t, wantActions, client.Actions()) +} + func TestGetNotFound(t *testing.T) { ctx, _, _, storage := makeTestSubject() diff --git a/internal/oidc/kube_storage.go b/internal/oidc/kube_storage.go index 8480c5949..0de1731b7 100644 --- a/internal/oidc/kube_storage.go +++ b/internal/oidc/kube_storage.go @@ -180,6 +180,10 @@ func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) e return k.refreshTokenStorage.RevokeRefreshToken(ctx, requestID) } +func (k KubeStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error { + return k.refreshTokenStorage.RevokeRefreshTokenMaybeGracePeriod(ctx, requestID, signature) +} + // // OAuth client definitions: // diff --git a/internal/oidc/nullstorage.go b/internal/oidc/nullstorage.go index c782b8482..b12b2099c 100644 --- a/internal/oidc/nullstorage.go +++ b/internal/oidc/nullstorage.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidc @@ -25,6 +25,10 @@ func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error { return errNullStorageNotImplemented } +func (NullStorage) RevokeRefreshTokenMaybeGracePeriod(_ context.Context, _ string, _ string) error { + return errNullStorageNotImplemented +} + func (NullStorage) RevokeAccessToken(_ context.Context, _ string) error { return errNullStorageNotImplemented }