Merge pull request #1236 from vmware-tanzu/dynamic_clients_in_downstream_flows

Allow dynamic clients to be used in downstream OIDC flows

apis/supervisor/config/v1alpha1/types_oidcclient.go.tmpl

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

deploy/supervisor/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.17/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.17/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.18/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.18/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.19/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.19/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.20/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.20/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.21/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.21/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.22/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.22/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.23/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.23/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/1.24/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

generated/1.24/crds/config.supervisor.pinniped.dev_oidcclients.yaml

@@ -18,6 +18,15 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
+    - jsonPath: .spec.allowedScopes[?(@ == "pinniped:request-audience")]
+      name: Privileged Scopes
+      type: string
+    - jsonPath: .status.totalClientSecrets
+      name: Client Secrets
+      type: integer
+    - jsonPath: .status.phase
+      name: Status
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date

generated/latest/apis/supervisor/config/v1alpha1/types_oidcclient.go

@@ -88,13 +88,17 @@ type OIDCClientStatus struct {
 	Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// totalClientSecrets is the current number of client secrets that are detected for this OIDCClient.
-	TotalClientSecrets int32 `json:"totalClientSecrets,omitempty"`
+	// +optional
+	TotalClientSecrets int32 `json:"totalClientSecrets"` // do not omitempty to allow it to show in the printer column even when it is 0
 }
 
 // OIDCClient describes the configuration of an OIDC client.
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=pinniped
+// +kubebuilder:printcolumn:name="Privileged Scopes",type=string,JSONPath=`.spec.allowedScopes[?(@ == "pinniped:request-audience")]`
+// +kubebuilder:printcolumn:name="Client Secrets",type=integer,JSONPath=`.status.totalClientSecrets`
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:subresource:status
 type OIDCClient struct {

internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go

@@ -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, oidcclientvalidator.DefaultMinBcryptCost)
 
-		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,

internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher_test.go

@@ -5,9 +5,7 @@ package oidcclientwatcher
 
 import (
 	"context"
-	"encoding/base32"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -166,15 +164,6 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 		testName      = "client.oauth.pinniped.dev-test-name"
 		testNamespace = "test-namespace"
 		testUID       = "test-uid-123"
-
-		//nolint:gosec // this is not a credential
-		testBcryptSecret1 = "$2y$15$Kh7cRj0ScSD5QelE3ZNSl.nF04JDv7zb3SgGN.tSfLIX.4kt3UX7m" // bcrypt of "password1" at cost 15
-		//nolint:gosec // this is not a credential
-		testBcryptSecret2 = "$2y$15$Kh7cRj0ScSD5QelE3ZNSl.nF04JDv7zb3SgGN.tSfLIX.4kt3UX7m" // bcrypt of "password2" at cost 15
-		//nolint:gosec // this is not a credential
-		testInvalidBcryptSecretCostTooLow = "$2y$14$njwk1cItiRy6cb6u9aiJLuhtJG83zM9111t.xU6MxvnqqYbkXxzwy" // bcrypt of "password1" at cost 14
-		//nolint:gosec // this is not a credential
-		testInvalidBcryptSecretInvalidFormat = "$2y$14$njwk1cItiRy6cb6u9aiJLuhtJG83zM9111t.xU6MxvnqqYbkXxz" // not enough characters in hash value
 	)
 
 	now := metav1.NewTime(time.Now().UTC())
@@ -257,51 +246,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 +282,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					},
 				},
 			},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{
 				{
@@ -367,7 +311,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{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword2AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -400,7 +344,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					TotalClientSecrets: 1,
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 0, // no updates
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -443,7 +387,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 +410,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 +434,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{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword1JustBelowSupervisorMinCost, testutil.HashedPassword1InvalidFormat}),
+			},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -500,10 +447,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 11 is below the required minimum of 12; "+
 								"hashed client secret at index 2: crypto/bcrypt: hashedSecret too short to be a bcrypted password"),
 					},
-					TotalClientSecrets: 3,
+					TotalClientSecrets: 0,
 				},
 			}},
 		},
@@ -522,7 +470,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					Spec:       configv1alpha1.OIDCClientSpec{},
 				},
 			},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData("uid1", secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, "uid1", []string{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 2, // one update for each OIDCClient
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{
 				{
@@ -570,7 +518,7 @@ func TestOIDCClientWatcherControllerSync(t *testing.T) {
 					TotalClientSecrets: 1,
 				},
 			}},
-			inputSecrets:   []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
+			inputSecrets:   []runtime.Object{testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 4567, UID: testUID},
@@ -596,7 +544,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -620,7 +568,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -649,7 +597,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
 				Status: configv1alpha1.OIDCClientStatus{
@@ -676,7 +624,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -700,7 +648,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -724,7 +672,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -748,7 +696,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -772,7 +720,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -796,7 +744,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -820,7 +768,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -844,7 +792,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -868,7 +816,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -892,7 +840,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -916,7 +864,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -940,7 +888,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -964,7 +912,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
@@ -988,7 +936,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{testutil.HashedPassword1AtSupervisorMinCost})},
 			wantAPIActions: 1, // one update
 			wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},

internal/crud/crud.go

@@ -193,14 +193,19 @@ func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON,
 		labelsToAdd[labelName] = labelValue
 	}
 
+	var annotations map[string]string
+	if s.lifetime > 0 {
+		annotations = map[string]string{
+			SecretLifetimeAnnotationKey: s.clock().Add(s.lifetime).UTC().Format(SecretLifetimeAnnotationDateFormat),
+		}
+	}
+
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            s.GetName(signature),
 			ResourceVersion: resourceVersion,
 			Labels:          labelsToAdd,
-			Annotations: map[string]string{
+			Annotations:     annotations,
-				SecretLifetimeAnnotationKey: s.clock().Add(s.lifetime).UTC().Format(SecretLifetimeAnnotationDateFormat),
-			},
 			OwnerReferences: nil,
 		},
 		Data: map[string][]byte{

internal/crud/crud_test.go

@@ -62,6 +62,7 @@ func TestStorage(t *testing.T) {
 		name        string
 		resource    string
 		mocks       func(*testing.T, mocker)
+		lifetime    func() time.Duration
 		run         func(*testing.T, Storage, *clocktesting.FakeClock) error
 		wantActions []coretesting.Action
 		wantSecrets []corev1.Secret
@@ -1014,7 +1015,69 @@ func TestStorage(t *testing.T) {
 			},
 			wantErr: "",
 		},
+		{
+			name:     "create and get with infinite lifetime when lifetime is specified as zero",
+			resource: "access-tokens",
+			mocks:    nil,
+			lifetime: func() time.Duration { return 0 }, // 0 == infinity
+			run: func(t *testing.T, storage Storage, fakeClock *clocktesting.FakeClock) error {
+				signature := hmac.AuthorizeCodeSignature(authorizationCode1)
+				require.NotEmpty(t, signature)
+				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
+
+				data := &testJSON{Data: "create-and-get"}
+				rv1, err := storage.Create(ctx, signature, data, nil)
+				require.Empty(t, rv1) // fake client does not set this
+				require.NoError(t, err)
+
+				out := &testJSON{}
+				rv2, err := storage.Get(ctx, signature, out)
+				require.Empty(t, rv2) // fake client does not set this
+				require.NoError(t, err)
+				require.Equal(t, data, out)
+
+				return nil
+			},
+			wantActions: []coretesting.Action{
+				coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						ResourceVersion: "",
+						// No garbage collection annotation was added.
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				}),
+				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"),
+			},
+			wantSecrets: []corev1.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						Namespace:       namespace,
+						ResourceVersion: "",
+						// No garbage collection annotation was added.
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				},
+			},
+			wantErr: "",
+		},
 	}
+
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
@@ -1024,9 +1087,13 @@ func TestStorage(t *testing.T) {
 			if tt.mocks != nil {
 				tt.mocks(t, client)
 			}
+			useLifetime := lifetime
+			if tt.lifetime != nil {
+				useLifetime = tt.lifetime()
+			}
 			secrets := client.CoreV1().Secrets(namespace)
 			fakeClock := clocktesting.NewFakeClock(fakeNow)
-			storage := New(tt.resource, secrets, fakeClock.Now, lifetime)
+			storage := New(tt.resource, secrets, fakeClock.Now, useLifetime)
 
 			err := tt.run(t, storage, fakeClock)
 

internal/oidc/auth/auth_handler.go

@@ -20,6 +20,7 @@ import (
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/clientregistry"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/login"
@@ -126,6 +127,10 @@ func handleAuthRequestForLDAPUpstreamCLIFlow(
 		return nil
 	}
 
+	if !requireStaticClientForUsernameAndPasswordHeaders(w, oauthHelper, authorizeRequester) {
+		return nil
+	}
+
 	username, password, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester)
 	if !hadUsernamePasswordValues {
 		return nil
@@ -199,6 +204,10 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
 		return nil
 	}
 
+	if !requireStaticClientForUsernameAndPasswordHeaders(w, oauthHelper, authorizeRequester) {
+		return nil
+	}
+
 	username, password, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester)
 	if !hadUsernamePasswordValues {
 		return nil
@@ -312,6 +321,15 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow(
 	return nil
 }
 
+func requireStaticClientForUsernameAndPasswordHeaders(w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) bool {
+	isStaticClient := authorizeRequester.GetClient().GetID() == clientregistry.PinnipedCLIClientID
+	if !isStaticClient {
+		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
+			fosite.ErrAccessDenied.WithHintf("This client is not allowed to submit username or password headers to this endpoint."), true)
+	}
+	return isStaticClient
+}
+
 func requireNonEmptyUsernameAndPasswordHeaders(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) (string, string, bool) {
 	username := r.Header.Get(supervisoroidc.AuthorizeUsernameHeaderName)
 	password := r.Header.Get(supervisoroidc.AuthorizePasswordHeaderName)
@@ -330,10 +348,12 @@ func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fos
 		return nil, false
 	}
 
-	// Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested.
+	// Automatically grant the openid, offline_access, pinniped:request-audience, and groups scopes, but only if they were requested.
 	// Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
 	// There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope
 	// at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite.
+	// This is instead of asking the user to approve these scopes. Note that `NewAuthorizeRequest` would have returned
+	// an error if the client requested a scope that they are not allowed to request, so we don't need to worry about that here.
 	downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope})
 
 	return authorizeRequester, true

internal/oidc/auth/auth_handler_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/gorilla/securecookie"
 	"github.com/ory/fosite"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 	"golang.org/x/oauth2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/authentication/user"
@@ -26,6 +27,8 @@ import (
 	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/utils/pointer"
 
+	supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
+	"go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
 	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/oidc"
@@ -67,11 +70,14 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		downstreamPKCEChallenge                = "some-challenge"
 		downstreamPKCEChallengeMethod          = "S256"
 		happyState                             = "8b-state"
-		downstreamClientID                     = "pinniped-cli"
 		upstreamLDAPURL                        = "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev"
 		htmlContentType                        = "text/html; charset=utf-8"
 		jsonContentType                        = "application/json; charset=utf-8"
 		formContentType                        = "application/x-www-form-urlencoded"
+
+		pinnipedCLIClientID = "pinniped-cli"
+		dynamicClientID     = "client.oauth.pinniped.dev-test-name"
+		dynamicClientUID    = "fake-client-uid"
 	)
 
 	require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
@@ -177,6 +183,12 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"state":             happyState,
 		}
 
+		fositeAccessDeniedWithUsernamePasswordHeadersDisallowedHintErrorQuery = map[string]string{
+			"error":             "access_denied",
+			"error_description": "The resource owner or authorization server denied the request. This client is not allowed to submit username or password headers to this endpoint.",
+			"state":             happyState,
+		}
+
 		fositeAccessDeniedWithInvalidEmailVerifiedHintErrorQuery = map[string]string{
 			"error":             "access_denied",
 			"error_description": "The resource owner or authorization server denied the request. Reason: email_verified claim in upstream ID token has invalid format.",
@@ -219,16 +231,20 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
 	timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
 
-	createOauthHelperWithRealStorage := func(secretsClient v1.SecretInterface) (fosite.OAuth2Provider, *oidc.KubeStorage) {
+	createOauthHelperWithRealStorage := func(secretsClient v1.SecretInterface, oidcClientsClient v1alpha1.OIDCClientInterface) (fosite.OAuth2Provider, *oidc.KubeStorage) {
 		// Configure fosite the same way that the production code would when using Kube storage.
 		// Inject this into our test subject at the last second so we get a fresh storage for every test.
-		kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration)
+		// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
+		kubeOauthStore := oidc.NewKubeStorage(secretsClient, oidcClientsClient, timeoutsConfiguration, bcrypt.MinCost)
 		return oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), kubeOauthStore
 	}
 
+	createOauthHelperWithNullStorage := func(secretsClient v1.SecretInterface, oidcClientsClient v1alpha1.OIDCClientInterface) (fosite.OAuth2Provider, *oidc.NullStorage) {
 		// Configure fosite the same way that the production code would, using NullStorage to turn off storage.
-	nullOauthStore := oidc.NullStorage{}
+		// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
-	oauthHelperWithNullStorage := oidc.FositeOauth2Helper(nullOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
+		nullOauthStore := oidc.NewNullStorage(secretsClient, oidcClientsClient, bcrypt.MinCost)
+		return oidc.FositeOauth2Helper(nullOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), nullOauthStore
+	}
 
 	upstreamAuthURL, err := url.Parse("https://some-upstream-idp:8443/auth")
 	require.NoError(t, err)
@@ -381,7 +397,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	happyGetRequestQueryMap := map[string]string{
 		"response_type":         "code",
 		"scope":                 strings.Join(happyDownstreamScopesRequested, " "),
-		"client_id":             downstreamClientID,
+		"client_id":             pinnipedCLIClientID,
 		"state":                 happyState,
 		"nonce":                 downstreamNonce,
 		"code_challenge":        downstreamPKCEChallenge,
@@ -494,6 +510,13 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		},
 	}
 
+	addFullyCapableDynamicClientAndSecretToKubeResources := func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+		oidcClient, secret := testutil.FullyCapableOIDCClientAndStorageSecret(t,
+			"some-namespace", dynamicClientID, dynamicClientUID, downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost})
+		require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+		require.NoError(t, kubeClient.Tracker().Add(secret))
+	}
+
 	// Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it
 	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState
 
@@ -517,6 +540,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		csrfCookie           string
 		customUsernameHeader *string // nil means do not send header, empty means send header with empty value
 		customPasswordHeader *string // nil means do not send header, empty means send header with empty value
+		kubeResources        func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset)
 
 		wantStatus                             int
 		wantContentType                        string
@@ -540,6 +564,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		wantDownstreamPKCEChallenge       string
 		wantDownstreamPKCEChallengeMethod string
 		wantDownstreamNonce               string
+		wantDownstreamClientID            string // defaults to wanting "pinniped-cli" when not set
 		wantUnnecessaryStoredRecords      int
 		wantPasswordGrantCall             *expectedPasswordGrant
 		wantDownstreamCustomSessionData   *psession.CustomSessionData
@@ -562,6 +587,24 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:                                   "OIDC upstream browser flow happy path using GET without a CSRF cookie using a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name:                                   "LDAP upstream browser flow happy path using GET without a CSRF cookie",
 			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
@@ -579,6 +622,24 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:                                   "LDAP upstream browser flow happy path using GET without a CSRF cookie using a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", ldapUpstreamName, "ldap")}),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name:                                   "Active Directory upstream browser flow happy path using GET without a CSRF cookie",
 			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
@@ -596,6 +657,24 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:                                   "Active Directory upstream browser flow happy path using GET without a CSRF cookie using a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodGet,
+			path:                                   modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        htmlContentType,
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", activeDirectoryUpstreamName, "activedirectory")}),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name:                              "OIDC upstream password grant happy path using GET",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -730,6 +809,26 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", oidcUpstreamName, "oidc"), nil),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
+		{
+			name:                                   "OIDC upstream browser flow happy path using POST with a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodPost,
+			path:                                   "/some/path",
+			contentType:                            formContentType,
+			body:                                   encodeQuery(modifiedHappyGetRequestQueryMap(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep})),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        "",
+			wantBodyString:                         "",
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
 		{
 			name:                                   "LDAP upstream browser flow happy path using POST",
 			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
@@ -749,6 +848,26 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", ldapUpstreamName, "ldap")}),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
+		{
+			name:                                   "LDAP upstream browser flow happy path using POST with a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodPost,
+			path:                                   "/some/path",
+			contentType:                            formContentType,
+			body:                                   encodeQuery(modifiedHappyGetRequestQueryMap(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep})),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        "",
+			wantBodyString:                         "",
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", ldapUpstreamName, "ldap")}),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
 		{
 			name:                                   "Active Directory upstream browser flow happy path using POST",
 			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
@@ -768,6 +887,26 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, "", activeDirectoryUpstreamName, "activedirectory")}),
 			wantUpstreamStateParamInLocationHeader: true,
 		},
+		{
+			name:                                   "Active Directory upstream browser flow happy path using POST with a dynamic client",
+			idps:                                   oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			kubeResources:                          addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:                           happyCSRFGenerator,
+			generatePKCE:                           happyPKCEGenerator,
+			generateNonce:                          happyNonceGenerator,
+			stateEncoder:                           happyStateEncoder,
+			cookieEncoder:                          happyCookieEncoder,
+			method:                                 http.MethodPost,
+			path:                                   "/some/path",
+			contentType:                            formContentType,
+			body:                                   encodeQuery(modifiedHappyGetRequestQueryMap(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep})),
+			wantStatus:                             http.StatusSeeOther,
+			wantContentType:                        "",
+			wantBodyString:                         "",
+			wantCSRFValueInCookieHeader:            happyCSRF,
+			wantLocationHeader:                     urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}, "", activeDirectoryUpstreamName, "activedirectory")}),
+			wantUpstreamStateParamInLocationHeader: true,
+		},
 		{
 			name:                              "OIDC upstream password grant happy path using POST",
 			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -945,6 +1084,32 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:          "OIDC upstream browser flow happy path using dynamic client when downstream redirect uri matches what is configured for client except for the port number",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:  happyCSRFGenerator,
+			generatePKCE:  happyPKCEGenerator,
+			generateNonce: happyNonceGenerator,
+			stateEncoder:  happyStateEncoder,
+			cookieEncoder: happyCookieEncoder,
+			method:        http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
+				"client_id":    dynamicClientID,
+				"scope":        testutil.AllDynamicClientScopesSpaceSep,
+			}),
+			wantStatus:                  http.StatusSeeOther,
+			wantContentType:             htmlContentType,
+			wantCSRFValueInCookieHeader: happyCSRF,
+			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{
+				"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
+				"client_id":    dynamicClientID,
+				"scope":        testutil.AllDynamicClientScopesSpaceSep,
+			}, "", oidcUpstreamName, "oidc"), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name:   "OIDC upstream password grant happy path when downstream redirect uri matches what is configured for client except for the port number",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1342,6 +1507,45 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithPasswordGrantDisallowedHintErrorQuery),
 			wantBodyString:       "",
 		},
+		{
+			name:                 "dynamic clients are not allowed to use OIDC password grant because we don't want them to handle user credentials",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:        addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithUsernamePasswordHeadersDisallowedHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "dynamic clients are not allowed to use LDAP CLI-flow authentication because we don't want them to handle user credentials",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources:        addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
+			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithUsernamePasswordHeadersDisallowedHintErrorQuery),
+			wantBodyString:       "",
+		},
+		{
+			name:                 "dynamic clients are not allowed to use Active Directory CLI-flow authentication because we don't want them to handle user credentials",
+			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			kubeResources:        addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:               http.MethodGet,
+			path:                 modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			customUsernameHeader: pointer.StringPtr(happyLDAPUsername),
+			customPasswordHeader: pointer.StringPtr(happyLDAPPassword),
+			wantStatus:           http.StatusFound,
+			wantContentType:      jsonContentType,
+			wantLocationHeader:   urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithUsernamePasswordHeadersDisallowedHintErrorQuery),
+			wantBodyString:       "",
+		},
 		{
 			name:          "downstream redirect uri does not match what is configured for client when using OIDC upstream browser flow",
 			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
@@ -1358,6 +1562,25 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType: jsonContentType,
 			wantBodyJSON:    fositeInvalidRedirectURIErrorBody,
 		},
+		{
+			name:          "downstream redirect uri does not match what is configured for client when using OIDC upstream browser flow with a dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:  happyCSRFGenerator,
+			generatePKCE:  happyPKCEGenerator,
+			generateNonce: happyNonceGenerator,
+			stateEncoder:  happyStateEncoder,
+			cookieEncoder: happyCookieEncoder,
+			method:        http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-dynamic-client",
+				"client_id":    dynamicClientID,
+				"scope":        testutil.AllDynamicClientScopesSpaceSep,
+			}),
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: jsonContentType,
+			wantBodyJSON:    fositeInvalidRedirectURIErrorBody,
+		},
 		{
 			name:   "downstream redirect uri does not match what is configured for client when using OIDC upstream password grant",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1455,6 +1678,26 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:          "response type is unsupported when using OIDC upstream browser flow with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:  happyCSRFGenerator,
+			generatePKCE:  happyPKCEGenerator,
+			generateNonce: happyNonceGenerator,
+			stateEncoder:  happyStateEncoder,
+			cookieEncoder: happyCookieEncoder,
+			method:        http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"response_type": "unsupported",
+				"client_id":     dynamicClientID,
+				"scope":         testutil.AllDynamicClientScopesSpaceSep,
+			}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "response type is unsupported when using OIDC upstream password grant",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1489,6 +1732,21 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:          "response type is unsupported when using LDAP browser upstream with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:        http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"response_type": "unsupported",
+				"client_id":     dynamicClientID,
+				"scope":         testutil.AllDynamicClientScopesSpaceSep,
+			}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "response type is unsupported when using active directory cli upstream",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
@@ -1511,6 +1769,21 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:          "response type is unsupported when using active directory browser upstream with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:        http.MethodGet,
+			path: modifiedHappyGetRequestPath(map[string]string{
+				"response_type": "unsupported",
+				"client_id":     dynamicClientID,
+				"scope":         testutil.AllDynamicClientScopesSpaceSep,
+			}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:               "downstream scopes do not match what is configured for client using OIDC upstream browser flow",
 			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
@@ -1526,6 +1799,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "downstream scopes do not match what is configured for client using OIDC upstream browser flow with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": "openid tuna"}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "downstream scopes do not match what is configured for client using OIDC upstream password grant",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1552,6 +1841,21 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantContentType: htmlContentType,
 			wantBodyRegex:   `<input type="hidden" name="encoded_params" value="error=invalid_scope&amp;error_description=The&#43;requested&#43;scope&#43;is&#43;invalid`,
 		},
+		{
+			name:            "response_mode form_post is not allowed for dynamic clients",
+			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:   addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:    happyCSRFGenerator,
+			generatePKCE:    happyPKCEGenerator,
+			generateNonce:   happyNonceGenerator,
+			stateEncoder:    happyStateEncoder,
+			cookieEncoder:   happyCookieEncoder,
+			method:          http.MethodGet,
+			path:            modifiedHappyGetRequestPath(map[string]string{"response_mode": "form_post", "client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep}),
+			wantStatus:      http.StatusOK, // this is weird, but fosite uses a form_post response to tell the client that it is not allowed to use form_post responses
+			wantContentType: htmlContentType,
+			wantBodyRegex:   `<input type="hidden" name="encoded_params" value="error=unsupported_response_mode&amp;error_description=The&#43;authorization&#43;server&#43;does&#43;not&#43;support&#43;obtaining&#43;a&#43;response&#43;using&#43;this&#43;response&#43;mode.`,
+		},
 		{
 			name:                 "downstream scopes do not match what is configured for client using LDAP upstream",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
@@ -1591,6 +1895,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "missing response type in request using OIDC upstream browser flow with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "response_type": ""}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "missing response type in request using OIDC upstream password grant",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1625,6 +1945,17 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "missing response type in request using LDAP browser upstream with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "response_type": ""}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "missing response type in request using Active Directory cli upstream",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
@@ -1647,6 +1978,17 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "missing response type in request using Active Directory browser upstream with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "response_type": ""}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:            "missing client id in request using OIDC upstream browser flow",
 			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
@@ -1696,6 +2038,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "missing PKCE code_challenge in request using OIDC upstream browser flow with dynamic client", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "code_challenge": ""}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                         "missing PKCE code_challenge in request using OIDC upstream password grant", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
 			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1738,6 +2096,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "invalid value for PKCE code_challenge_method in request using OIDC upstream browser flow with dynamic client", // https://tools.ietf.org/html/rfc7636#section-4.3
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "code_challenge_method": "this-is-not-a-valid-pkce-alg"}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                         "invalid value for PKCE code_challenge_method in request using OIDC upstream password grant", // https://tools.ietf.org/html/rfc7636#section-4.3
 			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1780,6 +2154,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "when PKCE code_challenge_method in request is `plain` using OIDC upstream browser flow with dynamic client", // https://tools.ietf.org/html/rfc7636#section-4.3
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "code_challenge_method": "plain"}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                         "when PKCE code_challenge_method in request is `plain` using OIDC upstream password grant", // https://tools.ietf.org/html/rfc7636#section-4.3
 			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1822,6 +2212,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "missing PKCE code_challenge_method in request using OIDC upstream browser flow with dynamic client", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "code_challenge_method": ""}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                         "missing PKCE code_challenge_method in request using OIDC upstream password grant", // See https://tools.ietf.org/html/rfc7636#section-4.4.1
 			idps:                         oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -1866,6 +2272,24 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			// This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running
+			// through that part of the fosite library when using an OIDC upstream browser flow with a dynamic client.
+			name:               "prompt param is not allowed to have none and another legal value at the same time using OIDC upstream browser flow with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "prompt": "none login"}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			// This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running
 			// through that part of the fosite library when using an OIDC upstream password grant.
@@ -1917,6 +2341,27 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantUpstreamStateParamInLocationHeader: true,
 			wantBodyStringWithLocationInHref:       true,
 		},
+		{
+			name:          "happy path: downstream OIDC validations are skipped when the openid scope was not requested using OIDC upstream browser flow with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:  happyCSRFGenerator,
+			generatePKCE:  happyPKCEGenerator,
+			generateNonce: happyNonceGenerator,
+			stateEncoder:  happyStateEncoder,
+			cookieEncoder: happyCookieEncoder,
+			method:        http.MethodGet,
+			// The following prompt value is illegal when openid is requested, but note that openid is not requested.
+			path:                        modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": "groups", "prompt": "none login"}),
+			wantStatus:                  http.StatusSeeOther,
+			wantContentType:             htmlContentType,
+			wantCSRFValueInCookieHeader: happyCSRF,
+			wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(
+				map[string]string{"client_id": dynamicClientID, "scope": "groups", "prompt": "none login"}, "", oidcUpstreamName, "oidc",
+			), nil),
+			wantUpstreamStateParamInLocationHeader: true,
+			wantBodyStringWithLocationInHref:       true,
+		},
 		{
 			name:   "happy path: downstream OIDC validations are skipped when the openid scope was not requested using OIDC upstream password grant",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -2396,6 +2841,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
 			wantBodyString:     "",
 		},
+		{
+			name:               "downstream state does not have enough entropy using OIDC upstream browser flow with dynamic client",
+			idps:               oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()),
+			kubeResources:      addFullyCapableDynamicClientAndSecretToKubeResources,
+			generateCSRF:       happyCSRFGenerator,
+			generatePKCE:       happyPKCEGenerator,
+			generateNonce:      happyNonceGenerator,
+			stateEncoder:       happyStateEncoder,
+			cookieEncoder:      happyCookieEncoder,
+			method:             http.MethodGet,
+			path:               modifiedHappyGetRequestPath(map[string]string{"client_id": dynamicClientID, "scope": testutil.AllDynamicClientScopesSpaceSep, "state": "short"}),
+			wantStatus:         http.StatusSeeOther,
+			wantContentType:    jsonContentType,
+			wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery),
+			wantBodyString:     "",
+		},
 		{
 			name:                 "downstream state does not have enough entropy using OIDC upstream password grant",
 			idps:                 oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()),
@@ -2573,7 +3034,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		},
 	}
 
-	runOneTestCase := func(t *testing.T, test testCase, subject http.Handler, kubeOauthStore *oidc.KubeStorage, kubeClient *fake.Clientset, secretsClient v1.SecretInterface) {
+	runOneTestCase := func(t *testing.T, test testCase, subject http.Handler, kubeOauthStore *oidc.KubeStorage, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset, secretsClient v1.SecretInterface) {
+		if test.kubeResources != nil {
+			test.kubeResources(t, supervisorClient, kubeClient)
+		}
+
 		reqContext := context.WithValue(context.Background(), struct{ name string }{name: "test"}, "request-context")
 		req := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body)).WithContext(reqContext)
 		req.Header.Set("Content-Type", test.contentType)
@@ -2622,8 +3087,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			// OIDC validations are checked in fosite after the OAuth authcode (and sometimes the OIDC session)
 			// is stored, so it is possible with an LDAP upstream to store objects and then return an error to
 			// the client anyway (which makes the stored objects useless, but oh well).
-			require.Len(t, kubeClient.Actions(), test.wantUnnecessaryStoredRecords)
+			require.Len(t, oidctestutil.FilterClientSecretCreateActions(kubeClient.Actions()), test.wantUnnecessaryStoredRecords)
 		case test.wantRedirectLocationRegexp != "":
+			if test.wantDownstreamClientID == "" {
+				test.wantDownstreamClientID = pinnipedCLIClientID // default assertion value when not provided by test case
+			}
 			require.Len(t, rsp.Header().Values("Location"), 1)
 			oidctestutil.RequireAuthCodeRegexpMatch(
 				t,
@@ -2640,7 +3108,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				test.wantDownstreamPKCEChallenge,
 				test.wantDownstreamPKCEChallengeMethod,
 				test.wantDownstreamNonce,
-				downstreamClientID,
+				test.wantDownstreamClientID,
 				test.wantDownstreamRedirectURI,
 				test.wantDownstreamCustomSessionData,
 			)
@@ -2688,8 +3156,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
 			kubeClient := fake.NewSimpleClientset()
+			supervisorClient := supervisorfake.NewSimpleClientset()
 			secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
-			oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient)
+			oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace")
+			oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient)
+			oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient)
 			subject := NewHandler(
 				downstreamIssuer,
 				test.idps.Build(),
@@ -2697,7 +3168,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 				test.generateCSRF, test.generatePKCE, test.generateNonce,
 				test.stateEncoder, test.cookieEncoder,
 			)
-			runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
+			runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient)
 		})
 	}
 
@@ -2707,8 +3178,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		require.Equal(t, "OIDC upstream browser flow happy path using GET without a CSRF cookie", test.name)
 
 		kubeClient := fake.NewSimpleClientset()
+		supervisorClient := supervisorfake.NewSimpleClientset()
 		secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
-		oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient)
+		oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace")
+		oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient)
+		oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient)
 		idpLister := test.idps.Build()
 		subject := NewHandler(
 			downstreamIssuer,
@@ -2718,7 +3192,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			test.stateEncoder, test.cookieEncoder,
 		)
 
-		runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
+		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient)
 
 		// Call the idpLister's setter to change the upstream IDP settings.
 		newProviderSettings := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
@@ -2756,7 +3230,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		// modified expectations. This should ensure that the implementation is using the in-memory cache
 		// of upstream IDP settings appropriately in terms of always getting the values from the cache
 		// on every request.
-		runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient)
+		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient)
 	})
 }
 

internal/oidc/callback/callback_handler.go

@@ -48,11 +48,14 @@ func NewHandler(
 		reconstitutedAuthRequest := &http.Request{Form: downstreamAuthParams}
 		authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), reconstitutedAuthRequest)
 		if err != nil {
-			plog.Error("error using state downstream auth params", err)
+			plog.Error("error using state downstream auth params", err,
+				"fositeErr", oidc.FositeErrorForLog(err))
 			return httperr.New(http.StatusBadRequest, "error using state downstream auth params")
 		}
 
-		// Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested.
+		// Automatically grant the openid, offline_access, pinniped:request-audience, and groups scopes, but only if they were requested.
+		// This is instead of asking the user to approve these scopes. Note that `NewAuthorizeRequest` would have returned
+		// an error if the client requested a scope that they are not allowed to request, so we don't need to worry about that here.
 		downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope})
 
 		token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens(
@@ -81,7 +84,8 @@ func NewHandler(
 
 		authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
 		if err != nil {
-			plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName())
+			plog.WarningErr("error while generating and saving authcode", err,
+				"upstreamName", upstreamIDPConfig.GetName(), "fositeErr", oidc.FositeErrorForLog(err))
 			return httperr.Wrap(http.StatusInternalServerError, "error while generating and saving authcode", err)
 		}
 

internal/oidc/callback/callback_handler_test.go

@@ -15,9 +15,11 @@ import (
 
 	"github.com/gorilla/securecookie"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
 
+	supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/psession"
@@ -52,7 +54,9 @@ const (
 
 	downstreamIssuer              = "https://my-downstream-issuer.com/path"
 	downstreamRedirectURI         = "http://127.0.0.1/callback"
-	downstreamClientID            = "pinniped-cli"
+	downstreamPinnipedClientID    = "pinniped-cli"
+	downstreamDynamicClientID     = "client.oauth.pinniped.dev-test-name"
+	downstreamDynamicClientUID    = "fake-client-uid"
 	downstreamNonce               = "some-nonce-value"
 	downstreamPKCEChallenge       = "some-challenge"
 	downstreamPKCEChallengeMethod = "S256"
@@ -68,7 +72,7 @@ var (
 	happyDownstreamRequestParamsQuery = url.Values{
 		"response_type":         []string{"code"},
 		"scope":                 []string{strings.Join(happyDownstreamScopesRequested, " ")},
-		"client_id":             []string{downstreamClientID},
+		"client_id":             []string{downstreamPinnipedClientID},
 		"state":                 []string{happyDownstreamState},
 		"nonce":                 []string{downstreamNonce},
 		"code_challenge":        []string{downstreamPKCEChallenge},
@@ -76,6 +80,11 @@ var (
 		"redirect_uri":          []string{downstreamRedirectURI},
 	}
 	happyDownstreamRequestParams = happyDownstreamRequestParamsQuery.Encode()
+
+	happyDownstreamRequestParamsForDynamicClient = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+		map[string]string{"client_id": downstreamDynamicClientID},
+	).Encode()
+
 	happyDownstreamCustomSessionData = &psession.CustomSessionData{
 		ProviderUID:  happyUpstreamIDPResourceUID,
 		ProviderName: happyUpstreamIDPName,
@@ -120,6 +129,7 @@ func TestCallbackEndpoint(t *testing.T) {
 	happyCookieCodec.SetSerializer(securecookie.JSONEncoder{})
 
 	happyState := happyUpstreamStateParam().Build(t, happyStateCodec)
+	happyStateForDynamicClient := happyUpstreamStateParamForDynamicClient().Build(t, happyStateCodec)
 
 	encodedIncomingCookieCSRFValue, err := happyCookieCodec.Encode("csrf", happyDownstreamCSRF)
 	require.NoError(t, err)
@@ -135,10 +145,18 @@ func TestCallbackEndpoint(t *testing.T) {
 	// Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it
 	happyDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState
 
+	addFullyCapableDynamicClientAndSecretToKubeResources := func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+		oidcClient, secret := testutil.FullyCapableOIDCClientAndStorageSecret(t,
+			"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost})
+		require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+		require.NoError(t, kubeClient.Tracker().Add(secret))
+	}
+
 	tests := []struct {
 		name string
 
 		idps          *oidctestutil.UpstreamIDPListerBuilder
+		kubeResources func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset)
 		method        string
 		path          string
 		csrfCookie    string
@@ -154,6 +172,7 @@ func TestCallbackEndpoint(t *testing.T) {
 		wantDownstreamIDTokenGroups       []string
 		wantDownstreamRequestedScopes     []string
 		wantDownstreamNonce               string
+		wantDownstreamClientID            string
 		wantDownstreamPKCEChallenge       string
 		wantDownstreamPKCEChallengeMethod string
 		wantDownstreamCustomSessionData   *psession.CustomSessionData
@@ -182,6 +201,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -205,6 +225,32 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name:                              "GET with good state and cookie and successful upstream token exchange returns 303 to downstream client callback with its state and code when using dynamic client",
+			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources:                     addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:                            http.MethodGet,
+			path:                              newRequestPath().WithState(happyStateForDynamicClient).String(),
+			csrfCookie:                        happyCSRFCookie,
+			wantStatus:                        http.StatusSeeOther,
+			wantRedirectLocationRegexp:        happyDownstreamRedirectLocationRegexp,
+			wantBody:                          "",
+			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
+			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamDynamicClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -228,6 +274,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamAccessTokenCustomSessionData,
@@ -260,6 +307,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     []string{"openid"},
 			wantDownstreamGrantedScopes:       []string{"openid"},
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -283,6 +331,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData: &psession.CustomSessionData{
@@ -318,6 +367,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -343,6 +393,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -370,6 +421,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -398,6 +450,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -528,6 +581,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -553,6 +607,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -578,6 +633,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -711,6 +767,42 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantContentType: htmlContentType,
 			wantBody:        "Bad Request: error using state downstream auth params\n",
 		},
+		{
+			name:   "state's downstream auth params have invalid client_id",
+			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": "bogus"}).Encode()).
+					Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error using state downstream auth params\n",
+		},
+		{
+			name:          "dynamic clients do not allow response_mode=form_post",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:        http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithAuthorizeRequestParams(
+					shallowCopyAndModifyQuery(
+						happyDownstreamRequestParamsQuery,
+						map[string]string{
+							"client_id":     downstreamDynamicClientID,
+							"response_mode": "form_post",
+							"scope":         "openid",
+						},
+					).Encode(),
+				).Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error using state downstream auth params\n",
+		},
 		{
 			name:   "state's downstream auth params does not contain openid scope",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
@@ -730,6 +822,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamGrantedScopes:       []string{"groups"},
 			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -756,6 +849,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamRequestedScopes:     []string{"profile", "email"},
 			wantDownstreamGrantedScopes:       []string{},
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -783,6 +877,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "groups"},
 			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -881,6 +976,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamIDTokenGroups:       []string{},
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
@@ -1070,13 +1166,20 @@ func TestCallbackEndpoint(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			client := fake.NewSimpleClientset()
+			kubeClient := fake.NewSimpleClientset()
-			secrets := client.CoreV1().Secrets("some-namespace")
+			supervisorClient := supervisorfake.NewSimpleClientset()
+			secrets := kubeClient.CoreV1().Secrets("some-namespace")
+			oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace")
+
+			if test.kubeResources != nil {
+				test.kubeResources(t, supervisorClient, kubeClient)
+			}
 
 			// 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.
 			timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
-			oauthStore := oidc.NewKubeStorage(secrets, timeoutsConfiguration)
+			// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
+			oauthStore := oidc.NewKubeStorage(secrets, oidcClientsClient, timeoutsConfiguration, bcrypt.MinCost)
 			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()
@@ -1118,7 +1221,7 @@ func TestCallbackEndpoint(t *testing.T) {
 					t,
 					rsp.Body.String(),
 					test.wantBodyFormResponseRegexp,
-					client,
+					kubeClient,
 					secrets,
 					oauthStore,
 					test.wantDownstreamGrantedScopes,
@@ -1129,7 +1232,7 @@ func TestCallbackEndpoint(t *testing.T) {
 					test.wantDownstreamPKCEChallenge,
 					test.wantDownstreamPKCEChallengeMethod,
 					test.wantDownstreamNonce,
-					downstreamClientID,
+					test.wantDownstreamClientID,
 					downstreamRedirectURI,
 					test.wantDownstreamCustomSessionData,
 				)
@@ -1145,7 +1248,7 @@ func TestCallbackEndpoint(t *testing.T) {
 					t,
 					rsp.Header().Get("Location"),
 					test.wantRedirectLocationRegexp,
-					client,
+					kubeClient,
 					secrets,
 					oauthStore,
 					test.wantDownstreamGrantedScopes,
@@ -1156,7 +1259,7 @@ func TestCallbackEndpoint(t *testing.T) {
 					test.wantDownstreamPKCEChallenge,
 					test.wantDownstreamPKCEChallengeMethod,
 					test.wantDownstreamNonce,
-					downstreamClientID,
+					test.wantDownstreamClientID,
 					downstreamRedirectURI,
 					test.wantDownstreamCustomSessionData,
 				)
@@ -1227,6 +1330,12 @@ func happyUpstreamStateParam() *oidctestutil.UpstreamStateParamBuilder {
 	}
 }
 
+func happyUpstreamStateParamForDynamicClient() *oidctestutil.UpstreamStateParamBuilder {
+	p := happyUpstreamStateParam()
+	p.P = happyDownstreamRequestParamsForDynamicClient
+	return p
+}
+
 func happyUpstream() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder {
 	return oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
 		WithName(happyUpstreamIDPName).

internal/oidc/clientregistry/clientregistry.go

@@ -7,51 +7,124 @@ package clientregistry
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
+	"k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
+	supervisorclient "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
+	"go.pinniped.dev/internal/plog"
 )
 
-// Client represents a Pinniped OAuth/OIDC client.
+const (
+	// PinnipedCLIClientID is the client ID of the statically defined public OIDC client which is used by the CLI.
+	PinnipedCLIClientID = "pinniped-cli"
+
+	requiredOIDCClientPrefix = "client.oauth.pinniped.dev-"
+)
+
+// Client represents a Pinniped OAuth/OIDC client. It can be the static pinniped-cli client
+// or a dynamic client defined by an OIDCClient CR.
 type Client struct {
 	fosite.DefaultOpenIDConnectClient
 }
 
-func (c Client) GetResponseModes() []fosite.ResponseModeType {
+// Client implements the base, OIDC, and response_mode client interfaces of Fosite.
-	// For now, all Pinniped clients always support "" (unspecified), "query", and "form_post" response modes.
-	return []fosite.ResponseModeType{fosite.ResponseModeDefault, fosite.ResponseModeQuery, fosite.ResponseModeFormPost}
-}
-
-// It implements both the base, OIDC, and response_mode client interfaces of Fosite.
 var (
 	_ fosite.Client              = (*Client)(nil)
 	_ fosite.OpenIDConnectClient = (*Client)(nil)
 	_ fosite.ResponseModeClient  = (*Client)(nil)
 )
 
-// StaticClientManager is a fosite.ClientManager with statically-defined clients.
+func (c *Client) GetResponseModes() []fosite.ResponseModeType {
-type StaticClientManager struct{}
+	if c.ID == PinnipedCLIClientID {
+		// The pinniped-cli client supports "" (unspecified), "query", and "form_post" response modes.
+		return []fosite.ResponseModeType{fosite.ResponseModeDefault, fosite.ResponseModeQuery, fosite.ResponseModeFormPost}
+	}
+	// For now, all other clients support only "" (unspecified) and "query" response modes.
+	return []fosite.ResponseModeType{fosite.ResponseModeDefault, fosite.ResponseModeQuery}
+}
 
-var _ fosite.ClientManager = (*StaticClientManager)(nil)
+// ClientManager is a fosite.ClientManager with a statically-defined client and with dynamically-defined clients.
+type ClientManager struct {
+	oidcClientsClient supervisorclient.OIDCClientInterface
+	storage           *oidcclientsecretstorage.OIDCClientSecretStorage
+	minBcryptCost     int
+}
 
-// GetClient returns a static client specified by the given ID.
+var _ fosite.ClientManager = (*ClientManager)(nil)
+
+func NewClientManager(
+	oidcClientsClient supervisorclient.OIDCClientInterface,
+	storage *oidcclientsecretstorage.OIDCClientSecretStorage,
+	minBcryptCost int,
+) *ClientManager {
+	return &ClientManager{
+		oidcClientsClient: oidcClientsClient,
+		storage:           storage,
+		minBcryptCost:     minBcryptCost,
+	}
+}
+
+// GetClient returns the client specified by the given ID.
 //
 // It returns a fosite.ErrNotFound if an unknown client is specified.
-func (StaticClientManager) GetClient(_ context.Context, id string) (fosite.Client, error) {
+// Other errors returned are plain errors, because fosite will wrap them into a new ErrInvalidClient error and
-	switch id {
+// use the plain error's text as that error's debug message (see client_authentication.go in fosite).
-	case "pinniped-cli":
+func (m *ClientManager) GetClient(ctx context.Context, id string) (fosite.Client, error) {
+	if id == PinnipedCLIClientID {
+		// Return the static client. No lookups needed.
 		return PinnipedCLI(), nil
-	default:
+	}
+
+	if !strings.HasPrefix(id, requiredOIDCClientPrefix) {
+		// It shouldn't really be possible to find this OIDCClient because the OIDCClient CRD validates the name prefix
+		// upon create, but just in case, don't even try to lookup clients which lack the required name prefix.
 		return nil, fosite.ErrNotFound.WithDescription("no such client")
 	}
+
+	// Try to look up an OIDCClient with the given client ID (which will be the Name of the OIDCClient).
+	oidcClient, err := m.oidcClientsClient.Get(ctx, id, v1.GetOptions{})
+	if errors.IsNotFound(err) {
+		return nil, fosite.ErrNotFound.WithDescription("no such client")
+	}
+	if err != nil {
+		// Log the error so an admin can see why the lookup failed at the time of the request.
+		plog.Error("OIDC client lookup GetClient() failed to get OIDCClient", err, "clientID", id)
+		return nil, fmt.Errorf("failed to get client %q", id)
+	}
+
+	// Try to find the corresponding client secret storage Secret.
+	storageSecret, err := m.storage.GetStorageSecret(ctx, oidcClient.UID)
+	if err != nil {
+		// Log the error so an admin can see why the lookup failed at the time of the request.
+		plog.Error("OIDC client lookup GetClient() failed to get storage secret for OIDCClient", err, "clientID", id)
+		return nil, fmt.Errorf("failed to get storage secret for client %q", id)
+	}
+
+	// Check if the OIDCClient and its corresponding Secret are valid.
+	valid, conditions, clientSecrets := oidcclientvalidator.Validate(oidcClient, storageSecret, m.minBcryptCost)
+	if !valid {
+		// Log the conditions so an admin can see exactly what was invalid at the time of the request.
+		plog.Debug("OIDC client lookup GetClient() found an invalid client", "clientID", id, "conditions", conditions)
+		return nil, fmt.Errorf("client %q exists but is invalid or not ready", id)
+	}
+
+	// Everything is valid, so return the client. Note that it has at least one client secret to be considered valid.
+	return oidcClientCRToFositeClient(oidcClient, clientSecrets), nil
 }
 
 // ClientAssertionJWTValid returns an error if the JTI is
 // known or the DB check failed and nil if the JTI is not known.
 //
-// This functionality is not supported by the StaticClientManager.
+// This functionality is not supported by the ClientManager.
-func (StaticClientManager) ClientAssertionJWTValid(ctx context.Context, jti string) error {
+func (*ClientManager) ClientAssertionJWTValid(ctx context.Context, jti string) error {
 	return fmt.Errorf("not implemented")
 }
 
@@ -60,8 +133,8 @@ func (StaticClientManager) ClientAssertionJWTValid(ctx context.Context, jti stri
 // up any existing JTIs that have expired as those tokens can
 // not be replayed due to the expiry.
 //
-// This functionality is not supported by the StaticClientManager.
+// This functionality is not supported by the ClientManager.
-func (StaticClientManager) SetClientAssertionJWT(ctx context.Context, jti string, exp time.Time) error {
+func (*ClientManager) SetClientAssertionJWT(ctx context.Context, jti string, exp time.Time) error {
 	return fmt.Errorf("not implemented")
 }
 
@@ -70,7 +143,7 @@ func PinnipedCLI() *Client {
 	return &Client{
 		DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{
 			DefaultClient: &fosite.DefaultClient{
-				ID:           "pinniped-cli",
+				ID:           PinnipedCLIClientID,
 				Secret:       nil,
 				RedirectURIs: []string{"http://127.0.0.1/callback"},
 				GrantTypes: fosite.Arguments{
@@ -99,3 +172,62 @@ func PinnipedCLI() *Client {
 		},
 	}
 }
+
+func oidcClientCRToFositeClient(oidcClient *configv1alpha1.OIDCClient, clientSecrets []string) *Client {
+	return &Client{
+		DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{
+			DefaultClient: &fosite.DefaultClient{
+				ID: oidcClient.Name,
+				// We set RotatedSecrets, but we don't need to also set Secret because the client_authentication.go code
+				// will always call the hasher on the empty Secret first, and the bcrypt hasher will always fail very
+				// quickly (ErrHashTooShort error), and then client_authentication.go will move on to using the
+				// RotatedSecrets instead.
+				RotatedSecrets: stringSliceToByteSlices(clientSecrets),
+				RedirectURIs:   redirectURIsToStrings(oidcClient.Spec.AllowedRedirectURIs),
+				GrantTypes:     grantTypesToArguments(oidcClient.Spec.AllowedGrantTypes),
+				ResponseTypes:  []string{"code"},
+				Scopes:         scopesToArguments(oidcClient.Spec.AllowedScopes),
+				Audience:       nil,
+				Public:         false,
+			},
+			RequestURIs:                       nil,
+			JSONWebKeys:                       nil,
+			JSONWebKeysURI:                    "",
+			RequestObjectSigningAlgorithm:     "",
+			TokenEndpointAuthSigningAlgorithm: oidc.RS256,
+			TokenEndpointAuthMethod:           "client_secret_basic",
+		},
+	}
+}
+
+func scopesToArguments(scopes []configv1alpha1.Scope) fosite.Arguments {
+	a := make(fosite.Arguments, len(scopes))
+	for i, scope := range scopes {
+		a[i] = string(scope)
+	}
+	return a
+}
+
+func grantTypesToArguments(grantTypes []configv1alpha1.GrantType) fosite.Arguments {
+	a := make(fosite.Arguments, len(grantTypes))
+	for i, grantType := range grantTypes {
+		a[i] = string(grantType)
+	}
+	return a
+}
+
+func redirectURIsToStrings(uris []configv1alpha1.RedirectURI) []string {
+	s := make([]string, len(uris))
+	for i, uri := range uris {
+		s[i] = string(uri)
+	}
+	return s
+}
+
+func stringSliceToByteSlices(s []string) [][]byte {
+	b := make([][]byte, len(s))
+	for i, str := range s {
+		b[i] = []byte(str)
+	}
+	return b
+}

internal/oidc/clientregistry/clientregistry_test.go

@@ -6,45 +6,264 @@ package clientregistry
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
 	"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/client-go/kubernetes/fake"
+	coretesting "k8s.io/client-go/testing"
+
+	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
+	supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
+	"go.pinniped.dev/internal/testutil"
 )
 
-func TestStaticRegistry(t *testing.T) {
+func TestClientManager(t *testing.T) {
 	ctx := context.Background()
 
-	t.Run("unimplemented methods", func(t *testing.T) {
+	const (
-		registry := StaticClientManager{}
+		testName      = "client.oauth.pinniped.dev-test-name"
-		require.EqualError(t, registry.ClientAssertionJWTValid(ctx, "some-token-id"), "not implemented")
+		testNamespace = "test-namespace"
-		require.EqualError(t, registry.SetClientAssertionJWT(ctx, "some-token-id", time.Now()), "not implemented")
+		testUID       = "test-uid-123"
-	})
+	)
 
-	t.Run("not found", func(t *testing.T) {
+	tests := []struct {
-		registry := StaticClientManager{}
+		name                   string
-		got, err := registry.GetClient(ctx, "does-not-exist")
+		secrets                []*corev1.Secret
+		oidcClients            []*configv1alpha1.OIDCClient
+		addKubeReactions       func(client *fake.Clientset)
+		addSupervisorReactions func(client *supervisorfake.Clientset)
+		run                    func(t *testing.T, subject *ClientManager)
+	}{
+		{
+			name: "unimplemented methods",
+			run: func(t *testing.T, subject *ClientManager) {
+				require.EqualError(t, subject.ClientAssertionJWTValid(ctx, "some-token-id"), "not implemented")
+				require.EqualError(t, subject.SetClientAssertionJWT(ctx, "some-token-id", time.Now()), "not implemented")
+			},
+		},
+		{
+			name: "find pinniped-cli client when no dynamic clients exist",
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, "pinniped-cli")
+				require.NoError(t, err)
+				require.IsType(t, &Client{}, got)
+				requireEqualsPinnipedCLI(t, got.(*Client))
+			},
+		},
+		{
+			name: "find pinniped-cli client when some dynamic clients also exist",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}},
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, "pinniped-cli")
+				require.NoError(t, err)
+				require.IsType(t, &Client{}, got)
+				requireEqualsPinnipedCLI(t, got.(*Client))
+			},
+		},
+		{
+			name: "client not found",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}},
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, "does-not-exist")
 				require.Error(t, err)
 				require.Nil(t, got)
 				rfcErr := fosite.ErrorToRFC6749Error(err)
 				require.NotNil(t, rfcErr)
 				require.Equal(t, rfcErr.CodeField, 404)
 				require.Equal(t, rfcErr.GetDescription(), "no such client")
+			},
+		},
+		{
+			name: "find a dynamic client when its storage secret does not exist (client is invalid because is has no client secret)",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+					Spec: configv1alpha1.OIDCClientSpec{
+						AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+						AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+						AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:80", "https://foobar.com/callback"},
+					},
+				},
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, testName)
+				require.EqualError(t, err, fmt.Sprintf("client %q exists but is invalid or not ready", testName))
+				require.Nil(t, got)
+			},
+		},
+		{
+			name: "find a dynamic client which is invalid due to its spec",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+					Spec: configv1alpha1.OIDCClientSpec{
+						AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code"},
+						AllowedScopes:       []configv1alpha1.Scope{}, // at least "openid" is required here, so this makes the client invalid
+						AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:80"},
+					},
+				},
+			},
+			secrets: []*corev1.Secret{
+				testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword2AtSupervisorMinCost}),
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, testName)
+				require.EqualError(t, err, fmt.Sprintf("client %q exists but is invalid or not ready", testName))
+				require.Nil(t, got)
+			},
+		},
+		{
+			name: "find a dynamic client which somehow does not have the required prefix in its name, just in case, although should not be possible since prefix is a validation on the CRD",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "does-not-have-prefix", Generation: 1234, UID: testUID},
+					Spec: configv1alpha1.OIDCClientSpec{
+						AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+						AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+						AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:80", "https://foobar.com/callback"},
+					},
+				},
+			},
+			secrets: []*corev1.Secret{
+				testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword2AtSupervisorMinCost}),
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, "does-not-have-prefix")
+				require.Error(t, err)
+				require.Nil(t, got)
+				rfcErr := fosite.ErrorToRFC6749Error(err)
+				require.NotNil(t, rfcErr)
+				require.Equal(t, rfcErr.CodeField, 404)
+				require.Equal(t, rfcErr.GetDescription(), "no such client")
+			},
+		},
+		{
+			name: "when there is an unexpected error getting the OIDCClient",
+			addSupervisorReactions: func(client *supervisorfake.Clientset) {
+				client.PrependReactor("get", "oidcclients", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, fmt.Errorf("some get OIDCClients error")
 				})
-
+			},
-	t.Run("pinniped CLI", func(t *testing.T) {
+			run: func(t *testing.T, subject *ClientManager) {
-		registry := StaticClientManager{}
+				got, err := subject.GetClient(ctx, testName)
-		got, err := registry.GetClient(ctx, "pinniped-cli")
+				require.EqualError(t, err, fmt.Sprintf("failed to get client %q", testName))
+				require.Nil(t, got)
+			},
+		},
+		{
+			name: "when there is an unexpected error getting the storage secret for the OIDCClient",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}},
+			},
+			addKubeReactions: func(client *fake.Clientset) {
+				client.PrependReactor("get", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, fmt.Errorf("some get Secrets error")
+				})
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, testName)
+				require.EqualError(t, err, fmt.Sprintf("failed to get storage secret for client %q", testName))
+				require.Nil(t, got)
+			},
+		},
+		{
+			name: "find a valid dynamic client",
+			oidcClients: []*configv1alpha1.OIDCClient{
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+					Spec: configv1alpha1.OIDCClientSpec{
+						AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+						AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+						AllowedRedirectURIs: []configv1alpha1.RedirectURI{"http://localhost:80", "https://foobar.com/callback"},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-client", Generation: 1234, UID: testUID},
+				},
+			},
+			secrets: []*corev1.Secret{
+				testutil.OIDCClientSecretStorageSecretForUID(t, testNamespace, testUID, []string{testutil.HashedPassword1AtSupervisorMinCost, testutil.HashedPassword2AtSupervisorMinCost}),
+			},
+			run: func(t *testing.T, subject *ClientManager) {
+				got, err := subject.GetClient(ctx, testName)
 				require.NoError(t, err)
-		require.NotNil(t, got)
 				require.IsType(t, &Client{}, got)
+				c := got.(*Client)
+
+				require.Equal(t, testName, c.GetID())
+				require.Nil(t, c.GetHashedSecret())
+				require.Len(t, c.GetRotatedHashes(), 2)
+				require.Equal(t, testutil.HashedPassword1AtSupervisorMinCost, string(c.GetRotatedHashes()[0]))
+				require.Equal(t, testutil.HashedPassword2AtSupervisorMinCost, string(c.GetRotatedHashes()[1]))
+				require.Equal(t, []string{"http://localhost:80", "https://foobar.com/callback"}, c.GetRedirectURIs())
+				require.Equal(t, fosite.Arguments{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"}, c.GetGrantTypes())
+				require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes())
+				require.Equal(t, fosite.Arguments{"openid", "offline_access", "pinniped:request-audience", "username", "groups"}, c.GetScopes())
+				require.False(t, c.IsPublic())
+				require.Nil(t, c.GetAudience())
+				require.Nil(t, c.GetRequestURIs())
+				require.Nil(t, c.GetJSONWebKeys())
+				require.Equal(t, "", c.GetJSONWebKeysURI())
+				require.Equal(t, "", c.GetRequestObjectSigningAlgorithm())
+				require.Equal(t, "client_secret_basic", c.GetTokenEndpointAuthMethod())
+				require.Equal(t, "RS256", c.GetTokenEndpointAuthSigningAlgorithm())
+				require.Equal(t, []fosite.ResponseModeType{"", "query"}, c.GetResponseModes())
+			},
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			kubeClient := fake.NewSimpleClientset()
+			secrets := kubeClient.CoreV1().Secrets(testNamespace)
+			supervisorClient := supervisorfake.NewSimpleClientset()
+			oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients(testNamespace)
+			subject := NewClientManager(
+				oidcClientsClient,
+				oidcclientsecretstorage.New(secrets, time.Now),
+				oidcclientvalidator.DefaultMinBcryptCost,
+			)
+
+			for _, secret := range test.secrets {
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			}
+			for _, oidcClient := range test.oidcClients {
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+			}
+			if test.addKubeReactions != nil {
+				test.addKubeReactions(kubeClient)
+			}
+			if test.addSupervisorReactions != nil {
+				test.addSupervisorReactions(supervisorClient)
+			}
+
+			test.run(t, subject)
 		})
+	}
 }
 
 func TestPinnipedCLI(t *testing.T) {
-	c := PinnipedCLI()
+	requireEqualsPinnipedCLI(t, PinnipedCLI())
+}
+
+func requireEqualsPinnipedCLI(t *testing.T, c *Client) {
 	require.Equal(t, "pinniped-cli", c.GetID())
 	require.Nil(t, c.GetHashedSecret())
 	require.Equal(t, []string{"http://127.0.0.1/callback"}, c.GetRedirectURIs())

internal/oidc/kube_storage.go

@@ -13,6 +13,7 @@ import (
 	fositepkce "github.com/ory/fosite/handler/pkce"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
+	"go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
 	"go.pinniped.dev/internal/fositestorage/accesstoken"
 	"go.pinniped.dev/internal/fositestorage/authorizationcode"
 	"go.pinniped.dev/internal/fositestorage/openidconnect"
@@ -20,6 +21,7 @@ import (
 	"go.pinniped.dev/internal/fositestorage/refreshtoken"
 	"go.pinniped.dev/internal/fositestoragei"
 	"go.pinniped.dev/internal/oidc/clientregistry"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
 )
 
 type KubeStorage struct {
@@ -33,10 +35,15 @@ type KubeStorage struct {
 
 var _ fositestoragei.AllFositeStorage = &KubeStorage{}
 
-func NewKubeStorage(secrets corev1client.SecretInterface, timeoutsConfiguration TimeoutsConfiguration) *KubeStorage {
+func NewKubeStorage(
+	secrets corev1client.SecretInterface,
+	oidcClientsClient v1alpha1.OIDCClientInterface,
+	timeoutsConfiguration TimeoutsConfiguration,
+	minBcryptCost int,
+) *KubeStorage {
 	nowFunc := time.Now
 	return &KubeStorage{
-		clientManager:            &clientregistry.StaticClientManager{},
+		clientManager:            clientregistry.NewClientManager(oidcClientsClient, oidcclientsecretstorage.New(secrets, nowFunc), minBcryptCost),
 		authorizationCodeStorage: authorizationcode.New(secrets, nowFunc, timeoutsConfiguration.AuthorizationCodeSessionStorageLifetime),
 		pkceStorage:              pkce.New(secrets, nowFunc, timeoutsConfiguration.PKCESessionStorageLifetime),
 		oidcStorage:              openidconnect.New(secrets, nowFunc, timeoutsConfiguration.OIDCSessionStorageLifetime),

internal/oidc/login/post_login_handler.go

@@ -41,11 +41,14 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider
 		if err != nil {
 			// This shouldn't really happen because the authorization endpoint has already validated these params
 			// by calling NewAuthorizeRequest() itself.
-			plog.Error("error using state downstream auth params", err)
+			plog.Error("error using state downstream auth params", err,
+				"fositeErr", oidc.FositeErrorForLog(err))
 			return httperr.New(http.StatusBadRequest, "error using state downstream auth params")
 		}
 
 		// Automatically grant the openid, offline_access, pinniped:request-audience and groups scopes, but only if they were requested.
+		// This is instead of asking the user to approve these scopes. Note that `NewAuthorizeRequest` would have returned
+		// an error if the client requested a scope that they are not allowed to request, so we don't need to worry about that here.
 		downstreamsession.GrantScopesIfRequested(authorizeRequester, []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, oidc.RequestAudienceScope, oidc.DownstreamGroupsScope})
 
 		// Get the username and password form params from the POST body.

internal/oidc/login/post_login_handler_test.go

@@ -13,9 +13,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/kubernetes/fake"
 
+	supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
@@ -36,7 +38,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 
 		downstreamIssuer              = "https://my-downstream-issuer.com/path"
 		downstreamRedirectURI         = "http://127.0.0.1/callback"
-		downstreamClientID            = "pinniped-cli"
+		downstreamPinnipedCLIClientID = "pinniped-cli"
+		downstreamDynamicClientID     = "client.oauth.pinniped.dev-test-name"
+		downstreamDynamicClientUID    = "fake-client-uid"
 		happyDownstreamState          = "8b-state"
 		downstreamNonce               = "some-nonce-value"
 		downstreamPKCEChallenge       = "some-challenge"
@@ -88,7 +92,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 	happyDownstreamRequestParamsQuery := url.Values{
 		"response_type":         []string{"code"},
 		"scope":                 []string{strings.Join(happyDownstreamScopesRequested, " ")},
-		"client_id":             []string{downstreamClientID},
+		"client_id":             []string{downstreamPinnipedCLIClientID},
 		"state":                 []string{happyDownstreamState},
 		"nonce":                 []string{downstreamNonce},
 		"code_challenge":        []string{downstreamPKCEChallenge},
@@ -97,14 +101,10 @@ func TestPostLoginEndpoint(t *testing.T) {
 	}
 	happyDownstreamRequestParams := happyDownstreamRequestParamsQuery.Encode()
 
-	copyOfHappyDownstreamRequestParamsQuery := func() url.Values {
+	happyDownstreamRequestParamsQueryForDynamicClient := shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-		params := url.Values{}
+		map[string]string{"client_id": downstreamDynamicClientID},
-		for k, v := range happyDownstreamRequestParamsQuery {
+	)
-			params[k] = make([]string, len(v))
+	happyDownstreamRequestParamsForDynamicClient := happyDownstreamRequestParamsQueryForDynamicClient.Encode()
-			copy(params[k], v)
-		}
-		return params
-	}
 
 	happyLDAPDecodedState := &oidc.UpstreamStateParamData{
 		AuthParams:    happyDownstreamRequestParams,
@@ -122,15 +122,20 @@ func TestPostLoginEndpoint(t *testing.T) {
 		return &copyOfHappyLDAPDecodedState
 	}
 
-	happyActiveDirectoryDecodedState := &oidc.UpstreamStateParamData{
+	happyLDAPDecodedStateForDynamicClient := modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-		AuthParams:    happyDownstreamRequestParams,
+		data.AuthParams = happyDownstreamRequestParamsForDynamicClient
-		UpstreamName:  activeDirectoryUpstreamName,
+	})
-		UpstreamType:  activeDirectoryUpstreamType,
+
-		Nonce:         happyDownstreamNonce,
+	happyActiveDirectoryDecodedState := modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-		CSRFToken:     happyDownstreamCSRF,
+		data.UpstreamName = activeDirectoryUpstreamName
-		PKCECode:      happyDownstreamPKCE,
+		data.UpstreamType = activeDirectoryUpstreamType
-		FormatVersion: happyDownstreamStateVersion,
+	})
-	}
+
+	happyActiveDirectoryDecodedStateForDynamicClient := modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+		data.AuthParams = happyDownstreamRequestParamsForDynamicClient
+		data.UpstreamName = activeDirectoryUpstreamName
+		data.UpstreamType = activeDirectoryUpstreamType
+	})
 
 	happyLDAPUsername := "some-ldap-user"
 	happyLDAPUsernameFromAuthenticator := "some-mapped-ldap-username"
@@ -230,9 +235,17 @@ func TestPostLoginEndpoint(t *testing.T) {
 		return urlToReturn
 	}
 
+	addFullyCapableDynamicClientAndSecretToKubeResources := func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+		oidcClient, secret := testutil.FullyCapableOIDCClientAndStorageSecret(t,
+			"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost})
+		require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+		require.NoError(t, kubeClient.Tracker().Add(secret))
+	}
+
 	tests := []struct {
 		name          string
 		idps          *oidctestutil.UpstreamIDPListerBuilder
+		kubeResources func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset)
 		decodedState  *oidc.UpstreamStateParamData
 		formParams    url.Values
 		reqURIQuery   url.Values
@@ -259,6 +272,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 		wantDownstreamPKCEChallenge       string
 		wantDownstreamPKCEChallengeMethod string
 		wantDownstreamNonce               string
+		wantDownstreamClient              string
 		wantDownstreamCustomSessionData   *psession.CustomSessionData
 
 		// Authorization requests for either a successful OIDC upstream or for an error with any upstream
@@ -286,6 +300,31 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP login with dynamic client",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one
+				WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider),
+			kubeResources:                     addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState:                      happyLDAPDecodedStateForDynamicClient,
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -308,6 +347,31 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyActiveDirectoryUpstreamCustomSession,
+		},
+		{
+			name: "happy AD login with dynamic client",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().
+				WithLDAP(&erroringUpstreamLDAPIdentityProvider).
+				WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // should pick this one
+			kubeResources:                     addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState:                      happyActiveDirectoryDecodedStateForDynamicClient,
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        happyAuthcodeDownstreamRedirectLocationRegexp,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyActiveDirectoryUpstreamCustomSession,
@@ -316,9 +380,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "happy LDAP login when downstream response_mode=form_post returns 200 with HTML+JS form",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["response_mode"] = []string{"form_post"}
+					map[string]string{"response_mode": "form_post"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:      happyUsernamePasswordFormParams,
 			wantStatus:      http.StatusOK,
@@ -332,6 +396,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -340,9 +405,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "happy LDAP login when downstream redirect uri matches what is configured for client except for the port number",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["redirect_uri"] = []string{"http://127.0.0.1:4242/callback"}
+					map[string]string{"redirect_uri": "http://127.0.0.1:4242/callback"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                        happyUsernamePasswordFormParams,
 			wantStatus:                        http.StatusSeeOther,
@@ -356,6 +421,33 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         "http://127.0.0.1:4242/callback",
 			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name:          "happy LDAP login when downstream redirect uri matches what is configured for client except for the port number with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"redirect_uri": "http://127.0.0.1:4242/callback"},
+				).Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamRedirectURI:         "http://127.0.0.1:4242/callback",
+			wantDownstreamGrantedScopes:       happyDownstreamScopesGranted,
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -364,9 +456,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "happy LDAP login when there are additional allowed downstream requested scopes",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["scope"] = []string{"openid offline_access pinniped:request-audience"}
+					map[string]string{"scope": "openid offline_access pinniped:request-audience"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                        happyUsernamePasswordFormParams,
 			wantStatus:                        http.StatusSeeOther,
@@ -380,6 +472,33 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "pinniped:request-audience"},
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name:          "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid offline_access pinniped:request-audience"},
+				).Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+pinniped%3Arequest-audience&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "pinniped:request-audience"},
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "pinniped:request-audience"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -388,11 +507,13 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "happy LDAP when downstream OIDC validations are skipped because the openid scope was not requested",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["scope"] = []string{"email"}
+					map[string]string{
+						"scope": "email",
 						// The following prompt value is illegal when openid is requested, but note that openid is not requested.
-				query["prompt"] = []string{"none login"}
+						"prompt": "none login",
-				data.AuthParams = query.Encode()
+					},
+				).Encode()
 			}),
 			formParams:                        happyUsernamePasswordFormParams,
 			wantStatus:                        http.StatusSeeOther,
@@ -406,6 +527,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       []string{}, // no scopes granted
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -416,9 +538,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 				WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one
 				WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["scope"] = []string{"openid"}
+					map[string]string{"scope": "openid"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                        happyUsernamePasswordFormParams,
 			wantStatus:                        http.StatusSeeOther,
@@ -431,6 +553,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       []string{"openid"},
 			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
@@ -499,9 +622,21 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "downstream redirect uri does not match what is configured for client",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["redirect_uri"] = []string{"http://127.0.0.1/wrong_callback"}
+					map[string]string{"redirect_uri": "http://127.0.0.1/wrong_callback"},
-				data.AuthParams = query.Encode()
+				).Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name:          "downstream redirect uri does not match what is configured for client with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"redirect_uri": "http://127.0.0.1/wrong_callback"},
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -510,9 +645,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "downstream client does not exist",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["client_id"] = []string{"wrong_client_id"}
+					map[string]string{"client_id": "wrong_client_id"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -521,9 +656,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "downstream client is missing",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				delete(query, "client_id")
+					map[string]string{"client_id": ""},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -532,9 +667,21 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "response type is unsupported",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["response_type"] = []string{"unsupported"}
+					map[string]string{"response_type": "unsupported"},
-				data.AuthParams = query.Encode()
+				).Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name:          "response type form_post is unsupported for dynamic clients",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"response_type": "form_post"},
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -543,9 +690,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "response type is missing",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				delete(query, "response_type")
+					map[string]string{"response_type": ""},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -554,9 +701,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "PKCE code_challenge is missing",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				delete(query, "code_challenge")
+					map[string]string{"code_challenge": ""},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                   happyUsernamePasswordFormParams,
 			wantStatus:                   http.StatusSeeOther,
@@ -569,9 +716,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "PKCE code_challenge_method is invalid",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["code_challenge_method"] = []string{"this-is-not-a-valid-pkce-alg"}
+					map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                   happyUsernamePasswordFormParams,
 			wantStatus:                   http.StatusSeeOther,
@@ -584,9 +731,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "PKCE code_challenge_method is `plain`",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["code_challenge_method"] = []string{"plain"} // plain is not allowed
+					map[string]string{"code_challenge_method": "plain"}, // plain is not allowed
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                   happyUsernamePasswordFormParams,
 			wantStatus:                   http.StatusSeeOther,
@@ -599,9 +746,25 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "PKCE code_challenge_method is missing",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				delete(query, "code_challenge_method")
+					map[string]string{"code_challenge_method": ""},
-				data.AuthParams = query.Encode()
+				).Encode()
+			}),
+			formParams:                   happyUsernamePasswordFormParams,
+			wantStatus:                   http.StatusSeeOther,
+			wantContentType:              htmlContentType,
+			wantBodyString:               "",
+			wantRedirectLocationString:   urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
+			wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error
+		},
+		{
+			name:          "PKCE code_challenge_method is missing with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"code_challenge_method": ""},
+				).Encode()
 			}),
 			formParams:                   happyUsernamePasswordFormParams,
 			wantStatus:                   http.StatusSeeOther,
@@ -614,9 +777,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "prompt param is not allowed to have none and another legal value at the same time",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["prompt"] = []string{"none login"}
+					map[string]string{"prompt": "none login"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams:                   happyUsernamePasswordFormParams,
 			wantStatus:                   http.StatusSeeOther,
@@ -629,9 +792,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "downstream state does not have enough entropy",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["state"] = []string{"short"}
+					map[string]string{"state": "short"},
-				data.AuthParams = query.Encode()
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -640,9 +803,21 @@ func TestPostLoginEndpoint(t *testing.T) {
 			name: "downstream scopes do not match what is configured for client",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
-				query := copyOfHappyDownstreamRequestParamsQuery()
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
-				query["scope"] = []string{"openid offline_access pinniped:request-audience scope_not_allowed"}
+					map[string]string{"scope": "openid offline_access pinniped:request-audience scope_not_allowed"},
-				data.AuthParams = query.Encode()
+				).Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name:          "downstream scopes do not match what is configured for client with dynamic client",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid offline_access pinniped:request-audience scope_not_allowed"},
+				).Encode()
 			}),
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
@@ -670,12 +845,19 @@ func TestPostLoginEndpoint(t *testing.T) {
 			t.Parallel()
 
 			kubeClient := fake.NewSimpleClientset()
+			supervisorClient := supervisorfake.NewSimpleClientset()
 			secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+			oidcClientsClient := supervisorClient.ConfigV1alpha1().OIDCClients("some-namespace")
+
+			if tt.kubeResources != nil {
+				tt.kubeResources(t, supervisorClient, kubeClient)
+			}
 
 			// 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.
 			timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration()
-			kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration)
+			// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
+			kubeOauthStore := oidc.NewKubeStorage(secretsClient, oidcClientsClient, timeoutsConfiguration, bcrypt.MinCost)
 			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()
@@ -694,7 +876,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			err := subject(rsp, req, happyEncodedUpstreamState, tt.decodedState)
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)
-				require.Empty(t, kubeClient.Actions())
+				require.Empty(t, oidctestutil.FilterClientSecretCreateActions(kubeClient.Actions()))
 				return // the http response doesn't matter when the function returns an error, because the caller should handle the error
 			}
 			// Otherwise, expect no error.
@@ -725,7 +907,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 					tt.wantDownstreamPKCEChallenge,
 					tt.wantDownstreamPKCEChallengeMethod,
 					tt.wantDownstreamNonce,
-					downstreamClientID,
+					tt.wantDownstreamClient,
 					tt.wantDownstreamRedirectURI,
 					tt.wantDownstreamCustomSessionData,
 				)
@@ -735,12 +917,12 @@ func TestPostLoginEndpoint(t *testing.T) {
 				expectedLocation := downstreamIssuer + oidc.PinnipedLoginPath +
 					"?err=" + tt.wantRedirectToLoginPageError + "&state=" + happyEncodedUpstreamState
 				require.Equal(t, expectedLocation, actualLocation)
-				require.Len(t, kubeClient.Actions(), tt.wantUnnecessaryStoredRecords)
+				require.Len(t, oidctestutil.FilterClientSecretCreateActions(kubeClient.Actions()), tt.wantUnnecessaryStoredRecords)
 			case tt.wantRedirectLocationString != "":
 				// Expecting an error redirect to the client.
 				require.Equal(t, tt.wantBodyString, rsp.Body.String())
 				require.Equal(t, tt.wantRedirectLocationString, actualLocation)
-				require.Len(t, kubeClient.Actions(), tt.wantUnnecessaryStoredRecords)
+				require.Len(t, oidctestutil.FilterClientSecretCreateActions(kubeClient.Actions()), tt.wantUnnecessaryStoredRecords)
 			case tt.wantBodyFormResponseRegexp != "":
 				// Expecting the body of the response to be a html page with a form (for "response_mode=form_post").
 				_, hasLocationHeader := rsp.Header()["Location"]
@@ -760,7 +942,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 					tt.wantDownstreamPKCEChallenge,
 					tt.wantDownstreamPKCEChallengeMethod,
 					tt.wantDownstreamNonce,
-					downstreamClientID,
+					tt.wantDownstreamClient,
 					tt.wantDownstreamRedirectURI,
 					tt.wantDownstreamCustomSessionData,
 				)
@@ -771,3 +953,18 @@ func TestPostLoginEndpoint(t *testing.T) {
 		})
 	}
 }
+
+func shallowCopyAndModifyQuery(query url.Values, modifications map[string]string) url.Values {
+	copied := url.Values{}
+	for key, value := range query {
+		copied[key] = value
+	}
+	for key, value := range modifications {
+		if value == "" {
+			copied.Del(key)
+		} else {
+			copied[key] = []string{value}
+		}
+	}
+	return copied
+}

internal/oidc/nullstorage.go

@@ -5,22 +5,37 @@ package oidc
 
 import (
 	"context"
+	"time"
 
 	"github.com/ory/fosite"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
+	"go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
 	"go.pinniped.dev/internal/constable"
 	"go.pinniped.dev/internal/fositestoragei"
 	"go.pinniped.dev/internal/oidc/clientregistry"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
 )
 
 const errNullStorageNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.")
 
 type NullStorage struct {
-	clientregistry.StaticClientManager
+	// The authorization endpoint uses NullStorage to avoid saving any data, but it still needs to perform client lookups.
+	*clientregistry.ClientManager
 }
 
 var _ fositestoragei.AllFositeStorage = &NullStorage{}
 
+func NewNullStorage(
+	secrets corev1client.SecretInterface,
+	oidcClientsClient v1alpha1.OIDCClientInterface,
+	minBcryptCost int,
+) *NullStorage {
+	return &NullStorage{
+		ClientManager: clientregistry.NewClientManager(oidcClientsClient, oidcclientsecretstorage.New(secrets, time.Now), minBcryptCost),
+	}
+}
+
 func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error {
 	return errNullStorageNotImplemented
 }

internal/oidc/oidc.go

@@ -457,7 +457,7 @@ func PerformAuthcodeRedirect(
 ) {
 	authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
 	if err != nil {
-		plog.WarningErr("error while generating and saving authcode", err)
+		plog.WarningErr("error while generating and saving authcode", err, "fositeErr", FositeErrorForLog(err))
 		WriteAuthorizeError(w, oauthHelper, authorizeRequester, err, isBrowserless)
 		return
 	}

internal/oidc/oidcclientvalidator/oidcclientvalidator.go

@@ -0,0 +1,235 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package oidcclientvalidator
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/coreos/go-oidc/v3/oidc"
+	"golang.org/x/crypto/bcrypt"
+	v1 "k8s.io/api/core/v1"
+
+	"go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
+)
+
+const (
+	DefaultMinBcryptCost = 12
+
+	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"
+)
+
+// Validate 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 bool to indicate if the client is valid,
+// along with a slice of conditions containing more details, and the list of client secrets in the
+// case that the client was valid.
+func Validate(oidcClient *v1alpha1.OIDCClient, secret *v1.Secret, minBcryptCost int) (bool, []*v1alpha1.Condition, []string) {
+	conds := make([]*v1alpha1.Condition, 0, 3)
+
+	conds, clientSecrets := validateSecret(secret, conds, minBcryptCost)
+	conds = validateAllowedGrantTypes(oidcClient, conds)
+	conds = validateAllowedScopes(oidcClient, conds)
+
+	valid := true
+	for _, cond := range conds {
+		if cond.Status != v1alpha1.ConditionTrue {
+			valid = false
+			break
+		}
+	}
+	return valid, conds, clientSecrets
+}
+
+// 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 client secrets found in that case that it is valid.
+func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition, minBcryptCost int) ([]*v1alpha1.Condition, []string) {
+	emptyList := []string{}
+
+	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, emptyList
+	}
+
+	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, emptyList
+	}
+
+	// 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, emptyList
+	}
+
+	// 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 < minBcryptCost {
+			bcryptErrs = append(bcryptErrs, fmt.Sprintf(
+				"hashed client secret at index %d: bcrypt cost %d is below the required minimum of %d",
+				i, cost, minBcryptCost))
+		}
+	}
+	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: fmt.Sprintf("%d stored client secrets found, but some were invalid, so none will be used: %s",
+				storedClientSecretsCount, strings.Join(bcryptErrs, "; ")),
+		})
+		return conditions, emptyList
+	}
+
+	// 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, storedClientSecret.SecretHashes
+}
+
+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
+}

internal/oidc/provider/manager/manager.go

@@ -10,6 +10,7 @@ import (
 
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
+	"go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/auth"
 	"go.pinniped.dev/internal/oidc/callback"
@@ -19,6 +20,7 @@ import (
 	"go.pinniped.dev/internal/oidc/idpdiscovery"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/oidc/login"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/oidc/token"
 	"go.pinniped.dev/internal/plog"
@@ -39,6 +41,7 @@ type Manager struct {
 	upstreamIDPs        oidc.UpstreamIdentityProvidersLister // in-memory cache of upstream IDPs
 	secretCache         *secret.Cache                        // in-memory cache of cryptographic material
 	secretsClient       corev1client.SecretInterface
+	oidcClientsClient   v1alpha1.OIDCClientInterface
 }
 
 // NewManager returns an empty Manager.
@@ -51,6 +54,7 @@ func NewManager(
 	upstreamIDPs oidc.UpstreamIdentityProvidersLister,
 	secretCache *secret.Cache,
 	secretsClient corev1client.SecretInterface,
+	oidcClientsClient v1alpha1.OIDCClientInterface,
 ) *Manager {
 	return &Manager{
 		providerHandlers:    make(map[string]http.Handler),
@@ -59,6 +63,7 @@ func NewManager(
 		upstreamIDPs:        upstreamIDPs,
 		secretCache:         secretCache,
 		secretsClient:       secretsClient,
+		oidcClientsClient:   oidcClientsClient,
 	}
 }
 
@@ -93,10 +98,22 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs
 
 		// 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, timeoutsConfiguration)
+		oauthHelperWithNullStorage := oidc.FositeOauth2Helper(
+			oidc.NewNullStorage(m.secretsClient, m.oidcClientsClient, oidcclientvalidator.DefaultMinBcryptCost),
+			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, timeoutsConfiguration), issuer, tokenHMACKeyGetter, m.dynamicJWKSProvider, timeoutsConfiguration)
+		oauthHelperWithKubeStorage := oidc.FositeOauth2Helper(
+			oidc.NewKubeStorage(m.secretsClient, m.oidcClientsClient, timeoutsConfiguration, oidcclientvalidator.DefaultMinBcryptCost),
+			issuer,
+			tokenHMACKeyGetter,
+			m.dynamicJWKSProvider,
+			timeoutsConfiguration,
+		)
 
 		var upstreamStateEncoder = dynamiccodec.New(
 			timeoutsConfiguration.UpstreamStateParamLifespan,

internal/oidc/provider/manager/manager_test.go

@@ -15,18 +15,18 @@ import (
 	"strings"
 	"testing"
 
-	"go.pinniped.dev/internal/secret"
-
 	"github.com/sclevine/spec"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/square/go-jose.v2"
 	"k8s.io/client-go/kubernetes/fake"
 
+	supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/discovery"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/oidc/provider"
+	"go.pinniped.dev/internal/secret"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/oidctestutil"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
@@ -271,6 +271,7 @@ func TestManager(t *testing.T) {
 
 			kubeClient = fake.NewSimpleClientset()
 			secretsClient := kubeClient.CoreV1().Secrets("some-namespace")
+			oidcClientsClient := supervisorfake.NewSimpleClientset().ConfigV1alpha1().OIDCClients("some-namespace")
 
 			cache := secret.Cache{}
 			cache.SetCSRFCookieEncoderHashKey([]byte("fake-csrf-hash-secret"))
@@ -283,7 +284,7 @@ func TestManager(t *testing.T) {
 			cache.SetStateEncoderHashKey(issuer2, []byte("some-state-encoder-hash-key-2"))
 			cache.SetStateEncoderBlockKey(issuer2, []byte("16-bytes-STATE02"))
 
-			subject = NewManager(nextHandler, dynamicJWKSProvider, idpLister, &cache, secretsClient)
+			subject = NewManager(nextHandler, dynamicJWKSProvider, idpLister, &cache, secretsClient, oidcClientsClient)
 		})
 
 		when("given no providers via SetProviders()", func() {

internal/oidc/token_exchange.go

@@ -14,9 +14,12 @@ import (
 	"github.com/ory/fosite/handler/oauth2"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/pkg/errors"
+
+	"go.pinniped.dev/internal/oidc/clientregistry"
 )
 
 const (
+	tokenExchangeGrantType     = "urn:ietf:params:oauth:grant-type:token-exchange" //nolint: gosec
 	tokenTypeAccessToken       = "urn:ietf:params:oauth:token-type:access_token"   //nolint: gosec
 	tokenTypeJWT               = "urn:ietf:params:oauth:token-type:jwt"            //nolint: gosec
 	pinnipedTokenExchangeScope = "pinniped:request-audience"                       //nolint: gosec
@@ -68,6 +71,18 @@ func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context
 		return errors.WithStack(err)
 	}
 
+	// Check that the currently authenticated client and the client which was originally used to get the access token are the same.
+	if originalRequester.GetClient().GetID() != requester.GetClient().GetID() {
+		// This error message is copied from the similar check in fosite's flow_authorize_code_token.go.
+		return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The OAuth 2.0 Client ID from this request does not match the one from the authorize request."))
+	}
+
+	// Check that the client is allowed to perform this grant type.
+	if !requester.GetClient().GetGrantTypes().Has(tokenExchangeGrantType) {
+		// This error message is trying to be similar to the analogous one in fosite's flow_authorize_code_token.go.
+		return errors.WithStack(fosite.ErrUnauthorizedClient.WithHintf(`The OAuth 2.0 Client is not allowed to use token exchange grant "%s".`, tokenExchangeGrantType))
+	}
+
 	// Require that the incoming access token has the pinniped:request-audience and OpenID scopes.
 	if !originalRequester.GetGrantedScopes().Has(pinnipedTokenExchangeScope) {
 		return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", pinnipedTokenExchangeScope))
@@ -142,8 +157,8 @@ func (t *TokenExchangeHandler) validateParams(params url.Values) (*stsParams, er
 	if strings.Contains(result.requestedAudience, ".pinniped.dev") {
 		return nil, fosite.ErrInvalidRequest.WithHintf("requested audience cannot contain '.pinniped.dev'")
 	}
-	if result.requestedAudience == "pinniped-cli" {
+	if result.requestedAudience == clientregistry.PinnipedCLIClientID {
-		return nil, fosite.ErrInvalidRequest.WithHintf("requested audience cannot equal 'pinniped-cli'")
+		return nil, fosite.ErrInvalidRequest.WithHintf("requested audience cannot equal '%s'", clientregistry.PinnipedCLIClientID)
 	}
 
 	return &result, nil
@@ -166,5 +181,5 @@ func (t *TokenExchangeHandler) CanSkipClientAuth(_ fosite.AccessRequester) bool
 }
 
 func (t *TokenExchangeHandler) CanHandleTokenEndpointRequest(requester fosite.AccessRequester) bool {
-	return requester.GetGrantTypes().ExactOne("urn:ietf:params:oauth:grant-type:token-exchange")
+	return requester.GetGrantTypes().ExactOne(tokenExchangeGrantType)
 }

internal/oidcclientsecretstorage/oidcclientsecretstorage.go

@@ -4,11 +4,14 @@
 package oidcclientsecretstorage
 
 import (
+	"context"
 	"encoding/base64"
 	"fmt"
 	"time"
 
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
@@ -26,6 +29,7 @@ const (
 
 type OIDCClientSecretStorage struct {
 	storage crud.Storage
+	secrets corev1client.SecretInterface
 }
 
 // StoredClientSecret defines the format of the content of a client's secrets when stored in a Secret
@@ -39,12 +43,27 @@ type StoredClientSecret struct {
 }
 
 func New(secrets corev1client.SecretInterface, clock func() time.Time) *OIDCClientSecretStorage {
-	// TODO make lifetime = 0 mean that it does not get annotated with any garbage collection annotation
+	return &OIDCClientSecretStorage{
-	return &OIDCClientSecretStorage{storage: crud.New(TypeLabelValue, secrets, clock, 0)}
+		storage: crud.New(TypeLabelValue, secrets, clock, 0),
+		secrets: secrets,
+	}
 }
 
 // TODO expose other methods as needed for get, create, update, etc.
 
+// GetStorageSecret gets the corev1.Secret which is used to store the client secrets for the given client.
+// Returns nil,nil when the corev1.Secret was not found, as this is not an error for a client to not have any secrets yet.
+func (s *OIDCClientSecretStorage) GetStorageSecret(ctx context.Context, oidcClientUID types.UID) (*corev1.Secret, error) {
+	secret, err := s.secrets.Get(ctx, s.GetName(oidcClientUID), metav1.GetOptions{})
+	if errors.IsNotFound(err) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	return secret, nil
+}
+
 // GetName returns the name of the Secret which would be used to store data for the given signature.
 func (s *OIDCClientSecretStorage) GetName(oidcClientUID types.UID) string {
 	// Avoid having s.storage.GetName() base64 decode something that wasn't ever encoded by encoding it here.
@@ -53,7 +72,7 @@ func (s *OIDCClientSecretStorage) GetName(oidcClientUID types.UID) string {
 }
 
 // ReadFromSecret reads the contents of a Secret as a StoredClientSecret.
-func ReadFromSecret(secret *v1.Secret) (*StoredClientSecret, error) {
+func ReadFromSecret(secret *corev1.Secret) (*StoredClientSecret, error) {
 	storedClientSecret := &StoredClientSecret{}
 	err := crud.FromSecret(TypeLabelValue, secret, storedClientSecret)
 	if err != nil {

internal/oidcclientsecretstorage/oidcclientsecretstorage_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"go.pinniped.dev/internal/testutil"
 )
 
 func TestGetName(t *testing.T) {
@@ -106,6 +108,31 @@ func TestReadFromSecret(t *testing.T) {
 			},
 			wantErr: "secret storage data has incorrect version",
 		},
+		{
+			name: "OIDCClientSecretStorageSecretForUID() test helper generates readable format, to ensure that test helpers are kept up to date",
+			secret: testutil.OIDCClientSecretStorageSecretForUID(t,
+				"some-namespace", "some-uid", []string{"first-hash", "second-hash"},
+			),
+			wantStored: &StoredClientSecret{
+				Version:      "1",
+				SecretHashes: []string{"first-hash", "second-hash"},
+			},
+		},
+		{
+			name: "OIDCClientSecretStorageSecretWithoutName() test helper generates readable format, to ensure that test helpers are kept up to date",
+			secret: testutil.OIDCClientSecretStorageSecretWithoutName(t,
+				"some-namespace", []string{"first-hash", "second-hash"},
+			),
+			wantStored: &StoredClientSecret{
+				Version:      "1",
+				SecretHashes: []string{"first-hash", "second-hash"},
+			},
+		},
+		{
+			name:    "OIDCClientSecretStorageSecretForUIDWithWrongVersion() test helper generates readable format, to ensure that test helpers are kept up to date",
+			secret:  testutil.OIDCClientSecretStorageSecretForUIDWithWrongVersion(t, "some-namespace", "some-uid"),
+			wantErr: "OIDC client secret storage data has wrong version: OIDC client secret storage has version wrong-version instead of 1",
+		},
 	}
 
 	for _, tt := range tests {

internal/supervisor/server/server.go

@@ -439,6 +439,7 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
 		dynamicUpstreamIDPProvider,
 		&secretCache,
 		clientWithoutLeaderElection.Kubernetes.CoreV1().Secrets(serverInstallationNamespace), // writes to kube storage are allowed for non-leaders
+		client.PinnipedSupervisor.ConfigV1alpha1().OIDCClients(serverInstallationNamespace),
 	)
 
 	// Get the "real" name of the client secret supervisor API group (i.e., the API group name with the

internal/testutil/assertions.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
 	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
 )
 
@@ -54,6 +55,23 @@ func RequireNumberOfSecretsMatchingLabelSelector(t *testing.T, secrets v1.Secret
 	require.Len(t, storedAuthcodeSecrets.Items, expectedNumberOfSecrets)
 }
 
+func RequireNumberOfSecretsExcludingLabelSelector(t *testing.T, secrets v1.SecretInterface, labelSet labels.Set, expectedNumberOfSecrets int) {
+	t.Helper()
+
+	selector := labels.Everything()
+	for k, v := range labelSet {
+		requirement, err := labels.NewRequirement(k, selection.NotEquals, []string{v})
+		require.NoError(t, err)
+		selector = selector.Add(*requirement)
+	}
+
+	storedAuthcodeSecrets, err := secrets.List(context.Background(), v12.ListOptions{
+		LabelSelector: selector.String(),
+	})
+	require.NoError(t, err)
+	require.Len(t, storedAuthcodeSecrets.Items, expectedNumberOfSecrets)
+}
+
 func RequireSecurityHeadersWithFormPostPageCSPs(t *testing.T, response *httptest.ResponseRecorder) {
 	// Loosely confirm that the unique CSPs needed for the form_post page were used.
 	cspHeader := response.Header().Get("Content-Security-Policy")

internal/testutil/oidcclient.go

@@ -0,0 +1,67 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import (
+	"strings"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
+)
+
+const (
+	AllDynamicClientScopesSpaceSep = "openid offline_access pinniped:request-audience username groups"
+
+	// PlaintextPassword1 is a fake client secret for use in unit tests, along with several flavors of the bcrypt
+	// hashed version of the password. Do not use for integration tests.
+	PlaintextPassword1                        = "password1"
+	HashedPassword1AtGoMinCost                = "$2a$04$JfX1ba/ctAt3AGk73E9Zz.Fdki5GiQtj.O/CnPbRRSKQWWfv1svoe" //nolint:gosec // this is not a credential
+	HashedPassword1JustBelowSupervisorMinCost = "$2a$11$w/incy7Z1/ljLYvv2XRg4.WrPgY9oR7phebcgr6rGA3u/5TG9MKOe" //nolint:gosec // this is not a credential
+	HashedPassword1AtSupervisorMinCost        = "$2a$12$id4i/yFYxS99txKOFEeboea2kU6DyZY0Nh4ul0eR46sDuoFoNTRV." //nolint:gosec // this is not a credential
+	HashedPassword1InvalidFormat              = "$2a$12$id4i/yFYxS99txKOFEeboea2kU6DyZY0Nh4ul0eR46sDuo"        //nolint:gosec // this is not a credential
+
+	// PlaintextPassword2 is a second fake client secret for use in unit tests, along with several flavors of the bcrypt
+	// hashed version of the password. Do not use for integration tests.
+	PlaintextPassword2                 = "password2"
+	HashedPassword2AtGoMinCost         = "$2a$04$VQ5z6kkgU8JPLGSGctg.s.iYyoac3Oisa/SIM3sDK5BxTrVbCkyNm" //nolint:gosec // this is not a credential
+	HashedPassword2AtSupervisorMinCost = "$2a$12$SdUqoJOn4/3yEQfJx616V.q.f76KaXD.ISgJT1oydqFdgfjJpBh6u" //nolint:gosec // this is not a credential
+)
+
+// allDynamicClientScopes returns a slice of all scopes that are supported by the Supervisor for dynamic clients.
+func allDynamicClientScopes() []configv1alpha1.Scope {
+	scopes := []configv1alpha1.Scope{}
+	for _, s := range strings.Split(AllDynamicClientScopesSpaceSep, " ") {
+		scopes = append(scopes, configv1alpha1.Scope(s))
+	}
+	return scopes
+}
+
+// fullyCapableOIDCClient returns an OIDC client which is allowed to use all grant types and all scopes that
+// are supported by the Supervisor for dynamic clients.
+func fullyCapableOIDCClient(namespace string, clientID string, clientUID string, redirectURI string) *configv1alpha1.OIDCClient {
+	return &configv1alpha1.OIDCClient{
+		ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: clientID, Generation: 1, UID: types.UID(clientUID)},
+		Spec: configv1alpha1.OIDCClientSpec{
+			AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+			AllowedScopes:       allDynamicClientScopes(),
+			AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(redirectURI)},
+		},
+	}
+}
+
+func FullyCapableOIDCClientAndStorageSecret(
+	t *testing.T,
+	namespace string,
+	clientID string,
+	clientUID string,
+	redirectURI string,
+	hashes []string,
+) (*configv1alpha1.OIDCClient, *corev1.Secret) {
+	return fullyCapableOIDCClient(namespace, clientID, clientUID, redirectURI),
+		OIDCClientSecretStorageSecretForUID(t, namespace, clientUID, hashes)
+}

internal/testutil/oidcclient_test.go

@@ -0,0 +1,61 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import (
+	"testing"
+
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
+)
+
+func TestBcryptConstants(t *testing.T) {
+	t.Parallel()
+
+	// It would be helpful to know if upgrading golang changes these constants some day, so test them here for visibility during upgrades.
+	require.Equal(t, 4, bcrypt.MinCost, "golang has changed bcrypt.MinCost: please consider implications to the other tests")
+	require.Equal(t, 10, bcrypt.DefaultCost, "golang has changed bcrypt.DefaultCost: please consider implications to the production code and tests")
+}
+
+func TestBcryptHashedPassword1TestHelpers(t *testing.T) {
+	t.Parallel()
+
+	// Can use this to help generate or regenerate the test helper hash constants.
+	// t.Log(generateHash(t, PlaintextPassword1, 12))
+
+	require.NoError(t, bcrypt.CompareHashAndPassword([]byte(HashedPassword1AtGoMinCost), []byte(PlaintextPassword1)))
+	require.NoError(t, bcrypt.CompareHashAndPassword([]byte(HashedPassword1JustBelowSupervisorMinCost), []byte(PlaintextPassword1)))
+	require.NoError(t, bcrypt.CompareHashAndPassword([]byte(HashedPassword1AtSupervisorMinCost), []byte(PlaintextPassword1)))
+
+	requireCost(t, bcrypt.MinCost, HashedPassword1AtGoMinCost)
+	requireCost(t, oidcclientvalidator.DefaultMinBcryptCost-1, HashedPassword1JustBelowSupervisorMinCost)
+	requireCost(t, oidcclientvalidator.DefaultMinBcryptCost, HashedPassword1AtSupervisorMinCost)
+}
+
+func TestBcryptHashedPassword2TestHelpers(t *testing.T) {
+	t.Parallel()
+
+	// Can use this to help generate or regenerate the test helper hash constants.
+	// t.Log(generateHash(t, PlaintextPassword2, 12))
+
+	require.NoError(t, bcrypt.CompareHashAndPassword([]byte(HashedPassword2AtGoMinCost), []byte(PlaintextPassword2)))
+	require.NoError(t, bcrypt.CompareHashAndPassword([]byte(HashedPassword2AtSupervisorMinCost), []byte(PlaintextPassword2)))
+
+	requireCost(t, bcrypt.MinCost, HashedPassword2AtGoMinCost)
+	requireCost(t, oidcclientvalidator.DefaultMinBcryptCost, HashedPassword2AtSupervisorMinCost)
+}
+
+func generateHash(t *testing.T, password string, cost int) string { //nolint:unused,deadcode // used in comments above
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), cost)
+	require.NoError(t, err)
+	return string(hash)
+}
+
+func requireCost(t *testing.T, wantCost int, hash string) {
+	cost, err := bcrypt.Cost([]byte(hash))
+	require.NoError(t, err)
+	require.Equal(t, wantCost, cost)
+}

internal/testutil/oidcclientsecretstorage.go

@@ -0,0 +1,51 @@
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import (
+	"encoding/base32"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func secretNameForUID(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)))
+}
+
+func OIDCClientSecretStorageSecretWithoutName(t *testing.T, namespace string, hashes []string) *corev1.Secret {
+	hashesJSON, err := json.Marshal(hashes)
+	require.NoError(t, err) // this shouldn't really happen since we can always encode a slice of strings
+
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Labels:    map[string]string{"storage.pinniped.dev/type": "oidc-client-secret"},
+		},
+		Type: "storage.pinniped.dev/oidc-client-secret",
+		Data: map[string][]byte{
+			"pinniped-storage-data":    []byte(`{"version":"1","hashes":` + string(hashesJSON) + `}`),
+			"pinniped-storage-version": []byte("1"),
+		},
+	}
+}
+
+func OIDCClientSecretStorageSecretForUID(t *testing.T, namespace string, oidcClientUID string, hashes []string) *corev1.Secret {
+	secret := OIDCClientSecretStorageSecretWithoutName(t, namespace, hashes)
+	secret.Name = secretNameForUID(oidcClientUID)
+	return secret
+}
+
+func OIDCClientSecretStorageSecretForUIDWithWrongVersion(t *testing.T, namespace string, oidcClientUID string) *corev1.Secret {
+	secret := OIDCClientSecretStorageSecretForUID(t, namespace, oidcClientUID, []string{})
+	secret.Data["pinniped-storage-data"] = []byte(`{"version":"wrong-version","hashes":[]}`)
+	return secret
+}

internal/testutil/oidctestutil/oidctestutil.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	kubetesting "k8s.io/client-go/testing"
 	"k8s.io/utils/strings/slices"
 
 	"go.pinniped.dev/internal/authenticators"
@@ -954,7 +955,7 @@ func RequireAuthCodeRegexpMatch(
 	if includesOpenIDScope(wantDownstreamGrantedScopes) {
 		expectedNumberOfCreatedSecrets++
 	}
-	require.Len(t, kubeClient.Actions(), expectedNumberOfCreatedSecrets)
+	require.Len(t, FilterClientSecretCreateActions(kubeClient.Actions()), expectedNumberOfCreatedSecrets)
 
 	// One authcode should have been stored.
 	testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
@@ -1164,3 +1165,20 @@ func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requ
 
 	return storedRequest, storedSession
 }
+
+// FilterClientSecretCreateActions ignores any reads made to get a storage secret corresponding to an OIDCClient, since these
+// are normal actions when the request is using a dynamic client's client_id, and we don't need to make assertions
+// about these Secrets since they are not related to session storage.
+func FilterClientSecretCreateActions(actions []kubetesting.Action) []kubetesting.Action {
+	filtered := make([]kubetesting.Action, 0, len(actions))
+	for _, action := range actions {
+		if action.Matches("get", "secrets") {
+			getAction := action.(kubetesting.GetAction)
+			if strings.HasPrefix(getAction.GetName(), "pinniped-storage-oidc-client-secret-") {
+				continue // filter out OIDCClient's storage secret reads
+			}
+		}
+		filtered = append(filtered, action) // otherwise include the action
+	}
+	return filtered
+}

test/integration/kube_api_discovery_test.go

@@ -527,6 +527,9 @@ func TestCRDAdditionalPrinterColumns_Parallel(t *testing.T) {
 		},
 		addSuffix("oidcclients.config.supervisor"): {
 			"v1alpha1": []apiextensionsv1.CustomResourceColumnDefinition{
+				{Name: "Privileged Scopes", Type: "string", JSONPath: `.spec.allowedScopes[?(@ == "pinniped:request-audience")]`},
+				{Name: "Client Secrets", Type: "integer", JSONPath: ".status.totalClientSecrets"},
+				{Name: "Status", Type: "string", JSONPath: ".status.phase"},
 				{Name: "Age", Type: "date", JSONPath: ".metadata.creationTimestamp"},
 			},
 		},

test/integration/supervisor_login_test.go

@@ -31,6 +31,7 @@ import (
 	idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
 	"go.pinniped.dev/internal/certauthority"
 	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
@@ -156,29 +157,94 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 		return ldapIDP, secret
 	}
 
+	// These tests attempt to exercise the entire login and refresh flow of the Supervisor for various cases.
+	// They do not use the Pinniped CLI as the client, which allows them to exercise the Supervisor as an
+	// OIDC provider in ways that the CLI might not use. Similar tests exist using the CLI in e2e_test.go.
+	//
+	// Each of these tests perform the following flow:
+	// 1. Create a FederationDomain with TLS configured and wait for its JWKS endpoint to be available.
+	// 2. Configure an IDP CR.
+	// 3. Call the authorization endpoint and log in as a specific user.
+	//    Note that these tests do not use form_post response type (which is tested by e2e_test.go).
+	// 4. Listen on a local callback server for the authorization redirect, and assert that it was success or failure.
+	// 5. Call the token endpoint to exchange the authcode.
+	// 6. Call the token endpoint to perform the RFC8693 token exchange for the cluster-scoped ID token.
+	// 7. Potentially edit the refresh session data or IDP settings before the refresh.
+	// 8. Call the token endpoint to perform a refresh, and expect it to succeed.
+	// 9. Call the token endpoint again to perform another RFC8693 token exchange for the cluster-scoped ID token,
+	//    this time using the recently refreshed tokens when submitting the request.
+	// 10. Potentially edit the refresh session data or IDP settings again, this time in such a way that the next
+	//     refresh should fail. If done, then perform one more refresh and expect failure.
 	tests := []struct {
 		name string
+
+		// This required function might choose to skip the test case, for example if the LDAP server is not
+		// available for an LDAP test.
 		maybeSkip func(t *testing.T)
-		createTestUser                       func(t *testing.T) (string, string)
+
-		deleteTestUser                       func(t *testing.T, username string)
+		// This required function should configure an IDP CR. It should also wait for it to be ready and schedule
-		requestAuthorization                 func(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client)
+		// its cleanup. Return the name of the IDP CR.
 		createIDP func(t *testing.T) string
+
+		// Optionally create an OIDCClient CR for the test to use. Return the client ID and client secret for the
+		// test to use. When not set, the test will default to using the "pinniped-cli" static client with no secret.
+		// When a client secret is returned, it will be used for authcode exchange, refresh requests, and RFC8693
+		// token exchanges for cluster-scoped tokens (client secrets are not needed in authorization requests).
+		createOIDCClient func(t *testing.T, callbackURL string) (string, string)
+
+		// Optionally return the username and password for the test to use when logging in. This username/password
+		// will be passed to requestAuthorization(), or empty strings will be passed to indicate that the defaults
+		// should be used. If there is any cleanup required, then this function should also schedule that cleanup.
+		testUser func(t *testing.T) (string, string)
+
+		// This required function should call the authorization endpoint using the given URL and also perform whatever
+		// interactions are needed to log in as the user.
+		requestAuthorization func(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client)
+
+		// This string will be used as the requested audience in the RFC8693 token exchange for
+		// the cluster-scoped ID token. When it is not specified, a default string will be used.
 		requestTokenExchangeAud string
+
+		// The scopes to request from the authorization endpoint. Defaults will be used when not specified.
 		downstreamScopes []string
+
+		// When we want the localhost callback to have never happened, then the flow will stop there. The login was
+		// unable to finish so there is nothing to assert about what should have happened with the callback, and there
+		// won't be any error sent to the callback either. This would happen, for example, when the user fails to log
+		// in at the LDAP/AD login page, because then they would be redirected back to that page again, instead of
+		// getting a callback success/error redirect.
 		wantLocalhostCallbackToNeverHappen bool
+
+		// The expected ID token subject claim value as a regexp, for the original ID token and the refreshed ID token.
 		wantDownstreamIDTokenSubjectToMatch string
+		// The expected ID token username claim value as a regexp, for the original ID token and the refreshed ID token.
 		wantDownstreamIDTokenUsernameToMatch func(username string) string
+		// The expected ID token groups claim value, for the original ID token and the refreshed ID token.
 		wantDownstreamIDTokenGroups []string
-		wantErrorDescription                 string
+
-		wantErrorType                        string
+		// Want the authorization endpoint to redirect to the callback with this error type.
+		// The rest of the flow will be skipped since the initial authorization failed.
+		wantAuthorizationErrorType string
+		// Want the authorization endpoint to redirect to the callback with this error description.
+		// Should be used with wantAuthorizationErrorType.
+		wantAuthorizationErrorDescription string
+
+		// Optionally want to the authcode exchange at the token endpoint to fail. The rest of the flow will be
+		// skipped since the authcode exchange failed.
+		wantAuthcodeExchangeError string
+
+		// Optionally make all required assertions about the response of the RFC8693 token exchange for
+		// the cluster-scoped ID token, given the http response status and response body from the token endpoint.
+		// When this is not specified then the appropriate default assertions for a successful exchange are made.
+		// Even if this expects failures, the rest of the flow will continue.
 		wantTokenExchangeResponse func(t *testing.T, status int, body string)
 
-		// Either revoke the user's session on the upstream provider, or manipulate the user's session
+		// Optionally edit the refresh session data between the initial login and the first refresh,
+		// which is still expected to succeed after these edits.
+		editRefreshSessionDataWithoutBreaking func(t *testing.T, sessionData *psession.PinnipedSession, idpName, username string) []string
+		// Optionally either revoke the user's session on the upstream provider, or manipulate the user's session
 		// data in such a way that it should cause the next upstream refresh attempt to fail.
 		breakRefreshSessionData func(t *testing.T, sessionData *psession.PinnipedSession, idpName, username string)
-		// Edit the refresh session data between the initial login and the refresh, which is expected to
-		// succeed.
-		editRefreshSessionDataWithoutBreaking func(t *testing.T, sessionData *psession.PinnipedSession, idpName, username string) []string
 	}{
 		{
 			name:      "oidc with default username and groups claim settings",
@@ -389,7 +455,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createLDAPIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				// return the username and password of the existing user that we want to use for this test
 				return env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login
 					env.SupervisorUpstreamLDAP.TestUserPassword // password to present to server during login
@@ -414,7 +480,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createLDAPIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				// return the username and password of the existing user that we want to use for this test
 				return env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login
 					"this is the wrong password" // password to present to server during login
@@ -429,7 +495,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createLDAPIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				// return the username and password of the existing user that we want to use for this test
 				return "this is the wrong username", // username to present to server during login
 					env.SupervisorUpstreamLDAP.TestUserPassword // password to present to server during login
@@ -444,7 +510,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createLDAPIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				// return the username and password of the existing user that we want to use for this test
 				return env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login
 					env.SupervisorUpstreamLDAP.TestUserPassword // password to present to server during login
@@ -612,8 +678,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					true,
 				)
 			},
-			wantErrorDescription: "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.",
+			wantAuthorizationErrorDescription: "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.",
-			wantErrorType:        "access_denied",
+			wantAuthorizationErrorType:        "access_denied",
 		},
 		{
 			name:      "ldap login still works after updating bind secret",
@@ -964,12 +1030,9 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createActiveDirectoryIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				return testlib.CreateFreshADTestUser(t, env)
 			},
-			deleteTestUser: func(t *testing.T, username string) {
-				testlib.DeleteTestADUser(t, env, username)
-			},
 			requestAuthorization: func(t *testing.T, _, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) {
 				requestAuthorizationUsingCLIPasswordFlow(t,
 					downstreamAuthorizeURL,
@@ -997,12 +1060,9 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createActiveDirectoryIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				return testlib.CreateFreshADTestUser(t, env)
 			},
-			deleteTestUser: func(t *testing.T, username string) {
-				testlib.DeleteTestADUser(t, env, username)
-			},
 			requestAuthorization: func(t *testing.T, _, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) {
 				requestAuthorizationUsingCLIPasswordFlow(t,
 					downstreamAuthorizeURL,
@@ -1030,12 +1090,9 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				idp, _ := createActiveDirectoryIdentityProvider(t, nil)
 				return idp.Name
 			},
-			createTestUser: func(t *testing.T) (string, string) {
+			testUser: func(t *testing.T) (string, string) {
 				return testlib.CreateFreshADTestUser(t, env)
 			},
-			deleteTestUser: func(t *testing.T, username string) {
-				testlib.DeleteTestADUser(t, env, username)
-			},
 			requestAuthorization: func(t *testing.T, _, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) {
 				requestAuthorizationUsingCLIPasswordFlow(t,
 					downstreamAuthorizeURL,
@@ -1073,8 +1130,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				)
 			},
 			breakRefreshSessionData:           nil,
-			wantErrorDescription:    "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.",
+			wantAuthorizationErrorDescription: "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.",
-			wantErrorType:           "access_denied",
+			wantAuthorizationErrorType:        "access_denied",
 		},
 		{
 			name:      "ldap refresh fails when username changes from email as username to dn as username",
@@ -1226,27 +1283,141 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					body)
 			},
 		},
+		{
+			name:      "oidc upstream with downstream dynamic client happy path",
+			maybeSkip: skipNever,
+			createIDP: func(t *testing.T) string {
+				return testlib.CreateTestOIDCIdentityProvider(t, basicOIDCIdentityProviderSpec(), idpv1alpha1.PhaseReady).Name
+			},
+			createOIDCClient: func(t *testing.T, callbackURL string) (string, string) {
+				return testlib.CreateOIDCClient(t, configv1alpha1.OIDCClientSpec{
+					AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(callbackURL)},
+					AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+					AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+				}, configv1alpha1.PhaseReady)
+			},
+			requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowOIDC,
+			// the ID token Subject should include the upstream user ID after the upstream issuer name
+			wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+",
+			// the ID token Username should include the upstream user ID after the upstream issuer name
+			wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+" },
+		},
+		{
+			name:      "ldap upstream with downstream dynamic client happy path",
+			maybeSkip: skipLDAPTests,
+			createIDP: func(t *testing.T) string {
+				idp, _ := createLDAPIdentityProvider(t, nil)
+				return idp.Name
+			},
+			createOIDCClient: func(t *testing.T, callbackURL string) (string, string) {
+				return testlib.CreateOIDCClient(t, configv1alpha1.OIDCClientSpec{
+					AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(callbackURL)},
+					AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+					AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+				}, configv1alpha1.PhaseReady)
+			},
+			testUser: func(t *testing.T) (string, string) {
+				// return the username and password of the existing user that we want to use for this test
+				return env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login
+					env.SupervisorUpstreamLDAP.TestUserPassword // password to present to server during login
+			},
+			requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowLDAP,
+			// the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute
+			wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(
+				"ldaps://"+env.SupervisorUpstreamLDAP.Host+
+					"?base="+url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase)+
+					"&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)),
+			) + "$",
+			// the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute
+			wantDownstreamIDTokenUsernameToMatch: func(_ string) string {
+				return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$"
+			},
+			wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs,
+		},
+		{
+			name:      "active directory with all default options with downstream dynamic client happy path",
+			maybeSkip: skipActiveDirectoryTests,
+			createIDP: func(t *testing.T) string {
+				idp, _ := createActiveDirectoryIdentityProvider(t, nil)
+				return idp.Name
+			},
+			createOIDCClient: func(t *testing.T, callbackURL string) (string, string) {
+				return testlib.CreateOIDCClient(t, configv1alpha1.OIDCClientSpec{
+					AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(callbackURL)},
+					AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+					AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+				}, configv1alpha1.PhaseReady)
+			},
+			requestAuthorization: func(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) {
+				requestAuthorizationUsingBrowserAuthcodeFlowLDAP(t,
+					downstreamIssuer,
+					downstreamAuthorizeURL,
+					downstreamCallbackURL,
+					env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login
+					env.SupervisorUpstreamActiveDirectory.TestUserPassword,           // password to present to server during login
+					httpClient,
+				)
+			},
+			// the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute
+			wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(
+				"ldaps://"+env.SupervisorUpstreamActiveDirectory.Host+
+					"?base="+url.QueryEscape(env.SupervisorUpstreamActiveDirectory.DefaultNamingContextSearchBase)+
+					"&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue,
+			) + "$",
+			// the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute
+			wantDownstreamIDTokenUsernameToMatch: func(_ string) string {
+				return "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$"
+			},
+			wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames,
+		},
+		{
+			name:      "ldap upstream with downstream dynamic client, failed client authentication",
+			maybeSkip: skipLDAPTests,
+			createIDP: func(t *testing.T) string {
+				idp, _ := createLDAPIdentityProvider(t, nil)
+				return idp.Name
+			},
+			createOIDCClient: func(t *testing.T, callbackURL string) (string, string) {
+				clientID, _ := testlib.CreateOIDCClient(t, configv1alpha1.OIDCClientSpec{
+					AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(callbackURL)},
+					AllowedGrantTypes:   []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
+					AllowedScopes:       []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
+				}, configv1alpha1.PhaseReady)
+				return clientID, "wrong-client-secret"
+			},
+			testUser: func(t *testing.T) (string, string) {
+				// return the username and password of the existing user that we want to use for this test
+				return env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login
+					env.SupervisorUpstreamLDAP.TestUserPassword // password to present to server during login
+			},
+			requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowLDAP,
+			wantAuthcodeExchangeError: "oauth2: cannot fetch token: 401 Unauthorized\n" +
+				`Response: {"error":"invalid_client","error_description":"Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method)."}`,
+		},
 	}
+
 	for _, test := range tests {
 		tt := test
 		t.Run(tt.name, func(t *testing.T) {
 			tt.maybeSkip(t)
 
-			testSupervisorLogin(t,
+			testSupervisorLogin(
+				t,
 				tt.createIDP,
 				tt.requestAuthorization,
 				tt.editRefreshSessionDataWithoutBreaking,
 				tt.breakRefreshSessionData,
-				tt.createTestUser,
+				tt.testUser,
-				tt.deleteTestUser,
+				tt.createOIDCClient,
 				tt.downstreamScopes,
 				tt.requestTokenExchangeAud,
 				tt.wantLocalhostCallbackToNeverHappen,
 				tt.wantDownstreamIDTokenSubjectToMatch,
 				tt.wantDownstreamIDTokenUsernameToMatch,
 				tt.wantDownstreamIDTokenGroups,
-				tt.wantErrorDescription,
+				tt.wantAuthorizationErrorType,
-				tt.wantErrorType,
+				tt.wantAuthorizationErrorDescription,
+				tt.wantAuthcodeExchangeError,
 				tt.wantTokenExchangeResponse,
 			)
 		})
@@ -1375,18 +1546,19 @@ func testSupervisorLogin(
 	t *testing.T,
 	createIDP func(t *testing.T) string,
 	requestAuthorization func(t *testing.T, downstreamIssuer string, downstreamAuthorizeURL string, downstreamCallbackURL string, username string, password string, httpClient *http.Client),
-	editRefreshSessionDataWithoutBreaking func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName, username string) []string,
+	editRefreshSessionDataWithoutBreaking func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName string, username string) []string,
-	breakRefreshSessionData func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName, username string),
+	breakRefreshSessionData func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName string, username string),
-	createTestUser func(t *testing.T) (string, string),
+	testUser func(t *testing.T) (string, string),
-	deleteTestUser func(t *testing.T, username string),
+	createOIDCClient func(t *testing.T, callbackURL string) (string, string),
 	downstreamScopes []string,
 	requestTokenExchangeAud string,
 	wantLocalhostCallbackToNeverHappen bool,
 	wantDownstreamIDTokenSubjectToMatch string,
 	wantDownstreamIDTokenUsernameToMatch func(username string) string,
 	wantDownstreamIDTokenGroups []string,
-	wantErrorDescription string,
+	wantAuthorizationErrorType string,
-	wantErrorType string,
+	wantAuthorizationErrorDescription string,
+	wantAuthcodeExchangeError string,
 	wantTokenExchangeResponse func(t *testing.T, status int, body string),
 ) {
 	env := testlib.IntegrationEnv(t)
@@ -1475,12 +1647,20 @@ func testSupervisorLogin(
 	// Create upstream IDP and wait for it to become ready.
 	idpName := createIDP(t)
 
-	username, password := "", ""
+	// Start a callback server on localhost.
-	if createTestUser != nil {
+	localCallbackServer := startLocalCallbackServer(t)
-		username, password = createTestUser(t)
+
-		if deleteTestUser != nil {
+	// Optionally create an OIDCClient. Default to using the hardcoded public client that the Supervisor supports.
-			defer deleteTestUser(t, username)
+	clientID, clientSecret := "pinniped-cli", "" //nolint:gosec // empty credential is not a hardcoded credential
+	if createOIDCClient != nil {
+		clientID, clientSecret = createOIDCClient(t, localCallbackServer.URL)
 	}
+
+	// Optionally override which user to use for the test, or choose zero values to mean use the default for
+	// the test's IDP.
+	username, password := "", ""
+	if testUser != nil {
+		username, password = testUser(t)
 	}
 
 	// Perform OIDC discovery for our downstream.
@@ -1491,21 +1671,25 @@ func testSupervisorLogin(
 		requireEventually.NoError(err)
 	}, 30*time.Second, 200*time.Millisecond)
 
-	// Start a callback server on localhost.
-	localCallbackServer := startLocalCallbackServer(t)
-
 	if downstreamScopes == nil {
 		downstreamScopes = []string{"openid", "pinniped:request-audience", "offline_access", "groups"}
 	}
 
-	// Form the OAuth2 configuration corresponding to our CLI client.
+	// Create the OAuth2 configuration.
 	// Note that this is not using response_type=form_post, so the Supervisor will redirect to the callback endpoint
 	// directly, without using the Javascript form_post HTML page to POST back to the callback endpoint. The e2e
 	// tests which use the Pinniped CLI are testing the form_post part of the flow, so that is covered elsewhere.
+	// When ClientSecret is set here, it will be used for all token endpoint requests, but not for the authorization
+	// request, where it is not needed.
+	endpoint := discovery.Endpoint()
+	if clientSecret != "" {
+		// We only support basic auth for dynamic clients, so use basic auth in these tests.
+		endpoint.AuthStyle = oauth2.AuthStyleInHeader
+	}
 	downstreamOAuth2Config := oauth2.Config{
-		// This is the hardcoded public client that the supervisor supports.
+		ClientID:     clientID,
-		ClientID:    "pinniped-cli",
+		ClientSecret: clientSecret,
-		Endpoint:    discovery.Endpoint(),
+		Endpoint:     endpoint,
 		RedirectURL:  localCallbackServer.URL,
 		Scopes:       downstreamScopes,
 	}
@@ -1540,7 +1724,16 @@ func testSupervisorLogin(
 	require.NoError(t, err)
 
 	t.Logf("got callback request: %s", testlib.MaskTokens(callback.URL.String()))
-	if wantErrorType == "" { // nolint:nestif
+
+	if wantAuthorizationErrorType != "" {
+		errorDescription := callback.URL.Query().Get("error_description")
+		errorType := callback.URL.Query().Get("error")
+		require.Equal(t, errorDescription, wantAuthorizationErrorDescription)
+		require.Equal(t, errorType, wantAuthorizationErrorType)
+		// The authorization has failed, so can't continue the login flow, making this the end of the test case.
+		return
+	}
+
 	require.Equal(t, stateParam.String(), callback.URL.Query().Get("state"))
 	require.ElementsMatch(t, downstreamScopes, strings.Split(callback.URL.Query().Get("scope"), " "))
 	authcode := callback.URL.Query().Get("code")
@@ -1551,8 +1744,12 @@ func testSupervisorLogin(
 
 	// Call the token endpoint to get tokens.
 	tokenResponse, err := downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier())
+	if wantAuthcodeExchangeError != "" {
+		require.EqualError(t, err, wantAuthcodeExchangeError)
+		// The authcode exchange has failed, so can't continue the login flow, making this the end of the test case.
+		return
+	}
 	require.NoError(t, err)
-
 	expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "username"}
 	if slices.Contains(downstreamScopes, "groups") {
 		expectedIDTokenClaims = append(expectedIDTokenClaims, "groups")
@@ -1573,9 +1770,9 @@ func testSupervisorLogin(
 		signatureOfLatestRefreshToken := getFositeDataSignature(t, latestRefreshToken)
 
 		// First use the latest downstream refresh token to look up the corresponding session in the Supervisor's storage.
-			kubeClient := testlib.NewKubernetesClientset(t)
+		supervisorSecretsClient := testlib.NewKubernetesClientset(t).CoreV1().Secrets(env.SupervisorNamespace)
-			supervisorSecretsClient := kubeClient.CoreV1().Secrets(env.SupervisorNamespace)
+		supervisorOIDCClientsClient := testlib.NewSupervisorClientset(t).ConfigV1alpha1().OIDCClients(env.SupervisorNamespace)
-			oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, oidc.DefaultOIDCTimeoutsConfiguration())
+		oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, supervisorOIDCClientsClient, oidc.DefaultOIDCTimeoutsConfiguration(), oidcclientvalidator.DefaultMinBcryptCost)
 		storedRefreshSession, err := oauthStore.GetRefreshTokenSession(ctx, signatureOfLatestRefreshToken, nil)
 		require.NoError(t, err)
 
@@ -1618,9 +1815,9 @@ func testSupervisorLogin(
 		signatureOfLatestRefreshToken := getFositeDataSignature(t, latestRefreshToken)
 
 		// First use the latest downstream refresh token to look up the corresponding session in the Supervisor's storage.
-			kubeClient := testlib.NewKubernetesClientset(t)
+		supervisorSecretsClient := testlib.NewKubernetesClientset(t).CoreV1().Secrets(env.SupervisorNamespace)
-			supervisorSecretsClient := kubeClient.CoreV1().Secrets(env.SupervisorNamespace)
+		supervisorOIDCClientsClient := testlib.NewSupervisorClientset(t).ConfigV1alpha1().OIDCClients(env.SupervisorNamespace)
-			oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, oidc.DefaultOIDCTimeoutsConfiguration())
+		oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, supervisorOIDCClientsClient, oidc.DefaultOIDCTimeoutsConfiguration(), oidcclientvalidator.DefaultMinBcryptCost)
 		storedRefreshSession, err := oauthStore.GetRefreshTokenSession(ctx, signatureOfLatestRefreshToken, nil)
 		require.NoError(t, err)
 
@@ -1645,12 +1842,6 @@ func testSupervisorLogin(
 			err.Error(),
 		)
 	}
-	} else {
-		errorDescription := callback.URL.Query().Get("error_description")
-		errorType := callback.URL.Query().Get("error")
-		require.Equal(t, errorDescription, wantErrorDescription)
-		require.Equal(t, errorType, wantErrorType)
-	}
 }
 
 // getFositeDataSignature returns the signature of the provided data. The provided data could be an auth code, access
@@ -1922,6 +2113,10 @@ func doTokenExchange(
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, config.Endpoint.TokenURL, reqBody)
 	require.NoError(t, err)
 	req.Header.Set("content-type", "application/x-www-form-urlencoded")
+	if config.ClientSecret != "" {
+		// We only support basic auth for dynamic clients, so use basic auth in these tests.
+		req.SetBasicAuth(config.ClientID, config.ClientSecret)
+	}
 
 	resp, err := httpClient.Do(req)
 	require.NoError(t, err)

test/integration/supervisor_oidc_client_test.go

@@ -528,16 +528,7 @@ func TestOIDCClientControllerValidations_Parallel(t *testing.T) {
 					AllowedScopes:       []supervisorconfigv1alpha1.Scope{"openid"},
 				},
 			},
-			secret: &corev1.Secret{
+			secret:    testutil.OIDCClientSecretStorageSecretWithoutName(t, env.SupervisorNamespace, []string{}),
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{"storage.pinniped.dev/type": "oidc-client-secret"},
-				},
-				Type: "storage.pinniped.dev/oidc-client-secret",
-				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"version":"1","hashes":[]}`),
-					"pinniped-storage-version": []byte("1"),
-				},
-			},
 			wantPhase: "Error",
 			wantConditions: []supervisorconfigv1alpha1.Condition{
 				{
@@ -572,16 +563,7 @@ func TestOIDCClientControllerValidations_Parallel(t *testing.T) {
 					AllowedScopes:       []supervisorconfigv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
 				},
 			},
-			secret: &corev1.Secret{
+			secret:    testutil.OIDCClientSecretStorageSecretWithoutName(t, env.SupervisorNamespace, []string{testutil.HashedPassword1AtSupervisorMinCost}),
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{"storage.pinniped.dev/type": "oidc-client-secret"},
-				},
-				Type: "storage.pinniped.dev/oidc-client-secret",
-				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"version":"1","hashes":["$2y$15$Kh7cRj0ScSD5QelE3ZNSl.nF04JDv7zb3SgGN.tSfLIX.4kt3UX7m"]}`),
-					"pinniped-storage-version": []byte("1"),
-				},
-			},
 			wantPhase: "Ready",
 			wantConditions: []supervisorconfigv1alpha1.Condition{
 				{

test/integration/supervisor_warnings_test.go

@@ -31,6 +31,7 @@ import (
 	idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
 	"go.pinniped.dev/internal/certauthority"
 	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/pkg/oidcclient"
@@ -186,9 +187,10 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 
 		// using the refresh token signature contained in the cache, get the refresh token session
 		// out of kube secret storage.
-		kubeClient := testlib.NewKubernetesClientset(t).CoreV1()
+		supervisorSecretsClient := testlib.NewKubernetesClientset(t).CoreV1().Secrets(env.SupervisorNamespace)
+		supervisorOIDCClientsClient := testlib.NewSupervisorClientset(t).ConfigV1alpha1().OIDCClients(env.SupervisorNamespace)
+		oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, supervisorOIDCClientsClient, oidc.DefaultOIDCTimeoutsConfiguration(), oidcclientvalidator.DefaultMinBcryptCost)
 		refreshTokenSignature := strings.Split(token.RefreshToken.Token, ".")[1]
-		oauthStore := oidc.NewKubeStorage(kubeClient.Secrets(env.SupervisorNamespace), oidc.DefaultOIDCTimeoutsConfiguration())
 		storedRefreshSession, err := oauthStore.GetRefreshTokenSession(ctx, refreshTokenSignature, nil)
 		require.NoError(t, err)
 
@@ -246,9 +248,6 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
 
 		expectedUsername, password := testlib.CreateFreshADTestUser(t, env)
-		t.Cleanup(func() {
-			testlib.DeleteTestADUser(t, env, expectedUsername)
-		})
 
 		sAMAccountName := expectedUsername + "@" + env.SupervisorUpstreamActiveDirectory.Domain
 		setupClusterForEndToEndActiveDirectoryTest(t, sAMAccountName, env)
@@ -308,9 +307,6 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 
 		// create an active directory group, and add our user to it.
 		groupName := testlib.CreateFreshADTestGroup(t, env)
-		t.Cleanup(func() {
-			testlib.DeleteTestADUser(t, env, groupName)
-		})
 		testlib.AddTestUserToGroup(t, env, groupName, expectedUsername)
 
 		// remove the credential cache, which includes the cached cert, so it won't be reused and the refresh flow will be triggered.
@@ -499,9 +495,10 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 
 		// using the refresh token signature contained in the cache, get the refresh token session
 		// out of kube secret storage.
-		kubeClient := testlib.NewKubernetesClientset(t).CoreV1()
+		supervisorSecretsClient := testlib.NewKubernetesClientset(t).CoreV1().Secrets(env.SupervisorNamespace)
+		supervisorOIDCClientsClient := testlib.NewSupervisorClientset(t).ConfigV1alpha1().OIDCClients(env.SupervisorNamespace)
+		oauthStore := oidc.NewKubeStorage(supervisorSecretsClient, supervisorOIDCClientsClient, oidc.DefaultOIDCTimeoutsConfiguration(), oidcclientvalidator.DefaultMinBcryptCost)
 		refreshTokenSignature := strings.Split(token.RefreshToken.Token, ".")[1]
-		oauthStore := oidc.NewKubeStorage(kubeClient.Secrets(env.SupervisorNamespace), oidc.DefaultOIDCTimeoutsConfiguration())
 		storedRefreshSession, err := oauthStore.GetRefreshTokenSession(ctx, refreshTokenSignature, nil)
 		require.NoError(t, err)
 

test/testlib/activedirectory.go

@@ -42,6 +42,11 @@ func CreateFreshADTestUser(t *testing.T, env *TestEnv) (string, string) {
 	err = conn.Add(a)
 	require.NoError(t, err)
 
+	// Now that it has been created, schedule it for cleanup.
+	t.Cleanup(func() {
+		deleteTestADUser(t, env, testUserName)
+	})
+
 	// modify password and enable account
 	testUserPassword := createRandomASCIIString(t, 20)
 	enc := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewEncoder()
@@ -83,6 +88,11 @@ func CreateFreshADTestGroup(t *testing.T, env *TestEnv) string {
 	err = conn.Add(a)
 	require.NoError(t, err)
 
+	// Now that it has been created, schedule it for cleanup.
+	t.Cleanup(func() {
+		deleteTestADUser(t, env, testGroupName)
+	})
+
 	time.Sleep(20 * time.Second) // intrasite domain controller replication can take up to 15 seconds, so wait to ensure the change has propogated.
 	return testGroupName
 }
@@ -164,8 +174,8 @@ func ChangeADTestUserPassword(t *testing.T, env *TestEnv, testUserName string) {
 	// don't bother to return the new password... we won't be using it, just checking that it's changed.
 }
 
-// DeleteTestADUser deletes the test user created for this test.
+// deleteTestADUser deletes the test user created for this test.
-func DeleteTestADUser(t *testing.T, env *TestEnv, testUserName string) {
+func deleteTestADUser(t *testing.T, env *TestEnv, testUserName string) {
 	t.Helper()
 	conn := dialTLS(t, env)
 	// bind

test/testlib/client.go

@@ -15,10 +15,14 @@ import (
 	"testing"
 	"time"
 
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
+
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -26,8 +30,6 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 	aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset"
 
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
-
 	auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
 	"go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1"
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
@@ -36,6 +38,7 @@ import (
 	supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
 	"go.pinniped.dev/internal/groupsuffix"
 	"go.pinniped.dev/internal/kubeclient"
+	"go.pinniped.dev/internal/oidcclientsecretstorage"
 
 	// Import to initialize client auth plugins - the kubeconfig that we use for
 	// testing may use gcloud, az, oidc, etc.
@@ -378,6 +381,89 @@ func CreateClientCredsSecret(t *testing.T, clientID string, clientSecret string)
 	)
 }
 
+func CreateOIDCClient(t *testing.T, spec configv1alpha1.OIDCClientSpec, expectedPhase configv1alpha1.OIDCClientPhase) (string, string) {
+	t.Helper()
+	env := IntegrationEnv(t)
+	client := NewSupervisorClientset(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	oidcClientClient := client.ConfigV1alpha1().OIDCClients(env.SupervisorNamespace)
+
+	// Create the OIDCClient using GenerateName to get a random name.
+	created, err := oidcClientClient.Create(ctx, &configv1alpha1.OIDCClient{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: "client.oauth.pinniped.dev-test-", // use the required name prefix
+			Labels:       map[string]string{"pinniped.dev/test": ""},
+			Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
+		},
+		Spec: spec,
+	}, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	// Always clean this up after this point.
+	t.Cleanup(func() {
+		t.Logf("cleaning up test OIDCClient %s/%s", created.Namespace, created.Name)
+		err := oidcClientClient.Delete(context.Background(), created.Name, metav1.DeleteOptions{})
+		require.NoError(t, err)
+	})
+	t.Logf("created test OIDCClient %s", created.Name)
+
+	// Create a client secret for the new OIDCClient.
+	clientSecret := createOIDCClientSecret(t, created)
+
+	// Wait for the OIDCClient to enter the expected phase (or time out).
+	var result *configv1alpha1.OIDCClient
+	RequireEventuallyf(t, func(requireEventually *require.Assertions) {
+		var err error
+		result, err = oidcClientClient.Get(ctx, created.Name, metav1.GetOptions{})
+		requireEventually.NoErrorf(err, "error while getting OIDCClient %s/%s", created.Namespace, created.Name)
+		requireEventually.Equal(expectedPhase, result.Status.Phase)
+	}, 60*time.Second, 1*time.Second, "expected the OIDCClient to go into phase %s, OIDCClient was: %s", expectedPhase, Sdump(result))
+
+	return created.Name, clientSecret
+}
+
+func createOIDCClientSecret(t *testing.T, forOIDCClient *configv1alpha1.OIDCClient) string {
+	// TODO Replace this with a call to the real Supervisor API for creating client secrets after that gets implemented.
+	//   For now, just manually create a Secret with the right format so the tests can work.
+	t.Helper()
+	env := IntegrationEnv(t)
+	kubeClient := NewKubernetesClientset(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	var buf [32]byte
+	_, err := io.ReadFull(rand.Reader, buf[:])
+	require.NoError(t, err)
+	randomSecret := hex.EncodeToString(buf[:])
+	hashedRandomSecret, err := bcrypt.GenerateFromPassword([]byte(randomSecret), oidcclientvalidator.DefaultMinBcryptCost)
+	require.NoError(t, err)
+
+	created, err := kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Create(ctx, &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        oidcclientsecretstorage.New(nil, nil).GetName(forOIDCClient.UID), // use the required name
+			Labels:      map[string]string{"storage.pinniped.dev/type": "oidc-client-secret", "pinniped.dev/test": ""},
+			Annotations: map[string]string{"pinniped.dev/testName": t.Name()},
+		},
+		Type: "storage.pinniped.dev/oidc-client-secret",
+		Data: map[string][]byte{
+			"pinniped-storage-data":    []byte(`{"version":"1","hashes":["` + string(hashedRandomSecret) + `"]}`),
+			"pinniped-storage-version": []byte("1"),
+		},
+	}, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		t.Logf("cleaning up test Secret %s/%s", created.Namespace, created.Name)
+		err := kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{})
+		require.NoError(t, err)
+	})
+
+	t.Logf("created test Secret %s", created.Name)
+	return randomSecret
+}
+
 func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityProviderSpec, expectedPhase idpv1alpha1.OIDCIdentityProviderPhase) *idpv1alpha1.OIDCIdentityProvider {
 	t.Helper()
 	env := IntegrationEnv(t)
@@ -385,9 +471,9 @@ func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityP
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer cancel()
 
-	// Create the OIDCIdentityProvider using GenerateName to get a random name.
 	upstreams := client.IDPV1alpha1().OIDCIdentityProviders(env.SupervisorNamespace)
 
+	// Create the OIDCIdentityProvider using GenerateName to get a random name.
 	created, err := upstreams.Create(ctx, &idpv1alpha1.OIDCIdentityProvider{
 		ObjectMeta: testObjectMeta(t, "upstream-oidc-idp"),
 		Spec:       spec,
@@ -420,9 +506,9 @@ func CreateTestLDAPIdentityProvider(t *testing.T, spec idpv1alpha1.LDAPIdentityP
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer cancel()
 
-	// Create the LDAPIdentityProvider using GenerateName to get a random name.
 	upstreams := client.IDPV1alpha1().LDAPIdentityProviders(env.SupervisorNamespace)
 
+	// Create the LDAPIdentityProvider using GenerateName to get a random name.
 	created, err := upstreams.Create(ctx, &idpv1alpha1.LDAPIdentityProvider{
 		ObjectMeta: testObjectMeta(t, "upstream-ldap-idp"),
 		Spec:       spec,
@@ -461,9 +547,9 @@ func CreateTestActiveDirectoryIdentityProvider(t *testing.T, spec idpv1alpha1.Ac
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer cancel()
 
-	// Create the ActiveDirectoryIdentityProvider using GenerateName to get a random name.
 	upstreams := client.IDPV1alpha1().ActiveDirectoryIdentityProviders(env.SupervisorNamespace)
 
+	// Create the ActiveDirectoryIdentityProvider using GenerateName to get a random name.
 	created, err := upstreams.Create(ctx, &idpv1alpha1.ActiveDirectoryIdentityProvider{
 		ObjectMeta: testObjectMeta(t, "upstream-ad-idp"),
 		Spec:       spec,
@@ -501,9 +587,9 @@ func CreateTestClusterRoleBinding(t *testing.T, subject rbacv1.Subject, roleRef
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
 
-	// Create the ClusterRoleBinding using GenerateName to get a random name.
 	clusterRoles := client.RbacV1().ClusterRoleBindings()
 
+	// Create the ClusterRoleBinding using GenerateName to get a random name.
 	created, err := clusterRoles.Create(ctx, &rbacv1.ClusterRoleBinding{
 		ObjectMeta: testObjectMeta(t, "cluster-role"),
 		Subjects:   []rbacv1.Subject{subject},