Add option to make ldap groups visible to everyone

A non-admin user was not allowed to add ldap group as member
of another group if user did not belong to this ldap group.

When the option ldap.visibleToAll in gerrit.config is set to
true ldap groups are visible to everyone.

Issue:2255
Change-Id: Ibd234c6dfc8d890edde2304e820d01d359fda0fd
This commit is contained in:
Olga Grinberg
2015-02-03 15:54:48 -05:00
committed by Edwin Kempin
parent 06af319c99
commit cf1b06a39f
8 changed files with 53 additions and 7 deletions

View File

@@ -2259,6 +2259,13 @@ perform a query.
+ +
By default, true, requiring the certificate to be verified. By default, true, requiring the certificate to be verified.
[[ldap.groupsVisibleToAll]]ldap.groupsVisibleToAll::
+
If true, LDAP groups are visible to all registered users.
+
By default, false, LDAP groups are visible only to administrators and
group members.
[[ldap.username]]ldap.username:: [[ldap.username]]ldap.username::
+ +
_(Optional)_ Username to bind to the LDAP server with. If not set, _(Optional)_ Username to bind to the LDAP server with. If not set,

View File

@@ -23,4 +23,9 @@ public abstract class AbstractGroupBackend implements GroupBackend {
public boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids) { public boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids) {
return membershipsOf(user).containsAnyOf(ids); return membershipsOf(user).containsAnyOf(ids);
} }
@Override
public boolean isVisibleToAll(AccountGroup.UUID uuid) {
return false;
}
} }

View File

@@ -58,4 +58,10 @@ public interface GroupBackend {
* given groups, {@code false} otherwise * given groups, {@code false} otherwise
*/ */
boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids); boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids);
/**
* @return {@code true} if the group with the given UUID is visible to all
* registered users.
*/
boolean isVisibleToAll(AccountGroup.UUID uuid);
} }

View File

@@ -45,7 +45,7 @@ public class GroupControl {
if (group == null) { if (group == null) {
throw new NoSuchGroupException(groupId); throw new NoSuchGroupException(groupId);
} }
return new GroupControl(who, group); return new GroupControl(who, group, groupBackend);
} }
} }
@@ -85,7 +85,7 @@ public class GroupControl {
} }
public GroupControl controlFor(GroupDescription.Basic group) { public GroupControl controlFor(GroupDescription.Basic group) {
return new GroupControl(user.get(), group); return new GroupControl(user.get(), group, groupBackend);
} }
public GroupControl validateFor(final AccountGroup.Id groupId) public GroupControl validateFor(final AccountGroup.Id groupId)
@@ -110,10 +110,12 @@ public class GroupControl {
private final CurrentUser user; private final CurrentUser user;
private final GroupDescription.Basic group; private final GroupDescription.Basic group;
private Boolean isOwner; private Boolean isOwner;
private final GroupBackend groupBackend;
GroupControl(CurrentUser who, GroupDescription.Basic gd) { GroupControl(CurrentUser who, GroupDescription.Basic gd, GroupBackend gb) {
user = who; user = who;
group = gd; group = gd;
groupBackend = gb;
} }
public GroupDescription.Basic getGroup() { public GroupDescription.Basic getGroup() {
@@ -126,16 +128,15 @@ public class GroupControl {
/** Can this user see this group exists? */ /** Can this user see this group exists? */
public boolean isVisible() { public boolean isVisible() {
AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
/* Check for canAdministrateServer may seem redundant, but allows /* Check for canAdministrateServer may seem redundant, but allows
* for visibility of all groups that are not an internal group to * for visibility of all groups that are not an internal group to
* server administrators. * server administrators.
*/ */
return (accountGroup != null && accountGroup.isVisibleToAll()) return user instanceof InternalUser
|| user instanceof InternalUser
|| user.memberOf(group.getGroupUUID()) || user.memberOf(group.getGroupUUID())
|| isOwner() || isOwner()
|| user.getCapabilities().canAdministrateServer(); || user.getCapabilities().canAdministrateServer()
|| groupBackend.isVisibleToAll(group.getGroupUUID());
} }
public boolean isOwner() { public boolean isOwner() {

View File

@@ -90,4 +90,10 @@ public class InternalGroupBackend extends AbstractGroupBackend {
public GroupMembership membershipsOf(IdentifiedUser user) { public GroupMembership membershipsOf(IdentifiedUser user) {
return groupMembershipFactory.create(user); return groupMembershipFactory.create(user);
} }
@Override
public boolean isVisibleToAll(AccountGroup.UUID uuid) {
GroupDescription.Internal g = get(uuid);
return g != null && g.getAccountGroup().isVisibleToAll();
}
} }

View File

@@ -206,4 +206,14 @@ public class UniversalGroupBackend extends AbstractGroupBackend {
return groups; return groups;
} }
} }
@Override
public boolean isVisibleToAll(AccountGroup.UUID uuid) {
for (GroupBackend g : backends) {
if (g.handles(uuid)) {
return g.isVisibleToAll(uuid);
}
}
return false;
}
} }

View File

@@ -70,6 +70,7 @@ import javax.security.auth.login.LoginException;
private final String readTimeoutMillis; private final String readTimeoutMillis;
private final String connectTimeoutMillis; private final String connectTimeoutMillis;
private final boolean useConnectionPooling; private final boolean useConnectionPooling;
private final boolean groupsVisibleToAll;
@Inject @Inject
Helper(@GerritServerConfig final Config config, Helper(@GerritServerConfig final Config config,
@@ -81,6 +82,7 @@ import javax.security.auth.login.LoginException;
this.password = LdapRealm.optional(config, "password", ""); this.password = LdapRealm.optional(config, "password", "");
this.referral = LdapRealm.optional(config, "referral", "ignore"); this.referral = LdapRealm.optional(config, "referral", "ignore");
this.sslVerify = config.getBoolean("ldap", "sslverify", true); this.sslVerify = config.getBoolean("ldap", "sslverify", true);
this.groupsVisibleToAll = config.getBoolean("ldap", "groupsVisibleToAll", false);
this.authentication = this.authentication =
LdapRealm.optional(config, "authentication", "simple"); LdapRealm.optional(config, "authentication", "simple");
String readTimeout = LdapRealm.optional(config, "readTimeout"); String readTimeout = LdapRealm.optional(config, "readTimeout");
@@ -309,6 +311,10 @@ import javax.security.auth.login.LoginException;
} }
} }
public boolean groupsVisibleToAll() {
return this.groupsVisibleToAll;
}
class LdapSchema { class LdapSchema {
final LdapType type; final LdapType type;

View File

@@ -250,4 +250,9 @@ public class LdapGroupBackend extends AbstractGroupBackend {
} }
return out; return out;
} }
@Override
public boolean isVisibleToAll(AccountGroup.UUID uuid) {
return handles(uuid) && helper.groupsVisibleToAll();
}
} }