diff options
author | Scott Dial <geekmug@gmail.com> | 2013-08-11 16:52:51 -0400 |
---|---|---|
committer | Scott Dial <geekmug@gmail.com> | 2013-08-12 21:00:26 -0400 |
commit | 78c978e40741fdbd5103449a14a5b60614d6fcff (patch) | |
tree | 0a458783ab05054161abe5767178cc3adbe157e5 | |
parent | 09a35b97254f84624775f15283aa6bffaadcda70 (diff) |
Expand capabilities of ldap.groupMemberPattern
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
3 files changed, 9 insertions, 15 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 6228a94ab7..27a0937f63 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -1812,8 +1812,8 @@ corresponding attribute (in this case, `fooBarAttribute`) as read from the user's account object matched under `ldap.accountBase`. Attributes such as `${dn}` or `${uidNumber}` may be useful. + -Default is `(memberUid=${username})` for RFC 2307, -and unset (disabled) for Active Directory. +Default is `(|(memberUid=${username})(gidNumber=${gidNumber}))` for +RFC 2307, and unset (disabled) for Active Directory. [[ldap.groupName]]ldap.groupName:: + diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java index 0151dde40f..7d0ad24325 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java @@ -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); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapType.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapType.java index db5baebfc0..3c1b0d22b3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapType.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapType.java @@ -57,7 +57,7 @@ abstract class LdapType { @Override String groupMemberPattern() { - return "(memberUid=${username})"; + return "(|(memberUid=${username})(gidNumber=${gidNumber}))"; } @Override |