diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 96a9db16..c25875c3 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -465,7 +465,6 @@ func (p *Provider) searchGroupsForUserDN(conn Conn, userDN string) ([]string, er } groups := []string{} -entries: for _, groupEntry := range searchResult.Entries { if len(groupEntry.DN) == 0 { return nil, fmt.Errorf(`searching for group memberships for user with DN %q resulted in search result without DN`, userDN) @@ -476,7 +475,6 @@ entries: return nil, fmt.Errorf("error finding groups for user %s: %w", userDN, err) } groups = append(groups, overrideGroupName) - continue entries } // if none of the overrides matched, use the default behavior (no mapping) mappedGroupName, err := p.getSearchResultAttributeValue(groupAttributeName, groupEntry, userDN) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 6f6a8314..ff38956e 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -307,6 +307,94 @@ func TestSupervisorLogin_Browser(t *testing.T) { }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, }, + { + name: "ldap with email as username and group search base that doesn't return anything, and using an LDAP provider which supports TLS", + 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.SupervisorUpstreamLDAP.UserSearchBase == env.SupervisorUpstreamLDAP.GroupSearchBase { + // This test relies on using the user search base as the group search base, to simulate + // searching for groups and not finding any. + // If the users and groups are stored in the same place, then we will get groups + // back, so this test wouldn't make sense. + t.Skip("must have a different user search base than group search base") + } + }, + createIDP: func(t *testing.T) string { + t.Helper() + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + ) + ldapIDP := testlib.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ + Host: env.SupervisorUpstreamLDAP.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), + }, + Bind: idpv1alpha1.LDAPIdentityProviderBind{ + SecretName: secret.Name, + }, + UserSearch: idpv1alpha1.LDAPIdentityProviderUserSearch{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderUserSearchAttributes{ + Username: env.SupervisorUpstreamLDAP.TestUserMailAttributeName, + UID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, + }, + }, + GroupSearch: idpv1alpha1.LDAPIdentityProviderGroupSearch{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, // groups not stored at the user search base + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderGroupSearchAttributes{ + GroupName: "dn", + }, + }, + }, idpv1alpha1.LDAPPhaseReady) + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamLDAP.Host, env.SupervisorUpstreamLDAP.BindUsername, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) + return ldapIDP.Name + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _, _, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login + env.SupervisorUpstreamLDAP.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + editRefreshSessionDataWithoutBreaking: func(t *testing.T, sessionData *psession.PinnipedSession) { + // even if we update this group to the wrong thing, we expect that it will return to the correct + // value after we refresh. + sessionData.Fosite.Claims.Extra["groups"] = []string{"some-wrong-group", "some-other-group"} + }, + 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) + fositeSessionData := pinnipedSession.Fosite + fositeSessionData.Claims.Subject = "not-right" + }, + // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute + wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta( + "ldaps://"+env.SupervisorUpstreamLDAP.Host+ + "?base="+url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase)+ + "&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(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$" + }, + wantDownstreamIDTokenGroups: []string{}, + }, { 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 maybeSkip: func(t *testing.T) {