From 29584619707d6dc6141a33b5d34d7db3ad2c1799 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Fri, 7 Jan 2022 15:04:58 -0800 Subject: [PATCH] Addressing PR feedback store issuer and subject in storage for refresh Clean up some constants Signed-off-by: Margo Crawford --- .../oidc_upstream_watcher.go | 6 +- .../oidc_upstream_watcher_test.go | 50 ++++++------ .../accesstoken/accesstoken_test.go | 6 +- .../authorizationcode/authorizationcode.go | 24 +++--- .../authorizationcode_test.go | 7 +- .../openidconnect/openidconnect_test.go | 4 +- internal/fositestorage/pkce/pkce_test.go | 4 +- .../refreshtoken/refreshtoken_test.go | 6 +- internal/oidc/auth/auth_handler.go | 18 ++++- internal/oidc/auth/auth_handler_test.go | 4 +- internal/oidc/callback/callback_handler.go | 13 ++- .../oidc/callback/callback_handler_test.go | 8 +- .../downstreamsession/downstream_session.go | 10 +-- .../provider/dynamic_upstream_idp_provider.go | 4 +- internal/oidc/token/token_handler.go | 32 ++++---- internal/oidc/token/token_handler_test.go | 21 ++--- internal/psession/pinniped_session.go | 4 +- internal/testutil/psession.go | 4 +- internal/upstreamoidc/upstreamoidc.go | 81 +++++++++---------- internal/upstreamoidc/upstreamoidc_test.go | 57 ------------- test/integration/supervisor_login_test.go | 6 +- 21 files changed, 178 insertions(+), 191 deletions(-) diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 909c07fb..4669cdc3 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package oidcupstreamwatcher implements a controller which watches OIDCIdentityProviders. @@ -499,7 +499,7 @@ func validateHTTPSURL(maybeHTTPSURL, endpointType, reason string) (*url.URL, *v1 Type: typeOIDCDiscoverySucceeded, Status: v1alpha1.ConditionFalse, Reason: reason, - Message: fmt.Sprintf(`%s URL scheme must be "https", not %q`, endpointType, parsedURL.Scheme), + Message: fmt.Sprintf(`%s URL '%s' must have "https" scheme, not %q`, endpointType, maybeHTTPSURL, parsedURL.Scheme), } } if len(parsedURL.Query()) != 0 || parsedURL.Fragment != "" { @@ -507,7 +507,7 @@ func validateHTTPSURL(maybeHTTPSURL, endpointType, reason string) (*url.URL, *v1 Type: typeOIDCDiscoverySucceeded, Status: v1alpha1.ConditionFalse, Reason: reason, - Message: fmt.Sprintf(`%s URL cannot contain query or fragment component`, endpointType), + Message: fmt.Sprintf(`%s URL '%s' cannot contain query or fragment component`, endpointType, maybeHTTPSURL), } } return parsedURL, nil diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 8a50d13a..62e308e2 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidcupstreamwatcher @@ -457,9 +457,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL scheme must be \"https\", not \"http\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL '` + strings.Replace(testIssuerURL, "https", "http", 1) + `' must have \"https\" scheme, not \"http\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL '` + strings.Replace(testIssuerURL, "https", "http", 1) + `' must have \"https\" scheme, not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -480,7 +480,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { Status: "False", LastTransitionTime: now, Reason: "Unreachable", - Message: `issuer URL scheme must be "https", not "http"`, + Message: `issuer URL '` + strings.Replace(testIssuerURL, "https", "http", 1) + `' must have "https" scheme, not "http"`, }, }, }, @@ -503,9 +503,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL cannot contain query or fragment component" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL '` + testIssuerURL + "?sub=foo" + `' cannot contain query or fragment component" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL cannot contain query or fragment component" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL '` + testIssuerURL + "?sub=foo" + `' cannot contain query or fragment component" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -526,7 +526,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { Status: "False", LastTransitionTime: now, Reason: "Unreachable", - Message: `issuer URL cannot contain query or fragment component`, + Message: `issuer URL '` + testIssuerURL + "?sub=foo" + `' cannot contain query or fragment component`, }, }, }, @@ -549,9 +549,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL cannot contain query or fragment component" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="issuer URL '` + testIssuerURL + "#fragment" + `' cannot contain query or fragment component" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL cannot contain query or fragment component" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="issuer URL '` + testIssuerURL + "#fragment" + `' cannot contain query or fragment component" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -572,7 +572,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { Status: "False", LastTransitionTime: now, Reason: "Unreachable", - Message: `issuer URL cannot contain query or fragment component`, + Message: `issuer URL '` + testIssuerURL + "#fragment" + `' cannot contain query or fragment component`, }, }, }, @@ -739,9 +739,9 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL 'http://example.com/authorize' must have \"https\" scheme, not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="authorization endpoint URL 'http://example.com/authorize' must have \"https\" scheme, not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -762,7 +762,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana Status: "False", LastTransitionTime: now, Reason: "InvalidResponse", - Message: `authorization endpoint URL scheme must be "https", not "http"`, + Message: `authorization endpoint URL 'http://example.com/authorize' must have "https" scheme, not "http"`, }, }, }, @@ -786,9 +786,9 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="revocation endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="revocation endpoint URL 'http://example.com/revoke' must have \"https\" scheme, not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="revocation endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="revocation endpoint URL 'http://example.com/revoke' must have \"https\" scheme, not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -809,7 +809,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana Status: "False", LastTransitionTime: now, Reason: "InvalidResponse", - Message: `revocation endpoint URL scheme must be "https", not "http"`, + Message: `revocation endpoint URL 'http://example.com/revoke' must have "https" scheme, not "http"`, }, }, }, @@ -833,9 +833,9 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="token endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="token endpoint URL 'http://example.com/token' must have \"https\" scheme, not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="token endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="token endpoint URL 'http://example.com/token' must have \"https\" scheme, not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -856,7 +856,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana Status: "False", LastTransitionTime: now, Reason: "InvalidResponse", - Message: `token endpoint URL scheme must be "https", not "http"`, + Message: `token endpoint URL 'http://example.com/token' must have "https" scheme, not "http"`, }, }, }, @@ -880,9 +880,9 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="token endpoint URL scheme must be \"https\", not \"\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="token endpoint URL '' must have \"https\" scheme, not \"\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="token endpoint URL scheme must be \"https\", not \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="token endpoint URL '' must have \"https\" scheme, not \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -903,7 +903,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana Status: "False", LastTransitionTime: now, Reason: "InvalidResponse", - Message: `token endpoint URL scheme must be "https", not ""`, + Message: `token endpoint URL '' must have "https" scheme, not ""`, }, }, }, @@ -927,9 +927,9 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL scheme must be \"https\", not \"\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL '' must have \"https\" scheme, not \"\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="authorization endpoint URL scheme must be \"https\", not \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="authorization endpoint URL '' must have \"https\" scheme, not \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -950,7 +950,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana Status: "False", LastTransitionTime: now, Reason: "InvalidResponse", - Message: `authorization endpoint URL scheme must be "https", not ""`, + Message: `authorization endpoint URL '' must have "https" scheme, not ""`, }, }, }, diff --git a/internal/fositestorage/accesstoken/accesstoken_test.go b/internal/fositestorage/accesstoken/accesstoken_test.go index 2623ea5f..5b503371 100644 --- a/internal/fositestorage/accesstoken/accesstoken_test.go +++ b/internal/fositestorage/accesstoken/accesstoken_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package accesstoken @@ -53,7 +53,7 @@ func TestAccessTokenStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/access-token", @@ -122,7 +122,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/access-token", diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index 1d678ec5..e7c6655b 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package authorizationcode @@ -370,29 +370,33 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ "providerName": "ʼn2ƋŢ觛ǂ焺nŐǛ", "providerType": "ɥ闣ʬ橳(ý綃ʃʚƟ覣k眐4", "oidc": { - "upstreamRefreshToken": "tC嵽痊w" + "upstreamRefreshToken": "tC嵽痊w", + "upstreamSubject": "a紽ǒ|鰽ŋ猊I", + "upstreamIssuer": "妬\u003e6鉢緋uƴŤȱʀ" }, "ldap": { - "userDN": "Ź榨Q|ôɵt毇妬\u003e6鉢緋", + "userDN": "Â?墖\u003cƬb獭潜Ʃ饾k|鬌R蜚蠣", "extraRefreshAttributes": { - "ď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ¡圔": "墀jMʥ", - "ƍ蛊ʚ£:設虝27": "b獭潜Ʃ饾k|鬌R蜚蠣麹概", - "藚ɏ¬Ê蒭堜]ȗ韚ʫ": "鷞aŚB碠k9帴ʘ赱" + "ȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ": "鷞aŚB碠k9帴ʘ赱" } }, "activedirectory": { - "userDN": "0D餹sêĝɓ", + "userDN": "瑹xȢ~1Įx欼笝?úT妼", "extraRefreshAttributes": { - "摱ì": "bEǎ儯惝Io" + "iYn": "麹Œ颛", + "İ\u003e×1飞O+î艔垎0OƉǢIȽ齤士": "ȐĨf跞@)¿,ɭS隑ip偶" } } } }, "requestedAudience": [ - "Ł" + "應,Ɣ鬅X¤", + "¤.岵骘胲ƤkǦ" ], "grantedAudience": [ - "r" + "鸖I¶媁y衑拁Ȃ", + "社Vƅȭǝ*擦28Dž 甍 ć", + "bņ抰蛖a³2ʫ承dʬ)ġ,TÀqy_" ] }, "version": "2" diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index 006ed61f..72c4cfba 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package authorizationcode @@ -65,7 +65,7 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", @@ -84,7 +84,7 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", @@ -389,6 +389,7 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { validSessionJSONBytes, err := json.MarshalIndent(validSession, "", "\t") require.NoError(t, err) authorizeCodeSessionJSONFromFuzzing := string(validSessionJSONBytes) + t.Log(authorizeCodeSessionJSONFromFuzzing) // the fuzzed session and storage session should have identical JSON require.JSONEq(t, authorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromStorage) diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go index 1ceafa9e..ac680a60 100644 --- a/internal/fositestorage/openidconnect/openidconnect_test.go +++ b/internal/fositestorage/openidconnect/openidconnect_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package openidconnect @@ -52,7 +52,7 @@ func TestOpenIdConnectStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/oidc", diff --git a/internal/fositestorage/pkce/pkce_test.go b/internal/fositestorage/pkce/pkce_test.go index e21dda33..b84798ce 100644 --- a/internal/fositestorage/pkce/pkce_test.go +++ b/internal/fositestorage/pkce/pkce_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package pkce @@ -52,7 +52,7 @@ func TestPKCEStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/pkce", diff --git a/internal/fositestorage/refreshtoken/refreshtoken_test.go b/internal/fositestorage/refreshtoken/refreshtoken_test.go index ae7a1226..ce74793a 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken_test.go +++ b/internal/fositestorage/refreshtoken/refreshtoken_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package refreshtoken @@ -52,7 +52,7 @@ func TestRefreshTokenStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/refresh-token", @@ -122,7 +122,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/refresh-token", diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index f9760825..b08d39bc 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package auth provides a handler for the OIDC authorization endpoint. @@ -191,6 +191,20 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, ) } + upstreamSubject, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenSubjectClaim, oidcUpstream.GetName(), token.IDToken.Claims) + if err != nil { + // Return a user-friendly error for this case which is entirely within our control. + return writeAuthorizeError(w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, + ) + } + upstreamIssuer, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenIssuerClaim, oidcUpstream.GetName(), token.IDToken.Claims) + if err != nil { + // Return a user-friendly error for this case which is entirely within our control. + return writeAuthorizeError(w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, + ) + } customSessionData := &psession.CustomSessionData{ ProviderUID: oidcUpstream.GetResourceUID(), @@ -198,6 +212,8 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( ProviderType: psession.ProviderTypeOIDC, OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: token.RefreshToken.Token, + UpstreamIssuer: upstreamIssuer, + UpstreamSubject: upstreamSubject, }, } return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups, customSessionData) diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 274ca0e9..845e39b4 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package auth @@ -470,6 +470,8 @@ func TestAuthorizationEndpoint(t *testing.T) { ProviderType: psession.ProviderTypeOIDC, OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: oidcPasswordGrantUpstreamRefreshToken, + UpstreamSubject: oidcUpstreamSubject, + UpstreamIssuer: oidcUpstreamIssuer, }, } diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 22f37a55..1a5a3aee 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package callback provides a handler for the OIDC callback endpoint. @@ -83,12 +83,23 @@ func NewHandler( return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } + upstreamSubject, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), token.IDToken.Claims) + if err != nil { + return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) + } + upstreamIssuer, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), token.IDToken.Claims) + if err != nil { + return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) + } + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, &psession.CustomSessionData{ ProviderUID: upstreamIDPConfig.GetResourceUID(), ProviderName: upstreamIDPConfig.GetName(), ProviderType: psession.ProviderTypeOIDC, OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: token.RefreshToken.Token, + UpstreamSubject: upstreamSubject, + UpstreamIssuer: upstreamIssuer, }, }) diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 48e04afd..d14c159f 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package callback @@ -77,7 +77,11 @@ var ( ProviderUID: happyUpstreamIDPResourceUID, ProviderName: happyUpstreamIDPName, ProviderType: psession.ProviderTypeOIDC, - OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamRefreshToken}, + OIDC: &psession.OIDCSessionData{ + UpstreamRefreshToken: oidcUpstreamRefreshToken, + UpstreamIssuer: oidcUpstreamIssuer, + UpstreamSubject: oidcUpstreamSubject, + }, } ) diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index 5f0cf5f0..265ed5c1 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package downstreamsession provides some shared helpers for creating downstream OIDC sessions. @@ -89,11 +89,11 @@ func getSubjectAndUsernameFromUpstreamIDToken( ) (string, string, error) { // The spec says the "sub" claim is only unique per issuer, // so we will prepend the issuer string to make it globally unique. - upstreamIssuer, err := extractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), idTokenClaims) + upstreamIssuer, err := ExtractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), idTokenClaims) if err != nil { return "", "", err } - upstreamSubject, err := extractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), idTokenClaims) + upstreamSubject, err := ExtractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), idTokenClaims) if err != nil { return "", "", err } @@ -128,7 +128,7 @@ func getSubjectAndUsernameFromUpstreamIDToken( } } - username, err := extractStringClaimValue(usernameClaimName, upstreamIDPConfig.GetName(), idTokenClaims) + username, err := ExtractStringClaimValue(usernameClaimName, upstreamIDPConfig.GetName(), idTokenClaims) if err != nil { return "", "", err } @@ -136,7 +136,7 @@ func getSubjectAndUsernameFromUpstreamIDToken( return subject, username, nil } -func extractStringClaimValue(claimName string, upstreamIDPName string, idTokenClaims map[string]interface{}) (string, error) { +func ExtractStringClaimValue(claimName string, upstreamIDPName string, idTokenClaims map[string]interface{}) (string, error) { value, ok := idTokenClaims[claimName] if !ok { plog.Warning( diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 9986f018..24b821e7 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package provider @@ -71,7 +71,7 @@ type UpstreamOIDCIdentityProviderI interface { // RevokeRefreshToken will attempt to revoke the given token, if the provider has a revocation endpoint. RevokeRefreshToken(ctx context.Context, refreshToken string) error - // ValidateToken will validate the ID token. It will also merge the claims from the userinfo endpoint response + // 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) (*oidctypes.Token, error) diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index b1888a18..e0fc4408 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package token provides a handler for the OIDC token endpoint. @@ -17,7 +17,6 @@ import ( "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" - "go.pinniped.dev/internal/upstreamoidc" ) var ( @@ -138,29 +137,27 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, // if we have any claims at all, we better have a subject, and it better match the previous value. // but it's possible that we don't because both returning a new refresh token on refresh and having a userinfo // endpoint are optional. - if len(validatedTokens.IDToken.Claims) != 0 { - newSub := claims["sub"] - oldDownstreamSubject := session.Fosite.Claims.Subject - oldIss, oldSub, err := upstreamoidc.ExtractUpstreamSubjectAndIssuerFromDownstream(oldDownstreamSubject) - if err != nil { - return errorsx.WithStack(errUpstreamRefreshError.WithHintf("Upstream refresh failed."). - WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + if len(validatedTokens.IDToken.Claims) != 0 { //nolint:nestif + newSub, hasSub := getString(claims, oidc.IDTokenSubjectClaim) + if !hasSub { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh not found")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } - if oldSub != newSub { + if s.OIDC.UpstreamSubject != newSub { return errorsx.WithStack(errUpstreamRefreshError.WithHintf( "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } usernameClaim := p.GetUsernameClaim() - newUsername := claims[usernameClaim] - oldUsername := session.Fosite.Claims.Extra["username"] + newUsername, hasUsername := getString(claims, usernameClaim) + oldUsername := session.Fosite.Claims.Extra[oidc.DownstreamUsernameClaim] // its possible this won't be returned. // but if it is, verify that it hasn't changed. - if newUsername != nil && oldUsername != newUsername { + if hasUsername && oldUsername != newUsername { return errorsx.WithStack(errUpstreamRefreshError.WithHintf( "Upstream refresh failed.").WithWrap(errors.New("username in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } - newIssuer := claims["iss"] - if newIssuer != nil && oldIss != newIssuer { + newIssuer, hasIssuer := getString(claims, oidc.IDTokenIssuerClaim) + if hasIssuer && s.OIDC.UpstreamIssuer != newIssuer { return errorsx.WithStack(errUpstreamRefreshError.WithHintf( "Upstream refresh failed.").WithWrap(errors.New("issuer in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } @@ -178,6 +175,11 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, return nil } +func getString(m map[string]interface{}, key string) (string, bool) { + val, ok := m[key].(string) + return val, ok +} + func findOIDCProviderByNameAndValidateUID( s *psession.CustomSessionData, providerCache oidc.UpstreamIdentityProvidersLister, diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 24fea4ad..2a8f039a 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package token @@ -57,6 +57,7 @@ import ( const ( goodIssuer = "https://some-issuer.com" + goodUpstreamSubject = "some-subject" goodClient = "pinniped-cli" goodRedirectURI = "http://127.0.0.1/callback" goodPKCECodeVerifier = "some-pkce-verifier-that-must-be-at-least-43-characters-to-meet-entropy-requirements" @@ -910,6 +911,8 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: oidcUpstreamInitialRefreshToken, + UpstreamSubject: goodUpstreamSubject, + UpstreamIssuer: goodIssuer, }, } } @@ -1049,7 +1052,7 @@ func TestRefreshGrant(t *testing.T) { upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ - "sub": "some-subject", + "sub": goodUpstreamSubject, }, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), @@ -1072,7 +1075,7 @@ func TestRefreshGrant(t *testing.T) { IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", - "sub": "some-subject", + "sub": goodUpstreamSubject, "username-claim": goodUsername, }, }, @@ -1146,7 +1149,7 @@ func TestRefreshGrant(t *testing.T) { upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ - "sub": "some-subject", + "sub": goodUpstreamSubject, }, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDTokenWithoutRefreshToken()).Build()), @@ -1168,7 +1171,7 @@ func TestRefreshGrant(t *testing.T) { upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ - "sub": "some-subject", + "sub": goodUpstreamSubject, }, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), @@ -1193,7 +1196,7 @@ func TestRefreshGrant(t *testing.T) { upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ - "sub": "some-subject", + "sub": goodUpstreamSubject, }, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), @@ -1229,7 +1232,7 @@ func TestRefreshGrant(t *testing.T) { upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ - "sub": "some-subject", + "sub": goodUpstreamSubject, }, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), @@ -1681,7 +1684,7 @@ func TestRefreshGrant(t *testing.T) { IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", - "sub": "some-subject", + "sub": goodUpstreamSubject, "username-claim": "some-changed-username", }, }, @@ -1712,7 +1715,7 @@ func TestRefreshGrant(t *testing.T) { IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", - "sub": "some-subject", + "sub": goodUpstreamSubject, "iss": "some-changed-issuer", }, }, diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index d7fd47df..bbcc4317 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package psession @@ -62,6 +62,8 @@ const ( // OIDCSessionData is the additional data needed by Pinniped when the upstream IDP is an OIDC provider. type OIDCSessionData struct { UpstreamRefreshToken string `json:"upstreamRefreshToken"` + UpstreamSubject string `json:"upstreamSubject"` + UpstreamIssuer string `json:"upstreamIssuer"` } // LDAPSessionData is the additional data needed by Pinniped when the upstream IDP is an LDAP provider. diff --git a/internal/testutil/psession.go b/internal/testutil/psession.go index 28e65968..83aacb13 100644 --- a/internal/testutil/psession.go +++ b/internal/testutil/psession.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package testutil @@ -29,6 +29,8 @@ func NewFakePinnipedSession() *psession.PinnipedSession { ProviderName: "fake-provider-name", OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: "fake-upstream-refresh-token", + UpstreamSubject: "some-subject", + UpstreamIssuer: "some-issuer", }, }, } diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go index f9256f23..29dd8cac 100644 --- a/internal/upstreamoidc/upstreamoidc.go +++ b/internal/upstreamoidc/upstreamoidc.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package upstreamoidc implements an abstraction of upstream OIDC provider interactions. @@ -7,7 +7,6 @@ package upstreamoidc import ( "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -238,53 +237,19 @@ func (p *ProviderConfig) tryRevokeRefreshToken( } } -func ExtractUpstreamSubjectAndIssuerFromDownstream(downstreamSubject string) (string, string, error) { - if !strings.Contains(downstreamSubject, "?sub=") { - return "", "", errors.New("downstream subject did not contain original upstream subject") - } - split := strings.SplitN(downstreamSubject, "?sub=", 2) - iss := split[0] - sub := split[1] - if iss == "" || sub == "" { - return "", "", errors.New("downstream subject was malformed") - } - return split[0], split[1], nil -} - // ValidateTokenAndMergeWithUserInfo will validate the ID token. It will also merge the claims from the userinfo endpoint response, // if the provider offers the userinfo endpoint. func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool) (*oidctypes.Token, error) { var validatedClaims = make(map[string]interface{}) - idTok, hasIDTok := tok.Extra("id_token").(string) var idTokenExpiry time.Time // if we require the id token, make sure we have it. // also, if it exists but wasn't required, still make sure it passes these checks. - // nolint:nestif - if hasIDTok || requireIDToken { - if !hasIDTok { - return nil, httperr.New(http.StatusBadRequest, "received response missing ID token") - } - validated, err := p.Provider.Verifier(&coreosoidc.Config{ClientID: p.GetClientID()}).Verify(coreosoidc.ClientContext(ctx, p.Client), idTok) - if err != nil { - return nil, httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) - } - if validated.AccessTokenHash != "" { - if err := validated.VerifyAccessToken(tok.AccessToken); err != nil { - return nil, httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) - } - } - if expectedIDTokenNonce != "" { - if err := expectedIDTokenNonce.Validate(validated); err != nil { - return nil, httperr.Wrap(http.StatusBadRequest, "received ID token with invalid nonce", err) - } - } - if err := validated.Claims(&validatedClaims); err != nil { - return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal id token claims", err) - } - maybeLogClaims("claims from ID token", p.Name, validatedClaims) - idTokenExpiry = validated.Expiry // keep track of the id token expiry if we have an id token. Otherwise, it'll just be the zero value. + idTokenExpiry, idTok, err := p.validateIDToken(ctx, tok, expectedIDTokenNonce, validatedClaims, requireIDToken) + if err != nil { + return nil, err } + idTokenSubject, _ := validatedClaims[oidc.IDTokenSubjectClaim].(string) if len(idTokenSubject) > 0 || !requireIDToken { @@ -310,10 +275,42 @@ func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context, }, nil } +func (p *ProviderConfig) validateIDToken(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, validatedClaims map[string]interface{}, requireIDToken bool) (time.Time, string, error) { + idTok, hasIDTok := tok.Extra("id_token").(string) + if !hasIDTok && !requireIDToken { + return time.Time{}, "", nil // exit early + } + + var idTokenExpiry time.Time + if !hasIDTok { + return time.Time{}, "", httperr.New(http.StatusBadRequest, "received response missing ID token") + } + validated, err := p.Provider.Verifier(&coreosoidc.Config{ClientID: p.GetClientID()}).Verify(coreosoidc.ClientContext(ctx, p.Client), idTok) + if err != nil { + return time.Time{}, "", httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) + } + if validated.AccessTokenHash != "" { + if err := validated.VerifyAccessToken(tok.AccessToken); err != nil { + return time.Time{}, "", httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) + } + } + if expectedIDTokenNonce != "" { + if err := expectedIDTokenNonce.Validate(validated); err != nil { + return time.Time{}, "", httperr.Wrap(http.StatusBadRequest, "received ID token with invalid nonce", err) + } + } + if err := validated.Claims(&validatedClaims); err != nil { + return time.Time{}, "", httperr.Wrap(http.StatusInternalServerError, "could not unmarshal id token claims", err) + } + maybeLogClaims("claims from ID token", p.Name, validatedClaims) + idTokenExpiry = validated.Expiry // keep track of the id token expiry if we have an id token. Otherwise, it'll just be the zero value. + return idTokenExpiry, idTok, nil +} + func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}, requireIDToken bool) error { idTokenSubject, _ := claims[oidc.IDTokenSubjectClaim].(string) - userInfo, err := p.fetchUserInfo(ctx, tok) + userInfo, err := p.maybeFetchUserInfo(ctx, tok) if err != nil { return err } @@ -356,7 +353,7 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t return nil } -func (p *ProviderConfig) fetchUserInfo(ctx context.Context, tok *oauth2.Token) (*coreosoidc.UserInfo, error) { +func (p *ProviderConfig) maybeFetchUserInfo(ctx context.Context, tok *oauth2.Token) (*coreosoidc.UserInfo, error) { providerJSON := &struct { UserInfoURL string `json:"userinfo_endpoint"` }{} diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go index e5b90c80..90dc6f1f 100644 --- a/internal/upstreamoidc/upstreamoidc_test.go +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -910,63 +910,6 @@ func TestProviderConfig(t *testing.T) { } }) - t.Run("ExtractUpstreamSubjectAndIssuerFromDownstream", func(t *testing.T) { - tests := []struct { - name string - downstreamSubject string - wantUpstreamSubject string - wantUpstreamIssuer string - wantErr string - }{ - { - name: "happy path", - downstreamSubject: "https://some-issuer?sub=some-subject", - wantUpstreamSubject: "some-subject", - wantUpstreamIssuer: "https://some-issuer", - }, - { - name: "subject in a subject", - downstreamSubject: "https://some-other-issuer?sub=https://some-issuer?sub=some-subject", - wantUpstreamSubject: "https://some-issuer?sub=some-subject", - wantUpstreamIssuer: "https://some-other-issuer", - }, - { - name: "sub is empty string", - downstreamSubject: "https://some-issuer?sub=", - wantErr: "downstream subject was malformed", - }, - { - name: "iss is empty string", - downstreamSubject: "?sub=some-subject", - wantErr: "downstream subject was malformed", - }, - { - name: "empty string", - downstreamSubject: "", - wantErr: "downstream subject did not contain original upstream subject", - }, - { - name: "doesn't contain sub=", - downstreamSubject: "something-invalid", - wantErr: "downstream subject did not contain original upstream subject", - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - actualUpstreamIssuer, actualUpstreamSubject, err := ExtractUpstreamSubjectAndIssuerFromDownstream(tt.downstreamSubject) - if tt.wantErr != "" { - require.Error(t, err) - require.Equal(t, tt.wantErr, err.Error()) - } else { - require.NoError(t, err) - require.Equal(t, tt.wantUpstreamSubject, actualUpstreamSubject) - require.Equal(t, tt.wantUpstreamIssuer, actualUpstreamIssuer) - } - }) - } - }) - t.Run("ExchangeAuthcodeAndValidateTokens", func(t *testing.T) { tests := []struct { name string diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index db2f617e..29a46e79 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -87,8 +87,8 @@ func TestSupervisorLogin(t *testing.T) { }, requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlow, breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { - fositeSessionData := pinnipedSession.Fosite - fositeSessionData.Claims.Subject = "wrong-subject" + pinnipedSessionData := pinnipedSession.Custom + pinnipedSessionData.OIDC.UpstreamIssuer = "wrong-issuer" }, // the ID token Subject should include the upstream user ID after the upstream issuer name wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+",