From 8db0203839c8add65d961254a2cc1d4d5de9aebc Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 25 Oct 2021 14:25:43 -0700 Subject: [PATCH 1/8] Add test for upstream ldap idp not found, wrong idp uid, and malformed fosite session storage --- internal/oidc/token/token_handler.go | 13 ++ internal/oidc/token/token_handler_test.go | 175 ++++++++++++++++++++++ 2 files changed, 188 insertions(+) diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index c9703e6e..fcaf4110 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -235,3 +235,16 @@ func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) } return downstreamUsername, nil } + +func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) (string, error) { + extra := session.Fosite.Claims.Extra + if extra == nil { + return "", errorsx.WithStack(errMissingUpstreamSessionInternalError) + } + downstreamUsernameInterface := extra["username"] + if downstreamUsernameInterface == nil { + return "", errorsx.WithStack(errMissingUpstreamSessionInternalError) + } + downstreamUsername := downstreamUsernameInterface.(string) + return downstreamUsername, nil +} diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index f3767bc2..77e6efd1 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -2058,6 +2058,181 @@ func TestRefreshGrant(t *testing.T) { }, }, }, + { + name: "upstream ldap idp not found", + idps: oidctestutil.NewUpstreamIDPListerBuilder(), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyLDAPCustomSessionData, + ), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "Error during upstream refresh. Provider from upstream session data was not found." + } + `), + }, + }, + }, + { + name: "upstream active directory idp not found", + idps: oidctestutil.NewUpstreamIDPListerBuilder(), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyActiveDirectoryCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyActiveDirectoryCustomSessionData, + ), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "Error during upstream refresh. Provider from upstream session data was not found." + } + `), + }, + }, + }, + { + name: "fosite session is empty", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: ldapUpstreamURL, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyLDAPCustomSessionData, + ), + }, + modifyRefreshTokenStorage: func(t *testing.T, oauthStore *oidc.KubeStorage, refreshToken string) { + refreshTokenSignature := getFositeDataSignature(t, refreshToken) + firstRequester, err := oauthStore.GetRefreshTokenSession(context.Background(), refreshTokenSignature, nil) + require.NoError(t, err) + session := firstRequester.GetSession().(*psession.PinnipedSession) + session.Fosite = &openid.DefaultSession{} + err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature) + require.NoError(t, err) + err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester) + require.NoError(t, err) + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusInternalServerError, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "There was an internal server error. Required upstream data not found in session." + } + `), + }, + }, + }, + { + name: "username not found in extra field", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: ldapUpstreamURL, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + //fositeSessionData: &openid.DefaultSession{}, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyLDAPCustomSessionData, + ), + }, + modifyRefreshTokenStorage: func(t *testing.T, oauthStore *oidc.KubeStorage, refreshToken string) { + refreshTokenSignature := getFositeDataSignature(t, refreshToken) + firstRequester, err := oauthStore.GetRefreshTokenSession(context.Background(), refreshTokenSignature, nil) + require.NoError(t, err) + session := firstRequester.GetSession().(*psession.PinnipedSession) + session.Fosite = &openid.DefaultSession{ + Claims: &jwt.IDTokenClaims{ + Extra: map[string]interface{}{}, + }, + } + err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature) + require.NoError(t, err) + err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester) + require.NoError(t, err) + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusInternalServerError, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "There was an internal server error. Required upstream data not found in session." + } + `), + }, + }, + }, + { + name: "when the ldap provider in the session storage is found but has the wrong resource UID during the refresh request", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: "the-wrong-uid", + URL: ldapUpstreamURL, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyLDAPCustomSessionData, + ), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "Error during upstream refresh. Provider 'some-ldap-idp' of type 'ldap' from upstream session data has changed its resource UID since authentication." + } + `), + }, + }, + }, + { + name: "when the active directory provider in the session storage is found but has the wrong resource UID during the refresh request", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: activeDirectoryUpstreamName, + ResourceUID: "the-wrong-uid", + URL: ldapUpstreamURL, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyActiveDirectoryCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyActiveDirectoryCustomSessionData, + ), + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusUnauthorized, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "Error during upstream refresh. Provider 'some-ad-idp' of type 'activedirectory' from upstream session data has changed its resource UID since authentication." + } + `), + }, + }, + }, } for _, test := range tests { test := test From da9b4620b3a379f70cfd6763ab3a4e5fa3defb2b Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 28 Oct 2021 12:00:56 -0700 Subject: [PATCH 2/8] Active Directory checks whether password has changed recently during upstream refresh Signed-off-by: Margo Crawford --- .../active_directory_upstream_watcher.go | 1 + .../active_directory_upstream_watcher_test.go | 24 ++ .../provider/dynamic_upstream_idp_provider.go | 10 +- internal/oidc/token/token_handler.go | 42 ++- internal/oidc/token/token_handler_test.go | 43 ++- .../testutil/oidctestutil/oidctestutil.go | 8 +- internal/upstreamldap/upstreamldap.go | 73 +++- internal/upstreamldap/upstreamldap_test.go | 180 +++++++++- test/integration/supervisor_login_test.go | 325 ++++++++++++++---- test/testlib/env.go | 2 + 10 files changed, 612 insertions(+), 96 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 7bd83f39..0fbd2167 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -317,6 +317,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, }, Dialer: c.ldapDialer, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, } if spec.GroupSearch.Attributes.GroupName == "" { 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 da204216..3baa47b7 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -221,6 +221,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, } // Make a copy with targeted changes. @@ -537,6 +538,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -593,6 +595,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -652,6 +655,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -711,6 +715,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), @@ -769,6 +774,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -898,6 +904,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1022,6 +1029,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1073,6 +1081,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1273,6 +1282,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": upstreamldap.GroupSAMAccountNameWithDomainSuffix}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1325,6 +1335,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1381,6 +1392,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1431,6 +1443,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1627,6 +1640,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1753,6 +1767,16 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { require.Equal(t, reflect.ValueOf(v).Pointer(), reflect.ValueOf(actualGroupAttributeParsingOverrides[k]).Pointer()) } + expectedRefreshAttributeChecks := copyOfExpectedValueForResultingCache.RefreshAttributeChecks + actualRefreshAttributeChecks := actualConfig.RefreshAttributeChecks + copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{} + actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{} + require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks)) + for k, v := range expectedRefreshAttributeChecks { + require.NotNil(t, actualRefreshAttributeChecks[k]) + require.Equal(t, reflect.ValueOf(v).Pointer(), reflect.ValueOf(actualRefreshAttributeChecks[k]).Pointer()) + } + require.Equal(t, copyOfExpectedValueForResultingCache, actualConfig) } diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index a054a4c8..1fd75cf2 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -7,6 +7,7 @@ import ( "context" "net/url" "sync" + "time" "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/types" @@ -93,7 +94,14 @@ type UpstreamLDAPIdentityProviderI interface { authenticators.UserAuthenticator // PerformRefresh performs a refresh against the upstream LDAP identity provider - PerformRefresh(ctx context.Context, userDN, expectedUsername, expectedSubject string) error + PerformRefresh(ctx context.Context, storedRefreshAttributes StoredRefreshAttributes) error +} + +type StoredRefreshAttributes struct { + Username string + Subject string + DN string + AuthTime time.Time } type DynamicUpstreamIDPProvider interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index fcaf4110..d578959b 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -75,11 +75,6 @@ func NewHandler( func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, providerCache oidc.UpstreamIdentityProvidersLister) error { session := accessRequest.GetSession().(*psession.PinnipedSession) - downstreamUsername, err := getDownstreamUsernameFromPinnipedSession(session) - if err != nil { - return err - } - downstreamSubject := session.Fosite.Claims.Subject customSessionData := session.Custom if customSessionData == nil { @@ -95,9 +90,9 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, case psession.ProviderTypeOIDC: return upstreamOIDCRefresh(ctx, customSessionData, providerCache) case psession.ProviderTypeLDAP: - return upstreamLDAPRefresh(ctx, customSessionData, providerCache, downstreamUsername, downstreamSubject) + return upstreamLDAPRefresh(ctx, providerCache, session) case psession.ProviderTypeActiveDirectory: - return upstreamLDAPRefresh(ctx, customSessionData, providerCache, downstreamUsername, downstreamSubject) + return upstreamLDAPRefresh(ctx, providerCache, session) default: return errorsx.WithStack(errMissingUpstreamSessionInternalError) } @@ -169,7 +164,15 @@ func findOIDCProviderByNameAndValidateUID( WithHint("Provider from upstream session data was not found.").WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } -func upstreamLDAPRefresh(ctx context.Context, s *psession.CustomSessionData, providerCache oidc.UpstreamIdentityProvidersLister, username string, subject string) error { +func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentityProvidersLister, session *psession.PinnipedSession) error { + username, err := getDownstreamUsernameFromPinnipedSession(session) + if err != nil { + return err + } + subject := session.Fosite.Claims.Subject + + s := session.Custom + // if you have neither a valid ldap session config nor a valid active directory session config validLDAP := s.ProviderType == psession.ProviderTypeLDAP && s.LDAP != nil && s.LDAP.UserDN != "" validAD := s.ProviderType == psession.ProviderTypeActiveDirectory && s.ActiveDirectory != nil && s.ActiveDirectory.UserDN != "" @@ -182,8 +185,16 @@ func upstreamLDAPRefresh(ctx context.Context, s *psession.CustomSessionData, pro if err != nil { return err } + if session.IDTokenClaims().AuthTime.IsZero() { + return errorsx.WithStack(errMissingUpstreamSessionInternalError) + } // run PerformRefresh - err = p.PerformRefresh(ctx, dn, username, subject) + err = p.PerformRefresh(ctx, provider.StoredRefreshAttributes{ + Username: username, + Subject: subject, + DN: dn, + AuthTime: session.IDTokenClaims().AuthTime, + }) if err != nil { return errorsx.WithStack(errUpstreamRefreshError.WithHint( "Upstream refresh failed.").WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) @@ -235,16 +246,3 @@ func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) } return downstreamUsername, nil } - -func getDownstreamUsernameFromPinnipedSession(session *psession.PinnipedSession) (string, error) { - extra := session.Fosite.Claims.Extra - if extra == nil { - return "", errorsx.WithStack(errMissingUpstreamSessionInternalError) - } - downstreamUsernameInterface := extra["username"] - if downstreamUsernameInterface == nil { - return "", errorsx.WithStack(errMissingUpstreamSessionInternalError) - } - downstreamUsername := downstreamUsernameInterface.(string) - return downstreamUsername, nil -} diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 77e6efd1..fa799dbe 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -2181,6 +2181,45 @@ func TestRefreshGrant(t *testing.T) { }, }, }, + { + name: "auth time is the zero value", // time.Times can never be nil, but it is possible that it would be the zero value which would mean something's wrong + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: ldapUpstreamName, + ResourceUID: ldapUpstreamResourceUID, + URL: ldapUpstreamURL, + }), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + customSessionData: happyLDAPCustomSessionData, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( + happyLDAPCustomSessionData, + ), + }, + modifyRefreshTokenStorage: func(t *testing.T, oauthStore *oidc.KubeStorage, refreshToken string) { + refreshTokenSignature := getFositeDataSignature(t, refreshToken) + firstRequester, err := oauthStore.GetRefreshTokenSession(context.Background(), refreshTokenSignature, nil) + require.NoError(t, err) + session := firstRequester.GetSession().(*psession.PinnipedSession) + fositeSessionClaims := session.Fosite.IDTokenClaims() + fositeSessionClaims.AuthTime = time.Time{} + session.Fosite.Claims = fositeSessionClaims + err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature) + require.NoError(t, err) + err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester) + require.NoError(t, err) + }, + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusInternalServerError, + wantErrorResponseBody: here.Doc(` + { + "error": "error", + "error_description": "There was an internal server error. Required upstream data not found in session." + } + `), + }, + }, + }, { name: "when the ldap provider in the session storage is found but has the wrong resource UID during the refresh request", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{ @@ -2201,7 +2240,7 @@ func TestRefreshGrant(t *testing.T) { wantErrorResponseBody: here.Doc(` { "error": "error", - "error_description": "Error during upstream refresh. Provider 'some-ldap-idp' of type 'ldap' from upstream session data has changed its resource UID since authentication." + "error_description": "Error during upstream refresh. Provider from upstream session data has changed its resource UID since authentication." } `), }, @@ -2227,7 +2266,7 @@ func TestRefreshGrant(t *testing.T) { wantErrorResponseBody: here.Doc(` { "error": "error", - "error_description": "Error during upstream refresh. Provider 'some-ad-idp' of type 'activedirectory' from upstream session data has changed its resource UID since authentication." + "error_description": "Error during upstream refresh. Provider from upstream session data has changed its resource UID since authentication." } `), }, diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 03603bdb..0af6e429 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -111,16 +111,16 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { return u.URL } -func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, userDN, expectedUsername, expectedSubject string) error { +func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) error { if u.performRefreshArgs == nil { u.performRefreshArgs = make([]*PerformRefreshArgs, 0) } u.performRefreshCallCount++ u.performRefreshArgs = append(u.performRefreshArgs, &PerformRefreshArgs{ Ctx: ctx, - DN: userDN, - ExpectedUsername: expectedUsername, - ExpectedSubject: expectedSubject, + DN: storedRefreshAttributes.DN, + ExpectedUsername: storedRefreshAttributes.Username, + ExpectedSubject: storedRefreshAttributes.Subject, }) if u.PerformRefreshErr != nil { return u.PerformRefreshErr diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index b69b37354..9c7c678a 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -15,6 +15,7 @@ import ( "net/url" "regexp" "sort" + "strconv" "strings" "time" @@ -40,6 +41,7 @@ const ( defaultLDAPPort = uint16(389) defaultLDAPSPort = uint16(636) sAMAccountNameAttribute = "sAMAccountName" + pwdLastSetAttribute = "pwdLastSet" ) // Conn abstracts the upstream LDAP communication protocol (mostly for testing). @@ -119,6 +121,9 @@ type ProviderConfig struct { // GroupNameMappingOverrides are the mappings between an attribute name and a way to parse it as a group // name when it comes out of LDAP. GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error) + + // RefreshAttributeChecks are extra checks that attributes in a refresh response are as expected. + RefreshAttributeChecks map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error } // UserSearchConfig contains information about how to search for users in the upstream LDAP IDP. @@ -170,9 +175,11 @@ func (p *Provider) GetConfig() ProviderConfig { return p.c } -func (p *Provider) PerformRefresh(ctx context.Context, userDN, expectedUsername, expectedSubject string) error { +func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) error { t := trace.FromContext(ctx).Nest("slow ldap refresh attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches + userDN := storedRefreshAttributes.DN + searchResult, err := p.performRefresh(ctx, userDN) if err != nil { p.traceRefreshFailure(t, err) @@ -196,9 +203,9 @@ func (p *Provider) PerformRefresh(ctx context.Context, userDN, expectedUsername, if err != nil { return err } - if newUsername != expectedUsername { + if newUsername != storedRefreshAttributes.Username { return fmt.Errorf(`searching for user "%s" returned a different username than the previous value. expected: "%s", actual: "%s"`, - userDN, expectedUsername, newUsername, + userDN, storedRefreshAttributes.Username, newUsername, ) } @@ -207,10 +214,15 @@ func (p *Provider) PerformRefresh(ctx context.Context, userDN, expectedUsername, return err } newSubject := downstreamsession.DownstreamLDAPSubject(newUID, *p.GetURL()) - if newSubject != expectedSubject { - return fmt.Errorf(`searching for user "%s" produced a different subject than the previous value. expected: "%s", actual: "%s"`, userDN, expectedSubject, newSubject) + if newSubject != storedRefreshAttributes.Subject { + return fmt.Errorf(`searching for user "%s" produced a different subject than the previous value. expected: "%s", actual: "%s"`, userDN, storedRefreshAttributes.Subject, newSubject) + } + for attribute, validateFunc := range p.c.RefreshAttributeChecks { + err = validateFunc(userEntry, storedRefreshAttributes) + if err != nil { + return fmt.Errorf(`validation for attribute "%s" failed during upstream refresh: %w`, attribute, err) + } } - // we checked that the user still exists and their information is the same, so just return. return nil } @@ -652,7 +664,7 @@ func (p *Provider) refreshUserSearchRequest(dn string) *ldap.SearchRequest { TimeLimit: 90, TypesOnly: false, Filter: "(objectClass=*)", // we already have the dn, so the filter doesn't matter - Attributes: p.userSearchRequestedAttributes(), + Attributes: p.refreshAttributes(), Controls: nil, // this could be used to enable paging, but we're already limiting the result max size } } @@ -679,6 +691,14 @@ func (p *Provider) groupSearchRequestedAttributes() []string { } } +func (p *Provider) refreshAttributes() []string { + attributes := p.userSearchRequestedAttributes() + for k := range p.c.RefreshAttributeChecks { + attributes = append(attributes, k) + } + return attributes +} + func (p *Provider) userSearchFilter(username string) string { safeUsername := p.escapeUsernameForSearchFilter(username) if len(p.c.UserSearch.Filter) == 0 { @@ -836,3 +856,42 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { } return strings.Join(domainComponents[1:], "."), nil } + +func PwdUnchangedSinceLogin(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + authTime := attributes.AuthTime + pwdLastSetWin32Format := entry.GetAttributeValues(pwdLastSetAttribute) + if len(pwdLastSetWin32Format) != 1 { + return fmt.Errorf("expected to find 1 value for %s attribute, but found %d", pwdLastSetAttribute, len(pwdLastSetWin32Format)) + } + // convert to a time.Time + pwdLastSetParsed, err := win32timestampToTime(pwdLastSetWin32Format[0]) + if err != nil { + return err + } + + // if pwdLastSet > authTime, that means that the password has been changed since the initial login. + // return an error so the user is prompted to log in again. + if pwdLastSetParsed.After(authTime) { + return fmt.Errorf("password has changed since login. login time: %s, password set time: %s", authTime, pwdLastSetParsed) + } + return nil +} + +func win32timestampToTime(win32timestamp string) (*time.Time, error) { + // take a win32 timestamp (represented as the number of 100 ns intervals since + // January 1, 1601) and make a time.Time + + const unixTimeBaseAsWin = 116444736000000000 // The unix base time (January 1, 1970 UTC) as 100 ns since Win32 epoch (1601-01-01) + const hundredNsToSecFactor = 10000000 + + win32Time, err := strconv.ParseUint(win32timestamp, 10, 64) + if err != nil { + return nil, fmt.Errorf("couldn't parse as timestamp") + } + + unixsec := int64(win32Time-unixTimeBaseAsWin) / hundredNsToSecFactor + unixns := int64(win32Time % hundredNsToSecFactor) + + convertedTime := time.Unix(unixsec, unixns).UTC() + return &convertedTime, nil +} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index ca2f3e6b..9e74ce43 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -16,6 +16,8 @@ import ( "testing" "time" + provider2 "go.pinniped.dev/internal/oidc/provider" + "github.com/go-ldap/ldap/v3" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -1217,6 +1219,7 @@ func TestEndUserAuthentication(t *testing.T) { } func TestUpstreamRefresh(t *testing.T) { + pwdLastSetAttribute := "pwdLastSet" expectedUserSearch := &ldap.SearchRequest{ BaseDN: testUserSearchResultDNValue, Scope: ldap.ScopeBaseObject, @@ -1225,7 +1228,7 @@ func TestUpstreamRefresh(t *testing.T) { TimeLimit: 90, TypesOnly: false, Filter: "(objectClass=*)", - Attributes: []string{testUserSearchUsernameAttribute, testUserSearchUIDAttribute}, + Attributes: []string{testUserSearchUsernameAttribute, testUserSearchUIDAttribute, pwdLastSetAttribute}, Controls: nil, // don't need paging because we set the SizeLimit so small } @@ -1242,6 +1245,10 @@ func TestUpstreamRefresh(t *testing.T) { Name: testUserSearchUIDAttribute, ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, }, + { + Name: pwdLastSetAttribute, + Values: []string{"132801740800000000"}, + }, }, }, }, @@ -1259,6 +1266,7 @@ func TestUpstreamRefresh(t *testing.T) { UIDAttribute: testUserSearchUIDAttribute, UsernameAttribute: testUserSearchUsernameAttribute, }, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider2.StoredRefreshAttributes) error{pwdLastSetAttribute: PwdUnchangedSinceLogin}, } tests := []struct { @@ -1512,6 +1520,36 @@ func TestUpstreamRefresh(t *testing.T) { }, wantErr: "found 2 values for attribute \"some-upstream-uid-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", }, + { + name: "search result has a recent pwdLastSet value", + providerConfig: providerConfig, + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + { + Name: testUserSearchUsernameAttribute, + Values: []string{testUserSearchResultUsernameAttributeValue}, + }, + { + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, + }, + { + Name: pwdLastSetAttribute, + Values: []string{"132803468800000000"}, + }, + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: "validation for attribute \"pwdLastSet\" failed during upstream refresh: password has changed since login. login time: 2021-11-01 23:43:19 +0000 UTC, password set time: 2021-11-02 17:14:40 +0000 UTC", + }, } for _, tt := range tests { @@ -1538,7 +1576,13 @@ func TestUpstreamRefresh(t *testing.T) { provider := New(*providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" - err := provider.PerformRefresh(context.Background(), testUserSearchResultDNValue, testUserSearchResultUsernameAttributeValue, subject) + authTime := time.Date(2021, time.November, 1, 23, 43, 19, 0, time.UTC) + err := provider.PerformRefresh(context.Background(), provider2.StoredRefreshAttributes{ + Username: testUserSearchResultUsernameAttributeValue, + Subject: subject, + DN: testUserSearchResultDNValue, + AuthTime: authTime, + }) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) @@ -1916,3 +1960,135 @@ func TestGetDomainFromDistinguishedName(t *testing.T) { }) } } + +func TestPwdUnchangedSinceLogin(t *testing.T) { + authTime := "2021-11-01T23:43:19.826433579Z" // this is the format that fosite automatically stores + authTimeParsed, err := time.Parse(time.RFC3339Nano, authTime) + require.NoError(t, err) + pwdResetTimeAfterAuthTime := "132803468800000000" // Nov 2 + pwdResetTimeBeforeAuthTime := "132801740800000000" // Oct 31 + tests := []struct { + name string + authTime *time.Time + entry *ldap.Entry + wantResult bool + wantErr string + }{ + { + name: "happy path where password has not been reset since login", + authTime: &authTimeParsed, + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "pwdLastSet", + Values: []string{pwdResetTimeBeforeAuthTime}, + }, + }, + }, + }, + { + name: "password has been reset since login", + authTime: &authTimeParsed, + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "pwdLastSet", + Values: []string{pwdResetTimeAfterAuthTime}, + }, + }, + }, + wantErr: "password has changed since login. login time: 2021-11-01 23:43:19.826433579 +0000 UTC, password set time: 2021-11-02 17:14:40 +0000 UTC", + }, + { + name: "ldap timestamp is in the wrong format", + authTime: &authTimeParsed, + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "pwdLastSet", + Values: []string{"invalid"}, + }, + }, + }, + wantErr: "couldn't parse as timestamp", + }, + { + name: "no value for pwdLastSet attribute", + authTime: &authTimeParsed, + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{}, + }, + wantErr: "expected to find 1 value for pwdLastSet attribute, but found 0", + }, + { + name: "too many values for pwdLastSet attribute", + authTime: &authTimeParsed, + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "pwdLastSet", + Values: []string{"val1", "val2"}, + }, + }, + }, + wantErr: "expected to find 1 value for pwdLastSet attribute, but found 2", + }, + } + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := PwdUnchangedSinceLogin(tt.entry, provider2.StoredRefreshAttributes{AuthTime: *tt.authTime}) + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestWin32TimestampToTime(t *testing.T) { + happyPasswordChangeTime := time.Date(2021, time.January, 2, 0, 12, 21, 0, time.UTC).UTC() + tests := []struct { + name string + timestampString string + wantTime *time.Time + wantErr string + }{ + { + name: "happy case with a valid timestamp", + timestampString: "132540199410000000", + wantTime: &happyPasswordChangeTime, + }, + { + name: "handles error with a string thats not a timestamp", + timestampString: "not timestamp", + wantErr: "couldn't parse as timestamp", + }, + { + name: "handles error with too big of a timestamp", + timestampString: "132540199410000000000", + wantErr: "couldn't parse as timestamp", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + actualTime, err := win32timestampToTime(tt.timestampString) + require.Equal(t, tt.wantTime, actualTime) + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 43b45e78..e8d18b5c 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -5,11 +5,16 @@ package integration import ( "context" + "crypto/rand" "crypto/tls" + "crypto/x509" "encoding/base64" + "encoding/hex" "encoding/json" "fmt" "io/ioutil" + "math/big" + "net" "net/http" "net/http/httptest" "net/url" @@ -19,9 +24,11 @@ import ( "time" coreosoidc "github.com/coreos/go-oidc/v3/oidc" + "github.com/go-ldap/ldap/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "golang.org/x/text/encoding/unicode" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,18 +51,19 @@ func TestSupervisorLogin(t *testing.T) { tests := []struct { name string maybeSkip func(t *testing.T) + createTestUser func(t *testing.T) (string, string) + deleteTestUser func(t *testing.T, username string) + requestAuthorization func(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client) createIDP func(t *testing.T) string - requestAuthorization func(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL string, httpClient *http.Client) wantDownstreamIDTokenSubjectToMatch string - wantDownstreamIDTokenUsernameToMatch string + wantDownstreamIDTokenUsernameToMatch func(username string) string wantDownstreamIDTokenGroups []string wantErrorDescription string wantErrorType string - // We don't necessarily have any way to revoke the user's session on the upstream provider, - // so to cause the upstream refresh to fail we can cheat by manipulating the user's session + // Either revoke the user's session on the upstream provider, or manipulate the user's session // data in such a way that it should cause the next upstream refresh attempt to fail. - breakRefreshSessionData func(t *testing.T, sessionData *psession.PinnipedSession, idpName string) + breakRefreshSessionData func(t *testing.T, sessionData *psession.PinnipedSession, idpName, username string) }{ { name: "oidc with default username and groups claim settings", @@ -76,7 +84,7 @@ func TestSupervisorLogin(t *testing.T) { return oidcIDP.Name }, requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlow, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeOIDC, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.OIDC.UpstreamRefreshToken) @@ -85,7 +93,7 @@ func TestSupervisorLogin(t *testing.T) { // the ID token Subject should include the upstream user ID after the upstream issuer name wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", // the ID token Username should include the upstream user ID after the upstream issuer name - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+" }, }, { name: "oidc with custom username and groups claim settings", @@ -113,14 +121,14 @@ func TestSupervisorLogin(t *testing.T) { return oidcIDP.Name }, requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlow, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeOIDC, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.OIDC.UpstreamRefreshToken) customSessionData.OIDC.UpstreamRefreshToken = "invalid-updated-refresh-token" }, wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamOIDC.ExpectedGroups, }, { @@ -144,7 +152,7 @@ func TestSupervisorLogin(t *testing.T) { }, idpv1alpha1.PhaseReady) return oidcIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamOIDC.Username, // username to present to server during login @@ -153,7 +161,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeOIDC, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.OIDC.UpstreamRefreshToken) @@ -162,7 +170,7 @@ func TestSupervisorLogin(t *testing.T) { // the ID token Subject should include the upstream user ID after the upstream issuer name wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", // the ID token Username should include the upstream user ID after the upstream issuer name - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+" }, }, { name: "ldap with email as username and groups names as DNs and using an LDAP provider which supports TLS", @@ -212,7 +220,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login @@ -221,7 +229,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeLDAP, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.LDAP.UserDN) @@ -235,8 +243,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, { name: "ldap with CN as username and group names as CNs and using an LDAP provider which only supports StartTLS", // try another variation of configuration options @@ -286,7 +296,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserCN, // username to present to server during login @@ -295,7 +305,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeLDAP, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.LDAP.UserDN) @@ -309,7 +319,7 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN) + "$", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN) + "$" }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsCNs, }, { @@ -360,7 +370,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login @@ -438,7 +448,7 @@ func TestSupervisorLogin(t *testing.T) { }, time.Minute, 500*time.Millisecond) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login @@ -447,7 +457,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeLDAP, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.LDAP.UserDN) @@ -460,8 +470,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, { name: "ldap login still works after deleting and recreating the bind secret", @@ -543,7 +555,7 @@ func TestSupervisorLogin(t *testing.T) { }, time.Minute, 500*time.Millisecond) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login @@ -552,7 +564,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeLDAP, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.LDAP.UserDN) @@ -565,8 +577,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, { name: "activedirectory with all default options", @@ -604,7 +618,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) return adIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login @@ -613,7 +627,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeActiveDirectory, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.ActiveDirectory.UserDN) @@ -627,8 +641,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, }, { name: "activedirectory with custom options", maybeSkip: func(t *testing.T) { @@ -679,7 +695,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) return adIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamActiveDirectory.TestUserMailAttributeValue, // username to present to server during login @@ -688,7 +704,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeActiveDirectory, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.ActiveDirectory.UserDN) @@ -702,8 +718,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserMailAttributeValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs, }, { name: "active directory login still works after updating bind secret", @@ -759,7 +777,7 @@ func TestSupervisorLogin(t *testing.T) { }, time.Minute, 500*time.Millisecond) return adIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login @@ -768,7 +786,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeActiveDirectory, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.ActiveDirectory.UserDN) @@ -781,8 +799,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, }, { name: "active directory login still works after deleting and recreating bind secret", @@ -853,7 +873,7 @@ func TestSupervisorLogin(t *testing.T) { }, time.Minute, 500*time.Millisecond) return adIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login @@ -862,7 +882,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _ string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { customSessionData := pinnipedSession.Custom require.Equal(t, psession.ProviderTypeActiveDirectory, customSessionData.ProviderType) require.NotEmpty(t, customSessionData.ActiveDirectory.UserDN) @@ -875,8 +895,72 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + }, + { + name: "active directory login fails after the user password is changed", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + if env.SupervisorUpstreamActiveDirectory.Host == "" { + t.Skip("Active Directory hostname not specified") + } + }, + createIDP: func(t *testing.T) string { + t.Helper() + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ad-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: secret.Name, + }, + }, idpv1alpha1.ActiveDirectoryPhaseReady) + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) + return adIDP.Name + }, + createTestUser: func(t *testing.T) (string, string) { + return createFreshADTestUser(t, env) + }, + deleteTestUser: func(t *testing.T, username string) { + deleteTestADUser(t, env, username) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + testUserName, // username to present to server during login + testUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + breakRefreshSessionData: func(t *testing.T, sessionData *psession.PinnipedSession, _, username string) { + changeADTestUserPassword(t, env, username) // this will fail for now + }, + // we can't know the subject ahead of time because we created a new user and don't know their uid, + // so skip wantDownstreamIDTokenSubjectToMatch + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: func(username string) string { + return "^" + regexp.QuoteMeta(username+"@"+env.SupervisorUpstreamActiveDirectory.Domain) + "$" + }, + wantDownstreamIDTokenGroups: []string{}, // none for now. }, { name: "logging in to activedirectory with a deactivated user fails", @@ -914,7 +998,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) return adIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamActiveDirectory.TestDeactivatedUserSAMAccountNameValue, // username to present to server during login @@ -975,7 +1059,7 @@ func TestSupervisorLogin(t *testing.T) { requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) return ldapIDP.Name }, - requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { requestAuthorizationUsingCLIPasswordFlow(t, downstreamAuthorizeURL, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login @@ -984,7 +1068,7 @@ func TestSupervisorLogin(t *testing.T) { false, ) }, - breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName string) { + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName, _ string) { // get the idp, update the config. client := testlib.NewSupervisorClientset(t) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) @@ -1007,8 +1091,10 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", - wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + wantDownstreamIDTokenUsernameToMatch: func(username string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, } for _, test := range tests { @@ -1020,6 +1106,8 @@ func TestSupervisorLogin(t *testing.T) { tt.createIDP, tt.requestAuthorization, tt.breakRefreshSessionData, + tt.createTestUser, + tt.deleteTestUser, tt.wantDownstreamIDTokenSubjectToMatch, tt.wantDownstreamIDTokenUsernameToMatch, tt.wantDownstreamIDTokenGroups, @@ -1150,10 +1238,15 @@ func requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t *tes func testSupervisorLogin( t *testing.T, createIDP func(t *testing.T) string, - requestAuthorization func(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL string, httpClient *http.Client), - breakRefreshSessionData func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName string), - wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch string, wantDownstreamIDTokenGroups []string, - wantErrorDescription string, wantErrorType string, + requestAuthorization func(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL, username, password string, httpClient *http.Client), + breakRefreshSessionData func(t *testing.T, pinnipedSession *psession.PinnipedSession, idpName, username string), + createTestUser func(t *testing.T) (string, string), + deleteTestUser func(t *testing.T, username string), + wantDownstreamIDTokenSubjectToMatch string, + wantDownstreamIDTokenUsernameToMatch func(username string) string, + wantDownstreamIDTokenGroups []string, + wantErrorDescription string, + wantErrorType string, ) { env := testlib.IntegrationEnv(t) @@ -1241,6 +1334,12 @@ func testSupervisorLogin( // Create upstream IDP and wait for it to become ready. idpName := createIDP(t) + username, password := "", "" + if createTestUser != nil { + username, password = createTestUser(t) + defer deleteTestUser(t, username) + } + // Perform OIDC discovery for our downstream. var discovery *coreosoidc.Provider testlib.RequireEventually(t, func(requireEventually *require.Assertions) { @@ -1276,7 +1375,7 @@ func testSupervisorLogin( ) // Perform parameterized auth code acquisition. - requestAuthorization(t, downstreamAuthorizeURL, localCallbackServer.URL, httpClient) + requestAuthorization(t, downstreamAuthorizeURL, localCallbackServer.URL, username, password, httpClient) // Expect that our callback handler was invoked. callback := localCallbackServer.waitForCallback(10 * time.Second) @@ -1294,7 +1393,7 @@ func testSupervisorLogin( expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "username", "groups"} verifyTokenResponse(t, tokenResponse, discovery, downstreamOAuth2Config, nonceParam, - expectedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch, wantDownstreamIDTokenGroups) + expectedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch(username), wantDownstreamIDTokenGroups) // token exchange on the original token doTokenExchange(t, &downstreamOAuth2Config, tokenResponse, httpClient, discovery) @@ -1308,7 +1407,7 @@ func testSupervisorLogin( expectRefreshedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "rat", "username", "groups", "at_hash"} verifyTokenResponse(t, refreshedTokenResponse, discovery, downstreamOAuth2Config, "", - expectRefreshedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch, wantDownstreamIDTokenGroups) + expectRefreshedIDTokenClaims, wantDownstreamIDTokenSubjectToMatch, wantDownstreamIDTokenUsernameToMatch(username), wantDownstreamIDTokenGroups) require.NotEqual(t, tokenResponse.AccessToken, refreshedTokenResponse.AccessToken) require.NotEqual(t, tokenResponse.RefreshToken, refreshedTokenResponse.RefreshToken) @@ -1333,7 +1432,7 @@ func testSupervisorLogin( // Next mutate the part of the session that is used during upstream refresh. pinnipedSession, ok := storedRefreshSession.GetSession().(*psession.PinnipedSession) require.True(t, ok, "should have been able to cast session data to PinnipedSession") - breakRefreshSessionData(t, pinnipedSession, idpName) + breakRefreshSessionData(t, pinnipedSession, idpName, username) // Then save the mutated Secret back to Kubernetes. // There is no update function, so delete and create again at the same name. @@ -1423,7 +1522,7 @@ func verifyTokenResponse( require.NotEmpty(t, tokenResponse.RefreshToken) } -func requestAuthorizationUsingBrowserAuthcodeFlow(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL string, httpClient *http.Client) { +func requestAuthorizationUsingBrowserAuthcodeFlow(t *testing.T, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) { t.Helper() env := testlib.IntegrationEnv(t) @@ -1595,3 +1694,113 @@ func expectSecurityHeaders(t *testing.T, response *http.Response, expectFositeTo assert.Equal(t, "no-cache", h.Get("Pragma")) assert.Equal(t, "0", h.Get("Expires")) } + +// create a fresh test user in AD to use for this test. +func createFreshADTestUser(t *testing.T, env *testlib.TestEnv) (string, string) { + t.Helper() + // dial tls + conn := dialTLS(t, env) + // bind + err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) + require.NoError(t, err) + + testUserName := "user-" + createRandomHexString(t, 7) // sAMAccountNames are limited to 20 characters, so this is as long as we can make it. + // create + userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) + a := ldap.NewAddRequest(userDN, []ldap.Control{}) + a.Attribute("objectClass", []string{"top", "person", "organizationalPerson", "user"}) + a.Attribute("userPrincipalName", []string{fmt.Sprintf("%s@%s", testUserName, env.SupervisorUpstreamActiveDirectory.Domain)}) + a.Attribute("sAMAccountName", []string{testUserName}) + + err = conn.Add(a) + require.NoError(t, err) + + // modify password and enable account + testUserPassword := createRandomASCIIString(t, 20) + enc := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewEncoder() + encodedTestUserPassword, err := enc.String("\"" + testUserPassword + "\"") + require.NoError(t, err) + + m := ldap.NewModifyRequest(userDN, []ldap.Control{}) + m.Replace("unicodePwd", []string{encodedTestUserPassword}) + m.Replace("userAccountControl", []string{"512"}) + err = conn.Modify(m) + require.NoError(t, err) + return testUserName, testUserPassword +} + +// change the user's password to a new one. +func changeADTestUserPassword(t *testing.T, env *testlib.TestEnv, testUserName string) { + conn := dialTLS(t, env) + // bind + err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) + require.NoError(t, err) + + newTestUserPassword := createRandomASCIIString(t, 20) + enc := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewEncoder() + encodedTestUserPassword, err := enc.String("\"" + newTestUserPassword + "\"") + require.NoError(t, err) + + userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) + m := ldap.NewModifyRequest(userDN, []ldap.Control{}) + m.Replace("unicodePwd", []string{encodedTestUserPassword}) + err = conn.Modify(m) + require.NoError(t, err) + // don't bother to return the new password... we won't be using it, just checking that it's changed. +} + +// delete the test user created for this test. +func deleteTestADUser(t *testing.T, env *testlib.TestEnv, testUserName string) { + t.Helper() + + conn := dialTLS(t, env) + // bind + err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) + require.NoError(t, err) + + userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) + d := ldap.NewDelRequest(userDN, []ldap.Control{}) + err = conn.Del(d) + require.NoError(t, err) +} + +func dialTLS(t *testing.T, env *testlib.TestEnv) *ldap.Conn { + t.Helper() + // dial tls + rootCAs := x509.NewCertPool() + success := rootCAs.AppendCertsFromPEM([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)) + require.True(t, success) + tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: rootCAs} + dialer := &tls.Dialer{NetDialer: &net.Dialer{Timeout: time.Minute}, Config: tlsConfig} + c, err := dialer.DialContext(context.Background(), "tcp", env.SupervisorUpstreamActiveDirectory.Host) + require.NoError(t, err) + conn := ldap.NewConn(c, true) + conn.Start() + return conn +} + +func createRandomHexString(t *testing.T, length int) string { + t.Helper() + bytes := make([]byte, length) + _, err := rand.Read(bytes) + require.NoError(t, err) + randomString := hex.EncodeToString(bytes) + return randomString +} + +func createRandomASCIIString(t *testing.T, length int) string { + result := "" + for { + if len(result) >= length { + return result + } + num, err := rand.Int(rand.Reader, big.NewInt(int64(127))) + require.NoError(t, err) + n := num.Int64() + // Make sure that the number/byte/letter is inside + // the range of printable ASCII characters (excluding space and DEL) + if n > 32 && n < 127 { + result += string(rune(n)) + } + } +} diff --git a/test/testlib/env.go b/test/testlib/env.go index f12b0163..7ce6d0be 100644 --- a/test/testlib/env.go +++ b/test/testlib/env.go @@ -84,6 +84,7 @@ type TestOIDCUpstream struct { type TestLDAPUpstream struct { Host string `json:"host"` + Domain string `json:"domain"` StartTLSOnlyHost string `json:"startTLSOnlyHost"` CABundle string `json:"caBundle"` BindUsername string `json:"bindUsername"` @@ -279,6 +280,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) { result.SupervisorUpstreamActiveDirectory = TestLDAPUpstream{ Host: wantEnv("PINNIPED_TEST_AD_HOST", ""), + Domain: wantEnv("PINNIPED_TEST_AD_DOMAIN", ""), CABundle: base64Decoded(t, os.Getenv("PINNIPED_TEST_AD_LDAPS_CA_BUNDLE")), BindUsername: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_USERNAME", ""), BindPassword: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_PASSWORD", ""), From f62e9a2d3339c5c4cf23508b5fa8e4dcf718102d Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Fri, 5 Nov 2021 11:53:07 -0700 Subject: [PATCH 3/8] Active directory checks for deactivated user Signed-off-by: Margo Crawford --- .../active_directory_upstream_watcher.go | 2 +- .../active_directory_upstream_watcher_test.go | 28 +++---- internal/upstreamldap/upstreamldap.go | 14 ++++ internal/upstreamldap/upstreamldap_test.go | 60 ++++++++++++++ test/integration/supervisor_login_test.go | 78 ++++++++++++++++++- 5 files changed, 166 insertions(+), 16 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 0fbd2167..45f1c043 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -317,7 +317,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, }, Dialer: c.ldapDialer, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, } if spec.GroupSearch.Attributes.GroupName == "" { 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 3baa47b7..470f9bcb 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -221,7 +221,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, } // Make a copy with targeted changes. @@ -538,7 +538,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -595,7 +595,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -655,7 +655,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -715,7 +715,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), @@ -774,7 +774,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -904,7 +904,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1029,7 +1029,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1081,7 +1081,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1282,7 +1282,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": upstreamldap.GroupSAMAccountNameWithDomainSuffix}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1335,7 +1335,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1392,7 +1392,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1443,7 +1443,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1640,7 +1640,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 9c7c678a..44422327 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -42,6 +42,7 @@ const ( defaultLDAPSPort = uint16(636) sAMAccountNameAttribute = "sAMAccountName" pwdLastSetAttribute = "pwdLastSet" + userAccountControlAttribute = "userAccountControl" ) // Conn abstracts the upstream LDAP communication protocol (mostly for testing). @@ -895,3 +896,16 @@ func win32timestampToTime(win32timestamp string) (*time.Time, error) { convertedTime := time.Unix(unixsec, unixns).UTC() return &convertedTime, nil } + +func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) + if err != nil { + return err + } + + deactivated := userAccountControl & 2 // bitwise and. + if deactivated != 0 { + return fmt.Errorf("user has been deactivated") + } + return nil +} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 9e74ce43..59c54d14 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -2092,3 +2092,63 @@ func TestWin32TimestampToTime(t *testing.T) { }) } } + +func TestValidUserAccountControl(t *testing.T) { + tests := []struct { + name string + entry *ldap.Entry + wantErr string + }{ + { + name: "happy normal user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"512"}, + }, + }, + }, + }, + { + name: "happy user whose password doesn't expire", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"65536"}, + }, + }, + }, + }, + { + name: "deactivated user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"514"}, + }, + }, + }, + wantErr: "user has been deactivated", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := ValidUserAccountControl(tt.entry, provider2.StoredRefreshAttributes{}) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index e8d18b5c..cc7beb40 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -952,7 +952,69 @@ func TestSupervisorLogin(t *testing.T) { ) }, breakRefreshSessionData: func(t *testing.T, sessionData *psession.PinnipedSession, _, username string) { - changeADTestUserPassword(t, env, username) // this will fail for now + changeADTestUserPassword(t, env, username) + }, + // we can't know the subject ahead of time because we created a new user and don't know their uid, + // so skip wantDownstreamIDTokenSubjectToMatch + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: func(username string) string { + return "^" + regexp.QuoteMeta(username+"@"+env.SupervisorUpstreamActiveDirectory.Domain) + "$" + }, + wantDownstreamIDTokenGroups: []string{}, // none for now. + }, + { + name: "active directory login fails after the user is deactivated", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + if env.SupervisorUpstreamActiveDirectory.Host == "" { + t.Skip("Active Directory hostname not specified") + } + }, + createIDP: func(t *testing.T) string { + t.Helper() + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ad-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: secret.Name, + }, + }, idpv1alpha1.ActiveDirectoryPhaseReady) + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) + return adIDP.Name + }, + createTestUser: func(t *testing.T) (string, string) { + return createFreshADTestUser(t, env) + }, + deleteTestUser: func(t *testing.T, username string) { + deleteTestADUser(t, env, username) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + testUserName, // username to present to server during login + testUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + breakRefreshSessionData: func(t *testing.T, sessionData *psession.PinnipedSession, _, username string) { + deactivateADTestUser(t, env, username) }, // we can't know the subject ahead of time because we created a new user and don't know their uid, // so skip wantDownstreamIDTokenSubjectToMatch @@ -1729,6 +1791,20 @@ func createFreshADTestUser(t *testing.T, env *testlib.TestEnv) (string, string) return testUserName, testUserPassword } +// deactivate the test user's password +func deactivateADTestUser(t *testing.T, env *testlib.TestEnv, testUserName string) { + conn := dialTLS(t, env) + // bind + err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) + require.NoError(t, err) + + userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) + m := ldap.NewModifyRequest(userDN, []ldap.Control{}) + m.Replace("userAccountControl", []string{"514"}) // normal user, account disabled + err = conn.Modify(m) + require.NoError(t, err) +} + // change the user's password to a new one. func changeADTestUserPassword(t *testing.T, env *testlib.TestEnv, testUserName string) { conn := dialTLS(t, env) From ef5a04c7ce92cd496fc78ce63da756cb1aab70ae Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 16 Nov 2021 16:31:32 -0800 Subject: [PATCH 4/8] Check for locked users on ad upstream refresh Signed-off-by: Margo Crawford --- .../active_directory_upstream_watcher.go | 2 +- .../active_directory_upstream_watcher_test.go | 85 ++++++++++++++---- internal/upstreamldap/upstreamldap.go | 28 ++++-- internal/upstreamldap/upstreamldap_test.go | 51 ++++++++++- test/integration/supervisor_login_test.go | 90 ++++++++++++++++++- 5 files changed, 229 insertions(+), 27 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 45f1c043..7a22b816 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -317,7 +317,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, }, Dialer: c.ldapDialer, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{upstreamldap.PwdLastSetAttribute: upstreamldap.PwdUnchangedSinceLogin, upstreamldap.UserAccountControlAttribute: upstreamldap.ValidUserAccountControl, upstreamldap.UserAccountControlComputedAttribute: upstreamldap.ValidComputedUserAccountControl}, } if spec.GroupSearch.Attributes.GroupName == "" { 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 470f9bcb..a4cec598 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -221,7 +221,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, } // Make a copy with targeted changes. @@ -538,7 +542,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -595,7 +603,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -655,7 +667,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -715,7 +731,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantErr: controllerlib.ErrSyntheticRequeue.Error(), @@ -774,7 +794,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -904,7 +928,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1029,8 +1057,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, - }, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }}, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234}, @@ -1081,7 +1112,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1282,7 +1317,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": upstreamldap.GroupSAMAccountNameWithDomainSuffix}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1335,7 +1374,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1392,7 +1435,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1443,7 +1490,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1640,7 +1691,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{"pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, "userAccountControl": upstreamldap.ValidUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "userAccountControl": upstreamldap.ValidUserAccountControl, + "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + }, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 44422327..f20fb8a1 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -41,8 +41,11 @@ const ( defaultLDAPPort = uint16(389) defaultLDAPSPort = uint16(636) sAMAccountNameAttribute = "sAMAccountName" - pwdLastSetAttribute = "pwdLastSet" - userAccountControlAttribute = "userAccountControl" + PwdLastSetAttribute = "pwdLastSet" + UserAccountControlAttribute = "userAccountControl" + UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" + accountDisabledBitmapValue = 2 + accountLockedBitmapValue = 16 ) // Conn abstracts the upstream LDAP communication protocol (mostly for testing). @@ -860,9 +863,9 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { func PwdUnchangedSinceLogin(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { authTime := attributes.AuthTime - pwdLastSetWin32Format := entry.GetAttributeValues(pwdLastSetAttribute) + pwdLastSetWin32Format := entry.GetAttributeValues(PwdLastSetAttribute) if len(pwdLastSetWin32Format) != 1 { - return fmt.Errorf("expected to find 1 value for %s attribute, but found %d", pwdLastSetAttribute, len(pwdLastSetWin32Format)) + return fmt.Errorf("expected to find 1 value for %s attribute, but found %d", PwdLastSetAttribute, len(pwdLastSetWin32Format)) } // convert to a time.Time pwdLastSetParsed, err := win32timestampToTime(pwdLastSetWin32Format[0]) @@ -898,14 +901,27 @@ func win32timestampToTime(win32timestamp string) (*time.Time, error) { } func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { - userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlAttribute)) if err != nil { return err } - deactivated := userAccountControl & 2 // bitwise and. + deactivated := userAccountControl & accountDisabledBitmapValue // bitwise and. if deactivated != 0 { return fmt.Errorf("user has been deactivated") } return nil } + +func ValidComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlComputedAttribute)) + if err != nil { + return err + } + + locked := userAccountControl & accountLockedBitmapValue // bitwise and + if locked != 0 { + return fmt.Errorf("user has been locked") + } + return nil +} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 59c54d14..fc056cc8 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -16,8 +16,6 @@ import ( "testing" "time" - provider2 "go.pinniped.dev/internal/oidc/provider" - "github.com/go-ldap/ldap/v3" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -28,6 +26,7 @@ import ( "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" + provider2 "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -2152,3 +2151,51 @@ func TestValidUserAccountControl(t *testing.T) { }) } } + +func TestValidComputedUserAccountControl(t *testing.T) { + tests := []struct { + name string + entry *ldap.Entry + wantErr string + }{ + { + name: "happy normal user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"0"}, + }, + }, + }, + }, + { + name: "locked user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"16"}, + }, + }, + }, + wantErr: "user has been locked", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := ValidComputedUserAccountControl(tt.entry, provider2.StoredRefreshAttributes{}) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index cc7beb40..66497052 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -45,6 +45,7 @@ import ( "go.pinniped.dev/test/testlib/browsertest" ) +// nolint:gocyclo func TestSupervisorLogin(t *testing.T) { env := testlib.IntegrationEnv(t) @@ -1024,6 +1025,68 @@ func TestSupervisorLogin(t *testing.T) { }, wantDownstreamIDTokenGroups: []string{}, // none for now. }, + { + name: "active directory login fails after the user is locked", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + if env.SupervisorUpstreamActiveDirectory.Host == "" { + t.Skip("Active Directory hostname not specified") + } + }, + createIDP: func(t *testing.T) string { + t.Helper() + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ad-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: secret.Name, + }, + }, idpv1alpha1.ActiveDirectoryPhaseReady) + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg) + return adIDP.Name + }, + createTestUser: func(t *testing.T) (string, string) { + return createFreshADTestUser(t, env) + }, + deleteTestUser: func(t *testing.T, username string) { + deleteTestADUser(t, env, username) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, testUserName, testUserPassword string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + testUserName, // username to present to server during login + testUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + breakRefreshSessionData: func(t *testing.T, sessionData *psession.PinnipedSession, _, username string) { + lockADTestUser(t, env, username) + }, + // we can't know the subject ahead of time because we created a new user and don't know their uid, + // so skip wantDownstreamIDTokenSubjectToMatch + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: func(username string) string { + return "^" + regexp.QuoteMeta(username+"@"+env.SupervisorUpstreamActiveDirectory.Domain) + "$" + }, + wantDownstreamIDTokenGroups: []string{}, + }, { name: "logging in to activedirectory with a deactivated user fails", maybeSkip: func(t *testing.T) { @@ -1153,7 +1216,7 @@ func TestSupervisorLogin(t *testing.T) { "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ) + "$", // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute - wantDownstreamIDTokenUsernameToMatch: func(username string) string { + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, @@ -1788,10 +1851,12 @@ func createFreshADTestUser(t *testing.T, env *testlib.TestEnv) (string, string) m.Replace("userAccountControl", []string{"512"}) err = conn.Modify(m) require.NoError(t, err) + + time.Sleep(20 * time.Second) // intrasite domain controller replication can take up to 15 seconds, so wait to ensure the change has propogated. return testUserName, testUserPassword } -// deactivate the test user's password +// deactivate the test user. func deactivateADTestUser(t *testing.T, env *testlib.TestEnv, testUserName string) { conn := dialTLS(t, env) // bind @@ -1803,6 +1868,24 @@ func deactivateADTestUser(t *testing.T, env *testlib.TestEnv, testUserName strin m.Replace("userAccountControl", []string{"514"}) // normal user, account disabled err = conn.Modify(m) require.NoError(t, err) + + time.Sleep(20 * time.Second) // intrasite domain controller replication can take up to 15 seconds, so wait to ensure the change has propogated. +} + +// lock the test user's account by entering the wrong password a bunch of times. +func lockADTestUser(t *testing.T, env *testlib.TestEnv, testUserName string) { + userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) + conn := dialTLS(t, env) + + for i := 0; i <= 21; i++ { // our password policy allows 20 wrong attempts before locking the account, so do 21. + err := conn.Bind(userDN, "not-the-right-password-"+fmt.Sprint(i)) + require.Error(t, err) // this should be an error + } + + err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) + require.NoError(t, err) + + time.Sleep(20 * time.Second) // intrasite domain controller replication can take up to 15 seconds, so wait to ensure the change has propogated. } // change the user's password to a new one. @@ -1822,13 +1905,14 @@ func changeADTestUserPassword(t *testing.T, env *testlib.TestEnv, testUserName s m.Replace("unicodePwd", []string{encodedTestUserPassword}) err = conn.Modify(m) require.NoError(t, err) + + time.Sleep(20 * time.Second) // intrasite domain controller replication can take up to 15 seconds, so wait to ensure the change has propogated. // don't bother to return the new password... we won't be using it, just checking that it's changed. } // delete the test user created for this test. func deleteTestADUser(t *testing.T, env *testlib.TestEnv, testUserName string) { t.Helper() - conn := dialTLS(t, env) // bind err := conn.Bind(env.SupervisorUpstreamActiveDirectory.BindUsername, env.SupervisorUpstreamActiveDirectory.BindPassword) From ee4f725209b49df7ff800be3fe3a911834620d7f Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 6 Dec 2021 16:24:31 -0800 Subject: [PATCH 5/8] Incorporate PR feedback --- .../active_directory_upstream_watcher.go | 6 +- internal/oidc/token/token_handler_test.go | 1 - internal/upstreamldap/upstreamldap.go | 30 ++++++---- internal/upstreamldap/upstreamldap_test.go | 60 +++++++++++++++---- test/integration/supervisor_login_test.go | 6 +- 5 files changed, 76 insertions(+), 27 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 7a22b816..970c895e 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -317,7 +317,11 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, }, Dialer: c.ldapDialer, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{upstreamldap.PwdLastSetAttribute: upstreamldap.PwdUnchangedSinceLogin, upstreamldap.UserAccountControlAttribute: upstreamldap.ValidUserAccountControl, upstreamldap.UserAccountControlComputedAttribute: upstreamldap.ValidComputedUserAccountControl}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + upstreamldap.PwdLastSetAttribute: upstreamldap.PwdUnchangedSinceLogin, + upstreamldap.UserAccountControlAttribute: upstreamldap.ValidUserAccountControl, + upstreamldap.UserAccountControlComputedAttribute: upstreamldap.ValidComputedUserAccountControl, + }, } if spec.GroupSearch.Attributes.GroupName == "" { diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index fa799dbe..81207357 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -2149,7 +2149,6 @@ func TestRefreshGrant(t *testing.T) { authcodeExchange: authcodeExchangeInputs{ modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, customSessionData: happyLDAPCustomSessionData, - //fositeSessionData: &openid.DefaultSession{}, want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess( happyLDAPCustomSessionData, ), diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index f20fb8a1..9030de81 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -41,11 +41,19 @@ const ( defaultLDAPPort = uint16(389) defaultLDAPSPort = uint16(636) sAMAccountNameAttribute = "sAMAccountName" - PwdLastSetAttribute = "pwdLastSet" - UserAccountControlAttribute = "userAccountControl" - UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" - accountDisabledBitmapValue = 2 - accountLockedBitmapValue = 16 + // PwdLastSetAttribute is the date and time that the password for this account was last changed. + // https://docs.microsoft.com/en-us/windows/win32/adschema/a-pwdlastset + PwdLastSetAttribute = "pwdLastSet" + // UserAccountControlAttribute represents a bitmap of user properties. + // https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties + UserAccountControlAttribute = "userAccountControl" + // UserAccountControlComputedAttribute represents a bitmap of user properties. + // https://docs.microsoft.com/en-us/windows/win32/adschema/a-msds-user-account-control-computed + UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" + // 0x0002 ACCOUNTDISABLE in userAccountControl bitmap. + accountDisabledBitmapValue = 2 + // 0x0010 UF_LOCKOUT in msDS-User-Account-Control-Computed bitmap. + accountLockedBitmapValue = 16 ) // Conn abstracts the upstream LDAP communication protocol (mostly for testing). @@ -881,23 +889,23 @@ func PwdUnchangedSinceLogin(entry *ldap.Entry, attributes provider.StoredRefresh return nil } -func win32timestampToTime(win32timestamp string) (*time.Time, error) { +func win32timestampToTime(win32timestamp string) (time.Time, error) { // take a win32 timestamp (represented as the number of 100 ns intervals since // January 1, 1601) and make a time.Time - const unixTimeBaseAsWin = 116444736000000000 // The unix base time (January 1, 1970 UTC) as 100 ns since Win32 epoch (1601-01-01) - const hundredNsToSecFactor = 10000000 + const unixTimeBaseAsWin = 116_444_736_000_000_000 // The unix base time (January 1, 1970 UTC) as 100 ns since Win32 epoch (1601-01-01) + const hundredNsToSecFactor = 10_000_000 win32Time, err := strconv.ParseUint(win32timestamp, 10, 64) if err != nil { - return nil, fmt.Errorf("couldn't parse as timestamp") + return time.Time{}, fmt.Errorf("couldn't parse as timestamp") } unixsec := int64(win32Time-unixTimeBaseAsWin) / hundredNsToSecFactor - unixns := int64(win32Time % hundredNsToSecFactor) + unixns := int64(win32Time%hundredNsToSecFactor) * 100 convertedTime := time.Unix(unixsec, unixns).UTC() - return &convertedTime, nil + return convertedTime, nil } func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index fc056cc8..b2619baf 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -26,7 +26,7 @@ import ( "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" - provider2 "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -1163,9 +1163,9 @@ func TestEndUserAuthentication(t *testing.T) { return conn, nil }) - provider := New(*tt.providerConfig) + ldapProvider := New(*tt.providerConfig) - authResponse, authenticated, err := provider.AuthenticateUser(context.Background(), tt.username, tt.password) + authResponse, authenticated, err := ldapProvider.AuthenticateUser(context.Background(), tt.username, tt.password) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1197,7 +1197,7 @@ func TestEndUserAuthentication(t *testing.T) { } // Skip tt.bindEndUserMocks since DryRunAuthenticateUser() never binds as the end user. - authResponse, authenticated, err = provider.DryRunAuthenticateUser(context.Background(), tt.username) + authResponse, authenticated, err = ldapProvider.DryRunAuthenticateUser(context.Background(), tt.username) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1265,7 +1265,7 @@ func TestUpstreamRefresh(t *testing.T) { UIDAttribute: testUserSearchUIDAttribute, UsernameAttribute: testUserSearchUsernameAttribute, }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider2.StoredRefreshAttributes) error{pwdLastSetAttribute: PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{pwdLastSetAttribute: PwdUnchangedSinceLogin}, } tests := []struct { @@ -1573,10 +1573,10 @@ func TestUpstreamRefresh(t *testing.T) { return conn, nil }) - provider := New(*providerConfig) + ldapProvider := New(*providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" authTime := time.Date(2021, time.November, 1, 23, 43, 19, 0, time.UTC) - err := provider.PerformRefresh(context.Background(), provider2.StoredRefreshAttributes{ + err := ldapProvider.PerformRefresh(context.Background(), provider.StoredRefreshAttributes{ Username: testUserSearchResultUsernameAttributeValue, Subject: subject, DN: testUserSearchResultDNValue, @@ -2041,7 +2041,7 @@ func TestPwdUnchangedSinceLogin(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := PwdUnchangedSinceLogin(tt.entry, provider2.StoredRefreshAttributes{AuthTime: *tt.authTime}) + err := PwdUnchangedSinceLogin(tt.entry, provider.StoredRefreshAttributes{AuthTime: *tt.authTime}) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) @@ -2057,13 +2057,13 @@ func TestWin32TimestampToTime(t *testing.T) { tests := []struct { name string timestampString string - wantTime *time.Time + wantTime time.Time wantErr string }{ { name: "happy case with a valid timestamp", timestampString: "132540199410000000", - wantTime: &happyPasswordChangeTime, + wantTime: happyPasswordChangeTime, }, { name: "handles error with a string thats not a timestamp", @@ -2075,6 +2075,16 @@ func TestWin32TimestampToTime(t *testing.T) { timestampString: "132540199410000000000", wantErr: "couldn't parse as timestamp", }, + { + name: "timestamp of zero", + timestampString: "0", + wantTime: time.Date(1601, time.January, 1, 0, 0, 0, 0, time.UTC).UTC(), + }, + { + name: "fractional seconds", + timestampString: "132540199410000001", + wantTime: time.Date(2021, time.January, 2, 0, 12, 21, 100, time.UTC).UTC(), + }, } for _, test := range tests { @@ -2135,12 +2145,25 @@ func TestValidUserAccountControl(t *testing.T) { }, wantErr: "user has been deactivated", }, + { + name: "non-integer result", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"not-an-int"}, + }, + }, + }, + wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", + }, } for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := ValidUserAccountControl(tt.entry, provider2.StoredRefreshAttributes{}) + err := ValidUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) @@ -2183,12 +2206,25 @@ func TestValidComputedUserAccountControl(t *testing.T) { }, wantErr: "user has been locked", }, + { + name: "non-integer result", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"not-an-int"}, + }, + }, + }, + wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", + }, } for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := ValidComputedUserAccountControl(tt.entry, provider2.StoredRefreshAttributes{}) + err := ValidComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 66497052..9f54d543 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -23,6 +23,8 @@ import ( "testing" "time" + "go.pinniped.dev/internal/crypto/ptls" + coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/go-ldap/ldap/v3" "github.com/stretchr/testify/assert" @@ -1897,7 +1899,7 @@ func changeADTestUserPassword(t *testing.T, env *testlib.TestEnv, testUserName s newTestUserPassword := createRandomASCIIString(t, 20) enc := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewEncoder() - encodedTestUserPassword, err := enc.String("\"" + newTestUserPassword + "\"") + encodedTestUserPassword, err := enc.String(`"` + newTestUserPassword + `"`) require.NoError(t, err) userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) @@ -1930,7 +1932,7 @@ func dialTLS(t *testing.T, env *testlib.TestEnv) *ldap.Conn { rootCAs := x509.NewCertPool() success := rootCAs.AppendCertsFromPEM([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)) require.True(t, success) - tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: rootCAs} + tlsConfig := ptls.DefaultLDAP(rootCAs) dialer := &tls.Dialer{NetDialer: &net.Dialer{Timeout: time.Minute}, Config: tlsConfig} c, err := dialer.DialContext(context.Background(), "tcp", env.SupervisorUpstreamActiveDirectory.Host) require.NoError(t, err) From 65f346499594c1a3fe63fba065d01f7534a00d62 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 7 Dec 2021 16:57:39 -0800 Subject: [PATCH 6/8] Fix issue with very high integer value parsing, add unit tests also add comment about urgent replication --- internal/upstreamldap/upstreamldap.go | 6 +++--- internal/upstreamldap/upstreamldap_test.go | 10 ++++++++++ test/integration/supervisor_login_test.go | 7 ++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 9030de81..781d9f5e 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -896,13 +896,13 @@ func win32timestampToTime(win32timestamp string) (time.Time, error) { const unixTimeBaseAsWin = 116_444_736_000_000_000 // The unix base time (January 1, 1970 UTC) as 100 ns since Win32 epoch (1601-01-01) const hundredNsToSecFactor = 10_000_000 - win32Time, err := strconv.ParseUint(win32timestamp, 10, 64) + win32Time, err := strconv.ParseInt(win32timestamp, 10, 64) if err != nil { return time.Time{}, fmt.Errorf("couldn't parse as timestamp") } - unixsec := int64(win32Time-unixTimeBaseAsWin) / hundredNsToSecFactor - unixns := int64(win32Time%hundredNsToSecFactor) * 100 + unixsec := (win32Time - unixTimeBaseAsWin) / hundredNsToSecFactor + unixns := (win32Time % hundredNsToSecFactor) * 100 convertedTime := time.Unix(unixsec, unixns).UTC() return convertedTime, nil diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index b2619baf..9b573b9c 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -2085,6 +2085,16 @@ func TestWin32TimestampToTime(t *testing.T) { timestampString: "132540199410000001", wantTime: time.Date(2021, time.January, 2, 0, 12, 21, 100, time.UTC).UTC(), }, + { + name: "max allowable value", + timestampString: "9223372036854775807", // 2^63-1 + wantTime: time.Date(30828, time.September, 14, 2, 48, 5, 477580700, time.UTC).UTC(), + }, + { + name: "just past max allowable value", + timestampString: "9223372036854775808", // 2^63 + wantErr: "couldn't parse as timestamp", + }, } for _, test := range tests { diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 9f54d543..812a32b9 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -1879,7 +1879,12 @@ func lockADTestUser(t *testing.T, env *testlib.TestEnv, testUserName string) { userDN := fmt.Sprintf("CN=%s,OU=test-users,%s", testUserName, env.SupervisorUpstreamActiveDirectory.UserSearchBase) conn := dialTLS(t, env) - for i := 0; i <= 21; i++ { // our password policy allows 20 wrong attempts before locking the account, so do 21. + // our password policy allows 20 wrong attempts before locking the account, so do 21. + // these wrong password attempts could go to different domain controllers, but account + // lockout changes are urgently replicated, meaning that the domain controllers will be + // synced asap rather than in the usual 15 second interval. + // See https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/cc961787(v=technet.10)#urgent-replication-of-account-lockout-changes + for i := 0; i <= 21; i++ { err := conn.Bind(userDN, "not-the-right-password-"+fmt.Sprint(i)) require.Error(t, err) // this should be an error } From acaad05341de0ff0335a72980e364d948d68ed37 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 8 Dec 2021 15:03:57 -0800 Subject: [PATCH 7/8] Make pwdLastSet stuff more generic and not require parsing the timestamp Signed-off-by: Margo Crawford --- .../active_directory_upstream_watcher.go | 2 +- .../active_directory_upstream_watcher_test.go | 28 +-- .../authorizationcode/authorizationcode.go | 39 +++- .../authorizationcode_test.go | 1 + internal/oidc/auth/auth_handler.go | 6 +- internal/oidc/auth/auth_handler_test.go | 7 +- .../provider/dynamic_upstream_idp_provider.go | 9 +- internal/oidc/token/token_handler.go | 16 +- internal/psession/pinniped_session.go | 6 +- internal/upstreamldap/upstreamldap.go | 74 +++---- internal/upstreamldap/upstreamldap_test.go | 196 ++++++++---------- 11 files changed, 198 insertions(+), 186 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 970c895e..bc89115b 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -318,7 +318,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, Dialer: c.ldapDialer, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - upstreamldap.PwdLastSetAttribute: upstreamldap.PwdUnchangedSinceLogin, + upstreamldap.PwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(upstreamldap.PwdLastSetAttribute), upstreamldap.UserAccountControlAttribute: upstreamldap.ValidUserAccountControl, upstreamldap.UserAccountControlComputedAttribute: upstreamldap.ValidComputedUserAccountControl, }, 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 a4cec598..6b98390e 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -222,7 +222,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -543,7 +543,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -604,7 +604,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -668,7 +668,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -732,7 +732,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -795,7 +795,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -929,7 +929,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1058,7 +1058,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }}, @@ -1113,7 +1113,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1318,7 +1318,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": upstreamldap.GroupSAMAccountNameWithDomainSuffix}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1375,7 +1375,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1436,7 +1436,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1491,7 +1491,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, @@ -1692,7 +1692,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - "pwdLastSet": upstreamldap.PwdUnchangedSinceLogin, + "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": upstreamldap.ValidUserAccountControl, "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, }, diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index 50c35788..02d3c761 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -348,20 +348,47 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ "upstreamRefreshToken": "嵽痊w©Ź榨Q|ôɵt毇妬" }, "ldap": { - "userDN": "6鉢緋uƴŤȱʀļÂ?墖\u003cƬb獭潜Ʃ饾" + "userDN": "6鉢緋uƴŤȱʀļÂ?墖\u003cƬb獭潜Ʃ饾", + "extraRefreshAttributes": { + "ĝ": [ + "IȽ齤士bEǎ", + "跞@)¿,ɭS隑ip偶宾儮猷V麹" + ], + "齁š%OpKȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ": [ + "aŚB碠k9帴ʘ赱ŕ瑹xȢ~1Į", + "睋邔\u0026Ű惫蜀Ģ¡圔鎥墀jMʥ", + "+î艔垎0" + ] + } }, "activedirectory": { - "userDN": "|鬌R蜚蠣麹概÷驣7Ʀ澉1æɽ誮rʨ鷞" + "userDN": "ȝƋ鬯犦獢9c5¤.岵", + "extraRefreshAttributes": { + "\u0026錝D肁Ŷɽ蔒PR}Ųʓ": [ + "_º$+溪ŸȢŒų崓ļ" + ], + "P姧骦:駝重Eȫ": [ + "ɂ/", + "Ƀɫ囤", + "鉌X縆跣ŠɞɮƎ賿礣©硇" + ], + "a齙\\蹼偦歛ơ": [ + "y衑拁Ȃ縅", + "Vƅȭǝ*擦28Dž ", + "ã置bņ抰蛖" + ] + } } } }, "requestedAudience": [ - "ŚB碠k9" + "õC嶃", + "ɣƜ/気ū齢q萮左/篣AÚƄŕ~čfV", + "x荃墎]ac[" ], "grantedAudience": [ - "ʘ赱", - "ď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ¡圔", - "墀jMʥ" + "XôĖ给溬d鞕", + "腿tʏƲ%}ſ¯Ɣ 籌Tǘ乚Ȥ2Ķě" ] }, "version": "2" diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index fdb9653e..e1f6a5c9 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -396,6 +396,7 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { // while the fuzzer will panic if AuthorizeRequest changes in a way that cannot be fuzzed, // if it adds a new field that can be fuzzed, this check will fail // thus if AuthorizeRequest changes, we will detect it here (though we could possibly miss an omitempty field) + t.Log(authorizeCodeSessionJSONFromFuzzing) require.JSONEq(t, ExpectedAuthorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromFuzzing) } diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index bbfdba75..d0023362 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -122,12 +122,14 @@ func handleAuthRequestForLDAPUpstream( if idpType == psession.ProviderTypeLDAP { customSessionData.LDAP = &psession.LDAPSessionData{ - UserDN: dn, + UserDN: dn, + ExtraRefreshAttributes: authenticateResponse.User.GetExtra(), } } if idpType == psession.ProviderTypeActiveDirectory { customSessionData.ActiveDirectory = &psession.ActiveDirectorySessionData{ - UserDN: dn, + UserDN: dn, + ExtraRefreshAttributes: authenticateResponse.User.GetExtra(), } } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 564ef380..47c69c9c 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -283,6 +283,7 @@ func TestAuthorizationEndpoint(t *testing.T) { Name: happyLDAPUsernameFromAuthenticator, UID: happyLDAPUID, Groups: happyLDAPGroups, + Extra: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, }, DN: happyLDAPUserDN, }, true, nil @@ -442,7 +443,8 @@ func TestAuthorizationEndpoint(t *testing.T) { OIDC: nil, LDAP: nil, ActiveDirectory: &psession.ActiveDirectorySessionData{ - UserDN: happyLDAPUserDN, + UserDN: happyLDAPUserDN, + ExtraRefreshAttributes: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, }, } @@ -452,7 +454,8 @@ func TestAuthorizationEndpoint(t *testing.T) { ProviderType: psession.ProviderTypeLDAP, OIDC: nil, LDAP: &psession.LDAPSessionData{ - UserDN: happyLDAPUserDN, + UserDN: happyLDAPUserDN, + ExtraRefreshAttributes: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, }, ActiveDirectory: nil, } diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 1fd75cf2..33894c33 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -98,10 +98,11 @@ type UpstreamLDAPIdentityProviderI interface { } type StoredRefreshAttributes struct { - Username string - Subject string - DN string - AuthTime time.Time + Username string + Subject string + DN string + AuthTime time.Time + AdditionalAttributes map[string][]string } type DynamicUpstreamIDPProvider interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index d578959b..d9de7fab 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -180,6 +180,13 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit return errorsx.WithStack(errMissingUpstreamSessionInternalError) } + var additionalAttributes map[string][]string + if s.ProviderType == psession.ProviderTypeLDAP { + additionalAttributes = s.LDAP.ExtraRefreshAttributes + } else { + additionalAttributes = s.ActiveDirectory.ExtraRefreshAttributes + } + // get ldap/ad provider out of cache p, dn, err := findLDAPProviderByNameAndValidateUID(s, providerCache) if err != nil { @@ -190,10 +197,11 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit } // run PerformRefresh err = p.PerformRefresh(ctx, provider.StoredRefreshAttributes{ - Username: username, - Subject: subject, - DN: dn, - AuthTime: session.IDTokenClaims().AuthTime, + Username: username, + Subject: subject, + DN: dn, + AuthTime: session.IDTokenClaims().AuthTime, + AdditionalAttributes: additionalAttributes, }) if err != nil { return errorsx.WithStack(errUpstreamRefreshError.WithHint( diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 8009f91d..3a33e1f6 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -66,12 +66,14 @@ type OIDCSessionData struct { // LDAPSessionData is the additional data needed by Pinniped when the upstream IDP is an LDAP provider. type LDAPSessionData struct { - UserDN string `json:"userDN"` + UserDN string `json:"userDN"` + ExtraRefreshAttributes map[string][]string `json:"extraRefreshAttributes,omitempty"` } // ActiveDirectorySessionData is the additional data needed by Pinniped when the upstream IDP is an Active Directory provider. type ActiveDirectorySessionData struct { - UserDN string `json:"userDN"` + UserDN string `json:"userDN"` + ExtraRefreshAttributes map[string][]string `json:"extraRefreshAttributes,omitempty"` } // NewPinnipedSession returns a new empty session. diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 781d9f5e..ea320168 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -593,6 +593,15 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c } sort.Strings(mappedGroupNames) + mappedRefreshAttributes := make(map[string][]string) + for k := range p.c.RefreshAttributeChecks { + mappedVal, err := p.getSearchResultAttributeValue(k, userEntry, username) + if err != nil { + return nil, err + } + mappedRefreshAttributes[k] = []string{mappedVal} + } + // Caution: Note that any other LDAP commands after this bind will be run as this user instead of as the configured BindUsername! err = bindFunc(conn, userEntry.DN) if err != nil { @@ -615,6 +624,7 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c Name: mappedUsername, UID: mappedUID, Groups: mappedGroupNames, + Extra: mappedRefreshAttributes, }, DN: userEntry.DN, } @@ -676,7 +686,7 @@ func (p *Provider) refreshUserSearchRequest(dn string) *ldap.SearchRequest { TimeLimit: 90, TypesOnly: false, Filter: "(objectClass=*)", // we already have the dn, so the filter doesn't matter - Attributes: p.refreshAttributes(), + Attributes: p.userSearchRequestedAttributes(), Controls: nil, // this could be used to enable paging, but we're already limiting the result max size } } @@ -689,6 +699,9 @@ func (p *Provider) userSearchRequestedAttributes() []string { if p.c.UserSearch.UIDAttribute != distinguishedNameAttributeName { attributes = append(attributes, p.c.UserSearch.UIDAttribute) } + for k := range p.c.RefreshAttributeChecks { + attributes = append(attributes, k) + } return attributes } @@ -703,14 +716,6 @@ func (p *Provider) groupSearchRequestedAttributes() []string { } } -func (p *Provider) refreshAttributes() []string { - attributes := p.userSearchRequestedAttributes() - for k := range p.c.RefreshAttributeChecks { - attributes = append(attributes, k) - } - return attributes -} - func (p *Provider) userSearchFilter(username string) string { safeUsername := p.escapeUsernameForSearchFilter(username) if len(p.c.UserSearch.Filter) == 0 { @@ -869,43 +874,22 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { return strings.Join(domainComponents[1:], "."), nil } -func PwdUnchangedSinceLogin(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { - authTime := attributes.AuthTime - pwdLastSetWin32Format := entry.GetAttributeValues(PwdLastSetAttribute) - if len(pwdLastSetWin32Format) != 1 { - return fmt.Errorf("expected to find 1 value for %s attribute, but found %d", PwdLastSetAttribute, len(pwdLastSetWin32Format)) +func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.StoredRefreshAttributes) error { + return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { + prevAttributeValues := storedAttributes.AdditionalAttributes[attribute] + newValues := entry.GetAttributeValues(attribute) + + if len(newValues) != 1 { + return fmt.Errorf(`expected to find 1 value for "%s" attribute, but found %d`, attribute, len(newValues)) + } + if len(prevAttributeValues) != 1 { + return fmt.Errorf(`expected to find 1 stored value for "%s" attribute, but found %d`, attribute, len(prevAttributeValues)) + } + if prevAttributeValues[0] != newValues[0] { + return fmt.Errorf(`value for attribute "%s" has changed since initial value at login. Previous value: "%s", new value: "%s"`, attribute, prevAttributeValues[0], newValues[0]) + } + return nil } - // convert to a time.Time - pwdLastSetParsed, err := win32timestampToTime(pwdLastSetWin32Format[0]) - if err != nil { - return err - } - - // if pwdLastSet > authTime, that means that the password has been changed since the initial login. - // return an error so the user is prompted to log in again. - if pwdLastSetParsed.After(authTime) { - return fmt.Errorf("password has changed since login. login time: %s, password set time: %s", authTime, pwdLastSetParsed) - } - return nil -} - -func win32timestampToTime(win32timestamp string) (time.Time, error) { - // take a win32 timestamp (represented as the number of 100 ns intervals since - // January 1, 1601) and make a time.Time - - const unixTimeBaseAsWin = 116_444_736_000_000_000 // The unix base time (January 1, 1970 UTC) as 100 ns since Win32 epoch (1601-01-01) - const hundredNsToSecFactor = 10_000_000 - - win32Time, err := strconv.ParseInt(win32timestamp, 10, 64) - if err != nil { - return time.Time{}, fmt.Errorf("couldn't parse as timestamp") - } - - unixsec := (win32Time - unixTimeBaseAsWin) / hundredNsToSecFactor - unixns := (win32Time % hundredNsToSecFactor) * 100 - - convertedTime := time.Unix(unixsec, unixns).UTC() - return convertedTime, nil } func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 9b573b9c..dd26501a 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -160,6 +160,7 @@ func TestEndUserAuthentication(t *testing.T) { Name: testUserSearchResultUsernameAttributeValue, UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{testGroupSearchResultGroupNameAttributeValue1, testGroupSearchResultGroupNameAttributeValue2}, + Extra: map[string][]string{}, } if editFunc != nil { editFunc(u) @@ -507,6 +508,7 @@ func TestEndUserAuthentication(t *testing.T) { Name: testUserSearchResultUsernameAttributeValue, UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{"a", "b", "c"}, + Extra: map[string][]string{}, }, DN: testUserSearchResultDNValue, }, @@ -610,6 +612,66 @@ func TestEndUserAuthentication(t *testing.T) { r.Groups = []string{"Animals@activedirectory.mycompany.example.com", "Mammals@activedirectory.mycompany.example.com"} }), }, + { + name: "requesting additional refresh related attributes", + username: testUpstreamUsername, + password: testUpstreamPassword, + providerConfig: providerConfig(func(p *ProviderConfig) { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + return nil + }, + } + }), + searchMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(func(r *ldap.SearchRequest) { + r.Attributes = append(r.Attributes, "some-attribute-to-check-during-refresh") + })).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute(testUserSearchUsernameAttribute, []string{testUserSearchResultUsernameAttributeValue}), + ldap.NewEntryAttribute(testUserSearchUIDAttribute, []string{testUserSearchResultUIDAttributeValue}), + ldap.NewEntryAttribute("some-attribute-to-check-during-refresh", []string{"some-attribute-value"}), + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().SearchWithPaging(expectedGroupSearch(nil), expectedGroupSearchPageSize). + Return(exampleGroupSearchResult, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) + }, + wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { + r.Extra = map[string][]string{"some-attribute-to-check-during-refresh": {"some-attribute-value"}} + }), + }, + { + name: "requesting additional refresh related attributes, but they aren't returned", + username: testUpstreamUsername, + password: testUpstreamPassword, + providerConfig: providerConfig(func(p *ProviderConfig) { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + return nil + }, + } + }), + searchMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(func(r *ldap.SearchRequest) { + r.Attributes = append(r.Attributes, "some-attribute-to-check-during-refresh") + })).Return(exampleUserSearchResult, nil).Times(1) + conn.EXPECT().SearchWithPaging(expectedGroupSearch(nil), expectedGroupSearchPageSize). + Return(exampleGroupSearchResult, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantError: "found 0 values for attribute \"some-attribute-to-check-during-refresh\" while searching for user \"some-upstream-username\", but expected 1 result", + }, { name: "override group parsing when domain can't be determined from dn", username: testUpstreamUsername, @@ -1265,7 +1327,9 @@ func TestUpstreamRefresh(t *testing.T) { UIDAttribute: testUserSearchUIDAttribute, UsernameAttribute: testUserSearchUsernameAttribute, }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{pwdLastSetAttribute: PwdUnchangedSinceLogin}, + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute), + }, } tests := []struct { @@ -1520,7 +1584,7 @@ func TestUpstreamRefresh(t *testing.T) { wantErr: "found 2 values for attribute \"some-upstream-uid-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", }, { - name: "search result has a recent pwdLastSet value", + name: "search result has a changed pwdLastSet value", providerConfig: providerConfig, setupMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) @@ -1539,7 +1603,7 @@ func TestUpstreamRefresh(t *testing.T) { }, { Name: pwdLastSetAttribute, - Values: []string{"132803468800000000"}, + Values: []string{"132801740800000001"}, }, }, }, @@ -1547,7 +1611,7 @@ func TestUpstreamRefresh(t *testing.T) { }, nil).Times(1) conn.EXPECT().Close().Times(1) }, - wantErr: "validation for attribute \"pwdLastSet\" failed during upstream refresh: password has changed since login. login time: 2021-11-01 23:43:19 +0000 UTC, password set time: 2021-11-02 17:14:40 +0000 UTC", + wantErr: "validation for attribute \"pwdLastSet\" failed during upstream refresh: value for attribute \"pwdLastSet\" has changed since initial value at login. Previous value: \"132801740800000000\", new value: \"132801740800000001\"", }, } @@ -1577,10 +1641,11 @@ func TestUpstreamRefresh(t *testing.T) { subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" authTime := time.Date(2021, time.November, 1, 23, 43, 19, 0, time.UTC) err := ldapProvider.PerformRefresh(context.Background(), provider.StoredRefreshAttributes{ - Username: testUserSearchResultUsernameAttributeValue, - Subject: subject, - DN: testUserSearchResultDNValue, - AuthTime: authTime, + Username: testUserSearchResultUsernameAttributeValue, + Subject: subject, + DN: testUserSearchResultDNValue, + AuthTime: authTime, + AdditionalAttributes: map[string][]string{pwdLastSetAttribute: {"132801740800000000"}}, }) if tt.wantErr != "" { require.Error(t, err) @@ -1960,148 +2025,67 @@ func TestGetDomainFromDistinguishedName(t *testing.T) { } } -func TestPwdUnchangedSinceLogin(t *testing.T) { - authTime := "2021-11-01T23:43:19.826433579Z" // this is the format that fosite automatically stores - authTimeParsed, err := time.Parse(time.RFC3339Nano, authTime) - require.NoError(t, err) - pwdResetTimeAfterAuthTime := "132803468800000000" // Nov 2 - pwdResetTimeBeforeAuthTime := "132801740800000000" // Oct 31 +func TestAttributeUnchangedSinceLogin(t *testing.T) { + initialVal := "some-attribute-value" + changedVal := "some-different-attribute-value" + attributeName := "some-attribute-name" tests := []struct { name string - authTime *time.Time entry *ldap.Entry wantResult bool wantErr string }{ { - name: "happy path where password has not been reset since login", - authTime: &authTimeParsed, + name: "happy path where value has not changed since login", entry: &ldap.Entry{ DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: "pwdLastSet", - Values: []string{pwdResetTimeBeforeAuthTime}, + Name: attributeName, + Values: []string{initialVal}, }, }, }, }, { - name: "password has been reset since login", - authTime: &authTimeParsed, + name: "password has been reset since login", entry: &ldap.Entry{ DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: "pwdLastSet", - Values: []string{pwdResetTimeAfterAuthTime}, + Name: attributeName, + Values: []string{changedVal}, }, }, }, - wantErr: "password has changed since login. login time: 2021-11-01 23:43:19.826433579 +0000 UTC, password set time: 2021-11-02 17:14:40 +0000 UTC", + wantErr: "value for attribute \"some-attribute-name\" has changed since initial value at login. Previous value: \"some-attribute-value\", new value: \"some-different-attribute-value\"", }, { - name: "ldap timestamp is in the wrong format", - authTime: &authTimeParsed, - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "pwdLastSet", - Values: []string{"invalid"}, - }, - }, - }, - wantErr: "couldn't parse as timestamp", - }, - { - name: "no value for pwdLastSet attribute", - authTime: &authTimeParsed, + name: "no value for attribute attribute", entry: &ldap.Entry{ DN: "some-dn", Attributes: []*ldap.EntryAttribute{}, }, - wantErr: "expected to find 1 value for pwdLastSet attribute, but found 0", + wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 0", }, { - name: "too many values for pwdLastSet attribute", - authTime: &authTimeParsed, + name: "too many values for attribute", entry: &ldap.Entry{ DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: "pwdLastSet", + Name: attributeName, Values: []string{"val1", "val2"}, }, }, }, - wantErr: "expected to find 1 value for pwdLastSet attribute, but found 2", + wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 2", }, } for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := PwdUnchangedSinceLogin(tt.entry, provider.StoredRefreshAttributes{AuthTime: *tt.authTime}) - if tt.wantErr != "" { - require.Error(t, err) - require.Equal(t, tt.wantErr, err.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestWin32TimestampToTime(t *testing.T) { - happyPasswordChangeTime := time.Date(2021, time.January, 2, 0, 12, 21, 0, time.UTC).UTC() - tests := []struct { - name string - timestampString string - wantTime time.Time - wantErr string - }{ - { - name: "happy case with a valid timestamp", - timestampString: "132540199410000000", - wantTime: happyPasswordChangeTime, - }, - { - name: "handles error with a string thats not a timestamp", - timestampString: "not timestamp", - wantErr: "couldn't parse as timestamp", - }, - { - name: "handles error with too big of a timestamp", - timestampString: "132540199410000000000", - wantErr: "couldn't parse as timestamp", - }, - { - name: "timestamp of zero", - timestampString: "0", - wantTime: time.Date(1601, time.January, 1, 0, 0, 0, 0, time.UTC).UTC(), - }, - { - name: "fractional seconds", - timestampString: "132540199410000001", - wantTime: time.Date(2021, time.January, 2, 0, 12, 21, 100, time.UTC).UTC(), - }, - { - name: "max allowable value", - timestampString: "9223372036854775807", // 2^63-1 - wantTime: time.Date(30828, time.September, 14, 2, 48, 5, 477580700, time.UTC).UTC(), - }, - { - name: "just past max allowable value", - timestampString: "9223372036854775808", // 2^63 - wantErr: "couldn't parse as timestamp", - }, - } - - for _, test := range tests { - tt := test - t.Run(tt.name, func(t *testing.T) { - actualTime, err := win32timestampToTime(tt.timestampString) - require.Equal(t, tt.wantTime, actualTime) + err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.StoredRefreshAttributes{AdditionalAttributes: map[string][]string{attributeName: {initialVal}}}) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) From 59d999956c75cd05343dfe61555cb8dafbe65634 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 9 Dec 2021 14:02:40 -0800 Subject: [PATCH 8/8] Move ad specific stuff to controller also make extra refresh attributes a separate field rather than part of Extra Signed-off-by: Margo Crawford --- internal/authenticators/authenticators.go | 5 +- .../active_directory_upstream_watcher.go | 110 +++- .../active_directory_upstream_watcher_test.go | 353 +++++++++++-- .../authorizationcode/authorizationcode.go | 34 +- .../authorizationcode_test.go | 1 - internal/oidc/auth/auth_handler.go | 4 +- internal/oidc/auth/auth_handler_test.go | 10 +- .../provider/dynamic_upstream_idp_provider.go | 2 +- internal/oidc/token/token_handler.go | 3 +- internal/psession/pinniped_session.go | 8 +- internal/upstreamldap/upstreamldap.go | 118 +---- internal/upstreamldap/upstreamldap_test.go | 471 ++---------------- test/integration/ldap_client_test.go | 81 ++- test/integration/supervisor_login_test.go | 3 +- 14 files changed, 547 insertions(+), 656 deletions(-) diff --git a/internal/authenticators/authenticators.go b/internal/authenticators/authenticators.go index f7a59e33..60151c18 100644 --- a/internal/authenticators/authenticators.go +++ b/internal/authenticators/authenticators.go @@ -35,6 +35,7 @@ type UserAuthenticator interface { } type Response struct { - User user.Info - DN string + User user.Info + DN string + ExtraRefreshAttributes map[string]string } diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index bc89115b..633fb438 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -7,8 +7,12 @@ package activedirectoryupstreamwatcher import ( "context" "fmt" + "regexp" + "strconv" + "strings" "github.com/go-ldap/ldap/v3" + "github.com/google/uuid" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,6 +56,21 @@ const ( // - has a member that matches the DN of the user we successfully logged in as. // - perform nested group search by default. defaultActiveDirectoryGroupSearchFilter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))" + + sAMAccountNameAttribute = "sAMAccountName" + // PwdLastSetAttribute is the date and time that the password for this account was last changed. + // https://docs.microsoft.com/en-us/windows/win32/adschema/a-pwdlastset + PwdLastSetAttribute = "pwdLastSet" + // UserAccountControlAttribute represents a bitmap of user properties. + // https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties + UserAccountControlAttribute = "userAccountControl" + // UserAccountControlComputedAttribute represents a bitmap of user properties. + // https://docs.microsoft.com/en-us/windows/win32/adschema/a-msds-user-account-control-computed + UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" + // 0x0002 ACCOUNTDISABLE in userAccountControl bitmap. + accountDisabledBitmapValue = 2 + // 0x0010 UF_LOCKOUT in msDS-User-Account-Control-Computed bitmap. + accountLockedBitmapValue = 16 ) type activeDirectoryUpstreamGenericLDAPImpl struct { @@ -316,16 +335,16 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, GroupNameAttribute: adUpstreamImpl.Spec().GroupSearch().GroupNameAttribute(), }, Dialer: c.ldapDialer, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - upstreamldap.PwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(upstreamldap.PwdLastSetAttribute), - upstreamldap.UserAccountControlAttribute: upstreamldap.ValidUserAccountControl, - upstreamldap.UserAccountControlComputedAttribute: upstreamldap.ValidComputedUserAccountControl, + PwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(PwdLastSetAttribute), + UserAccountControlAttribute: ValidUserAccountControl, + UserAccountControlComputedAttribute: ValidComputedUserAccountControl, }, } if spec.GroupSearch.Attributes.GroupName == "" { - config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){defaultActiveDirectoryGroupNameAttributeName: upstreamldap.GroupSAMAccountNameWithDomainSuffix} + config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){defaultActiveDirectoryGroupNameAttributeName: GroupSAMAccountNameWithDomainSuffix} } conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config) @@ -358,3 +377,84 @@ func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, ups log.Error(err, "failed to update status") } } + +func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) { + // validation has already been done so we can just get the attribute... + return func(entry *ldap.Entry) (string, error) { + binaryUUID := entry.GetRawAttributeValue(attributeName) + return microsoftUUIDFromBinary(binaryUUID) + } +} + +func microsoftUUIDFromBinary(binaryUUID []byte) (string, error) { + uuidVal, err := uuid.FromBytes(binaryUUID) // start out with the RFC4122 version + if err != nil { + return "", err + } + // then swap it because AD stores the first 3 fields little-endian rather than the expected + // big-endian. + uuidVal[0], uuidVal[1], uuidVal[2], uuidVal[3] = uuidVal[3], uuidVal[2], uuidVal[1], uuidVal[0] + uuidVal[4], uuidVal[5] = uuidVal[5], uuidVal[4] + uuidVal[6], uuidVal[7] = uuidVal[7], uuidVal[6] + return uuidVal.String(), nil +} + +func GroupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) { + sAMAccountNameAttributeValues := entry.GetAttributeValues(sAMAccountNameAttribute) + + if len(sAMAccountNameAttributeValues) != 1 { + return "", fmt.Errorf(`found %d values for attribute "%s", but expected 1 result`, + len(sAMAccountNameAttributeValues), sAMAccountNameAttribute, + ) + } + + sAMAccountName := sAMAccountNameAttributeValues[0] + if len(sAMAccountName) == 0 { + return "", fmt.Errorf(`found empty value for attribute "%s", but expected value to be non-empty`, + sAMAccountNameAttribute, + ) + } + + distinguishedName := entry.DN + domain, err := getDomainFromDistinguishedName(distinguishedName) + if err != nil { + return "", err + } + return sAMAccountName + "@" + domain, nil +} + +var domainComponentsRegexp = regexp.MustCompile(",DC=|,dc=") + +func getDomainFromDistinguishedName(distinguishedName string) (string, error) { + domainComponents := domainComponentsRegexp.Split(distinguishedName, -1) + if len(domainComponents) == 1 { + return "", fmt.Errorf("did not find domain components in group dn: %s", distinguishedName) + } + return strings.Join(domainComponents[1:], "."), nil +} + +func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlAttribute)) + if err != nil { + return err + } + + deactivated := userAccountControl & accountDisabledBitmapValue // bitwise and. + if deactivated != 0 { + return fmt.Errorf("user has been deactivated") + } + return nil +} + +func ValidComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlComputedAttribute)) + if err != nil { + return err + } + + locked := userAccountControl & accountLockedBitmapValue // bitwise and + if locked != 0 { + return fmt.Errorf("user has been locked") + } + return nil +} 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 6b98390e..5c47a5b8 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -220,11 +220,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, } @@ -541,11 +541,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -602,11 +602,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: "sAMAccountName", }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -666,11 +666,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -730,11 +730,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -793,11 +793,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -927,11 +927,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1056,11 +1056,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }}, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1111,11 +1111,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1315,12 +1315,12 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))", GroupNameAttribute: "sAMAccountName", }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, - GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": upstreamldap.GroupSAMAccountNameWithDomainSuffix}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": GroupSAMAccountNameWithDomainSuffix}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1373,11 +1373,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1434,11 +1434,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1489,11 +1489,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1690,11 +1690,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": upstreamldap.ValidUserAccountControl, - "msDS-User-Account-Control-Computed": upstreamldap.ValidComputedUserAccountControl, + "userAccountControl": ValidUserAccountControl, + "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, }, }, }, @@ -1879,3 +1879,270 @@ func normalizeActiveDirectoryUpstreams(upstreams []v1alpha1.ActiveDirectoryIdent return result } + +func TestGroupSAMAccountNameWithDomainSuffix(t *testing.T) { + tests := []struct { + name string + entry *ldap.Entry + wantResult string + wantErr string + }{ + { + name: "happy path with DN and valid sAMAccountName", + entry: &ldap.Entry{ + DN: "CN=animals,OU=Users,OU=pinniped-ad,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), + }, + }, + wantResult: "Mammals@mycompany.example.com", + }, + { + name: "no domain components in DN", + entry: &ldap.Entry{ + DN: "no-domain-components", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), + }, + }, + wantErr: "did not find domain components in group dn: no-domain-components", + }, + { + name: "multiple values for sAMAccountName attribute", + entry: &ldap.Entry{ + DN: "CN=animals,OU=Users,OU=pinniped-ad,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals", "Eukaryotes"}), + }, + }, + wantErr: "found 2 values for attribute \"sAMAccountName\", but expected 1 result", + }, + { + name: "no values for sAMAccountName attribute", + entry: &ldap.Entry{ + DN: "CN=animals,OU=Users,OU=pinniped-ad,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{}), + }, + }, + wantErr: "found 0 values for attribute \"sAMAccountName\", but expected 1 result", + }, + } + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + suffixedSAMAccountName, err := GroupSAMAccountNameWithDomainSuffix(tt.entry) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantResult, suffixedSAMAccountName) + }) + } +} + +func TestGetMicrosoftFormattedUUID(t *testing.T) { + tests := []struct { + name string + binaryUUID []byte + wantString string + wantErr string + }{ + { + name: "happy path", + binaryUUID: []byte("\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11\x12\x13\x14\x15\x16"), + wantString: "04030201-0605-0807-0910-111213141516", + }, + { + name: "not the right length", + binaryUUID: []byte("2\xf8\xb0\xaa\xb6V\xb1D\x8b(\xee"), + wantErr: "invalid UUID (got 11 bytes)", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + actualUUIDString, err := microsoftUUIDFromBinary(tt.binaryUUID) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantString, actualUUIDString) + }) + } +} + +func TestGetDomainFromDistinguishedName(t *testing.T) { + tests := []struct { + name string + distinguishedName string + wantDomain string + wantErr string + }{ + { + name: "happy path", + distinguishedName: "CN=Mammals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", + wantDomain: "activedirectory.mycompany.example.com", + }, + { + name: "lowercased happy path", + distinguishedName: "cn=Mammals,ou=Users,ou=pinniped-ad,dc=activedirectory,dc=mycompany,dc=example,dc=com", + wantDomain: "activedirectory.mycompany.example.com", + }, + { + name: "no domain components", + distinguishedName: "not-a-dn", + wantErr: "did not find domain components in group dn: not-a-dn", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + actualDomain, err := getDomainFromDistinguishedName(tt.distinguishedName) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantDomain, actualDomain) + }) + } +} + +func TestValidUserAccountControl(t *testing.T) { + tests := []struct { + name string + entry *ldap.Entry + wantErr string + }{ + { + name: "happy normal user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"512"}, + }, + }, + }, + }, + { + name: "happy user whose password doesn't expire", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"65536"}, + }, + }, + }, + }, + { + name: "deactivated user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"514"}, + }, + }, + }, + wantErr: "user has been deactivated", + }, + { + name: "non-integer result", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "userAccountControl", + Values: []string{"not-an-int"}, + }, + }, + }, + wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := ValidUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestValidComputedUserAccountControl(t *testing.T) { + tests := []struct { + name string + entry *ldap.Entry + wantErr string + }{ + { + name: "happy normal user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"0"}, + }, + }, + }, + }, + { + name: "locked user", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"16"}, + }, + }, + }, + wantErr: "user has been locked", + }, + { + name: "non-integer result", + entry: &ldap.Entry{ + DN: "some-dn", + Attributes: []*ldap.EntryAttribute{ + { + Name: "msDS-User-Account-Control-Computed", + Values: []string{"not-an-int"}, + }, + }, + }, + wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + err := ValidComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index 02d3c761..9a152521 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -350,45 +350,23 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ "ldap": { "userDN": "6鉢緋uƴŤȱʀļÂ?墖\u003cƬb獭潜Ʃ饾", "extraRefreshAttributes": { - "ĝ": [ - "IȽ齤士bEǎ", - "跞@)¿,ɭS隑ip偶宾儮猷V麹" - ], - "齁š%OpKȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ": [ - "aŚB碠k9帴ʘ赱ŕ瑹xȢ~1Į", - "睋邔\u0026Ű惫蜀Ģ¡圔鎥墀jMʥ", - "+î艔垎0" - ] + "ď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ¡圔": "墀jMʥ", + "齁š%OpKȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ": "鷞aŚB碠k9帴ʘ赱" } }, "activedirectory": { - "userDN": "ȝƋ鬯犦獢9c5¤.岵", + "userDN": "0D餹sêĝɓ", "extraRefreshAttributes": { - "\u0026錝D肁Ŷɽ蔒PR}Ųʓ": [ - "_º$+溪ŸȢŒų崓ļ" - ], - "P姧骦:駝重Eȫ": [ - "ɂ/", - "Ƀɫ囤", - "鉌X縆跣ŠɞɮƎ賿礣©硇" - ], - "a齙\\蹼偦歛ơ": [ - "y衑拁Ȃ縅", - "Vƅȭǝ*擦28Dž ", - "ã置bņ抰蛖" - ] + "摱ì": "bEǎ儯惝Io" } } } }, "requestedAudience": [ - "õC嶃", - "ɣƜ/気ū齢q萮左/篣AÚƄŕ~čfV", - "x荃墎]ac[" + "Ł" ], "grantedAudience": [ - "XôĖ给溬d鞕", - "腿tʏƲ%}ſ¯Ɣ 籌Tǘ乚Ȥ2Ķě" + "r" ] }, "version": "2" diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index e1f6a5c9..fdb9653e 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -396,7 +396,6 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { // while the fuzzer will panic if AuthorizeRequest changes in a way that cannot be fuzzed, // if it adds a new field that can be fuzzed, this check will fail // thus if AuthorizeRequest changes, we will detect it here (though we could possibly miss an omitempty field) - t.Log(authorizeCodeSessionJSONFromFuzzing) require.JSONEq(t, ExpectedAuthorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromFuzzing) } diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index d0023362..fdf1787e 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -123,13 +123,13 @@ func handleAuthRequestForLDAPUpstream( if idpType == psession.ProviderTypeLDAP { customSessionData.LDAP = &psession.LDAPSessionData{ UserDN: dn, - ExtraRefreshAttributes: authenticateResponse.User.GetExtra(), + ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes, } } if idpType == psession.ProviderTypeActiveDirectory { customSessionData.ActiveDirectory = &psession.ActiveDirectorySessionData{ UserDN: dn, - ExtraRefreshAttributes: authenticateResponse.User.GetExtra(), + ExtraRefreshAttributes: authenticateResponse.ExtraRefreshAttributes, } } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 47c69c9c..0fb0dd70 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -269,6 +269,8 @@ func TestAuthorizationEndpoint(t *testing.T) { happyLDAPUID := "some-ldap-uid" happyLDAPUserDN := "cn=foo,dn=bar" happyLDAPGroups := []string{"group1", "group2", "group3"} + happyLDAPExtraRefreshAttribute := "some-refresh-attribute" + happyLDAPExtraRefreshValue := "some-refresh-attribute-value" parsedUpstreamLDAPURL, err := url.Parse(upstreamLDAPURL) require.NoError(t, err) @@ -283,9 +285,11 @@ func TestAuthorizationEndpoint(t *testing.T) { Name: happyLDAPUsernameFromAuthenticator, UID: happyLDAPUID, Groups: happyLDAPGroups, - Extra: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, }, DN: happyLDAPUserDN, + ExtraRefreshAttributes: map[string]string{ + happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue, + }, }, true, nil } return nil, false, nil @@ -444,7 +448,7 @@ func TestAuthorizationEndpoint(t *testing.T) { LDAP: nil, ActiveDirectory: &psession.ActiveDirectorySessionData{ UserDN: happyLDAPUserDN, - ExtraRefreshAttributes: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, + ExtraRefreshAttributes: map[string]string{happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue}, }, } @@ -455,7 +459,7 @@ func TestAuthorizationEndpoint(t *testing.T) { OIDC: nil, LDAP: &psession.LDAPSessionData{ UserDN: happyLDAPUserDN, - ExtraRefreshAttributes: map[string][]string{"some-refresh-attribute": {"some-refresh-attribute-value"}}, + ExtraRefreshAttributes: map[string]string{happyLDAPExtraRefreshAttribute: happyLDAPExtraRefreshValue}, }, ActiveDirectory: nil, } diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 33894c33..0770e2c3 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -102,7 +102,7 @@ type StoredRefreshAttributes struct { Subject string DN string AuthTime time.Time - AdditionalAttributes map[string][]string + AdditionalAttributes map[string]string } type DynamicUpstreamIDPProvider interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index d9de7fab..afaee0b1 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -180,7 +180,7 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit return errorsx.WithStack(errMissingUpstreamSessionInternalError) } - var additionalAttributes map[string][]string + var additionalAttributes map[string]string if s.ProviderType == psession.ProviderTypeLDAP { additionalAttributes = s.LDAP.ExtraRefreshAttributes } else { @@ -200,7 +200,6 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit Username: username, Subject: subject, DN: dn, - AuthTime: session.IDTokenClaims().AuthTime, AdditionalAttributes: additionalAttributes, }) if err != nil { diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 3a33e1f6..d7fd47df 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -66,14 +66,14 @@ type OIDCSessionData struct { // LDAPSessionData is the additional data needed by Pinniped when the upstream IDP is an LDAP provider. type LDAPSessionData struct { - UserDN string `json:"userDN"` - ExtraRefreshAttributes map[string][]string `json:"extraRefreshAttributes,omitempty"` + UserDN string `json:"userDN"` + ExtraRefreshAttributes map[string]string `json:"extraRefreshAttributes,omitempty"` } // ActiveDirectorySessionData is the additional data needed by Pinniped when the upstream IDP is an Active Directory provider. type ActiveDirectorySessionData struct { - UserDN string `json:"userDN"` - ExtraRefreshAttributes map[string][]string `json:"extraRefreshAttributes,omitempty"` + UserDN string `json:"userDN"` + ExtraRefreshAttributes map[string]string `json:"extraRefreshAttributes,omitempty"` } // NewPinnipedSession returns a new empty session. diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index ea320168..594aac1f 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -13,14 +13,11 @@ import ( "fmt" "net" "net/url" - "regexp" "sort" - "strconv" "strings" "time" "github.com/go-ldap/ldap/v3" - "github.com/google/uuid" "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/utils/trace" @@ -40,20 +37,6 @@ const ( groupSearchPageSize = uint32(250) defaultLDAPPort = uint16(389) defaultLDAPSPort = uint16(636) - sAMAccountNameAttribute = "sAMAccountName" - // PwdLastSetAttribute is the date and time that the password for this account was last changed. - // https://docs.microsoft.com/en-us/windows/win32/adschema/a-pwdlastset - PwdLastSetAttribute = "pwdLastSet" - // UserAccountControlAttribute represents a bitmap of user properties. - // https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties - UserAccountControlAttribute = "userAccountControl" - // UserAccountControlComputedAttribute represents a bitmap of user properties. - // https://docs.microsoft.com/en-us/windows/win32/adschema/a-msds-user-account-control-computed - UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" - // 0x0002 ACCOUNTDISABLE in userAccountControl bitmap. - accountDisabledBitmapValue = 2 - // 0x0010 UF_LOCKOUT in msDS-User-Account-Control-Computed bitmap. - accountLockedBitmapValue = 16 ) // Conn abstracts the upstream LDAP communication protocol (mostly for testing). @@ -593,13 +576,13 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c } sort.Strings(mappedGroupNames) - mappedRefreshAttributes := make(map[string][]string) + mappedRefreshAttributes := make(map[string]string) for k := range p.c.RefreshAttributeChecks { - mappedVal, err := p.getSearchResultAttributeValue(k, userEntry, username) + mappedVal, err := p.getSearchResultAttributeRawValueEncoded(k, userEntry, username) if err != nil { return nil, err } - mappedRefreshAttributes[k] = []string{mappedVal} + mappedRefreshAttributes[k] = mappedVal } // Caution: Note that any other LDAP commands after this bind will be run as this user instead of as the configured BindUsername! @@ -624,9 +607,9 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c Name: mappedUsername, UID: mappedUID, Groups: mappedGroupNames, - Extra: mappedRefreshAttributes, }, - DN: userEntry.DN, + DN: userEntry.DN, + ExtraRefreshAttributes: mappedRefreshAttributes, } return response, nil @@ -819,101 +802,18 @@ func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) { ) } -func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) { - // validation has already been done so we can just get the attribute... - return func(entry *ldap.Entry) (string, error) { - binaryUUID := entry.GetRawAttributeValue(attributeName) - return microsoftUUIDFromBinary(binaryUUID) - } -} - -func microsoftUUIDFromBinary(binaryUUID []byte) (string, error) { - uuidVal, err := uuid.FromBytes(binaryUUID) // start out with the RFC4122 version - if err != nil { - return "", err - } - // then swap it because AD stores the first 3 fields little-endian rather than the expected - // big-endian. - uuidVal[0], uuidVal[1], uuidVal[2], uuidVal[3] = uuidVal[3], uuidVal[2], uuidVal[1], uuidVal[0] - uuidVal[4], uuidVal[5] = uuidVal[5], uuidVal[4] - uuidVal[6], uuidVal[7] = uuidVal[7], uuidVal[6] - return uuidVal.String(), nil -} - -func GroupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) { - sAMAccountNameAttributeValues := entry.GetAttributeValues(sAMAccountNameAttribute) - - if len(sAMAccountNameAttributeValues) != 1 { - return "", fmt.Errorf(`found %d values for attribute "%s", but expected 1 result`, - len(sAMAccountNameAttributeValues), sAMAccountNameAttribute, - ) - } - - sAMAccountName := sAMAccountNameAttributeValues[0] - if len(sAMAccountName) == 0 { - return "", fmt.Errorf(`found empty value for attribute "%s", but expected value to be non-empty`, - sAMAccountNameAttribute, - ) - } - - distinguishedName := entry.DN - domain, err := getDomainFromDistinguishedName(distinguishedName) - if err != nil { - return "", err - } - return sAMAccountName + "@" + domain, nil -} - -var domainComponentsRegexp = regexp.MustCompile(",DC=|,dc=") - -func getDomainFromDistinguishedName(distinguishedName string) (string, error) { - domainComponents := domainComponentsRegexp.Split(distinguishedName, -1) - if len(domainComponents) == 1 { - return "", fmt.Errorf("did not find domain components in group dn: %s", distinguishedName) - } - return strings.Join(domainComponents[1:], "."), nil -} - func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.StoredRefreshAttributes) error { return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { - prevAttributeValues := storedAttributes.AdditionalAttributes[attribute] + prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] newValues := entry.GetAttributeValues(attribute) if len(newValues) != 1 { return fmt.Errorf(`expected to find 1 value for "%s" attribute, but found %d`, attribute, len(newValues)) } - if len(prevAttributeValues) != 1 { - return fmt.Errorf(`expected to find 1 stored value for "%s" attribute, but found %d`, attribute, len(prevAttributeValues)) - } - if prevAttributeValues[0] != newValues[0] { - return fmt.Errorf(`value for attribute "%s" has changed since initial value at login. Previous value: "%s", new value: "%s"`, attribute, prevAttributeValues[0], newValues[0]) + encodedNewValue := base64.RawURLEncoding.EncodeToString(entry.GetRawAttributeValue(attribute)) + if prevAttributeValue != encodedNewValue { + return fmt.Errorf(`value for attribute "%s" has changed since initial value at login`, attribute) } return nil } } - -func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { - userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlAttribute)) - if err != nil { - return err - } - - deactivated := userAccountControl & accountDisabledBitmapValue // bitwise and. - if deactivated != 0 { - return fmt.Errorf("user has been deactivated") - } - return nil -} - -func ValidComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { - userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlComputedAttribute)) - if err != nil { - return err - } - - locked := userAccountControl & accountLockedBitmapValue // bitwise and - if locked != 0 { - return fmt.Errorf("user has been locked") - } - return nil -} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index dd26501a..f7c884fb 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -155,17 +155,17 @@ func TestEndUserAuthentication(t *testing.T) { } // The auth response which matches the exampleUserSearchResult and exampleGroupSearchResult. - expectedAuthResponse := func(editFunc func(r *user.DefaultInfo)) *authenticators.Response { + expectedAuthResponse := func(editFunc func(r *authenticators.Response)) *authenticators.Response { u := &user.DefaultInfo{ Name: testUserSearchResultUsernameAttributeValue, UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{testGroupSearchResultGroupNameAttributeValue1, testGroupSearchResultGroupNameAttributeValue2}, - Extra: map[string][]string{}, } + response := &authenticators.Response{User: u, DN: testUserSearchResultDNValue, ExtraRefreshAttributes: map[string]string{}} if editFunc != nil { - editFunc(u) + editFunc(response) } - return &authenticators.Response{User: u, DN: testUserSearchResultDNValue} + return response } tests := []struct { @@ -252,8 +252,9 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Groups = []string{} + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.Groups = []string{} }), }, { @@ -284,8 +285,9 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Name = testUserSearchResultDNValue + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.Name = testUserSearchResultDNValue }), }, { @@ -316,8 +318,9 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.UID = base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultDNValue)) + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.UID = base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultDNValue)) }), }, { @@ -339,8 +342,9 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Groups = []string{testGroupSearchResultDNValue1, testGroupSearchResultDNValue2} + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.Groups = []string{testGroupSearchResultDNValue1, testGroupSearchResultDNValue2} }), }, { @@ -362,8 +366,9 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Groups = []string{testGroupSearchResultDNValue1, testGroupSearchResultDNValue2} + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + info := r.User.(*user.DefaultInfo) + info.Groups = []string{testGroupSearchResultDNValue1, testGroupSearchResultDNValue2} }), }, { @@ -508,110 +513,11 @@ func TestEndUserAuthentication(t *testing.T) { Name: testUserSearchResultUsernameAttributeValue, UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{"a", "b", "c"}, - Extra: map[string][]string{}, }, - DN: testUserSearchResultDNValue, + DN: testUserSearchResultDNValue, + ExtraRefreshAttributes: map[string]string{}, }, }, - { - name: "override UID parsing to work with microsoft style objectGUIDs", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.UIDAttributeParsingOverrides = map[string]func(entry *ldap.Entry) (string, error){ - "objectGUID": MicrosoftUUIDFromBinary("objectGUID"), - } - p.UserSearch.UIDAttribute = "objectGUID" - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(func(r *ldap.SearchRequest) { - r.Attributes = []string{testUserSearchUsernameAttribute, "objectGUID"} - })).Return(&ldap.SearchResult{ - Entries: []*ldap.Entry{ - { - DN: testUserSearchResultDNValue, - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute(testUserSearchUsernameAttribute, []string{testUserSearchResultUsernameAttributeValue}), - ldap.NewEntryAttribute("objectGUID", []string{"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11\x12\x13\x14\x15\x16"}), - }, - }, - }}, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(nil), expectedGroupSearchPageSize). - Return(exampleGroupSearchResult, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - bindEndUserMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) - }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.UID = "04030201-0605-0807-0910-111213141516" - }), - }, - { - name: "override UID parsing when the attribute name doesn't match what's returned does default parsing", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.UIDAttributeParsingOverrides = map[string]func(entry *ldap.Entry) (string, error){ - "objectGUID": MicrosoftUUIDFromBinary("objectGUID"), - } - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(nil), expectedGroupSearchPageSize). - Return(exampleGroupSearchResult, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - bindEndUserMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) - }, - wantAuthResponse: expectedAuthResponse(nil), - }, - { - name: "override group parsing to create new group names", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.GroupSearch.GroupNameAttribute = "sAMAccountName" - p.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){ - "sAMAccountName": GroupSAMAccountNameWithDomainSuffix, - } - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { - r.Attributes = []string{"sAMAccountName"} - }), expectedGroupSearchPageSize). - Return(&ldap.SearchResult{ - Entries: []*ldap.Entry{ - { - DN: "CN=Mammals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), - }, - }, - { - DN: "CN=Animals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute("sAMAccountName", []string{"Animals"}), - }, - }, - }, - Referrals: []string{}, // note that we are not following referrals at this time - Controls: []ldap.Control{}, - }, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - bindEndUserMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) - }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Groups = []string{"Animals@activedirectory.mycompany.example.com", "Mammals@activedirectory.mycompany.example.com"} - }), - }, { name: "requesting additional refresh related attributes", username: testUpstreamUsername, @@ -646,8 +552,8 @@ func TestEndUserAuthentication(t *testing.T) { bindEndUserMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, - wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.Extra = map[string][]string{"some-attribute-to-check-during-refresh": {"some-attribute-value"}} + wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { + r.ExtraRefreshAttributes = map[string]string{"some-attribute-to-check-during-refresh": "c29tZS1hdHRyaWJ1dGUtdmFsdWU"} }), }, { @@ -672,105 +578,6 @@ func TestEndUserAuthentication(t *testing.T) { }, wantError: "found 0 values for attribute \"some-attribute-to-check-during-refresh\" while searching for user \"some-upstream-username\", but expected 1 result", }, - { - name: "override group parsing when domain can't be determined from dn", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.GroupSearch.GroupNameAttribute = "sAMAccountName" - p.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){ - "sAMAccountName": GroupSAMAccountNameWithDomainSuffix, - } - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { - r.Attributes = []string{"sAMAccountName"} - }), expectedGroupSearchPageSize). - Return(&ldap.SearchResult{ - Entries: []*ldap.Entry{ - { - DN: "no-domain-components", - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), - }, - }, - { - DN: "CN=Animals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute("sAMAccountName", []string{"Animals"}), - }, - }, - }, - Referrals: []string{}, // note that we are not following referrals at this time - Controls: []ldap.Control{}, - }, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - wantError: "error finding groups for user some-upstream-user-dn: did not find domain components in group dn: no-domain-components", - }, - { - name: "override group parsing when entry has multiple values for attribute", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.GroupSearch.GroupNameAttribute = "sAMAccountName" - p.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){ - "sAMAccountName": GroupSAMAccountNameWithDomainSuffix, - } - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { - r.Attributes = []string{"sAMAccountName"} - }), expectedGroupSearchPageSize). - Return(&ldap.SearchResult{ - Entries: []*ldap.Entry{ - { - DN: "no-domain-components", - Attributes: []*ldap.EntryAttribute{ - ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals", "Eukaryotes"}), - }, - }, - }, - Referrals: []string{}, // note that we are not following referrals at this time - Controls: []ldap.Control{}, - }, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - wantError: "error finding groups for user some-upstream-user-dn: found 2 values for attribute \"sAMAccountName\", but expected 1 result", - }, { - name: "override group parsing when entry has no values for attribute", - username: testUpstreamUsername, - password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { - p.GroupSearch.GroupNameAttribute = "sAMAccountName" - p.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){ - "sAMAccountName": GroupSAMAccountNameWithDomainSuffix, - } - }), - searchMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) - conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { - r.Attributes = []string{"sAMAccountName"} - }), expectedGroupSearchPageSize). - Return(&ldap.SearchResult{ - Entries: []*ldap.Entry{ - { - DN: "no-domain-components", - Attributes: []*ldap.EntryAttribute{}, - }, - }, - Referrals: []string{}, // note that we are not following referrals at this time - Controls: []ldap.Control{}, - }, nil).Times(1) - conn.EXPECT().Close().Times(1) - }, - wantError: "error finding groups for user some-upstream-user-dn: found 0 values for attribute \"sAMAccountName\", but expected 1 result", - }, { name: "when dial fails", username: testUpstreamUsername, @@ -1307,8 +1114,9 @@ func TestUpstreamRefresh(t *testing.T) { ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, }, { - Name: pwdLastSetAttribute, - Values: []string{"132801740800000000"}, + Name: pwdLastSetAttribute, + Values: []string{"132801740800000000"}, + ByteValues: [][]byte{[]byte("132801740800000000")}, }, }, }, @@ -1611,7 +1419,7 @@ func TestUpstreamRefresh(t *testing.T) { }, nil).Times(1) conn.EXPECT().Close().Times(1) }, - wantErr: "validation for attribute \"pwdLastSet\" failed during upstream refresh: value for attribute \"pwdLastSet\" has changed since initial value at login. Previous value: \"132801740800000000\", new value: \"132801740800000001\"", + wantErr: "validation for attribute \"pwdLastSet\" failed during upstream refresh: value for attribute \"pwdLastSet\" has changed since initial value at login", }, } @@ -1637,15 +1445,14 @@ func TestUpstreamRefresh(t *testing.T) { return conn, nil }) + initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000")) ldapProvider := New(*providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" - authTime := time.Date(2021, time.November, 1, 23, 43, 19, 0, time.UTC) err := ldapProvider.PerformRefresh(context.Background(), provider.StoredRefreshAttributes{ Username: testUserSearchResultUsernameAttributeValue, Subject: subject, DN: testUserSearchResultDNValue, - AuthTime: authTime, - AdditionalAttributes: map[string][]string{pwdLastSetAttribute: {"132801740800000000"}}, + AdditionalAttributes: map[string]string{pwdLastSetAttribute: initialPwdLastSetEncoded}, }) if tt.wantErr != "" { require.Error(t, err) @@ -1954,77 +1761,6 @@ func TestRealTLSDialing(t *testing.T) { } } -func TestGetMicrosoftFormattedUUID(t *testing.T) { - tests := []struct { - name string - binaryUUID []byte - wantString string - wantErr string - }{ - { - name: "happy path", - binaryUUID: []byte("\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11\x12\x13\x14\x15\x16"), - wantString: "04030201-0605-0807-0910-111213141516", - }, - { - name: "not the right length", - binaryUUID: []byte("2\xf8\xb0\xaa\xb6V\xb1D\x8b(\xee"), - wantErr: "invalid UUID (got 11 bytes)", - }, - } - - for _, test := range tests { - tt := test - t.Run(tt.name, func(t *testing.T) { - actualUUIDString, err := microsoftUUIDFromBinary(tt.binaryUUID) - if tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.Equal(t, tt.wantString, actualUUIDString) - }) - } -} - -func TestGetDomainFromDistinguishedName(t *testing.T) { - tests := []struct { - name string - distinguishedName string - wantDomain string - wantErr string - }{ - { - name: "happy path", - distinguishedName: "CN=Mammals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", - wantDomain: "activedirectory.mycompany.example.com", - }, - { - name: "lowercased happy path", - distinguishedName: "cn=Mammals,ou=Users,ou=pinniped-ad,dc=activedirectory,dc=mycompany,dc=example,dc=com", - wantDomain: "activedirectory.mycompany.example.com", - }, - { - name: "no domain components", - distinguishedName: "not-a-dn", - wantErr: "did not find domain components in group dn: not-a-dn", - }, - } - - for _, test := range tests { - tt := test - t.Run(tt.name, func(t *testing.T) { - actualDomain, err := getDomainFromDistinguishedName(tt.distinguishedName) - if tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.Equal(t, tt.wantDomain, actualDomain) - }) - } -} - func TestAttributeUnchangedSinceLogin(t *testing.T) { initialVal := "some-attribute-value" changedVal := "some-different-attribute-value" @@ -2041,8 +1777,9 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: attributeName, - Values: []string{initialVal}, + Name: attributeName, + Values: []string{initialVal}, + ByteValues: [][]byte{[]byte(initialVal)}, }, }, }, @@ -2053,12 +1790,13 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: attributeName, - Values: []string{changedVal}, + Name: attributeName, + Values: []string{changedVal}, + ByteValues: [][]byte{[]byte(changedVal)}, }, }, }, - wantErr: "value for attribute \"some-attribute-name\" has changed since initial value at login. Previous value: \"some-attribute-value\", new value: \"some-different-attribute-value\"", + wantErr: "value for attribute \"some-attribute-name\" has changed since initial value at login", }, { name: "no value for attribute attribute", @@ -2085,141 +1823,8 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.StoredRefreshAttributes{AdditionalAttributes: map[string][]string{attributeName: {initialVal}}}) - if tt.wantErr != "" { - require.Error(t, err) - require.Equal(t, tt.wantErr, err.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestValidUserAccountControl(t *testing.T) { - tests := []struct { - name string - entry *ldap.Entry - wantErr string - }{ - { - name: "happy normal user", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "userAccountControl", - Values: []string{"512"}, - }, - }, - }, - }, - { - name: "happy user whose password doesn't expire", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "userAccountControl", - Values: []string{"65536"}, - }, - }, - }, - }, - { - name: "deactivated user", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "userAccountControl", - Values: []string{"514"}, - }, - }, - }, - wantErr: "user has been deactivated", - }, - { - name: "non-integer result", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "userAccountControl", - Values: []string{"not-an-int"}, - }, - }, - }, - wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", - }, - } - - for _, test := range tests { - tt := test - t.Run(tt.name, func(t *testing.T) { - err := ValidUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) - - if tt.wantErr != "" { - require.Error(t, err) - require.Equal(t, tt.wantErr, err.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestValidComputedUserAccountControl(t *testing.T) { - tests := []struct { - name string - entry *ldap.Entry - wantErr string - }{ - { - name: "happy normal user", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "msDS-User-Account-Control-Computed", - Values: []string{"0"}, - }, - }, - }, - }, - { - name: "locked user", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "msDS-User-Account-Control-Computed", - Values: []string{"16"}, - }, - }, - }, - wantErr: "user has been locked", - }, - { - name: "non-integer result", - entry: &ldap.Entry{ - DN: "some-dn", - Attributes: []*ldap.EntryAttribute{ - { - Name: "msDS-User-Account-Control-Computed", - Values: []string{"not-an-int"}, - }, - }, - }, - wantErr: "strconv.Atoi: parsing \"not-an-int\": invalid syntax", - }, - } - - for _, test := range tests { - tt := test - t.Run(tt.name, func(t *testing.T) { - err := ValidComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) - + initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal)) + err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.StoredRefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}}) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index 1d913f8a..1897d924 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -83,7 +83,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(nil)), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -95,7 +97,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.ConnectionProtocol = upstreamldap.StartTLS })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -104,7 +108,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "dc=pinniped,dc=dev" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -113,7 +119,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "(cn={})" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -125,7 +133,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.UserSearch.Filter = "cn={}" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "cn=pinny,ou=users,dc=pinniped,dc=dev", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "cn=pinny,ou=users,dc=pinniped,dc=dev", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -136,7 +146,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.UserSearch.Filter = "(|(cn={})(mail={}))" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -147,7 +159,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.UserSearch.Filter = "(|(cn={})(mail={}))" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -156,7 +170,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UIDAttribute = "dn" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("cn=pinny,ou=users,dc=pinniped,dc=dev"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("cn=pinny,ou=users,dc=pinniped,dc=dev"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -165,7 +181,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UIDAttribute = "sn" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("Seal"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("Seal"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -174,7 +192,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UsernameAttribute = "sn" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "Seal", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", // note that the final answer has case preserved from the entry + User: &user.DefaultInfo{Name: "Seal", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", // note that the final answer has case preserved from the entry + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -187,7 +207,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.UserSearch.UIDAttribute = "givenName" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "Pinny the 🦭", UID: b64("Pinny the 🦭"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "Pinny the 🦭", UID: b64("Pinny the 🦭"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -199,7 +221,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.UserSearch.UsernameAttribute = "cn" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -220,7 +244,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Base = "" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -231,7 +257,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Base = "ou=users,dc=pinniped,dc=dev" // there are no groups under this part of the tree })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -245,7 +273,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{ "cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev", "cn=seals,ou=groups,dc=pinniped,dc=dev", - }}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + }}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -259,7 +289,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{ "cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev", "cn=seals,ou=groups,dc=pinniped,dc=dev", - }}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + }}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -270,7 +302,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.GroupNameAttribute = "objectClass" // silly example, but still a meaningful test })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"groupOfNames", "groupOfNames"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"groupOfNames", "groupOfNames"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -281,7 +315,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Filter = "(&(&(objectClass=groupOfNames)(member={}))(cn=seals))" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"seals"}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -292,7 +328,9 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Filter = "foobar={}" // foobar is not a valid attribute name for this LDAP server's schema })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, }, { @@ -671,8 +709,9 @@ func TestSimultaneousLDAPRequestsOnSingleProvider(t *testing.T) { assert.NoError(t, result.err) assert.True(t, result.authenticated, "expected the user to be authenticated, but they were not") assert.Equal(t, &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, - DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, }, result.response) } } diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 812a32b9..b10f650e 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -23,8 +23,6 @@ import ( "testing" "time" - "go.pinniped.dev/internal/crypto/ptls" - coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/go-ldap/ldap/v3" "github.com/stretchr/testify/assert" @@ -37,6 +35,7 @@ import ( configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/psession" "go.pinniped.dev/internal/testutil"