Merge pull request #278 from vmware-tanzu/fosite-storage-gc

Garbage collect the fosite secrets to limit amount of storage used

cmd/pinniped-supervisor/main.go

@@ -29,6 +29,7 @@ import (
 	"go.pinniped.dev/internal/config/supervisor"
 	"go.pinniped.dev/internal/controller/supervisorconfig"
 	"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatcher"
+	"go.pinniped.dev/internal/controller/supervisorstorage"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/downward"
 	"go.pinniped.dev/internal/oidc/jwks"
@@ -84,6 +85,15 @@ func startControllers(
 	// Create controller manager.
 	controllerManager := controllerlib.
 		NewManager().
+		WithController(
+			supervisorstorage.GarbageCollectorController(
+				clock.RealClock{},
+				kubeClient,
+				kubeInformers.Core().V1().Secrets(),
+				controllerlib.WithInformer,
+			),
+			singletonWorker,
+		).
 		WithController(
 			supervisorconfig.NewOIDCProviderWatcherController(
 				issuerManager,

internal/controller/supervisorstorage/garbage_collector.go

@@ -0,0 +1,105 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package supervisorstorage
+
+import (
+	"time"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/clock"
+	corev1informers "k8s.io/client-go/informers/core/v1"
+	"k8s.io/client-go/kubernetes"
+
+	pinnipedcontroller "go.pinniped.dev/internal/controller"
+	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/crud"
+	"go.pinniped.dev/internal/plog"
+)
+
+const minimumRepeatInterval = 30 * time.Second
+
+type garbageCollectorController struct {
+	secretInformer        corev1informers.SecretInformer
+	kubeClient            kubernetes.Interface
+	clock                 clock.Clock
+	timeOfMostRecentSweep time.Time
+}
+
+func GarbageCollectorController(
+	clock clock.Clock,
+	kubeClient kubernetes.Interface,
+	secretInformer corev1informers.SecretInformer,
+	withInformer pinnipedcontroller.WithInformerOptionFunc,
+) controllerlib.Controller {
+	return controllerlib.New(
+		controllerlib.Config{
+			Name: "garbage-collector-controller",
+			Syncer: &garbageCollectorController{
+				secretInformer: secretInformer,
+				kubeClient:     kubeClient,
+				clock:          clock,
+			},
+		},
+		withInformer(
+			secretInformer,
+			pinnipedcontroller.MatchAnythingFilter(nil),
+			controllerlib.InformerOption{},
+		),
+	)
+}
+
+func (c *garbageCollectorController) Sync(ctx controllerlib.Context) error {
+	// The Sync method is triggered upon any change to any Secret, which would make this
+	// controller too chatty, so it rate limits itself to a more reasonable interval.
+	// Note that even during a period when no secrets are changing, it will still run
+	// at the informer's full-resync interval (as long as there are some secrets).
+	if c.clock.Now().Sub(c.timeOfMostRecentSweep) < minimumRepeatInterval {
+		return nil
+	}
+
+	plog.Info("starting storage garbage collection sweep")
+	c.timeOfMostRecentSweep = c.clock.Now()
+
+	listOfSecrets, err := c.secretInformer.Lister().List(labels.Everything())
+	if err != nil {
+		return err
+	}
+
+	for i := range listOfSecrets {
+		secret := listOfSecrets[i]
+
+		timeString, ok := secret.Annotations[crud.SecretLifetimeAnnotationKey]
+		if !ok {
+			continue
+		}
+
+		garbageCollectAfterTime, err := time.Parse(crud.SecretLifetimeAnnotationDateFormat, timeString)
+		if err != nil {
+			plog.WarningErr("could not parse resource timestamp for garbage collection", err, logKV(secret))
+			continue
+		}
+
+		if garbageCollectAfterTime.Before(c.clock.Now()) {
+			err = c.kubeClient.CoreV1().Secrets(secret.Namespace).Delete(ctx.Context, secret.Name, metav1.DeleteOptions{})
+			if err != nil {
+				plog.WarningErr("failed to garbage collect resource", err, logKV(secret))
+				continue
+			}
+			plog.Info("storage garbage collector deleted resource", logKV(secret))
+		}
+	}
+
+	return nil
+}
+
+func logKV(secret *v1.Secret) []interface{} {
+	return []interface{}{
+		"secretName", secret.Name,
+		"secretNamespace", secret.Namespace,
+		"secretType", string(secret.Type),
+		"garbageCollectAfter", secret.Annotations[crud.SecretLifetimeAnnotationKey],
+	}
+}

internal/controller/supervisorstorage/garbage_collector_test.go

@@ -0,0 +1,358 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package supervisorstorage
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/sclevine/spec"
+	"github.com/sclevine/spec/report"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
+	kubeinformers "k8s.io/client-go/informers"
+	kubernetesfake "k8s.io/client-go/kubernetes/fake"
+	kubetesting "k8s.io/client-go/testing"
+
+	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/testutil"
+)
+
+func TestGarbageCollectorControllerInformerFilters(t *testing.T) {
+	spec.Run(t, "informer filters", func(t *testing.T, when spec.G, it spec.S) {
+		var (
+			r                            *require.Assertions
+			observableWithInformerOption *testutil.ObservableWithInformerOption
+			secretsInformerFilter        controllerlib.Filter
+		)
+
+		it.Before(func() {
+			r = require.New(t)
+			observableWithInformerOption = testutil.NewObservableWithInformerOption()
+			secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()
+			_ = GarbageCollectorController(
+				clock.RealClock{},
+				nil,
+				secretsInformer,
+				observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters
+			)
+			secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer)
+		})
+
+		when("watching Secret objects", func() {
+			var (
+				subject             controllerlib.Filter
+				secret, otherSecret *corev1.Secret
+			)
+
+			it.Before(func() {
+				subject = secretsInformerFilter
+				secret = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "any-name", Namespace: "any-namespace"}}
+				otherSecret = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "any-other-name", Namespace: "any-other-namespace"}}
+			})
+
+			when("any Secret changes", func() {
+				it("returns true to trigger the sync function for all secrets", func() {
+					r.True(subject.Add(secret))
+					r.True(subject.Update(secret, otherSecret))
+					r.True(subject.Update(otherSecret, secret))
+					r.True(subject.Delete(secret))
+				})
+			})
+		})
+	}, spec.Parallel(), spec.Report(report.Terminal{}))
+}
+
+func TestGarbageCollectorControllerSync(t *testing.T) {
+	secretsGVR := schema.GroupVersionResource{
+		Group:    "",
+		Version:  "v1",
+		Resource: "secrets",
+	}
+
+	spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) {
+		const (
+			installedInNamespace = "some-namespace"
+		)
+
+		var (
+			r                    *require.Assertions
+			subject              controllerlib.Controller
+			kubeInformerClient   *kubernetesfake.Clientset
+			kubeClient           *kubernetesfake.Clientset
+			kubeInformers        kubeinformers.SharedInformerFactory
+			timeoutContext       context.Context
+			timeoutContextCancel context.CancelFunc
+			syncContext          *controllerlib.Context
+			fakeClock            *clock.FakeClock
+			frozenNow            time.Time
+		)
+
+		// Defer starting the informers until the last possible moment so that the
+		// nested Before's can keep adding things to the informer caches.
+		var startInformersAndController = func() {
+			// Set this at the last second to allow for injection of server override.
+			subject = GarbageCollectorController(
+				fakeClock,
+				kubeClient,
+				kubeInformers.Core().V1().Secrets(),
+				controllerlib.WithInformer,
+			)
+
+			// Set this at the last second to support calling subject.Name().
+			syncContext = &controllerlib.Context{
+				Context: timeoutContext,
+				Name:    subject.Name(),
+				Key: controllerlib.Key{
+					Namespace: "",
+					Name:      "",
+				},
+			}
+
+			// Must start informers before calling TestRunSynchronously()
+			kubeInformers.Start(timeoutContext.Done())
+			controllerlib.TestRunSynchronously(t, subject)
+		}
+
+		it.Before(func() {
+			r = require.New(t)
+
+			timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3)
+
+			kubeInformerClient = kubernetesfake.NewSimpleClientset()
+			kubeClient = kubernetesfake.NewSimpleClientset()
+			kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0)
+			frozenNow = time.Now().UTC()
+			fakeClock = clock.NewFakeClock(frozenNow)
+
+			unrelatedSecret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "some other unrelated secret",
+					Namespace: installedInNamespace,
+				},
+			}
+			r.NoError(kubeInformerClient.Tracker().Add(unrelatedSecret))
+			r.NoError(kubeClient.Tracker().Add(unrelatedSecret))
+		})
+
+		it.After(func() {
+			timeoutContextCancel()
+		})
+
+		when("there are secrets without the garbage-collect-after annotation", func() {
+			it("does not delete those secrets", func() {
+				startInformersAndController()
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+
+				require.Empty(t, kubeClient.Actions())
+				list, err := kubeClient.CoreV1().Secrets(installedInNamespace).List(context.Background(), metav1.ListOptions{})
+				r.NoError(err)
+				r.Len(list.Items, 1)
+				r.Equal("some other unrelated secret", list.Items[0].Name)
+			})
+		})
+
+		when("there are secrets with the garbage-collect-after annotation", func() {
+			it.Before(func() {
+				firstExpiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "first expired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(firstExpiredSecret))
+				r.NoError(kubeClient.Tracker().Add(firstExpiredSecret))
+				secondExpiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "second expired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-2 * time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(secondExpiredSecret))
+				r.NoError(kubeClient.Tracker().Add(secondExpiredSecret))
+				unexpiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "unexpired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(unexpiredSecret))
+				r.NoError(kubeClient.Tracker().Add(unexpiredSecret))
+			})
+
+			it("should delete any that are past their expiration", func() {
+				startInformersAndController()
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+
+				r.ElementsMatch(
+					[]kubetesting.Action{
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "first expired secret"),
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "second expired secret"),
+					},
+					kubeClient.Actions(),
+				)
+				list, err := kubeClient.CoreV1().Secrets(installedInNamespace).List(context.Background(), metav1.ListOptions{})
+				r.NoError(err)
+				r.Len(list.Items, 2)
+				r.ElementsMatch([]string{"unexpired secret", "some other unrelated secret"}, []string{list.Items[0].Name, list.Items[1].Name})
+			})
+		})
+
+		when("very little time has passed since the previous sync call", func() {
+			it.Before(func() {
+				// Add a secret that will expire in 20 seconds.
+				expiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "expired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(20 * time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(expiredSecret))
+				r.NoError(kubeClient.Tracker().Add(expiredSecret))
+			})
+
+			it("should do nothing to avoid being super chatty since it is called for every change to any Secret, until more time has passed", func() {
+				startInformersAndController()
+				require.Empty(t, kubeClient.Actions())
+
+				// Run sync once with the current time set to frozenTime.
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+				require.Empty(t, kubeClient.Actions())
+
+				// Run sync again when not enough time has passed since the most recent run, so no delete
+				// operations should happen even though there is a expired secret now.
+				fakeClock.Step(29 * time.Second)
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+				require.Empty(t, kubeClient.Actions())
+
+				// Step to the exact threshold and run Sync again. Now we are past the rate limiting period.
+				fakeClock.Step(1*time.Second + 1*time.Millisecond)
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+
+				// It should have deleted the expired secret.
+				r.ElementsMatch(
+					[]kubetesting.Action{
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "expired secret"),
+					},
+					kubeClient.Actions(),
+				)
+				list, err := kubeClient.CoreV1().Secrets(installedInNamespace).List(context.Background(), metav1.ListOptions{})
+				r.NoError(err)
+				r.Len(list.Items, 1)
+				r.Equal("some other unrelated secret", list.Items[0].Name)
+			})
+		})
+
+		when("there is a secret with a malformed garbage-collect-after date", func() {
+			it.Before(func() {
+				malformedSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "malformed secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": "not-a-real-date-string",
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(malformedSecret))
+				r.NoError(kubeClient.Tracker().Add(malformedSecret))
+				expiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "expired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(expiredSecret))
+				r.NoError(kubeClient.Tracker().Add(expiredSecret))
+			})
+
+			it("does not delete that secret", func() {
+				startInformersAndController()
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+
+				r.ElementsMatch(
+					[]kubetesting.Action{
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "expired secret"),
+					},
+					kubeClient.Actions(),
+				)
+				list, err := kubeClient.CoreV1().Secrets(installedInNamespace).List(context.Background(), metav1.ListOptions{})
+				r.NoError(err)
+				r.Len(list.Items, 2)
+				r.ElementsMatch([]string{"malformed secret", "some other unrelated secret"}, []string{list.Items[0].Name, list.Items[1].Name})
+			})
+		})
+
+		when("the kube API delete call fails", func() {
+			it.Before(func() {
+				erroringSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "erroring secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(erroringSecret))
+				r.NoError(kubeClient.Tracker().Add(erroringSecret))
+				kubeClient.PrependReactor("delete", "secrets", func(action kubetesting.Action) (bool, runtime.Object, error) {
+					if action.(kubetesting.DeleteActionImpl).Name == "erroring secret" {
+						return true, nil, errors.New("delete failed: some delete error")
+					}
+					return false, nil, nil
+				})
+				expiredSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "expired secret",
+						Namespace: installedInNamespace,
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
+						},
+					},
+				}
+				r.NoError(kubeInformerClient.Tracker().Add(expiredSecret))
+				r.NoError(kubeClient.Tracker().Add(expiredSecret))
+			})
+
+			it("ignores the error and continues on to delete the next expired Secret", func() {
+				startInformersAndController()
+				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+
+				r.ElementsMatch(
+					[]kubetesting.Action{
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "erroring secret"),
+						kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "expired secret"),
+					},
+					kubeClient.Actions(),
+				)
+				list, err := kubeClient.CoreV1().Secrets(installedInNamespace).List(context.Background(), metav1.ListOptions{})
+				r.NoError(err)
+				r.Len(list.Items, 2)
+				r.ElementsMatch([]string{"erroring secret", "some other unrelated secret"}, []string{list.Items[0].Name, list.Items[1].Name})
+			})
+		})
+	}, spec.Parallel(), spec.Report(report.Terminal{}))
+}

internal/crud/crud.go

@@ -11,6 +11,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -24,6 +25,9 @@ import (
 const (
 	SecretLabelKey = "storage.pinniped.dev/type"
 
+	SecretLifetimeAnnotationKey        = "storage.pinniped.dev/garbage-collect-after"
+	SecretLifetimeAnnotationDateFormat = time.RFC3339
+
 	secretNameFormat = "pinniped-storage-%s-%s"
 	secretTypeFormat = "storage.pinniped.dev/%s"
 	secretVersion    = "1"
@@ -45,12 +49,14 @@ type Storage interface {
 
 type JSON interface{} // document that we need valid JSON types
 
-func New(resource string, secrets corev1client.SecretInterface) Storage {
+func New(resource string, secrets corev1client.SecretInterface, clock func() time.Time, lifetime time.Duration) Storage {
 	return &secretsStorage{
 		resource:      resource,
 		secretType:    corev1.SecretType(fmt.Sprintf(secretTypeFormat, resource)),
 		secretVersion: []byte(secretVersion),
 		secrets:       secrets,
+		clock:         clock,
+		lifetime:      lifetime,
 	}
 }
 
@@ -59,6 +65,8 @@ type secretsStorage struct {
 	secretType    corev1.SecretType
 	secretVersion []byte
 	secrets       corev1client.SecretInterface
+	clock         func() time.Time
+	lifetime      time.Duration
 }
 
 func (s *secretsStorage) Create(ctx context.Context, signature string, data JSON, additionalLabels map[string]string) (string, error) {
@@ -129,6 +137,9 @@ func (s *secretsStorage) DeleteByLabel(ctx context.Context, labelName string, la
 	if err != nil {
 		return fmt.Errorf(`failed to list secrets for resource "%s" matching label "%s=%s": %w`, s.resource, labelName, labelValue, err)
 	}
+	if len(list.Items) == 0 {
+		return fmt.Errorf(`failed to delete secrets for resource "%s" matching label "%s=%s": none found`, s.resource, labelName, labelValue)
+	}
 	// TODO try to delete all of the items and consolidate all of the errors and return them all
 	for _, secret := range list.Items {
 		err = s.secrets.Delete(ctx, secret.Name, metav1.DeleteOptions{})
@@ -156,18 +167,21 @@ func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON,
 		return nil, fmt.Errorf("failed to encode secret data for %s: %w", s.getName(signature), err)
 	}
 
-	labels := map[string]string{
+	labelsToAdd := map[string]string{
 		SecretLabelKey: s.resource, // make it easier to find this stuff via kubectl
 	}
 	for labelName, labelValue := range additionalLabels {
-		labels[labelName] = labelValue
+		labelsToAdd[labelName] = labelValue
 	}
 
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            s.getName(signature),
 			ResourceVersion: resourceVersion,
-			Labels:          labels,
+			Labels:          labelsToAdd,
+			Annotations: map[string]string{
+				SecretLifetimeAnnotationKey: s.clock().Add(s.lifetime).UTC().Format(SecretLifetimeAnnotationDateFormat),
+			},
 			OwnerReferences: nil,
 		},
 		Data: map[string][]byte{

internal/crud/crud_test.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/ory/fosite/compose"
 	"github.com/stretchr/testify/require"
@@ -17,6 +18,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
 	coretesting "k8s.io/client-go/testing"
 )
@@ -45,6 +47,10 @@ func TestStorage(t *testing.T) {
 
 	validateSecretName := validation.NameIsDNSSubdomain // matches k/k
 
+	fakeNow := time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC)
+	lifetime := time.Minute * 10
+	fakeNowPlusLifetimeAsString := metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339)
+
 	const (
 		namespace          = "test-ns"
 		authorizationCode1 = "81qE408EKL-e99gcXo3UnXBz9W05yGm92_hBmvXeadM.R5h38Bmw7yOaWNy0ypB3feh9toM-3T2zlwMXQyeE9B0"
@@ -56,7 +62,7 @@ func TestStorage(t *testing.T) {
 		name        string
 		resource    string
 		mocks       func(*testing.T, mocker)
-		run         func(*testing.T, Storage) error
+		run         func(*testing.T, Storage, *clock.FakeClock) error
 		wantActions []coretesting.Action
 		wantSecrets []corev1.Secret
 		wantErr     string
@@ -65,7 +71,7 @@ func TestStorage(t *testing.T) {
 			name:     "get non-existent",
 			resource: "authcode",
 			mocks:    nil,
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				_, err := storage.Get(ctx, "not-exists", nil)
 				return err
 			},
@@ -79,7 +85,7 @@ func TestStorage(t *testing.T) {
 			name:     "delete non-existent",
 			resource: "tokens",
 			mocks:    nil,
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				return storage.Delete(ctx, "not-a-token")
 			},
 			wantActions: []coretesting.Action{
@@ -88,12 +94,26 @@ func TestStorage(t *testing.T) {
 			wantSecrets: nil,
 			wantErr:     `failed to delete tokens for signature not-a-token: secrets "pinniped-storage-tokens-t2fx427lnci6s" not found`,
 		},
-		// TODO make a delete non-existent test for DeleteByLabel
+		{
+			name:     "delete non-existent by label",
+			resource: "tokens",
+			mocks:    nil,
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
+				return storage.DeleteByLabel(ctx, "additionalLabel", "matching-value")
+			},
+			wantActions: []coretesting.Action{
+				coretesting.NewListAction(secretsGVR, schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Secret"}, namespace, metav1.ListOptions{
+					LabelSelector: "storage.pinniped.dev/type=tokens,additionalLabel=matching-value",
+				}),
+			},
+			wantSecrets: nil,
+			wantErr:     `failed to delete secrets for resource "tokens" matching label "additionalLabel=matching-value": none found`,
+		},
 		{
 			name:     "create and get",
 			resource: "access-tokens",
 			mocks:    nil,
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode1)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -119,6 +139,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "access-tokens",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
@@ -137,6 +160,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "access-tokens",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
@@ -147,11 +173,106 @@ func TestStorage(t *testing.T) {
 			},
 			wantErr: "",
 		},
+		{
+			name:     "create multiple, each gets the correct lifetime timestamp",
+			resource: "access-tokens",
+			mocks:    nil,
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
+				data := &testJSON{Data: "create1"}
+				rv1, err := storage.Create(ctx, "sig1", data, nil)
+				require.Empty(t, rv1) // fake client does not set this
+				require.NoError(t, err)
+
+				fakeClock.Step(42 * time.Minute) // simulate that a known amount of time has passed
+
+				data = &testJSON{Data: "create2"}
+				rv1, err = storage.Create(ctx, "sig2", data, nil)
+				require.Empty(t, rv1) // fake client does not set this
+				require.NoError(t, err)
+
+				return nil
+			},
+			wantActions: []coretesting.Action{
+				coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-wiudk",
+						ResourceVersion: "",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create1"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				}),
+				coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-wiudm",
+						ResourceVersion: "",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": metav1.Time{Time: fakeNow.Add(42 * time.Minute).Add(lifetime)}.Format(time.RFC3339),
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create2"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				}),
+			},
+			wantSecrets: []corev1.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-wiudk",
+						Namespace:       namespace,
+						ResourceVersion: "",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create1"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-wiudm",
+						Namespace:       namespace,
+						ResourceVersion: "",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": metav1.Time{Time: fakeNow.Add(42 * time.Minute).Add(lifetime)}.Format(time.RFC3339),
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create2"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				},
+			},
+			wantErr: "",
+		},
 		{
 			name:     "create and get with additional labels",
 			resource: "access-tokens",
 			mocks:    nil,
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode1)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -179,6 +300,9 @@ func TestStorage(t *testing.T) {
 							"label1":                    "value1",
 							"label2":                    "value2",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
@@ -199,6 +323,9 @@ func TestStorage(t *testing.T) {
 							"label1":                    "value1",
 							"label2":                    "value2",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
@@ -221,6 +348,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "pandas-are-best",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"snorlax"}`),
@@ -230,7 +360,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode2)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -256,6 +386,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "pandas-are-best",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"snorlax"}`),
@@ -278,6 +411,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "stores",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"pants"}`),
@@ -293,7 +429,7 @@ func TestStorage(t *testing.T) {
 					return false, nil, nil // we mutated the secret in place but we do not "handle" it
 				})
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -327,6 +463,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "stores",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"shirts"}`),
@@ -345,6 +484,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "stores",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"shirts"}`),
@@ -367,6 +509,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "seals",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal"}`),
@@ -376,7 +521,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode2)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -402,6 +547,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",
 							"additionalLabel":           "matching-value",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal"}`),
@@ -418,6 +566,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",
 							"additionalLabel":           "matching-value",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"happy-seal"}`),
@@ -434,6 +585,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",              // same type as above
 							"additionalLabel":           "non-matching-value", // different value for the same label
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal2"}`),
@@ -450,6 +604,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "walruses",       // different type from above
 							"additionalLabel":           "matching-value", // same value for the same label as above
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal3"}`),
@@ -458,7 +615,7 @@ func TestStorage(t *testing.T) {
 					Type: "storage.pinniped.dev/walruses",
 				}))
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				return storage.DeleteByLabel(ctx, "additionalLabel", "matching-value")
 			},
 			wantActions: []coretesting.Action{
@@ -479,6 +636,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",              // same type as above
 							"additionalLabel":           "non-matching-value", // different value for the same label
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal2"}`),
@@ -496,6 +656,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "walruses",       // different type from above
 							"additionalLabel":           "matching-value", // same value for the same label as above
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal3"}`),
@@ -519,6 +682,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",
 							"additionalLabel":           "matching-value",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal"}`),
@@ -530,7 +696,7 @@ func TestStorage(t *testing.T) {
 					return true, nil, fmt.Errorf("some delete error")
 				})
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				return storage.DeleteByLabel(ctx, "additionalLabel", "matching-value")
 			},
 			wantActions: []coretesting.Action{
@@ -549,6 +715,9 @@ func TestStorage(t *testing.T) {
 							"storage.pinniped.dev/type": "seals",
 							"additionalLabel":           "matching-value",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"sad-seal"}`),
@@ -580,7 +749,7 @@ func TestStorage(t *testing.T) {
 					return true, nil, fmt.Errorf("some listing error")
 				})
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				return storage.DeleteByLabel(ctx, "additionalLabel", "matching-value")
 			},
 			wantActions: []coretesting.Action{
@@ -602,6 +771,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -611,7 +783,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -637,6 +809,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -659,6 +834,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies-are-bad",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -668,7 +846,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -694,6 +872,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies-are-bad",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -716,6 +897,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -725,7 +909,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -751,6 +935,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"twizzlers"}`),
@@ -773,6 +960,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`}}bad data{{`),
@@ -782,7 +972,7 @@ func TestStorage(t *testing.T) {
 				})
 				require.NoError(t, err)
 			},
-			run: func(t *testing.T, storage Storage) error {
+			run: func(t *testing.T, storage Storage, fakeClock *clock.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
@@ -807,6 +997,9 @@ func TestStorage(t *testing.T) {
 						Labels: map[string]string{
 							"storage.pinniped.dev/type": "candies",
 						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`}}bad data{{`),
@@ -828,9 +1021,10 @@ func TestStorage(t *testing.T) {
 				tt.mocks(t, client)
 			}
 			secrets := client.CoreV1().Secrets(namespace)
-			storage := New(tt.resource, secrets)
+			fakeClock := clock.NewFakeClock(fakeNow)
+			storage := New(tt.resource, secrets, fakeClock.Now, lifetime)
 
-			err := tt.run(t, storage)
+			err := tt.run(t, storage, fakeClock)
 
 			require.Equal(t, tt.wantErr, errString(err))
 			require.Equal(t, tt.wantActions, client.Actions())

internal/fositestorage/accesstoken/accesstoken.go

@@ -6,6 +6,7 @@ package accesstoken
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/oauth2"
@@ -43,8 +44,8 @@ type session struct {
 	Version string          `json:"version"`
 }
 
-func New(secrets corev1client.SecretInterface) RevocationStorage {
+func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionStorageLifetime time.Duration) RevocationStorage {
-	return &accessTokenStorage{storage: crud.New(TypeLabelValue, secrets)}
+	return &accessTokenStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
 }
 
 func (a *accessTokenStorage) RevokeAccessToken(ctx context.Context, requestID string) error {

internal/fositestorage/accesstoken/accesstoken_test.go

@@ -16,12 +16,18 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	coretesting "k8s.io/client-go/testing"
 )
 
 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 secretsGVR = schema.GroupVersionResource{
 	Group:    "",
 	Version:  "v1",
@@ -29,8 +35,6 @@ var secretsGVR = schema.GroupVersionResource{
 }
 
 func TestAccessTokenStorage(t *testing.T) {
-	ctx := context.Background()
-
 	wantActions := []coretesting.Action{
 		coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -40,6 +44,9 @@ func TestAccessTokenStorage(t *testing.T) {
 					"storage.pinniped.dev/type":       "access-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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -51,9 +58,7 @@ func TestAccessTokenStorage(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-access-token-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -103,8 +108,6 @@ func TestAccessTokenStorage(t *testing.T) {
 }
 
 func TestAccessTokenStorageRevocation(t *testing.T) {
-	ctx := context.Background()
-
 	wantActions := []coretesting.Action{
 		coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -114,6 +117,9 @@ func TestAccessTokenStorageRevocation(t *testing.T) {
 					"storage.pinniped.dev/type":       "access-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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -127,9 +133,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-access-token-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -159,10 +163,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) {
 }
 
 func TestGetNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	_, notFoundErr := storage.GetAccessTokenSession(ctx, "non-existent-signature", nil)
 	require.EqualError(t, notFoundErr, "not_found")
@@ -170,10 +171,7 @@ func TestGetNotFound(t *testing.T) {
 }
 
 func TestWrongVersion(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -182,6 +180,9 @@ func TestWrongVersion(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "access-token",
 			},
+			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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"not-the-right-version"}`),
@@ -198,10 +199,7 @@ func TestWrongVersion(t *testing.T) {
 }
 
 func TestNilSessionRequest(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -210,6 +208,9 @@ func TestNilSessionRequest(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "access-token",
 			},
+			Annotations: map[string]string{
+				"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+			},
 		},
 		Data: map[string][]byte{
 			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"1"}`),
@@ -226,20 +227,14 @@ func TestNilSessionRequest(t *testing.T) {
 }
 
 func TestCreateWithNilRequester(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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 := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		Session: nil,
@@ -257,10 +252,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
 }
 
 func TestCreateWithoutRequesterID(t *testing.T) {
-	ctx := context.Background()
+	ctx, client, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:      "", // empty ID
@@ -280,3 +272,9 @@ func TestCreateWithoutRequesterID(t *testing.T) {
 	// The generated secret was labeled with that auto-generated request ID
 	require.Equal(t, request.ID, actualSecret.Labels["storage.pinniped.dev/request-id"])
 }
+
+func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) {
+	client := fake.NewSimpleClientset()
+	secrets := client.CoreV1().Secrets(namespace)
+	return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
+}

internal/fositestorage/authorizationcode/authorizationcode.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	stderrors "errors"
 	"fmt"
+	"time"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/oauth2"
@@ -40,8 +41,8 @@ type AuthorizeCodeSession struct {
 	Version string          `json:"version"`
 }
 
-func New(secrets corev1client.SecretInterface) oauth2.AuthorizeCodeStorage {
+func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionStorageLifetime time.Duration) oauth2.AuthorizeCodeStorage {
-	return &authorizeCodeStorage{storage: crud.New(TypeLabelValue, secrets)}
+	return &authorizeCodeStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
 }
 
 func (a *authorizeCodeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error {

internal/fositestorage/authorizationcode/authorizationcode_test.go

@@ -15,19 +15,21 @@ import (
 	"testing"
 	"time"
 
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
-
 	fuzz "github.com/google/gofuzz"
 	"github.com/ory/fosite"
+	"github.com/ory/fosite/handler/oauth2"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/square/go-jose.v2"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	kubetesting "k8s.io/client-go/testing"
 
 	"go.pinniped.dev/internal/fositestorage"
@@ -35,8 +37,11 @@ import (
 
 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)
+
 func TestAuthorizationCodeStorage(t *testing.T) {
-	ctx := context.Background()
 	secretsGVR := schema.GroupVersionResource{
 		Group:    "",
 		Version:  "v1",
@@ -51,6 +56,9 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 				Labels: map[string]string{
 					"storage.pinniped.dev/type": "authcode",
 				},
+				Annotations: map[string]string{
+					"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+				},
 			},
 			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":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -67,6 +75,9 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 				Labels: map[string]string{
 					"storage.pinniped.dev/type": "authcode",
 				},
+				Annotations: map[string]string{
+					"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+				},
 			},
 			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":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -76,9 +87,7 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 		}),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -133,10 +142,7 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 }
 
 func TestGetNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	_, notFoundErr := storage.GetAuthorizeCodeSession(ctx, "non-existent-signature", nil)
 	require.EqualError(t, notFoundErr, "not_found")
@@ -144,10 +150,7 @@ func TestGetNotFound(t *testing.T) {
 }
 
 func TestInvalidateWhenNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	notFoundErr := storage.InvalidateAuthorizeCodeSession(ctx, "non-existent-signature")
 	require.EqualError(t, notFoundErr, "not_found")
@@ -155,10 +158,7 @@ func TestInvalidateWhenNotFound(t *testing.T) {
 }
 
 func TestInvalidateWhenConflictOnUpdateHappens(t *testing.T) {
-	ctx := context.Background()
+	ctx, client, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	client.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
 		return true, nil, apierrors.NewConflict(schema.GroupResource{
@@ -179,10 +179,7 @@ func TestInvalidateWhenConflictOnUpdateHappens(t *testing.T) {
 }
 
 func TestWrongVersion(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -207,10 +204,7 @@ func TestWrongVersion(t *testing.T) {
 }
 
 func TestNilSessionRequest(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -235,20 +229,14 @@ func TestNilSessionRequest(t *testing.T) {
 }
 
 func TestCreateWithNilRequester(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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 := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		Session: nil,
@@ -265,6 +253,12 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
 	require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient")
 }
 
+func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, oauth2.AuthorizeCodeStorage) {
+	client := fake.NewSimpleClientset()
+	secrets := client.CoreV1().Secrets(namespace)
+	return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
+}
+
 // TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession asserts that we can correctly round trip our authorize code session.
 // It will detect any changes to fosite.AuthorizeRequest and guarantees that all interface types have concrete implementations.
 func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) {
@@ -365,7 +359,7 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) {
 	const name = "fuzz" // value is irrelevant
 	ctx := context.Background()
 	secrets := fake.NewSimpleClientset().CoreV1().Secrets(name)
-	storage := New(secrets)
+	storage := New(secrets, func() time.Time { return fakeNow }, lifetime)
 
 	// issue a create using the fuzzed request to confirm that marshalling works
 	err = storage.CreateAuthorizeCodeSession(ctx, name, validSession.Request)

internal/fositestorage/openidconnect/openidconnect.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
@@ -39,8 +40,8 @@ type session struct {
 	Version string          `json:"version"`
 }
 
-func New(secrets corev1client.SecretInterface) openid.OpenIDConnectRequestStorage {
+func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionStorageLifetime time.Duration) openid.OpenIDConnectRequestStorage {
-	return &openIDConnectRequestStorage{storage: crud.New(TypeLabelValue, secrets)}
+	return &openIDConnectRequestStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
 }
 
 func (a *openIDConnectRequestStorage) CreateOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) error {

internal/fositestorage/openidconnect/openidconnect_test.go

@@ -16,14 +16,19 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	coretesting "k8s.io/client-go/testing"
 )
 
 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)
+
 func TestOpenIdConnectStorage(t *testing.T) {
-	ctx := context.Background()
 	secretsGVR := schema.GroupVersionResource{
 		Group:    "",
 		Version:  "v1",
@@ -38,6 +43,9 @@ func TestOpenIdConnectStorage(t *testing.T) {
 				Labels: map[string]string{
 					"storage.pinniped.dev/type": "oidc",
 				},
+				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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -49,9 +57,7 @@ func TestOpenIdConnectStorage(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-oidc-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -101,10 +107,7 @@ func TestOpenIdConnectStorage(t *testing.T) {
 }
 
 func TestGetNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	_, notFoundErr := storage.GetOpenIDConnectSession(ctx, "authcode.non-existent-signature", nil)
 	require.EqualError(t, notFoundErr, "not_found")
@@ -112,10 +115,7 @@ func TestGetNotFound(t *testing.T) {
 }
 
 func TestWrongVersion(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -140,10 +140,7 @@ func TestWrongVersion(t *testing.T) {
 }
 
 func TestNilSessionRequest(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -168,20 +165,14 @@ func TestNilSessionRequest(t *testing.T) {
 }
 
 func TestCreateWithNilRequester(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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 := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		Session: nil,
@@ -199,11 +190,14 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
 }
 
 func TestAuthcodeHasNoDot(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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) {
+	client := fake.NewSimpleClientset()
+	secrets := client.CoreV1().Secrets(namespace)
+	return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
+}

internal/fositestorage/pkce/pkce.go

@@ -6,6 +6,7 @@ package pkce
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
@@ -38,8 +39,8 @@ type session struct {
 	Version string          `json:"version"`
 }
 
-func New(secrets corev1client.SecretInterface) pkce.PKCERequestStorage {
+func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionStorageLifetime time.Duration) pkce.PKCERequestStorage {
-	return &pkceStorage{storage: crud.New(TypeLabelValue, secrets)}
+	return &pkceStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
 }
 
 func (a *pkceStorage) CreatePKCERequestSession(ctx context.Context, signature string, requester fosite.Requester) error {

internal/fositestorage/pkce/pkce_test.go

@@ -11,19 +11,25 @@ import (
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
+	"github.com/ory/fosite/handler/pkce"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	coretesting "k8s.io/client-go/testing"
 )
 
 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)
+
 func TestPKCEStorage(t *testing.T) {
-	ctx := context.Background()
 	secretsGVR := schema.GroupVersionResource{
 		Group:    "",
 		Version:  "v1",
@@ -38,6 +44,9 @@ func TestPKCEStorage(t *testing.T) {
 				Labels: map[string]string{
 					"storage.pinniped.dev/type": "pkce",
 				},
+				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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -49,9 +58,7 @@ func TestPKCEStorage(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-pkce-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -101,10 +108,7 @@ func TestPKCEStorage(t *testing.T) {
 }
 
 func TestGetNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	_, notFoundErr := storage.GetPKCERequestSession(ctx, "non-existent-signature", nil)
 	require.EqualError(t, notFoundErr, "not_found")
@@ -112,10 +116,7 @@ func TestGetNotFound(t *testing.T) {
 }
 
 func TestWrongVersion(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -124,6 +125,9 @@ func TestWrongVersion(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "pkce",
 			},
+			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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"not-the-right-version"}`),
@@ -140,10 +144,7 @@ func TestWrongVersion(t *testing.T) {
 }
 
 func TestNilSessionRequest(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -152,6 +153,9 @@ func TestNilSessionRequest(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "pkce",
 			},
+			Annotations: map[string]string{
+				"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+			},
 		},
 		Data: map[string][]byte{
 			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"1"}`),
@@ -168,20 +172,14 @@ func TestNilSessionRequest(t *testing.T) {
 }
 
 func TestCreateWithNilRequester(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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 := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		Session: nil,
@@ -197,3 +195,9 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
 	err = storage.CreatePKCERequestSession(ctx, "signature-doesnt-matter", request)
 	require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient")
 }
+
+func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, pkce.PKCERequestStorage) {
+	client := fake.NewSimpleClientset()
+	secrets := client.CoreV1().Secrets(namespace)
+	return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
+}

internal/fositestorage/refreshtoken/refreshtoken.go

@@ -6,6 +6,7 @@ package refreshtoken
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/oauth2"
@@ -43,8 +44,8 @@ type session struct {
 	Version string          `json:"version"`
 }
 
-func New(secrets corev1client.SecretInterface) RevocationStorage {
+func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionStorageLifetime time.Duration) RevocationStorage {
-	return &refreshTokenStorage{storage: crud.New(TypeLabelValue, secrets)}
+	return &refreshTokenStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
 }
 
 func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID string) error {

internal/fositestorage/refreshtoken/refreshtoken_test.go

@@ -16,7 +16,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/kubernetes/fake"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	coretesting "k8s.io/client-go/testing"
 )
 
@@ -27,10 +29,11 @@ var secretsGVR = schema.GroupVersionResource{
 	Version:  "v1",
 	Resource: "secrets",
 }
+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)
 
 func TestRefreshTokenStorage(t *testing.T) {
-	ctx := context.Background()
-
 	wantActions := []coretesting.Action{
 		coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -40,6 +43,9 @@ func TestRefreshTokenStorage(t *testing.T) {
 					"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -51,9 +57,7 @@ func TestRefreshTokenStorage(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -103,8 +107,6 @@ func TestRefreshTokenStorage(t *testing.T) {
 }
 
 func TestRefreshTokenStorageRevocation(t *testing.T) {
-	ctx := context.Background()
-
 	wantActions := []coretesting.Action{
 		coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -114,6 +116,9 @@ func TestRefreshTokenStorageRevocation(t *testing.T) {
 					"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`),
@@ -127,9 +132,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) {
 		coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4"),
 	}
 
-	client := fake.NewSimpleClientset()
+	ctx, client, _, storage := makeTestSubject()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:          "abcd-1",
@@ -159,10 +162,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) {
 }
 
 func TestGetNotFound(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	_, notFoundErr := storage.GetRefreshTokenSession(ctx, "non-existent-signature", nil)
 	require.EqualError(t, notFoundErr, "not_found")
@@ -170,10 +170,7 @@ func TestGetNotFound(t *testing.T) {
 }
 
 func TestWrongVersion(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -182,6 +179,9 @@ func TestWrongVersion(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "refresh-token",
 			},
+			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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"not-the-right-version"}`),
@@ -198,10 +198,7 @@ func TestWrongVersion(t *testing.T) {
 }
 
 func TestNilSessionRequest(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, secrets, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -210,6 +207,9 @@ func TestNilSessionRequest(t *testing.T) {
 			Labels: map[string]string{
 				"storage.pinniped.dev/type": "refresh-token",
 			},
+			Annotations: map[string]string{
+				"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+			},
 		},
 		Data: map[string][]byte{
 			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"1"}`),
@@ -226,20 +226,14 @@ func TestNilSessionRequest(t *testing.T) {
 }
 
 func TestCreateWithNilRequester(t *testing.T) {
-	ctx := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	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 := context.Background()
+	ctx, _, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		Session: nil,
@@ -257,10 +251,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
 }
 
 func TestCreateWithoutRequesterID(t *testing.T) {
-	ctx := context.Background()
+	ctx, client, _, storage := makeTestSubject()
-	client := fake.NewSimpleClientset()
-	secrets := client.CoreV1().Secrets(namespace)
-	storage := New(secrets)
 
 	request := &fosite.Request{
 		ID:      "", // empty ID
@@ -280,3 +271,9 @@ func TestCreateWithoutRequesterID(t *testing.T) {
 	// The generated secret was labeled with that auto-generated request ID
 	require.Equal(t, request.ID, actualSecret.Labels["storage.pinniped.dev/request-id"])
 }
+
+func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, RevocationStorage) {
+	client := fake.NewSimpleClientset()
+	secrets := client.CoreV1().Secrets(namespace)
+	return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
+}

internal/oidc/callback/callback_handler_test.go

@@ -457,11 +457,12 @@ func TestCallbackEndpoint(t *testing.T) {
 
 			// Configure fosite the same way that the production code would.
 			// Inject this into our test subject at the last second so we get a fresh storage for every test.
-			oauthStore := oidc.NewKubeStorage(secrets)
+			timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
+			oauthStore := oidc.NewKubeStorage(secrets, timeoutsConfiguration)
 			hmacSecret := []byte("some secret - must have at least 32 bytes")
 			require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes")
 			jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
-			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret, jwksProviderIsUnused, oidc.DefaultOIDCTimeoutsConfiguration())
+			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret, jwksProviderIsUnused, timeoutsConfiguration)
 
 			idpListGetter := oidctestutil.NewIDPListGetter(&test.idp)
 			subject := NewHandler(idpListGetter, oauthHelper, happyStateCodec, happyCookieCodec, happyUpstreamRedirectURI)

internal/oidc/kube_storage.go

@@ -31,13 +31,14 @@ type KubeStorage struct {
 	refreshTokenStorage      refreshtoken.RevocationStorage
 }
 
-func NewKubeStorage(secrets corev1client.SecretInterface) *KubeStorage {
+func NewKubeStorage(secrets corev1client.SecretInterface, timeoutsConfiguration TimeoutsConfiguration) *KubeStorage {
+	nowFunc := time.Now
 	return &KubeStorage{
-		authorizationCodeStorage: authorizationcode.New(secrets),
+		authorizationCodeStorage: authorizationcode.New(secrets, nowFunc, timeoutsConfiguration.AuthorizationCodeSessionStorageLifetime),
-		pkceStorage:              pkce.New(secrets),
+		pkceStorage:              pkce.New(secrets, nowFunc, timeoutsConfiguration.PKCESessionStorageLifetime),
-		oidcStorage:              openidconnect.New(secrets),
+		oidcStorage:              openidconnect.New(secrets, nowFunc, timeoutsConfiguration.OIDCSessionStorageLifetime),
-		accessTokenStorage:       accesstoken.New(secrets),
+		accessTokenStorage:       accesstoken.New(secrets, nowFunc, timeoutsConfiguration.AccessTokenSessionStorageLifetime),
-		refreshTokenStorage:      refreshtoken.New(secrets),
+		refreshTokenStorage:      refreshtoken.New(secrets, nowFunc, timeoutsConfiguration.RefreshTokenSessionStorageLifetime),
 	}
 }
 

internal/oidc/provider/manager/manager.go

@@ -77,12 +77,14 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
 
 		fositeHMACSecretForThisProvider := []byte("some secret - must have at least 32 bytes") // TODO replace this secret
 
+		timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
+
 		// Use NullStorage for the authorize endpoint because we do not actually want to store anything until
 		// the upstream callback endpoint is called later.
-		oauthHelperWithNullStorage := oidc.FositeOauth2Helper(oidc.NullStorage{}, issuer, fositeHMACSecretForThisProvider, nil, oidc.DefaultOIDCTimeoutsConfiguration())
+		oauthHelperWithNullStorage := oidc.FositeOauth2Helper(oidc.NullStorage{}, issuer, fositeHMACSecretForThisProvider, nil, timeoutsConfiguration)
 
 		// For all the other endpoints, make another oauth helper with exactly the same settings except use real storage.
-		oauthHelperWithKubeStorage := oidc.FositeOauth2Helper(oidc.NewKubeStorage(m.secretsClient), issuer, fositeHMACSecretForThisProvider, m.dynamicJWKSProvider, oidc.DefaultOIDCTimeoutsConfiguration())
+		oauthHelperWithKubeStorage := oidc.FositeOauth2Helper(oidc.NewKubeStorage(m.secretsClient, timeoutsConfiguration), issuer, fositeHMACSecretForThisProvider, m.dynamicJWKSProvider, timeoutsConfiguration)
 
 		// TODO use different codecs for the state and the cookie, because:
 		//  1. we would like to state to have an embedded expiration date while the cookie does not need that

internal/oidc/token/token_handler_test.go

@@ -495,29 +495,6 @@ func TestTokenEndpoint(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "auth code is invalidated",
-			authcodeExchange: authcodeExchangeInputs{
-				modifyStorage: func(
-					t *testing.T,
-					s interface {
-						oauth2.TokenRevocationStorage
-						oauth2.CoreStorage
-						openid.OpenIDConnectRequestStorage
-						pkce.PKCERequestStorage
-						fosite.ClientManager
-					},
-					authCode string,
-				) {
-					err := s.InvalidateAuthorizeCodeSession(context.Background(), getFositeDataSignature(t, authCode))
-					require.NoError(t, err)
-				},
-				want: tokenEndpointResponseExpectedValues{
-					wantStatus:            http.StatusBadRequest,
-					wantErrorResponseBody: fositeReusedAuthCodeErrorBody,
-				},
-			},
-		},
 		{
 			name: "redirect uri is missing in request",
 			authcodeExchange: authcodeExchangeInputs{
@@ -1155,7 +1132,7 @@ func exchangeAuthcodeForTokens(t *testing.T, test authcodeExchangeInputs) (
 
 	var oauthHelper fosite.OAuth2Provider
 
-	oauthStore = oidc.NewKubeStorage(secrets)
+	oauthStore = oidc.NewKubeStorage(secrets, oidc.DefaultOIDCTimeoutsConfiguration())
 	if test.makeOathHelper != nil {
 		oauthHelper, authCode, jwtSigningKey = test.makeOathHelper(t, authRequest, oauthStore)
 	} else {

test/integration/supervisor_storage_garbage_collection_test.go

@@ -0,0 +1,130 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package integration
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
+
+	"go.pinniped.dev/internal/crud"
+	"go.pinniped.dev/test/library"
+)
+
+func TestStorageGarbageCollection(t *testing.T) {
+	// Run this test in parallel with the other integration tests because it does a lot of waiting
+	// and will not impact other tests, or be impacted by other tests, when run in parallel.
+	t.Parallel()
+
+	env := library.IntegrationEnv(t)
+	client := library.NewClientset(t)
+	secrets := client.CoreV1().Secrets(env.SupervisorNamespace)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	secretAlreadyExpired := createSecret(ctx, t, secrets, "past", time.Now().Add(-time.Second))
+	secretWhichWillExpireBeforeTheTestEnds := createSecret(ctx, t, secrets, "near-future", time.Now().Add(30*time.Second))
+	secretNotYetExpired := createSecret(ctx, t, secrets, "far-future", time.Now().Add(10*time.Minute))
+
+	var err error
+	secretIsNotFound := func(secretName string) func() bool {
+		return func() bool {
+			_, err = secrets.Get(ctx, secretName, metav1.GetOptions{})
+			return k8serrors.IsNotFound(err)
+		}
+	}
+
+	// Start a background goroutine which will end as soon as the test ends.
+	// Keep updating a secret in the same namespace just to get the controller to respond faster.
+	// This is just a performance optimization because otherwise this test has to wait
+	// ~3 minutes for the controller's next full-resync.
+	stopCh := make(chan bool, 1) // It is important that this channel be buffered.
+	go createAndUpdateSecretEveryTwoSeconds(t, stopCh, secrets)
+	t.Cleanup(func() {
+		stopCh <- true
+	})
+
+	// Wait long enough for the next periodic sweep of the GC controller for the secrets to be deleted, which
+	// is the worst-case length of time that we should ever need to wait. Because of the goroutine above,
+	// in practice we should only need to wait about 30 seconds, which is the GC controller's self-imposed
+	// rate throttling time period.
+	slightlyLongerThanGCControllerFullResyncPeriod := 3*time.Minute + 30*time.Second
+	assert.Eventually(t, secretIsNotFound(secretAlreadyExpired.Name), slightlyLongerThanGCControllerFullResyncPeriod, 250*time.Millisecond)
+	require.Truef(t, k8serrors.IsNotFound(err), "wanted a NotFound error but got %v", err) // prints out the error and stops the test in case of failure
+	assert.Eventually(t, secretIsNotFound(secretWhichWillExpireBeforeTheTestEnds.Name), slightlyLongerThanGCControllerFullResyncPeriod, 250*time.Millisecond)
+	require.Truef(t, k8serrors.IsNotFound(err), "wanted a NotFound error but got %v", err) // prints out the error and stops the test in case of failure
+
+	// The unexpired secret should not have been deleted within the timeframe of this test run.
+	_, err = secrets.Get(ctx, secretNotYetExpired.Name, metav1.GetOptions{})
+	require.NoError(t, err)
+}
+
+func createAndUpdateSecretEveryTwoSeconds(t *testing.T, stopCh chan bool, secrets corev1client.SecretInterface) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	unrelatedSecret := createSecret(ctx, t, secrets, "unrelated-to-gc", time.Time{})
+
+	i := 0
+	for {
+		select {
+		case <-stopCh:
+			// Got a signal, so stop running.
+			return
+		default:
+			// Channel had no message, so keep running.
+		}
+
+		time.Sleep(2 * time.Second)
+
+		i++
+		unrelatedSecret.Data["foo"] = []byte(fmt.Sprintf("bar-%d", i))
+		var updateErr error
+		unrelatedSecret, updateErr = secrets.Update(ctx, unrelatedSecret, metav1.UpdateOptions{})
+		require.NoError(t, updateErr)
+	}
+}
+
+func createSecret(ctx context.Context, t *testing.T, secrets corev1client.SecretInterface, name string, expiresAt time.Time) *v1.Secret {
+	secret, err := secrets.Create(ctx, newSecret("pinniped-storage-gc-integration-test-"+name+"-", expiresAt), metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	// Make sure the Secret is deleted when the test ends.
+	t.Cleanup(func() {
+		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		defer cancel()
+		err := secrets.Delete(ctx, secret.Name, metav1.DeleteOptions{})
+		notFound := k8serrors.IsNotFound(err)
+		if !notFound {
+			// it's okay if the Secret was already deleted, but other errors are cleanup failures
+			require.NoError(t, err)
+		}
+	})
+
+	return secret
+}
+
+func newSecret(namePrefix string, expiresAt time.Time) *v1.Secret {
+	annotations := map[string]string{}
+	if !expiresAt.Equal(time.Time{}) {
+		// Mark the secret for garbage collection.
+		annotations[crud.SecretLifetimeAnnotationKey] = expiresAt.UTC().Format(time.RFC3339)
+	}
+	return &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: namePrefix,
+			Annotations:  annotations,
+		},
+		Data: map[string][]byte{"some-key": []byte("fake-data")},
+		Type: "storage.pinniped.dev/gc-test-integration-test",
+	}
+}

test/integration/supervisor_storage_test.go

@@ -14,10 +14,12 @@ import (
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"go.pinniped.dev/internal/fositestorage/authorizationcode"
+	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/test/library"
 )
 
@@ -54,7 +56,8 @@ func TestAuthorizeCodeStorage(t *testing.T) {
 	err := json.Unmarshal([]byte(authorizationcode.ExpectedAuthorizeCodeSessionJSONFromFuzzing), session)
 	require.NoError(t, err)
 
-	storage := authorizationcode.New(secrets)
+	sessionStorageLifetime := 5 * time.Minute
+	storage := authorizationcode.New(secrets, time.Now, sessionStorageLifetime)
 
 	// the session for this signature should not exist yet
 	notFoundRequest, err := storage.GetAuthorizeCodeSession(ctx, signature, nil)
@@ -75,6 +78,19 @@ func TestAuthorizeCodeStorage(t *testing.T) {
 	require.NoError(t, err)
 	require.JSONEq(t, authorizationcode.ExpectedAuthorizeCodeSessionJSONFromFuzzing, string(initialSecret.Data["pinniped-storage-data"]))
 
+	// check that the Secret got the expected annotations
+	actualGCAfterValue := initialSecret.Annotations["storage.pinniped.dev/garbage-collect-after"]
+	require.NotEmpty(t, actualGCAfterValue)
+	parsedActualGCAfterValue, err := time.Parse(time.RFC3339, actualGCAfterValue)
+	require.NoError(t, err)
+	testutil.RequireTimeInDelta(t, time.Now().Add(sessionStorageLifetime), parsedActualGCAfterValue, 30*time.Second)
+
+	// check that the Secret got the right labels
+	require.Equal(t, map[string]string{"storage.pinniped.dev/type": "authcode"}, initialSecret.Labels)
+
+	// check that the Secret got the right type
+	require.Equal(t, v1.SecretType("storage.pinniped.dev/authcode"), initialSecret.Type)
+
 	// we should be able to get the session now and the request should be the same as what we put in
 	request, err := storage.GetAuthorizeCodeSession(ctx, signature, nil)
 	require.NoError(t, err)