From af9612e98e9b6ddb65d5924f7195aca18e1e2e66 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 16 Apr 2024 14:55:13 -0700 Subject: [PATCH] Update more unit tests for configurable token lifetimes --- internal/crud/crud.go | 2 +- internal/crud/crud_test.go | 1 - .../clientregistry/clientregistry_test.go | 102 ++++++++++++--- .../idtokenlifespan/idtoken_lifespan_test.go | 77 ++++++++++++ internal/federationdomain/oidc/oidc_test.go | 117 ++++++++++++++++++ .../accesstoken/accesstoken_test.go | 39 ++++-- .../authorizationcode_test.go | 41 ++++-- .../openidconnect/openidconnect_test.go | 37 ++++-- internal/fositestorage/pkce/pkce_test.go | 34 +++-- .../refreshtoken/refreshtoken_test.go | 41 ++++-- 10 files changed, 416 insertions(+), 75 deletions(-) create mode 100644 internal/federationdomain/idtokenlifespan/idtoken_lifespan_test.go create mode 100644 internal/federationdomain/oidc/oidc_test.go diff --git a/internal/crud/crud.go b/internal/crud/crud.go index 0160bc182..03aff68e8 100644 --- a/internal/crud/crud.go +++ b/internal/crud/crud.go @@ -200,7 +200,7 @@ func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON, labelsToAdd[SecretLabelKey] = s.resource // make it easier to find this stuff via kubectl var annotations map[string]string - if lifetime > 0 && s.clock != nil { + if lifetime > 0 { annotations = map[string]string{ SecretLifetimeAnnotationKey: s.clock().Add(lifetime).UTC().Format(SecretLifetimeAnnotationDateFormat), } diff --git a/internal/crud/crud_test.go b/internal/crud/crud_test.go index 44fa81e4c..ea9d8c369 100644 --- a/internal/crud/crud_test.go +++ b/internal/crud/crud_test.go @@ -1235,7 +1235,6 @@ func TestStorage(t *testing.T) { require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is data := &testJSON{Data: "create-and-get"} - // TODO: Note that this test will pass with just about any value for lifetime rv1, err := storage.Create(ctx, signature, data, nil, nil, 0) // 0 == infinity require.Empty(t, rv1) // fake client does not set this require.NoError(t, err) diff --git a/internal/federationdomain/clientregistry/clientregistry_test.go b/internal/federationdomain/clientregistry/clientregistry_test.go index b3b798137..5f800ff9f 100644 --- a/internal/federationdomain/clientregistry/clientregistry_test.go +++ b/internal/federationdomain/clientregistry/clientregistry_test.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" + "k8s.io/utils/ptr" configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" @@ -180,7 +181,7 @@ func TestClientManager(t *testing.T) { }, }, { - name: "find a valid dynamic client", + name: "find a valid dynamic client without an ID token lifetime configuration", oidcClients: []*configv1alpha1.OIDCClient{ { ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, @@ -188,6 +189,7 @@ func TestClientManager(t *testing.T) { AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"}, AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"}, AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:80", "https://foobar.com/callback"}, + TokenLifetimes: configv1alpha1.OIDCClientTokenLifetimes{IDTokenSeconds: nil}, }, }, { @@ -203,24 +205,49 @@ func TestClientManager(t *testing.T) { require.IsType(t, &Client{}, got) c := got.(*Client) - require.Equal(t, testName, c.GetID()) - require.Nil(t, c.GetHashedSecret()) - require.Len(t, c.GetRotatedHashes(), 2) - require.Equal(t, testutil.HashedPassword1AtSupervisorMinCost, string(c.GetRotatedHashes()[0])) - require.Equal(t, testutil.HashedPassword2AtSupervisorMinCost, string(c.GetRotatedHashes()[1])) - require.Equal(t, []string{"http://localhost:80", "https://foobar.com/callback"}, c.GetRedirectURIs()) - require.Equal(t, fosite.Arguments{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"}, c.GetGrantTypes()) - require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes()) - require.Equal(t, fosite.Arguments{"openid", "offline_access", "pinniped:request-audience", "username", "groups"}, c.GetScopes()) - require.False(t, c.IsPublic()) - require.Nil(t, c.GetAudience()) - require.Nil(t, c.GetRequestURIs()) - require.Nil(t, c.GetJSONWebKeys()) - require.Equal(t, "", c.GetJSONWebKeysURI()) - require.Equal(t, "", c.GetRequestObjectSigningAlgorithm()) - require.Equal(t, "client_secret_basic", c.GetTokenEndpointAuthMethod()) - require.Equal(t, "RS256", c.GetTokenEndpointAuthSigningAlgorithm()) - require.Equal(t, []fosite.ResponseModeType{"", "query"}, c.GetResponseModes()) + requireDynamicOIDCClient(t, c, + testName, + []string{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword2AtSupervisorMinCost}, + fosite.Arguments{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"}, + fosite.Arguments{"openid", "offline_access", "pinniped:request-audience", "username", "groups"}, + []string{"http://localhost:80", "https://foobar.com/callback"}, + 0*time.Second, + ) + }, + }, + { + name: "find a valid dynamic client with an ID token lifetime configuration", + oidcClients: []*configv1alpha1.OIDCClient{ + { + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, + Spec: configv1alpha1.OIDCClientSpec{ + AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"}, + AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "username", "groups"}, + AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:8080"}, + TokenLifetimes: configv1alpha1.OIDCClientTokenLifetimes{IDTokenSeconds: ptr.To[int32]((4242))}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-client", Generation: 1234, UID: testUID}, + }, + }, + secrets: []*corev1.Secret{ + testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword2AtSupervisorMinCost, testutil.HashedPassword1AtSupervisorMinCost}), + }, + run: func(t *testing.T, subject *ClientManager) { + got, err := subject.GetClient(ctx, testName) + require.NoError(t, err) + require.IsType(t, &Client{}, got) + c := got.(*Client) + + requireDynamicOIDCClient(t, c, + testName, + []string{testutil.HashedPassword2AtSupervisorMinCost, testutil.HashedPassword1AtSupervisorMinCost}, + fosite.Arguments{"authorization_code", "refresh_token"}, + fosite.Arguments{"openid", "offline_access", "username", "groups"}, + []string{"http://localhost:8080"}, + 4242*time.Second, + ) }, }, } @@ -279,6 +306,7 @@ func requireEqualsPinnipedCLI(t *testing.T, c *Client) { require.Equal(t, "none", c.GetTokenEndpointAuthMethod()) require.Equal(t, "RS256", c.GetTokenEndpointAuthSigningAlgorithm()) require.Equal(t, []fosite.ResponseModeType{"", "query", "form_post"}, c.GetResponseModes()) + require.Equal(t, 0*time.Second, c.IDTokenLifetimeConfiguration) marshaled, err := json.Marshal(c) require.NoError(t, err) @@ -316,3 +344,39 @@ func requireEqualsPinnipedCLI(t *testing.T, c *Client) { "IDTokenLifetimeConfiguration": 0 }`, string(marshaled)) } + +func requireDynamicOIDCClient( + t *testing.T, + c *Client, + wantClientID string, + wantHashes []string, + wantGrantTypes fosite.Arguments, + wantScopes fosite.Arguments, + wantRedirectURIs []string, + wantIDTokenLifetimeConfiguration time.Duration, +) { + require.Equal(t, wantClientID, c.GetID()) + + require.Len(t, c.GetRotatedHashes(), len(wantHashes)) + for i := range wantHashes { + require.Equal(t, wantHashes[i], string(c.GetRotatedHashes()[i])) + } + + require.Equal(t, wantRedirectURIs, c.GetRedirectURIs()) + require.Equal(t, wantGrantTypes, c.GetGrantTypes()) + require.Equal(t, wantScopes, c.GetScopes()) + require.Equal(t, wantIDTokenLifetimeConfiguration, c.IDTokenLifetimeConfiguration) + + // The following are always the same for all OIDCClients. + require.Nil(t, c.GetHashedSecret()) + require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes()) + require.False(t, c.IsPublic()) + require.Nil(t, c.GetAudience()) + require.Nil(t, c.GetRequestURIs()) + require.Nil(t, c.GetJSONWebKeys()) + require.Equal(t, "", c.GetJSONWebKeysURI()) + require.Equal(t, "", c.GetRequestObjectSigningAlgorithm()) + require.Equal(t, "client_secret_basic", c.GetTokenEndpointAuthMethod()) + require.Equal(t, "RS256", c.GetTokenEndpointAuthSigningAlgorithm()) + require.Equal(t, []fosite.ResponseModeType{"", "query"}, c.GetResponseModes()) +} diff --git a/internal/federationdomain/idtokenlifespan/idtoken_lifespan_test.go b/internal/federationdomain/idtokenlifespan/idtoken_lifespan_test.go new file mode 100644 index 000000000..0501525a0 --- /dev/null +++ b/internal/federationdomain/idtokenlifespan/idtoken_lifespan_test.go @@ -0,0 +1,77 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package idtokenlifespan + +import ( + "context" + "testing" + "time" + + "github.com/ory/fosite" + "github.com/stretchr/testify/require" +) + +func TestOverrideIDTokenLifespanInContext(t *testing.T) { + tests := []struct { + name string + defaultLifespan time.Duration + overrideLifespan func(ctx context.Context) context.Context + wantLifespan time.Duration + }{ + { + name: "does not override the context's default timeout", + defaultLifespan: 10 * time.Second, + overrideLifespan: func(baseCtx context.Context) context.Context { + return baseCtx // no-op on the context + }, + wantLifespan: 10 * time.Second, + }, + { + name: "overrides the context's default to be 42 seconds", + defaultLifespan: 10 * time.Second, + overrideLifespan: func(baseCtx context.Context) context.Context { + return OverrideIDTokenLifespanInContext(baseCtx, 42*time.Second) + }, + wantLifespan: 42 * time.Second, + }, + { + name: "overrides the context's default to be 42 minutes", + defaultLifespan: 10 * time.Second, + overrideLifespan: func(baseCtx context.Context) context.Context { + return OverrideIDTokenLifespanInContext(baseCtx, 42*time.Minute) + }, + wantLifespan: 42 * time.Minute, + }, + { + name: "somehow accidentally overrides the context's default timeout to be the wrong type", + defaultLifespan: 10 * time.Second, + overrideLifespan: func(baseCtx context.Context) context.Context { + return context.WithValue(baseCtx, idTokenLifetimeOverrideKey, "this should be a duration but is a string") + }, + wantLifespan: 10 * time.Second, // should ignore the illegal value and just return the default + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + baseConfig := fosite.Config{ + IDTokenLifespan: tt.defaultLifespan, + } + + contextAwareProvider := &contextAwareIDTokenLifespanProvider{ + DelegateConfig: &baseConfig, + } + + // Possibly override the default lifespan on the context. + updatedCtx := tt.overrideLifespan(context.Background()) + + // Read the lifespan from the context. + gotLifespan := contextAwareProvider.GetIDTokenLifespan(updatedCtx) + require.Equal(t, tt.wantLifespan, gotLifespan) + }) + } +} diff --git a/internal/federationdomain/oidc/oidc_test.go b/internal/federationdomain/oidc/oidc_test.go new file mode 100644 index 000000000..bf9c30890 --- /dev/null +++ b/internal/federationdomain/oidc/oidc_test.go @@ -0,0 +1,117 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidc + +import ( + "testing" + "time" + + "github.com/ory/fosite" + "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/federationdomain/clientregistry" +) + +func TestDefaultLifespans(t *testing.T) { + c := DefaultOIDCTimeoutsConfiguration() + + require.Equal(t, 90*time.Minute, c.UpstreamStateParamLifespan) + require.Equal(t, 10*time.Minute, c.AuthorizeCodeLifespan) + require.Equal(t, 2*time.Minute, c.AccessTokenLifespan) + require.Equal(t, 2*time.Minute, c.IDTokenLifespan) + require.Equal(t, 9*time.Hour, c.RefreshTokenLifespan) +} + +func TestStorageLifetimes(t *testing.T) { + c := DefaultOIDCTimeoutsConfiguration() + + // These are currently hard-coded. + require.Equal(t, 9*time.Hour+10*time.Minute, c.AuthorizationCodeSessionStorageLifetime(nil)) + require.Equal(t, 11*time.Minute, c.PKCESessionStorageLifetime(nil)) + require.Equal(t, 11*time.Minute, c.OIDCSessionStorageLifetime(nil)) + require.Equal(t, 9*time.Hour+2*time.Minute, c.AccessTokenSessionStorageLifetime(nil)) + require.Equal(t, 9*time.Hour+2*time.Minute, c.RefreshTokenSessionStorageLifetime(nil)) +} + +func TestOverrideDefaultAccessTokenLifespan(t *testing.T) { + c := DefaultOIDCTimeoutsConfiguration() + + // We are not yet overriding access token lifetimes. + doOverride, newLifespan := c.OverrideDefaultAccessTokenLifespan(nil) + require.Equal(t, false, doOverride) + require.Equal(t, time.Duration(0), newLifespan) +} + +func TestOverrideIDTokenLifespan(t *testing.T) { + tests := []struct { + name string + accessRequest fosite.AccessRequester + wantOverride bool + wantLifespan time.Duration + }{ + { + name: "the client does not override the default ID token lifespan", + accessRequest: &fosite.AccessRequest{ + GrantTypes: fosite.Arguments{"foo"}, + Request: fosite.Request{ + Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 0, // 0 means use the default, so this is not an override + }, + }, + }, + wantOverride: false, + wantLifespan: 0, + }, + { + name: "the client overrides the default ID token lifespan", + accessRequest: &fosite.AccessRequest{ + GrantTypes: fosite.Arguments{"foo"}, + Request: fosite.Request{ + Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, + }, + }, + }, + wantOverride: true, + wantLifespan: 42 * time.Second, + }, + { + name: "the client overrides the default ID token lifespan, but the request is for the token exchange, so the override is ignored", + accessRequest: &fosite.AccessRequest{ + GrantTypes: fosite.Arguments{"urn:ietf:params:oauth:grant-type:token-exchange"}, + Request: fosite.Request{ + Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, + }, + }, + }, + wantOverride: false, + wantLifespan: 0, + }, + { + name: "the client is not the expected data type (which shouldn't really happen), so it is assumed to not override the ID token lifespan", + accessRequest: &fosite.AccessRequest{ + GrantTypes: fosite.Arguments{"foo"}, + Request: fosite.Request{ + Client: &fosite.DefaultClient{}, + }, + }, + wantOverride: false, + wantLifespan: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := DefaultOIDCTimeoutsConfiguration() + + doOverride, newLifespan := c.OverrideDefaultIDTokenLifespan(tt.accessRequest) + require.Equal(t, tt.wantOverride, doOverride) + require.Equal(t, tt.wantLifespan, newLifespan) + }) + } +} diff --git a/internal/fositestorage/accesstoken/accesstoken_test.go b/internal/fositestorage/accesstoken/accesstoken_test.go index adfb67466..a36e09f80 100644 --- a/internal/fositestorage/accesstoken/accesstoken_test.go +++ b/internal/fositestorage/accesstoken/accesstoken_test.go @@ -23,6 +23,7 @@ import ( clocktesting "k8s.io/utils/clock/testing" "go.pinniped.dev/internal/federationdomain/clientregistry" + "go.pinniped.dev/internal/federationdomain/timeouts" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" ) @@ -32,6 +33,7 @@ const namespace = "test-ns" var fakeNow = time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) var lifetime = time.Minute * 10 var fakeNowPlusLifetimeAsString = metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339) +var lifetimeFunc = func(requester fosite.Requester) time.Duration { return lifetime } var secretsGVR = schema.GroupVersionResource{ Group: "", @@ -54,7 +56,7 @@ func TestAccessTokenStorage(t *testing.T) { }, }, 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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/access-token", @@ -63,12 +65,19 @@ func TestAccessTokenStorage(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-access-token-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + storageLifetimeFuncCallCount := 0 + var storageLifetimeFuncCallRequesterArg fosite.Requester + ctx, client, _, storage := makeTestSubject(func(requester fosite.Requester) time.Duration { + storageLifetimeFuncCallCount++ + storageLifetimeFuncCallRequesterArg = requester + return lifetime + }) request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ DefaultClient: &fosite.DefaultClient{ ID: "pinny", @@ -96,6 +105,8 @@ func TestAccessTokenStorage(t *testing.T) { } err := storage.CreateAccessTokenSession(ctx, "fancy-signature", request) require.NoError(t, err) + require.Equal(t, 1, storageLifetimeFuncCallCount) + require.Equal(t, request, storageLifetimeFuncCallRequesterArg) newRequest, err := storage.GetAccessTokenSession(ctx, "fancy-signature", nil) require.NoError(t, err) @@ -106,6 +117,9 @@ func TestAccessTokenStorage(t *testing.T) { testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed require.Equal(t, wantActions, client.Actions()) + + // Check that there were no more calls to the lifetime func since the original create. + require.Equal(t, 1, storageLifetimeFuncCallCount) } func TestAccessTokenStorageRevocation(t *testing.T) { @@ -134,7 +148,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-access-token-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ ID: "abcd-1", @@ -164,7 +178,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) { } func TestGetNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) _, notFoundErr := storage.GetAccessTokenSession(ctx, "non-existent-signature", nil) require.EqualError(t, notFoundErr, "not_found") @@ -172,7 +186,7 @@ func TestGetNotFound(t *testing.T) { } func TestWrongVersion(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -200,7 +214,7 @@ func TestWrongVersion(t *testing.T) { } func TestNilSessionRequest(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -228,14 +242,14 @@ func TestNilSessionRequest(t *testing.T) { } func TestCreateWithNilRequester(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreateAccessTokenSession(ctx, "signature-doesnt-matter", nil) require.EqualError(t, err, "requester must be of type fosite.Request") } func TestCreateWithWrongRequesterDataTypes(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ Session: nil, @@ -253,7 +267,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { } func TestCreateWithoutRequesterID(t *testing.T) { - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ ID: "", // empty ID @@ -274,10 +288,13 @@ func TestCreateWithoutRequesterID(t *testing.T) { require.Equal(t, request.ID, actualSecret.Labels["storage.pinniped.dev/request-id"]) } -func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) { +func makeTestSubject(lifetimeFunc timeouts.StorageLifetime) (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) { client := fake.NewSimpleClientset() secrets := client.CoreV1().Secrets(namespace) - return context.Background(), client, secrets, New(secrets, clocktesting.NewFakeClock(fakeNow).Now, func(requester fosite.Requester) time.Duration { return lifetime }) + return context.Background(), + client, + secrets, + New(secrets, clocktesting.NewFakeClock(fakeNow).Now, lifetimeFunc) } func TestReadFromSecret(t *testing.T) { diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index c43f32bfc..a057062f2 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -35,6 +35,7 @@ import ( clocktesting "k8s.io/utils/clock/testing" "go.pinniped.dev/internal/federationdomain/clientregistry" + "go.pinniped.dev/internal/federationdomain/timeouts" "go.pinniped.dev/internal/fositestorage" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" @@ -45,6 +46,7 @@ const namespace = "test-ns" var fakeNow = time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) var lifetime = time.Minute * 10 var fakeNowPlusLifetimeAsString = metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339) +var lifetimeFunc = func(requester fosite.Requester) time.Duration { return lifetime } func TestAuthorizationCodeStorage(t *testing.T) { secretsGVR := schema.GroupVersionResource{ @@ -66,7 +68,7 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":true,"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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "pinniped-storage-data": []byte(`{"active":true,"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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", @@ -86,19 +88,26 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":false,"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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "pinniped-storage-data": []byte(`{"active":false,"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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", }), } - ctx, client, _, storage := makeTestSubject() + storageLifetimeFuncCallCount := 0 + var storageLifetimeFuncCallRequesterArg fosite.Requester + ctx, client, _, storage := makeTestSubject(func(requester fosite.Requester) time.Duration { + storageLifetimeFuncCallCount++ + storageLifetimeFuncCallRequesterArg = requester + return lifetime + }) request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ DefaultClient: &fosite.DefaultClient{ ID: "pinny", @@ -127,6 +136,8 @@ func TestAuthorizationCodeStorage(t *testing.T) { } err := storage.CreateAuthorizeCodeSession(ctx, "fancy-signature", request) require.NoError(t, err) + require.Equal(t, 1, storageLifetimeFuncCallCount) + require.Equal(t, request, storageLifetimeFuncCallRequesterArg) newRequest, err := storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) require.NoError(t, err) @@ -143,10 +154,13 @@ func TestAuthorizationCodeStorage(t *testing.T) { invalidatedRequest, err := storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) require.EqualError(t, err, "authorization code session for fancy-signature has already been used: Authorization code has ben invalidated") require.Equal(t, "abcd-1", invalidatedRequest.GetID()) + + // Check that there were no more calls to the lifetime func since the original create. + require.Equal(t, 1, storageLifetimeFuncCallCount) } func TestGetNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) _, notFoundErr := storage.GetAuthorizeCodeSession(ctx, "non-existent-signature", nil) require.EqualError(t, notFoundErr, "not_found") @@ -154,7 +168,7 @@ func TestGetNotFound(t *testing.T) { } func TestInvalidateWhenNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) notFoundErr := storage.InvalidateAuthorizeCodeSession(ctx, "non-existent-signature") require.EqualError(t, notFoundErr, "not_found") @@ -162,7 +176,7 @@ func TestInvalidateWhenNotFound(t *testing.T) { } func TestInvalidateWhenConflictOnUpdateHappens(t *testing.T) { - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) client.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, apierrors.NewConflict(schema.GroupResource{ @@ -183,7 +197,7 @@ func TestInvalidateWhenConflictOnUpdateHappens(t *testing.T) { } func TestWrongVersion(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -208,7 +222,7 @@ func TestWrongVersion(t *testing.T) { } func TestNilSessionRequest(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -233,14 +247,14 @@ func TestNilSessionRequest(t *testing.T) { } func TestCreateWithNilRequester(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", nil) require.EqualError(t, err, "requester must be of type fosite.Request") } func TestCreateWithWrongRequesterDataTypes(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ Session: nil, @@ -257,10 +271,13 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } -func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, oauth2.AuthorizeCodeStorage) { +func makeTestSubject(lifetimeFunc timeouts.StorageLifetime) (context.Context, *fake.Clientset, corev1client.SecretInterface, oauth2.AuthorizeCodeStorage) { client := fake.NewSimpleClientset() secrets := client.CoreV1().Secrets(namespace) - return context.Background(), client, secrets, New(secrets, clocktesting.NewFakeClock(fakeNow).Now, func(requester fosite.Requester) time.Duration { return lifetime }) + return context.Background(), + client, + secrets, + New(secrets, clocktesting.NewFakeClock(fakeNow).Now, lifetimeFunc) } // TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession asserts that we can correctly round trip our authorize code session. diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go index 882eff9ba..781973d36 100644 --- a/internal/fositestorage/openidconnect/openidconnect_test.go +++ b/internal/fositestorage/openidconnect/openidconnect_test.go @@ -22,6 +22,7 @@ import ( clocktesting "k8s.io/utils/clock/testing" "go.pinniped.dev/internal/federationdomain/clientregistry" + "go.pinniped.dev/internal/federationdomain/timeouts" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" ) @@ -31,6 +32,7 @@ const namespace = "test-ns" var fakeNow = time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) var lifetime = time.Minute * 10 var fakeNowPlusLifetimeAsString = metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339) +var lifetimeFunc = func(requester fosite.Requester) time.Duration { return lifetime } func TestOpenIdConnectStorage(t *testing.T) { secretsGVR := schema.GroupVersionResource{ @@ -52,7 +54,7 @@ func TestOpenIdConnectStorage(t *testing.T) { }, }, 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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/oidc", @@ -61,12 +63,19 @@ func TestOpenIdConnectStorage(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-oidc-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + storageLifetimeFuncCallCount := 0 + var storageLifetimeFuncCallRequesterArg fosite.Requester + ctx, client, _, storage := makeTestSubject(func(requester fosite.Requester) time.Duration { + storageLifetimeFuncCallCount++ + storageLifetimeFuncCallRequesterArg = requester + return lifetime + }) request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ DefaultClient: &fosite.DefaultClient{ ID: "pinny", @@ -95,6 +104,8 @@ func TestOpenIdConnectStorage(t *testing.T) { } err := storage.CreateOpenIDConnectSession(ctx, "fancy-code.fancy-signature", request) require.NoError(t, err) + require.Equal(t, 1, storageLifetimeFuncCallCount) + require.Equal(t, request, storageLifetimeFuncCallRequesterArg) newRequest, err := storage.GetOpenIDConnectSession(ctx, "fancy-code.fancy-signature", nil) require.NoError(t, err) @@ -105,10 +116,13 @@ func TestOpenIdConnectStorage(t *testing.T) { testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed require.Equal(t, wantActions, client.Actions()) + + // Check that there were no more calls to the lifetime func since the original create. + require.Equal(t, 1, storageLifetimeFuncCallCount) } func TestGetNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) _, notFoundErr := storage.GetOpenIDConnectSession(ctx, "authcode.non-existent-signature", nil) require.EqualError(t, notFoundErr, "not_found") @@ -116,7 +130,7 @@ func TestGetNotFound(t *testing.T) { } func TestWrongVersion(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -141,7 +155,7 @@ func TestWrongVersion(t *testing.T) { } func TestNilSessionRequest(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -166,14 +180,14 @@ func TestNilSessionRequest(t *testing.T) { } func TestCreateWithNilRequester(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", nil) require.EqualError(t, err, "requester must be of type fosite.Request") } func TestCreateWithWrongRequesterDataTypes(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ Session: nil, @@ -191,14 +205,17 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { } func TestAuthcodeHasNoDot(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreateOpenIDConnectSession(ctx, "all-one-part", nil) require.EqualError(t, err, "malformed authorization code") } -func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, openid.OpenIDConnectRequestStorage) { +func makeTestSubject(lifetimeFunc timeouts.StorageLifetime) (context.Context, *fake.Clientset, corev1client.SecretInterface, openid.OpenIDConnectRequestStorage) { client := fake.NewSimpleClientset() secrets := client.CoreV1().Secrets(namespace) - return context.Background(), client, secrets, New(secrets, clocktesting.NewFakeClock(fakeNow).Now, func(requester fosite.Requester) time.Duration { return lifetime }) + return context.Background(), + client, + secrets, + New(secrets, clocktesting.NewFakeClock(fakeNow).Now, lifetimeFunc) } diff --git a/internal/fositestorage/pkce/pkce_test.go b/internal/fositestorage/pkce/pkce_test.go index 2f1003951..100cbd211 100644 --- a/internal/fositestorage/pkce/pkce_test.go +++ b/internal/fositestorage/pkce/pkce_test.go @@ -22,6 +22,7 @@ import ( clocktesting "k8s.io/utils/clock/testing" "go.pinniped.dev/internal/federationdomain/clientregistry" + "go.pinniped.dev/internal/federationdomain/timeouts" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" ) @@ -31,6 +32,7 @@ const namespace = "test-ns" var fakeNow = time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) var lifetime = time.Minute * 10 var fakeNowPlusLifetimeAsString = metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339) +var lifetimeFunc = func(requester fosite.Requester) time.Duration { return lifetime } func TestPKCEStorage(t *testing.T) { secretsGVR := schema.GroupVersionResource{ @@ -52,7 +54,7 @@ func TestPKCEStorage(t *testing.T) { }, }, 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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/pkce", @@ -61,12 +63,19 @@ func TestPKCEStorage(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-pkce-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + storageLifetimeFuncCallCount := 0 + var storageLifetimeFuncCallRequesterArg fosite.Requester + ctx, client, _, storage := makeTestSubject(func(requester fosite.Requester) time.Duration { + storageLifetimeFuncCallCount++ + storageLifetimeFuncCallRequesterArg = requester + return lifetime + }) request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ DefaultClient: &fosite.DefaultClient{ ID: "pinny", @@ -95,6 +104,8 @@ func TestPKCEStorage(t *testing.T) { } err := storage.CreatePKCERequestSession(ctx, "fancy-signature", request) require.NoError(t, err) + require.Equal(t, 1, storageLifetimeFuncCallCount) + require.Equal(t, request, storageLifetimeFuncCallRequesterArg) newRequest, err := storage.GetPKCERequestSession(ctx, "fancy-signature", nil) require.NoError(t, err) @@ -105,10 +116,12 @@ func TestPKCEStorage(t *testing.T) { testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed require.Equal(t, wantActions, client.Actions()) + // Check that there were no more calls to the lifetime func since the original create. + require.Equal(t, 1, storageLifetimeFuncCallCount) } func TestGetNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) _, notFoundErr := storage.GetPKCERequestSession(ctx, "non-existent-signature", nil) require.EqualError(t, notFoundErr, "not_found") @@ -116,7 +129,7 @@ func TestGetNotFound(t *testing.T) { } func TestWrongVersion(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -144,7 +157,7 @@ func TestWrongVersion(t *testing.T) { } func TestNilSessionRequest(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -172,14 +185,14 @@ func TestNilSessionRequest(t *testing.T) { } func TestCreateWithNilRequester(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreatePKCERequestSession(ctx, "signature-doesnt-matter", nil) require.EqualError(t, err, "requester must be of type fosite.Request") } func TestCreateWithWrongRequesterDataTypes(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ Session: nil, @@ -196,8 +209,11 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } -func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, pkce.PKCERequestStorage) { +func makeTestSubject(lifetimeFunc timeouts.StorageLifetime) (context.Context, *fake.Clientset, corev1client.SecretInterface, pkce.PKCERequestStorage) { client := fake.NewSimpleClientset() secrets := client.CoreV1().Secrets(namespace) - return context.Background(), client, secrets, New(secrets, clocktesting.NewFakeClock(fakeNow).Now, func(requester fosite.Requester) time.Duration { return lifetime }) + return context.Background(), + client, + secrets, + New(secrets, clocktesting.NewFakeClock(fakeNow).Now, lifetimeFunc) } diff --git a/internal/fositestorage/refreshtoken/refreshtoken_test.go b/internal/fositestorage/refreshtoken/refreshtoken_test.go index f2e3ac0e9..d6d987c38 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken_test.go +++ b/internal/fositestorage/refreshtoken/refreshtoken_test.go @@ -23,6 +23,7 @@ import ( clocktesting "k8s.io/utils/clock/testing" "go.pinniped.dev/internal/federationdomain/clientregistry" + "go.pinniped.dev/internal/federationdomain/timeouts" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" ) @@ -37,6 +38,7 @@ var secretsGVR = schema.GroupVersionResource{ var fakeNow = time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) var lifetime = time.Minute * 10 var fakeNowPlusLifetimeAsString = metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339) +var lifetimeFunc = func(requester fosite.Requester) time.Duration { return lifetime } func TestRefreshTokenStorage(t *testing.T) { wantActions := []coretesting.Action{ @@ -53,7 +55,7 @@ func TestRefreshTokenStorage(t *testing.T) { }, }, 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":"","IDTokenLifetimeConfiguration":0},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), + "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":"","IDTokenLifetimeConfiguration":42000000000},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/refresh-token", @@ -62,12 +64,19 @@ func TestRefreshTokenStorage(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + storageLifetimeFuncCallCount := 0 + var storageLifetimeFuncCallRequesterArg fosite.Requester + ctx, client, _, storage := makeTestSubject(func(requester fosite.Requester) time.Duration { + storageLifetimeFuncCallCount++ + storageLifetimeFuncCallRequesterArg = requester + return lifetime + }) request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, Client: &clientregistry.Client{ + IDTokenLifetimeConfiguration: 42 * time.Second, DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ DefaultClient: &fosite.DefaultClient{ ID: "pinny", @@ -96,6 +105,8 @@ func TestRefreshTokenStorage(t *testing.T) { } err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", request) require.NoError(t, err) + require.Equal(t, 1, storageLifetimeFuncCallCount) + require.Equal(t, request, storageLifetimeFuncCallRequesterArg) newRequest, err := storage.GetRefreshTokenSession(ctx, "fancy-signature", nil) require.NoError(t, err) @@ -106,6 +117,9 @@ func TestRefreshTokenStorage(t *testing.T) { testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed require.Equal(t, wantActions, client.Actions()) + + // Check that there were no more calls to the lifetime func since the original create. + require.Equal(t, 1, storageLifetimeFuncCallCount) } func TestRefreshTokenStorageRevocation(t *testing.T) { @@ -134,7 +148,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ ID: "abcd-1", @@ -189,7 +203,7 @@ func TestRefreshTokenStorageRevokeRefreshTokenMaybeGracePeriod(t *testing.T) { coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"), } - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ ID: "abcd-1", @@ -220,7 +234,7 @@ func TestRefreshTokenStorageRevokeRefreshTokenMaybeGracePeriod(t *testing.T) { } func TestGetNotFound(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) _, notFoundErr := storage.GetRefreshTokenSession(ctx, "non-existent-signature", nil) require.EqualError(t, notFoundErr, "not_found") @@ -228,7 +242,7 @@ func TestGetNotFound(t *testing.T) { } func TestWrongVersion(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -256,7 +270,7 @@ func TestWrongVersion(t *testing.T) { } func TestNilSessionRequest(t *testing.T) { - ctx, _, secrets, storage := makeTestSubject() + ctx, _, secrets, storage := makeTestSubject(lifetimeFunc) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -284,14 +298,14 @@ func TestNilSessionRequest(t *testing.T) { } func TestCreateWithNilRequester(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", nil) require.EqualError(t, err, "requester must be of type fosite.Request") } func TestCreateWithWrongRequesterDataTypes(t *testing.T) { - ctx, _, _, storage := makeTestSubject() + ctx, _, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ Session: nil, @@ -309,7 +323,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { } func TestCreateWithoutRequesterID(t *testing.T) { - ctx, client, _, storage := makeTestSubject() + ctx, client, _, storage := makeTestSubject(lifetimeFunc) request := &fosite.Request{ ID: "", // empty ID @@ -330,10 +344,13 @@ func TestCreateWithoutRequesterID(t *testing.T) { require.Equal(t, request.ID, actualSecret.Labels["storage.pinniped.dev/request-id"]) } -func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) { +func makeTestSubject(lifetimeFunc timeouts.StorageLifetime) (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) { client := fake.NewSimpleClientset() secrets := client.CoreV1().Secrets(namespace) - return context.Background(), client, secrets, New(secrets, clocktesting.NewFakeClock(fakeNow).Now, func(requester fosite.Requester) time.Duration { return lifetime }) + return context.Background(), + client, + secrets, + New(secrets, clocktesting.NewFakeClock(fakeNow).Now, lifetimeFunc) } func TestReadFromSecret(t *testing.T) {