opendev/gerrit
Code Issues Proposed changes

Remove isVisibleToAll from GroupDescription

Browse Source 
The notion of who can view a group is a security detail that
belongs delegated to the group backend and should not be part
of the public GroupDescription.Basic interface.

Drop it and move the code into the internal group backend only.

Change-Id: Ia967e957147860fe98d1a4d74b005d63f48e4343
This commit is contained in:
Shawn Pearce
2013-02-07 20:48:58 -08:00
parent 9021d245c8
commit b39b746f40
5 changed files with 8 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDescription.java
View File

@@ -32,9 +32,6 @@ public class GroupDescription {
/** @return the non-null name of the group. */
String getName();
/** @return whether the group is visible to all accounts. */
boolean isVisibleToAll();
/**
* @return optional email address to send to the group's members. If
* provided, Gerrit will use this email address to send

5
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDescriptions.java
View File

@@ -44,11 +44,6 @@ public class GroupDescriptions {
return group.getName();
}
@Override
public boolean isVisibleToAll() {
return group.isVisibleToAll();
}
@Override
public AccountGroup getAccountGroup() {
return group;

7
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
View File

@@ -103,7 +103,8 @@ public class GroupControl {
/** Can this user see this group exists? */
public boolean isVisible() {
return group.isVisibleToAll()
AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
return (accountGroup != null && accountGroup.isVisibleToAll())
|| user.getEffectiveGroups().contains(group.getGroupUUID())
|| isOwner();
}
@@ -149,6 +150,8 @@ public class GroupControl {
}
private boolean canSeeMembers() {
return group.isVisibleToAll() || isOwner();
AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
return (accountGroup != null && accountGroup.isVisibleToAll())
|| isOwner();
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
View File

@@ -149,11 +149,6 @@ public class LdapGroupBackend implements GroupBackend {
return name;
}
@Override
public boolean isVisibleToAll() {
return false;
}
@Override
@Nullable
public String getEmailAddress() {

4
gerrit-server/src/main/java/com/google/gerrit/server/group/GroupOptionsInfo.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.reviewdb.client.AccountGroup;
public class GroupOptionsInfo {
@@ -22,7 +23,8 @@ public class GroupOptionsInfo {
public Boolean visibleToAll;
public GroupOptionsInfo(GroupDescription.Basic group) {
visibleToAll = group.isVisibleToAll() ? true : null;
AccountGroup ag = GroupDescriptions.toAccountGroup(group);
visibleToAll = ag != null && ag.isVisibleToAll() ? true : null;
}
public GroupOptionsInfo(AccountGroup group) {