Merge remote-tracking branch 'upstream/main' into secret-generation

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

cmd/pinniped-supervisor/main.go

@@ -38,6 +38,7 @@ import (
 	"go.pinniped.dev/internal/controller/supervisorconfig/generator"
 	"go.pinniped.dev/internal/controller/supervisorconfig/generator/symmetricsecrethelper"
 	"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"
@@ -78,6 +79,7 @@ func waitForSignal() os.Signal {
 	return <-signalCh
 }
 
+//nolint:funlen
 func startControllers(
 	ctx context.Context,
 	cfg *supervisor.Config,
@@ -98,6 +100,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/auth/auth_handler.go

@@ -121,13 +121,22 @@ func NewHandler(
 			}
 		}
 
-		http.Redirect(w, r,
+		authCodeOptions := []oauth2.AuthCodeOption{
-			upstreamOAuthConfig.AuthCodeURL(
-				encodedStateParamValue,
 			oauth2.AccessTypeOffline,
 			nonceValue.Param(),
 			pkceValue.Challenge(),
 			pkceValue.Method(),
+		}
+
+		promptParam := r.Form.Get("prompt")
+		if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
+			authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam))
+		}
+
+		http.Redirect(w, r,
+			upstreamOAuthConfig.AuthCodeURL(
+				encodedStateParamValue,
+				authCodeOptions...,
 			),
 			302,
 		)

internal/oidc/auth/auth_handler_test.go

@@ -32,8 +32,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		downstreamIssuer                       = "https://my-downstream-issuer.com/some-path"
 		downstreamRedirectURI                  = "http://127.0.0.1/callback"
 		downstreamRedirectURIWithDifferentPort = "http://127.0.0.1:42/callback"
+		happyState                             = "8b-state"
 	)
 
+	require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
+
 	var (
 		fositeInvalidClientErrorBody = here.Doc(`
 			{
@@ -57,50 +60,50 @@ func TestAuthorizationEndpoint(t *testing.T) {
 
 		fositePromptHasNoneAndOtherValueErrorQuery = map[string]string{
 			"error":             "invalid_request",
-			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nUsed unknown value \"[none login]\" for prompt parameter",
+			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nParameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
-			"error_hint":        "Used unknown value \"[none login]\" for prompt parameter",
+			"error_hint":        "Parameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeMissingCodeChallengeErrorQuery = map[string]string{
 			"error":             "invalid_request",
 			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nClients must include a code_challenge when performing the authorize code flow, but it is missing.",
 			"error_hint":        "Clients must include a code_challenge when performing the authorize code flow, but it is missing.",
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeMissingCodeChallengeMethodErrorQuery = map[string]string{
 			"error":             "invalid_request",
 			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nClients must use code_challenge_method=S256, plain is not allowed.",
 			"error_hint":        "Clients must use code_challenge_method=S256, plain is not allowed.",
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeInvalidCodeChallengeErrorQuery = map[string]string{
 			"error":             "invalid_request",
 			"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nThe code_challenge_method is not supported, use S256 instead.",
 			"error_hint":        "The code_challenge_method is not supported, use S256 instead.",
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeUnsupportedResponseTypeErrorQuery = map[string]string{
 			"error":             "unsupported_response_type",
 			"error_description": "The authorization server does not support obtaining a token using this method\n\nThe client is not allowed to request response_type \"unsupported\".",
 			"error_hint":        `The client is not allowed to request response_type "unsupported".`,
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeInvalidScopeErrorQuery = map[string]string{
 			"error":             "invalid_scope",
 			"error_description": "The requested scope is invalid, unknown, or malformed\n\nThe OAuth 2.0 Client is not allowed to request scope \"tuna\".",
 			"error_hint":        `The OAuth 2.0 Client is not allowed to request scope "tuna".`,
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 
 		fositeInvalidStateErrorQuery = map[string]string{
 			"error":             "invalid_state",
-			"error_description": "The state is missing or does not have enough characters and is therefore considered too weak\n\nRequest parameter \"state\" must be at least be 32 characters long to ensure sufficient entropy.",
+			"error_description": "The state is missing or does not have enough characters and is therefore considered too weak\n\nRequest parameter \"state\" must be at least be 8 characters long to ensure sufficient entropy.",
-			"error_hint":        `Request parameter "state" must be at least be 32 characters long to ensure sufficient entropy.`,
+			"error_hint":        `Request parameter "state" must be at least be 8 characters long to ensure sufficient entropy.`,
 			"state":             "short",
 		}
 
@@ -108,7 +111,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"error":             "unsupported_response_type",
 			"error_description": "The authorization server does not support obtaining a token using this method\n\nThe request is missing the \"response_type\"\" parameter.",
 			"error_hint":        `The request is missing the "response_type"" parameter.`,
-			"state":             "some-state-value-that-is-32-byte",
+			"state":             happyState,
 		}
 	)
 
@@ -131,7 +134,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 
 	happyCSRF := "test-csrf"
 	happyPKCE := "test-pkce"
-	happyNonce := "test-nonce-that-is-32-bytes-long"
+	happyNonce := "test-nonce"
 	happyCSRFGenerator := func() (csrftoken.CSRFToken, error) { return csrftoken.CSRFToken(happyCSRF), nil }
 	happyPKCEGenerator := func() (pkce.Code, error) { return pkce.Code(happyPKCE), nil }
 	happyNonceGenerator := func() (nonce.Nonce, error) { return nonce.Nonce(happyNonce), nil }
@@ -177,7 +180,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		"response_type":         "code",
 		"scope":                 "openid profile email",
 		"client_id":             "pinniped-cli",
-		"state":                 "some-state-value-that-is-32-byte",
+		"state":                 happyState,
 		"nonce":                 "some-nonce-value",
 		"code_challenge":        "some-challenge",
 		"code_challenge_method": "S256",
@@ -229,8 +232,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		return encoded
 	}
 
-	expectedRedirectLocation := func(expectedUpstreamState string) string {
+	expectedRedirectLocation := func(expectedUpstreamState string, expectedPrompt string) string {
-		return urlWithQuery(upstreamAuthURL.String(), map[string]string{
+		query := map[string]string{
 			"response_type":         "code",
 			"access_type":           "offline",
 			"scope":                 "scope1 scope2",
@@ -240,7 +243,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"code_challenge":        expectedUpstreamCodeChallenge,
 			"code_challenge_method": "S256",
 			"redirect_uri":          downstreamIssuer + "/callback",
-		})
+		}
+		if expectedPrompt != "" {
+			query["prompt"] = expectedPrompt
+		}
+		return urlWithQuery(upstreamAuthURL.String(), query)
 	}
 
 	incomingCookieCSRFValue := "csrf-value-from-cookie"
@@ -288,7 +295,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantStatus:                             http.StatusFound,
 			wantContentType:                        "text/html; charset=utf-8",
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
+			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -306,7 +313,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			csrfCookie:                             "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
 			wantStatus:                             http.StatusFound,
 			wantContentType:                        "text/html; charset=utf-8",
-			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, "")),
+			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -327,7 +334,27 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType:                        "",
 			wantBodyString:                         "",
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
+			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
+		{
+			name:                                   "happy path with prompt param login passed through to redirect uri",
+			issuer:                                 downstreamIssuer,
+			idpListGetter:                          oidctestutil.NewIDPListGetter(&upstreamOIDCIdentityProvider),
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   modifiedHappyGetRequestPath(map[string]string{"prompt": "login"}),
+			contentType:                            "application/x-www-form-urlencoded",
+			body:                                   encodeQuery(happyGetRequestQueryMap),
+			wantStatus:                             http.StatusFound,
+			wantContentType:                        "text/html; charset=utf-8",
+			wantBodyStringWithLocationInHref:       true,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), "login"),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
 		{
@@ -346,7 +373,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType: "text/html; charset=utf-8",
 			// Generated a new CSRF cookie and set it in the response.
 			wantCSRFValueInCookieHeader:            happyCSRF,
-			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
+			wantLocationHeader:                     expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -368,7 +395,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{
 				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
-			}, "", "")),
+			}, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -388,7 +415,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{
 				"scope": "openid offline_access",
-			}, "", "")),
+			}, "", ""), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
@@ -586,7 +613,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantCSRFValueInCookieHeader: happyCSRF,
 			wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(
 				map[string]string{"prompt": "none login", "scope": "email"}, "", "",
-			)),
+			), ""),
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},

internal/oidc/callback/callback_handler_test.go

@@ -48,7 +48,7 @@ const (
 
 	happyUpstreamRedirectURI = "https://example.com/callback"
 
-	happyDownstreamState        = "some-downstream-state-with-at-least-32-bytes"
+	happyDownstreamState        = "8b-state"
 	happyDownstreamCSRF         = "test-csrf"
 	happyDownstreamPKCE         = "test-pkce"
 	happyDownstreamNonce        = "test-nonce"
@@ -84,6 +84,8 @@ var (
 )
 
 func TestCallbackEndpoint(t *testing.T) {
+	require.Len(t, happyDownstreamState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
+
 	otherUpstreamOIDCIdentityProvider := oidctestutil.TestUpstreamOIDCIdentityProvider{
 		Name:     "other-upstream-idp-name",
 		ClientID: "other-some-client-id",
@@ -457,11 +459,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)
 			hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") }
 			require.GreaterOrEqual(t, len(hmacSecretFunc()), 32, "fosite requires that hmac secrets have at least 32 bytes")
 			jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
-			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, oidc.DefaultOIDCTimeoutsConfiguration())
+			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
 
 			idpListGetter := oidctestutil.NewIDPListGetter(&test.idp)
 			subject := NewHandler(idpListGetter, oauthHelper, happyStateCodec, happyCookieCodec, happyUpstreamRedirectURI)

internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go

@@ -30,7 +30,7 @@ func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) {
 		clientID     = "some-client-id"
 		goodSubject  = "some-subject"
 		goodUsername = "some-username"
-		goodNonce    = "some-nonce-that-is-at-least-32-characters-to-meet-entropy-requirements"
+		goodNonce    = "some-nonce-value-with-enough-bytes-to-exceed-min-allowed"
 	)
 
 	ecPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)

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/oidc.go

@@ -205,11 +205,19 @@ func FositeOauth2Helper(
 		RefreshTokenLifespan:  timeoutsConfiguration.RefreshTokenLifespan,
 
 		ScopeStrategy: fosite.ExactScopeStrategy,
-		AudienceMatchingStrategy: nil,
 		EnforcePKCE:   true,
-		AllowedPromptValues:      []string{"none"},                        // TODO unclear what we should set here
+
-		RefreshTokenScopes:       []string{coreosoidc.ScopeOfflineAccess}, // as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
+		// "offline_access" as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
-		MinParameterEntropy:      32,                                      // TODO is 256 bits too high?
+		RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess},
+
+		// The default is to support all prompt values from the spec.
+		// See https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+		// We'll make a best effort to support these by passing the value of this prompt param to the upstream IDP
+		// and rely on its implementation of this param.
+		AllowedPromptValues: nil,
+
+		// Use the fosite default to make it more likely that off the shelf OIDC clients can work with the supervisor.
+		MinParameterEntropy: fosite.MinParameterEntropy,
 	}
 
 	return compose.Compose(
@@ -256,9 +264,16 @@ type IDPListGetter interface {
 }
 
 func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) {
-	for _, scope := range authorizeRequester.GetRequestedScopes() {
+	if ScopeWasRequested(authorizeRequester, scopeName) {
-		if scope == scopeName {
+		authorizeRequester.GrantScope(scopeName)
-			authorizeRequester.GrantScope(scope)
-		}
 	}
 }
+
+func ScopeWasRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) bool {
+	for _, scope := range authorizeRequester.GetRequestedScopes() {
+		if scope == scopeName {
+			return true
+		}
+	}
+	return false
+}

internal/oidc/provider/manager/manager.go

@@ -86,19 +86,20 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
 	for _, incomingProvider := range oidcProviders {
 		issuer := incomingProvider.Issuer()
 		issuerHostWithPath := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath()
-		oidcTimeouts := oidc.DefaultOIDCTimeoutsConfiguration()
 
 		tokenHMACKeyGetter := wrapGetter(incomingProvider.Issuer(), m.secretCache.GetTokenHMACKey)
 
+		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, tokenHMACKeyGetter, nil, oidcTimeouts)
+		oauthHelperWithNullStorage := oidc.FositeOauth2Helper(oidc.NullStorage{}, issuer, tokenHMACKeyGetter, 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, tokenHMACKeyGetter, m.dynamicJWKSProvider, oidcTimeouts)
+		oauthHelperWithKubeStorage := oidc.FositeOauth2Helper(oidc.NewKubeStorage(m.secretsClient, timeoutsConfiguration), issuer, tokenHMACKeyGetter, m.dynamicJWKSProvider, timeoutsConfiguration)
 
 		var upstreamStateEncoder = dynamiccodec.New(
-			oidcTimeouts.UpstreamStateParamLifespan,
+			timeoutsConfiguration.UpstreamStateParamLifespan,
 			wrapGetter(incomingProvider.Issuer(), m.secretCache.GetStateEncoderHashKey),
 			wrapGetter(incomingProvider.Issuer(), m.secretCache.GetStateEncoderBlockKey),
 		)

internal/oidc/provider/manager/manager_test.go

@@ -144,7 +144,7 @@ func TestManager(t *testing.T) {
 			actualLocationQueryParams := parsedLocation.Query()
 			r.Contains(actualLocationQueryParams, "code")
 			r.Equal("openid", actualLocationQueryParams.Get("scope"))
-			r.Equal("some-state-value-that-is-32-byte", actualLocationQueryParams.Get("state"))
+			r.Equal("some-state-value-with-enough-bytes-to-exceed-min-allowed", actualLocationQueryParams.Get("state"))
 
 			// Make sure that we wired up the callback endpoint to use kube storage for fosite sessions.
 			r.Equal(len(kubeClient.Actions()), numberOfKubeActionsBeforeThisRequest+3,
@@ -306,8 +306,8 @@ func TestManager(t *testing.T) {
 				"response_type":         []string{"code"},
 				"scope":                 []string{"openid profile email"},
 				"client_id":             []string{downstreamClientID},
-				"state":                 []string{"some-state-value-that-is-32-byte"},
+				"state":                 []string{"some-state-value-with-enough-bytes-to-exceed-min-allowed"},
-				"nonce":                 []string{"some-nonce-value-that-is-at-least-32-bytes"},
+				"nonce":                 []string{"some-nonce-value-with-enough-bytes-to-exceed-min-allowed"},
 				"code_challenge":        []string{testutil.SHA256(downstreamPKCECodeVerifier)},
 				"code_challenge_method": []string{"S256"},
 				"redirect_uri":          []string{downstreamRedirectURL},

internal/oidc/token/token_handler_test.go

@@ -52,7 +52,7 @@ const (
 	goodClient           = "pinniped-cli"
 	goodRedirectURI      = "http://127.0.0.1/callback"
 	goodPKCECodeVerifier = "some-pkce-verifier-that-must-be-at-least-43-characters-to-meet-entropy-requirements"
-	goodNonce            = "some-nonce-that-is-at-least-32-characters-to-meet-entropy-requirements"
+	goodNonce            = "some-nonce-value-with-enough-bytes-to-exceed-min-allowed"
 	goodSubject          = "some-subject"
 	goodUsername         = "some-username"
 
@@ -217,7 +217,7 @@ var (
 			"response_type":         {"code"},
 			"scope":                 {"openid profile email"},
 			"client_id":             {goodClient},
-			"state":                 {"some-state-value-that-is-32-byte"},
+			"state":                 {"some-state-value-with-enough-bytes-to-exceed-min-allowed"},
 			"nonce":                 {goodNonce},
 			"code_challenge":        {testutil.SHA256(goodPKCECodeVerifier)},
 			"code_challenge_method": {"S256"},
@@ -499,29 +499,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{
@@ -1159,7 +1136,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)