Don't do ldap group search when group scope not specified

Signed-off-by: Margo Crawford <margaretc@vmware.com>
This commit is contained in:
Margo Crawford 2022-06-22 10:58:08 -07:00
parent 9903c5f79e
commit c70a0b99a8
11 changed files with 88 additions and 60 deletions

View File

@ -31,7 +31,7 @@ import (
// See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator
// interface, as well as the Response type. // interface, as well as the Response type.
type UserAuthenticator interface { 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 { type Response struct {

View File

@ -338,7 +338,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){ UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){
"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"), "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), pwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute),
userAccountControlAttribute: validUserAccountControl, userAccountControlAttribute: validUserAccountControl,
userAccountControlComputedAttribute: validComputedUserAccountControl, userAccountControlComputedAttribute: validComputedUserAccountControl,
@ -437,7 +437,7 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) {
return strings.Join(domainComponents[1:], "."), nil 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)) userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute))
if err != nil { if err != nil {
return err return err
@ -450,7 +450,7 @@ func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttribut
return nil 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)) userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute))
if err != nil { if err != nil {
return err return err

View File

@ -222,7 +222,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -564,7 +564,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -633,7 +633,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: "sAMAccountName", GroupNameAttribute: "sAMAccountName",
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -705,7 +705,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -784,7 +784,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -847,7 +847,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -997,7 +997,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1146,7 +1146,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1217,7 +1217,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "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")}, UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1542,7 +1542,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1605,7 +1605,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1668,7 +1668,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1879,7 +1879,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
GroupNameAttribute: testGroupNameAttrName, GroupNameAttribute: testGroupNameAttrName,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -1941,7 +1941,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
SkipGroupRefresh: true, SkipGroupRefresh: true,
}, },
UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, 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"), "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
"userAccountControl": validUserAccountControl, "userAccountControl": validUserAccountControl,
"msDS-User-Account-Control-Computed": validComputedUserAccountControl, "msDS-User-Account-Control-Computed": validComputedUserAccountControl,
@ -2083,8 +2083,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
expectedRefreshAttributeChecks := copyOfExpectedValueForResultingCache.RefreshAttributeChecks expectedRefreshAttributeChecks := copyOfExpectedValueForResultingCache.RefreshAttributeChecks
actualRefreshAttributeChecks := actualConfig.RefreshAttributeChecks actualRefreshAttributeChecks := actualConfig.RefreshAttributeChecks
copyOfExpectedValueForResultingCache.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.StoredRefreshAttributes) error{} actualConfig.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{}
require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks)) require.Equal(t, len(expectedRefreshAttributeChecks), len(actualRefreshAttributeChecks))
for k, v := range expectedRefreshAttributeChecks { for k, v := range expectedRefreshAttributeChecks {
require.NotNil(t, actualRefreshAttributeChecks[k]) require.NotNil(t, actualRefreshAttributeChecks[k])
@ -2333,7 +2333,7 @@ func TestValidUserAccountControl(t *testing.T) {
for _, test := range tests { for _, test := range tests {
tt := test tt := test
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
err := validUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) err := validUserAccountControl(tt.entry, provider.RefreshAttributes{})
if tt.wantErr != "" { if tt.wantErr != "" {
require.Error(t, err) require.Error(t, err)
@ -2394,7 +2394,7 @@ func TestValidComputedUserAccountControl(t *testing.T) {
for _, test := range tests { for _, test := range tests {
tt := test tt := test
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
err := validComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) err := validComputedUserAccountControl(tt.entry, provider.RefreshAttributes{})
if tt.wantErr != "" { if tt.wantErr != "" {
require.Error(t, err) require.Error(t, err)

View File

@ -131,7 +131,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow(
return nil 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 { if err != nil {
plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName())
return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication")

View File

@ -60,7 +60,7 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider
} }
// Attempt to authenticate the user with the upstream IDP. // 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 { if err != nil {
plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName())
// There was some problem during authentication with the upstream, aside from bad username/password. // There was some problem during authentication with the upstream, aside from bad username/password.

View File

@ -108,17 +108,18 @@ type UpstreamLDAPIdentityProviderI interface {
authenticators.UserAuthenticator authenticators.UserAuthenticator
// PerformRefresh performs a refresh against the upstream LDAP identity provider // 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. // and previous refreshes.
type StoredRefreshAttributes struct { type RefreshAttributes struct {
Username string Username string
Subject string Subject string
DN string DN string
Groups []string Groups []string
AdditionalAttributes map[string]string AdditionalAttributes map[string]string
GrantedScopes []string
} }
type DynamicUpstreamIDPProvider interface { type DynamicUpstreamIDPProvider interface {

View File

@ -181,7 +181,7 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession,
} }
groupsScope := slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) 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 // 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 // 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 // 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()) return errorsx.WithStack(errMissingUpstreamSessionInternalError())
} }
// run PerformRefresh // run PerformRefresh
groups, err := p.PerformRefresh(ctx, provider.StoredRefreshAttributes{ groups, err := p.PerformRefresh(ctx, provider.RefreshAttributes{
Username: username, Username: username,
Subject: subject, Subject: subject,
DN: dn, DN: dn,
Groups: oldGroups, Groups: oldGroups,
AdditionalAttributes: additionalAttributes, AdditionalAttributes: additionalAttributes,
GrantedScopes: grantedScopes,
}) })
if err != nil { if err != nil {
return errUpstreamRefreshError().WithHint( return errUpstreamRefreshError().WithHint(

View File

@ -92,7 +92,7 @@ type ValidateTokenAndMergeWithUserInfoArgs struct {
type ValidateRefreshArgs struct { type ValidateRefreshArgs struct {
Ctx context.Context Ctx context.Context
Tok *oauth2.Token Tok *oauth2.Token
StoredAttributes provider.StoredRefreshAttributes StoredAttributes provider.RefreshAttributes
} }
type TestUpstreamLDAPIdentityProvider struct { type TestUpstreamLDAPIdentityProvider struct {
@ -116,7 +116,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetName() string {
return u.Name 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) return u.AuthenticateFunc(ctx, username, password)
} }
@ -124,7 +124,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL {
return u.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 { if u.performRefreshArgs == nil {
u.performRefreshArgs = make([]*PerformRefreshArgs, 0) u.performRefreshArgs = make([]*PerformRefreshArgs, 0)
} }

View File

@ -16,6 +16,10 @@ import (
"strings" "strings"
"time" "time"
"go.pinniped.dev/internal/oidc"
"k8s.io/utils/strings/slices"
"github.com/go-ldap/ldap/v3" "github.com/go-ldap/ldap/v3"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
@ -118,7 +122,7 @@ type ProviderConfig struct {
GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error) GroupAttributeParsingOverrides map[string]func(*ldap.Entry) (string, error)
// RefreshAttributeChecks are extra checks that attributes in a refresh response are as expected. // 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. // 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 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()}) 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 defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches
userDN := storedRefreshAttributes.DN userDN := storedRefreshAttributes.DN
@ -238,6 +242,10 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p
if p.c.GroupSearch.SkipGroupRefresh { if p.c.GroupSearch.SkipGroupRefresh {
return storedRefreshAttributes.Groups, nil 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) mappedGroupNames, err := p.searchGroupsForUserDN(conn, userDN)
if err != nil { 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 // 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 // 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. // 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 { endUserBindFunc := func(conn Conn, foundUserDN string) error {
// Act as if the end user bind always succeeds. // Act as if the end user bind always succeeds.
return nil 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. // 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 { endUserBindFunc := func(conn Conn, foundUserDN string) error {
return conn.Bind(foundUserDN, password) 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()}) 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 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) 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 { if err != nil {
p.traceAuthFailure(t, err) p.traceAuthFailure(t, err)
return nil, false, err return nil, false, err
@ -540,7 +548,7 @@ func (p *Provider) SearchForDefaultNamingContext(ctx context.Context) (string, e
return searchBase, nil 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)) searchResult, err := conn.Search(p.userSearchRequest(username))
if err != nil { if err != nil {
plog.All(`error searching for user`, plog.All(`error searching for user`,
@ -586,9 +594,12 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c
return nil, err return nil, err
} }
mappedGroupNames, err := p.searchGroupsForUserDN(conn, userEntry.DN) var mappedGroupNames []string
if err != nil { if slices.Contains(grantedScopes, oidc.DownstreamGroupsScope) {
return nil, err mappedGroupNames, err = p.searchGroupsForUserDN(conn, userEntry.DN)
if err != nil {
return nil, err
}
} }
mappedRefreshAttributes := make(map[string]string) 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 { func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error {
return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error {
prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] prevAttributeValue := storedAttributes.AdditionalAttributes[attribute]
newValues := entry.GetRawAttributeValues(attribute) newValues := entry.GetRawAttributeValues(attribute)

View File

@ -638,8 +638,8 @@ func TestEndUserAuthentication(t *testing.T) {
username: testUpstreamUsername, username: testUpstreamUsername,
password: testUpstreamPassword, password: testUpstreamPassword,
providerConfig: providerConfig(func(p *ProviderConfig) { providerConfig: providerConfig(func(p *ProviderConfig) {
p.RefreshAttributeChecks = map[string]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.StoredRefreshAttributes) error { "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error {
return nil return nil
}, },
} }
@ -676,8 +676,8 @@ func TestEndUserAuthentication(t *testing.T) {
username: testUpstreamUsername, username: testUpstreamUsername,
password: testUpstreamPassword, password: testUpstreamPassword,
providerConfig: providerConfig(func(p *ProviderConfig) { providerConfig: providerConfig(func(p *ProviderConfig) {
p.RefreshAttributeChecks = map[string]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.StoredRefreshAttributes) error { "some-attribute-to-check-during-refresh": func(entry *ldap.Entry, attributes provider.RefreshAttributes) error {
return nil return nil
}, },
} }
@ -1167,7 +1167,7 @@ func TestEndUserAuthentication(t *testing.T) {
ldapProvider := New(*tt.providerConfig) 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) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted)
switch { switch {
case tt.wantError != "": case tt.wantError != "":
@ -1199,7 +1199,7 @@ func TestEndUserAuthentication(t *testing.T) {
} }
// Skip tt.bindEndUserMocks since DryRunAuthenticateUser() never binds as the end user. // 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) require.Equal(t, !tt.wantToSkipDial, dialWasAttempted)
switch { switch {
case tt.wantError != "": case tt.wantError != "":
@ -1318,7 +1318,7 @@ func TestUpstreamRefresh(t *testing.T) {
Filter: testGroupSearchFilter, Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupSearchGroupNameAttribute, GroupNameAttribute: testGroupSearchGroupNameAttribute,
}, },
RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute), pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute),
}, },
} }
@ -1772,11 +1772,12 @@ func TestUpstreamRefresh(t *testing.T) {
initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000")) initialPwdLastSetEncoded := base64.RawURLEncoding.EncodeToString([]byte("132801740800000000"))
ldapProvider := New(*tt.providerConfig) ldapProvider := New(*tt.providerConfig)
subject := "ldaps://ldap.example.com:8443?base=some-upstream-user-base-dn&sub=c29tZS11cHN0cmVhbS11aWQtdmFsdWU" 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, Username: testUserSearchResultUsernameAttributeValue,
Subject: subject, Subject: subject,
DN: tt.refreshUserDN, DN: tt.refreshUserDN,
AdditionalAttributes: map[string]string{pwdLastSetAttribute: initialPwdLastSetEncoded}, AdditionalAttributes: map[string]string{pwdLastSetAttribute: initialPwdLastSetEncoded},
GrantedScopes: []string{"groups"},
}) })
if tt.wantErr != "" { if tt.wantErr != "" {
require.Error(t, err) require.Error(t, err)
@ -2149,7 +2150,7 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) {
tt := test tt := test
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal)) 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 != "" { if tt.wantErr != "" {
require.Error(t, err) require.Error(t, err)
require.Equal(t, tt.wantErr, err.Error()) require.Equal(t, tt.wantErr, err.Error())

View File

@ -73,6 +73,7 @@ func TestLDAPSearch_Parallel(t *testing.T) {
name string name string
username string username string
password string password string
grantedScopes []string
provider *upstreamldap.Provider provider *upstreamldap.Provider
wantError string wantError string
wantAuthResponse *authenticators.Response wantAuthResponse *authenticators.Response
@ -114,6 +115,18 @@ func TestLDAPSearch_Parallel(t *testing.T) {
ExtraRefreshAttributes: map[string]string{}, 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", name: "when the user search filter is already wrapped by parenthesis",
username: "pinny", username: "pinny",
@ -636,7 +649,10 @@ func TestLDAPSearch_Parallel(t *testing.T) {
for _, test := range tests { for _, test := range tests {
tt := test tt := test
t.Run(tt.name, func(t *testing.T) { 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 { switch {
case tt.wantError != "": case tt.wantError != "":
@ -694,9 +710,7 @@ func TestSimultaneousLDAPRequestsOnSingleProvider(t *testing.T) {
authUserCtx, authUserCtxCancelFunc := context.WithTimeout(context.Background(), 2*time.Minute) authUserCtx, authUserCtxCancelFunc := context.WithTimeout(context.Background(), 2*time.Minute)
defer authUserCtxCancelFunc() defer authUserCtxCancelFunc()
authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, authResponse, authenticated, err := provider.AuthenticateUser(authUserCtx, env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword, []string{"groups"})
env.SupervisorUpstreamLDAP.TestUserCN, env.SupervisorUpstreamLDAP.TestUserPassword,
)
resultCh <- authUserResult{ resultCh <- authUserResult{
response: authResponse, response: authResponse,
authenticated: authenticated, authenticated: authenticated,