From 7af75dfe3c775ca924a27502f42b4d75ccca84a5 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 8 May 2023 14:07:38 -0700 Subject: [PATCH] First draft of implementation of multiple IDPs support --- hack/prepare-supervisor-on-kind.sh | 245 ++++++++++---- .../active_directory_upstream_watcher.go | 18 +- .../active_directory_upstream_watcher_test.go | 41 +-- .../federation_domain_watcher.go | 299 +++++++++++++++++- .../ldap_upstream_watcher.go | 8 +- .../ldap_upstream_watcher_test.go | 3 +- .../oidc_upstream_watcher.go | 8 +- .../oidc_upstream_watcher_test.go | 5 +- .../upstreamwatchers/upstream_watchers.go | 4 +- .../supervisorstorage/garbage_collector.go | 11 +- .../garbage_collector_test.go | 21 +- internal/controller/utils.go | 12 +- .../fositestorage/accesstoken/accesstoken.go | 5 +- .../authorizationcode/authorizationcode.go | 5 +- .../openidconnect/openidconnect.go | 5 +- internal/fositestorage/pkce/pkce.go | 5 +- .../refreshtoken/refreshtoken.go | 5 +- .../generate.go | 4 +- .../mockupstreamoidcidentityprovider.go | 6 +- internal/oidc/auth/auth_handler.go | 165 ++++++---- internal/oidc/auth/auth_handler_test.go | 4 +- internal/oidc/callback/callback_handler.go | 25 +- .../downstreamsession/downstream_session.go | 68 +++- .../idpdiscovery/idp_discovery_handler.go | 22 +- .../idp_discovery_handler_test.go | 10 +- .../oidc/idplister/upstream_idp_lister.go | 26 ++ internal/oidc/login/get_login_handler_test.go | 5 +- internal/oidc/login/post_login_handler.go | 32 +- internal/oidc/oidc.go | 58 +--- .../provider/dynamic_upstream_idp_provider.go | 153 ++------- ...ration_domain_identity_providers_lister.go | 232 ++++++++++++++ .../oidc/provider/federation_domain_issuer.go | 40 ++- internal/oidc/provider/manager/manager.go | 57 ++-- .../upstreamprovider/upsteam_provider.go | 126 ++++++++ internal/oidc/token/token_handler.go | 190 +++++++---- internal/psession/pinniped_session.go | 19 +- internal/supervisor/server/server.go | 3 + .../testutil/oidctestutil/oidctestutil.go | 29 +- internal/upstreamldap/upstreamldap.go | 8 +- internal/upstreamldap/upstreamldap_test.go | 16 +- internal/upstreamoidc/upstreamoidc.go | 9 +- internal/upstreamoidc/upstreamoidc_test.go | 37 +-- pkg/oidcclient/login.go | 17 +- pkg/oidcclient/login_test.go | 30 +- 44 files changed, 1465 insertions(+), 626 deletions(-) create mode 100644 internal/oidc/idplister/upstream_idp_lister.go create mode 100644 internal/oidc/provider/federation_domain_identity_providers_lister.go create mode 100644 internal/oidc/provider/upstreamprovider/upsteam_provider.go diff --git a/hack/prepare-supervisor-on-kind.sh b/hack/prepare-supervisor-on-kind.sh index a56e5970..e8c937d0 100755 --- a/hack/prepare-supervisor-on-kind.sh +++ b/hack/prepare-supervisor-on-kind.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +# Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 # @@ -81,7 +81,7 @@ while (("$#")); do done if [[ "$use_oidc_upstream" == "no" && "$use_ldap_upstream" == "no" && "$use_ad_upstream" == "no" ]]; then - log_error "Error: Please use --oidc, --ldap, or --ad to specify which type of upstream identity provider(s) you would like" + log_error "Error: Please use --oidc, --ldap, or --ad to specify which type(s) of upstream identity provider(s) you would like. May use one or multiple." exit 1 fi @@ -103,42 +103,6 @@ audience="my-workload-cluster-$(openssl rand -hex 4)" issuer_host="pinniped-supervisor-clusterip.supervisor.svc.cluster.local" issuer="https://$issuer_host/some/path" -# Create a CA and TLS serving certificates for the Supervisor. -step certificate create \ - "Supervisor CA" "$root_ca_crt_path" "$root_ca_key_path" \ - --profile root-ca \ - --no-password --insecure --force -step certificate create \ - "$issuer_host" "$tls_crt_path" "$tls_key_path" \ - --profile leaf \ - --not-after 8760h \ - --ca "$root_ca_crt_path" --ca-key "$root_ca_key_path" \ - --no-password --insecure --force - -# Put the TLS certificate into a Secret for the Supervisor. -kubectl create secret tls -n "$PINNIPED_TEST_SUPERVISOR_NAMESPACE" my-federation-domain-tls --cert "$tls_crt_path" --key "$tls_key_path" \ - --dry-run=client --output yaml | kubectl apply -f - - -# Make a FederationDomain using the TLS Secret from above. -cat <kubeconfig +if [[ "$use_oidc_upstream" == "yes" ]]; then + https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" \ + ./pinniped get kubeconfig --oidc-skip-browser $flow_arg --upstream-identity-provider-type oidc >kubeconfig-oidc.yaml +fi +if [[ "$use_ldap_upstream" == "yes" ]]; then + https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" \ + ./pinniped get kubeconfig --oidc-skip-browser $flow_arg --upstream-identity-provider-type ldap >kubeconfig-ldap.yaml +fi +if [[ "$use_ad_upstream" == "yes" ]]; then + https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" \ + ./pinniped get kubeconfig --oidc-skip-browser $flow_arg --upstream-identity-provider-type activedirectory >kubeconfig-ad.yaml +fi # Clear the local CLI cache to ensure that the kubectl command below will need to perform a fresh login. rm -f "$HOME/.config/pinniped/sessions.yaml" @@ -304,37 +420,48 @@ echo "Ready! 🚀" if [[ "$use_oidc_upstream" == "yes" || "$use_flow" == "browser_authcode" ]]; then echo - echo "To be able to access the login URL shown below, start Chrome like this:" + echo "To be able to access the Supervisor URL during login, start Chrome like this:" echo " open -a \"Google Chrome\" --args --proxy-server=\"$PINNIPED_TEST_PROXY\"" echo "Note that Chrome must be fully quit before being started with --proxy-server." echo "Then open the login URL shown below in that new Chrome window." echo echo "When prompted for username and password, use these values:" + echo fi if [[ "$use_oidc_upstream" == "yes" ]]; then - echo " Username: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME" - echo " Password: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD" + echo " OIDC Username: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME" + echo " OIDC Password: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD" + echo fi if [[ "$use_ldap_upstream" == "yes" ]]; then - echo " Username: $PINNIPED_TEST_LDAP_USER_CN" - echo " Password: $PINNIPED_TEST_LDAP_USER_PASSWORD" + echo " LDAP Username: $PINNIPED_TEST_LDAP_USER_CN" + echo " LDAP Password: $PINNIPED_TEST_LDAP_USER_PASSWORD" + echo fi if [[ "$use_ad_upstream" == "yes" ]]; then - echo " Username: $PINNIPED_TEST_AD_USER_USER_PRINCIPAL_NAME" - echo " Password: $PINNIPED_TEST_AD_USER_PASSWORD" + echo " AD Username: $PINNIPED_TEST_AD_USER_USER_PRINCIPAL_NAME" + echo " AD Password: $PINNIPED_TEST_AD_USER_PASSWORD" + echo fi -# Perform a login using the kubectl plugin. This should print the URL to be followed for the Dex login page -# if using an OIDC upstream, or should prompt on the CLI for username/password if using an LDAP upstream. -echo -echo "Running: PINNIPED_DEBUG=true https_proxy=\"$PINNIPED_TEST_PROXY\" no_proxy=\"127.0.0.1\" kubectl --kubeconfig ./kubeconfig get pods -A" -PINNIPED_DEBUG=true https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" kubectl --kubeconfig ./kubeconfig get pods -A - -# Print the identity of the currently logged in user. The CLI has cached your tokens, and will automatically refresh -# your short-lived credentials whenever they expire, so you should not be prompted to log in again for the rest of the day. -echo -echo "Running: PINNIPED_DEBUG=true https_proxy=\"$PINNIPED_TEST_PROXY\" no_proxy=\"127.0.0.1\" ./pinniped whoami --kubeconfig ./kubeconfig" -PINNIPED_DEBUG=true https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" ./pinniped whoami --kubeconfig ./kubeconfig +# Echo the commands that may be used to login and print the identity of the currently logged in user. +# Once the CLI has cached your tokens, it will automatically refresh your short-lived credentials whenever +# they expire, so you should not be prompted to log in again for the rest of the day. +if [[ "$use_oidc_upstream" == "yes" ]]; then + echo "To log in using OIDC, run:" + echo "PINNIPED_DEBUG=true https_proxy=\"$PINNIPED_TEST_PROXY\" no_proxy=\"127.0.0.1\" ./pinniped whoami --kubeconfig ./kubeconfig-oidc.yaml" + echo +fi +if [[ "$use_ldap_upstream" == "yes" ]]; then + echo "To log in using LDAP, run:" + echo "PINNIPED_DEBUG=true https_proxy=\"$PINNIPED_TEST_PROXY\" no_proxy=\"127.0.0.1\" ./pinniped whoami --kubeconfig ./kubeconfig-ldap.yaml" + echo +fi +if [[ "$use_ad_upstream" == "yes" ]]; then + echo "To log in using AD, run:" + echo "PINNIPED_DEBUG=true https_proxy=\"$PINNIPED_TEST_PROXY\" no_proxy=\"127.0.0.1\" ./pinniped whoami --kubeconfig ./kubeconfig-ad.yaml" + echo +fi diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index f2d658f6..b1f21496 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -26,7 +26,7 @@ import ( "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamldap" ) @@ -225,7 +225,7 @@ func (s *activeDirectoryUpstreamGenericLDAPStatus) Conditions() []metav1.Conditi // UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamActiveDirectoryIdentityProviderICache interface { - SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI) + SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI) } type activeDirectoryWatcherController struct { @@ -299,7 +299,7 @@ func (c *activeDirectoryWatcherController) Sync(ctx controllerlib.Context) error } requeue := false - validatedUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) + validatedUpstreams := make([]upstreamprovider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) for _, upstream := range actualUpstreams { valid, requestedRequeue := c.validateUpstream(ctx.Context, upstream) if valid != nil { @@ -318,7 +318,7 @@ func (c *activeDirectoryWatcherController) Sync(ctx controllerlib.Context) error return nil } -func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider) (p provider.UpstreamLDAPIdentityProviderI, requeue bool) { +func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider) (p upstreamprovider.UpstreamLDAPIdentityProviderI, requeue bool) { spec := upstream.Spec adUpstreamImpl := &activeDirectoryUpstreamGenericLDAPImpl{activeDirectoryIdentityProvider: *upstream} @@ -344,7 +344,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){ "objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"), }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ pwdLastSetAttribute: attributeUnchangedSinceLogin(pwdLastSetAttribute), userAccountControlAttribute: validUserAccountControl, userAccountControlComputedAttribute: validComputedUserAccountControl, @@ -445,7 +445,7 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { } //nolint:gochecknoglobals // this needs to be a global variable so that tests can check pointer equality -var validUserAccountControl = func(entry *ldap.Entry, _ provider.RefreshAttributes) error { +var validUserAccountControl = func(entry *ldap.Entry, _ upstreamprovider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) if err != nil { return err @@ -459,7 +459,7 @@ var validUserAccountControl = func(entry *ldap.Entry, _ provider.RefreshAttribut } //nolint:gochecknoglobals // this needs to be a global variable so that tests can check pointer equality -var validComputedUserAccountControl = func(entry *ldap.Entry, _ provider.RefreshAttributes) error { +var validComputedUserAccountControl = func(entry *ldap.Entry, _ upstreamprovider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute)) if err != nil { return err @@ -473,8 +473,8 @@ var validComputedUserAccountControl = func(entry *ldap.Entry, _ provider.Refresh } //nolint:gochecknoglobals // this needs to be a global variable so that tests can check pointer equality -var attributeUnchangedSinceLogin = func(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error { - return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error { +var attributeUnchangedSinceLogin = func(attribute string) func(*ldap.Entry, upstreamprovider.RefreshAttributes) error { + return func(entry *ldap.Entry, storedAttributes upstreamprovider.RefreshAttributes) error { prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] newValues := entry.GetRawAttributeValues(attribute) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index bb830aa2..c223441d 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -31,6 +31,7 @@ import ( "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/upstreamldap" ) @@ -229,7 +230,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -572,7 +573,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -642,7 +643,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -715,7 +716,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -795,7 +796,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -859,7 +860,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1010,7 +1011,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1160,7 +1161,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1232,7 +1233,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1499,7 +1500,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1559,7 +1560,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1623,7 +1624,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1687,7 +1688,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1899,7 +1900,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupSearchNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1962,7 +1963,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { SkipGroupRefresh: true, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ "pwdLastSet": attributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -2010,7 +2011,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset(tt.inputSecrets...) kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) cache := provider.NewDynamicUpstreamIDPProvider() - cache.SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + cache.SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ upstreamldap.New(upstreamldap.ProviderConfig{Name: "initial-entry"}), }) @@ -2104,8 +2105,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { expectedRefreshAttributeChecks := copyOfExpectedValueForResultingCache.RefreshAttributeChecks actualRefreshAttributeChecks := actualConfig.RefreshAttributeChecks - copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} - actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} + copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{} + actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{} require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks)) for k, v := range expectedRefreshAttributeChecks { require.NotNil(t, actualRefreshAttributeChecks[k]) @@ -2354,7 +2355,7 @@ func TestValidUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validUserAccountControl(tt.entry, provider.RefreshAttributes{}) + err := validUserAccountControl(tt.entry, upstreamprovider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) @@ -2415,7 +2416,7 @@ func TestValidComputedUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validComputedUserAccountControl(tt.entry, provider.RefreshAttributes{}) + err := validComputedUserAccountControl(tt.entry, upstreamprovider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 08492e17..9df42508 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisorconfig @@ -8,9 +8,11 @@ import ( "fmt" "net/url" "strings" + "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" @@ -19,8 +21,11 @@ import ( configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" configinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/config/v1alpha1" + idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1" + "go.pinniped.dev/internal/celtransformer" pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" ) @@ -33,10 +38,14 @@ type ProvidersSetter interface { } type federationDomainWatcherController struct { - providerSetter ProvidersSetter - clock clock.Clock - client pinnipedclientset.Interface - federationDomainInformer configinformers.FederationDomainInformer + providerSetter ProvidersSetter + clock clock.Clock + client pinnipedclientset.Interface + + federationDomainInformer configinformers.FederationDomainInformer + oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer + ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer + activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer } // NewFederationDomainWatcherController creates a controllerlib.Controller that watches @@ -46,16 +55,22 @@ func NewFederationDomainWatcherController( clock clock.Clock, client pinnipedclientset.Interface, federationDomainInformer configinformers.FederationDomainInformer, + oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer, + ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, + activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: "FederationDomainWatcherController", Syncer: &federationDomainWatcherController{ - providerSetter: providerSetter, - clock: clock, - client: client, - federationDomainInformer: federationDomainInformer, + providerSetter: providerSetter, + clock: clock, + client: client, + federationDomainInformer: federationDomainInformer, + oidcIdentityProviderInformer: oidcIdentityProviderInformer, + ldapIdentityProviderInformer: ldapIdentityProviderInformer, + activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer, }, }, withInformer( @@ -63,6 +78,27 @@ func NewFederationDomainWatcherController( pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), + withInformer( + oidcIdentityProviderInformer, + // Since this controller only cares about IDP metadata names and UIDs (immutable fields), + // we only need to trigger Sync on creates and deletes. + pinnipedcontroller.MatchAnythingIgnoringUpdatesFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + ldapIdentityProviderInformer, + // Since this controller only cares about IDP metadata names and UIDs (immutable fields), + // we only need to trigger Sync on creates and deletes. + pinnipedcontroller.MatchAnythingIgnoringUpdatesFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + activeDirectoryIdentityProviderInformer, + // Since this controller only cares about IDP metadata names and UIDs (immutable fields), + // we only need to trigger Sync on creates and deletes. + pinnipedcontroller.MatchAnythingIgnoringUpdatesFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), ) } @@ -143,8 +179,239 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro continue } - federationDomainIssuer, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer) // This validates the Issuer URL. + // TODO: Move all this identity provider stuff into helper functions. This is just a sketch of how the code would + // work in the sense that this is not doing error handling, is not validating everything that it should, and + // is not updating the status of the FederationDomain with anything related to these identity providers. + // This code may crash on invalid inputs since it is not handling any errors. However, when given valid inputs, + // this correctly implements the multiple IDPs features. + // Create the list of IDPs for this FederationDomain. + // Don't worry if the IDP CRs themselves is phase=Ready because those which are not ready will not be loaded + // into the provider cache, so they cannot actually be used to authenticate. + federationDomainIdentityProviders := []*provider.FederationDomainIdentityProvider{} + var defaultFederationDomainIdentityProvider *provider.FederationDomainIdentityProvider + if len(federationDomain.Spec.IdentityProviders) == 0 { + // When the FederationDomain does not list any IDPs, then we might be in backwards compatibility mode. + oidcIdentityProviders, _ := c.oidcIdentityProviderInformer.Lister().List(labels.Everything()) + ldapIdentityProviders, _ := c.ldapIdentityProviderInformer.Lister().List(labels.Everything()) + activeDirectoryIdentityProviders, _ := c.activeDirectoryIdentityProviderInformer.Lister().List(labels.Everything()) + // TODO handle err return value for each of the above three lines + + // Check if that there is exactly one IDP defined in the Supervisor namespace of any IDP CRD type. + idpCRsCount := len(oidcIdentityProviders) + len(ldapIdentityProviders) + len(activeDirectoryIdentityProviders) + if idpCRsCount == 1 { + // If so, default that IDP's DisplayName to be the same as its resource Name. + defaultFederationDomainIdentityProvider = &provider.FederationDomainIdentityProvider{} + switch { + case len(oidcIdentityProviders) == 1: + defaultFederationDomainIdentityProvider.DisplayName = oidcIdentityProviders[0].Name + defaultFederationDomainIdentityProvider.UID = oidcIdentityProviders[0].UID + case len(ldapIdentityProviders) == 1: + defaultFederationDomainIdentityProvider.DisplayName = ldapIdentityProviders[0].Name + defaultFederationDomainIdentityProvider.UID = ldapIdentityProviders[0].UID + case len(activeDirectoryIdentityProviders) == 1: + defaultFederationDomainIdentityProvider.DisplayName = activeDirectoryIdentityProviders[0].Name + defaultFederationDomainIdentityProvider.UID = activeDirectoryIdentityProviders[0].UID + } + // Backwards compatibility mode always uses an empty identity transformation pipline since no + // transformations are defined on the FederationDomain. + defaultFederationDomainIdentityProvider.Transforms = idtransform.NewTransformationPipeline() + plog.Warning("detected FederationDomain identity provider backwards compatibility mode: using the one existing identity provider for authentication", + "federationDomain", federationDomain.Name) + } else { + // There are no IDP CRs or there is more than one IDP CR. Either way, we are not in the backwards + // compatibility mode because there is not exactly one IDP CR in the namespace, despite the fact that no + // IDPs are listed on the FederationDomain. Create a FederationDomain which has no IDPs and therefore + // cannot actually be used to log in, but still serves endpoints. + // TODO: Write something into the FederationDomain's status to explain what's happening and how to fix it. + plog.Warning("FederationDomain has no identity providers listed and there is not exactly one identity provider defined in the namespace: authentication disabled", + "federationDomain", federationDomain.Name, + "namespace", federationDomain.Namespace, + "identityProvidersCustomResourcesCount", idpCRsCount, + ) + } + } + + // If there is an explicit list of IDPs on the FederationDomain, then process the list. + celTransformer, _ := celtransformer.NewCELTransformer(time.Second) // TODO: what is a good duration limit here? + // TODO: handle err + for _, idp := range federationDomain.Spec.IdentityProviders { + var idpResourceUID types.UID + var idpResourceName string + // TODO: Validate that all displayNames are unique within this FederationDomain's spec's list of identity providers. + // TODO: Validate that idp.ObjectRef.APIGroup is the expected APIGroup for IDP CRs "idp.supervisor.pinniped.dev" + // Validate that each objectRef resolves to an existing IDP. It does not matter if the IDP itself + // is phase=Ready, because it will not be loaded into the cache if not ready. For each objectRef + // that does not resolve, put an error on the FederationDomain status. + switch idp.ObjectRef.Kind { + case "LDAPIdentityProvider": + ldapIDP, _ := c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name) + // TODO: handle notfound err and also unexpected errors + idpResourceName = ldapIDP.Name + idpResourceUID = ldapIDP.UID + case "ActiveDirectoryIdentityProvider": + adIDP, _ := c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name) + // TODO: handle notfound err and also unexpected errors + idpResourceName = adIDP.Name + idpResourceUID = adIDP.UID + case "OIDCIdentityProvider": + oidcIDP, _ := c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name) + // TODO: handle notfound err and also unexpected errors + idpResourceName = oidcIDP.Name + idpResourceUID = oidcIDP.UID + default: + // TODO: handle bad user input + } + plog.Debug("resolved identity provider object reference", + "kind", idp.ObjectRef.Kind, + "name", idp.ObjectRef.Name, + "foundResourceName", idpResourceName, + "foundResourceUID", idpResourceUID, + ) + + // Prepare the transformations. + pipeline := idtransform.NewTransformationPipeline() + consts := &celtransformer.TransformationConstants{ + StringConstants: map[string]string{}, + StringListConstants: map[string][]string{}, + } + // Read all the declared constants. + for _, c := range idp.Transforms.Constants { + switch c.Type { + case "string": + consts.StringConstants[c.Name] = c.StringValue + case "stringList": + consts.StringListConstants[c.Name] = c.StringListValue + default: + // TODO: this shouldn't really happen since the CRD validates it, but handle it as an error + } + } + // Compile all the expressions and add them to the pipeline. + for idx, e := range idp.Transforms.Expressions { + var rawTransform celtransformer.CELTransformation + switch e.Type { + case "username/v1": + rawTransform = &celtransformer.UsernameTransformation{Expression: e.Expression} + case "groups/v1": + rawTransform = &celtransformer.GroupsTransformation{Expression: e.Expression} + case "policy/v1": + rawTransform = &celtransformer.AllowAuthenticationPolicy{ + Expression: e.Expression, + RejectedAuthenticationMessage: e.Message, + } + default: + // TODO: this shouldn't really happen since the CRD validates it, but handle it as an error + } + compiledTransform, err := celTransformer.CompileTransformation(rawTransform, consts) + if err != nil { + // TODO: handle compile err + plog.Error("error compiling identity transformation", err, + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "transformationIndex", idx, + "transformationType", e.Type, + "transformationExpression", e.Expression, + ) + } + pipeline.AppendTransformation(compiledTransform) + plog.Debug("successfully compiled identity transformation expression", + "type", e.Type, + "expr", e.Expression, + "policyMessage", e.Message, + ) + } + // Run all the provided transform examples. If any fail, put errors on the FederationDomain status. + for idx, e := range idp.Transforms.Examples { + // TODO: use a real context param below + result, _ := pipeline.Evaluate(context.TODO(), e.Username, e.Groups) + // TODO: handle err + resultWasAuthRejected := !result.AuthenticationAllowed + if e.Expects.Rejected && !resultWasAuthRejected { + // TODO: handle this failed example + plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not", + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "exampleIndex", idx, + "expectedRejected", e.Expects.Rejected, + "actualRejectedResult", resultWasAuthRejected, + "expectedMessage", e.Expects.Message, + "actualMessageResult", result.RejectedAuthenticationMessage, + ) + } else if !e.Expects.Rejected && resultWasAuthRejected { + // TODO: handle this failed example + plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected", + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "exampleIndex", idx, + "expectedRejected", e.Expects.Rejected, + "actualRejectedResult", resultWasAuthRejected, + "expectedMessage", e.Expects.Message, + "actualMessageResult", result.RejectedAuthenticationMessage, + ) + } else if e.Expects.Rejected && resultWasAuthRejected && e.Expects.Message != result.RejectedAuthenticationMessage { + // TODO: when expected message is blank, then treat it like it expects the default message + // TODO: handle this failed example + plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message", + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "exampleIndex", idx, + "expectedRejected", e.Expects.Rejected, + "actualRejectedResult", resultWasAuthRejected, + "expectedMessage", e.Expects.Message, + "actualMessageResult", result.RejectedAuthenticationMessage, + ) + } else if result.AuthenticationAllowed { + // In the case where the user expected the auth to be allowed and it was allowed, then compare + // the expected username and group names to the actual username and group names. + // TODO: when both of these fail, put both errors onto the status (not just the first one) + if e.Expects.Username != result.Username { + // TODO: handle this failed example + plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username", + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "exampleIndex", idx, + "expectedUsername", e.Expects.Username, + "actualUsernameResult", result.Username, + ) + } + if !stringSlicesEqual(e.Expects.Groups, result.Groups) { + // TODO: Do we need to make this insensitive to ordering, or should the transformations evaluator be changed to always return sorted group names at the end of the pipeline? + // TODO: What happens if the user did not write any group expectation? Treat it like expecting any empty list of groups? + // TODO: handle this failed example + plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list", + "federationDomain", federationDomain.Name, + "idpDisplayName", idp.DisplayName, + "exampleIndex", idx, + "expectedGroups", e.Expects.Groups, + "actualGroupsResult", result.Groups, + ) + } + } + } + // For each valid IDP (unique displayName, valid objectRef + valid transforms), add it to the list. + federationDomainIdentityProviders = append(federationDomainIdentityProviders, &provider.FederationDomainIdentityProvider{ + DisplayName: idp.DisplayName, + UID: idpResourceUID, + Transforms: pipeline, + }) + plog.Debug("loaded FederationDomain identity provider", + "federationDomain", federationDomain.Name, + "identityProviderDisplayName", idp.DisplayName, + "identityProviderResourceUID", idpResourceUID, + ) + } + + // Now that we have the list of IDPs for this FederationDomain, create the issuer. + var federationDomainIssuer *provider.FederationDomainIssuer + err = nil + if defaultFederationDomainIdentityProvider != nil { + // This is the constructor for the backwards compatibility mode. + federationDomainIssuer, err = provider.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider) + } else { + // This is the constructor for any other case, including when there is an empty list of IDPs. + federationDomainIssuer, err = provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer, federationDomainIdentityProviders) + } if err != nil { + // Note that the FederationDomainIssuer constructors validate the Issuer URL. if err := c.updateStatus( ctx.Context, federationDomain.Namespace, @@ -176,6 +443,18 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro return errors.NewAggregate(errs) } +func stringSlicesEqual(a []string, b []string) bool { + if len(a) != len(b) { + return false + } + for i, itemFromA := range a { + if b[i] != itemFromA { + return false + } + } + return true +} + func (c *federationDomainWatcherController) updateStatus( ctx context.Context, namespace, name string, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index aa9ce940..fcf3a7e3 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -20,7 +20,7 @@ import ( "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamldap" ) @@ -133,7 +133,7 @@ func (s *ldapUpstreamGenericLDAPStatus) Conditions() []metav1.Condition { // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamLDAPIdentityProviderICache interface { - SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI) + SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI) } type ldapWatcherController struct { @@ -207,7 +207,7 @@ func (c *ldapWatcherController) Sync(ctx controllerlib.Context) error { } requeue := false - validatedUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) + validatedUpstreams := make([]upstreamprovider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) for _, upstream := range actualUpstreams { valid, requestedRequeue := c.validateUpstream(ctx.Context, upstream) if valid != nil { @@ -226,7 +226,7 @@ func (c *ldapWatcherController) Sync(ctx controllerlib.Context) error { return nil } -func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider) (p provider.UpstreamLDAPIdentityProviderI, requeue bool) { +func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider) (p upstreamprovider.UpstreamLDAPIdentityProviderI, requeue bool) { spec := upstream.Spec config := &upstreamldap.ProviderConfig{ diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index cc8e0188..46b0b1a8 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -30,6 +30,7 @@ import ( "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/upstreamldap" ) @@ -1139,7 +1140,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset(tt.inputSecrets...) kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) cache := provider.NewDynamicUpstreamIDPProvider() - cache.SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + cache.SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ upstreamldap.New(upstreamldap.ProviderConfig{Name: "initial-entry"}), }) diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 3cfbc7e2..f56e4fc9 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -35,7 +35,7 @@ import ( "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/net/phttp" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamoidc" ) @@ -91,7 +91,7 @@ var ( // UpstreamOIDCIdentityProviderICache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations. type UpstreamOIDCIdentityProviderICache interface { - SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI) + SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI) } // lruValidatorCache caches the *oidc.Provider associated with a particular issuer/TLS configuration. @@ -175,13 +175,13 @@ func (c *oidcWatcherController) Sync(ctx controllerlib.Context) error { } requeue := false - validatedUpstreams := make([]provider.UpstreamOIDCIdentityProviderI, 0, len(actualUpstreams)) + validatedUpstreams := make([]upstreamprovider.UpstreamOIDCIdentityProviderI, 0, len(actualUpstreams)) for _, upstream := range actualUpstreams { valid := c.validateUpstream(ctx, upstream) if valid == nil { requeue = true } else { - validatedUpstreams = append(validatedUpstreams, provider.UpstreamOIDCIdentityProviderI(valid)) + validatedUpstreams = append(validatedUpstreams, upstreamprovider.UpstreamOIDCIdentityProviderI(valid)) } } c.cache.SetOIDCIdentityProviders(validatedUpstreams) diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 7077cf57..814b425a 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -29,6 +29,7 @@ import ( "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" @@ -81,7 +82,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset() kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) cache := provider.NewDynamicUpstreamIDPProvider() - cache.SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI{ + cache.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{ &upstreamoidc.ProviderConfig{Name: "initial-entry"}, }) secretInformer := kubeInformers.Core().V1().Secrets() @@ -1416,7 +1417,7 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) testLog := testlogger.NewLegacy(t) //nolint:staticcheck // old test with lots of log statements cache := provider.NewDynamicUpstreamIDPProvider() - cache.SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI{ + cache.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{ &upstreamoidc.ProviderConfig{Name: "initial-entry"}, }) diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 1ab87787..114a66b5 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -16,7 +16,7 @@ import ( "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/constable" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamldap" ) @@ -365,7 +365,7 @@ func validateAndSetLDAPServerConnectivityAndSearchBase( return ldapConnectionValidCondition, searchBaseFoundCondition } -func EvaluateConditions(conditions GradatedConditions, config *upstreamldap.ProviderConfig) (provider.UpstreamLDAPIdentityProviderI, bool) { +func EvaluateConditions(conditions GradatedConditions, config *upstreamldap.ProviderConfig) (upstreamprovider.UpstreamLDAPIdentityProviderI, bool) { for _, gradatedCondition := range conditions.gradatedConditions { if gradatedCondition.condition.Status != metav1.ConditionTrue && gradatedCondition.isFatal { // Invalid provider, so do not load it into the cache. diff --git a/internal/controller/supervisorstorage/garbage_collector.go b/internal/controller/supervisorstorage/garbage_collector.go index c11aa8c3..0006f687 100644 --- a/internal/controller/supervisorstorage/garbage_collector.go +++ b/internal/controller/supervisorstorage/garbage_collector.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisorstorage @@ -27,6 +27,7 @@ import ( "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestorage/refreshtoken" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" ) @@ -43,7 +44,7 @@ type garbageCollectorController struct { // UpstreamOIDCIdentityProviderICache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations. type UpstreamOIDCIdentityProviderICache interface { - GetOIDCIdentityProviders() []provider.UpstreamOIDCIdentityProviderI + GetOIDCIdentityProviders() []upstreamprovider.UpstreamOIDCIdentityProviderI } func GarbageCollectorController( @@ -244,7 +245,7 @@ func (c *garbageCollectorController) tryRevokeUpstreamOIDCToken(ctx context.Cont } // Try to find the provider that was originally used to create the stored session. - var foundOIDCIdentityProviderI provider.UpstreamOIDCIdentityProviderI + var foundOIDCIdentityProviderI upstreamprovider.UpstreamOIDCIdentityProviderI for _, p := range c.idpCache.GetOIDCIdentityProviders() { if p.GetName() == customSessionData.ProviderName && p.GetResourceUID() == customSessionData.ProviderUID { foundOIDCIdentityProviderI = p @@ -260,7 +261,7 @@ func (c *garbageCollectorController) tryRevokeUpstreamOIDCToken(ctx context.Cont upstreamAccessToken := customSessionData.OIDC.UpstreamAccessToken if upstreamRefreshToken != "" { - err := foundOIDCIdentityProviderI.RevokeToken(ctx, upstreamRefreshToken, provider.RefreshTokenType) + err := foundOIDCIdentityProviderI.RevokeToken(ctx, upstreamRefreshToken, upstreamprovider.RefreshTokenType) if err != nil { return err } @@ -268,7 +269,7 @@ func (c *garbageCollectorController) tryRevokeUpstreamOIDCToken(ctx context.Cont } if upstreamAccessToken != "" { - err := foundOIDCIdentityProviderI.RevokeToken(ctx, upstreamAccessToken, provider.AccessTokenType) + err := foundOIDCIdentityProviderI.RevokeToken(ctx, upstreamAccessToken, upstreamprovider.AccessTokenType) if err != nil { return err } diff --git a/internal/controller/supervisorstorage/garbage_collector_test.go b/internal/controller/supervisorstorage/garbage_collector_test.go index d845e558..2266725b 100644 --- a/internal/controller/supervisorstorage/garbage_collector_test.go +++ b/internal/controller/supervisorstorage/garbage_collector_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisorstorage @@ -30,6 +30,7 @@ import ( "go.pinniped.dev/internal/fositestorage/refreshtoken" "go.pinniped.dev/internal/oidc/clientregistry" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" @@ -369,7 +370,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -493,7 +494,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-access-token", - TokenType: provider.AccessTokenType, + TokenType: upstreamprovider.AccessTokenType, }, ) @@ -785,7 +786,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -810,7 +811,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -889,7 +890,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -1012,7 +1013,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -1136,7 +1137,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-access-token", - TokenType: provider.AccessTokenType, + TokenType: upstreamprovider.AccessTokenType, }, ) @@ -1214,7 +1215,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-refresh-token", - TokenType: provider.RefreshTokenType, + TokenType: upstreamprovider.RefreshTokenType, }, ) @@ -1291,7 +1292,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) { &oidctestutil.RevokeTokenArgs{ Ctx: syncContext.Context, Token: "fake-upstream-access-token", - TokenType: provider.AccessTokenType, + TokenType: upstreamprovider.AccessTokenType, }, ) diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 55e02d45..6880fc2d 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package controller @@ -18,6 +18,16 @@ func NameAndNamespaceExactMatchFilterFactory(name, namespace string) controllerl }, nil) } +// MatchAnythingIgnoringUpdatesFilter returns a controllerlib.Filter that allows all objects but ignores updates. +func MatchAnythingIgnoringUpdatesFilter(parentFunc controllerlib.ParentFunc) controllerlib.Filter { + return controllerlib.FilterFuncs{ + AddFunc: func(object metav1.Object) bool { return true }, + UpdateFunc: func(oldObj, newObj metav1.Object) bool { return false }, + DeleteFunc: func(object metav1.Object) bool { return true }, + ParentFunc: parentFunc, + } +} + // MatchAnythingFilter returns a controllerlib.Filter that allows all objects. func MatchAnythingFilter(parentFunc controllerlib.ParentFunc) controllerlib.Filter { return SimpleFilter(func(object metav1.Object) bool { return true }, parentFunc) diff --git a/internal/fositestorage/accesstoken/accesstoken.go b/internal/fositestorage/accesstoken/accesstoken.go index a70f1a70..07b065ed 100644 --- a/internal/fositestorage/accesstoken/accesstoken.go +++ b/internal/fositestorage/accesstoken/accesstoken.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package accesstoken @@ -31,7 +31,8 @@ const ( // Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request. // Version 3 is when we added the Username field to the psession.CustomSessionData. // Version 4 is when fosite added json tags to their openid.DefaultSession struct. - accessTokenStorageVersion = "4" + // Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData. + accessTokenStorageVersion = "5" ) type RevocationStorage interface { diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index fd34ce2c..abfa9e39 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package authorizationcode @@ -32,7 +32,8 @@ const ( // Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request. // Version 3 is when we added the Username field to the psession.CustomSessionData. // Version 4 is when fosite added json tags to their openid.DefaultSession struct. - authorizeCodeStorageVersion = "4" + // Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData. + authorizeCodeStorageVersion = "5" ) var _ oauth2.AuthorizeCodeStorage = &authorizeCodeStorage{} diff --git a/internal/fositestorage/openidconnect/openidconnect.go b/internal/fositestorage/openidconnect/openidconnect.go index d020e885..4770e41f 100644 --- a/internal/fositestorage/openidconnect/openidconnect.go +++ b/internal/fositestorage/openidconnect/openidconnect.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package openidconnect @@ -32,7 +32,8 @@ const ( // Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request. // Version 3 is when we added the Username field to the psession.CustomSessionData. // Version 4 is when fosite added json tags to their openid.DefaultSession struct. - oidcStorageVersion = "4" + // Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData. + oidcStorageVersion = "5" ) var _ openid.OpenIDConnectRequestStorage = &openIDConnectRequestStorage{} diff --git a/internal/fositestorage/pkce/pkce.go b/internal/fositestorage/pkce/pkce.go index 92fe9f83..3f44a00d 100644 --- a/internal/fositestorage/pkce/pkce.go +++ b/internal/fositestorage/pkce/pkce.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package pkce @@ -30,7 +30,8 @@ const ( // Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request. // Version 3 is when we added the Username field to the psession.CustomSessionData. // Version 4 is when fosite added json tags to their openid.DefaultSession struct. - pkceStorageVersion = "4" + // Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData. + pkceStorageVersion = "5" ) var _ pkce.PKCERequestStorage = &pkceStorage{} diff --git a/internal/fositestorage/refreshtoken/refreshtoken.go b/internal/fositestorage/refreshtoken/refreshtoken.go index 28c2b5cb..9feaed55 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken.go +++ b/internal/fositestorage/refreshtoken/refreshtoken.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package refreshtoken @@ -31,7 +31,8 @@ const ( // Version 2 is when we switched to storing psession.PinnipedSession inside the fosite request. // Version 3 is when we added the Username field to the psession.CustomSessionData. // Version 4 is when fosite added json tags to their openid.DefaultSession struct. - refreshTokenStorageVersion = "4" + // Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData. + refreshTokenStorageVersion = "5" ) type RevocationStorage interface { diff --git a/internal/mocks/mockupstreamoidcidentityprovider/generate.go b/internal/mocks/mockupstreamoidcidentityprovider/generate.go index cb9c46df..d1ba84a0 100644 --- a/internal/mocks/mockupstreamoidcidentityprovider/generate.go +++ b/internal/mocks/mockupstreamoidcidentityprovider/generate.go @@ -1,6 +1,6 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package mockupstreamoidcidentityprovider -//go:generate go run -v github.com/golang/mock/mockgen -destination=mockupstreamoidcidentityprovider.go -package=mockupstreamoidcidentityprovider -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/oidc/provider UpstreamOIDCIdentityProviderI +//go:generate go run -v github.com/golang/mock/mockgen -destination=mockupstreamoidcidentityprovider.go -package=mockupstreamoidcidentityprovider -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/oidc/provider/upstreamprovider UpstreamOIDCIdentityProviderI diff --git a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go index 15a709a5..cc66519f 100644 --- a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go +++ b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go @@ -3,7 +3,7 @@ // // Code generated by MockGen. DO NOT EDIT. -// Source: go.pinniped.dev/internal/oidc/provider (interfaces: UpstreamOIDCIdentityProviderI) +// Source: go.pinniped.dev/internal/oidc/provider/upstreamprovider (interfaces: UpstreamOIDCIdentityProviderI) // Package mockupstreamoidcidentityprovider is a generated GoMock package. package mockupstreamoidcidentityprovider @@ -14,7 +14,7 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - provider "go.pinniped.dev/internal/oidc/provider" + upstreamprovider "go.pinniped.dev/internal/oidc/provider/upstreamprovider" nonce "go.pinniped.dev/pkg/oidcclient/nonce" oidctypes "go.pinniped.dev/pkg/oidcclient/oidctypes" pkce "go.pinniped.dev/pkg/oidcclient/pkce" @@ -245,7 +245,7 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) PerformRefresh(arg0, ar } // RevokeToken mocks base method. -func (m *MockUpstreamOIDCIdentityProviderI) RevokeToken(arg0 context.Context, arg1 string, arg2 provider.RevocableTokenType) error { +func (m *MockUpstreamOIDCIdentityProviderI) RevokeToken(arg0 context.Context, arg1 string, arg2 upstreamprovider.RevocableTokenType) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RevokeToken", arg0, arg1, arg2) ret0, _ := ret[0].(error) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 772d1291..7b94694f 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -18,12 +18,14 @@ import ( 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/idtransform" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/downstreamsession" "go.pinniped.dev/internal/oidc/login" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/formposthtml" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -37,7 +39,7 @@ const ( func NewHandler( downstreamIssuer string, - idpLister oidc.UpstreamIdentityProvidersLister, + idpFinder provider.FederationDomainIdentityProvidersFinderI, oauthHelperWithoutStorage fosite.OAuth2Provider, oauthHelperWithStorage fosite.OAuth2Provider, generateCSRF func() (csrftoken.CSRFToken, error), @@ -57,20 +59,25 @@ func NewHandler( // 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 oidcapi.FindUpstreamIDPByNameAndType() when the params are present. - oidcUpstream, ldapUpstream, idpType, err := chooseUpstreamIDP(idpLister) + idpNameQueryParamValue := r.URL.Query().Get(oidcapi.AuthorizeUpstreamIDPNameParamName) + oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpNameQueryParamValue, idpFinder) if err != nil { plog.WarningErr("authorize upstream config", err) return err } - if idpType == psession.ProviderTypeOIDC { + if oidcUpstream != nil { 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) + return handleAuthRequestForOIDCUpstreamPasswordGrant( + r, + w, + oauthHelperWithStorage, + oidcUpstream.Provider, + oidcUpstream.Transforms, + idpNameQueryParamValue, + ) } return handleAuthRequestForOIDCUpstreamBrowserFlow(r, w, oauthHelperWithoutStorage, @@ -79,6 +86,7 @@ func NewHandler( downstreamIssuer, upstreamStateEncoder, cookieCodec, + idpNameQueryParamValue, ) } @@ -88,8 +96,10 @@ func NewHandler( // The client set a username header, so they are trying to log in with a username/password. return handleAuthRequestForLDAPUpstreamCLIFlow(r, w, oauthHelperWithStorage, - ldapUpstream, - idpType, + ldapUpstream.Provider, + ldapUpstream.SessionProviderType, + ldapUpstream.Transforms, + idpNameQueryParamValue, ) } return handleAuthRequestForLDAPUpstreamBrowserFlow( @@ -100,10 +110,11 @@ func NewHandler( generateNonce, generatePKCE, ldapUpstream, - idpType, + ldapUpstream.SessionProviderType, downstreamIssuer, upstreamStateEncoder, cookieCodec, + idpNameQueryParamValue, ) }) @@ -117,24 +128,28 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, - ldapUpstream provider.UpstreamLDAPIdentityProviderI, + ldapUpstream upstreamprovider.UpstreamLDAPIdentityProviderI, idpType psession.ProviderType, + identityTransforms *idtransform.TransformationPipeline, + idpNameQueryParamValue string, ) error { authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, true) if !created { return nil } + maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) + if !requireStaticClientForUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) { return nil } - username, password, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) + submittedUsername, submittedPassword, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) if !hadUsernamePasswordValues { return nil } - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), submittedUsername, submittedPassword, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") @@ -146,9 +161,18 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( } subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse) - username = authenticateResponse.User.GetName() - groups := authenticateResponse.User.GetGroups() - customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username) + upstreamUsername := authenticateResponse.User.GetName() + upstreamGroups := authenticateResponse.User.GetGroups() + + username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), identityTransforms, upstreamUsername, upstreamGroups) + if err != nil { + oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, + ) + return nil + } + + customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username, upstreamUsername, upstreamGroups) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData, map[string]interface{}{}) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true) @@ -163,11 +187,12 @@ func handleAuthRequestForLDAPUpstreamBrowserFlow( generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), generatePKCE func() (pkce.Code, error), - ldapUpstream provider.UpstreamLDAPIdentityProviderI, + ldapUpstream *provider.FederationDomainResolvedLDAPIdentityProvider, idpType psession.ProviderType, downstreamIssuer string, upstreamStateEncoder oidc.Encoder, cookieCodec oidc.Codec, + idpNameQueryParamValue string, ) error { authRequestState, err := handleBrowserFlowAuthRequest( r, @@ -176,10 +201,11 @@ func handleAuthRequestForLDAPUpstreamBrowserFlow( generateCSRF, generateNonce, generatePKCE, - ldapUpstream.GetName(), + ldapUpstream.DisplayName, idpType, cookieCodec, upstreamStateEncoder, + idpNameQueryParamValue, ) if err != nil { return err @@ -196,18 +222,22 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, - oidcUpstream provider.UpstreamOIDCIdentityProviderI, + oidcUpstream upstreamprovider.UpstreamOIDCIdentityProviderI, + identityTransforms *idtransform.TransformationPipeline, + idpNameQueryParamValue string, ) error { authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, true) if !created { return nil } + maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) + if !requireStaticClientForUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) { return nil } - username, password, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) + submittedUsername, submittedPassword, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) if !hadUsernamePasswordValues { return nil } @@ -220,7 +250,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( return nil } - token, err := oidcUpstream.PasswordCredentialsGrantAndValidateTokens(r.Context(), username, password) + token, err := oidcUpstream.PasswordCredentialsGrantAndValidateTokens(r.Context(), submittedUsername, submittedPassword) if err != nil { // Upstream password grant errors can be generic errors (e.g. a network failure) or can be oauth2.RetrieveError errors // which represent the http response from the upstream server. These could be a 5XX or some other unexpected error, @@ -234,7 +264,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( return nil } - subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) + subject, upstreamUsername, upstreamGroups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) if err != nil { // Return a user-friendly error for this case which is entirely within our control. oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, @@ -243,9 +273,17 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( return nil } + username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), identityTransforms, upstreamUsername, upstreamGroups) + if err != nil { + oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, + ) + return nil + } + additionalClaims := downstreamsession.MapAdditionalClaimsFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) - customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token, username) + customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token, username, upstreamUsername, upstreamGroups) if err != nil { oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, @@ -268,10 +306,11 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), generatePKCE func() (pkce.Code, error), - oidcUpstream provider.UpstreamOIDCIdentityProviderI, + oidcUpstream *provider.FederationDomainResolvedOIDCIdentityProvider, downstreamIssuer string, upstreamStateEncoder oidc.Encoder, cookieCodec oidc.Codec, + idpNameQueryParamValue string, ) error { authRequestState, err := handleBrowserFlowAuthRequest( r, @@ -280,10 +319,11 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( generateCSRF, generateNonce, generatePKCE, - oidcUpstream.GetName(), + oidcUpstream.DisplayName, psession.ProviderTypeOIDC, cookieCodec, upstreamStateEncoder, + idpNameQueryParamValue, ) if err != nil { return err @@ -294,12 +334,12 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( } upstreamOAuthConfig := oauth2.Config{ - ClientID: oidcUpstream.GetClientID(), + ClientID: oidcUpstream.Provider.GetClientID(), Endpoint: oauth2.Endpoint{ - AuthURL: oidcUpstream.GetAuthorizationURL().String(), + AuthURL: oidcUpstream.Provider.GetAuthorizationURL().String(), }, RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer), - Scopes: oidcUpstream.GetScopes(), + Scopes: oidcUpstream.Provider.GetScopes(), } authCodeOptions := []oauth2.AuthCodeOption{ @@ -308,7 +348,7 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( authRequestState.pkce.Method(), } - for key, val := range oidcUpstream.GetAdditionalAuthcodeParams() { + for key, val := range oidcUpstream.Provider.GetAdditionalAuthcodeParams() { authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(key, val)) } @@ -382,39 +422,31 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { // chooseUpstreamIDP selects either an OIDC, an LDAP, or an AD IDP, or returns an error. // Note that AD and LDAP IDPs both return the same interface type, but different ProviderTypes values. -func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, psession.ProviderType, error) { - oidcUpstreams := idpLister.GetOIDCIdentityProviders() - ldapUpstreams := idpLister.GetLDAPIdentityProviders() - adUpstreams := idpLister.GetActiveDirectoryIdentityProviders() - switch { - case len(oidcUpstreams)+len(ldapUpstreams)+len(adUpstreams) == 0: - return nil, nil, "", httperr.New( - http.StatusUnprocessableEntity, - "No upstream providers are configured", - ) - case len(oidcUpstreams)+len(ldapUpstreams)+len(adUpstreams) > 1: - var upstreamIDPNames []string - for _, idp := range oidcUpstreams { - upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) - } - for _, idp := range ldapUpstreams { - upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) - } - for _, idp := range adUpstreams { - upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) - } - plog.Warning("Too many upstream providers are configured (found: %s)", upstreamIDPNames) - return nil, nil, "", httperr.New( - http.StatusUnprocessableEntity, - "Too many upstream providers are configured (support for multiple upstreams is not yet implemented)", - ) - case len(oidcUpstreams) == 1: - return oidcUpstreams[0], nil, psession.ProviderTypeOIDC, nil - case len(adUpstreams) == 1: - return nil, adUpstreams[0], psession.ProviderTypeActiveDirectory, nil - default: - return nil, ldapUpstreams[0], psession.ProviderTypeLDAP, nil +func chooseUpstreamIDP(idpDisplayName string, idpLister provider.FederationDomainIdentityProvidersFinderI) (*provider.FederationDomainResolvedOIDCIdentityProvider, *provider.FederationDomainResolvedLDAPIdentityProvider, error) { + // When a request is made to the authorization endpoint which does not specify the IDP name, then it might + // be an old dynamic client (OIDCClient). We need to make this work, but only in the backwards compatibility case + // where there is exactly one IDP defined in the namespace and no IDPs listed on the FederationDomain. + // This backwards compatibility mode is handled by FindDefaultIDP(). + if len(idpDisplayName) == 0 { + return idpLister.FindDefaultIDP() } + return idpLister.FindUpstreamIDPByDisplayName(idpDisplayName) +} + +func maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue string, authorizeRequester fosite.AuthorizeRequester) { + if len(idpNameQueryParamValue) != 0 { + return + } + plog.Warning("Client attempted to perform an authorization flow (user login) without specifying the "+ + "query param to choose an identity provider. "+ + "This will not work when identity providers are configured explicitly on a FederationDomain. "+ + "Additionally, this behavior is deprecated and support for any authorization requests missing this query param "+ + "may be removed in a future release. "+ + "Please ask the author of this client to update the authorization request URL to include this query parameter. "+ + "The value of the parameter should be equal to the displayName of the identity provider as declared in the FederationDomain.", + "missingParameterName", oidcapi.AuthorizeUpstreamIDPNameParamName, + "clientID", authorizeRequester.GetClient().GetID(), + ) } type browserFlowAuthRequestState struct { @@ -438,16 +470,19 @@ func handleBrowserFlowAuthRequest( generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), generatePKCE func() (pkce.Code, error), - upstreamName string, + upstreamDisplayName string, idpType psession.ProviderType, cookieCodec oidc.Codec, upstreamStateEncoder oidc.Encoder, + idpNameQueryParamValue string, ) (*browserFlowAuthRequestState, error) { authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false) if !created { return nil, nil // already wrote the error response, don't return error } + maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) + now := time.Now() _, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{ Fosite: &openid.DefaultSession{ @@ -476,7 +511,7 @@ func handleBrowserFlowAuthRequest( encodedStateParamValue, err := upstreamStateParam( authorizeRequester, - upstreamName, + upstreamDisplayName, string(idpType), nonceValue, csrfValue, @@ -532,7 +567,7 @@ func generateValues( func upstreamStateParam( authorizeRequester fosite.AuthorizeRequester, - upstreamName string, + upstreamDisplayName string, upstreamType string, nonceValue nonce.Nonce, csrfValue csrftoken.CSRFToken, @@ -546,7 +581,7 @@ func upstreamStateParam( // The UpstreamName and UpstreamType struct fields can be used instead. // Remove those params here to avoid potential confusion about which should be used later. AuthParams: removeCustomIDPParams(authorizeRequester.GetRequestForm()).Encode(), - UpstreamName: upstreamName, + UpstreamName: upstreamDisplayName, UpstreamType: upstreamType, Nonce: nonceValue, CSRFToken: csrfValue, diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index d2c8e262..308adcc4 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -35,7 +35,7 @@ import ( "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/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" @@ -3287,7 +3287,7 @@ func TestAuthorizationEndpoint(t *testing.T) { WithScopes([]string{"some-other-new-scope1", "some-other-new-scope2"}). WithAdditionalAuthcodeParams(map[string]string{"prompt": "consent", "abc": "123"}). Build() - idpLister.SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI{provider.UpstreamOIDCIdentityProviderI(newProviderSettings)}) + idpLister.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{upstreamprovider.UpstreamOIDCIdentityProviderI(newProviderSettings)}) // Update the expectations of the test case to match the new upstream IDP settings. test.wantLocationHeader = urlWithQuery(upstreamAuthURL.String(), diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 05337b68..e260d201 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -20,7 +20,7 @@ import ( ) func NewHandler( - upstreamIDPs oidc.UpstreamOIDCIdentityProvidersLister, + upstreamIDPs provider.FederationDomainIdentityProvidersFinderI, oauthHelper fosite.OAuth2Provider, stateDecoder, cookieDecoder oidc.Decoder, redirectURI string, @@ -31,11 +31,12 @@ func NewHandler( return err } - upstreamIDPConfig := findUpstreamIDPConfig(state.UpstreamName, upstreamIDPs) - if upstreamIDPConfig == nil { + resolvedOIDCIdentityProvider, _, err := upstreamIDPs.FindUpstreamIDPByDisplayName(state.UpstreamName) + if err != nil || resolvedOIDCIdentityProvider == nil { plog.Warning("upstream provider not found") return httperr.New(http.StatusUnprocessableEntity, "upstream provider not found") } + upstreamIDPConfig := resolvedOIDCIdentityProvider.Provider downstreamAuthParams, err := url.ParseQuery(state.AuthParams) if err != nil { @@ -69,14 +70,19 @@ func NewHandler( return httperr.New(http.StatusBadGateway, "error exchanging and validating upstream tokens") } - subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) + subject, upstreamUsername, upstreamGroups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) + if err != nil { + return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) + } + + username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), resolvedOIDCIdentityProvider.Transforms, upstreamUsername, upstreamGroups) if err != nil { return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } additionalClaims := downstreamsession.MapAdditionalClaimsFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) - customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(upstreamIDPConfig, token, username) + customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(upstreamIDPConfig, token, username, upstreamUsername, upstreamGroups) if err != nil { return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } @@ -120,12 +126,3 @@ func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder) return decodedState, nil } - -func findUpstreamIDPConfig(upstreamName string, upstreamIDPs oidc.UpstreamOIDCIdentityProvidersLister) provider.UpstreamOIDCIdentityProviderI { - for _, p := range upstreamIDPs.GetOIDCIdentityProviders() { - if p.GetName() == upstreamName { - return p - } - } - return nil -} diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index 11f6c340..f40bdff8 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -5,6 +5,7 @@ package downstreamsession import ( + "context" "errors" "fmt" "net/url" @@ -19,8 +20,9 @@ import ( oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/oidc" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" "go.pinniped.dev/pkg/oidcclient/oidctypes" @@ -38,6 +40,8 @@ const ( requiredClaimEmptyErr = constable.Error("required claim in upstream ID token is empty") emailVerifiedClaimInvalidFormatErr = constable.Error("email_verified claim in upstream ID token has invalid format") emailVerifiedClaimFalseErr = constable.Error("email_verified claim in upstream ID token has false value") + idTransformUnexpectedErr = constable.Error("configured identity transformation or policy resulted in unexpected error") + idTransformPolicyErr = constable.Error("configured identity policy rejected this authentication") ) // MakeDownstreamSession creates a downstream OIDC session. @@ -82,16 +86,20 @@ func MakeDownstreamSession( } func MakeDownstreamLDAPOrADCustomSessionData( - ldapUpstream provider.UpstreamLDAPIdentityProviderI, + ldapUpstream upstreamprovider.UpstreamLDAPIdentityProviderI, idpType psession.ProviderType, authenticateResponse *authenticators.Response, username string, + untransformedUpstreamUsername string, + untransformedUpstreamGroups []string, ) *psession.CustomSessionData { customSessionData := &psession.CustomSessionData{ - Username: username, - ProviderUID: ldapUpstream.GetResourceUID(), - ProviderName: ldapUpstream.GetName(), - ProviderType: idpType, + Username: username, + UpstreamUsername: untransformedUpstreamUsername, + UpstreamGroups: untransformedUpstreamGroups, + ProviderUID: ldapUpstream.GetResourceUID(), + ProviderName: ldapUpstream.GetName(), + ProviderType: idpType, } if idpType == psession.ProviderTypeLDAP { @@ -112,9 +120,11 @@ func MakeDownstreamLDAPOrADCustomSessionData( } func MakeDownstreamOIDCCustomSessionData( - oidcUpstream provider.UpstreamOIDCIdentityProviderI, + oidcUpstream upstreamprovider.UpstreamOIDCIdentityProviderI, token *oidctypes.Token, username string, + untransformedUpstreamUsername string, + untransformedUpstreamGroups []string, ) (*psession.CustomSessionData, error) { upstreamSubject, err := ExtractStringClaimValue(oidcapi.IDTokenClaimSubject, oidcUpstream.GetName(), token.IDToken.Claims) if err != nil { @@ -126,10 +136,12 @@ func MakeDownstreamOIDCCustomSessionData( } customSessionData := &psession.CustomSessionData{ - Username: username, - ProviderUID: oidcUpstream.GetResourceUID(), - ProviderName: oidcUpstream.GetName(), - ProviderType: psession.ProviderTypeOIDC, + Username: username, + UpstreamUsername: untransformedUpstreamUsername, + UpstreamGroups: untransformedUpstreamGroups, + ProviderUID: oidcUpstream.GetResourceUID(), + ProviderName: oidcUpstream.GetName(), + ProviderType: psession.ProviderTypeOIDC, OIDC: &psession.OIDCSessionData{ UpstreamIssuer: upstreamIssuer, UpstreamSubject: upstreamSubject, @@ -200,7 +212,7 @@ func AutoApproveScopes(authorizeRequester fosite.AuthorizeRequester) { // GetDownstreamIdentityFromUpstreamIDToken returns the mapped subject, username, and group names, in that order. func GetDownstreamIdentityFromUpstreamIDToken( - upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, + upstreamIDPConfig upstreamprovider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) (string, string, []string, error) { subject, username, err := getSubjectAndUsernameFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims) @@ -218,7 +230,7 @@ func GetDownstreamIdentityFromUpstreamIDToken( // MapAdditionalClaimsFromUpstreamIDToken returns the additionalClaims mapped from the upstream token, if any. func MapAdditionalClaimsFromUpstreamIDToken( - upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, + upstreamIDPConfig upstreamprovider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) map[string]interface{} { mapped := make(map[string]interface{}, len(upstreamIDPConfig.GetAdditionalClaimMappings())) @@ -237,8 +249,32 @@ func MapAdditionalClaimsFromUpstreamIDToken( return mapped } +func ApplyIdentityTransformations( + ctx context.Context, + identityTransforms *idtransform.TransformationPipeline, + username string, + groups []string, +) (string, []string, error) { + transformationResult, err := identityTransforms.Evaluate(ctx, username, groups) + if err != nil { + plog.Error("unexpected identity transformation error during authentication", err, "inputUsername", username) + return "", nil, idTransformUnexpectedErr + } + if !transformationResult.AuthenticationAllowed { + plog.Debug("authentication rejected by configured policy", "inputUsername", username, "inputGroups", groups) + return "", nil, idTransformPolicyErr + } + plog.Debug("identity transformation successfully applied during authentication", + "originalUsername", username, + "newUsername", transformationResult.Username, + "originalGroups", groups, + "newGroups", transformationResult.Groups, + ) + return transformationResult.Username, transformationResult.Groups, nil +} + func getSubjectAndUsernameFromUpstreamIDToken( - upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, + upstreamIDPConfig upstreamprovider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) (string, string, error) { // The spec says the "sub" claim is only unique per issuer, @@ -323,7 +359,7 @@ func ExtractStringClaimValue(claimName string, upstreamIDPName string, idTokenCl return valueAsString, nil } -func DownstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentityProviderI, authenticateResponse *authenticators.Response) string { +func DownstreamSubjectFromUpstreamLDAP(ldapUpstream upstreamprovider.UpstreamLDAPIdentityProviderI, authenticateResponse *authenticators.Response) string { ldapURL := *ldapUpstream.GetURL() return DownstreamLDAPSubject(authenticateResponse.User.GetUID(), ldapURL) } @@ -343,7 +379,7 @@ func downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString string, upstreamSu // It returns nil when there is no configured groups claim name, or then when the configured claim name is not found // in the provided map of claims. It returns an error when the claim exists but its value cannot be parsed. func GetGroupsFromUpstreamIDToken( - upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, + upstreamIDPConfig upstreamprovider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, ) ([]string, error) { groupsClaimName := upstreamIDPConfig.GetGroupsClaim() diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler.go b/internal/oidc/idpdiscovery/idp_discovery_handler.go index 66a974c9..0b93b2b7 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package idpdiscovery provides a handler for the upstream IDP discovery endpoint. @@ -11,11 +11,11 @@ import ( "sort" "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" - "go.pinniped.dev/internal/oidc" + "go.pinniped.dev/internal/oidc/provider" ) // NewHandler returns an http.Handler that serves the upstream IDP discovery endpoint. -func NewHandler(upstreamIDPs oidc.UpstreamIdentityProvidersLister) http.Handler { +func NewHandler(upstreamIDPs provider.FederationDomainIdentityProvidersListerI) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { http.Error(w, `Method not allowed (try GET)`, http.StatusMethodNotAllowed) @@ -36,31 +36,31 @@ func NewHandler(upstreamIDPs oidc.UpstreamIdentityProvidersLister) http.Handler }) } -func responseAsJSON(upstreamIDPs oidc.UpstreamIdentityProvidersLister) ([]byte, error) { +func responseAsJSON(upstreamIDPs provider.FederationDomainIdentityProvidersListerI) ([]byte, error) { r := v1alpha1.IDPDiscoveryResponse{PinnipedIDPs: []v1alpha1.PinnipedIDP{}} // The cache of IDPs could change at any time, so always recalculate the list. - for _, provider := range upstreamIDPs.GetLDAPIdentityProviders() { + for _, federationDomainIdentityProvider := range upstreamIDPs.GetLDAPIdentityProviders() { r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{ - Name: provider.GetName(), + Name: federationDomainIdentityProvider.DisplayName, Type: v1alpha1.IDPTypeLDAP, Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode}, }) } - for _, provider := range upstreamIDPs.GetActiveDirectoryIdentityProviders() { + for _, federationDomainIdentityProvider := range upstreamIDPs.GetActiveDirectoryIdentityProviders() { r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{ - Name: provider.GetName(), + Name: federationDomainIdentityProvider.DisplayName, Type: v1alpha1.IDPTypeActiveDirectory, Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode}, }) } - for _, provider := range upstreamIDPs.GetOIDCIdentityProviders() { + for _, federationDomainIdentityProvider := range upstreamIDPs.GetOIDCIdentityProviders() { flows := []v1alpha1.IDPFlow{v1alpha1.IDPFlowBrowserAuthcode} - if provider.AllowsPasswordGrant() { + if federationDomainIdentityProvider.Provider.AllowsPasswordGrant() { flows = append(flows, v1alpha1.IDPFlowCLIPassword) } r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{ - Name: provider.GetName(), + Name: federationDomainIdentityProvider.DisplayName, Type: v1alpha1.IDPTypeOIDC, Flows: flows, }) diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go index b33ab2d8..7ab52d07 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package idpdiscovery @@ -12,7 +12,7 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/oidc" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil/oidctestutil" ) @@ -99,16 +99,16 @@ func TestIDPDiscovery(t *testing.T) { } // Change the list of IDPs in the cache. - idpLister.SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + idpLister.SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-1"}, &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-2"}, }) - idpLister.SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI{ + idpLister.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{ &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-1", AllowPasswordGrant: true}, &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-2"}, }) - idpLister.SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + idpLister.SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI{ &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ad-idp-2"}, &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ad-idp-1"}, }) diff --git a/internal/oidc/idplister/upstream_idp_lister.go b/internal/oidc/idplister/upstream_idp_lister.go new file mode 100644 index 00000000..3ea4ea64 --- /dev/null +++ b/internal/oidc/idplister/upstream_idp_lister.go @@ -0,0 +1,26 @@ +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package idplister + +import ( + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" +) + +type UpstreamOIDCIdentityProvidersLister interface { + GetOIDCIdentityProviders() []upstreamprovider.UpstreamOIDCIdentityProviderI +} + +type UpstreamLDAPIdentityProvidersLister interface { + GetLDAPIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI +} + +type UpstreamActiveDirectoryIdentityProviderLister interface { + GetActiveDirectoryIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI +} + +type UpstreamIdentityProvidersLister interface { + UpstreamOIDCIdentityProvidersLister + UpstreamLDAPIdentityProvidersLister + UpstreamActiveDirectoryIdentityProviderLister +} diff --git a/internal/oidc/login/get_login_handler_test.go b/internal/oidc/login/get_login_handler_test.go index bb85b8f2..30567309 100644 --- a/internal/oidc/login/get_login_handler_test.go +++ b/internal/oidc/login/get_login_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package login @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "go.pinniped.dev/internal/oidc" + "go.pinniped.dev/internal/oidc/idplister" "go.pinniped.dev/internal/oidc/login/loginhtml" "go.pinniped.dev/internal/testutil" ) @@ -28,7 +29,7 @@ func TestGetLogin(t *testing.T) { decodedState *oidc.UpstreamStateParamData encodedState string errParam string - idps oidc.UpstreamIdentityProvidersLister + idps idplister.UpstreamIdentityProvidersLister wantStatus int wantContentType string wantBody string diff --git a/internal/oidc/login/post_login_handler.go b/internal/oidc/login/post_login_handler.go index 49f9c5b2..8c9a5970 100644 --- a/internal/oidc/login/post_login_handler.go +++ b/internal/oidc/login/post_login_handler.go @@ -12,13 +12,14 @@ import ( "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/downstreamsession" + "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" ) -func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvidersLister, oauthHelper fosite.OAuth2Provider) HandlerFunc { +func NewPostHandler(issuerURL string, upstreamIDPs provider.FederationDomainIdentityProvidersFinderI, oauthHelper fosite.OAuth2Provider) HandlerFunc { return func(w http.ResponseWriter, r *http.Request, encodedState string, decodedState *oidc.UpstreamStateParamData) error { // Note that the login handler prevents this handler from being called with OIDC upstreams. - _, ldapUpstream, idpType, err := oidc.FindUpstreamIDPByNameAndType(upstreamIDPs, decodedState.UpstreamName, decodedState.UpstreamType) + _, ldapUpstream, err := upstreamIDPs.FindUpstreamIDPByDisplayName(decodedState.UpstreamName) if err != nil { // This shouldn't normally happen because the authorization endpoint ensured that this provider existed // at that time. It would be possible in the unlikely event that the provider was deleted during the login. @@ -51,20 +52,20 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider downstreamsession.AutoApproveScopes(authorizeRequester) // Get the username and password form params from the POST body. - username := r.PostFormValue(usernameParamName) - password := r.PostFormValue(passwordParamName) + submittedUsername := r.PostFormValue(usernameParamName) + submittedPassword := r.PostFormValue(passwordParamName) // Treat blank username or password as a bad username/password combination, as opposed to an internal error. - if username == "" || password == "" { + if submittedUsername == "" || submittedPassword == "" { // User forgot to enter one of the required fields. // The user may try to log in again if they'd like, so redirect back to the login page with an error. return RedirectToLoginPage(r, w, issuerURL, encodedState, ShowBadUserPassErr) } // Attempt to authenticate the user with the upstream IDP. - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) + authenticateResponse, authenticated, err := ldapUpstream.Provider.AuthenticateUser(r.Context(), submittedUsername, submittedPassword, authorizeRequester.GetGrantedScopes()) if err != nil { - plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) + plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.Provider.GetName()) // There was some problem during authentication with the upstream, aside from bad username/password. // The user may try to log in again if they'd like, so redirect back to the login page with an error. return RedirectToLoginPage(r, w, issuerURL, encodedState, ShowInternalError) @@ -79,10 +80,19 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider // Now the upstream IDP has authenticated the user, so now we're back into the regular OIDC authcode flow steps. // Both success and error responses from this point onwards should look like the usual fosite redirect // responses, and a happy redirect response will include a downstream authcode. - subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse) - username = authenticateResponse.User.GetName() - groups := authenticateResponse.User.GetGroups() - customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username) + subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream.Provider, authenticateResponse) + upstreamUsername := authenticateResponse.User.GetName() + upstreamGroups := authenticateResponse.User.GetGroups() + + username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), ldapUpstream.Transforms, upstreamUsername, upstreamGroups) + if err != nil { + oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), false, + ) + return nil + } + + customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream.Provider, ldapUpstream.SessionProviderType, authenticateResponse, username, upstreamUsername, upstreamGroups) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData, map[string]interface{}{}) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false) diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 71bc914d..1367f35b 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package oidc contains common OIDC functionality needed by Pinniped. @@ -7,7 +7,6 @@ package oidc import ( "context" "crypto/subtle" - "errors" "fmt" "net/http" "net/url" @@ -18,12 +17,10 @@ import ( "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" - "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/formposthtml" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" @@ -279,24 +276,6 @@ func FositeErrorForLog(err error) []interface{} { return keysAndValues } -type UpstreamOIDCIdentityProvidersLister interface { - GetOIDCIdentityProviders() []provider.UpstreamOIDCIdentityProviderI -} - -type UpstreamLDAPIdentityProvidersLister interface { - GetLDAPIdentityProviders() []provider.UpstreamLDAPIdentityProviderI -} - -type UpstreamActiveDirectoryIdentityProviderLister interface { - GetActiveDirectoryIdentityProviders() []provider.UpstreamLDAPIdentityProviderI -} - -type UpstreamIdentityProvidersLister interface { - UpstreamOIDCIdentityProvidersLister - UpstreamLDAPIdentityProvidersLister - UpstreamActiveDirectoryIdentityProviderLister -} - func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) { if ScopeWasRequested(authorizeRequester, scopeName) { authorizeRequester.GrantScope(scopeName) @@ -377,41 +356,6 @@ func validateCSRFValue(state *UpstreamStateParamData, csrfCookieValue csrftoken. return nil } -// FindUpstreamIDPByNameAndType finds the requested IDP by name and type, or returns an error. -// Note that AD and LDAP IDPs both return the same interface type, but different ProviderTypes values. -func FindUpstreamIDPByNameAndType( - idpLister UpstreamIdentityProvidersLister, - upstreamName string, - upstreamType string, -) ( - provider.UpstreamOIDCIdentityProviderI, - provider.UpstreamLDAPIdentityProviderI, - psession.ProviderType, - error, -) { - switch upstreamType { - case string(v1alpha1.IDPTypeOIDC): - for _, p := range idpLister.GetOIDCIdentityProviders() { - if p.GetName() == upstreamName { - return p, nil, psession.ProviderTypeOIDC, nil - } - } - case string(v1alpha1.IDPTypeLDAP): - for _, p := range idpLister.GetLDAPIdentityProviders() { - if p.GetName() == upstreamName { - return nil, p, psession.ProviderTypeLDAP, nil - } - } - case string(v1alpha1.IDPTypeActiveDirectory): - for _, p := range idpLister.GetActiveDirectoryIdentityProviders() { - if p.GetName() == upstreamName { - return nil, p, psession.ProviderTypeActiveDirectory, nil - } - } - } - return nil, nil, "", errors.New("provider not found") -} - // WriteAuthorizeError writes an authorization error as it should be returned by the authorization endpoint and other // similar endpoints that are the end of the downstream authcode flow. Errors responses are written in the usual fosite style. func WriteAuthorizeError(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester, err error, isBrowserless bool) { diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index ca26451e..e9a4333a 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -4,182 +4,67 @@ package provider import ( - "context" "fmt" - "net/url" "sync" - "golang.org/x/oauth2" - "k8s.io/apimachinery/pkg/types" - - "go.pinniped.dev/internal/authenticators" - "go.pinniped.dev/pkg/oidcclient/nonce" - "go.pinniped.dev/pkg/oidcclient/oidctypes" - "go.pinniped.dev/pkg/oidcclient/pkce" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" ) -type RevocableTokenType string - -// These strings correspond to the token types defined by https://datatracker.ietf.org/doc/html/rfc7009#section-2.1 -const ( - RefreshTokenType RevocableTokenType = "refresh_token" - AccessTokenType RevocableTokenType = "access_token" -) - -type UpstreamOIDCIdentityProviderI interface { - // GetName returns a name for this upstream provider, which will be used as a component of the path for the - // callback endpoint hosted by the Supervisor. - GetName() string - - // GetClientID returns the OAuth client ID registered with the upstream provider to be used in the authorization code flow. - GetClientID() string - - // GetResourceUID returns the Kubernetes resource ID - GetResourceUID() types.UID - - // GetAuthorizationURL returns the Authorization Endpoint fetched from discovery. - GetAuthorizationURL() *url.URL - - // HasUserInfoURL returns whether there is a non-empty value for userinfo_endpoint fetched from discovery. - HasUserInfoURL() bool - - // GetScopes returns the scopes to request in authorization (authcode or password grant) flow. - GetScopes() []string - - // GetUsernameClaim returns the ID Token username claim name. May return empty string, in which case we - // will use some reasonable defaults. - GetUsernameClaim() string - - // GetGroupsClaim returns the ID Token groups claim name. May return empty string, in which case we won't - // try to read groups from the upstream provider. - GetGroupsClaim() string - - // AllowsPasswordGrant returns true if a client should be allowed to use the resource owner password credentials grant - // flow with this upstream provider. When false, it should not be allowed. - AllowsPasswordGrant() bool - - // GetAdditionalAuthcodeParams returns additional params to be sent on authcode requests. - GetAdditionalAuthcodeParams() map[string]string - - // GetAdditionalClaimMappings returns additional claims to be mapped from the upstream ID token. - GetAdditionalClaimMappings() map[string]string - - // PasswordCredentialsGrantAndValidateTokens performs upstream OIDC resource owner password credentials grant and - // token validation. Returns the validated raw tokens as well as the parsed claims of the ID token. - PasswordCredentialsGrantAndValidateTokens(ctx context.Context, username, password string) (*oidctypes.Token, error) - - // ExchangeAuthcodeAndValidateTokens performs upstream OIDC authorization code exchange and token validation. - // Returns the validated raw tokens as well as the parsed claims of the ID token. - ExchangeAuthcodeAndValidateTokens( - ctx context.Context, - authcode string, - pkceCodeVerifier pkce.Code, - expectedIDTokenNonce nonce.Nonce, - redirectURI string, - ) (*oidctypes.Token, error) - - // PerformRefresh will call the provider's token endpoint to perform a refresh grant. The provider may or may not - // return a new ID or refresh token in the response. If it returns an ID token, then use ValidateToken to - // validate the ID token. - PerformRefresh(ctx context.Context, refreshToken string) (*oauth2.Token, error) - - // RevokeToken will attempt to revoke the given token, if the provider has a revocation endpoint. - // It may return an error wrapped by a RetryableRevocationError, which is an error indicating that it may - // be worth trying to revoke the same token again later. Any other error returned should be assumed to - // represent an error such that it is not worth retrying revocation later, even though revocation failed. - RevokeToken(ctx context.Context, token string, tokenType RevocableTokenType) error - - // ValidateTokenAndMergeWithUserInfo will validate the ID token. It will also merge the claims from the userinfo endpoint response - // into the ID token's claims, if the provider offers the userinfo endpoint. It returns the validated/updated - // tokens, or an error. - ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool, requireUserInfo bool) (*oidctypes.Token, error) -} - -type UpstreamLDAPIdentityProviderI interface { - // GetName returns a name for this upstream provider. - GetName() string - - // GetURL returns a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234". - // This URL is not used for connecting to the provider, but rather is used for creating a globally unique user - // identifier by being combined with the user's UID, since user UIDs are only unique within one provider. - GetURL() *url.URL - - // GetResourceUID returns the Kubernetes resource ID - GetResourceUID() types.UID - - // UserAuthenticator adds an interface method for performing user authentication against the upstream LDAP provider. - authenticators.UserAuthenticator - - // PerformRefresh performs a refresh against the upstream LDAP identity provider - PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes) (groups []string, err error) -} - -// RefreshAttributes contains information about the user from the original login request -// and previous refreshes. -type RefreshAttributes struct { - Username string - Subject string - DN string - Groups []string - AdditionalAttributes map[string]string - GrantedScopes []string -} - type DynamicUpstreamIDPProvider interface { - SetOIDCIdentityProviders(oidcIDPs []UpstreamOIDCIdentityProviderI) - GetOIDCIdentityProviders() []UpstreamOIDCIdentityProviderI - SetLDAPIdentityProviders(ldapIDPs []UpstreamLDAPIdentityProviderI) - GetLDAPIdentityProviders() []UpstreamLDAPIdentityProviderI - SetActiveDirectoryIdentityProviders(adIDPs []UpstreamLDAPIdentityProviderI) - GetActiveDirectoryIdentityProviders() []UpstreamLDAPIdentityProviderI + SetOIDCIdentityProviders(oidcIDPs []upstreamprovider.UpstreamOIDCIdentityProviderI) + GetOIDCIdentityProviders() []upstreamprovider.UpstreamOIDCIdentityProviderI + SetLDAPIdentityProviders(ldapIDPs []upstreamprovider.UpstreamLDAPIdentityProviderI) + GetLDAPIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI + SetActiveDirectoryIdentityProviders(adIDPs []upstreamprovider.UpstreamLDAPIdentityProviderI) + GetActiveDirectoryIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI } type dynamicUpstreamIDPProvider struct { - oidcUpstreams []UpstreamOIDCIdentityProviderI - ldapUpstreams []UpstreamLDAPIdentityProviderI - activeDirectoryUpstreams []UpstreamLDAPIdentityProviderI + oidcUpstreams []upstreamprovider.UpstreamOIDCIdentityProviderI + ldapUpstreams []upstreamprovider.UpstreamLDAPIdentityProviderI + activeDirectoryUpstreams []upstreamprovider.UpstreamLDAPIdentityProviderI mutex sync.RWMutex } func NewDynamicUpstreamIDPProvider() DynamicUpstreamIDPProvider { return &dynamicUpstreamIDPProvider{ - oidcUpstreams: []UpstreamOIDCIdentityProviderI{}, - ldapUpstreams: []UpstreamLDAPIdentityProviderI{}, - activeDirectoryUpstreams: []UpstreamLDAPIdentityProviderI{}, + oidcUpstreams: []upstreamprovider.UpstreamOIDCIdentityProviderI{}, + ldapUpstreams: []upstreamprovider.UpstreamLDAPIdentityProviderI{}, + activeDirectoryUpstreams: []upstreamprovider.UpstreamLDAPIdentityProviderI{}, } } -func (p *dynamicUpstreamIDPProvider) SetOIDCIdentityProviders(oidcIDPs []UpstreamOIDCIdentityProviderI) { +func (p *dynamicUpstreamIDPProvider) SetOIDCIdentityProviders(oidcIDPs []upstreamprovider.UpstreamOIDCIdentityProviderI) { p.mutex.Lock() // acquire a write lock defer p.mutex.Unlock() p.oidcUpstreams = oidcIDPs } -func (p *dynamicUpstreamIDPProvider) GetOIDCIdentityProviders() []UpstreamOIDCIdentityProviderI { +func (p *dynamicUpstreamIDPProvider) GetOIDCIdentityProviders() []upstreamprovider.UpstreamOIDCIdentityProviderI { p.mutex.RLock() // acquire a read lock defer p.mutex.RUnlock() return p.oidcUpstreams } -func (p *dynamicUpstreamIDPProvider) SetLDAPIdentityProviders(ldapIDPs []UpstreamLDAPIdentityProviderI) { +func (p *dynamicUpstreamIDPProvider) SetLDAPIdentityProviders(ldapIDPs []upstreamprovider.UpstreamLDAPIdentityProviderI) { p.mutex.Lock() // acquire a write lock defer p.mutex.Unlock() p.ldapUpstreams = ldapIDPs } -func (p *dynamicUpstreamIDPProvider) GetLDAPIdentityProviders() []UpstreamLDAPIdentityProviderI { +func (p *dynamicUpstreamIDPProvider) GetLDAPIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI { p.mutex.RLock() // acquire a read lock defer p.mutex.RUnlock() return p.ldapUpstreams } -func (p *dynamicUpstreamIDPProvider) SetActiveDirectoryIdentityProviders(adIDPs []UpstreamLDAPIdentityProviderI) { +func (p *dynamicUpstreamIDPProvider) SetActiveDirectoryIdentityProviders(adIDPs []upstreamprovider.UpstreamLDAPIdentityProviderI) { p.mutex.Lock() // acquire a write lock defer p.mutex.Unlock() p.activeDirectoryUpstreams = adIDPs } -func (p *dynamicUpstreamIDPProvider) GetActiveDirectoryIdentityProviders() []UpstreamLDAPIdentityProviderI { +func (p *dynamicUpstreamIDPProvider) GetActiveDirectoryIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI { p.mutex.RLock() // acquire a read lock defer p.mutex.RUnlock() return p.activeDirectoryUpstreams diff --git a/internal/oidc/provider/federation_domain_identity_providers_lister.go b/internal/oidc/provider/federation_domain_identity_providers_lister.go new file mode 100644 index 00000000..ce333d3c --- /dev/null +++ b/internal/oidc/provider/federation_domain_identity_providers_lister.go @@ -0,0 +1,232 @@ +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package provider + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + + "go.pinniped.dev/internal/idtransform" + "go.pinniped.dev/internal/oidc/idplister" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" + "go.pinniped.dev/internal/psession" +) + +// FederationDomainIdentityProvider represents an identity provider as configured in a FederationDomain's spec. +// All the fields are required and must be non-zero values. Note that this might be a reference to an IDP +// which is not currently loaded into the cache of available IDPs, e.g. due to the IDP's CR having validation errors. +type FederationDomainIdentityProvider struct { + DisplayName string + UID types.UID + Transforms *idtransform.TransformationPipeline +} + +// FederationDomainResolvedOIDCIdentityProvider represents a FederationDomainIdentityProvider which has +// been resolved dynamically based on the currently loaded IDP CRs to include the provider.UpstreamOIDCIdentityProviderI +// and other metadata about the provider. +type FederationDomainResolvedOIDCIdentityProvider struct { + DisplayName string + Provider upstreamprovider.UpstreamOIDCIdentityProviderI + SessionProviderType psession.ProviderType + Transforms *idtransform.TransformationPipeline +} + +// FederationDomainResolvedLDAPIdentityProvider represents a FederationDomainIdentityProvider which has +// been resolved dynamically based on the currently loaded IDP CRs to include the provider.UpstreamLDAPIdentityProviderI +// and other metadata about the provider. +type FederationDomainResolvedLDAPIdentityProvider struct { + DisplayName string + Provider upstreamprovider.UpstreamLDAPIdentityProviderI + SessionProviderType psession.ProviderType + Transforms *idtransform.TransformationPipeline +} + +type FederationDomainIdentityProvidersFinderI interface { + FindDefaultIDP() ( + *FederationDomainResolvedOIDCIdentityProvider, + *FederationDomainResolvedLDAPIdentityProvider, + error, + ) + + FindUpstreamIDPByDisplayName(upstreamIDPDisplayName string) ( + *FederationDomainResolvedOIDCIdentityProvider, + *FederationDomainResolvedLDAPIdentityProvider, + error, + ) +} + +type FederationDomainIdentityProvidersListerI interface { + GetOIDCIdentityProviders() []*FederationDomainResolvedOIDCIdentityProvider + GetLDAPIdentityProviders() []*FederationDomainResolvedLDAPIdentityProvider + GetActiveDirectoryIdentityProviders() []*FederationDomainResolvedLDAPIdentityProvider +} + +// FederationDomainIdentityProvidersLister wraps an UpstreamIdentityProvidersLister. The lister which is being +// wrapped should contain all valid upstream providers that are currently defined in the Supervisor. +// FederationDomainIdentityProvidersLister provides a lookup method which only looks up IDPs within those which +// have allowed resource IDs, and also uses display names (name aliases) instead of the actual resource names to do the +// lookups. It also provides list methods which only list the allowed identity providers (to be used by the IDP +// discovery endpoint, for example). +type FederationDomainIdentityProvidersLister struct { + wrappedLister idplister.UpstreamIdentityProvidersLister + configuredIdentityProviders []*FederationDomainIdentityProvider + defaultIdentityProvider *FederationDomainIdentityProvider + idpDisplayNamesToResourceUIDsMap map[string]types.UID + allowedIDPResourceUIDs sets.Set[types.UID] +} + +// NewFederationDomainUpstreamIdentityProvidersLister returns a new FederationDomainIdentityProvidersLister +// which only lists those IDPs allowed by its parameter. Every FederationDomainIdentityProvider in the +// federationDomainIssuer parameter's IdentityProviders() list must have a unique DisplayName. +// Note that a single underlying IDP UID may be used by multiple FederationDomainIdentityProvider in the parameter. +// The wrapped lister should contain all valid upstream providers that are defined in the Supervisor, and is expected to +// be thread-safe and to change its contents over time. The FederationDomainIdentityProvidersLister will filter out the +// ones that don't apply to this federation domain. +func NewFederationDomainUpstreamIdentityProvidersLister( + federationDomainIssuer *FederationDomainIssuer, + wrappedLister idplister.UpstreamIdentityProvidersLister, +) *FederationDomainIdentityProvidersLister { + // Create a copy of the input slice so we won't need to worry about the caller accidentally changing it. + copyOfFederationDomainIdentityProviders := []*FederationDomainIdentityProvider{} + // Create a map and a set for quick lookups of the same data that was passed in via the + // federationDomainIssuer parameter. + allowedResourceUIDs := sets.New[types.UID]() + idpDisplayNamesToResourceUIDsMap := map[string]types.UID{} + for _, idp := range federationDomainIssuer.IdentityProviders() { + allowedResourceUIDs.Insert(idp.UID) + idpDisplayNamesToResourceUIDsMap[idp.DisplayName] = idp.UID + shallowCopyOfIDP := *idp + copyOfFederationDomainIdentityProviders = append(copyOfFederationDomainIdentityProviders, &shallowCopyOfIDP) + } + + return &FederationDomainIdentityProvidersLister{ + wrappedLister: wrappedLister, + configuredIdentityProviders: copyOfFederationDomainIdentityProviders, + defaultIdentityProvider: federationDomainIssuer.DefaultIdentityProvider(), + idpDisplayNamesToResourceUIDsMap: idpDisplayNamesToResourceUIDsMap, + allowedIDPResourceUIDs: allowedResourceUIDs, + } +} + +// FindUpstreamIDPByDisplayName selects either an OIDC, LDAP, or ActiveDirectory IDP, or returns an error. +// It only considers the allowed IDPs while doing the lookup by display name. +// Note that ActiveDirectory and LDAP IDPs both return the same type, but with different SessionProviderType values. +func (u *FederationDomainIdentityProvidersLister) FindUpstreamIDPByDisplayName(upstreamIDPDisplayName string) ( + *FederationDomainResolvedOIDCIdentityProvider, + *FederationDomainResolvedLDAPIdentityProvider, + error, +) { + // Given a display name, look up the identity provider's UID for that display name. + idpUIDForDisplayName, ok := u.idpDisplayNamesToResourceUIDsMap[upstreamIDPDisplayName] + if !ok { + return nil, nil, fmt.Errorf("identity provider not found: %q", upstreamIDPDisplayName) + } + // Find the IDP with that UID. It could be any type, so look at all types to find it. + for _, p := range u.GetOIDCIdentityProviders() { + if p.Provider.GetResourceUID() == idpUIDForDisplayName { + return p, nil, nil + } + } + for _, p := range u.GetLDAPIdentityProviders() { + if p.Provider.GetResourceUID() == idpUIDForDisplayName { + return nil, p, nil + } + } + for _, p := range u.GetActiveDirectoryIdentityProviders() { + if p.Provider.GetResourceUID() == idpUIDForDisplayName { + return nil, p, nil + } + } + return nil, nil, fmt.Errorf("identity provider not found: %q", upstreamIDPDisplayName) +} + +// FindDefaultIDP works like FindUpstreamIDPByDisplayName, but finds the default IDP instead of finding by name. +// If there is no default IDP for this federation domain, then FindDefaultIDP will return an error. +// This can be used to handle the backwards compatibility mode where an authorization request could be made +// without specifying an IDP name, and there are no IDPs explicitly specified on the FederationDomain, and there +// is exactly one IDP CR defined in the Supervisor namespace. +func (u *FederationDomainIdentityProvidersLister) FindDefaultIDP() ( + *FederationDomainResolvedOIDCIdentityProvider, + *FederationDomainResolvedLDAPIdentityProvider, + error, +) { + if u.defaultIdentityProvider == nil { + return nil, nil, fmt.Errorf("identity provider not found: this federation domain does not have a default identity provider") + } + return u.FindUpstreamIDPByDisplayName(u.defaultIdentityProvider.DisplayName) +} + +// GetOIDCIdentityProviders lists only the OIDC providers for this FederationDomain. +func (u *FederationDomainIdentityProvidersLister) GetOIDCIdentityProviders() []*FederationDomainResolvedOIDCIdentityProvider { + // Get the cached providers once at the start in case they change during the rest of this function. + cachedProviders := u.wrappedLister.GetOIDCIdentityProviders() + providers := []*FederationDomainResolvedOIDCIdentityProvider{} + // Every configured identityProvider on the FederationDomain uses an objetRef to an underlying IDP CR that might + // be available as a provider in the wrapped cache. For each configured identityProvider/displayName... + for _, idp := range u.configuredIdentityProviders { + // Check if the IDP used by that displayName is in the cached available OIDC providers. + for _, p := range cachedProviders { + if idp.UID == p.GetResourceUID() { + // Found it, so append it to the result. + providers = append(providers, &FederationDomainResolvedOIDCIdentityProvider{ + DisplayName: idp.DisplayName, + Provider: p, + SessionProviderType: psession.ProviderTypeOIDC, + Transforms: idp.Transforms, + }) + } + } + } + return providers +} + +// GetLDAPIdentityProviders lists only the LDAP providers for this FederationDomain. +func (u *FederationDomainIdentityProvidersLister) GetLDAPIdentityProviders() []*FederationDomainResolvedLDAPIdentityProvider { + // Get the cached providers once at the start in case they change during the rest of this function. + cachedProviders := u.wrappedLister.GetLDAPIdentityProviders() + providers := []*FederationDomainResolvedLDAPIdentityProvider{} + // Every configured identityProvider on the FederationDomain uses an objetRef to an underlying IDP CR that might + // be available as a provider in the wrapped cache. For each configured identityProvider/displayName... + for _, idp := range u.configuredIdentityProviders { + // Check if the IDP used by that displayName is in the cached available LDAP providers. + for _, p := range cachedProviders { + if idp.UID == p.GetResourceUID() { + // Found it, so append it to the result. + providers = append(providers, &FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: idp.DisplayName, + Provider: p, + SessionProviderType: psession.ProviderTypeLDAP, + Transforms: idp.Transforms, + }) + } + } + } + return providers +} + +// GetActiveDirectoryIdentityProviders lists only the ActiveDirectory providers for this FederationDomain. +func (u *FederationDomainIdentityProvidersLister) GetActiveDirectoryIdentityProviders() []*FederationDomainResolvedLDAPIdentityProvider { + // Get the cached providers once at the start in case they change during the rest of this function. + cachedProviders := u.wrappedLister.GetActiveDirectoryIdentityProviders() + providers := []*FederationDomainResolvedLDAPIdentityProvider{} + // Every configured identityProvider on the FederationDomain uses an objetRef to an underlying IDP CR that might + // be available as a provider in the wrapped cache. For each configured identityProvider/displayName... + for _, idp := range u.configuredIdentityProviders { + // Check if the IDP used by that displayName is in the cached available ActiveDirectory providers. + for _, p := range cachedProviders { + if idp.UID == p.GetResourceUID() { + // Found it, so append it to the result. + providers = append(providers, &FederationDomainResolvedLDAPIdentityProvider{ + DisplayName: idp.DisplayName, + Provider: p, + SessionProviderType: psession.ProviderTypeActiveDirectory, + Transforms: idp.Transforms, + }) + } + } + } + return providers +} diff --git a/internal/oidc/provider/federation_domain_issuer.go b/internal/oidc/provider/federation_domain_issuer.go index 29e3683c..bdb27f14 100644 --- a/internal/oidc/provider/federation_domain_issuer.go +++ b/internal/oidc/provider/federation_domain_issuer.go @@ -17,20 +17,42 @@ type FederationDomainIssuer struct { issuer string issuerHost string issuerPath string + + // identityProviders should be used when they are explicitly specified in the FederationDomain's spec. + identityProviders []*FederationDomainIdentityProvider + // defaultIdentityProvider should be used only for the backwards compatibility mode where identity providers + // are not explicitly specified in the FederationDomain's spec, and there is exactly one IDP CR defined in the + // Supervisor's namespace. + defaultIdentityProvider *FederationDomainIdentityProvider } // NewFederationDomainIssuer returns a FederationDomainIssuer. // Performs validation, and returns any error from validation. -func NewFederationDomainIssuer(issuer string) (*FederationDomainIssuer, error) { - p := FederationDomainIssuer{issuer: issuer} - err := p.validate() +func NewFederationDomainIssuer( + issuer string, + identityProviders []*FederationDomainIdentityProvider, +) (*FederationDomainIssuer, error) { + p := FederationDomainIssuer{issuer: issuer, identityProviders: identityProviders} + err := p.validateURL() if err != nil { return nil, err } return &p, nil } -func (p *FederationDomainIssuer) validate() error { +func NewFederationDomainIssuerWithDefaultIDP( + issuer string, + defaultIdentityProvider *FederationDomainIdentityProvider, +) (*FederationDomainIssuer, error) { + fdi, err := NewFederationDomainIssuer(issuer, []*FederationDomainIdentityProvider{defaultIdentityProvider}) + if err != nil { + return nil, err + } + fdi.defaultIdentityProvider = defaultIdentityProvider + return fdi, nil +} + +func (p *FederationDomainIssuer) validateURL() error { if p.issuer == "" { return constable.Error("federation domain must have an issuer") } @@ -84,3 +106,13 @@ func (p *FederationDomainIssuer) IssuerHost() string { func (p *FederationDomainIssuer) IssuerPath() string { return p.issuerPath } + +// IdentityProviders returns the IdentityProviders. +func (p *FederationDomainIssuer) IdentityProviders() []*FederationDomainIdentityProvider { + return p.identityProviders +} + +// DefaultIdentityProvider will return nil when there is no default. +func (p *FederationDomainIssuer) DefaultIdentityProvider() *FederationDomainIdentityProvider { + return p.defaultIdentityProvider +} diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index 0cd3e3e3..5793765d 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -19,6 +19,7 @@ import ( "go.pinniped.dev/internal/oidc/discovery" "go.pinniped.dev/internal/oidc/dynamiccodec" "go.pinniped.dev/internal/oidc/idpdiscovery" + "go.pinniped.dev/internal/oidc/idplister" "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/login" "go.pinniped.dev/internal/oidc/oidcclientvalidator" @@ -36,11 +37,11 @@ import ( type Manager struct { mu sync.RWMutex providers []*provider.FederationDomainIssuer - providerHandlers map[string]http.Handler // map of all routes for all providers - nextHandler http.Handler // the next handler in a chain, called when this manager didn't know how to handle a request - dynamicJWKSProvider jwks.DynamicJWKSProvider // in-memory cache of per-issuer JWKS data - upstreamIDPs oidc.UpstreamIdentityProvidersLister // in-memory cache of upstream IDPs - secretCache *secret.Cache // in-memory cache of cryptographic material + providerHandlers map[string]http.Handler // map of all routes for all providers + nextHandler http.Handler // the next handler in a chain, called when this manager didn't know how to handle a request + dynamicJWKSProvider jwks.DynamicJWKSProvider // in-memory cache of per-issuer JWKS data + upstreamIDPs idplister.UpstreamIdentityProvidersLister // in-memory cache of upstream IDPs + secretCache *secret.Cache // in-memory cache of cryptographic material secretsClient corev1client.SecretInterface oidcClientsClient v1alpha1.OIDCClientInterface } @@ -52,7 +53,7 @@ type Manager struct { func NewManager( nextHandler http.Handler, dynamicJWKSProvider jwks.DynamicJWKSProvider, - upstreamIDPs oidc.UpstreamIdentityProvidersLister, + upstreamIDPs idplister.UpstreamIdentityProvidersLister, secretCache *secret.Cache, secretsClient corev1client.SecretInterface, oidcClientsClient v1alpha1.OIDCClientInterface, @@ -83,17 +84,17 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs m.providers = federationDomains m.providerHandlers = make(map[string]http.Handler) - var csrfCookieEncoder = dynamiccodec.New( + csrfCookieEncoder := dynamiccodec.New( oidc.CSRFCookieLifespan, m.secretCache.GetCSRFCookieEncoderHashKey, func() []byte { return nil }, ) - for _, incomingProvider := range federationDomains { - issuer := incomingProvider.Issuer() - issuerHostWithPath := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath() + for _, incomingFederationDomain := range federationDomains { + issuerURL := incomingFederationDomain.Issuer() + issuerHostWithPath := strings.ToLower(incomingFederationDomain.IssuerHost()) + "/" + incomingFederationDomain.IssuerPath() - tokenHMACKeyGetter := wrapGetter(incomingProvider.Issuer(), m.secretCache.GetTokenHMACKey) + tokenHMACKeyGetter := wrapGetter(incomingFederationDomain.Issuer(), m.secretCache.GetTokenHMACKey) timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration() @@ -101,7 +102,7 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs // the upstream callback endpoint is called later. oauthHelperWithNullStorage := oidc.FositeOauth2Helper( oidc.NewNullStorage(m.secretsClient, m.oidcClientsClient, oidcclientvalidator.DefaultMinBcryptCost), - issuer, + issuerURL, tokenHMACKeyGetter, nil, timeoutsConfiguration, @@ -110,27 +111,29 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs // For all the other endpoints, make another oauth helper with exactly the same settings except use real storage. oauthHelperWithKubeStorage := oidc.FositeOauth2Helper( oidc.NewKubeStorage(m.secretsClient, m.oidcClientsClient, timeoutsConfiguration, oidcclientvalidator.DefaultMinBcryptCost), - issuer, + issuerURL, tokenHMACKeyGetter, m.dynamicJWKSProvider, timeoutsConfiguration, ) - var upstreamStateEncoder = dynamiccodec.New( + upstreamStateEncoder := dynamiccodec.New( timeoutsConfiguration.UpstreamStateParamLifespan, - wrapGetter(incomingProvider.Issuer(), m.secretCache.GetStateEncoderHashKey), - wrapGetter(incomingProvider.Issuer(), m.secretCache.GetStateEncoderBlockKey), + wrapGetter(incomingFederationDomain.Issuer(), m.secretCache.GetStateEncoderHashKey), + wrapGetter(incomingFederationDomain.Issuer(), m.secretCache.GetStateEncoderBlockKey), ) - m.providerHandlers[(issuerHostWithPath + oidc.WellKnownEndpointPath)] = discovery.NewHandler(issuer) + idpLister := provider.NewFederationDomainUpstreamIdentityProvidersLister(incomingFederationDomain, m.upstreamIDPs) - m.providerHandlers[(issuerHostWithPath + oidc.JWKSEndpointPath)] = jwks.NewHandler(issuer, m.dynamicJWKSProvider) + m.providerHandlers[(issuerHostWithPath + oidc.WellKnownEndpointPath)] = discovery.NewHandler(issuerURL) - m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPathV1Alpha1)] = idpdiscovery.NewHandler(m.upstreamIDPs) + m.providerHandlers[(issuerHostWithPath + oidc.JWKSEndpointPath)] = jwks.NewHandler(issuerURL, m.dynamicJWKSProvider) + + m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPathV1Alpha1)] = idpdiscovery.NewHandler(idpLister) m.providerHandlers[(issuerHostWithPath + oidc.AuthorizationEndpointPath)] = auth.NewHandler( - issuer, - m.upstreamIDPs, + issuerURL, + idpLister, oauthHelperWithNullStorage, oauthHelperWithKubeStorage, csrftoken.Generate, @@ -141,26 +144,26 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs ) m.providerHandlers[(issuerHostWithPath + oidc.CallbackEndpointPath)] = callback.NewHandler( - m.upstreamIDPs, + idpLister, oauthHelperWithKubeStorage, upstreamStateEncoder, csrfCookieEncoder, - issuer+oidc.CallbackEndpointPath, + issuerURL+oidc.CallbackEndpointPath, ) m.providerHandlers[(issuerHostWithPath + oidc.TokenEndpointPath)] = token.NewHandler( - m.upstreamIDPs, + idpLister, oauthHelperWithKubeStorage, ) m.providerHandlers[(issuerHostWithPath + oidc.PinnipedLoginPath)] = login.NewHandler( upstreamStateEncoder, csrfCookieEncoder, - login.NewGetHandler(incomingProvider.IssuerPath()+oidc.PinnipedLoginPath), - login.NewPostHandler(issuer, m.upstreamIDPs, oauthHelperWithKubeStorage), + login.NewGetHandler(incomingFederationDomain.IssuerPath()+oidc.PinnipedLoginPath), + login.NewPostHandler(issuerURL, idpLister, oauthHelperWithKubeStorage), ) - plog.Debug("oidc provider manager added or updated issuer", "issuer", issuer) + plog.Debug("oidc provider manager added or updated issuer", "issuer", issuerURL) } } diff --git a/internal/oidc/provider/upstreamprovider/upsteam_provider.go b/internal/oidc/provider/upstreamprovider/upsteam_provider.go new file mode 100644 index 00000000..68695b25 --- /dev/null +++ b/internal/oidc/provider/upstreamprovider/upsteam_provider.go @@ -0,0 +1,126 @@ +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package upstreamprovider + +import ( + "context" + "net/url" + + "golang.org/x/oauth2" + "k8s.io/apimachinery/pkg/types" + + "go.pinniped.dev/internal/authenticators" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/oidctypes" + "go.pinniped.dev/pkg/oidcclient/pkce" +) + +type RevocableTokenType string + +// These strings correspond to the token types defined by https://datatracker.ietf.org/doc/html/rfc7009#section-2.1 +const ( + RefreshTokenType RevocableTokenType = "refresh_token" + AccessTokenType RevocableTokenType = "access_token" +) + +// RefreshAttributes contains information about the user from the original login request +// and previous refreshes. +type RefreshAttributes struct { + Username string + Subject string + DN string + Groups []string + AdditionalAttributes map[string]string + GrantedScopes []string +} + +type UpstreamOIDCIdentityProviderI interface { + // GetName returns a name for this upstream provider. The controller watching the OIDCIdentityProviders will + // set this to be the Name of the CR from its metadata. Note that this is different from the DisplayName configured + // in each FederationDomain that uses this provider, so this name is for internal use only, not for interacting + // with clients. Clients should not expect to see this name or send this name. + GetName() string + + // GetClientID returns the OAuth client ID registered with the upstream provider to be used in the authorization code flow. + GetClientID() string + + // GetResourceUID returns the Kubernetes resource ID + GetResourceUID() types.UID + + // GetAuthorizationURL returns the Authorization Endpoint fetched from discovery. + GetAuthorizationURL() *url.URL + + // HasUserInfoURL returns whether there is a non-empty value for userinfo_endpoint fetched from discovery. + HasUserInfoURL() bool + + // GetScopes returns the scopes to request in authorization (authcode or password grant) flow. + GetScopes() []string + + // GetUsernameClaim returns the ID Token username claim name. May return empty string, in which case we + // will use some reasonable defaults. + GetUsernameClaim() string + + // GetGroupsClaim returns the ID Token groups claim name. May return empty string, in which case we won't + // try to read groups from the upstream provider. + GetGroupsClaim() string + + // AllowsPasswordGrant returns true if a client should be allowed to use the resource owner password credentials grant + // flow with this upstream provider. When false, it should not be allowed. + AllowsPasswordGrant() bool + + // GetAdditionalAuthcodeParams returns additional params to be sent on authcode requests. + GetAdditionalAuthcodeParams() map[string]string + + // GetAdditionalClaimMappings returns additional claims to be mapped from the upstream ID token. + GetAdditionalClaimMappings() map[string]string + + // PasswordCredentialsGrantAndValidateTokens performs upstream OIDC resource owner password credentials grant and + // token validation. Returns the validated raw tokens as well as the parsed claims of the ID token. + PasswordCredentialsGrantAndValidateTokens(ctx context.Context, username, password string) (*oidctypes.Token, error) + + // ExchangeAuthcodeAndValidateTokens performs upstream OIDC authorization code exchange and token validation. + // Returns the validated raw tokens as well as the parsed claims of the ID token. + ExchangeAuthcodeAndValidateTokens( + ctx context.Context, + authcode string, + pkceCodeVerifier pkce.Code, + expectedIDTokenNonce nonce.Nonce, + redirectURI string, + ) (*oidctypes.Token, error) + + // PerformRefresh will call the provider's token endpoint to perform a refresh grant. The provider may or may not + // return a new ID or refresh token in the response. If it returns an ID token, then use ValidateToken to + // validate the ID token. + PerformRefresh(ctx context.Context, refreshToken string) (*oauth2.Token, error) + + // RevokeToken will attempt to revoke the given token, if the provider has a revocation endpoint. + // It may return an error wrapped by a RetryableRevocationError, which is an error indicating that it may + // be worth trying to revoke the same token again later. Any other error returned should be assumed to + // represent an error such that it is not worth retrying revocation later, even though revocation failed. + RevokeToken(ctx context.Context, token string, tokenType RevocableTokenType) error + + // ValidateTokenAndMergeWithUserInfo will validate the ID token. It will also merge the claims from the userinfo endpoint response + // into the ID token's claims, if the provider offers the userinfo endpoint. It returns the validated/updated + // tokens, or an error. + ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool, requireUserInfo bool) (*oidctypes.Token, error) +} + +type UpstreamLDAPIdentityProviderI interface { + // GetName returns a name for this upstream provider. + GetName() string + + // GetURL returns a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234". + // This URL is not used for connecting to the provider, but rather is used for creating a globally unique user + // identifier by being combined with the user's UID, since user UIDs are only unique within one provider. + GetURL() *url.URL + + // GetResourceUID returns the Kubernetes resource ID + GetResourceUID() types.UID + + // UserAuthenticator adds an interface method for performing user authentication against the upstream LDAP provider. + authenticators.UserAuthenticator + + // PerformRefresh performs a refresh against the upstream LDAP identity provider + PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes) (groups []string, err error) +} diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 9e016ee2..4d2bf708 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package token provides a handler for the OIDC token endpoint. @@ -19,15 +19,17 @@ import ( oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/httputil/httperr" + "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/downstreamsession" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" ) func NewHandler( - idpLister oidc.UpstreamIdentityProvidersLister, + idpLister provider.FederationDomainIdentityProvidersListerI, oauthHelper fosite.OAuth2Provider, ) http.Handler { return httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { @@ -95,7 +97,7 @@ func errUpstreamRefreshError() *fosite.RFC6749Error { } } -func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, providerCache oidc.UpstreamIdentityProvidersLister) error { +func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, idpLister provider.FederationDomainIdentityProvidersListerI) error { session := accessRequest.GetSession().(*psession.PinnipedSession) customSessionData := session.Custom @@ -113,11 +115,11 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, switch customSessionData.ProviderType { case psession.ProviderTypeOIDC: - return upstreamOIDCRefresh(ctx, session, providerCache, grantedScopes, clientID) + return upstreamOIDCRefresh(ctx, session, idpLister, grantedScopes, clientID) case psession.ProviderTypeLDAP: - return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes, clientID) + return upstreamLDAPRefresh(ctx, idpLister, session, grantedScopes, clientID) case psession.ProviderTypeActiveDirectory: - return upstreamLDAPRefresh(ctx, providerCache, session, grantedScopes, clientID) + return upstreamLDAPRefresh(ctx, idpLister, session, grantedScopes, clientID) default: return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } @@ -126,7 +128,7 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, func upstreamOIDCRefresh( ctx context.Context, session *psession.PinnipedSession, - providerCache oidc.UpstreamIdentityProvidersLister, + idpLister provider.FederationDomainIdentityProvidersListerI, grantedScopes []string, clientID string, ) error { @@ -143,7 +145,7 @@ func upstreamOIDCRefresh( return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } - p, err := findOIDCProviderByNameAndValidateUID(s, providerCache) + p, err := findOIDCProviderByNameAndValidateUID(s, idpLister) if err != nil { return err } @@ -153,7 +155,7 @@ func upstreamOIDCRefresh( var tokens *oauth2.Token if refreshTokenStored { - tokens, err = p.PerformRefresh(ctx, s.OIDC.UpstreamRefreshToken) + tokens, err = p.Provider.PerformRefresh(ctx, s.OIDC.UpstreamRefreshToken) if err != nil { return errUpstreamRefreshError().WithHint( "Upstream refresh failed.", @@ -174,7 +176,7 @@ func upstreamOIDCRefresh( // way to check that the user's session was not revoked on the server. // The spec is not 100% clear about whether an ID token from the refresh flow should include a nonce, and at // least some providers do not include one, so we skip the nonce validation here (but not other validations). - validatedTokens, err := p.ValidateTokenAndMergeWithUserInfo(ctx, tokens, "", hasIDTok, accessTokenStored) + validatedTokens, err := p.Provider.ValidateTokenAndMergeWithUserInfo(ctx, tokens, "", hasIDTok, accessTokenStored) if err != nil { return errUpstreamRefreshError().WithHintf( "Upstream refresh returned an invalid ID token or UserInfo response.").WithTrace(err). @@ -182,12 +184,22 @@ func upstreamOIDCRefresh( } mergedClaims := validatedTokens.IDToken.Claims - // To the extent possible, check that the user's basic identity hasn't changed. - err = validateIdentityUnchangedSinceInitialLogin(mergedClaims, session, p.GetUsernameClaim()) + oldTransformedUsername, err := getDownstreamUsernameFromPinnipedSession(session) + if err != nil { + return err + } + oldTransformedGroups, err := getDownstreamGroupsFromPinnipedSession(session) if err != nil { return err } + // To the extent possible, check that the user's basic identity hasn't changed. + err = validateSubjectAndIssuerUnchangedSinceInitialLogin(mergedClaims, session) + if err != nil { + return err + } + + var refreshedUntransformedGroups []string 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 @@ -197,26 +209,44 @@ func upstreamOIDCRefresh( // If the claim is found, then use it to update the user's group membership in the session. // If the claim is not found, then we have no new information about groups, so skip updating the group membership // and let any old groups memberships in the session remain. - refreshedGroups, err := downstreamsession.GetGroupsFromUpstreamIDToken(p, mergedClaims) + refreshedUntransformedGroups, err = downstreamsession.GetGroupsFromUpstreamIDToken(p.Provider, mergedClaims) if err != nil { return errUpstreamRefreshError().WithHintf( "Upstream refresh error while extracting groups claim.").WithTrace(err). WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) } - if refreshedGroups != nil { - oldGroups, err := getDownstreamGroupsFromPinnipedSession(session) - if err != nil { - return err - } - username, err := getDownstreamUsernameFromPinnipedSession(session) - if err != nil { - return err - } - warnIfGroupsChanged(ctx, oldGroups, refreshedGroups, username, clientID) - session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = refreshedGroups - } } + // It's possible that a username wasn't returned by the upstream provider during refresh, + // but if it is, verify that the transformed version of it hasn't changed. + refreshedUntransformedUsername, hasRefreshedUntransformedUsername := getString(mergedClaims, p.Provider.GetUsernameClaim()) + + if !hasRefreshedUntransformedUsername { + // If we could not get a new username, then we still need the untransformed username to be able to + // run the transformations again, so fetch the original untransformed username from the session. + refreshedUntransformedUsername = s.UpstreamUsername + } + if refreshedUntransformedGroups == nil { + // If we could not get a new list of groups, then we still need the untransformed groups list to be able to + // run the transformations again, so fetch the original untransformed groups list from the session. + refreshedUntransformedGroups = s.UpstreamGroups + } + + transformationResult, err := transformRefreshedIdentity(ctx, + p.Transforms, + oldTransformedUsername, + refreshedUntransformedUsername, + refreshedUntransformedGroups, + s.ProviderName, + s.ProviderType, + ) + if err != nil { + return err + } + + warnIfGroupsChanged(ctx, oldTransformedGroups, transformationResult.Groups, transformationResult.Username, clientID) + session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = refreshedUntransformedGroups + // Upstream refresh may or may not return a new refresh token. If we got a new refresh token, then update it in // the user's session. If we did not get a new refresh token, then keep the old one in the session by avoiding // overwriting the old one. @@ -238,7 +268,7 @@ func diffSortedGroups(oldGroups, newGroups []string) ([]string, []string) { return added.List(), removed.List() } -func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interface{}, session *psession.PinnipedSession, usernameClaimName string) error { +func validateSubjectAndIssuerUnchangedSinceInitialLogin(mergedClaims map[string]interface{}, session *psession.PinnipedSession) error { s := session.Custom // If we have any claims at all, we better have a subject, and it better match the previous value. @@ -260,19 +290,6 @@ func validateIdentityUnchangedSinceInitialLogin(mergedClaims map[string]interfac WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) } - newUsername, hasUsername := getString(mergedClaims, usernameClaimName) - 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 { - return errUpstreamRefreshError().WithHintf( - "Upstream refresh failed.").WithTrace(errors.New("username in upstream refresh does not match previous value")). - WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) - } - 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. @@ -292,11 +309,11 @@ func getString(m map[string]interface{}, key string) (string, bool) { func findOIDCProviderByNameAndValidateUID( s *psession.CustomSessionData, - providerCache oidc.UpstreamIdentityProvidersLister, -) (provider.UpstreamOIDCIdentityProviderI, error) { - for _, p := range providerCache.GetOIDCIdentityProviders() { - if p.GetName() == s.ProviderName { - if p.GetResourceUID() != s.ProviderUID { + idpLister provider.FederationDomainIdentityProvidersListerI, +) (*provider.FederationDomainResolvedOIDCIdentityProvider, error) { + for _, p := range idpLister.GetOIDCIdentityProviders() { + if p.Provider.GetName() == s.ProviderName { + if p.Provider.GetResourceUID() != s.ProviderUID { return nil, errorsx.WithStack(errUpstreamRefreshError().WithHint( "Provider from upstream session data has changed its resource UID since authentication.")) } @@ -310,27 +327,25 @@ func findOIDCProviderByNameAndValidateUID( func upstreamLDAPRefresh( ctx context.Context, - providerCache oidc.UpstreamIdentityProvidersLister, + idpLister provider.FederationDomainIdentityProvidersListerI, session *psession.PinnipedSession, grantedScopes []string, clientID string, ) error { - username, err := getDownstreamUsernameFromPinnipedSession(session) + oldTransformedUsername, err := getDownstreamUsernameFromPinnipedSession(session) if err != nil { return err } subject := session.Fosite.Claims.Subject - var oldGroups []string + var oldTransformedGroups []string if slices.Contains(grantedScopes, oidcapi.ScopeGroups) { - oldGroups, err = getDownstreamGroupsFromPinnipedSession(session) + oldTransformedGroups, err = getDownstreamGroupsFromPinnipedSession(session) if err != nil { return err } } - s := session.Custom - // if you have neither a valid ldap session config nor a valid active directory session config validLDAP := s.ProviderType == psession.ProviderTypeLDAP && s.LDAP != nil && s.LDAP.UserDN != "" validAD := s.ProviderType == psession.ProviderTypeActiveDirectory && s.ActiveDirectory != nil && s.ActiveDirectory.UserDN != "" if !(validLDAP || validAD) { @@ -344,20 +359,19 @@ func upstreamLDAPRefresh( additionalAttributes = s.ActiveDirectory.ExtraRefreshAttributes } - // get ldap/ad provider out of cache - p, dn, err := findLDAPProviderByNameAndValidateUID(s, providerCache) + p, dn, err := findLDAPProviderByNameAndValidateUID(s, idpLister) if err != nil { return err } if session.IDTokenClaims().AuthTime.IsZero() { return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } - // run PerformRefresh - groups, err := p.PerformRefresh(ctx, provider.RefreshAttributes{ - Username: username, + + refreshedUntransformedGroups, err := p.Provider.PerformRefresh(ctx, upstreamprovider.RefreshAttributes{ + Username: s.UpstreamUsername, Subject: subject, DN: dn, - Groups: oldGroups, + Groups: s.UpstreamGroups, AdditionalAttributes: additionalAttributes, GrantedScopes: grantedScopes, }) @@ -366,33 +380,79 @@ func upstreamLDAPRefresh( "Upstream refresh failed.").WithTrace(err). WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType) } + + transformationResult, err := transformRefreshedIdentity(ctx, + p.Transforms, + oldTransformedUsername, + s.UpstreamUsername, + refreshedUntransformedGroups, + s.ProviderName, + s.ProviderType, + ) + if err != nil { + return err + } + groupsScope := slices.Contains(grantedScopes, oidcapi.ScopeGroups) if groupsScope { - warnIfGroupsChanged(ctx, oldGroups, groups, username, clientID) + warnIfGroupsChanged(ctx, oldTransformedGroups, transformationResult.Groups, transformationResult.Username, clientID) // Replace the old value with the new value. - session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = groups + session.Fosite.Claims.Extra[oidcapi.IDTokenClaimGroups] = transformationResult.Groups } return nil } +func transformRefreshedIdentity( + ctx context.Context, + transforms *idtransform.TransformationPipeline, + oldTransformedUsername string, + upstreamUsername string, + upstreamGroups []string, + providerName string, + providerType psession.ProviderType, +) (*idtransform.TransformationResult, error) { + transformationResult, err := transforms.Evaluate(ctx, upstreamUsername, upstreamGroups) + if err != nil { + return nil, errUpstreamRefreshError().WithHintf( + "Upstream refresh error while applying configured identity transformations."). + WithTrace(err). + WithDebugf("provider name: %q, provider type: %q", providerName, providerType) + } + + if !transformationResult.AuthenticationAllowed { + return nil, errUpstreamRefreshError().WithHintf( + "Upstream refresh rejected by configured identity policy: %s.", transformationResult.RejectedAuthenticationMessage). + WithDebugf("provider name: %q, provider type: %q", providerName, providerType) + } + + if oldTransformedUsername != transformationResult.Username { + return nil, errUpstreamRefreshError().WithHintf( + "Upstream refresh failed."). + WithTrace(errors.New("username in upstream refresh does not match previous value")). + WithDebugf("provider name: %q, provider type: %q", providerName, providerType) + } + + return transformationResult, nil +} + func findLDAPProviderByNameAndValidateUID( s *psession.CustomSessionData, - providerCache oidc.UpstreamIdentityProvidersLister, -) (provider.UpstreamLDAPIdentityProviderI, string, error) { - var providers []provider.UpstreamLDAPIdentityProviderI + idpLister provider.FederationDomainIdentityProvidersListerI, +) (*provider.FederationDomainResolvedLDAPIdentityProvider, string, error) { + var providers []*provider.FederationDomainResolvedLDAPIdentityProvider var dn string if s.ProviderType == psession.ProviderTypeLDAP { - providers = providerCache.GetLDAPIdentityProviders() + providers = idpLister.GetLDAPIdentityProviders() dn = s.LDAP.UserDN } else if s.ProviderType == psession.ProviderTypeActiveDirectory { - providers = providerCache.GetActiveDirectoryIdentityProviders() + providers = idpLister.GetActiveDirectoryIdentityProviders() dn = s.ActiveDirectory.UserDN } for _, p := range providers { - if p.GetName() == s.ProviderName { - if p.GetResourceUID() != s.ProviderUID { + if p.Provider.GetName() == s.ProviderName { + if p.Provider.GetResourceUID() != s.ProviderUID { return nil, "", errorsx.WithStack(errUpstreamRefreshError().WithHint( "Provider from upstream session data has changed its resource UID since authentication."). WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 136b2312..bc995ba0 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package psession @@ -32,6 +32,18 @@ type CustomSessionData struct { // all users must have a username. Username string `json:"username"` + // UpstreamUsername is the username from the upstream identity provider during the user's initial login before + // identity transformations were applied. We store this so that we can still reapply identity transformations + // during refresh flows even when an upstream OIDC provider does not return the username again during the upstream + // refresh, and so we can validate that same untransformed username was found during an LDAP refresh. + UpstreamUsername string `json:"upstreamUsername"` + + // UpstreamGroups is the groups list from the upstream identity provider during the user's initial login before + // identity transformations were applied. We store this so that we can still reapply identity transformations + // during refresh flows even when an OIDC provider does not return the groups again during the upstream + // refresh, and when the LDAP search was configured to skip group refreshes. + UpstreamGroups []string `json:"upstreamGroups"` + // 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. @@ -41,11 +53,12 @@ type CustomSessionData struct { // The Kubernetes resource name of the identity provider CRD for the upstream IDP used to start this session. // Used during a downstream refresh to decide which upstream to refresh. - // Also used to decide which of the pointer types below should be used. + // Also used by the session storage garbage collector to decide which upstream to use for token revocation. ProviderName string `json:"providerName"` // The type of the identity provider for the upstream IDP used to start this session. // Used during a downstream refresh to decide which upstream to refresh. + // Also used to decide which of the pointer types below should be used. ProviderType ProviderType `json:"providerType"` // Warnings that were encountered at some point during login that should be emitted to the client. @@ -55,8 +68,10 @@ type CustomSessionData struct { // Only used when ProviderType == "oidc". OIDC *OIDCSessionData `json:"oidc,omitempty"` + // Only used when ProviderType == "ldap". LDAP *LDAPSessionData `json:"ldap,omitempty"` + // Only used when ProviderType == "activedirectory". ActiveDirectory *ActiveDirectorySessionData `json:"activedirectory,omitempty"` } diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index bfbb36bf..11b5c988 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -169,6 +169,9 @@ func prepareControllers( clock.RealClock{}, pinnipedClient, federationDomainInformer, + pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), + pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders(), + pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(), controllerlib.WithInformer, ), singletonWorker, diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 474e5cbc..ee566c1f 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -35,6 +35,7 @@ import ( pkce2 "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestoragei" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -77,7 +78,7 @@ type PerformRefreshArgs struct { type RevokeTokenArgs struct { Ctx context.Context Token string - TokenType provider.RevocableTokenType + TokenType upstreamprovider.RevocableTokenType } // ValidateTokenAndMergeWithUserInfoArgs is used to spy on calls to @@ -93,7 +94,7 @@ type ValidateTokenAndMergeWithUserInfoArgs struct { type ValidateRefreshArgs struct { Ctx context.Context Tok *oauth2.Token - StoredAttributes provider.RefreshAttributes + StoredAttributes upstreamprovider.RefreshAttributes } type TestUpstreamLDAPIdentityProvider struct { @@ -107,7 +108,7 @@ type TestUpstreamLDAPIdentityProvider struct { PerformRefreshGroups []string } -var _ provider.UpstreamLDAPIdentityProviderI = &TestUpstreamLDAPIdentityProvider{} +var _ upstreamprovider.UpstreamLDAPIdentityProviderI = &TestUpstreamLDAPIdentityProvider{} func (u *TestUpstreamLDAPIdentityProvider) GetResourceUID() types.UID { return u.ResourceUID @@ -125,7 +126,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { return u.URL } -func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { +func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes upstreamprovider.RefreshAttributes) ([]string, error) { if u.performRefreshArgs == nil { u.performRefreshArgs = make([]*PerformRefreshArgs, 0) } @@ -182,7 +183,7 @@ type TestUpstreamOIDCIdentityProvider struct { PerformRefreshFunc func(ctx context.Context, refreshToken string) (*oauth2.Token, error) - RevokeTokenFunc func(ctx context.Context, refreshToken string, tokenType provider.RevocableTokenType) error + RevokeTokenFunc func(ctx context.Context, refreshToken string, tokenType upstreamprovider.RevocableTokenType) error ValidateTokenAndMergeWithUserInfoFunc func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) @@ -198,7 +199,7 @@ type TestUpstreamOIDCIdentityProvider struct { validateTokenAndMergeWithUserInfoArgs []*ValidateTokenAndMergeWithUserInfoArgs } -var _ provider.UpstreamOIDCIdentityProviderI = &TestUpstreamOIDCIdentityProvider{} +var _ upstreamprovider.UpstreamOIDCIdentityProviderI = &TestUpstreamOIDCIdentityProvider{} func (u *TestUpstreamOIDCIdentityProvider) GetResourceUID() types.UID { return u.ResourceUID @@ -302,7 +303,7 @@ func (u *TestUpstreamOIDCIdentityProvider) PerformRefresh(ctx context.Context, r return u.PerformRefreshFunc(ctx, refreshToken) } -func (u *TestUpstreamOIDCIdentityProvider) RevokeToken(ctx context.Context, token string, tokenType provider.RevocableTokenType) error { +func (u *TestUpstreamOIDCIdentityProvider) RevokeToken(ctx context.Context, token string, tokenType upstreamprovider.RevocableTokenType) error { if u.revokeTokenArgs == nil { u.revokeTokenArgs = make([]*RevokeTokenArgs, 0) } @@ -387,21 +388,21 @@ func (b *UpstreamIDPListerBuilder) WithActiveDirectory(upstreamActiveDirectoryId func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { idpProvider := provider.NewDynamicUpstreamIDPProvider() - oidcUpstreams := make([]provider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders)) + oidcUpstreams := make([]upstreamprovider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders)) for i := range b.upstreamOIDCIdentityProviders { - oidcUpstreams[i] = provider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i]) + oidcUpstreams[i] = upstreamprovider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i]) } idpProvider.SetOIDCIdentityProviders(oidcUpstreams) - ldapUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, len(b.upstreamLDAPIdentityProviders)) + ldapUpstreams := make([]upstreamprovider.UpstreamLDAPIdentityProviderI, len(b.upstreamLDAPIdentityProviders)) for i := range b.upstreamLDAPIdentityProviders { - ldapUpstreams[i] = provider.UpstreamLDAPIdentityProviderI(b.upstreamLDAPIdentityProviders[i]) + ldapUpstreams[i] = upstreamprovider.UpstreamLDAPIdentityProviderI(b.upstreamLDAPIdentityProviders[i]) } idpProvider.SetLDAPIdentityProviders(ldapUpstreams) - adUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, len(b.upstreamActiveDirectoryIdentityProviders)) + adUpstreams := make([]upstreamprovider.UpstreamLDAPIdentityProviderI, len(b.upstreamActiveDirectoryIdentityProviders)) for i := range b.upstreamActiveDirectoryIdentityProviders { - adUpstreams[i] = provider.UpstreamLDAPIdentityProviderI(b.upstreamActiveDirectoryIdentityProviders[i]) + adUpstreams[i] = upstreamprovider.UpstreamLDAPIdentityProviderI(b.upstreamActiveDirectoryIdentityProviders[i]) } idpProvider.SetActiveDirectoryIdentityProviders(adUpstreams) @@ -822,7 +823,7 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent } return u.refreshedTokens, nil }, - RevokeTokenFunc: func(ctx context.Context, refreshToken string, tokenType provider.RevocableTokenType) error { + RevokeTokenFunc: func(ctx context.Context, refreshToken string, tokenType upstreamprovider.RevocableTokenType) error { return u.revokeTokenErr }, ValidateTokenAndMergeWithUserInfoFunc: func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index beb6a037..7dd0dcc6 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -28,7 +28,7 @@ import ( "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/oidc/downstreamsession" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" ) @@ -120,7 +120,7 @@ type ProviderConfig struct { GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error) // RefreshAttributeChecks are extra checks that attributes in a refresh response are as expected. - RefreshAttributeChecks map[string]func(*ldap.Entry, provider.RefreshAttributes) error + RefreshAttributeChecks map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error } // UserSearchConfig contains information about how to search for users in the upstream LDAP IDP. @@ -167,7 +167,7 @@ type Provider struct { c ProviderConfig } -var _ provider.UpstreamLDAPIdentityProviderI = &Provider{} +var _ upstreamprovider.UpstreamLDAPIdentityProviderI = &Provider{} var _ authenticators.UserAuthenticator = &Provider{} // New creates a Provider. The config is not a pointer to ensure that a copy of the config is created, @@ -188,7 +188,7 @@ func closeAndLogError(conn Conn, doingWhat string) { } } -func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { +func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes upstreamprovider.RefreshAttributes) ([]string, error) { t := trace.FromContext(ctx).Nest("slow ldap refresh attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches userDN := storedRefreshAttributes.DN diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 7de3f828..02476b25 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -26,7 +26,7 @@ import ( "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/tlsassertions" "go.pinniped.dev/internal/testutil/tlsserver" @@ -661,8 +661,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes upstreamprovider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes upstreamprovider.RefreshAttributes) error { return nil }, } @@ -699,8 +699,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes upstreamprovider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes upstreamprovider.RefreshAttributes) error { return nil }, } @@ -1575,8 +1575,8 @@ func TestUpstreamRefresh(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupSearchGroupNameAttribute, }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ - pwdLastSetAttribute: func(*ldap.Entry, provider.RefreshAttributes) error { return nil }, + RefreshAttributeChecks: map[string]func(*ldap.Entry, upstreamprovider.RefreshAttributes) error{ + pwdLastSetAttribute: func(*ldap.Entry, upstreamprovider.RefreshAttributes) error { return nil }, }, } if editFunc != nil { @@ -2280,7 +2280,7 @@ func TestUpstreamRefresh(t *testing.T) { initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000")) ldapProvider := New(*tt.providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" - groups, err := ldapProvider.PerformRefresh(context.Background(), provider.RefreshAttributes{ + groups, err := ldapProvider.PerformRefresh(context.Background(), upstreamprovider.RefreshAttributes{ Username: testUserSearchResultUsernameAttributeValue, Subject: subject, DN: tt.refreshUserDN, diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go index ff8b83fc..dc61b11f 100644 --- a/internal/upstreamoidc/upstreamoidc.go +++ b/internal/upstreamoidc/upstreamoidc.go @@ -23,13 +23,14 @@ import ( oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" "go.pinniped.dev/pkg/oidcclient/pkce" ) -func New(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { +func New(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { return &ProviderConfig{Config: config, Provider: provider, Client: client} } @@ -52,7 +53,7 @@ type ProviderConfig struct { } } -var _ provider.UpstreamOIDCIdentityProviderI = (*ProviderConfig)(nil) +var _ upstreamprovider.UpstreamOIDCIdentityProviderI = (*ProviderConfig)(nil) func (p *ProviderConfig) GetResourceUID() types.UID { return p.ResourceUID @@ -160,7 +161,7 @@ func (p *ProviderConfig) PerformRefresh(ctx context.Context, refreshToken string // It may return an error wrapped by a RetryableRevocationError, which is an error indicating that it may // be worth trying to revoke the same token again later. Any other error returned should be assumed to // represent an error such that it is not worth retrying revocation later, even though revocation failed. -func (p *ProviderConfig) RevokeToken(ctx context.Context, token string, tokenType provider.RevocableTokenType) error { +func (p *ProviderConfig) RevokeToken(ctx context.Context, token string, tokenType upstreamprovider.RevocableTokenType) error { if p.RevocationURL == nil { plog.Trace("RevokeToken() was called but upstream provider has no available revocation endpoint", "providerName", p.Name, @@ -188,7 +189,7 @@ func (p *ProviderConfig) RevokeToken(ctx context.Context, token string, tokenTyp func (p *ProviderConfig) tryRevokeToken( ctx context.Context, token string, - tokenType provider.RevocableTokenType, + tokenType upstreamprovider.RevocableTokenType, useBasicAuth bool, ) (tryAnotherClientAuthMethod bool, err error) { clientID := p.Config.ClientID diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go index 1155deb3..e477f1ba 100644 --- a/internal/upstreamoidc/upstreamoidc_test.go +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -25,6 +25,7 @@ import ( "go.pinniped.dev/internal/mocks/mockkeyset" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" @@ -484,7 +485,7 @@ func TestProviderConfig(t *testing.T) { t.Run("RevokeToken", func(t *testing.T) { tests := []struct { name string - tokenType provider.RevocableTokenType + tokenType upstreamprovider.RevocableTokenType nilRevocationURL bool unreachableServer bool returnStatusCodes []int @@ -496,33 +497,33 @@ func TestProviderConfig(t *testing.T) { }{ { name: "success without calling the server when there is no revocation URL set for refresh token", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, nilRevocationURL: true, wantNumRequests: 0, }, { name: "success without calling the server when there is no revocation URL set for access token", - tokenType: provider.AccessTokenType, + tokenType: upstreamprovider.AccessTokenType, nilRevocationURL: true, wantNumRequests: 0, }, { name: "success when the server returns 200 OK on the first call for refresh token", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusOK}, wantNumRequests: 1, wantTokenTypeHint: "refresh_token", }, { name: "success when the server returns 200 OK on the first call for access token", - tokenType: provider.AccessTokenType, + tokenType: upstreamprovider.AccessTokenType, returnStatusCodes: []int{http.StatusOK}, wantNumRequests: 1, wantTokenTypeHint: "access_token", }, { name: "success when the server returns 400 Bad Request on the first call due to client auth, then 200 OK on second call for refresh token", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest, http.StatusOK}, // https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 defines this as the error for client auth failure returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`}, @@ -531,7 +532,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "success when the server returns 400 Bad Request on the first call due to client auth, then 200 OK on second call for access token", - tokenType: provider.AccessTokenType, + tokenType: upstreamprovider.AccessTokenType, returnStatusCodes: []int{http.StatusBadRequest, http.StatusOK}, // https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 defines this as the error for client auth failure returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`}, @@ -540,7 +541,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when the server returns 400 Bad Request on the first call due to client auth, then any 400 error on second call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest, http.StatusBadRequest}, returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`, `{ "error":"anything", "error_description":"unhappy" }`}, wantErr: testutil.WantExactErrorString(`server responded with status 400 with body: { "error":"anything", "error_description":"unhappy" }`), @@ -550,7 +551,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when the server returns 400 Bad Request with bad JSON body on the first call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest}, returnErrBodies: []string{`invalid JSON body`}, wantErr: testutil.WantExactErrorString(`error parsing response body "invalid JSON body" on response with status code 400: invalid character 'i' looking for beginning of value`), @@ -560,7 +561,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when the server returns 400 Bad Request with empty body", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest}, returnErrBodies: []string{``}, wantErr: testutil.WantExactErrorString(`error parsing response body "" on response with status code 400: unexpected end of JSON input`), @@ -570,7 +571,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when the server returns 400 Bad Request on the first call due to client auth, then any other error on second call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest, http.StatusForbidden}, returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`, ""}, wantErr: testutil.WantExactErrorString("server responded with status 403"), @@ -580,7 +581,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when server returns any other 400 error on first call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusBadRequest}, returnErrBodies: []string{`{ "error":"anything_else", "error_description":"unhappy" }`}, wantErr: testutil.WantExactErrorString(`server responded with status 400 with body: { "error":"anything_else", "error_description":"unhappy" }`), @@ -590,7 +591,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "error when server returns any other error aside from 400 on first call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusForbidden}, returnErrBodies: []string{""}, wantErr: testutil.WantExactErrorString("server responded with status 403"), @@ -600,7 +601,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "retryable error when server returns 503 on first call", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusServiceUnavailable}, // 503 returnErrBodies: []string{""}, wantErr: testutil.WantExactErrorString("retryable revocation error: server responded with status 503"), @@ -610,7 +611,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "retryable error when the server returns 400 Bad Request on the first call due to client auth, then 503 on second call", - tokenType: provider.AccessTokenType, + tokenType: upstreamprovider.AccessTokenType, returnStatusCodes: []int{http.StatusBadRequest, http.StatusServiceUnavailable}, // 400, 503 returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`, ""}, wantErr: testutil.WantExactErrorString("retryable revocation error: server responded with status 503"), @@ -620,7 +621,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "retryable error when server returns any 5xx status on first call, testing lower bound of 5xx range", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{http.StatusInternalServerError}, // 500 returnErrBodies: []string{""}, wantErr: testutil.WantExactErrorString("retryable revocation error: server responded with status 500"), @@ -630,7 +631,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "retryable error when server returns any 5xx status on first call, testing upper bound of 5xx range", - tokenType: provider.RefreshTokenType, + tokenType: upstreamprovider.RefreshTokenType, returnStatusCodes: []int{599}, // not defined by an RFC, but sometimes considered Network Connect Timeout Error returnErrBodies: []string{""}, wantErr: testutil.WantExactErrorString("retryable revocation error: server responded with status 599"), @@ -640,7 +641,7 @@ func TestProviderConfig(t *testing.T) { }, { name: "retryable error when the server cannot be reached", - tokenType: provider.AccessTokenType, + tokenType: upstreamprovider.AccessTokenType, unreachableServer: true, wantErr: testutil.WantMatchingErrorString("^retryable revocation error: Post .*: dial tcp .*: connect: connection refused$"), wantRetryableErrType: true, diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index b75f743e..341d1ffd 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -33,7 +33,7 @@ import ( "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/httputil/securityheader" "go.pinniped.dev/internal/net/phttp" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamoidc" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -107,7 +107,7 @@ type handlerState struct { getEnv func(key string) string listen func(string, string) (net.Listener, error) isTTY func(int) bool - getProvider func(*oauth2.Config, *coreosoidc.Provider, *http.Client) provider.UpstreamOIDCIdentityProviderI + getProvider func(*oauth2.Config, *coreosoidc.Provider, *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI validateIDToken func(ctx context.Context, provider *coreosoidc.Provider, audience string, token string) (*coreosoidc.IDToken, error) promptForValue func(ctx context.Context, promptLabel string) (string, error) promptForSecret func(promptLabel string) (string, error) @@ -196,10 +196,11 @@ func WithSkipListen() Option { // SessionCacheKey contains the data used to select a valid session cache entry. type SessionCacheKey struct { - Issuer string `json:"issuer"` - ClientID string `json:"clientID"` - Scopes []string `json:"scopes"` - RedirectURI string `json:"redirect_uri"` + Issuer string `json:"issuer"` + ClientID string `json:"clientID"` + Scopes []string `json:"scopes"` + RedirectURI string `json:"redirect_uri"` + UpstreamProviderName string `json:"upstream_provider_name,omitempty"` } type SessionCache interface { @@ -351,6 +352,10 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { ClientID: h.clientID, Scopes: h.scopes, RedirectURI: (&url.URL{Scheme: "http", Host: h.listenAddr, Path: h.callbackPath}).String(), + // When using a Supervisor with multiple IDPs, the cache keys need to be different for each IDP + // so a user can have multiple sessions going for each IDP at the same time. + // When using a non-Supervisor OIDC provider, then this value will be blank, so it won't be part of the key. + UpstreamProviderName: h.upstreamIdentityProviderName, } // If the ID token is still valid for a bit, return it immediately and skip the rest of the flow. diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index ab393030..fa896aa9 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidcclient @@ -32,7 +32,7 @@ import ( "go.pinniped.dev/internal/httputil/roundtripper" "go.pinniped.dev/internal/mocks/mockupstreamoidcidentityprovider" "go.pinniped.dev/internal/net/phttp" - "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/testlogger" @@ -504,7 +504,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { require.NoError(t, WithClient(newClientForServer(successServer))(h)) - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -553,7 +553,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { require.NoError(t, WithClient(newClientForServer(successServer))(h)) - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -1159,7 +1159,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo fmt.Sprintf("http://127.0.0.1:0/callback?code=%s&state=test-state", fakeAuthCode), }}, }, nil) - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1181,7 +1181,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1281,7 +1281,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1392,7 +1392,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1855,7 +1855,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) h.cache = cache - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -1993,7 +1993,7 @@ func TestHandlePasteCallback(t *testing.T) { return "invalid", nil } h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "invalid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -2017,7 +2017,7 @@ func TestHandlePasteCallback(t *testing.T) { return "valid", nil } h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -2237,7 +2237,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "invalid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -2256,7 +2256,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -2282,7 +2282,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { return func(h *handlerState) error { h.useFormPost = true h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -2311,7 +2311,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { return func(h *handlerState) error { h.useFormPost = true h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI).