From 1bd346cbeb7826654edda57f7e0b88d10044cde0 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Fri, 8 Oct 2021 15:48:21 -0700 Subject: [PATCH] Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time --- .../active_directory_upstream_watcher.go | 5 +- .../active_directory_upstream_watcher_test.go | 82 +++-- .../ldap_upstream_watcher.go | 5 +- .../ldap_upstream_watcher_test.go | 52 ++-- .../oidc_upstream_watcher.go | 9 +- .../oidc_upstream_watcher_test.go | 87 +++--- .../authorizationcode_test.go | 4 + .../mockupstreamoidcidentityprovider.go | 29 ++ internal/oidc/auth/auth_handler.go | 61 +++- internal/oidc/auth/auth_handler_test.go | 292 ++++++++++++------ internal/oidc/callback/callback_handler.go | 18 +- .../oidc/callback/callback_handler_test.go | 58 +++- .../downstreamsession/downstream_session.go | 3 +- .../provider/dynamic_upstream_idp_provider.go | 10 + .../oidc/provider/manager/manager_test.go | 1 + internal/oidc/token/token_handler_test.go | 2 +- internal/psession/pinniped_session.go | 21 +- .../testutil/oidctestutil/oidctestutil.go | 106 +++++-- internal/testutil/psession.go | 2 +- internal/upstreamldap/upstreamldap.go | 8 + internal/upstreamoidc/upstreamoidc.go | 25 +- 21 files changed, 636 insertions(+), 244 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index d6490846..7bd83f39 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -301,8 +301,9 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, adUpstreamImpl := &activeDirectoryUpstreamGenericLDAPImpl{activeDirectoryIdentityProvider: *upstream} config := &upstreamldap.ProviderConfig{ - Name: upstream.Name, - Host: spec.Host, + Name: upstream.Name, + ResourceUID: upstream.UID, + Host: spec.Host, UserSearch: upstreamldap.UserSearchConfig{ Base: spec.UserSearch.Base, Filter: adUpstreamImpl.Spec().UserSearch().Filter(), 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 33aea395..da204216 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -150,6 +150,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { const ( testNamespace = "test-namespace" + testResourceUID = "test-uid" testName = "test-name" testSecretName = "test-bind-secret" testBindUsername = "test-bind-username" @@ -172,7 +173,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { testCABundleBase64Encoded := base64.StdEncoding.EncodeToString(testCABundle) validUpstream := &v1alpha1.ActiveDirectoryIdentityProvider{ - ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234, UID: testResourceUID}, Spec: v1alpha1.ActiveDirectoryIdentityProviderSpec{ Host: testHost, TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testCABundleBase64Encoded}, @@ -202,6 +203,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { providerConfigForValidUpstreamWithTLS := &upstreamldap.ProviderConfig{ Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -364,7 +366,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -379,7 +381,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -407,7 +409,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -434,7 +436,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -460,7 +462,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -486,7 +488,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -517,6 +519,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: nil, @@ -537,7 +540,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -572,6 +575,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: nil, @@ -592,7 +596,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -630,6 +634,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: "ldap.example.com", ConnectionProtocol: upstreamldap.StartTLS, // successfully fell back to using StartTLS CABundle: testCABundle, @@ -650,7 +655,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -688,6 +693,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { // even though the connection test failed, still loads into the cache because it is treated like a warning { Name: testName, + ResourceUID: testResourceUID, Host: "ldap.example.com:5678", ConnectionProtocol: upstreamldap.TLS, // need to pick TLS or StartTLS to load into the cache when both fail, so choose TLS CABundle: testCABundle, @@ -709,7 +715,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -745,6 +751,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: nil, @@ -765,7 +772,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -779,6 +786,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Name = "other-upstream" upstream.Generation = 42 upstream.Spec.Bind.SecretName = "non-existent-secret" + upstream.UID = "other-uid" })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, setupMocks: func(conn *mockldapconn.MockConn) { @@ -790,7 +798,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{ { - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42, UID: "other-uid"}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -807,7 +815,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, { - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -831,7 +839,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -872,6 +880,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -892,7 +901,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -928,7 +937,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -966,7 +975,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -995,6 +1004,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1015,7 +1025,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -1045,6 +1055,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1065,7 +1076,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -1100,7 +1111,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithStartTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -1137,7 +1148,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -1175,7 +1186,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -1212,7 +1223,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -1243,6 +1254,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1264,7 +1276,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -1295,6 +1307,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1315,7 +1328,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -1350,6 +1363,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1370,7 +1384,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -1399,6 +1413,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1419,7 +1434,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -1447,7 +1462,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -1483,7 +1498,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -1525,7 +1540,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -1554,7 +1569,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -1594,6 +1609,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -1614,7 +1630,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index af748da1..bcbd6d91 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -226,8 +226,9 @@ func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream * spec := upstream.Spec config := &upstreamldap.ProviderConfig{ - Name: upstream.Name, - Host: spec.Host, + Name: upstream.Name, + ResourceUID: upstream.UID, + Host: spec.Host, UserSearch: upstreamldap.UserSearchConfig{ Base: spec.UserSearch.Base, Filter: spec.UserSearch.Filter, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index e9ab4813..c562d969 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -150,6 +150,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { const ( testNamespace = "test-namespace" testName = "test-name" + testResourceUID = "test-resource-uid" testSecretName = "test-bind-secret" testBindUsername = "test-bind-username" testBindPassword = "test-bind-password" @@ -171,7 +172,12 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { testCABundleBase64Encoded := base64.StdEncoding.EncodeToString(testCABundle) validUpstream := &v1alpha1.LDAPIdentityProvider{ - ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + Generation: 1234, + UID: testResourceUID, + }, Spec: v1alpha1.LDAPIdentityProviderSpec{ Host: testHost, TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testCABundleBase64Encoded}, @@ -201,6 +207,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { providerConfigForValidUpstreamWithTLS := &upstreamldap.ProviderConfig{ Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: testCABundle, @@ -299,7 +306,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -320,7 +327,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -348,7 +355,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -375,7 +382,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -401,7 +408,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -427,7 +434,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -458,6 +465,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: nil, @@ -477,7 +485,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -519,6 +527,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: "ldap.example.com", ConnectionProtocol: upstreamldap.StartTLS, // successfully fell back to using StartTLS CABundle: testCABundle, @@ -538,7 +547,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -580,6 +589,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { // even though the connection test failed, still loads into the cache because it is treated like a warning { Name: testName, + ResourceUID: testResourceUID, Host: "ldap.example.com:5678", ConnectionProtocol: upstreamldap.TLS, // need to pick TLS or StartTLS to load into the cache when both fail, so choose TLS CABundle: testCABundle, @@ -600,7 +610,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -635,6 +645,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{ { Name: testName, + ResourceUID: testResourceUID, Host: testHost, ConnectionProtocol: upstreamldap.TLS, CABundle: nil, @@ -654,7 +665,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -674,6 +685,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { upstream.Name = "other-upstream" upstream.Generation = 42 upstream.Spec.Bind.SecretName = "non-existent-secret" + upstream.UID = "other-uid" })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, setupMocks: func(conn *mockldapconn.MockConn) { @@ -685,7 +697,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{ { - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42, UID: "other-uid"}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -702,7 +714,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, { - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -729,7 +741,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ @@ -771,7 +783,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -805,7 +817,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithStartTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -841,7 +853,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -877,7 +889,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -917,7 +929,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), @@ -954,7 +966,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID}, Status: v1alpha1.LDAPIdentityProviderStatus{ Phase: "Ready", Conditions: allConditionsTrue(1234, "4242"), diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index f7b52325..31293951 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -172,9 +172,11 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst Config: &oauth2.Config{ Scopes: computeScopes(upstream.Spec.AuthorizationConfig.AdditionalScopes), }, - UsernameClaim: upstream.Spec.Claims.Username, - GroupsClaim: upstream.Spec.Claims.Groups, - AllowPasswordGrant: upstream.Spec.AuthorizationConfig.AllowPasswordGrant, + UsernameClaim: upstream.Spec.Claims.Username, + GroupsClaim: upstream.Spec.Claims.Groups, + AllowPasswordGrant: upstream.Spec.AuthorizationConfig.AllowPasswordGrant, + AdditionalAuthcodeParams: map[string]string{"prompt": "consent"}, + ResourceUID: upstream.UID, } conditions := []*v1alpha1.Condition{ c.validateSecret(upstream, &result), @@ -374,6 +376,7 @@ func computeScopes(additionalScopes []string) []string { // First compute the unique set of scopes, including "openid" (de-duplicate). set := make(map[string]bool, len(additionalScopes)+1) set["openid"] = true + set["offline_access"] = true for _, s := range additionalScopes { set[s] = true } diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index fd1a864d..73b0d2eb 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -18,6 +18,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" @@ -119,16 +120,18 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wrongCABase64 := base64.StdEncoding.EncodeToString(wrongCA.Bundle()) var ( - testNamespace = "test-namespace" - testName = "test-name" - testSecretName = "test-client-secret" - testAdditionalScopes = []string{"scope1", "scope2", "scope3"} - testExpectedScopes = []string{"openid", "scope1", "scope2", "scope3"} - testClientID = "test-oidc-client-id" - testClientSecret = "test-oidc-client-secret" - testValidSecretData = map[string][]byte{"clientID": []byte(testClientID), "clientSecret": []byte(testClientSecret)} - testGroupsClaim = "test-groups-claim" - testUsernameClaim = "test-username-claim" + testNamespace = "test-namespace" + testName = "test-name" + testSecretName = "test-client-secret" + testAdditionalScopes = []string{"scope1", "scope2", "scope3"} + testExpectedScopes = []string{"offline_access", "openid", "scope1", "scope2", "scope3"} + testExpectedAdditionalParams = map[string]string{"prompt": "consent"} + testClientID = "test-oidc-client-id" + testClientSecret = "test-oidc-client-secret" + testValidSecretData = map[string][]byte{"clientID": []byte(testClientID), "clientSecret": []byte(testClientSecret)} + testGroupsClaim = "test-groups-claim" + testUsernameClaim = "test-username-claim" + testUID = types.UID("test-uid") ) tests := []struct { name string @@ -561,13 +564,13 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana { name: "upstream with error becomes valid", inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name", UID: testUID}, Spec: v1alpha1.OIDCIdentityProviderSpec{ Issuer: testIssuerURL, TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{ - AdditionalScopes: append(testAdditionalScopes, "xyz", "openid"), + AdditionalScopes: append(testAdditionalScopes, "xyz", "openid", "offline_access"), AllowPasswordGrant: true, }, Claims: v1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim}, @@ -591,17 +594,19 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana }, wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{ &oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: testName, - ClientID: testClientID, - AuthorizationURL: *testIssuerAuthorizeURL, - Scopes: append(testExpectedScopes, "xyz"), - UsernameClaim: testUsernameClaim, - GroupsClaim: testGroupsClaim, - AllowPasswordGrant: true, + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: append(testExpectedScopes, "xyz"), + UsernameClaim: testUsernameClaim, + GroupsClaim: testGroupsClaim, + AllowPasswordGrant: true, + AdditionalAuthcodeParams: testExpectedAdditionalParams, + ResourceUID: testUID, }, }, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testUID}, Status: v1alpha1.OIDCIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -614,7 +619,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana { name: "existing valid upstream", inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, Spec: v1alpha1.OIDCIdentityProviderSpec{ Issuer: testIssuerURL, TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, @@ -644,17 +649,19 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana }, wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{ &oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: testName, - ClientID: testClientID, - AuthorizationURL: *testIssuerAuthorizeURL, - Scopes: testExpectedScopes, - UsernameClaim: testUsernameClaim, - GroupsClaim: testGroupsClaim, - AllowPasswordGrant: false, + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: testExpectedScopes, + UsernameClaim: testUsernameClaim, + GroupsClaim: testGroupsClaim, + AllowPasswordGrant: false, + AdditionalAuthcodeParams: testExpectedAdditionalParams, + ResourceUID: testUID, }, }, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, Status: v1alpha1.OIDCIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -667,7 +674,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana { name: "existing valid upstream with trailing slash", inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, Spec: v1alpha1.OIDCIdentityProviderSpec{ Issuer: testIssuerURL + "/ends-with-slash/", TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, @@ -694,17 +701,19 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana }, wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{ &oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: testName, - ClientID: testClientID, - AuthorizationURL: *testIssuerAuthorizeURL, - Scopes: testExpectedScopes, - UsernameClaim: testUsernameClaim, - GroupsClaim: testGroupsClaim, - AllowPasswordGrant: false, + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: testExpectedScopes, + UsernameClaim: testUsernameClaim, + GroupsClaim: testGroupsClaim, + AllowPasswordGrant: false, + AdditionalAuthcodeParams: testExpectedAdditionalParams, + ResourceUID: testUID, }, }, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID}, Status: v1alpha1.OIDCIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ @@ -860,6 +869,8 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs require.Equal(t, tt.wantResultingCache[i].GetUsernameClaim(), actualIDP.GetUsernameClaim()) require.Equal(t, tt.wantResultingCache[i].GetGroupsClaim(), actualIDP.GetGroupsClaim()) require.Equal(t, tt.wantResultingCache[i].AllowsPasswordGrant(), actualIDP.AllowsPasswordGrant()) + require.Equal(t, tt.wantResultingCache[i].GetAdditionalAuthcodeParams(), actualIDP.GetAdditionalAuthcodeParams()) + require.Equal(t, tt.wantResultingCache[i].GetResourceUID(), actualIDP.GetResourceUID()) require.ElementsMatch(t, tt.wantResultingCache[i].GetScopes(), actualIDP.GetScopes()) // We always want to use the proxy from env on these clients, so although the following assertions diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index fdc98e96..50ac0c28 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" @@ -343,6 +344,9 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { func(s *fosite.TokenType, c fuzz.Continue) { *s = fosite.TokenType(randString(c)) }, + func(s *types.UID, c fuzz.Continue) { + *s = types.UID(randString(c)) + }, // handle string type alias func(s *fosite.Arguments, c fuzz.Continue) { n := c.Intn(3) + 1 // 1 to 3 items diff --git a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go index 152f33e2..dfbe785b 100644 --- a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go +++ b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go @@ -18,6 +18,7 @@ import ( oidctypes "go.pinniped.dev/pkg/oidcclient/oidctypes" pkce "go.pinniped.dev/pkg/oidcclient/pkce" oauth2 "golang.org/x/oauth2" + types "k8s.io/apimachinery/pkg/types" ) // MockUpstreamOIDCIdentityProviderI is a mock of UpstreamOIDCIdentityProviderI interface. @@ -72,6 +73,20 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) ExchangeAuthcodeAndVali return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExchangeAuthcodeAndValidateTokens", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).ExchangeAuthcodeAndValidateTokens), arg0, arg1, arg2, arg3, arg4) } +// GetAdditionalAuthcodeParams mocks base method. +func (m *MockUpstreamOIDCIdentityProviderI) GetAdditionalAuthcodeParams() map[string]string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAdditionalAuthcodeParams") + ret0, _ := ret[0].(map[string]string) + return ret0 +} + +// GetAdditionalAuthcodeParams indicates an expected call of GetAdditionalAuthcodeParams. +func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) GetAdditionalAuthcodeParams() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAdditionalAuthcodeParams", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).GetAdditionalAuthcodeParams)) +} + // GetAuthorizationURL mocks base method. func (m *MockUpstreamOIDCIdentityProviderI) GetAuthorizationURL() *url.URL { m.ctrl.T.Helper() @@ -128,6 +143,20 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) GetName() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetName", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).GetName)) } +// GetResourceUID mocks base method. +func (m *MockUpstreamOIDCIdentityProviderI) GetResourceUID() types.UID { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetResourceUID") + ret0, _ := ret[0].(types.UID) + return ret0 +} + +// GetResourceUID indicates an expected call of GetResourceUID. +func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) GetResourceUID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResourceUID", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).GetResourceUID)) +} + // GetScopes mocks base method. func (m *MockUpstreamOIDCIdentityProviderI) GetScopes() []string { m.ctrl.T.Helper() diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 5ff4c1a9..270678bc 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -30,7 +30,10 @@ import ( "go.pinniped.dev/pkg/oidcclient/pkce" ) -const promptParamNone = "none" +const ( + promptParamName = "prompt" + promptParamNone = "none" +) func NewHandler( downstreamIssuer string, @@ -51,13 +54,13 @@ func NewHandler( return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method) } - oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpLister) + oidcUpstream, ldapUpstream, idpType, err := chooseUpstreamIDP(idpLister) if err != nil { plog.WarningErr("authorize upstream config", err) return err } - if oidcUpstream != nil { + if idpType == psession.ProviderTypeOIDC { if len(r.Header.Values(supervisoroidc.AuthorizeUsernameHeaderName)) > 0 { // The client set a username header, so they are trying to log in with a username/password. return handleAuthRequestForOIDCUpstreamPasswordGrant(r, w, oauthHelperWithStorage, oidcUpstream) @@ -74,6 +77,7 @@ func NewHandler( return handleAuthRequestForLDAPUpstream(r, w, oauthHelperWithStorage, ldapUpstream, + idpType, ) })) } @@ -83,6 +87,7 @@ func handleAuthRequestForLDAPUpstream( w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, ldapUpstream provider.UpstreamLDAPIdentityProviderI, + idpType psession.ProviderType, ) error { authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper) if !created { @@ -108,7 +113,14 @@ func handleAuthRequestForLDAPUpstream( username = authenticateResponse.User.GetName() groups := authenticateResponse.User.GetGroups() - return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups) + customSessionData := &psession.CustomSessionData{ + ProviderUID: ldapUpstream.GetResourceUID(), + ProviderName: ldapUpstream.GetName(), + ProviderType: idpType, + } + + return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, + oauthHelper, authorizeRequester, subject, username, groups, customSessionData) } func handleAuthRequestForOIDCUpstreamPasswordGrant( @@ -147,6 +159,15 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( fosite.ErrAccessDenied.WithDebug(err.Error())) // WithDebug hides the error from the client } + if token.RefreshToken == nil || token.RefreshToken.Token == "" { + plog.Warning("refresh token not returned by upstream provider during password grant", + "upstreamName", oidcUpstream.GetName(), + "scopes", oidcUpstream.GetScopes()) + return writeAuthorizeError(w, oauthHelper, authorizeRequester, + fosite.ErrAccessDenied.WithHint( + "Refresh token not returned by upstream provider during password grant.")) + } + subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) if err != nil { // Return a user-friendly error for this case which is entirely within our control. @@ -155,7 +176,15 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( ) } - return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups) + customSessionData := &psession.CustomSessionData{ + ProviderUID: oidcUpstream.GetResourceUID(), + ProviderName: oidcUpstream.GetName(), + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamRefreshToken: token.RefreshToken.Token, + }, + } + return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups, customSessionData) } func handleAuthRequestForOIDCUpstreamAuthcodeGrant( @@ -223,17 +252,20 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant( } authCodeOptions := []oauth2.AuthCodeOption{ - oauth2.AccessTypeOffline, nonceValue.Param(), pkceValue.Challenge(), pkceValue.Method(), } - promptParam := r.Form.Get("prompt") + promptParam := r.Form.Get(promptParamName) if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) { return writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired) } + for key, val := range oidcUpstream.GetAdditionalAuthcodeParams() { + authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(key, val)) + } + if csrfFromCookie == "" { // We did not receive an incoming CSRF cookie, so write a new one. err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec) @@ -280,8 +312,9 @@ func makeDownstreamSessionAndReturnAuthcodeRedirect( subject string, username string, groups []string, + customSessionData *psession.CustomSessionData, ) error { - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { @@ -340,13 +373,13 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { } // Select either an OIDC, an LDAP or an AD IDP, or return an error. -func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, error) { +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( + return nil, nil, "", httperr.New( http.StatusUnprocessableEntity, "No upstream providers are configured", ) @@ -362,16 +395,16 @@ func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) } plog.Warning("Too many upstream providers are configured (found: %s)", upstreamIDPNames) - return nil, nil, httperr.New( + 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, nil + return oidcUpstreams[0], nil, psession.ProviderTypeOIDC, nil case len(adUpstreams) == 1: - return nil, adUpstreams[0], nil + return nil, adUpstreams[0], psession.ProviderTypeActiveDirectory, nil default: - return nil, ldapUpstreams[0], nil + return nil, ldapUpstreams[0], psession.ProviderTypeLDAP, nil } } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 8762f9aa..7f61c103 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -30,6 +30,7 @@ import ( "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -38,16 +39,23 @@ import ( func TestAuthorizationEndpoint(t *testing.T) { const ( - oidcUpstreamName = "some-oidc-idp" - oidcPasswordGrantUpstreamName = "some-password-granting-oidc-idp" + oidcUpstreamName = "some-oidc-idp" + oidcUpstreamResourceUID = "oidc-resource-uid" + oidcPasswordGrantUpstreamName = "some-password-granting-oidc-idp" + oidcPasswordGrantUpstreamResourceUID = "some-password-granting-resource-uid" + ldapUpstreamName = "some-ldap-idp" + ldapUpstreamResourceUID = "ldap-resource-uid" + activeDirectoryUpstreamName = "some-active-directory-idp" + activeDirectoryUpstreamResourceUID = "active-directory-resource-uid" - oidcUpstreamIssuer = "https://my-upstream-issuer.com" - oidcUpstreamSubject = "abc123-some guid" // has a space character which should get escaped in URL - oidcUpstreamSubjectQueryEscaped = "abc123-some+guid" - oidcUpstreamUsername = "test-oidc-pinniped-username" - oidcUpstreamPassword = "test-oidc-pinniped-password" //nolint: gosec - oidcUpstreamUsernameClaim = "the-user-claim" - oidcUpstreamGroupsClaim = "the-groups-claim" + oidcUpstreamIssuer = "https://my-upstream-issuer.com" + oidcUpstreamSubject = "abc123-some guid" // has a space character which should get escaped in URL + oidcUpstreamSubjectQueryEscaped = "abc123-some+guid" + oidcUpstreamUsername = "test-oidc-pinniped-username" + oidcUpstreamPassword = "test-oidc-pinniped-password" //nolint: gosec + oidcUpstreamUsernameClaim = "the-user-claim" + oidcUpstreamGroupsClaim = "the-groups-claim" + oidcPasswordGrantUpstreamRefreshToken = "some-opaque-token" //nolint: gosec downstreamIssuer = "https://my-downstream-issuer.com/some-path" downstreamRedirectURI = "http://127.0.0.1/callback" @@ -146,6 +154,12 @@ func TestAuthorizationEndpoint(t *testing.T) { "state": happyState, } + fositeAccessDeniedWithMissingRefreshTokenErrorQuery = map[string]string{ + "error": "access_denied", + "error_description": "The resource owner or authorization server denied the request. Refresh token not returned by upstream provider during password grant.", + "state": happyState, + } + fositeAccessDeniedWithPasswordGrantDisallowedHintErrorQuery = map[string]string{ "error": "access_denied", "error_description": "The resource owner or authorization server denied the request. Resource owner password credentials grant is not allowed for this upstream provider according to its configuration.", @@ -208,20 +222,22 @@ func TestAuthorizationEndpoint(t *testing.T) { upstreamAuthURL, err := url.Parse("https://some-upstream-idp:8443/auth") require.NoError(t, err) - upstreamOIDCIdentityProvider := func() *oidctestutil.TestUpstreamOIDCIdentityProvider { + upstreamOIDCIdentityProviderBuilder := func() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder { return oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). WithName(oidcUpstreamName). + WithResourceUID(oidcUpstreamResourceUID). WithClientID("some-client-id"). WithAuthorizationURL(*upstreamAuthURL). WithScopes([]string{"scope1", "scope2"}). // the scopes to request when starting the upstream authorization flow WithAllowPasswordGrant(false). - WithPasswordGrantError(errors.New("should not have used password grant on this instance")). - Build() + WithAdditionalAuthcodeParams(map[string]string{}). + WithPasswordGrantError(errors.New("should not have used password grant on this instance")) } passwordGrantUpstreamOIDCIdentityProviderBuilder := func() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder { return oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). WithName(oidcPasswordGrantUpstreamName). + WithResourceUID(oidcPasswordGrantUpstreamResourceUID). WithClientID("some-client-id"). WithAuthorizationURL(*upstreamAuthURL). WithScopes([]string{"scope1", "scope2"}). // the scopes to request when starting the upstream authorization flow @@ -234,6 +250,8 @@ func TestAuthorizationEndpoint(t *testing.T) { WithIDTokenClaim(oidcUpstreamGroupsClaim, oidcUpstreamGroupMembership). WithIDTokenClaim("other-claim", "should be ignored"). WithAllowPasswordGrant(true). + WithRefreshToken(oidcPasswordGrantUpstreamRefreshToken). + WithAdditionalAuthcodeParams(map[string]string{"should-be-ignored": "doesn't apply to password grant"}). WithUpstreamAuthcodeExchangeError(errors.New("should not have tried to exchange upstream authcode on this instance")) } @@ -254,28 +272,39 @@ func TestAuthorizationEndpoint(t *testing.T) { parsedUpstreamLDAPURL, err := url.Parse(upstreamLDAPURL) require.NoError(t, err) + ldapAuthenticateFunc := func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { + if username == "" || password == "" { + return nil, false, fmt.Errorf("should not have passed empty username or password to the authenticator") + } + if username == happyLDAPUsername && password == happyLDAPPassword { + return &authenticator.Response{ + User: &user.DefaultInfo{ + Name: happyLDAPUsernameFromAuthenticator, + UID: happyLDAPUID, + Groups: happyLDAPGroups, + }, + }, true, nil + } + return nil, false, nil + } + upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: "some-ldap-idp", - URL: parsedUpstreamLDAPURL, - AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { - if username == "" || password == "" { - return nil, false, fmt.Errorf("should not have passed empty username or password to the authenticator") - } - if username == happyLDAPUsername && password == happyLDAPPassword { - return &authenticator.Response{ - User: &user.DefaultInfo{ - Name: happyLDAPUsernameFromAuthenticator, - UID: happyLDAPUID, - Groups: happyLDAPGroups, - }, - }, true, nil - } - return nil, false, nil - }, + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: parsedUpstreamLDAPURL, + AuthenticateFunc: ldapAuthenticateFunc, + } + + upstreamActiveDirectoryIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: activeDirectoryUpstreamName, + ResourceUID: activeDirectoryUpstreamResourceUID, + URL: parsedUpstreamLDAPURL, + AuthenticateFunc: ldapAuthenticateFunc, } erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ - Name: "some-ldap-idp", + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { return nil, false, fmt.Errorf("some ldap upstream auth error") }, @@ -387,10 +416,9 @@ func TestAuthorizationEndpoint(t *testing.T) { return encoded } - expectedRedirectLocationForUpstreamOIDC := func(expectedUpstreamState string, expectedPrompt string) string { + expectedRedirectLocationForUpstreamOIDC := func(expectedUpstreamState string, expectedAdditionalParams map[string]string) string { query := map[string]string{ "response_type": "code", - "access_type": "offline", "scope": "scope1 scope2", "client_id": "some-client-id", "state": expectedUpstreamState, @@ -399,12 +427,35 @@ func TestAuthorizationEndpoint(t *testing.T) { "code_challenge_method": downstreamPKCEChallengeMethod, "redirect_uri": downstreamIssuer + "/callback", } - if expectedPrompt != "" { - query["prompt"] = expectedPrompt + for key, val := range expectedAdditionalParams { + query[key] = val } return urlWithQuery(upstreamAuthURL.String(), query) } + expectedHappyActiveDirectoryUpstreamCustomSession := &psession.CustomSessionData{ + ProviderUID: activeDirectoryUpstreamResourceUID, + ProviderName: activeDirectoryUpstreamName, + ProviderType: psession.ProviderTypeActiveDirectory, + OIDC: nil, + } + + expectedHappyLDAPUpstreamCustomSession := &psession.CustomSessionData{ + ProviderUID: ldapUpstreamResourceUID, + ProviderName: ldapUpstreamName, + ProviderType: psession.ProviderTypeLDAP, + OIDC: nil, + } + + expectedHappyOIDCPasswordGrantCustomSession := &psession.CustomSessionData{ + ProviderUID: oidcPasswordGrantUpstreamResourceUID, + ProviderName: oidcPasswordGrantUpstreamName, + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamRefreshToken: oidcPasswordGrantUpstreamRefreshToken, + }, + } + // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyState @@ -452,11 +503,12 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce string wantUnnecessaryStoredRecords int wantPasswordGrantCall *expectedPasswordGrant + wantDownstreamCustomSessionData *psession.CustomSessionData } tests := []testCase{ { name: "OIDC upstream browser flow happy path using GET without a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -467,7 +519,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantContentType: htmlContentType, wantCSRFValueInCookieHeader: happyCSRF, - wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, @@ -491,6 +543,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "LDAP upstream happy path using GET", @@ -511,10 +564,11 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, }, { name: "ActiveDirectory upstream happy path using GET", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(happyLDAPUsername), @@ -531,10 +585,11 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyActiveDirectoryUpstreamCustomSession, }, { name: "OIDC upstream browser flow happy path using GET with a CSRF cookie", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -545,13 +600,13 @@ func TestAuthorizationEndpoint(t *testing.T) { csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ", wantStatus: http.StatusFound, wantContentType: htmlContentType, - wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), ""), + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, { name: "OIDC upstream browser flow happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -565,7 +620,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: "", wantBodyString: "", wantCSRFValueInCookieHeader: happyCSRF, - wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil), wantUpstreamStateParamInLocationHeader: true, }, { @@ -590,6 +645,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "LDAP upstream happy path using POST", @@ -612,10 +668,11 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, }, { name: "Active Directory upstream happy path using POST", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodPost, path: "/some/path", contentType: "application/x-www-form-urlencoded", @@ -634,10 +691,11 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyActiveDirectoryUpstreamCustomSession, }, { name: "OIDC upstream browser flow happy path with prompt param other than none that gets ignored", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -651,12 +709,31 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, wantBodyStringWithLocationInHref: true, wantCSRFValueInCookieHeader: happyCSRF, - wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), ""), + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), nil), + wantUpstreamStateParamInLocationHeader: true, + }, + { + name: "OIDC upstream browser flow happy path with extra params that get passed through", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().WithAdditionalAuthcodeParams(map[string]string{"prompt": "consent", "abc": "123", "def": "456"}).Build()), + generateCSRF: happyCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"prompt": "login"}), + contentType: "application/x-www-form-urlencoded", + body: encodeQuery(happyGetRequestQueryMap), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantBodyStringWithLocationInHref: true, + wantCSRFValueInCookieHeader: happyCSRF, + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), map[string]string{"prompt": "consent", "abc": "123", "def": "456"}), wantUpstreamStateParamInLocationHeader: true, }, { name: "OIDC upstream browser flow with prompt param none throws an error because we want to independently decide the upstream prompt param", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -673,7 +750,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "OIDC upstream browser flow with error while decoding CSRF cookie just generates a new cookie and succeeds as usual", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -686,13 +763,13 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, // Generated a new CSRF cookie and set it in the response. wantCSRFValueInCookieHeader: happyCSRF, - wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), + wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, { name: "OIDC upstream browser flow happy path when downstream redirect uri matches what is configured for client except for the port number", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -707,7 +784,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{ "redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client - }, "", ""), ""), + }, "", ""), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, @@ -733,6 +810,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "LDAP upstream happy path when downstream redirect uri matches what is configured for client except for the port number", @@ -755,10 +833,11 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, }, { name: "OIDC upstream browser flow happy path when downstream requested scopes include offline_access", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -771,7 +850,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{ "scope": "openid offline_access", - }, "", ""), ""), + }, "", ""), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, @@ -834,7 +913,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream password for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(happyLDAPUsername), @@ -858,7 +937,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "wrong upstream username for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr("wrong-username"), @@ -882,7 +961,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream username on request for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: nil, // do not send header @@ -906,7 +985,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing upstream password on request for Active Directory authentication", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(happyLDAPUsername), @@ -916,6 +995,32 @@ func TestAuthorizationEndpoint(t *testing.T) { wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery), wantBodyString: "", }, + { + name: "return an error when upstream IDP did not return a refresh token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingRefreshTokenErrorQuery), + wantBodyString: "", + }, + { + name: "return an error when upstream IDP did not return a refresh token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingRefreshTokenErrorQuery), + wantBodyString: "", + }, { name: "missing upstream password on request for OIDC password grant authentication", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().Build()), @@ -930,7 +1035,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "using the custom username header on request for OIDC password grant authentication when OIDCIdentityProvider does not allow password grants", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -942,7 +1047,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client when using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -984,7 +1089,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream redirect uri does not match what is configured for client when using active directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{ "redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-pinniped-cli-client", @@ -997,7 +1102,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream client does not exist when using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1031,7 +1136,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream client does not exist when using active directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}), wantStatus: http.StatusUnauthorized, @@ -1040,7 +1145,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1077,7 +1182,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "response type is unsupported when using active directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), wantStatus: http.StatusFound, @@ -1087,7 +1192,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1126,7 +1231,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream scopes do not match what is configured for client using Active Directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid tuna"}), customUsernameHeader: pointer.StringPtr(happyLDAPUsername), @@ -1138,7 +1243,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1175,7 +1280,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing response type in request using Active Directory upstream", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), wantStatus: http.StatusFound, @@ -1185,7 +1290,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing client id in request using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1219,7 +1324,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing PKCE code_challenge in request using OIDC upstream browser flow", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1261,7 +1366,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "invalid value for PKCE code_challenge_method in request using OIDC upstream browser flow", // https://tools.ietf.org/html/rfc7636#section-4.3 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1303,7 +1408,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "when PKCE code_challenge_method in request is `plain` using OIDC upstream browser flow", // https://tools.ietf.org/html/rfc7636#section-4.3 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1345,7 +1450,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "missing PKCE code_challenge_method in request using OIDC upstream browser flow", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1389,7 +1494,7 @@ func TestAuthorizationEndpoint(t *testing.T) { // This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running // through that part of the fosite library when using an OIDC upstream browser flow. name: "prompt param is not allowed to have none and another legal value at the same time using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1435,7 +1540,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "happy path: downstream OIDC validations are skipped when the openid scope was not requested using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1449,7 +1554,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam( map[string]string{"prompt": "none login", "scope": "email"}, "", "", - ), ""), + ), nil), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, @@ -1474,6 +1579,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "happy path: downstream OIDC validations are skipped when the openid scope was not requested using LDAP upstream", @@ -1495,6 +1601,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyLDAPUpstreamCustomSession, }, { name: "OIDC upstream password grant: upstream IDP provides no username or group claim configuration, so we use default username claim and skip groups", @@ -1518,6 +1625,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is missing", @@ -1543,6 +1651,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with true value", @@ -1569,6 +1678,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP configures username claim as anything other than special claim `email` and `email_verified` upstream claim is present with false value", @@ -1596,6 +1706,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with illegal value", @@ -1655,6 +1766,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP's configured groups claim in the ID token has a non-array value", @@ -1679,6 +1791,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream IDP's configured groups claim in the ID token is a slice of interfaces", @@ -1703,6 +1816,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream ID token does not contain requested username claim", @@ -1741,6 +1855,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSession, }, { name: "OIDC upstream password grant: upstream ID token contains username claim with weird format", @@ -1909,7 +2024,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "downstream state does not have enough entropy using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1948,7 +2063,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error while encoding upstream state param using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1962,7 +2077,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error while encoding CSRF cookie value for new cookie using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1976,7 +2091,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error while generating CSRF token using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: sadCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: happyNonceGenerator, @@ -1990,7 +2105,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error while generating nonce using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, generateNonce: sadNonceGenerator, @@ -2004,7 +2119,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "error while generating PKCE using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), generateCSRF: happyCSRFGenerator, generatePKCE: sadPKCEGenerator, generateNonce: happyNonceGenerator, @@ -2027,7 +2142,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: multiple OIDC", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider(), upstreamOIDCIdentityProvider()), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build(), upstreamOIDCIdentityProviderBuilder().Build()), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -2054,7 +2169,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: both OIDC and LDAP", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()).WithLDAP(&upstreamLDAPIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(&upstreamLDAPIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -2063,7 +2178,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "too many upstream providers are configured: OIDC, LDAP and AD", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()).WithLDAP(&upstreamLDAPIdentityProvider).WithActiveDirectory(&upstreamLDAPIdentityProvider), // more than one not allowed + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()).WithLDAP(&upstreamLDAPIdentityProvider).WithActiveDirectory(&upstreamActiveDirectoryIdentityProvider), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusUnprocessableEntity, @@ -2072,7 +2187,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "PUT is a bad method", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), method: http.MethodPut, path: "/some/path", wantStatus: http.StatusMethodNotAllowed, @@ -2081,7 +2196,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "PATCH is a bad method", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), method: http.MethodPatch, path: "/some/path", wantStatus: http.StatusMethodNotAllowed, @@ -2090,7 +2205,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { name: "DELETE is a bad method", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProvider()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), method: http.MethodDelete, path: "/some/path", wantStatus: http.StatusMethodNotAllowed, @@ -2166,6 +2281,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test.wantDownstreamNonce, downstreamClientID, test.wantDownstreamRedirectURI, + test.wantDownstreamCustomSessionData, ) default: require.Empty(t, rsp.Header().Values("Location")) @@ -2239,6 +2355,7 @@ func TestAuthorizationEndpoint(t *testing.T) { WithClientID("some-other-new-client-id"). WithAuthorizationURL(*upstreamAuthURL). 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)}) @@ -2246,7 +2363,8 @@ func TestAuthorizationEndpoint(t *testing.T) { test.wantLocationHeader = urlWithQuery(upstreamAuthURL.String(), map[string]string{ "response_type": "code", - "access_type": "offline", + "prompt": "consent", + "abc": "123", "scope": "some-other-new-scope1 some-other-new-scope2", // updated expectation "client_id": "some-other-new-client-id", // updated expectation "state": expectedUpstreamStateParam( diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index b168e4b9..e99d1678 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -19,6 +19,7 @@ import ( "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/formposthtml" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/psession" ) func NewHandler( @@ -68,12 +69,27 @@ func NewHandler( return httperr.New(http.StatusBadGateway, "error exchanging and validating upstream tokens") } + if token.RefreshToken == nil || token.RefreshToken.Token == "" { + plog.Warning("refresh token not returned by upstream provider during authcode exchange", + "upstreamName", upstreamIDPConfig.GetName(), + "scopes", upstreamIDPConfig.GetScopes(), + "additionalParams", upstreamIDPConfig.GetAdditionalAuthcodeParams()) + return httperr.New(http.StatusUnprocessableEntity, "refresh token not returned by upstream provider during authcode exchange") + } + subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) if err != nil { return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, &psession.CustomSessionData{ + ProviderUID: upstreamIDPConfig.GetResourceUID(), + ProviderName: upstreamIDPConfig.GetName(), + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamRefreshToken: token.RefreshToken.Token, + }, + }) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 9cc5779e..6bfcf340 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -18,6 +18,7 @@ import ( "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/jwks" + "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/oidctestutil" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -25,9 +26,11 @@ import ( ) const ( - happyUpstreamIDPName = "upstream-idp-name" + happyUpstreamIDPName = "upstream-idp-name" + happyUpstreamIDPResourceUID = "upstream-uid" oidcUpstreamIssuer = "https://my-upstream-issuer.com" + oidcUpstreamRefreshToken = "test-refresh-token" oidcUpstreamSubject = "abc123-some guid" // has a space character which should get escaped in URL oidcUpstreamSubjectQueryEscaped = "abc123-some+guid" oidcUpstreamUsername = "test-pinniped-username" @@ -69,7 +72,13 @@ var ( "code_challenge_method": []string{downstreamPKCEChallengeMethod}, "redirect_uri": []string{downstreamRedirectURI}, } - happyDownstreamRequestParams = happyDownstreamRequestParamsQuery.Encode() + happyDownstreamRequestParams = happyDownstreamRequestParamsQuery.Encode() + happyDownstreamCustomSessionData = &psession.CustomSessionData{ + ProviderUID: happyUpstreamIDPResourceUID, + ProviderName: happyUpstreamIDPName, + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{UpstreamRefreshToken: oidcUpstreamRefreshToken}, + } ) func TestCallbackEndpoint(t *testing.T) { @@ -130,6 +139,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce string wantDownstreamPKCEChallenge string wantDownstreamPKCEChallengeMethod string + wantDownstreamCustomSessionData *psession.CustomSessionData wantAuthcodeExchangeCall *expectedAuthcodeExchange }{ @@ -157,6 +167,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -179,6 +190,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -203,6 +215,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -227,6 +240,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -253,6 +267,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -280,6 +295,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -302,6 +318,34 @@ func TestCallbackEndpoint(t *testing.T) { args: happyExchangeAndValidateTokensArgs, }, }, + { + name: "return an error when upstream IDP did not return a refresh token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: refresh token not returned by upstream provider during authcode exchange\n", + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, + { + name: "return an error when upstream IDP returned an empty refresh token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: refresh token not returned by upstream provider during authcode exchange\n", + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, { name: "upstream IDP configures username claim as special claim `email` and `email_verified` upstream claim is present with false value", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( @@ -339,6 +383,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -363,6 +408,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -387,6 +433,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -537,6 +584,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -563,6 +611,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -660,6 +709,7 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamNonce: downstreamNonce, wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamCustomSessionData, wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, @@ -907,6 +957,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamNonce, downstreamClientID, downstreamRedirectURI, + test.wantDownstreamCustomSessionData, ) // Otherwise, expect an empty response body. @@ -933,6 +984,7 @@ func TestCallbackEndpoint(t *testing.T) { test.wantDownstreamNonce, downstreamClientID, downstreamRedirectURI, + test.wantDownstreamCustomSessionData, ) } }) @@ -1036,6 +1088,7 @@ func (b *upstreamStateParamBuilder) WithStateVersion(version string) *upstreamSt func happyUpstream() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder { return oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder(). WithName(happyUpstreamIDPName). + WithResourceUID(happyUpstreamIDPResourceUID). WithClientID("some-client-id"). WithScopes([]string{"scope1", "scope2"}). WithUsernameClaim(oidcUpstreamUsernameClaim). @@ -1046,6 +1099,7 @@ func happyUpstream() *oidctestutil.TestUpstreamOIDCIdentityProviderBuilder { WithIDTokenClaim(oidcUpstreamGroupsClaim, oidcUpstreamGroupMembership). WithIDTokenClaim("other-claim", "should be ignored"). WithAllowPasswordGrant(false). + WithRefreshToken(oidcUpstreamRefreshToken). WithPasswordGrantError(errors.New("the callback endpoint should not use password grants")) } diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index bd29646e..0fee5a78 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -36,7 +36,7 @@ const ( ) // MakeDownstreamSession creates a downstream OIDC session. -func MakeDownstreamSession(subject string, username string, groups []string) *psession.PinnipedSession { +func MakeDownstreamSession(subject string, username string, groups []string, custom *psession.CustomSessionData) *psession.PinnipedSession { now := time.Now().UTC() openIDSession := &psession.PinnipedSession{ Fosite: &openid.DefaultSession{ @@ -46,6 +46,7 @@ func MakeDownstreamSession(subject string, username string, groups []string) *ps AuthTime: now, }, }, + Custom: custom, } if groups == nil { groups = []string{} diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index ef57f6e5..6d0af8ec 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -9,6 +9,7 @@ import ( "sync" "golang.org/x/oauth2" + "k8s.io/apimachinery/pkg/types" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -24,6 +25,9 @@ type UpstreamOIDCIdentityProviderI interface { // 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 @@ -42,6 +46,9 @@ type UpstreamOIDCIdentityProviderI interface { // 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 + // 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) @@ -68,6 +75,9 @@ type UpstreamLDAPIdentityProviderI interface { // 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 } diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 2731b488..e78b9126 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -264,6 +264,7 @@ func TestManager(t *testing.T) { "groups": "test-group1", }, }, + RefreshToken: &oidctypes.RefreshToken{Token: "some-opaque-token"}, }, nil }, }).Build() diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index c8a36153..262f5756 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -1374,7 +1374,7 @@ func simulateAuthEndpointHavingAlreadyRun(t *testing.T, authRequest *http.Reques Subject: "", // not used, note that callback_handler.go does not set this Username: "", // not used, note that callback_handler.go does not set this }, - Custom: &psession.PinnipedSessionData{ + Custom: &psession.CustomSessionData{ OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: "starting-fake-refresh-token", }, diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 334dce06..72ea3bdb 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -10,6 +10,7 @@ import ( "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" + "k8s.io/apimachinery/pkg/types" ) // PinnipedSession is a session container which includes the fosite standard stuff plus custom Pinniped stuff. @@ -18,20 +19,20 @@ type PinnipedSession struct { Fosite *openid.DefaultSession `json:"fosite,omitempty"` // Custom Pinniped extensions to the session data. - Custom *PinnipedSessionData `json:"custom,omitempty"` + Custom *CustomSessionData `json:"custom,omitempty"` } var _ openid.Session = &PinnipedSession{} -// PinnipedSessionData is the custom session data needed by Pinniped. It should be treated as a union type, +// CustomSessionData is the custom session data needed by Pinniped. It should be treated as a union type, // where the value of ProviderType decides which other fields to use. -type PinnipedSessionData struct { +type CustomSessionData struct { // 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. // This implies that when a user deletes an identity provider CRD, then the sessions that were started // using that identity provider will not be able to perform any more downstream refreshes. - ProviderUID string `json:"providerUID"` + ProviderUID types.UID `json:"providerUID"` // 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. @@ -40,12 +41,20 @@ type PinnipedSessionData struct { // 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. - ProviderType string `json:"providerType"` + ProviderType ProviderType `json:"providerType"` // Only used when ProviderType == "oidc". OIDC *OIDCSessionData `json:"oidc,omitempty"` } +type ProviderType string + +const ( + ProviderTypeOIDC ProviderType = "oidc" + ProviderTypeLDAP ProviderType = "ldap" + ProviderTypeActiveDirectory ProviderType = "activedirectory" +) + // OIDCSessionData is the additional data needed by Pinniped when the upstream IDP is an OIDC provider. type OIDCSessionData struct { UpstreamRefreshToken string `json:"upstreamRefreshToken"` @@ -58,7 +67,7 @@ func NewPinnipedSession() *PinnipedSession { Claims: &jwt.IDTokenClaims{}, Headers: &jwt.Headers{}, }, - Custom: &PinnipedSessionData{}, + Custom: &CustomSessionData{}, } } diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 5ba9512a..3f8c3f57 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -20,6 +20,7 @@ import ( "golang.org/x/oauth2" "gopkg.in/square/go-jose.v2" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/client-go/kubernetes/fake" v1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -59,12 +60,17 @@ type PasswordCredentialsGrantAndValidateTokensArgs struct { type TestUpstreamLDAPIdentityProvider struct { Name string + ResourceUID types.UID URL *url.URL AuthenticateFunc func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) } var _ provider.UpstreamLDAPIdentityProviderI = &TestUpstreamLDAPIdentityProvider{} +func (u *TestUpstreamLDAPIdentityProvider) GetResourceUID() types.UID { + return u.ResourceUID +} + func (u *TestUpstreamLDAPIdentityProvider) GetName() string { return u.Name } @@ -78,13 +84,15 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { } type TestUpstreamOIDCIdentityProvider struct { - Name string - ClientID string - AuthorizationURL url.URL - UsernameClaim string - GroupsClaim string - Scopes []string - AllowPasswordGrant bool + Name string + ClientID string + ResourceUID types.UID + AuthorizationURL url.URL + UsernameClaim string + GroupsClaim string + Scopes []string + AdditionalAuthcodeParams map[string]string + AllowPasswordGrant bool ExchangeAuthcodeAndValidateTokensFunc func( ctx context.Context, @@ -105,6 +113,16 @@ type TestUpstreamOIDCIdentityProvider struct { passwordCredentialsGrantAndValidateTokensArgs []*PasswordCredentialsGrantAndValidateTokensArgs } +var _ provider.UpstreamOIDCIdentityProviderI = &TestUpstreamOIDCIdentityProvider{} + +func (u *TestUpstreamOIDCIdentityProvider) GetResourceUID() types.UID { + return u.ResourceUID +} + +func (u *TestUpstreamOIDCIdentityProvider) GetAdditionalAuthcodeParams() map[string]string { + return u.AdditionalAuthcodeParams +} + func (u *TestUpstreamOIDCIdentityProvider) GetName() string { return u.Name } @@ -303,16 +321,19 @@ func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder { } type TestUpstreamOIDCIdentityProviderBuilder struct { - name string - clientID string - scopes []string - idToken map[string]interface{} - usernameClaim string - groupsClaim string - authorizationURL url.URL - allowPasswordGrant bool - authcodeExchangeErr error - passwordGrantErr error + name string + resourceUID types.UID + clientID string + scopes []string + idToken map[string]interface{} + refreshToken *oidctypes.RefreshToken + usernameClaim string + groupsClaim string + authorizationURL url.URL + additionalAuthcodeParams map[string]string + allowPasswordGrant bool + authcodeExchangeErr error + passwordGrantErr error } func (u *TestUpstreamOIDCIdentityProviderBuilder) WithName(value string) *TestUpstreamOIDCIdentityProviderBuilder { @@ -320,6 +341,11 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithName(value string) *TestUp return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithResourceUID(value types.UID) *TestUpstreamOIDCIdentityProviderBuilder { + u.resourceUID = value + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) WithClientID(value string) *TestUpstreamOIDCIdentityProviderBuilder { u.clientID = value return u @@ -373,6 +399,26 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutIDTokenClaim(claim stri return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAdditionalAuthcodeParams(params map[string]string) *TestUpstreamOIDCIdentityProviderBuilder { + u.additionalAuthcodeParams = params + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithRefreshToken(token string) *TestUpstreamOIDCIdentityProviderBuilder { + u.refreshToken = &oidctypes.RefreshToken{Token: token} + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithEmptyRefreshToken() *TestUpstreamOIDCIdentityProviderBuilder { + u.refreshToken = &oidctypes.RefreshToken{Token: ""} + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutRefreshToken() *TestUpstreamOIDCIdentityProviderBuilder { + u.refreshToken = nil + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) WithUpstreamAuthcodeExchangeError(err error) *TestUpstreamOIDCIdentityProviderBuilder { u.authcodeExchangeErr = err return u @@ -385,24 +431,26 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithPasswordGrantError(err err func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdentityProvider { return &TestUpstreamOIDCIdentityProvider{ - Name: u.name, - ClientID: u.clientID, - UsernameClaim: u.usernameClaim, - GroupsClaim: u.groupsClaim, - Scopes: u.scopes, - AllowPasswordGrant: u.allowPasswordGrant, - AuthorizationURL: u.authorizationURL, + Name: u.name, + ClientID: u.clientID, + ResourceUID: u.resourceUID, + UsernameClaim: u.usernameClaim, + GroupsClaim: u.groupsClaim, + Scopes: u.scopes, + AllowPasswordGrant: u.allowPasswordGrant, + AuthorizationURL: u.authorizationURL, + AdditionalAuthcodeParams: u.additionalAuthcodeParams, ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { if u.authcodeExchangeErr != nil { return nil, u.authcodeExchangeErr } - return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}}, nil + return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken}, nil }, PasswordCredentialsGrantAndValidateTokensFunc: func(ctx context.Context, username, password string) (*oidctypes.Token, error) { if u.passwordGrantErr != nil { return nil, u.passwordGrantErr } - return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}}, nil + return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken}, nil }, } } @@ -479,6 +527,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamNonce string, wantDownstreamClientID string, wantDownstreamRedirectURI string, + wantCustomSessionData *psession.CustomSessionData, ) { t.Helper() @@ -513,6 +562,7 @@ func RequireAuthCodeRegexpMatch( wantDownstreamRequestedScopes, wantDownstreamClientID, wantDownstreamRedirectURI, + wantCustomSessionData, ) // One PKCE should have been stored. @@ -563,6 +613,7 @@ func validateAuthcodeStorage( wantDownstreamRequestedScopes []string, wantDownstreamClientID string, wantDownstreamRedirectURI string, + wantCustomSessionData *psession.CustomSessionData, ) (*fosite.Request, *psession.PinnipedSession) { t.Helper() @@ -634,6 +685,9 @@ func validateAuthcodeStorage( require.Empty(t, actualClaims.AuthenticationContextClassReference) require.Empty(t, actualClaims.AuthenticationMethodsReference) + // Check that the custom Pinniped session data matches. + require.Equal(t, wantCustomSessionData, storedSessionFromAuthcode.Custom) + return storedRequestFromAuthcode, storedSessionFromAuthcode } diff --git a/internal/testutil/psession.go b/internal/testutil/psession.go index d6f4bcc8..28e65968 100644 --- a/internal/testutil/psession.go +++ b/internal/testutil/psession.go @@ -23,7 +23,7 @@ func NewFakePinnipedSession() *psession.PinnipedSession { Username: "snorlax", Subject: "panda", }, - Custom: &psession.PinnipedSessionData{ + Custom: &psession.CustomSessionData{ ProviderUID: "fake-provider-uid", ProviderType: "fake-provider-type", ProviderName: "fake-provider-name", diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 1a147b43..1baab58b 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -20,6 +20,7 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/google/uuid" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/utils/trace" @@ -82,6 +83,9 @@ type ProviderConfig struct { // Name is the unique name of this upstream LDAP IDP. Name string + // ResourceUID is the Kubernetes resource UID of this identity provider. + ResourceUID types.UID + // Host is the hostname or "hostname:port" of the LDAP server. When the port is not specified, // the default LDAP port will be used. Host string @@ -265,6 +269,10 @@ func (p *Provider) GetName() string { return p.c.Name } +func (p *Provider) GetResourceUID() types.UID { + return p.c.ResourceUID +} + // Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234?base=user-search-base". // 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. diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go index 3a249c1d..17e70915 100644 --- a/internal/upstreamoidc/upstreamoidc.go +++ b/internal/upstreamoidc/upstreamoidc.go @@ -14,6 +14,7 @@ import ( coreosoidc "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "go.pinniped.dev/internal/httputil/httperr" @@ -31,19 +32,29 @@ func New(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Clie // ProviderConfig holds the active configuration of an upstream OIDC provider. type ProviderConfig struct { - Name string - UsernameClaim string - GroupsClaim string - Config *oauth2.Config - Client *http.Client - AllowPasswordGrant bool - Provider interface { + Name string + ResourceUID types.UID + UsernameClaim string + GroupsClaim string + Config *oauth2.Config + Client *http.Client + AllowPasswordGrant bool + AdditionalAuthcodeParams map[string]string + Provider interface { Verifier(*coreosoidc.Config) *coreosoidc.IDTokenVerifier Claims(v interface{}) error UserInfo(ctx context.Context, tokenSource oauth2.TokenSource) (*coreosoidc.UserInfo, error) } } +func (p *ProviderConfig) GetResourceUID() types.UID { + return p.ResourceUID +} + +func (p *ProviderConfig) GetAdditionalAuthcodeParams() map[string]string { + return p.AdditionalAuthcodeParams +} + func (p *ProviderConfig) GetName() string { return p.Name }