From c70a0b99a846b399027abdae1a2fdace636d5c8c Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 22 Jun 2022 10:58:08 -0700 Subject: [PATCH] Don't do ldap group search when group scope not specified Signed-off-by: Margo Crawford --- internal/authenticators/authenticators.go | 2 +- .../active_directory_upstream_watcher.go | 6 +-- .../active_directory_upstream_watcher_test.go | 38 +++++++++--------- internal/oidc/auth/auth_handler.go | 2 +- internal/oidc/login/post_login_handler.go | 2 +- .../provider/dynamic_upstream_idp_provider.go | 7 ++-- internal/oidc/token/token_handler.go | 5 ++- .../testutil/oidctestutil/oidctestutil.go | 6 +-- internal/upstreamldap/upstreamldap.go | 39 ++++++++++++------- internal/upstreamldap/upstreamldap_test.go | 19 ++++----- test/integration/ldap_client_test.go | 22 +++++++++-- 11 files changed, 88 insertions(+), 60 deletions(-) diff --git a/internal/authenticators/authenticators.go b/internal/authenticators/authenticators.go index e343ecd1..9d675c2a 100644 --- a/internal/authenticators/authenticators.go +++ b/internal/authenticators/authenticators.go @@ -31,7 +31,7 @@ import ( // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator // interface, as well as the Response type. type UserAuthenticator interface { - AuthenticateUser(ctx context.Context, username, password string) (*Response, bool, error) + AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*Response, bool, error) } type Response struct { diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 4aaa41b9..03bdf332 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -338,7 +338,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){ "objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"), }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ pwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute), userAccountControlAttribute: validUserAccountControl, userAccountControlComputedAttribute: validComputedUserAccountControl, @@ -437,7 +437,7 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { return strings.Join(domainComponents[1:], "."), nil } -func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { +func validUserAccountControl(entry *ldap.Entry, _ provider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) if err != nil { return err @@ -450,7 +450,7 @@ func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttribut return nil } -func validComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { +func validComputedUserAccountControl(entry *ldap.Entry, _ provider.RefreshAttributes) error { userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute)) if err != nil { return err 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 ce69b4af..dc5fa693 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) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -564,7 +564,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -633,7 +633,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: "sAMAccountName", }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -705,7 +705,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -784,7 +784,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -847,7 +847,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -997,7 +997,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1146,7 +1146,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1217,7 +1217,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1483,7 +1483,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1542,7 +1542,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1605,7 +1605,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1668,7 +1668,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1879,7 +1879,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { GroupNameAttribute: testGroupNameAttrName, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -1941,7 +1941,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { SkipGroupRefresh: true, }, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), "userAccountControl": validUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl, @@ -2083,8 +2083,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { 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{} + copyOfExpectedValueForResultingCache.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} + actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{} require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks)) for k, v := range expectedRefreshAttributeChecks { require.NotNil(t, actualRefreshAttributeChecks[k]) @@ -2333,7 +2333,7 @@ func TestValidUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validUserAccountControl(tt.entry, provider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) @@ -2394,7 +2394,7 @@ func TestValidComputedUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := validComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validComputedUserAccountControl(tt.entry, provider.RefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 67b1581b..adbbec7c 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -131,7 +131,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( return nil } - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password) + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") diff --git a/internal/oidc/login/post_login_handler.go b/internal/oidc/login/post_login_handler.go index dafe6e06..bc851c54 100644 --- a/internal/oidc/login/post_login_handler.go +++ b/internal/oidc/login/post_login_handler.go @@ -60,7 +60,7 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider } // Attempt to authenticate the user with the upstream IDP. - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password) + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) // There was some problem during authentication with the upstream, aside from bad username/password. diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index befcddb8..a5eabea5 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -108,17 +108,18 @@ type UpstreamLDAPIdentityProviderI interface { authenticators.UserAuthenticator // PerformRefresh performs a refresh against the upstream LDAP identity provider - PerformRefresh(ctx context.Context, storedRefreshAttributes StoredRefreshAttributes) (groups []string, err error) + PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes) (groups []string, err error) } -// StoredRefreshAttributes contains information about the user from the original login request +// RefreshAttributes contains information about the user from the original login request // and previous refreshes. -type StoredRefreshAttributes struct { +type RefreshAttributes struct { Username string Subject string DN string Groups []string AdditionalAttributes map[string]string + GrantedScopes []string } type DynamicUpstreamIDPProvider interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 6c3bf575..76727a12 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -181,7 +181,7 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, } groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) - if groupsScope { + if groupsScope { //nolint:nestif // If possible, update the user's group memberships. The configured groups claim name (if there is one) may or // may not be included in the newly fetched and merged claims. It could be missing due to a misconfiguration of the // claim name. It could also be missing because the claim was originally found in the ID token during login, but @@ -333,12 +333,13 @@ func upstreamLDAPRefresh(ctx context.Context, providerCache oidc.UpstreamIdentit return errorsx.WithStack(errMissingUpstreamSessionInternalError()) } // run PerformRefresh - groups, err := p.PerformRefresh(ctx, provider.StoredRefreshAttributes{ + groups, err := p.PerformRefresh(ctx, provider.RefreshAttributes{ Username: username, Subject: subject, DN: dn, Groups: oldGroups, AdditionalAttributes: additionalAttributes, + GrantedScopes: grantedScopes, }) if err != nil { return errUpstreamRefreshError().WithHint( diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 508e4bf0..fb1e8a7a 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -92,7 +92,7 @@ type ValidateTokenAndMergeWithUserInfoArgs struct { type ValidateRefreshArgs struct { Ctx context.Context Tok *oauth2.Token - StoredAttributes provider.StoredRefreshAttributes + StoredAttributes provider.RefreshAttributes } type TestUpstreamLDAPIdentityProvider struct { @@ -116,7 +116,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetName() string { return u.Name } -func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { +func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*authenticators.Response, bool, error) { return u.AuthenticateFunc(ctx, username, password) } @@ -124,7 +124,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { return u.URL } -func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) ([]string, error) { +func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { if u.performRefreshArgs == nil { u.performRefreshArgs = make([]*PerformRefreshArgs, 0) } diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index ddee048b..7317d939 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -16,6 +16,10 @@ import ( "strings" "time" + "go.pinniped.dev/internal/oidc" + + "k8s.io/utils/strings/slices" + "github.com/go-ldap/ldap/v3" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -118,7 +122,7 @@ type ProviderConfig struct { GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error) // RefreshAttributeChecks are extra checks that attributes in a refresh response are as expected. - RefreshAttributeChecks map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error + RefreshAttributeChecks map[string]func(*ldap.Entry, provider.RefreshAttributes) error } // UserSearchConfig contains information about how to search for users in the upstream LDAP IDP. @@ -175,7 +179,7 @@ func (p *Provider) GetConfig() ProviderConfig { return p.c } -func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.StoredRefreshAttributes) ([]string, error) { +func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes provider.RefreshAttributes) ([]string, error) { t := trace.FromContext(ctx).Nest("slow ldap refresh attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches userDN := storedRefreshAttributes.DN @@ -238,6 +242,10 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p if p.c.GroupSearch.SkipGroupRefresh { return storedRefreshAttributes.Groups, nil } + // if we were not granted the groups scope, we should not search for groups or return any. + if !slices.Contains(storedRefreshAttributes.GrantedScopes, oidc.DownstreamGroupsScope) { + return nil, nil + } mappedGroupNames, err := p.searchGroupsForUserDN(conn, userDN) if err != nil { @@ -398,23 +406,23 @@ func (p *Provider) TestConnection(ctx context.Context) error { // authentication for a given end user's username. It runs the same logic as AuthenticateUser except it does // not bind as that user, so it does not test their password. It returns the same values that a real call to // AuthenticateUser with the correct password would return. -func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) (*authenticators.Response, bool, error) { +func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string, grantedScopes []string) (*authenticators.Response, bool, error) { endUserBindFunc := func(conn Conn, foundUserDN string) error { // Act as if the end user bind always succeeds. return nil } - return p.authenticateUserImpl(ctx, username, endUserBindFunc) + return p.authenticateUserImpl(ctx, username, grantedScopes, endUserBindFunc) } // Authenticate an end user and return their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. -func (p *Provider) AuthenticateUser(ctx context.Context, username, password string) (*authenticators.Response, bool, error) { +func (p *Provider) AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*authenticators.Response, bool, error) { endUserBindFunc := func(conn Conn, foundUserDN string) error { return conn.Bind(foundUserDN, password) } - return p.authenticateUserImpl(ctx, username, endUserBindFunc) + return p.authenticateUserImpl(ctx, username, grantedScopes, endUserBindFunc) } -func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, bool, error) { +func (p *Provider) authenticateUserImpl(ctx context.Context, username string, grantedScopes []string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, bool, error) { t := trace.FromContext(ctx).Nest("slow ldap authenticate user attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches @@ -443,7 +451,7 @@ func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bi return nil, false, fmt.Errorf(`error binding as %q before user search: %w`, p.c.BindUsername, err) } - response, err := p.searchAndBindUser(conn, username, bindFunc) + response, err := p.searchAndBindUser(conn, username, grantedScopes, bindFunc) if err != nil { p.traceAuthFailure(t, err) return nil, false, err @@ -540,7 +548,7 @@ func (p *Provider) SearchForDefaultNamingContext(ctx context.Context) (string, e return searchBase, nil } -func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, error) { +func (p *Provider) searchAndBindUser(conn Conn, username string, grantedScopes []string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticators.Response, error) { searchResult, err := conn.Search(p.userSearchRequest(username)) if err != nil { plog.All(`error searching for user`, @@ -586,9 +594,12 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c return nil, err } - mappedGroupNames, err := p.searchGroupsForUserDN(conn, userEntry.DN) - if err != nil { - return nil, err + var mappedGroupNames []string + if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) { + mappedGroupNames, err = p.searchGroupsForUserDN(conn, userEntry.DN) + if err != nil { + return nil, err + } } mappedRefreshAttributes := make(map[string]string) @@ -822,8 +833,8 @@ func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) { ) } -func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.StoredRefreshAttributes) error { - return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { +func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error { + return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error { prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] newValues := entry.GetRawAttributeValues(attribute) diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index b4ee6bdf..9a9ca782 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -638,8 +638,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { return nil }, } @@ -676,8 +676,8 @@ func TestEndUserAuthentication(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(func(p *ProviderConfig) { - p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error{ - "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.StoredRefreshAttributes) error { + p.RefreshAttributeChecks = map[string]func(entry *ldap.Entry, attributes provider.RefreshAttributes) error{ + "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error { return nil }, } @@ -1167,7 +1167,7 @@ func TestEndUserAuthentication(t *testing.T) { ldapProvider := New(*tt.providerConfig) - authResponse, authenticated, err := ldapProvider.AuthenticateUser(context.Background(), tt.username, tt.password) + authResponse, authenticated, err := ldapProvider.AuthenticateUser(context.Background(), tt.username, tt.password, []string{"groups"}) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1199,7 +1199,7 @@ func TestEndUserAuthentication(t *testing.T) { } // Skip tt.bindEndUserMocks since DryRunAuthenticateUser() never binds as the end user. - authResponse, authenticated, err = ldapProvider.DryRunAuthenticateUser(context.Background(), tt.username) + authResponse, authenticated, err = ldapProvider.DryRunAuthenticateUser(context.Background(), tt.username, []string{"groups"}) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": @@ -1318,7 +1318,7 @@ func TestUpstreamRefresh(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupSearchGroupNameAttribute, }, - RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ + RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{ pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute), }, } @@ -1772,11 +1772,12 @@ func TestUpstreamRefresh(t *testing.T) { initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000")) ldapProvider := New(*tt.providerConfig) subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" - groups, err := ldapProvider.PerformRefresh(context.Background(), provider.StoredRefreshAttributes{ + groups, err := ldapProvider.PerformRefresh(context.Background(), provider.RefreshAttributes{ Username: testUserSearchResultUsernameAttributeValue, Subject: subject, DN: tt.refreshUserDN, AdditionalAttributes: map[string]string{pwdLastSetAttribute: initialPwdLastSetEncoded}, + GrantedScopes: []string{"groups"}, }) if tt.wantErr != "" { require.Error(t, err) @@ -2149,7 +2150,7 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { tt := test t.Run(tt.name, func(t *testing.T) { initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal)) - err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.StoredRefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}}) + err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.RefreshAttributes{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 584b68f5..3541dfa0 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -73,6 +73,7 @@ func TestLDAPSearch_Parallel(t *testing.T) { name string username string password string + grantedScopes []string provider *upstreamldap.Provider wantError string wantAuthResponse *authenticators.Response @@ -114,6 +115,18 @@ func TestLDAPSearch_Parallel(t *testing.T) { ExtraRefreshAttributes: map[string]string{}, }, }, + { + name: "groups scope not in granted scopes", + username: "pinny", + password: pinnyPassword, + grantedScopes: []string{}, + provider: upstreamldap.New(*providerConfig(nil)), + wantAuthResponse: &authenticators.Response{ + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: nil}, + DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", + ExtraRefreshAttributes: map[string]string{}, + }, + }, { name: "when the user search filter is already wrapped by parenthesis", username: "pinny", @@ -636,7 +649,10 @@ func TestLDAPSearch_Parallel(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - authResponse, authenticated, err := tt.provider.AuthenticateUser(ctx, tt.username, tt.password) + if tt.grantedScopes == nil { + tt.grantedScopes = []string{"groups"} + } + authResponse, authenticated, err := tt.provider.AuthenticateUser(ctx, tt.username, tt.password, tt.grantedScopes) switch { case tt.wantError != "": @@ -694,9 +710,7 @@ func TestSimultaneousLDAPRequestsOnSingleProvider(t *testing.T) { authUserCtx, authUserCtxCancelFunc := context.WithTimeout(context.Background(), 2*time.Minute) defer authUserCtxCancelFunc() - authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, - env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword, - ) + authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword, []string{"groups"}) resultCh <- authUserResult{ response: authResponse, authenticated: authenticated,