Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Feb 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9992855
Refactor error handling in authorize endpoint (changes some responses)
cfryanr Feb 13, 2024
9db8713
More refactoring of auth handler and related refactor of upstreamldap
cfryanr Feb 14, 2024
1bc13e9
Refactor to extract interface for upstream IDP interactions
cfryanr Feb 15, 2024
1e8e7b9
Refactor token endpoint to add interface for IDP upstream refresh
cfryanr Feb 16, 2024
b341e52
Refactor to move invocation of identity transforms out of IDP interfaces
cfryanr Feb 17, 2024
4b4a4ad
Rename a func and collapse applying id transforms into creating session
cfryanr Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apis/supervisor/oidc/types_supervisor_oidc.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"

Expand Down
6 changes: 5 additions & 1 deletion generated/1.21/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.22/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.23/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.24/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.25/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.26/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.27/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/1.28/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated/latest/apis/supervisor/oidc/types_supervisor_oidc.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/authenticators/authenticators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 14 additions & 13 deletions internal/controller/supervisorstorage/garbage_collector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
Loading
Loading