opendev/gerrit
Code Issues Proposed changes

Expand capabilities of ldap.groupMemberPattern

Browse Source 
Previously, the pattern supported a limited set of variables that were
either accidentially available (due to their use in other queries) or
hard-coded (e.g., `username` is a special-case that was added).
Furthermore, the documentation made reference to being able to use
variables such as `${uidNumber}` even though they are not actually
supported (since `uidNumber` is normally never queried). Under the
default RFC 2307 configuration of LDAP, the only variables available
were `displayName, `mail`, `uid`, and `username` (It's noteworthy
that `username` was added as a special-case due to the default
`groupMemberPattern` containing `${username}` even though `username`
is substitued by Gerrit and not LDAP).

This changeset removes the artificial restrictions on the attributes
used in the `groupMemberPattern`. Any variable is assumed to
originate from the account, but `username` is still overridden and
provided by Gerrit (as before). This allows more expressive patterns,
which allows us to fix an outstanding bug in group matching. Prevously,
a user whose `gidNumber` matched the group's `gidNumber` would not have
been included in the group. This changeset updates the default
`groupMemberPattern` to account for this issue by adding the additional
case of `(gidNumber=${gidNumber}`.

Bug: Issue 2054
Change-Id: Iff3a14c569a10c1ef693b672f4710fb6f2f8d9a6
This commit is contained in:
Scott Dial
2013-08-11 16:52:51 -04:00
parent 76e7b42490
commit 50457503f2
3 changed files with 9 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
View File

@@ -197,13 +197,11 @@ import javax.security.auth.login.LoginException;
if (!schema.groupMemberQueryList.isEmpty()) {
final HashMap<String, String> params = new HashMap<String, String>();
if (schema.groupNeedsAccount) {
if (account == null) {
account = findAccount(schema, ctx, username);
}
for (String name : schema.groupMemberQueryList.get(0).getParameters()) {
params.put(name, account.get(name));
}
if (account == null) {
account = findAccount(schema, ctx, username);
}
for (String name : schema.groupMemberQueryList.get(0).getParameters()) {
params.put(name, account.get(name));
}
params.put(LdapRealm.USERNAME, username);
@@ -286,7 +284,6 @@ import javax.security.auth.login.LoginException;
final String accountMemberField;
final List<LdapQuery> accountQueryList;
boolean groupNeedsAccount;
final List<String> groupBases;
final SearchScope groupScope;
final ParameterizedString groupPattern;
@@ -321,10 +318,7 @@ import javax.security.auth.login.LoginException;
}
for (final String name : groupMemberQuery.getParameters()) {
if (!LdapRealm.USERNAME.equals(name)) {
groupNeedsAccount = true;
accountAtts.add(name);
}
accountAtts.add(name);
}
groupMemberQueryList.add(groupMemberQuery);

2
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapType.java
View File

@@ -57,7 +57,7 @@ abstract class LdapType {
@Override
String groupMemberPattern() {
return "(memberUid=${username})";
return "(|(memberUid=${username})(gidNumber=${gidNumber}))";
}
@Override