Merge pull request #1867 from vmware-tanzu/refactor_supervisor_authen…

…ticators

Refactor Supervisor to make interface for upstream IDPs, to better separate upstream and downstream concerns

apis/supervisor/oidc/types_supervisor_oidc.go.tmpl

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package oidc
@@ -29,6 +29,10 @@ const (
 	// IDTokenClaimSubject is name of the subject claim defined by the OIDC spec.
 	IDTokenClaimSubject = "sub"
 
+	// IDTokenSubClaimIDPNameQueryParam is the name of the query param used in the values of the "sub" claim
+	// in Supervisor-issued ID tokens to identify with which external identity provider the user authenticated.
+	IDTokenSubClaimIDPNameQueryParam = "idpName"
+
 	// IDTokenClaimAuthorizedParty is name of the authorized party claim defined by the OIDC spec.
 	IDTokenClaimAuthorizedParty = "azp"
 

internal/authenticators/authenticators.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package authenticators contains authenticator interfaces.
@@ -31,7 +31,7 @@ import (
 // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator
 // interface, as well as the Response type.
 type UserAuthenticator interface {
-	AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*Response, bool, error)
+	AuthenticateUser(ctx context.Context, username, password string, skipGroups bool) (*Response, bool, error)
 }
 
 type Response struct {

internal/controller/supervisorstorage/garbage_collector_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package supervisorstorage
@@ -34,6 +34,7 @@ import (
 	"go.pinniped.dev/internal/psession"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/oidctestutil"
+	"go.pinniped.dev/internal/testutil/testidplister"
 )
 
 func TestGarbageCollectorControllerInformerFilters(t *testing.T) {
@@ -359,7 +360,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -483,7 +484,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -560,7 +561,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -631,7 +632,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -702,7 +703,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -775,7 +776,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithResourceUID("upstream-oidc-provider-uid").
 					// make the upstream revocation fail in a retryable way
 					WithRevokeTokenError(dynamicupstreamprovider.NewRetryableRevocationError(errors.New("some retryable upstream revocation error")))
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -800,7 +801,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithResourceUID("upstream-oidc-provider-uid").
 					// make the upstream revocation fail in a non-retryable way
 					WithRevokeTokenError(errors.New("some upstream revocation error not worth retrying"))
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -879,7 +880,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(errors.New("some upstream revocation error")) // the upstream revocation will fail
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -1002,7 +1003,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -1126,7 +1127,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -1204,7 +1205,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
@@ -1281,7 +1282,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 					WithName("upstream-oidc-provider-name").
 					WithResourceUID("upstream-oidc-provider-uid").
 					WithRevokeTokenError(nil)
-				idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
+				idpListerBuilder := testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
 
 				startInformersAndController(idpListerBuilder.BuildDynamicUpstreamIDPProvider())
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))