Create username scope, required for clients to get username in ID token

- For backwards compatibility with older Pinniped CLIs, the pinniped-cli
  client does not need to request the username or groups scopes for them
  to be granted. For dynamic clients, the usual OAuth2 rules apply:
  the client must be allowed to request the scopes according to its
  configuration, and the client must actually request the scopes in the
  authorization request.
- If the username scope was not granted, then there will be no username
  in the ID token, and the cluster-scoped token exchange will fail since
  there would be no username in the resulting cluster-scoped ID token.
- The OIDC well-known discovery endpoint lists the username and groups
  scopes in the scopes_supported list, and lists the username and groups
  claims in the claims_supported list.
- Add username and groups scopes to the default list of scopes
  put into kubeconfig files by "pinniped get kubeconfig" CLI command,
  and the default list of scopes used by "pinniped login oidc" when
  no list of scopes is specified in the kubeconfig file
- The warning header about group memberships changing during upstream
  refresh will only be sent to the pinniped-cli client, since it is
  only intended for kubectl and it could leak the username to the
  client (which may not have the username scope granted) through the
  warning message text.
- Add the user's username to the session storage as a new field, so that
  during upstream refresh we can compare the original username from the
  initial authorization to the refreshed username, even in the case when
  the username scope was not granted (and therefore the username is not
  stored in the ID token claims of the session storage)
- Bump the Supervisor session storage format version from 2 to 3
  due to the username field being added to the session struct
- Extract commonly used string constants related to OIDC flows to api
  package.
- Change some import names to make them consistent:
  - Always import github.com/coreos/go-oidc/v3/oidc as "coreosoidc"
  - Always import go.pinniped.dev/generated/latest/apis/supervisor/oidc
    as "oidcapi"
  - Always import go.pinniped.dev/internal/oidc as "oidc"

internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/klog/v2"
 
 	auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	authinformers "go.pinniped.dev/generated/latest/client/concierge/informers/externalversions/authentication/v1alpha1"
 	pinnipedcontroller "go.pinniped.dev/internal/controller"
 	pinnipedauthenticator "go.pinniped.dev/internal/controller/authenticator"
@@ -32,8 +33,8 @@ import (
 // These default values come from the way that the Supervisor issues and signs tokens. We make these
 // the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor.
 const (
-	defaultUsernameClaim = "username"
-	defaultGroupsClaim   = "groups"
+	defaultUsernameClaim = oidcapi.IDTokenClaimUsername
+	defaultGroupsClaim   = oidcapi.IDTokenClaimGroups
 )
 
 // defaultSupportedSigningAlgos returns the default signing algos that this JWTAuthenticator

internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go

@@ -15,6 +15,7 @@ import (
 	corev1informers "k8s.io/client-go/informers/core/v1"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
 	configInformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/config/v1alpha1"
 	pinnipedcontroller "go.pinniped.dev/internal/controller"
@@ -27,7 +28,7 @@ import (
 
 const (
 	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
+	oidcClientPrefixToObserve = oidcapi.ClientIDRequiredOIDCClientPrefix
 )
 
 type oidcClientWatcherController struct {

internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go

@@ -14,7 +14,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coreos/go-oidc/v3/oidc"
+	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/go-logr/logr"
 	"golang.org/x/oauth2"
 	corev1 "k8s.io/api/core/v1"
@@ -26,6 +26,7 @@ import (
 	corev1informers "k8s.io/client-go/informers/core/v1"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
 	idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1"
 	"go.pinniped.dev/internal/constable"
@@ -97,11 +98,11 @@ type UpstreamOIDCIdentityProviderICache interface {
 type lruValidatorCache struct{ cache *cache.Expiring }
 
 type lruValidatorCacheEntry struct {
-	provider *oidc.Provider
+	provider *coreosoidc.Provider
 	client   *http.Client
 }
 
-func (c *lruValidatorCache) getProvider(spec *v1alpha1.OIDCIdentityProviderSpec) (*oidc.Provider, *http.Client) {
+func (c *lruValidatorCache) getProvider(spec *v1alpha1.OIDCIdentityProviderSpec) (*coreosoidc.Provider, *http.Client) {
 	if result, ok := c.cache.Get(c.cacheKey(spec)); ok {
 		entry := result.(*lruValidatorCacheEntry)
 		return entry.provider, entry.client
@@ -109,7 +110,7 @@ func (c *lruValidatorCache) getProvider(spec *v1alpha1.OIDCIdentityProviderSpec)
 	return nil, nil
 }
 
-func (c *lruValidatorCache) putProvider(spec *v1alpha1.OIDCIdentityProviderSpec, provider *oidc.Provider, client *http.Client) {
+func (c *lruValidatorCache) putProvider(spec *v1alpha1.OIDCIdentityProviderSpec, provider *coreosoidc.Provider, client *http.Client) {
 	c.cache.Set(c.cacheKey(spec), &lruValidatorCacheEntry{provider: provider, client: client}, oidcValidatorCacheTTL)
 }
 
@@ -129,8 +130,8 @@ type oidcWatcherController struct {
 	oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer
 	secretInformer               corev1informers.SecretInformer
 	validatorCache               interface {
-		getProvider(*v1alpha1.OIDCIdentityProviderSpec) (*oidc.Provider, *http.Client)
-		putProvider(*v1alpha1.OIDCIdentityProviderSpec, *oidc.Provider, *http.Client)
+		getProvider(*v1alpha1.OIDCIdentityProviderSpec) (*coreosoidc.Provider, *http.Client)
+		putProvider(*v1alpha1.OIDCIdentityProviderSpec, *coreosoidc.Provider, *http.Client)
 	}
 }
 
@@ -329,7 +330,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *v1
 			return issuerURLCondition
 		}
 
-		discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer)
+		discoveredProvider, err = coreosoidc.NewProvider(coreosoidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer)
 		if err != nil {
 			c.log.V(plog.KlogLevelTrace).WithValues(
 				"namespace", upstream.Namespace,
@@ -457,12 +458,12 @@ func defaultClientShortTimeout(rootCAs *x509.CertPool) *http.Client {
 func computeScopes(additionalScopes []string) []string {
 	// If none are set then provide a reasonable default which only tries to use scopes defined in the OIDC spec.
 	if len(additionalScopes) == 0 {
-		return []string{"openid", "offline_access", "email", "profile"}
+		return []string{oidcapi.ScopeOpenID, oidcapi.ScopeOfflineAccess, oidcapi.ScopeEmail, oidcapi.ScopeProfile}
 	}
 
 	// Otherwise, first compute the unique set of scopes, including "openid" (de-duplicate).
 	set := sets.NewString()
-	set.Insert("openid")
+	set.Insert(oidcapi.ScopeOpenID)
 	for _, s := range additionalScopes {
 		set.Insert(s)
 	}

internal/controller/supervisorstorage/garbage_collector.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"time"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -17,8 +16,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/utils/clock"
 	clocktesting "k8s.io/utils/clock/testing"
-	"k8s.io/utils/strings/slices"
 
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	pinnipedcontroller "go.pinniped.dev/internal/controller"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/crud"
@@ -204,7 +203,7 @@ func (c *garbageCollectorController) maybeRevokeUpstreamOIDCToken(ctx context.Co
 			return err
 		}
 		pinnipedSession := accessTokenSession.Request.Session.(*psession.PinnipedSession)
-		if slices.Contains(accessTokenSession.Request.GetGrantedScopes(), coreosoidc.ScopeOfflineAccess) {
+		if accessTokenSession.Request.GetGrantedScopes().Has(oidcapi.ScopeOfflineAccess) {
 			return nil
 		}
 		return c.tryRevokeUpstreamOIDCToken(ctx, pinnipedSession.Custom, secret)

internal/controller/supervisorstorage/garbage_collector_test.go

@@ -263,13 +263,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired authcode secrets which contain upstream refresh tokens", func() {
 			it.Before(func() {
 				activeOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -307,13 +308,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 				r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
 
 				inactiveOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  false,
 					Request: &fosite.Request{
 						ID:     "request-id-2",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -385,13 +387,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired authcode secrets which contain upstream access tokens", func() {
 			it.Before(func() {
 				activeOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -429,13 +432,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 				r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
 
 				inactiveOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  false,
 					Request: &fosite.Request{
 						ID:     "request-id-2",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -507,13 +511,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there is an invalid, expired authcode secret", func() {
 			it.Before(func() {
 				invalidOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "", // it is invalid for there to be a missing request ID
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -575,13 +580,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there is a valid, expired authcode secret but its upstream name does not match any existing upstream", func() {
 			it.Before(func() {
 				wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name-will-not-match",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -645,13 +651,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there is a valid, expired authcode secret but its upstream UID does not match any existing upstream", func() {
 			it.Before(func() {
 				wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid-will-not-match",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -715,13 +722,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there is a valid, recently expired authcode secret but the upstream revocation fails", func() {
 			it.Before(func() {
 				activeOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -819,13 +827,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there is a valid, long-since expired authcode secret but the upstream revocation fails", func() {
 			it.Before(func() {
 				activeOIDCAuthcodeSession := &authorizationcode.Session{
-					Version: "2",
+					Version: "3",
 					Active:  true,
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -897,13 +906,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired access token secrets which contain upstream refresh tokens", func() {
 			it.Before(func() {
 				offlineAccessGrantedOIDCAccessTokenSession := &accesstoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						GrantedScope: fosite.Arguments{"scope1", "scope2", "offline_access"},
 						ID:           "request-id-1",
 						Client:       &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -941,13 +951,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 				r.NoError(kubeClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))
 
 				offlineAccessNotGrantedOIDCAccessTokenSession := &accesstoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						GrantedScope: fosite.Arguments{"scope1", "scope2"},
 						ID:           "request-id-2",
 						Client:       &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -1019,13 +1030,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired access token secrets which contain upstream access tokens", func() {
 			it.Before(func() {
 				offlineAccessGrantedOIDCAccessTokenSession := &accesstoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						GrantedScope: fosite.Arguments{"scope1", "scope2", "offline_access"},
 						ID:           "request-id-1",
 						Client:       &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -1063,13 +1075,14 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 				r.NoError(kubeClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))
 
 				offlineAccessNotGrantedOIDCAccessTokenSession := &accesstoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						GrantedScope: fosite.Arguments{"scope1", "scope2"},
 						ID:           "request-id-2",
 						Client:       &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -1141,12 +1154,13 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired refresh secrets which contain upstream refresh tokens", func() {
 			it.Before(func() {
 				oidcRefreshSession := &refreshtoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,
@@ -1217,12 +1231,13 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		when("there are valid, expired refresh secrets which contain upstream access tokens", func() {
 			it.Before(func() {
 				oidcRefreshSession := &refreshtoken.Session{
-					Version: "2",
+					Version: "3",
 					Request: &fosite.Request{
 						ID:     "request-id-1",
 						Client: &clientregistry.Client{},
 						Session: &psession.PinnipedSession{
 							Custom: &psession.CustomSessionData{
+								Username:     "should be ignored by garbage collector",
 								ProviderUID:  "upstream-oidc-provider-uid",
 								ProviderName: "upstream-oidc-provider-name",
 								ProviderType: psession.ProviderTypeOIDC,

internal/fositestorage/accesstoken/accesstoken.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package accesstoken
@@ -29,7 +29,8 @@ const (
 
 	// Version 1 was the initial release of storage.
 	// Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request.
-	accessTokenStorageVersion = "2"
+	// Version 3 is when we added the Username field to the psession.CustomSessionData.
+	accessTokenStorageVersion = "3"
 )
 
 type RevocationStorage interface {

internal/fositestorage/accesstoken/accesstoken_test.go

@@ -53,7 +53,7 @@ func TestAccessTokenStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/access-token",
@@ -122,7 +122,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/access-token",
@@ -195,7 +195,7 @@ func TestWrongVersion(t *testing.T) {
 
 	_, err = storage.GetAccessTokenSession(ctx, "fancy-signature", nil)
 
-	require.EqualError(t, err, "access token request data has wrong version: access token session for fancy-signature has version not-the-right-version instead of 2")
+	require.EqualError(t, err, "access token request data has wrong version: access token session for fancy-signature has version not-the-right-version instead of 3")
 }
 
 func TestNilSessionRequest(t *testing.T) {
@@ -213,7 +213,7 @@ func TestNilSessionRequest(t *testing.T) {
 			},
 		},
 		Data: map[string][]byte{
-			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"2"}`),
+			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"3"}`),
 			"pinniped-storage-version": []byte("1"),
 		},
 		Type: "storage.pinniped.dev/access-token",
@@ -297,13 +297,13 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/access-token",
 			},
 			wantSession: &Session{
-				Version: "2",
+				Version: "3",
 				Request: &fosite.Request{
 					ID:     "abcd-1",
 					Client: &clientregistry.Client{},
@@ -313,6 +313,7 @@ func TestReadFromSecret(t *testing.T) {
 							Subject:  "panda",
 						},
 						Custom: &psession.CustomSessionData{
+							Username:     "fake-username",
 							ProviderUID:  "fake-provider-uid",
 							ProviderName: "fake-provider-name",
 							ProviderType: "fake-provider-type",
@@ -335,7 +336,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/not-access-token",
@@ -358,7 +359,7 @@ func TestReadFromSecret(t *testing.T) {
 				},
 				Type: "storage.pinniped.dev/access-token",
 			},
-			wantErr: "access token request data has wrong version: access token session has version wrong-version-here instead of 2",
+			wantErr: "access token request data has wrong version: access token session has version wrong-version-here instead of 3",
 		},
 		{
 			name: "missing request",
@@ -371,7 +372,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/access-token",

internal/fositestorage/authorizationcode/authorizationcode.go

@@ -30,7 +30,8 @@ const (
 
 	// Version 1 was the initial release of storage.
 	// Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request.
-	authorizeCodeStorageVersion = "2"
+	// Version 3 is when we added the Username field to the psession.CustomSessionData.
+	authorizeCodeStorageVersion = "3"
 )
 
 var _ oauth2.AuthorizeCodeStorage = &authorizeCodeStorage{}
@@ -366,45 +367,43 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{
 				"Subject": "\u0026¥潝邎Ȗ莅ŝǔ盕戙鵮碡ʯiŬŽ"
 			},
 			"custom": {
-				"providerUID": "Ĝ眧Ĭ",
-				"providerName": "ʼn2ƋŢ觛ǂ焺nŐǛ",
-				"providerType": "ɥ闣ʬ橳(ý綃ʃʚƟ覣k眐4",
+				"username": "Ĝ眧Ĭ",
+				"providerUID": "ʼn2ƋŢ觛ǂ焺nŐǛ",
+				"providerName": "ɥ闣ʬ橳(ý綃ʃʚƟ覣k眐4",
+				"providerType": "ȣ掘ʃƸ澺淗a紽ǒ|鰽",
 				"warnings": [
-					"掘ʃƸ澺淗a紽ǒ|鰽ŋ猊",
-					"毇妬\u003e6鉢緋uƴŤȱʀļÂ?"
+					"t毇妬\u003e6鉢緋uƴŤȱʀļÂ",
+					"虝27就伒犘c钡ɏȫ齁š"
 				],
 				"oidc": {
-					"upstreamRefreshToken": "\u003cƬb",
-					"upstreamAccessToken": "犘c钡ɏȫ",
-					"upstreamSubject": "鬌",
-					"upstreamIssuer": "%OpKȱ藚ɏ¬Ê蒭堜"
+					"upstreamRefreshToken": "OpKȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ繕ȫ碰+ʫ",
+					"upstreamAccessToken": "k9帴",
+					"upstreamSubject": "磊ůď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ",
+					"upstreamIssuer": "4İ"
 				},
 				"ldap": {
-					"userDN": "ȗ韚ʫ繕ȫ碰+",
+					"userDN": "×",
 					"extraRefreshAttributes": {
-						"+î艔垎0": "ĝ",
-						"4İ": "墀jMʥ",
-						"k9帴": "磊ůď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ"
+						"ʥ笿0D": "s"
 					}
 				},
 				"activedirectory": {
-					"userDN": "%Ä摱ìÓȐĨf跞@)¿,ɭS隑i",
+					"userDN": "ĝ",
 					"extraRefreshAttributes": {
-						" 皦pSǬŝ社Vƅȭǝ*擦28Dž": "vư",
-						"艱iYn面@yȝƋ鬯犦獢9c5¤.岵": "浛a齙\\蹼偦歛"
+						"IȽ齤士bEǎ": "跞@)¿,ɭS隑ip偶宾儮猷V麹",
+						"ȝƋ鬯犦獢9c5¤.岵": "浛a齙\\蹼偦歛"
 					}
 				}
 			}
 		},
 		"requestedAudience": [
-			"置b",
-			"筫MN\u0026錝D肁Ŷɽ蔒PR}Ųʓl{"
+			" 皦pSǬŝ社Vƅȭǝ*擦28Dž",
+			"vư"
 		],
 		"grantedAudience": [
-			"jÃ轘屔挝",
-			"Œų崓ļ憽-蹐È_¸]fś",
-			"ɵʮGɃɫ囤1+,Ȳ"
+			"置b",
+			"筫MN\u0026錝D肁Ŷɽ蔒PR}Ųʓl{"
 		]
 	},
-	"version": "2"
+	"version": "3"
 }`

internal/fositestorage/authorizationcode/authorizationcode_test.go

@@ -65,7 +65,7 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/authcode",
@@ -84,7 +84,7 @@ func TestAuthorizationCodeStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/authcode",
@@ -202,7 +202,7 @@ func TestWrongVersion(t *testing.T) {
 
 	_, err = storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil)
 
-	require.EqualError(t, err, "authorization request data has wrong version: authorization code session for fancy-signature has version not-the-right-version instead of 2")
+	require.EqualError(t, err, "authorization request data has wrong version: authorization code session for fancy-signature has version not-the-right-version instead of 3")
 }
 
 func TestNilSessionRequest(t *testing.T) {
@@ -217,7 +217,7 @@ func TestNilSessionRequest(t *testing.T) {
 			},
 		},
 		Data: map[string][]byte{
-			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value", "version":"2", "active": true}`),
+			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value", "version":"3", "active": true}`),
 			"pinniped-storage-version": []byte("1"),
 		},
 		Type: "storage.pinniped.dev/authcode",
@@ -384,7 +384,7 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) {
 
 	// set these to match CreateAuthorizeCodeSession so that .JSONEq works
 	validSession.Active = true
-	validSession.Version = "2"
+	validSession.Version = "3"
 
 	validSessionJSONBytes, err := json.MarshalIndent(validSession, "", "\t")
 	require.NoError(t, err)
@@ -419,13 +419,13 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/authcode",
 			},
 			wantSession: &Session{
-				Version: "2",
+				Version: "3",
 				Active:  true,
 				Request: &fosite.Request{
 					ID:     "abcd-1",
@@ -436,6 +436,7 @@ func TestReadFromSecret(t *testing.T) {
 							Subject:  "panda",
 						},
 						Custom: &psession.CustomSessionData{
+							Username:     "fake-username",
 							ProviderUID:  "fake-provider-uid",
 							ProviderName: "fake-provider-name",
 							ProviderType: "fake-provider-type",
@@ -458,7 +459,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/not-authcode",
@@ -481,7 +482,7 @@ func TestReadFromSecret(t *testing.T) {
 				},
 				Type: "storage.pinniped.dev/authcode",
 			},
-			wantErr: "authorization request data has wrong version: authorization code session has version wrong-version-here instead of 2",
+			wantErr: "authorization request data has wrong version: authorization code session has version wrong-version-here instead of 3",
 		},
 		{
 			name: "missing request",
@@ -494,7 +495,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/authcode",

internal/fositestorage/openidconnect/openidconnect.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package openidconnect
@@ -30,7 +30,8 @@ const (
 
 	// Version 1 was the initial release of storage.
 	// Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request.
-	oidcStorageVersion = "2"
+	// Version 3 is when we added the Username field to the psession.CustomSessionData.
+	oidcStorageVersion = "3"
 )
 
 var _ openid.OpenIDConnectRequestStorage = &openIDConnectRequestStorage{}

internal/fositestorage/openidconnect/openidconnect_test.go

@@ -52,7 +52,7 @@ func TestOpenIdConnectStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/oidc",
@@ -137,7 +137,7 @@ func TestWrongVersion(t *testing.T) {
 
 	_, err = storage.GetOpenIDConnectSession(ctx, "fancy-code.fancy-signature", nil)
 
-	require.EqualError(t, err, "oidc request data has wrong version: oidc session for fancy-signature has version not-the-right-version instead of 2")
+	require.EqualError(t, err, "oidc request data has wrong version: oidc session for fancy-signature has version not-the-right-version instead of 3")
 }
 
 func TestNilSessionRequest(t *testing.T) {
@@ -152,7 +152,7 @@ func TestNilSessionRequest(t *testing.T) {
 			},
 		},
 		Data: map[string][]byte{
-			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"2"}`),
+			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"3"}`),
 			"pinniped-storage-version": []byte("1"),
 		},
 		Type: "storage.pinniped.dev/oidc",

internal/fositestorage/pkce/pkce.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package pkce
@@ -28,7 +28,8 @@ const (
 
 	// Version 1 was the initial release of storage.
 	// Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request.
-	pkceStorageVersion = "2"
+	// Version 3 is when we added the Username field to the psession.CustomSessionData.
+	pkceStorageVersion = "3"
 )
 
 var _ pkce.PKCERequestStorage = &pkceStorage{}

internal/fositestorage/pkce/pkce_test.go

@@ -52,7 +52,7 @@ func TestPKCEStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/pkce",
@@ -140,7 +140,7 @@ func TestWrongVersion(t *testing.T) {
 
 	_, err = storage.GetPKCERequestSession(ctx, "fancy-signature", nil)
 
-	require.EqualError(t, err, "pkce request data has wrong version: pkce session for fancy-signature has version not-the-right-version instead of 2")
+	require.EqualError(t, err, "pkce request data has wrong version: pkce session for fancy-signature has version not-the-right-version instead of 3")
 }
 
 func TestNilSessionRequest(t *testing.T) {
@@ -158,7 +158,7 @@ func TestNilSessionRequest(t *testing.T) {
 			},
 		},
 		Data: map[string][]byte{
-			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"2"}`),
+			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"3"}`),
 			"pinniped-storage-version": []byte("1"),
 		},
 		Type: "storage.pinniped.dev/pkce",

internal/fositestorage/refreshtoken/refreshtoken.go

@@ -29,7 +29,8 @@ const (
 
 	// Version 1 was the initial release of storage.
 	// Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request.
-	refreshTokenStorageVersion = "2"
+	// Version 3 is when we added the Username field to the psession.CustomSessionData.
+	refreshTokenStorageVersion = "3"
 )
 
 type RevocationStorage interface {

internal/fositestorage/refreshtoken/refreshtoken_test.go

@@ -52,7 +52,7 @@ func TestRefreshTokenStorage(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/refresh-token",
@@ -122,7 +122,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/refresh-token",
@@ -177,7 +177,7 @@ func TestRefreshTokenStorageRevokeRefreshTokenMaybeGracePeriod(t *testing.T) {
 				},
 			},
 			Data: map[string][]byte{
-				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`),
+				"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"3"}`),
 				"pinniped-storage-version": []byte("1"),
 			},
 			Type: "storage.pinniped.dev/refresh-token",
@@ -251,7 +251,7 @@ func TestWrongVersion(t *testing.T) {
 
 	_, err = storage.GetRefreshTokenSession(ctx, "fancy-signature", nil)
 
-	require.EqualError(t, err, "refresh token request data has wrong version: refresh token session for fancy-signature has version not-the-right-version instead of 2")
+	require.EqualError(t, err, "refresh token request data has wrong version: refresh token session for fancy-signature has version not-the-right-version instead of 3")
 }
 
 func TestNilSessionRequest(t *testing.T) {
@@ -269,7 +269,7 @@ func TestNilSessionRequest(t *testing.T) {
 			},
 		},
 		Data: map[string][]byte{
-			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"2"}`),
+			"pinniped-storage-data":    []byte(`{"nonsense-key": "nonsense-value","version":"3"}`),
 			"pinniped-storage-version": []byte("1"),
 		},
 		Type: "storage.pinniped.dev/refresh-token",
@@ -353,13 +353,13 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"username":"fake-username","providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/refresh-token",
 			},
 			wantSession: &Session{
-				Version: "2",
+				Version: "3",
 				Request: &fosite.Request{
 					ID:     "abcd-1",
 					Client: &clientregistry.Client{},
@@ -369,6 +369,7 @@ func TestReadFromSecret(t *testing.T) {
 							Subject:  "panda",
 						},
 						Custom: &psession.CustomSessionData{
+							Username:     "fake-username",
 							ProviderUID:  "fake-provider-uid",
 							ProviderName: "fake-provider-name",
 							ProviderType: "fake-provider-type",
@@ -391,7 +392,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"request":{"id":"abcd-1"},"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/not-refresh-token",
@@ -414,7 +415,7 @@ func TestReadFromSecret(t *testing.T) {
 				},
 				Type: "storage.pinniped.dev/refresh-token",
 			},
-			wantErr: "refresh token request data has wrong version: refresh token session has version wrong-version-here instead of 2",
+			wantErr: "refresh token request data has wrong version: refresh token session has version wrong-version-here instead of 3",
 		},
 		{
 			name: "missing request",
@@ -427,7 +428,7 @@ func TestReadFromSecret(t *testing.T) {
 					},
 				},
 				Data: map[string][]byte{
-					"pinniped-storage-data":    []byte(`{"version":"2","active": true}`),
+					"pinniped-storage-data":    []byte(`{"version":"3","active": true}`),
 					"pinniped-storage-version": []byte("1"),
 				},
 				Type: "storage.pinniped.dev/refresh-token",

internal/oidc/auth/auth_handler.go

@@ -10,17 +10,15 @@ import (
 	"net/url"
 	"time"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/ory/fosite/token/jwt"
 	"golang.org/x/oauth2"
 
-	supervisoroidc "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"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"
@@ -56,12 +54,12 @@ func NewHandler(
 			return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method)
 		}
 
-		// Note that the client might have used supervisoroidc.AuthorizeUpstreamIDPNameParamName and
-		// supervisoroidc.AuthorizeUpstreamIDPTypeParamName query params to request a certain upstream IDP.
+		// Note that the client might have used oidcapi.AuthorizeUpstreamIDPNameParamName and
+		// oidcapi.AuthorizeUpstreamIDPTypeParamName query params to request a certain upstream IDP.
 		// The Pinniped CLI has been sending these params since v0.9.0.
 		// Currently, these are ignored because the Supervisor does not yet support logins when multiple IDPs
 		// are configured. However, these params should be honored in the future when choosing an upstream
-		// here, e.g. by calling supervisoroidc.FindUpstreamIDPByNameAndType() when the params are present.
+		// here, e.g. by calling oidcapi.FindUpstreamIDPByNameAndType() when the params are present.
 		oidcUpstream, ldapUpstream, idpType, err := chooseUpstreamIDP(idpLister)
 		if err != nil {
 			plog.WarningErr("authorize upstream config", err)
@@ -69,8 +67,8 @@ func NewHandler(
 		}
 
 		if idpType == psession.ProviderTypeOIDC {
-			if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 ||
-				len(r.Header.Values(supervisoroidc.AuthorizePasswordHeaderName)) > 0 {
+			if len(r.Header.Values(oidcapi.AuthorizeUsernameHeaderName)) > 0 ||
+				len(r.Header.Values(oidcapi.AuthorizePasswordHeaderName)) > 0 {
 				// The client set a username header, so they are trying to log in with a username/password.
 				return handleAuthRequestForOIDCUpstreamPasswordGrant(r, w, oauthHelperWithStorage, oidcUpstream)
 			}
@@ -85,8 +83,8 @@ func NewHandler(
 		}
 
 		// We know it's an AD/LDAP upstream.
-		if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 ||
-			len(r.Header.Values(supervisoroidc.AuthorizePasswordHeaderName)) > 0 {
+		if len(r.Header.Values(oidcapi.AuthorizeUsernameHeaderName)) > 0 ||
+			len(r.Header.Values(oidcapi.AuthorizePasswordHeaderName)) > 0 {
 			// The client set a username header, so they are trying to log in with a username/password.
 			return handleAuthRequestForLDAPUpstreamCLIFlow(r, w,
 				oauthHelperWithStorage,
@@ -150,7 +148,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow(
 	subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse)
 	username = authenticateResponse.User.GetName()
 	groups := authenticateResponse.User.GetGroups()
-	customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse)
+	customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username)
 	openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData)
 	oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true)
 
@@ -244,7 +242,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
 		return nil
 	}
 
-	customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token)
+	customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token, username)
 	if err != nil {
 		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true,
@@ -322,7 +320,7 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow(
 }
 
 func requireStaticClientForUsernameAndPasswordHeaders(w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) bool {
-	isStaticClient := authorizeRequester.GetClient().GetID() == clientregistry.PinnipedCLIClientID
+	isStaticClient := authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI
 	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)
@@ -331,8 +329,8 @@ func requireStaticClientForUsernameAndPasswordHeaders(w http.ResponseWriter, oau
 }
 
 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)
+	username := r.Header.Get(oidcapi.AuthorizeUsernameHeaderName)
+	password := r.Header.Get(oidcapi.AuthorizePasswordHeaderName)
 	if username == "" || password == "" {
 		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester,
 			fosite.ErrAccessDenied.WithHintf("Missing or blank username or password."), true)
@@ -348,13 +346,13 @@ func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fos
 		return nil, false
 	}
 
-	// Automatically grant the openid, offline_access, pinniped:request-audience, and groups scopes, but only if they were requested.
+	// Automatically grant certain 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})
+	downstreamsession.AutoApproveScopes(authorizeRequester)
 
 	return authorizeRequester, true
 }
@@ -487,7 +485,7 @@ func handleBrowserFlowAuthRequest(
 	}
 
 	promptParam := r.Form.Get(promptParamName)
-	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
+	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, oidcapi.ScopeOpenID) {
 		oidc.WriteAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
 		return nil, nil // already wrote the error response, don't return error
 	}
@@ -538,8 +536,8 @@ func upstreamStateParam(
 	encoder oidc.Encoder,
 ) (string, error) {
 	stateParamData := oidc.UpstreamStateParamData{
-		// The auth params might have included supervisoroidc.AuthorizeUpstreamIDPNameParamName and
-		// supervisoroidc.AuthorizeUpstreamIDPTypeParamName, but those can be ignored by other handlers
+		// The auth params might have included oidcapi.AuthorizeUpstreamIDPNameParamName and
+		// oidcapi.AuthorizeUpstreamIDPTypeParamName, but those can be ignored by other handlers
 		// that are reading from the encoded upstream state param being built here.
 		// The UpstreamName and UpstreamType struct fields can be used instead.
 		// Remove those params here to avoid potential confusion about which should be used later.
@@ -565,8 +563,8 @@ func removeCustomIDPParams(params url.Values) url.Values {
 		p[k] = v
 	}
 	// Remove the unnecessary params.
-	delete(p, supervisoroidc.AuthorizeUpstreamIDPNameParamName)
-	delete(p, supervisoroidc.AuthorizeUpstreamIDPTypeParamName)
+	delete(p, oidcapi.AuthorizeUpstreamIDPNameParamName)
+	delete(p, oidcapi.AuthorizeUpstreamIDPTypeParamName)
 	return p
 }
 

internal/oidc/auth/auth_handler_test.go

@@ -34,6 +34,7 @@ import (
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
@@ -391,8 +392,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		return urlToReturn
 	}
 
-	happyDownstreamScopesRequested := []string{"openid", "profile", "email", "groups"}
-	happyDownstreamScopesGranted := []string{"openid", "groups"}
+	happyDownstreamScopesRequested := []string{"openid", "profile", "email", "username", "groups"}
+	happyDownstreamScopesGranted := []string{"openid", "username", "groups"}
 
 	happyGetRequestQueryMap := map[string]string{
 		"response_type":         "code",
@@ -465,6 +466,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	}
 
 	expectedHappyActiveDirectoryUpstreamCustomSession := &psession.CustomSessionData{
+		Username:     happyLDAPUsernameFromAuthenticator,
 		ProviderUID:  activeDirectoryUpstreamResourceUID,
 		ProviderName: activeDirectoryUpstreamName,
 		ProviderType: psession.ProviderTypeActiveDirectory,
@@ -477,6 +479,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	}
 
 	expectedHappyLDAPUpstreamCustomSession := &psession.CustomSessionData{
+		Username:     happyLDAPUsernameFromAuthenticator,
 		ProviderUID:  ldapUpstreamResourceUID,
 		ProviderName: ldapUpstreamName,
 		ProviderType: psession.ProviderTypeLDAP,
@@ -489,6 +492,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 	}
 
 	expectedHappyOIDCPasswordGrantCustomSession := &psession.CustomSessionData{
+		Username:     oidcUpstreamUsername,
 		ProviderUID:  oidcPasswordGrantUpstreamResourceUID,
 		ProviderName: oidcPasswordGrantUpstreamName,
 		ProviderType: psession.ProviderTypeOIDC,
@@ -499,7 +503,16 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		},
 	}
 
+	expectedHappyOIDCPasswordGrantCustomSessionWithUsername := func(wantUsername string) *psession.CustomSessionData {
+		copyOfCustomSession := *expectedHappyOIDCPasswordGrantCustomSession
+		copyOfOIDC := *(expectedHappyOIDCPasswordGrantCustomSession.OIDC)
+		copyOfCustomSession.OIDC = &copyOfOIDC
+		copyOfCustomSession.Username = wantUsername
+		return &copyOfCustomSession
+	}
+
 	expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken := &psession.CustomSessionData{
+		Username:     oidcUpstreamUsername,
 		ProviderUID:  oidcPasswordGrantUpstreamResourceUID,
 		ProviderName: oidcPasswordGrantUpstreamName,
 		ProviderType: psession.ProviderTypeOIDC,
@@ -512,13 +525,14 @@ 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})
+			"some-namespace", dynamicClientID, dynamicClientUID, downstreamRedirectURI,
+			[]string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
 		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
+	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyState
 
 	incomingCookieCSRFValue := "csrf-value-from-cookie"
 	encodedIncomingCookieCSRFValue, err := happyCookieEncoder.Encode("csrf", incomingCookieCSRFValue)
@@ -528,6 +542,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 		name string
 
 		idps                 *oidctestutil.UpstreamIDPListerBuilder
+		kubeResources        func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset)
 		generateCSRF         func() (csrftoken.CSRFToken, error)
 		generatePKCE         func() (pkce.Code, error)
 		generateNonce        func() (nonce.Nonce, error)
@@ -540,7 +555,6 @@ 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
@@ -1122,7 +1136,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantPasswordGrantCall:             happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                        http.StatusFound,
 			wantContentType:                   htmlContentType,
-			wantRedirectLocationRegexp:        downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState,
+			wantRedirectLocationRegexp:        downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyState,
 			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
 			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
 			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
@@ -1145,7 +1159,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:              pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                        http.StatusFound,
 			wantContentType:                   htmlContentType,
-			wantRedirectLocationRegexp:        downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+groups&state=` + happyState,
+			wantRedirectLocationRegexp:        downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
 			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
 			wantDownstreamIDTokenGroups:       happyLDAPGroups,
@@ -1219,6 +1233,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData: &psession.CustomSessionData{
+				Username:     oidcUpstreamUsername,
 				ProviderUID:  oidcPasswordGrantUpstreamResourceUID,
 				ProviderName: oidcPasswordGrantUpstreamName,
 				ProviderType: psession.ProviderTypeOIDC,
@@ -2373,13 +2388,13 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantPasswordGrantCall:             happyUpstreamPasswordGrantMockExpectation,
 			wantStatus:                        http.StatusFound,
 			wantContentType:                   htmlContentType,
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyState, // no scopes granted
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=username\+groups&state=` + happyState, // username and groups scopes were not requested, but are granted anyway for backwards compatibility
 			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
-			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
-			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
-			wantDownstreamRequestedScopes:     []string{"email"}, // only email was requested
+			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,        // username scope was not requested, but is granted anyway for backwards compatibility
+			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership, // groups scope was not requested, but is granted anyway for backwards compatibility
+			wantDownstreamRequestedScopes:     []string{"email"},           // only email was requested
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
-			wantDownstreamGrantedScopes:       []string{}, // no scopes granted
+			wantDownstreamGrantedScopes:       []string{"username", "groups"}, // username and groups scopes were not requested, but are granted anyway for backwards compatibility
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
@@ -2395,13 +2410,13 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			customPasswordHeader:              pointer.StringPtr(happyLDAPPassword),
 			wantStatus:                        http.StatusFound,
 			wantContentType:                   htmlContentType,
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyState, // no scopes granted
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=username\+groups&state=` + happyState, // username and groups scopes were not requested, but are granted anyway for backwards compatibility
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
-			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
-			wantDownstreamIDTokenGroups:       happyLDAPGroups,
-			wantDownstreamRequestedScopes:     []string{"email"}, // only email was requested
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator, // username scope was not requested, but is granted anyway for backwards compatibility
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,                    // groups scope was not requested, but is granted anyway for backwards compatibility
+			wantDownstreamRequestedScopes:     []string{"email"},                  // only email was requested
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
-			wantDownstreamGrantedScopes:       []string{}, // no scopes granted
+			wantDownstreamGrantedScopes:       []string{"username", "groups"}, // username and groups scopes were not requested, but are granted anyway for backwards compatibility
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
@@ -2429,7 +2444,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
+			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithUsername(oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped),
 		},
 		{
 			name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is missing",
@@ -2455,7 +2470,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
+			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithUsername("joe@whitehouse.gov"),
 		},
 		{
 			name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with true value",
@@ -2482,7 +2497,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
+			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithUsername("joe@whitehouse.gov"),
 		},
 		{
 			name: "OIDC upstream password grant: upstream IDP configures username claim as anything other than special claim `email` and `email_verified` upstream claim is present with false value",
@@ -2510,7 +2525,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
+			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithUsername("joe"),
 		},
 		{
 			name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with illegal value",
@@ -2570,7 +2585,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSession,
+			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithUsername(oidcUpstreamSubject),
 		},
 		{
 			name: "OIDC upstream password grant: upstream IDP's configured groups claim in the ID token has a non-array value",

internal/oidc/callback/callback_handler.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"net/url"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
 
 	"go.pinniped.dev/internal/httputil/httperr"
@@ -53,10 +52,10 @@ func NewHandler(
 			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.
+		// Automatically grant certain 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})
+		downstreamsession.AutoApproveScopes(authorizeRequester)
 
 		token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens(
 			r.Context(),
@@ -75,7 +74,7 @@ func NewHandler(
 			return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err)
 		}
 
-		customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(upstreamIDPConfig, token)
+		customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(upstreamIDPConfig, token, username)
 		if err != nil {
 			return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err)
 		}

internal/oidc/callback/callback_handler_test.go

@@ -19,9 +19,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
 
+	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"
 	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/oidctestutil"
@@ -66,8 +68,8 @@ const (
 
 var (
 	oidcUpstreamGroupMembership    = []string{"test-pinniped-group-0", "test-pinniped-group-1"}
-	happyDownstreamScopesRequested = []string{"openid", "groups"}
-	happyDownstreamScopesGranted   = []string{"openid", "groups"}
+	happyDownstreamScopesRequested = []string{"openid", "username", "groups"}
+	happyDownstreamScopesGranted   = []string{"openid", "username", "groups"}
 
 	happyDownstreamRequestParamsQuery = url.Values{
 		"response_type":         []string{"code"},
@@ -81,11 +83,13 @@ var (
 	}
 	happyDownstreamRequestParams = happyDownstreamRequestParamsQuery.Encode()
 
-	happyDownstreamRequestParamsForDynamicClient = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+	happyDownstreamRequestParamsQueryForDynamicClient = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
 		map[string]string{"client_id": downstreamDynamicClientID},
-	).Encode()
+	)
+	happyDownstreamRequestParamsForDynamicClient = happyDownstreamRequestParamsQueryForDynamicClient.Encode()
 
 	happyDownstreamCustomSessionData = &psession.CustomSessionData{
+		Username:     oidcUpstreamUsername,
 		ProviderUID:  happyUpstreamIDPResourceUID,
 		ProviderName: happyUpstreamIDPName,
 		ProviderType: psession.ProviderTypeOIDC,
@@ -95,7 +99,15 @@ var (
 			UpstreamSubject:      oidcUpstreamSubject,
 		},
 	}
+	happyDownstreamCustomSessionDataWithUsername = func(wantUsername string) *psession.CustomSessionData {
+		copyOfCustomSession := *happyDownstreamCustomSessionData
+		copyOfOIDC := *(happyDownstreamCustomSessionData.OIDC)
+		copyOfCustomSession.OIDC = &copyOfOIDC
+		copyOfCustomSession.Username = wantUsername
+		return &copyOfCustomSession
+	}
 	happyDownstreamAccessTokenCustomSessionData = &psession.CustomSessionData{
+		Username:     oidcUpstreamUsername,
 		ProviderUID:  happyUpstreamIDPResourceUID,
 		ProviderName: happyUpstreamIDPName,
 		ProviderType: psession.ProviderTypeOIDC,
@@ -143,11 +155,12 @@ 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
+	happyDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+username\+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})
+			"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, downstreamRedirectURI,
+			[]string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
 		require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
 		require.NoError(t, kubeClient.Tracker().Add(secret))
 	}
@@ -284,7 +297,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			},
 		},
 		{
-			name:   "form_post happy path with no groups scope requested",
+			name:   "form_post happy path without username or groups scopes requested",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
 			method: http.MethodGet,
 			path: newRequestPath().WithState(
@@ -298,14 +311,16 @@ func TestCallbackEndpoint(t *testing.T) {
 					).Encode(),
 				).Build(t, happyStateCodec),
 			).String(),
-			csrfCookie:                        happyCSRFCookie,
-			wantStatus:                        http.StatusOK,
-			wantContentType:                   "text/html;charset=UTF-8",
-			wantBodyFormResponseRegexp:        `<code id="manual-auth-code">(.+)</code>`,
-			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
-			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
-			wantDownstreamRequestedScopes:     []string{"openid"},
-			wantDownstreamGrantedScopes:       []string{"openid"},
+			csrfCookie:                    happyCSRFCookie,
+			wantStatus:                    http.StatusOK,
+			wantContentType:               "text/html;charset=UTF-8",
+			wantBodyFormResponseRegexp:    `<code id="manual-auth-code">(.+)</code>`,
+			wantDownstreamIDTokenSubject:  oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername: oidcUpstreamUsername,
+			wantDownstreamRequestedScopes: []string{"openid"},
+			wantDownstreamIDTokenGroups:   oidcUpstreamGroupMembership,
+			// username and groups scopes were not requested but are granted anyway for the pinniped-cli client for backwards compatibility
+			wantDownstreamGrantedScopes:       []string{"openid", "username", "groups"},
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
@@ -335,6 +350,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData: &psession.CustomSessionData{
+				Username:     oidcUpstreamUsername,
 				ProviderUID:  happyUpstreamIDPResourceUID,
 				ProviderName: happyUpstreamIDPName,
 				ProviderType: psession.ProviderTypeOIDC,
@@ -370,7 +386,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionDataWithUsername(oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped),
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -396,7 +412,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionDataWithUsername("joe@whitehouse.gov"),
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -424,7 +440,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionDataWithUsername("joe@whitehouse.gov"),
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -453,7 +469,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionDataWithUsername("joe"),
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -584,7 +600,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
-			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionDataWithUsername(oidcUpstreamSubject),
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -642,6 +658,152 @@ func TestCallbackEndpoint(t *testing.T) {
 				args:                    happyExchangeAndValidateTokensArgs,
 			},
 		},
+		{
+			name:          "using dynamic client which is allowed to request username scope, but does not actually request username scope in authorize request, does not get username in ID token",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:        http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParamForDynamicClient().
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+						map[string]string{"scope": "openid groups offline_access"}).Encode()).
+					Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:                        happyCSRFCookie,
+			wantStatus:                        http.StatusSeeOther,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+groups&state=` + happyDownstreamState,
+			wantBody:                          "",
+			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername:     "", // username scope was not requested
+			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
+			wantDownstreamRequestedScopes:     []string{"openid", "groups", "offline_access"},
+			wantDownstreamGrantedScopes:       []string{"openid", "groups", "offline_access"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name:          "using dynamic client which is allowed to request groups scope, but does not actually request groups scope in authorize request, does not get groups in ID token",
+			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
+			method:        http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParamForDynamicClient().
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+						map[string]string{"scope": "openid username offline_access"}).Encode()).
+					Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:                        happyCSRFCookie,
+			wantStatus:                        http.StatusSeeOther,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+username&state=` + happyDownstreamState,
+			wantBody:                          "",
+			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
+			wantDownstreamIDTokenGroups:       nil, // groups scope was not requested
+			wantDownstreamRequestedScopes:     []string{"openid", "username", "offline_access"},
+			wantDownstreamGrantedScopes:       []string{"openid", "username", "offline_access"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "using dynamic client which is not allowed to request username scope, and does not actually request username scope in authorize request, does not get username in ID token",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude username scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "groups"},      // username not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithAuthorizeRequestParams(
+					shallowCopyAndModifyQuery(
+						happyDownstreamRequestParamsQuery,
+						map[string]string{
+							"client_id": downstreamDynamicClientID,
+							"scope":     "openid offline_access groups",
+						},
+					).Encode(),
+				).Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:                        happyCSRFCookie,
+			wantStatus:                        http.StatusSeeOther,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+groups&state=` + happyDownstreamState,
+			wantBody:                          "",
+			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername:     "", // username scope was not requested
+			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
+			wantDownstreamRequestedScopes:     []string{"openid", "groups", "offline_access"},
+			wantDownstreamGrantedScopes:       []string{"openid", "groups", "offline_access"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "using dynamic client which is not allowed to request groups scope, and does not actually request groups scope in authorize request, does not get groups in ID token",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude groups scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "username"},    // groups not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithAuthorizeRequestParams(
+					shallowCopyAndModifyQuery(
+						happyDownstreamRequestParamsQuery,
+						map[string]string{
+							"client_id": downstreamDynamicClientID,
+							"scope":     "openid offline_access username",
+						},
+					).Encode(),
+				).Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:                        happyCSRFCookie,
+			wantStatus:                        http.StatusSeeOther,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+username&state=` + happyDownstreamState,
+			wantBody:                          "",
+			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
+			wantDownstreamIDTokenGroups:       nil, // groups scope was not requested
+			wantDownstreamRequestedScopes:     []string{"openid", "username", "offline_access"},
+			wantDownstreamGrantedScopes:       []string{"openid", "username", "offline_access"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClientID:            downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   happyDownstreamCustomSessionData,
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
 
 		// Pre-upstream-exchange verification
 		{
@@ -718,7 +880,8 @@ func TestCallbackEndpoint(t *testing.T) {
 			method: http.MethodGet,
 			path: newRequestPath().WithState(
 				happyUpstreamStateParam().
-					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"prompt": "none login"}).Encode()).
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+						map[string]string{"prompt": "none login"}).Encode()).
 					Build(t, happyStateCodec),
 			).String(),
 			csrfCookie: happyCSRFCookie,
@@ -759,7 +922,8 @@ func TestCallbackEndpoint(t *testing.T) {
 			method: http.MethodGet,
 			path: newRequestPath().WithState(
 				happyUpstreamStateParam().
-					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": ""}).Encode()).
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+						map[string]string{"client_id": ""}).Encode()).
 					Build(t, happyStateCodec),
 			).String(),
 			csrfCookie:      happyCSRFCookie,
@@ -773,7 +937,8 @@ func TestCallbackEndpoint(t *testing.T) {
 			method: http.MethodGet,
 			path: newRequestPath().WithState(
 				happyUpstreamStateParam().
-					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": "bogus"}).Encode()).
+					WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+						map[string]string{"client_id": "bogus"}).Encode()).
 					Build(t, happyStateCodec),
 			).String(),
 			csrfCookie:      happyCSRFCookie,
@@ -803,6 +968,64 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantContentType: htmlContentType,
 			wantBody:        "Bad Request: error using state downstream auth params\n",
 		},
+		{
+			name: "using dynamic client which is not allowed to request username scope in authorize request but requests it anyway",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude username scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "groups"},      // username not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithAuthorizeRequestParams(
+					shallowCopyAndModifyQuery(
+						happyDownstreamRequestParamsQuery,
+						map[string]string{
+							"client_id": downstreamDynamicClientID,
+							"scope":     "openid username",
+						},
+					).Encode(),
+				).Build(t, happyStateCodec),
+			).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusBadRequest,
+			wantContentType: htmlContentType,
+			wantBody:        "Bad Request: error using state downstream auth params\n",
+		},
+		{
+			name: "using dynamic client which is not allowed to request groups scope in authorize request but requests it anyway",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude groups scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "username"},    // groups not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			method: http.MethodGet,
+			path: newRequestPath().WithState(
+				happyUpstreamStateParam().WithAuthorizeRequestParams(
+					shallowCopyAndModifyQuery(
+						happyDownstreamRequestParamsQuery,
+						map[string]string{
+							"client_id": downstreamDynamicClientID,
+							"scope":     "openid groups",
+						},
+					).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()),
@@ -810,16 +1033,17 @@ func TestCallbackEndpoint(t *testing.T) {
 			path: newRequestPath().
 				WithState(
 					happyUpstreamStateParam().
-						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "profile email groups"}).Encode()).
+						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+							map[string]string{"scope": "profile username email groups"}).Encode()).
 						Build(t, happyStateCodec),
 				).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusSeeOther,
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=groups&state=` + happyDownstreamState,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
 			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
-			wantDownstreamRequestedScopes:     []string{"profile", "email", "groups"},
-			wantDownstreamGrantedScopes:       []string{"groups"},
+			wantDownstreamRequestedScopes:     []string{"profile", "email", "username", "groups"},
+			wantDownstreamGrantedScopes:       []string{"username", "groups"},
 			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClientID:            downstreamPinnipedClientID,
@@ -832,22 +1056,25 @@ func TestCallbackEndpoint(t *testing.T) {
 			},
 		},
 		{
-			name:   "state's downstream auth params does not contain openid or groups scope",
+			name:   "state's downstream auth params does not contain openid, username, or groups scope",
 			idps:   oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()),
 			method: http.MethodGet,
 			path: newRequestPath().
 				WithState(
 					happyUpstreamStateParam().
-						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "profile email"}).Encode()).
+						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+							map[string]string{"scope": "profile email"}).Encode()).
 						Build(t, happyStateCodec),
 				).String(),
-			csrfCookie:                        happyCSRFCookie,
-			wantStatus:                        http.StatusSeeOther,
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyDownstreamState,
-			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
-			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
-			wantDownstreamRequestedScopes:     []string{"profile", "email"},
-			wantDownstreamGrantedScopes:       []string{},
+			csrfCookie:                    happyCSRFCookie,
+			wantStatus:                    http.StatusSeeOther,
+			wantRedirectLocationRegexp:    downstreamRedirectURI + `\?code=([^&]+)&scope=username\+groups&state=` + happyDownstreamState,
+			wantDownstreamIDTokenUsername: oidcUpstreamUsername,
+			wantDownstreamIDTokenGroups:   oidcUpstreamGroupMembership,
+			wantDownstreamIDTokenSubject:  oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
+			wantDownstreamRequestedScopes: []string{"profile", "email"},
+			// username and groups scopes were not requested but are granted anyway for the pinniped-cli client for backwards compatibility
+			wantDownstreamGrantedScopes:       []string{"username", "groups"},
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClientID:            downstreamPinnipedClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
@@ -865,16 +1092,17 @@ func TestCallbackEndpoint(t *testing.T) {
 			path: newRequestPath().
 				WithState(
 					happyUpstreamStateParam().
-						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access groups"}).Encode()).
+						WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery,
+							map[string]string{"scope": "openid offline_access username groups"}).Encode()).
 						Build(t, happyStateCodec),
 				).String(),
 			csrfCookie:                        happyCSRFCookie,
 			wantStatus:                        http.StatusSeeOther,
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+groups&state=` + happyDownstreamState,
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenUsername:     oidcUpstreamUsername,
 			wantDownstreamIDTokenSubject:      oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped,
-			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "groups"},
-			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "groups"},
+			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "username", "groups"},
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "username", "groups"},
 			wantDownstreamIDTokenGroups:       oidcUpstreamGroupMembership,
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClientID:            downstreamPinnipedClientID,

internal/oidc/clientregistry/clientregistry.go

@@ -10,25 +10,19 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coreos/go-oidc/v3/oidc"
+	coreosoidc "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"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	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"
 )
 
-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 {
@@ -43,7 +37,7 @@ var (
 )
 
 func (c *Client) GetResponseModes() []fosite.ResponseModeType {
-	if c.ID == PinnipedCLIClientID {
+	if c.ID == oidcapi.ClientIDPinnipedCLI {
 		// The pinniped-cli client supports "" (unspecified), "query", and "form_post" response modes.
 		return []fosite.ResponseModeType{fosite.ResponseModeDefault, fosite.ResponseModeQuery, fosite.ResponseModeFormPost}
 	}
@@ -78,12 +72,12 @@ func NewClientManager(
 // Other errors returned are plain errors, because fosite will wrap them into a new ErrInvalidClient error and
 // use the plain error's text as that error's debug message (see client_authentication.go in fosite).
 func (m *ClientManager) GetClient(ctx context.Context, id string) (fosite.Client, error) {
-	if id == PinnipedCLIClientID {
+	if id == oidcapi.ClientIDPinnipedCLI {
 		// Return the static client. No lookups needed.
 		return PinnipedCLI(), nil
 	}
 
-	if !strings.HasPrefix(id, requiredOIDCClientPrefix) {
+	if !strings.HasPrefix(id, oidcapi.ClientIDRequiredOIDCClientPrefix) {
 		// 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")
@@ -143,22 +137,23 @@ func PinnipedCLI() *Client {
 	return &Client{
 		DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{
 			DefaultClient: &fosite.DefaultClient{
-				ID:           PinnipedCLIClientID,
+				ID:           oidcapi.ClientIDPinnipedCLI,
 				Secret:       nil,
 				RedirectURIs: []string{"http://127.0.0.1/callback"},
 				GrantTypes: fosite.Arguments{
-					"authorization_code",
-					"refresh_token",
-					"urn:ietf:params:oauth:grant-type:token-exchange",
+					oidcapi.GrantTypeAuthorizationCode,
+					oidcapi.GrantTypeRefreshToken,
+					oidcapi.GrantTypeTokenExchange,
 				},
 				ResponseTypes: []string{"code"},
 				Scopes: fosite.Arguments{
-					oidc.ScopeOpenID,
-					oidc.ScopeOfflineAccess,
-					"profile",
-					"email",
-					"pinniped:request-audience",
-					"groups",
+					oidcapi.ScopeOpenID,
+					oidcapi.ScopeOfflineAccess,
+					oidcapi.ScopeProfile,
+					oidcapi.ScopeEmail,
+					oidcapi.ScopeRequestAudience,
+					oidcapi.ScopeUsername,
+					oidcapi.ScopeGroups,
 				},
 				Audience: nil,
 				Public:   true,
@@ -167,7 +162,7 @@ func PinnipedCLI() *Client {
 			JSONWebKeys:                       nil,
 			JSONWebKeysURI:                    "",
 			RequestObjectSigningAlgorithm:     "",
-			TokenEndpointAuthSigningAlgorithm: oidc.RS256,
+			TokenEndpointAuthSigningAlgorithm: coreosoidc.RS256,
 			TokenEndpointAuthMethod:           "none",
 		},
 	}
@@ -194,7 +189,7 @@ func oidcClientCRToFositeClient(oidcClient *configv1alpha1.OIDCClient, clientSec
 			JSONWebKeys:                       nil,
 			JSONWebKeysURI:                    "",
 			RequestObjectSigningAlgorithm:     "",
-			TokenEndpointAuthSigningAlgorithm: oidc.RS256,
+			TokenEndpointAuthSigningAlgorithm: coreosoidc.RS256,
 			TokenEndpointAuthMethod:           "client_secret_basic",
 		},
 	}

internal/oidc/clientregistry/clientregistry_test.go

@@ -269,7 +269,7 @@ func requireEqualsPinnipedCLI(t *testing.T, c *Client) {
 	require.Equal(t, []string{"http://127.0.0.1/callback"}, c.GetRedirectURIs())
 	require.Equal(t, fosite.Arguments{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, c.GetGrantTypes())
 	require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes())
-	require.Equal(t, fosite.Arguments{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience", "groups"}, c.GetScopes())
+	require.Equal(t, fosite.Arguments{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience", "username", "groups"}, c.GetScopes())
 	require.True(t, c.IsPublic())
 	require.Nil(t, c.GetAudience())
 	require.Nil(t, c.GetRequestURIs())
@@ -302,6 +302,7 @@ func requireEqualsPinnipedCLI(t *testing.T, c *Client) {
 			"profile",
 			"email",
 			"pinniped:request-audience",
+			"username",
 			"groups"
 		  ],
 		  "audience": null,

internal/oidc/discovery/discovery_handler.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/oidc"
 )
 
@@ -68,8 +69,8 @@ func NewHandler(issuerURL string) http.Handler {
 		IDTokenSigningAlgValuesSupported:  []string{"ES256"},
 		TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"},
 		CodeChallengeMethodsSupported:     []string{"S256"},
-		ScopesSupported:                   []string{"openid", "offline"},
-		ClaimsSupported:                   []string{"groups"},
+		ScopesSupported:                   []string{oidcapi.ScopeOpenID, oidcapi.ScopeOfflineAccess, oidcapi.ScopeRequestAudience, oidcapi.ScopeUsername, oidcapi.ScopeGroups},
+		ClaimsSupported:                   []string{oidcapi.IDTokenClaimUsername, oidcapi.IDTokenClaimGroups},
 	}
 
 	var b bytes.Buffer

internal/oidc/discovery/discovery_handler_test.go

@@ -45,9 +45,9 @@ func TestDiscovery(t *testing.T) {
 				"subject_types_supported": ["public"],
 				"id_token_signing_alg_values_supported": ["ES256"],
 				"token_endpoint_auth_methods_supported": ["client_secret_basic"],
-				"scopes_supported": ["openid", "offline"],
+				"scopes_supported": ["openid", "offline_access", "pinniped:request-audience", "username", "groups"],
 				"code_challenge_methods_supported": ["S256"],
-				"claims_supported": ["groups"],
+				"claims_supported": ["username", "groups"],
 				"discovery.supervisor.pinniped.dev/v1alpha1": {
 					"pinniped_identity_providers_endpoint": "https://some-issuer.com/some/path/v1alpha1/pinniped_identity_providers"
 				}

internal/oidc/downstreamsession/downstream_session.go

@@ -16,6 +16,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/strings/slices"
 
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/constable"
 	"go.pinniped.dev/internal/oidc"
@@ -27,7 +28,7 @@ import (
 
 const (
 	// The name of the email claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
-	emailClaimName = "email"
+	emailClaimName = oidcapi.ScopeEmail
 
 	// The name of the email_verified claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
 	emailVerifiedClaimName = "email_verified"
@@ -55,11 +56,12 @@ func MakeDownstreamSession(subject string, username string, groups []string, gra
 	if groups == nil {
 		groups = []string{}
 	}
-	openIDSession.IDTokenClaims().Extra = map[string]interface{}{
-		oidc.DownstreamUsernameClaim: username,
+	openIDSession.IDTokenClaims().Extra = map[string]interface{}{}
+	if slices.Contains(grantedScopes, oidcapi.ScopeUsername) {
+		openIDSession.IDTokenClaims().Extra[oidcapi.IDTokenClaimUsername] = username
 	}
-	if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) {
-		openIDSession.IDTokenClaims().Extra[oidc.DownstreamGroupsClaim] = groups
+	if slices.Contains(grantedScopes, oidcapi.ScopeGroups) {
+		openIDSession.IDTokenClaims().Extra[oidcapi.IDTokenClaimGroups] = groups
 	}
 	return openIDSession
 }
@@ -68,8 +70,10 @@ func MakeDownstreamLDAPOrADCustomSessionData(
 	ldapUpstream provider.UpstreamLDAPIdentityProviderI,
 	idpType psession.ProviderType,
 	authenticateResponse *authenticators.Response,
+	username string,
 ) *psession.CustomSessionData {
 	customSessionData := &psession.CustomSessionData{
+		Username:     username,
 		ProviderUID:  ldapUpstream.GetResourceUID(),
 		ProviderName: ldapUpstream.GetName(),
 		ProviderType: idpType,
@@ -92,17 +96,22 @@ func MakeDownstreamLDAPOrADCustomSessionData(
 	return customSessionData
 }
 
-func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdentityProviderI, token *oidctypes.Token) (*psession.CustomSessionData, error) {
-	upstreamSubject, err := ExtractStringClaimValue(oidc.IDTokenSubjectClaim, oidcUpstream.GetName(), token.IDToken.Claims)
+func MakeDownstreamOIDCCustomSessionData(
+	oidcUpstream provider.UpstreamOIDCIdentityProviderI,
+	token *oidctypes.Token,
+	username string,
+) (*psession.CustomSessionData, error) {
+	upstreamSubject, err := ExtractStringClaimValue(oidcapi.IDTokenClaimSubject, oidcUpstream.GetName(), token.IDToken.Claims)
 	if err != nil {
 		return nil, err
 	}
-	upstreamIssuer, err := ExtractStringClaimValue(oidc.IDTokenIssuerClaim, oidcUpstream.GetName(), token.IDToken.Claims)
+	upstreamIssuer, err := ExtractStringClaimValue(oidcapi.IDTokenClaimIssuer, oidcUpstream.GetName(), token.IDToken.Claims)
 	if err != nil {
 		return nil, err
 	}
 
 	customSessionData := &psession.CustomSessionData{
+		Username:     username,
 		ProviderUID:  oidcUpstream.GetResourceUID(),
 		ProviderName: oidcUpstream.GetName(),
 		ProviderType: psession.ProviderTypeOIDC,
@@ -148,11 +157,30 @@ func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdent
 	return customSessionData, nil
 }
 
-// GrantScopesIfRequested auto-grants the scopes for which we do not require end-user approval, if they were requested.
-func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester, scopes []string) {
-	for _, scope := range scopes {
+// AutoApproveScopes auto-grants the scopes which we support and for which we do not require end-user approval,
+// if they were requested. This should only be called after it has been validated that the client is allowed to request
+// the scopes that it requested (which is a check performed by fosite).
+func AutoApproveScopes(authorizeRequester fosite.AuthorizeRequester) {
+	for _, scope := range []string{
+		oidcapi.ScopeOpenID,
+		oidcapi.ScopeOfflineAccess,
+		oidcapi.ScopeRequestAudience,
+		oidcapi.ScopeUsername,
+		oidcapi.ScopeGroups,
+	} {
 		oidc.GrantScopeIfRequested(authorizeRequester, scope)
 	}
+
+	// For backwards-compatibility with old pinniped CLI binaries which never request the username and groups scopes
+	// (because those scopes did not exist yet when those CLIs were released), grant/approve the username and groups
+	// scopes even if the CLI did not request them. Basically, pretend that the CLI requested them and auto-approve
+	// them. Newer versions of the CLI binaries will request these scopes, so after enough time has passed that
+	// we can assume the old versions of the CLI are no longer in use in the wild, then we can remove this code and
+	// just let the above logic handle all clients.
+	if authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI {
+		authorizeRequester.GrantScope(oidcapi.ScopeUsername)
+		authorizeRequester.GrantScope(oidcapi.ScopeGroups)
+	}
 }
 
 // GetDownstreamIdentityFromUpstreamIDToken returns the mapped subject, username, and group names, in that order.
@@ -179,11 +207,11 @@ func getSubjectAndUsernameFromUpstreamIDToken(
 ) (string, string, error) {
 	// The spec says the "sub" claim is only unique per issuer,
 	// so we will prepend the issuer string to make it globally unique.
-	upstreamIssuer, err := ExtractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), idTokenClaims)
+	upstreamIssuer, err := ExtractStringClaimValue(oidcapi.IDTokenClaimIssuer, upstreamIDPConfig.GetName(), idTokenClaims)
 	if err != nil {
 		return "", "", err
 	}
-	upstreamSubject, err := ExtractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), idTokenClaims)
+	upstreamSubject, err := ExtractStringClaimValue(oidcapi.IDTokenClaimSubject, upstreamIDPConfig.GetName(), idTokenClaims)
 	if err != nil {
 		return "", "", err
 	}
@@ -266,13 +294,13 @@ func DownstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentit
 
 func DownstreamLDAPSubject(uid string, ldapURL url.URL) string {
 	q := ldapURL.Query()
-	q.Set(oidc.IDTokenSubjectClaim, uid)
+	q.Set(oidcapi.IDTokenClaimSubject, uid)
 	ldapURL.RawQuery = q.Encode()
 	return ldapURL.String()
 }
 
 func downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString string, upstreamSubject string) string {
-	return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, url.QueryEscape(upstreamSubject))
+	return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidcapi.IDTokenClaimSubject, url.QueryEscape(upstreamSubject))
 }
 
 // GetGroupsFromUpstreamIDToken returns mapped group names coerced into a slice of strings.

internal/oidc/login/post_login_handler.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"net/url"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
 
 	"go.pinniped.dev/internal/httputil/httperr"
@@ -46,10 +45,10 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider
 			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.
+		// Automatically grant certain 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})
+		downstreamsession.AutoApproveScopes(authorizeRequester)
 
 		// Get the username and password form params from the POST body.
 		username := r.PostFormValue(usernameParamName)
@@ -83,7 +82,7 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider
 		subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse)
 		username = authenticateResponse.User.GetName()
 		groups := authenticateResponse.User.GetGroups()
-		customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse)
+		customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username)
 		openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData)
 		oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false)
 

internal/oidc/login/post_login_handler_test.go

@@ -17,10 +17,12 @@ import (
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/kubernetes/fake"
 
+	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/authenticators"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
+	"go.pinniped.dev/internal/oidc/oidcclientvalidator"
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/oidctestutil"
@@ -86,8 +88,8 @@ func TestPostLoginEndpoint(t *testing.T) {
 		}
 	)
 
-	happyDownstreamScopesRequested := []string{"openid", "groups"}
-	happyDownstreamScopesGranted := []string{"openid", "groups"}
+	happyDownstreamScopesRequested := []string{"openid", "username", "groups"}
+	happyDownstreamScopesGranted := []string{"openid", "username", "groups"}
 
 	happyDownstreamRequestParamsQuery := url.Values{
 		"response_type":         []string{"code"},
@@ -192,6 +194,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 	}
 
 	expectedHappyActiveDirectoryUpstreamCustomSession := &psession.CustomSessionData{
+		Username:     happyLDAPUsernameFromAuthenticator,
 		ProviderUID:  activeDirectoryUpstreamResourceUID,
 		ProviderName: activeDirectoryUpstreamName,
 		ProviderType: psession.ProviderTypeActiveDirectory,
@@ -204,6 +207,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 	}
 
 	expectedHappyLDAPUpstreamCustomSession := &psession.CustomSessionData{
+		Username:     happyLDAPUsernameFromAuthenticator,
 		ProviderUID:  ldapUpstreamResourceUID,
 		ProviderName: ldapUpstreamName,
 		ProviderType: psession.ProviderTypeLDAP,
@@ -216,7 +220,7 @@ func TestPostLoginEndpoint(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
-	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState
+	happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyDownstreamState
 
 	happyUsernamePasswordFormParams := url.Values{userParam: []string{happyLDAPUsername}, passParam: []string{happyLDAPPassword}}
 
@@ -237,7 +241,8 @@ func TestPostLoginEndpoint(t *testing.T) {
 
 	addFullyCapableDynamicClientAndSecretToKubeResources := func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
 		oidcClient, secret := testutil.FullyCapableOIDCClientAndStorageSecret(t,
-			"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost})
+			"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID, downstreamRedirectURI,
+			[]string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
 		require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
 		require.NoError(t, kubeClient.Tracker().Add(secret))
 	}
@@ -413,7 +418,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantStatus:                        http.StatusSeeOther,
 			wantContentType:                   htmlContentType,
 			wantBodyString:                    "",
-			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState,
+			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
 			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
 			wantDownstreamIDTokenGroups:       happyLDAPGroups,
@@ -439,7 +444,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantStatus:                        http.StatusSeeOther,
 			wantContentType:                   htmlContentType,
 			wantBodyString:                    "",
-			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+groups&state=` + happyDownstreamState,
+			wantRedirectLocationRegexp:        "http://127.0.0.1:4242/callback" + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
 			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
 			wantDownstreamIDTokenGroups:       happyLDAPGroups,
@@ -460,17 +465,18 @@ func TestPostLoginEndpoint(t *testing.T) {
 					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,
+			formParams:      happyUsernamePasswordFormParams,
+			wantStatus:      http.StatusSeeOther,
+			wantContentType: htmlContentType,
+			wantBodyString:  "",
+			// username and groups scopes were not requested but are granted anyway for the pinniped-cli client for backwards compatibility
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+pinniped%3Arequest-audience\+username\+groups&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"},
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
@@ -478,7 +484,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
 		},
 		{
-			name:          "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client",
+			name:          "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is allowed to request username and groups but does not request them",
 			idps:          oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
 			kubeResources: addFullyCapableDynamicClientAndSecretToKubeResources,
 			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
@@ -492,8 +498,8 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantBodyString:                    "",
 			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+pinniped%3Arequest-audience&state=` + happyDownstreamState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
-			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
-			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamIDTokenUsername:     "",         // username scope was not requested, so there should be no username in the ID token
+			wantDownstreamIDTokenGroups:       []string{}, // groups scope was not requested, so there should be no groups in the ID token
 			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "pinniped:request-audience"},
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
 			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "pinniped:request-audience"},
@@ -503,6 +509,74 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
 		},
+		{
+			name: "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is not allowed to request username and does not request username",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude username scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "groups"},      // username not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid groups offline_access"},
+				).Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+groups&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     "", // username scope was not requested, so there should be no username in the ID token
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
+			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "groups"},
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "groups"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
+		{
+			name: "happy LDAP login when there are additional allowed downstream requested scopes with dynamic client, when dynamic client is not allowed to request groups and does not request groups",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude groups scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "username"},    // groups not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid username offline_access"},
+				).Encode()
+			}),
+			formParams:                        happyUsernamePasswordFormParams,
+			wantStatus:                        http.StatusSeeOther,
+			wantContentType:                   htmlContentType,
+			wantBodyString:                    "",
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access\+username&state=` + happyDownstreamState,
+			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
+			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       []string{}, // groups scope was not requested, so there should be no groups in the ID token
+			wantDownstreamRequestedScopes:     []string{"openid", "offline_access", "username"},
+			wantDownstreamRedirectURI:         downstreamRedirectURI,
+			wantDownstreamGrantedScopes:       []string{"openid", "offline_access", "username"},
+			wantDownstreamNonce:               downstreamNonce,
+			wantDownstreamClient:              downstreamDynamicClientID,
+			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
+			wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
+			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
+		},
 		{
 			name: "happy LDAP when downstream OIDC validations are skipped because the openid scope was not requested",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
@@ -515,17 +589,18 @@ func TestPostLoginEndpoint(t *testing.T) {
 					},
 				).Encode()
 			}),
-			formParams:                        happyUsernamePasswordFormParams,
-			wantStatus:                        http.StatusSeeOther,
-			wantContentType:                   htmlContentType,
-			wantBodyString:                    "",
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyDownstreamState, // no scopes granted
+			formParams:      happyUsernamePasswordFormParams,
+			wantStatus:      http.StatusSeeOther,
+			wantContentType: htmlContentType,
+			wantBodyString:  "",
+			// username and groups scopes were not requested but are granted anyway for the pinniped-cli client for backwards compatibility
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
 			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
 			wantDownstreamIDTokenGroups:       happyLDAPGroups,
 			wantDownstreamRequestedScopes:     []string{"email"}, // only email was requested
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
-			wantDownstreamGrantedScopes:       []string{}, // no scopes granted
+			wantDownstreamGrantedScopes:       []string{"username", "groups"},
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
@@ -533,7 +608,7 @@ func TestPostLoginEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyLDAPUpstreamCustomSession,
 		},
 		{
-			name: "happy LDAP login when groups scope is not requested",
+			name: "happy LDAP login when username and groups scopes are not requested",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().
 				WithLDAP(&upstreamLDAPIdentityProvider). // should pick this one
 				WithActiveDirectory(&erroringUpstreamLDAPIdentityProvider),
@@ -542,16 +617,18 @@ func TestPostLoginEndpoint(t *testing.T) {
 					map[string]string{"scope": "openid"},
 				).Encode()
 			}),
-			formParams:                        happyUsernamePasswordFormParams,
-			wantStatus:                        http.StatusSeeOther,
-			wantContentType:                   htmlContentType,
-			wantBodyString:                    "",
-			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyDownstreamState,
+			formParams:      happyUsernamePasswordFormParams,
+			wantStatus:      http.StatusSeeOther,
+			wantContentType: htmlContentType,
+			wantBodyString:  "",
+			// username and groups scopes were not requested but are granted anyway for the pinniped-cli client for backwards compatibility
+			wantRedirectLocationRegexp:        downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+username\+groups&state=` + happyDownstreamState,
 			wantDownstreamIDTokenSubject:      upstreamLDAPURL + "&sub=" + happyLDAPUID,
 			wantDownstreamIDTokenUsername:     happyLDAPUsernameFromAuthenticator,
+			wantDownstreamIDTokenGroups:       happyLDAPGroups,
 			wantDownstreamRequestedScopes:     []string{"openid"},
 			wantDownstreamRedirectURI:         downstreamRedirectURI,
-			wantDownstreamGrantedScopes:       []string{"openid"},
+			wantDownstreamGrantedScopes:       []string{"openid", "username", "groups"},
 			wantDownstreamNonce:               downstreamNonce,
 			wantDownstreamClient:              downstreamPinnipedCLIClientID,
 			wantDownstreamPKCEChallenge:       downstreamPKCEChallenge,
@@ -810,6 +887,46 @@ func TestPostLoginEndpoint(t *testing.T) {
 			formParams: happyUsernamePasswordFormParams,
 			wantErr:    "error using state downstream auth params",
 		},
+		{
+			name: "using dynamic client which is not allowed to request username scope in authorize request but requests it anyway",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude username scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "groups"},      // username not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid username offline_access"},
+				).Encode()
+			}),
+			formParams: happyUsernamePasswordFormParams,
+			wantErr:    "error using state downstream auth params",
+		},
+		{
+			name: "using dynamic client which is not allowed to request groups scope in authorize request but requests it anyway",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider),
+			kubeResources: func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset) {
+				oidcClient, secret := testutil.OIDCClientAndStorageSecret(t,
+					"some-namespace", downstreamDynamicClientID, downstreamDynamicClientUID,
+					[]configv1alpha1.GrantType{"authorization_code", "refresh_token"}, // token exchange not allowed (required to exclude groups scope)
+					[]configv1alpha1.Scope{"openid", "offline_access", "username"},    // groups not allowed
+					downstreamRedirectURI, []string{testutil.HashedPassword1AtGoMinCost}, oidcclientvalidator.Validate)
+				require.NoError(t, supervisorClient.Tracker().Add(oidcClient))
+				require.NoError(t, kubeClient.Tracker().Add(secret))
+			},
+			decodedState: modifyHappyLDAPDecodedState(func(data *oidc.UpstreamStateParamData) {
+				data.AuthParams = shallowCopyAndModifyQuery(happyDownstreamRequestParamsQueryForDynamicClient,
+					map[string]string{"scope": "openid groups offline_access"},
+				).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),

internal/oidc/oidc.go

@@ -11,13 +11,13 @@ import (
 	"net/http"
 	"time"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/felixge/httpsnoop"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
 	errorsx "github.com/pkg/errors"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/oidc/csrftoken"
 	"go.pinniped.dev/internal/oidc/jwks"
@@ -40,16 +40,17 @@ const (
 )
 
 const (
-	// Just in case we need to make a breaking change to the format of the upstream state param,
-	// we are including a format version number. This gives the opportunity for a future version of Pinniped
-	// to have the consumer of this format decide to reject versions that it doesn't understand.
+	// UpstreamStateParamFormatVersion exists just in case we need to make a breaking change to the format of the
+	// upstream state param, we are including a format version number. This gives the opportunity for a future version
+	// of Pinniped to have the consumer of this format decide to reject versions that it doesn't understand.
 	//
 	// Version 1 was the original version.
 	// Version 2 added the UpstreamType field to the UpstreamStateParamData struct.
 	UpstreamStateParamFormatVersion = "2"
 
-	// The `name` passed to the encoder for encoding the upstream state param value. This name is short
-	// because it will be encoded into the upstream state param value and we're trying to keep that small.
+	// UpstreamStateParamEncodingName is the `name` passed to the encoder for encoding the upstream state param value.
+	// This name is short because it will be encoded into the upstream state param value, and we're trying to keep that
+	// small.
 	UpstreamStateParamEncodingName = "s"
 
 	// CSRFCookieName is the name of the browser cookie which shall hold our CSRF value.
@@ -61,29 +62,6 @@ const (
 	// cookie contents.
 	CSRFCookieEncodingName = "csrf"
 
-	// The name of the issuer claim specified in the OIDC spec.
-	IDTokenIssuerClaim = "iss"
-
-	// The name of the subject claim specified in the OIDC spec.
-	IDTokenSubjectClaim = "sub"
-
-	// DownstreamUsernameClaim is a custom claim in the downstream ID token
-	// whose value is mapped from a claim in the upstream token.
-	// By default the value is the same as the downstream subject claim's.
-	DownstreamUsernameClaim = "username"
-
-	// DownstreamGroupsClaim is what we will use to encode the groups in the downstream OIDC ID token
-	// information.
-	DownstreamGroupsClaim = "groups"
-
-	// DownstreamGroupsScope is a custom scope that determines whether the
-	// groups claim will be returned in ID tokens.
-	DownstreamGroupsScope = "groups"
-
-	// RequestAudienceScope is a custom scope that determines whether a RFC8693 token
-	// exchange is allowed to request a different audience.
-	RequestAudienceScope = "pinniped:request-audience"
-
 	// CSRFCookieLifespan is the length of time that the CSRF cookie is valid. After this time, the
 	// Supervisor's authorization endpoint should give the browser a new CSRF cookie. We set it to
 	// a week so that it is unlikely to expire during a login.
@@ -229,7 +207,7 @@ func FositeOauth2Helper(
 		EnforcePKCE:   true,
 
 		// "offline_access" as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
-		RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess},
+		RefreshTokenScopes: []string{oidcapi.ScopeOfflineAccess},
 
 		// The default is to support all prompt values from the spec.
 		// See https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

internal/oidc/oidcclientvalidator/oidcclientvalidator.go

@@ -7,11 +7,11 @@ 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"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/oidcclientsecretstorage"
 )
 
@@ -27,16 +27,6 @@ const (
 	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"
 )
@@ -67,21 +57,21 @@ func Validate(oidcClient *v1alpha1.OIDCClient, secret *v1.Secret, minBcryptCost
 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 !allowedScopesContains(oidcClient, oidcapi.ScopeOpenID) {
+		m = append(m, fmt.Sprintf("%q must always be included in %q", oidcapi.ScopeOpenID, allowedScopesFieldName))
 	}
-	if allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName) && !allowedScopesContains(oidcClient, offlineAccessScopeName) {
+	if allowedGrantTypesContains(oidcClient, oidcapi.GrantTypeRefreshToken) && !allowedScopesContains(oidcClient, oidcapi.ScopeOfflineAccess) {
 		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			offlineAccessScopeName, allowedScopesFieldName, refreshTokenGrantTypeName, allowedGrantTypesFieldName))
+			oidcapi.ScopeOfflineAccess, allowedScopesFieldName, oidcapi.GrantTypeRefreshToken, allowedGrantTypesFieldName))
 	}
-	if allowedScopesContains(oidcClient, requestAudienceScopeName) &&
-		(!allowedScopesContains(oidcClient, usernameScopeName) || !allowedScopesContains(oidcClient, groupsScopeName)) {
+	if allowedScopesContains(oidcClient, oidcapi.ScopeRequestAudience) &&
+		(!allowedScopesContains(oidcClient, oidcapi.ScopeUsername) || !allowedScopesContains(oidcClient, oidcapi.ScopeGroups)) {
 		m = append(m, fmt.Sprintf("%q and %q must be included in %q when %q is included in %q",
-			usernameScopeName, groupsScopeName, allowedScopesFieldName, requestAudienceScopeName, allowedScopesFieldName))
+			oidcapi.ScopeUsername, oidcapi.ScopeGroups, allowedScopesFieldName, oidcapi.ScopeRequestAudience, allowedScopesFieldName))
 	}
-	if allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName) && !allowedScopesContains(oidcClient, requestAudienceScopeName) {
+	if allowedGrantTypesContains(oidcClient, oidcapi.GrantTypeTokenExchange) && !allowedScopesContains(oidcClient, oidcapi.ScopeRequestAudience) {
 		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			requestAudienceScopeName, allowedScopesFieldName, tokenExchangeGrantTypeName, allowedGrantTypesFieldName))
+			oidcapi.ScopeRequestAudience, allowedScopesFieldName, oidcapi.GrantTypeTokenExchange, allowedGrantTypesFieldName))
 	}
 
 	if len(m) == 0 {
@@ -107,17 +97,17 @@ func validateAllowedScopes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alph
 func validateAllowedGrantTypes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
 	m := make([]string, 0, 3)
 
-	if !allowedGrantTypesContains(oidcClient, authorizationCodeGrantTypeName) {
+	if !allowedGrantTypesContains(oidcClient, oidcapi.GrantTypeAuthorizationCode) {
 		m = append(m, fmt.Sprintf("%q must always be included in %q",
-			authorizationCodeGrantTypeName, allowedGrantTypesFieldName))
+			oidcapi.GrantTypeAuthorizationCode, allowedGrantTypesFieldName))
 	}
-	if allowedScopesContains(oidcClient, offlineAccessScopeName) && !allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName) {
+	if allowedScopesContains(oidcClient, oidcapi.ScopeOfflineAccess) && !allowedGrantTypesContains(oidcClient, oidcapi.GrantTypeRefreshToken) {
 		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			refreshTokenGrantTypeName, allowedGrantTypesFieldName, offlineAccessScopeName, allowedScopesFieldName))
+			oidcapi.GrantTypeRefreshToken, allowedGrantTypesFieldName, oidcapi.ScopeOfflineAccess, allowedScopesFieldName))
 	}
-	if allowedScopesContains(oidcClient, requestAudienceScopeName) && !allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName) {
+	if allowedScopesContains(oidcClient, oidcapi.ScopeRequestAudience) && !allowedGrantTypesContains(oidcClient, oidcapi.GrantTypeTokenExchange) {
 		m = append(m, fmt.Sprintf("%q must be included in %q when %q is included in %q",
-			tokenExchangeGrantTypeName, allowedGrantTypesFieldName, requestAudienceScopeName, allowedScopesFieldName))
+			oidcapi.GrantTypeTokenExchange, allowedGrantTypesFieldName, oidcapi.ScopeRequestAudience, allowedScopesFieldName))
 	}
 
 	if len(m) == 0 {

internal/oidc/provider/manager/manager_test.go

@@ -171,7 +171,7 @@ func TestManager(t *testing.T) {
 			r.NoError(err)
 			actualLocationQueryParams := parsedLocation.Query()
 			r.Contains(actualLocationQueryParams, "code")
-			r.Equal("openid", actualLocationQueryParams.Get("scope"))
+			r.Equal("openid username groups", actualLocationQueryParams.Get("scope"))
 			r.Equal("some-state-value-with-enough-bytes-to-exceed-min-allowed", actualLocationQueryParams.Get("state"))
 
 			// Make sure that we wired up the callback endpoint to use kube storage for fosite sessions.
@@ -343,7 +343,7 @@ func TestManager(t *testing.T) {
 
 			authRequestParams := "?" + url.Values{
 				"response_type":         []string{"code"},
-				"scope":                 []string{"openid profile email"},
+				"scope":                 []string{"openid profile email username groups"},
 				"client_id":             []string{downstreamClientID},
 				"state":                 []string{"some-state-value-with-enough-bytes-to-exceed-min-allowed"},
 				"nonce":                 []string{"some-nonce-value-with-enough-bytes-to-exceed-min-allowed"},

internal/oidc/token/token_handler.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/apiserver/pkg/warning"
 	"k8s.io/utils/strings/slices"
 
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/downstreamsession"
@@ -39,7 +40,7 @@ func NewHandler(
 		}
 
 		// Check if we are performing a refresh grant.
-		if accessRequest.GetGrantTypes().ExactOne("refresh_token") {
+		if accessRequest.GetGrantTypes().ExactOne(oidcapi.GrantTypeRefreshToken) {
 			// The above call to NewAccessRequest has loaded the session from storage into the accessRequest variable.
 			// The session, requested scopes, and requested audience from the original authorize request was retrieved
 			// from the Kube storage layer and added to the accessRequest. Additionally, the audience and scopes may
@@ -54,7 +55,7 @@ func NewHandler(
 
 		// When we are in the authorization code flow, check if we have any warnings that previous handlers want us
 		// to send to the client to be printed on the CLI.
-		if accessRequest.GetGrantTypes().ExactOne("authorization_code") {
+		if accessRequest.GetGrantTypes().ExactOne(oidcapi.GrantTypeAuthorizationCode) {
 			storedSession := accessRequest.GetSession().(*psession.PinnipedSession)
 			customSessionData := storedSession.Custom
 			if customSessionData != nil {
@@ -108,20 +109,27 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester,
 	}
 
 	grantedScopes := accessRequest.GetGrantedScopes()
+	clientID := accessRequest.GetClient().GetID()
 
 	switch customSessionData.ProviderType {
 	case psession.ProviderTypeOIDC:
-		return upstreamOIDCRefresh(ctx, session, providerCache, grantedScopes)
+		return upstreamOIDCRefresh(ctx, session, providerCache, grantedScopes, clientID)
 	case psession.ProviderTypeLDAP:
-		return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes)
+		return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes, clientID)
 	case psession.ProviderTypeActiveDirectory:
-		return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes)
+		return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes, clientID)
 	default:
 		return errorsx.WithStack(errMissingUpstreamSessionInternalError())
 	}
 }
 
-func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, providerCache oidc.UpstreamIdentityProvidersLister, grantedScopes []string) error {
+func upstreamOIDCRefresh(
+	ctx context.Context,
+	session *psession.PinnipedSession,
+	providerCache oidc.UpstreamIdentityProvidersLister,
+	grantedScopes []string,
+	clientID string,
+) error {
 	s := session.Custom
 	if s.OIDC == nil {
 		return errorsx.WithStack(errMissingUpstreamSessionInternalError())
@@ -180,7 +188,7 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession,
 		return err
 	}
 
-	groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope)
+	groupsScope := slices.Contains(grantedScopes, oidcapi.ScopeGroups)
 	if groupsScope { //nolint:nestif
 		// If possible, update the user's group memberships. The configured groups claim name (if there is one) may or
 		// may not be included in the newly fetched and merged claims. It could be missing due to a misconfiguration of the
@@ -204,8 +212,8 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession,
 			if err != nil {
 				return err
 			}
-			warnIfGroupsChanged(ctx, oldGroups, refreshedGroups, username)
-			session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = refreshedGroups
+			warnIfGroupsChanged(ctx, oldGroups, refreshedGroups, username, clientID)
+			session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = refreshedGroups
 		}
 	}
 
@@ -240,7 +248,7 @@ func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interfac
 		return nil
 	}
 
-	newSub, hasSub := getString(mergedClaims, oidc.IDTokenSubjectClaim)
+	newSub, hasSub := getString(mergedClaims, oidcapi.IDTokenClaimSubject)
 	if !hasSub {
 		return errUpstreamRefreshError().WithHintf(
 			"Upstream refresh failed.").WithTrace(errors.New("subject in upstream refresh not found")).
@@ -253,7 +261,10 @@ func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interfac
 	}
 
 	newUsername, hasUsername := getString(mergedClaims, usernameClaimName)
-	oldUsername := session.Fosite.Claims.Extra[oidc.DownstreamUsernameClaim]
+	oldUsername, err := getDownstreamUsernameFromPinnipedSession(session)
+	if err != nil {
+		return err
+	}
 	// It's possible that a username wasn't returned by the upstream provider during refresh,
 	// but if it is, verify that it hasn't changed.
 	if hasUsername && oldUsername != newUsername {
@@ -262,7 +273,7 @@ func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interfac
 			WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)
 	}
 
-	newIssuer, hasIssuer := getString(mergedClaims, oidc.IDTokenIssuerClaim)
+	newIssuer, hasIssuer := getString(mergedClaims, oidcapi.IDTokenClaimIssuer)
 	// It's possible that an issuer wasn't returned by the upstream provider during refresh,
 	// but if it is, verify that it hasn't changed.
 	if hasIssuer && s.OIDC.UpstreamIssuer != newIssuer {
@@ -297,14 +308,20 @@ func findOIDCProviderByNameAndValidateUID(
 		WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType))
 }
 
-func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentityProvidersLister, session *psession.PinnipedSession, grantedScopes []string) error {
+func upstreamLDAPRefresh(
+	ctx context.Context,
+	providerCache oidc.UpstreamIdentityProvidersLister,
+	session *psession.PinnipedSession,
+	grantedScopes []string,
+	clientID string,
+) error {
 	username, err := getDownstreamUsernameFromPinnipedSession(session)
 	if err != nil {
 		return err
 	}
 	subject := session.Fosite.Claims.Subject
 	var oldGroups []string
-	if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) {
+	if slices.Contains(grantedScopes, oidcapi.ScopeGroups) {
 		oldGroups, err = getDownstreamGroupsFromPinnipedSession(session)
 		if err != nil {
 			return err
@@ -349,12 +366,11 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit
 			"Upstream refresh failed.").WithTrace(err).
 			WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)
 	}
-	groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope)
+	groupsScope := slices.Contains(grantedScopes, oidcapi.ScopeGroups)
 	if groupsScope {
+		warnIfGroupsChanged(ctx, oldGroups, groups, username, clientID)
 		// Replace the old value with the new value.
-		session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = groups
-
-		warnIfGroupsChanged(ctx, oldGroups, groups, username)
+		session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = groups
 	}
 
 	return nil
@@ -391,16 +407,8 @@ func findLDAPProviderByNameAndValidateUID(
 }
 
 func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) (string, error) {
-	extra := session.Fosite.Claims.Extra
-	if extra == nil {
-		return "", errorsx.WithStack(errMissingUpstreamSessionInternalError())
-	}
-	downstreamUsernameInterface := extra[oidc.DownstreamUsernameClaim]
-	if downstreamUsernameInterface == nil {
-		return "", errorsx.WithStack(errMissingUpstreamSessionInternalError())
-	}
-	downstreamUsername, ok := downstreamUsernameInterface.(string)
-	if !ok || len(downstreamUsername) == 0 {
+	downstreamUsername := session.Custom.Username
+	if len(downstreamUsername) == 0 {
 		return "", errorsx.WithStack(errMissingUpstreamSessionInternalError())
 	}
 	return downstreamUsername, nil
@@ -411,7 +419,7 @@ func getDownstreamGroupsFromPinnipedSession(session *psession.PinnipedSession) (
 	if extra == nil {
 		return nil, errorsx.WithStack(errMissingUpstreamSessionInternalError())
 	}
-	downstreamGroupsInterface := extra[oidc.DownstreamGroupsClaim]
+	downstreamGroupsInterface := extra[oidcapi.IDTokenClaimGroups]
 	if downstreamGroupsInterface == nil {
 		return nil, errorsx.WithStack(errMissingUpstreamSessionInternalError())
 	}
@@ -431,8 +439,17 @@ func getDownstreamGroupsFromPinnipedSession(session *psession.PinnipedSession) (
 	return downstreamGroups, nil
 }
 
-func warnIfGroupsChanged(ctx context.Context, oldGroups, newGroups []string, username string) {
+func warnIfGroupsChanged(ctx context.Context, oldGroups, newGroups []string, username string, clientID string) {
+	if clientID != oidcapi.ClientIDPinnipedCLI {
+		// Only send these warnings to the CLI client. They are intended for kubectl to print to the screen.
+		// A webapp using a dynamic client wouldn't know to look for these special warning headers, and
+		// if the dynamic client lacked the username scope, then these warning messages would be leaking
+		// the user's username to the client within the text of the warning.
+		return
+	}
+
 	added, removed := diffSortedGroups(oldGroups, newGroups)
+
 	if len(added) > 0 {
 		warning.AddWarning(ctx, "", fmt.Sprintf("User %q has been added to the following groups: %q", username, added))
 	}

internal/oidc/token_exchange.go

@@ -8,21 +8,19 @@ import (
 	"net/url"
 	"strings"
 
-	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/compose"
 	"github.com/ory/fosite/handler/oauth2"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/pkg/errors"
 
-	"go.pinniped.dev/internal/oidc/clientregistry"
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
+	"go.pinniped.dev/internal/psession"
 )
 
 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
+	tokenTypeAccessToken = "urn:ietf:params:oauth:token-type:access_token" //nolint: gosec
+	tokenTypeJWT         = "urn:ietf:params:oauth:token-type:jwt"          //nolint: gosec
 )
 
 type stsParams struct {
@@ -78,17 +76,22 @@ func (t *TokenExchangeHandler) PopulateTokenEndpointResponse(ctx context.Context
 	}
 
 	// Check that the client is allowed to perform this grant type.
-	if !requester.GetClient().GetGrantTypes().Has(tokenExchangeGrantType) {
+	if !requester.GetClient().GetGrantTypes().Has(oidcapi.GrantTypeTokenExchange) {
 		// 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))
+		return errors.WithStack(fosite.ErrUnauthorizedClient.WithHintf(`The OAuth 2.0 Client is not allowed to use token exchange grant "%s".`, oidcapi.GrantTypeTokenExchange))
 	}
 
 	// 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))
+	if !originalRequester.GetGrantedScopes().Has(oidcapi.ScopeRequestAudience) {
+		return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", oidcapi.ScopeRequestAudience))
 	}
-	if !originalRequester.GetGrantedScopes().Has(oidc.ScopeOpenID) {
-		return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", oidc.ScopeOpenID))
+	if !originalRequester.GetGrantedScopes().Has(oidcapi.ScopeOpenID) {
+		return errors.WithStack(fosite.ErrAccessDenied.WithHintf("missing the %q scope", oidcapi.ScopeOpenID))
+	}
+
+	// Check that the stored session meets the minimum requirements for token exchange.
+	if err := t.validateSession(originalRequester); err != nil {
+		return errors.WithStack(err)
 	}
 
 	// Use the original authorize request information, along with the requested audience, to mint a new JWT.
@@ -110,6 +113,22 @@ func (t *TokenExchangeHandler) mintJWT(ctx context.Context, requester fosite.Req
 	return t.idTokenStrategy.GenerateIDToken(ctx, downscoped)
 }
 
+func (t *TokenExchangeHandler) validateSession(requester fosite.Requester) error {
+	pSession, ok := requester.GetSession().(*psession.PinnipedSession)
+	if !ok {
+		// This shouldn't really happen.
+		return fosite.ErrServerError.WithHint("Invalid session storage.")
+	}
+	username, ok := pSession.IDTokenClaims().Extra[oidcapi.IDTokenClaimUsername].(string)
+	if !ok || username == "" {
+		// No username was stored in the session's ID token claims (or the stored username was not a string, which
+		// shouldn't really happen). Usernames will not be stored in the session's ID token claims when the username
+		// scope was not requested/granted, but otherwise they should be stored.
+		return fosite.ErrAccessDenied.WithHintf("No username found in session. Ensure that the %q scope was requested and granted at the authorization endpoint.", oidcapi.ScopeUsername)
+	}
+	return nil
+}
+
 func (t *TokenExchangeHandler) validateParams(params url.Values) (*stsParams, error) {
 	var result stsParams
 
@@ -157,8 +176,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 == clientregistry.PinnipedCLIClientID {
-		return nil, fosite.ErrInvalidRequest.WithHintf("requested audience cannot equal '%s'", clientregistry.PinnipedCLIClientID)
+	if result.requestedAudience == oidcapi.ClientIDPinnipedCLI {
+		return nil, fosite.ErrInvalidRequest.WithHintf("requested audience cannot equal '%s'", oidcapi.ClientIDPinnipedCLI)
 	}
 
 	return &result, nil
@@ -181,5 +200,5 @@ func (t *TokenExchangeHandler) CanSkipClientAuth(_ fosite.AccessRequester) bool
 }
 
 func (t *TokenExchangeHandler) CanHandleTokenEndpointRequest(requester fosite.AccessRequester) bool {
-	return requester.GetGrantTypes().ExactOne(tokenExchangeGrantType)
+	return requester.GetGrantTypes().ExactOne(oidcapi.GrantTypeTokenExchange)
 }

internal/psession/pinniped_session.go

@@ -27,6 +27,11 @@ var _ openid.Session = &PinnipedSession{}
 // CustomSessionData is the custom session data needed by Pinniped. It should be treated as a union type,
 // where the value of ProviderType decides which other fields to use.
 type CustomSessionData struct {
+	// Username will contain the downstream username determined during initial authorization. We store this
+	// so that we can validate that it does not change upon refresh. This should normally never be empty, since
+	// all users must have a username.
+	Username string `json:"username"`
+
 	// The Kubernetes resource UID of the identity provider CRD for the upstream IDP used to start this session.
 	// This should be validated again upon downstream refresh to make sure that we are not refreshing against
 	// a different identity provider CRD which just happens to have the same name.

internal/testutil/oidcclient.go

@@ -7,6 +7,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -41,19 +43,30 @@ func allDynamicClientScopes() []configv1alpha1.Scope {
 	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 {
+func newOIDCClient(
+	namespace string,
+	clientID string,
+	clientUID string,
+	redirectURI string,
+	allowedGrantTypes []configv1alpha1.GrantType,
+	allowedScopes []configv1alpha1.Scope,
+) *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(),
+			AllowedGrantTypes:   allowedGrantTypes,
+			AllowedScopes:       allowedScopes,
 			AllowedRedirectURIs: []configv1alpha1.RedirectURI{configv1alpha1.RedirectURI(redirectURI)},
 		},
 	}
 }
 
+// OIDCClientValidatorFunc is an interface-like type that allows these test helpers to avoid having a direct dependency
+// on the production code, to avoid circular module dependencies. Implemented by oidcclientvalidator.Validate.
+type OIDCClientValidatorFunc func(oidcClient *configv1alpha1.OIDCClient, secret *corev1.Secret, minBcryptCost int) (bool, []*configv1alpha1.Condition, []string)
+
+// FullyCapableOIDCClientAndStorageSecret returns an OIDC client which is allowed to use all grant types and all scopes
+// that are supported by the Supervisor for dynamic clients, along with a corresponding client secret storage Secret.
 func FullyCapableOIDCClientAndStorageSecret(
 	t *testing.T,
 	namespace string,
@@ -61,7 +74,38 @@ func FullyCapableOIDCClientAndStorageSecret(
 	clientUID string,
 	redirectURI string,
 	hashes []string,
+	validateFunc OIDCClientValidatorFunc,
 ) (*configv1alpha1.OIDCClient, *corev1.Secret) {
-	return fullyCapableOIDCClient(namespace, clientID, clientUID, redirectURI),
-		OIDCClientSecretStorageSecretForUID(t, namespace, clientUID, hashes)
+	allScopes := allDynamicClientScopes()
+
+	allGrantTypes := []configv1alpha1.GrantType{
+		"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token",
+	}
+
+	return OIDCClientAndStorageSecret(t, namespace, clientID, clientUID, allGrantTypes, allScopes, redirectURI, hashes, validateFunc)
+}
+
+// OIDCClientAndStorageSecret returns an OIDC client which is allowed to use the specified grant types and scopes,
+// along with a corresponding client secret storage Secret. It also validates the client to make sure that the specified
+// combination of grant types and scopes is considered valid before returning the client.
+func OIDCClientAndStorageSecret(
+	t *testing.T,
+	namespace string,
+	clientID string,
+	clientUID string,
+	allowedGrantTypes []configv1alpha1.GrantType,
+	allowedScopes []configv1alpha1.Scope,
+	redirectURI string,
+	hashes []string,
+	validateFunc OIDCClientValidatorFunc,
+) (*configv1alpha1.OIDCClient, *corev1.Secret) {
+	oidcClient := newOIDCClient(namespace, clientID, clientUID, redirectURI, allowedGrantTypes, allowedScopes)
+	secret := OIDCClientSecretStorageSecretForUID(t, namespace, clientUID, hashes)
+
+	// If a test made an invalid OIDCClient then inform the author of the test, so they can fix the test case.
+	// This is an easy mistake to make when writing tests because there are lots of validations on OIDCClients.
+	valid, conditions, _ := validateFunc(oidcClient, secret, bcrypt.MinCost)
+	require.True(t, valid, "Test's OIDCClient should have been valid. See conditions for errors: %s", conditions)
+
+	return oidcClient, secret
 }

internal/testutil/oidctestutil/oidctestutil.go

@@ -1064,14 +1064,22 @@ func validateAuthcodeStorage(
 
 	// Check the user's identity, which are put into the downstream ID token's subject, username and groups claims.
 	require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject)
-	require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"])
+	wantDownstreamIDTokenUsernameClaimToExist := 1
+	if wantDownstreamIDTokenUsername == "" {
+		wantDownstreamIDTokenUsernameClaimToExist = 0
+		require.NotContains(t, actualClaims.Extra, "username")
+	} else {
+		require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"])
+	}
 	if slices.Contains(wantDownstreamGrantedScopes, "groups") {
-		require.Len(t, actualClaims.Extra, 2)
+		require.Len(t, actualClaims.Extra, wantDownstreamIDTokenUsernameClaimToExist+1)
 		actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
 		require.NotNil(t, actualDownstreamIDTokenGroups)
 		require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups)
 	} else {
-		require.Len(t, actualClaims.Extra, 1)
+		require.Emptyf(t, wantDownstreamIDTokenGroups, "test case did not want the groups scope to be granted, "+
+			"but wanted something in the groups claim, which doesn't make sense. please review the test case's expectations.")
+		require.Len(t, actualClaims.Extra, wantDownstreamIDTokenUsernameClaimToExist)
 		actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
 		require.Nil(t, actualDownstreamIDTokenGroups)
 	}

internal/testutil/psession.go

@@ -24,6 +24,7 @@ func NewFakePinnipedSession() *psession.PinnipedSession {
 			Subject:   "panda",
 		},
 		Custom: &psession.CustomSessionData{
+			Username:     "fake-username",
 			ProviderUID:  "fake-provider-uid",
 			ProviderType: "fake-provider-type",
 			ProviderName: "fake-provider-name",

internal/upstreamldap/upstreamldap.go

@@ -23,10 +23,10 @@ import (
 	"k8s.io/utils/strings/slices"
 	"k8s.io/utils/trace"
 
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/authenticators"
 	"go.pinniped.dev/internal/crypto/ptls"
 	"go.pinniped.dev/internal/endpointaddr"
-	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
@@ -241,7 +241,7 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p
 		return storedRefreshAttributes.Groups, nil
 	}
 	// if we were not granted the groups scope, we should not search for groups or return any.
-	if !slices.Contains(storedRefreshAttributes.GrantedScopes, oidc.DownstreamGroupsScope) {
+	if !slices.Contains(storedRefreshAttributes.GrantedScopes, oidcapi.ScopeGroups) {
 		return nil, nil
 	}
 
@@ -593,7 +593,7 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, grantedScopes [
 	}
 
 	var mappedGroupNames []string
-	if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) {
+	if slices.Contains(grantedScopes, oidcapi.ScopeGroups) {
 		mappedGroupNames, err = p.searchGroupsForUserDN(conn, userEntry.DN)
 		if err != nil {
 			return nil, err

internal/upstreamoidc/upstreamoidc.go

@@ -20,8 +20,8 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 
+	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/httputil/httperr"
-	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
@@ -286,7 +286,7 @@ func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context,
 		return nil, err
 	}
 
-	idTokenSubject, _ := validatedClaims[oidc.IDTokenSubjectClaim].(string)
+	idTokenSubject, _ := validatedClaims[oidcapi.IDTokenClaimSubject].(string)
 
 	if len(idTokenSubject) > 0 || !requireIDToken {
 		// only fetch userinfo if the ID token has a subject or if we are ignoring the id token completely.
@@ -346,7 +346,7 @@ func (p *ProviderConfig) validateIDToken(ctx context.Context, tok *oauth2.Token,
 }
 
 func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}, requireIDToken bool, requireUserInfo bool) error {
-	idTokenSubject, _ := claims[oidc.IDTokenSubjectClaim].(string)
+	idTokenSubject, _ := claims[oidcapi.IDTokenClaimSubject].(string)
 
 	userInfo, err := p.maybeFetchUserInfo(ctx, tok, requireUserInfo)
 	if err != nil {
@@ -371,7 +371,7 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
 	}
 
 	// keep track of the issuer from the ID token
-	idTokenIssuer := claims["iss"]
+	idTokenIssuer := claims[oidcapi.IDTokenClaimIssuer]
 
 	// merge existing claims with user info claims
 	if err := userInfo.Claims(&claims); err != nil {
@@ -381,9 +381,9 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
 	//  "If signed, the UserInfo Response SHOULD contain the Claims iss (issuer) and aud (audience) as members. The iss value SHOULD be the OP's Issuer Identifier URL."
 	//  See https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
 	//  So we just ignore it and use it the version from the id token, which has stronger guarantees.
-	delete(claims, "iss")
+	delete(claims, oidcapi.IDTokenClaimIssuer)
 	if idTokenIssuer != nil {
-		claims["iss"] = idTokenIssuer
+		claims[oidcapi.IDTokenClaimIssuer] = idTokenIssuer
 	}
 
 	maybeLogClaims("claims from ID token and userinfo", p.Name, claims)