Allow dynamic clients to be used in downstream OIDC flows

This is only a first commit towards making this feature work. - Hook dynamic clients into fosite by returning them from the storage interface (after finding and validating them) - In the auth endpoint, prevent the use of the username and password headers for dynamic clients to force them to use the browser-based login flows for all the upstream types - Add happy path integration tests in supervisor_login_test.go - Add lots of comments (and some small refactors) in supervisor_login_test.go to make it much easier to understand - Add lots of unit tests for the auth endpoint regarding dynamic clients (more unit tests to be added for other endpoints in follow-up commits) - Enhance crud.go to make lifetime=0 mean never garbage collect, since we want client secret storage Secrets to last forever - Move the OIDCClient validation code to a package where it can be shared between the controller and the fosite storage interface - Make shared test helpers for tests that need to create OIDC client secret storage Secrets - Create a public const for "pinniped-cli" now that we are using that string in several places in the production code
2026-09-05 07:37:12 +00:00 · 2022-07-14 09:51:11 -07:00
parent be85e1ed0a
commit e0ecdc004b
28 changed files with 1692 additions and 461 deletions
@@ -8,9 +8,6 @@ import (
 	"fmt"
 	"strings"

-	"github.com/coreos/go-oidc/v3/oidc"
-	"golang.org/x/crypto/bcrypt"
-	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -23,37 +20,14 @@ import (
 	pinnipedcontroller "go.pinniped.dev/internal/controller"
 	"go.pinniped.dev/internal/controller/conditionsutil"
 	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/oidcclientsecretstorage"
 	"go.pinniped.dev/internal/plog"
 )

 const (
-	clientSecretExists     = "ClientSecretExists"
-	allowedGrantTypesValid = "AllowedGrantTypesValid"
-	allowedScopesValid     = "AllowedScopesValid"
-
-	reasonSuccess                  = "Success"
-	reasonMissingRequiredValue     = "MissingRequiredValue"
-	reasonNoClientSecretFound      = "NoClientSecretFound"
-	reasonInvalidClientSecretFound = "InvalidClientSecretFound"
-
-	authorizationCodeGrantTypeName = "authorization_code"
-	refreshTokenGrantTypeName      = "refresh_token"
-	tokenExchangeGrantTypeName     = "urn:ietf:params:oauth:grant-type:token-exchange" //nolint:gosec // this is not a credential
-
-	openidScopeName          = oidc.ScopeOpenID
-	offlineAccessScopeName   = oidc.ScopeOfflineAccess
-	requestAudienceScopeName = "pinniped:request-audience"
-	usernameScopeName        = "username"
-	groupsScopeName          = "groups"
-
-	allowedGrantTypesFieldName = "allowedGrantTypes"
-	allowedScopesFieldName     = "allowedScopes"
-
 	secretTypeToObserve       = "storage.pinniped.dev/oidc-client-secret" //nolint:gosec // this is not a credential
 	oidcClientPrefixToObserve = "client.oauth.pinniped.dev-"              //nolint:gosec // this is not a credential
-
-	minimumRequiredBcryptCost = 15
 )

 type oidcClientWatcherController struct {
@@ -133,9 +107,9 @@ func (c *oidcClientWatcherController) Sync(ctx controllerlib.Context) error {
 			secret = nil
 		}

-		conditions, totalClientSecrets := validateOIDCClient(oidcClient, secret)
+		_, conditions, clientSecrets := oidcclientvalidator.Validate(oidcClient, secret)

-		if err := c.updateStatus(ctx.Context, oidcClient, conditions, totalClientSecrets); err != nil {
+		if err := c.updateStatus(ctx.Context, oidcClient, conditions, len(clientSecrets)); err != nil {
 			return fmt.Errorf("cannot update OIDCClient '%s/%s': %w", oidcClient.Namespace, oidcClient.Name, err)
 		}

@@ -150,185 +124,6 @@ func (c *oidcClientWatcherController) Sync(ctx controllerlib.Context) error {
 	return nil
 }

-// validateOIDCClient validates the OIDCClient and its corresponding client secret storage Secret.
-// When the corresponding client secret storage Secret was not found, pass nil to this function to
-// get the validation error for that case. It returns a slice of conditions along with the number
-// of client secrets found.
-func validateOIDCClient(oidcClient *v1alpha1.OIDCClient, secret *v1.Secret) ([]*v1alpha1.Condition, int) {
-	c, totalClientSecrets := validateSecret(secret, make([]*v1alpha1.Condition, 0, 3))
-	c = validateAllowedGrantTypes(oidcClient, c)
-	c = validateAllowedScopes(oidcClient, c)
-	return c, totalClientSecrets
-}
-
-// validateAllowedScopes checks if allowedScopes is valid on the OIDCClient.
-func validateAllowedScopes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
-	m := make([]string, 0, 4)
-
-	if !allowedScopesContains(oidcClient, openidScopeName) {
-		m = append(m, fmt.Sprintf("%q must always be included in %q", openidScopeName, allowedScopesFieldName))
-	}
-	if allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName) && !allowedScopesContains(oidcClient, offlineAccessScopeName) {
-		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			offlineAccessScopeName, allowedScopesFieldName, refreshTokenGrantTypeName, allowedGrantTypesFieldName))
-	}
-	if allowedScopesContains(oidcClient, requestAudienceScopeName) &&
-		(!allowedScopesContains(oidcClient, usernameScopeName) || !allowedScopesContains(oidcClient, groupsScopeName)) {
-		m = append(m, fmt.Sprintf("%q and %q must be included in %q when %q is included in %q",
-			usernameScopeName, groupsScopeName, allowedScopesFieldName, requestAudienceScopeName, allowedScopesFieldName))
-	}
-	if allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName) && !allowedScopesContains(oidcClient, requestAudienceScopeName) {
-		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			requestAudienceScopeName, allowedScopesFieldName, tokenExchangeGrantTypeName, allowedGrantTypesFieldName))
-	}
-
-	if len(m) == 0 {
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    allowedScopesValid,
-			Status:  v1alpha1.ConditionTrue,
-			Reason:  reasonSuccess,
-			Message: fmt.Sprintf("%q is valid", allowedScopesFieldName),
-		})
-	} else {
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    allowedScopesValid,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonMissingRequiredValue,
-			Message: strings.Join(m, "; "),
-		})
-	}
-
-	return conditions
-}
-
-// validateAllowedGrantTypes checks if allowedGrantTypes is valid on the OIDCClient.
-func validateAllowedGrantTypes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
-	m := make([]string, 0, 3)
-
-	if !allowedGrantTypesContains(oidcClient, authorizationCodeGrantTypeName) {
-		m = append(m, fmt.Sprintf("%q must always be included in %q",
-			authorizationCodeGrantTypeName, allowedGrantTypesFieldName))
-	}
-	if allowedScopesContains(oidcClient, offlineAccessScopeName) && !allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName) {
-		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			refreshTokenGrantTypeName, allowedGrantTypesFieldName, offlineAccessScopeName, allowedScopesFieldName))
-	}
-	if allowedScopesContains(oidcClient, requestAudienceScopeName) && !allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName) {
-		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			tokenExchangeGrantTypeName, allowedGrantTypesFieldName, requestAudienceScopeName, allowedScopesFieldName))
-	}
-
-	if len(m) == 0 {
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    allowedGrantTypesValid,
-			Status:  v1alpha1.ConditionTrue,
-			Reason:  reasonSuccess,
-			Message: fmt.Sprintf("%q is valid", allowedGrantTypesFieldName),
-		})
-	} else {
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    allowedGrantTypesValid,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonMissingRequiredValue,
-			Message: strings.Join(m, "; "),
-		})
-	}
-
-	return conditions
-}
-
-// validateSecret checks if the client secret storage Secret is valid and contains at least one client secret.
-// It returns the updated conditions slice along with the number of client secrets found.
-func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition) ([]*v1alpha1.Condition, int) {
-	if secret == nil {
-		// Invalid: no storage Secret found.
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    clientSecretExists,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonNoClientSecretFound,
-			Message: "no client secret found (no Secret storage found)",
-		})
-		return conditions, 0
-	}
-
-	storedClientSecret, err := oidcclientsecretstorage.ReadFromSecret(secret)
-	if err != nil {
-		// Invalid: storage Secret exists but its data could not be parsed.
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    clientSecretExists,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonNoClientSecretFound,
-			Message: fmt.Sprintf("error reading client secret storage: %s", err.Error()),
-		})
-		return conditions, 0
-	}
-
-	// Successfully read the stored client secrets, so check if there are any stored in the list.
-	storedClientSecretsCount := len(storedClientSecret.SecretHashes)
-	if storedClientSecretsCount == 0 {
-		// Invalid: no client secrets stored.
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    clientSecretExists,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonNoClientSecretFound,
-			Message: "no client secret found (empty list in storage)",
-		})
-		return conditions, 0
-	}
-
-	// Check each hashed password's format and bcrypt cost.
-	bcryptErrs := make([]string, 0, storedClientSecretsCount)
-	for i, p := range storedClientSecret.SecretHashes {
-		cost, err := bcrypt.Cost([]byte(p))
-		if err != nil {
-			bcryptErrs = append(bcryptErrs, fmt.Sprintf(
-				"hashed client secret at index %d: %s",
-				i, err.Error()))
-		} else if cost < minimumRequiredBcryptCost {
-			bcryptErrs = append(bcryptErrs, fmt.Sprintf(
-				"hashed client secret at index %d: bcrypt cost %d is below the required minimum of %d",
-				i, cost, minimumRequiredBcryptCost))
-		}
-	}
-	if len(bcryptErrs) > 0 {
-		// Invalid: some stored client secrets were not valid.
-		conditions = append(conditions, &v1alpha1.Condition{
-			Type:    clientSecretExists,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  reasonInvalidClientSecretFound,
-			Message: strings.Join(bcryptErrs, "; "),
-		})
-		return conditions, storedClientSecretsCount
-	}
-
-	// Valid: has at least one client secret stored for this OIDC client, and all stored client secrets are valid.
-	conditions = append(conditions, &v1alpha1.Condition{
-		Type:    clientSecretExists,
-		Status:  v1alpha1.ConditionTrue,
-		Reason:  reasonSuccess,
-		Message: fmt.Sprintf("%d client secret(s) found", storedClientSecretsCount),
-	})
-	return conditions, storedClientSecretsCount
-}
-
-func allowedGrantTypesContains(haystack *v1alpha1.OIDCClient, needle string) bool {
-	for _, hay := range haystack.Spec.AllowedGrantTypes {
-		if hay == v1alpha1.GrantType(needle) {
-			return true
-		}
-	}
-	return false
-}
-
-func allowedScopesContains(haystack *v1alpha1.OIDCClient, needle string) bool {
-	for _, hay := range haystack.Spec.AllowedScopes {
-		if hay == v1alpha1.Scope(needle) {
-			return true
-		}
-	}
-	return false
-}
-
 func (c *oidcClientWatcherController) updateStatus(
 	ctx context.Context,
 	upstream *v1alpha1.OIDCClient,
@@ -5,9 +5,7 @@ package oidcclientwatcher

 import (
 	"context"
-	"encoding/base32"
 	"fmt"
-	"strings"
 	"testing"
 	"time"

@@ -257,51 +255,6 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 		}
 	}

-	secretNameForUID := func(uid string) string {
-		// See GetName() in OIDCClientSecretStorage for how the production code determines the Secret name.
-		// This test helper is intended to choose the same name.
-		return "pinniped-storage-oidc-client-secret-" +
-			strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString([]byte(uid)))
-	}
-
-	secretStringDataWithZeroClientSecrets := map[string][]byte{
-		"pinniped-storage-data":    []byte(`{"version":"1","hashes":[]}`),
-		"pinniped-storage-version": []byte("1"),
-	}
-
-	secretStringDataWithOneClientSecret := map[string][]byte{
-		"pinniped-storage-data":    []byte(`{"version":"1","hashes":["` + testBcryptSecret1 + `"]}`),
-		"pinniped-storage-version": []byte("1"),
-	}
-
-	secretStringDataWithTwoClientSecrets := map[string][]byte{
-		"pinniped-storage-data":    []byte(`{"version":"1","hashes":["` + testBcryptSecret1 + `","` + testBcryptSecret2 + `"]}`),
-		"pinniped-storage-version": []byte("1"),
-	}
-
-	secretStringDataWithSomeInvalidClientSecrets := map[string][]byte{
-		"pinniped-storage-data": []byte(`{"version":"1","hashes":["` +
-			testBcryptSecret1 + `","` + testInvalidBcryptSecretCostTooLow + `","` + testInvalidBcryptSecretInvalidFormat + `"]}`),
-		"pinniped-storage-version": []byte("1"),
-	}
-
-	secretStringDataWithWrongVersion := map[string][]byte{
-		"pinniped-storage-data":    []byte(`{"version":"wrong-version","hashes":[]}`),
-		"pinniped-storage-version": []byte("1"),
-	}
-
-	storageSecretForUIDWithData := func(uid string, data map[string][]byte) *corev1.Secret {
-		return &corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: testNamespace,
-				Name:      secretNameForUID(uid),
-				Labels:    map[string]string{"storage.pinniped.dev/type": "oidc-client-secret"},
-			},
-			Type: "storage.pinniped.dev/oidc-client-secret",
-			Data: data,
-		}
-	}
-
 	tests := []struct {
 		name                     string
 		inputObjects             []runtime.Object
@@ -338,7 +291,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					},
 				},
 			},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{
 				{
@@ -367,7 +320,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithTwoClientSecrets)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1, testBcryptSecret2})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -400,7 +353,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					TotalClientSecrets: 1,
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 0, // no updates
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -443,7 +396,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithWrongVersion)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUIDWithWrongVersion(t, testNamespace, testUID)},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -466,7 +419,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithZeroClientSecrets)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -490,7 +443,10 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithSomeInvalidClientSecrets)},
+			inputSecrets: []runtime.Object{
+				testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID,
+					[]string{testBcryptSecret1, testInvalidBcryptSecretCostTooLow, testInvalidBcryptSecretInvalidFormat}),
+			},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -500,10 +456,11 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 						happyAllowedGrantTypesCondition(now, 1234),
 						happyAllowedScopesCondition(now, 1234),
 						sadInvalidClientSecretsCondition(now, 1234,
-							"hashed client secret at index 1: bcrypt cost 14 is below the required minimum of 15; "+
+							"3 stored client secrets found, but some were invalid, so none will be used: "+
+								"hashed client secret at index 1: bcrypt cost 14 is below the required minimum of 15; "+
 								"hashed client secret at index 2: crypto/bcrypt: hashedSecret too short to be a bcrypted password"),
 					},
-					TotalClientSecrets: 3,
+					TotalClientSecrets: 0,
 				},
 			}},
 		},
@@ -522,7 +479,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					Spec:       configv1alpha1.OIDCClientSpec{},
 				},
 			},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData("uid1", secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, "uid1", []string{testBcryptSecret1})},
 			wantAPIActions: 2, // one update for each OIDCClient
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{
 				{
@@ -570,7 +527,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					TotalClientSecrets: 1,
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 4567, UID: testUID},
@@ -596,7 +553,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 				},
 			}},
 			wantAPIActions: 1, // one update
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -620,7 +577,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 				},
 			}},
 			wantAPIActions: 1, // one update
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -649,7 +606,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 				},
 			}},
 			wantAPIActions: 1, // one update
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -676,7 +633,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "pinniped:request-audience", "username", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -700,7 +657,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -724,7 +681,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "pinniped:request-audience"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -748,7 +705,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "pinniped:request-audience", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -772,7 +729,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "pinniped:request-audience", "username"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -796,7 +753,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -820,7 +777,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -844,7 +801,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "offline_access"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -868,7 +825,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "offline_access", "username"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -892,7 +849,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "offline_access", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -916,7 +873,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "offline_access", "username", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -940,7 +897,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "username"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -964,7 +921,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "username"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -988,7 +945,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					AllowedScopes:     []configv1alpha1.Scope{"openid", "username", "groups"},
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testBcryptSecret1})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},