1 files changed, 42 insertions, 15 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
index 602b59338a..f7451a8488 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
@@ -15,9 +15,10 @@
 package com.google.gerrit.server.account;
 
 import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.reviewdb.Account;
-import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -39,11 +40,20 @@ public class GroupControl {
       if (group == null) {
         throw new NoSuchGroupException(groupId);
       }
-      return new GroupControl(user.get(), group);
+      return new GroupControl(groupCache, user.get(), group);
+    }
+
+    public GroupControl controlFor(final AccountGroup.UUID groupId)
+        throws NoSuchGroupException {
+      final AccountGroup group = groupCache.get(groupId);
+      if (group == null) {
+        throw new NoSuchGroupException(groupId);
+      }
+      return new GroupControl(groupCache, user.get(), group);
     }
 
     public GroupControl controlFor(final AccountGroup group) {
-      return new GroupControl(user.get(), group);
+      return new GroupControl(groupCache, user.get(), group);
     }
 
     public GroupControl validateFor(final AccountGroup.Id groupId)
@@ -56,10 +66,13 @@ public class GroupControl {
     }
   }
 
+  private final GroupCache groupCache;
   private final CurrentUser user;
   private final AccountGroup group;
+  private Boolean isOwner;
 
-  GroupControl(final CurrentUser who, final AccountGroup gc) {
+  GroupControl(GroupCache g, CurrentUser who, AccountGroup gc) {
+    groupCache = g;
     user = who;
     group = gc;
   }
@@ -74,36 +87,50 @@ public class GroupControl {
 
   /** Can this user see this group exists? */
   public boolean isVisible() {
-    return group.isVisibleToAll() || isOwner();
+    return group.isVisibleToAll()
+      || user.getEffectiveGroups().contains(group.getGroupUUID())
+      || isOwner();
   }
 
   public boolean isOwner() {
-    final AccountGroup.Id owner = group.getOwnerGroupId();
-    return getCurrentUser().getEffectiveGroups().contains(owner)
-        || getCurrentUser().isAdministrator();
+    if (isOwner == null) {
+      AccountGroup g = groupCache.get(group.getOwnerGroupId());
+      AccountGroup.UUID ownerUUID = g != null ? g.getGroupUUID() : null;
+      isOwner = getCurrentUser().getEffectiveGroups().contains(ownerUUID)
+             || getCurrentUser().getCapabilities().canAdministrateServer();
+    }
+    return isOwner;
   }
 
-  public boolean canAddMember(final Account.Id id) {
+  public boolean canAddMember(Account.Id id) {
     return isOwner();
   }
 
-  public boolean canRemoveMember(final Account.Id id) {
+  public boolean canRemoveMember(Account.Id id) {
     return isOwner();
   }
 
   public boolean canSeeMember(Account.Id id) {
-    return isVisible();
+    if (user instanceof IdentifiedUser
+        && ((IdentifiedUser) user).getAccountId().equals(id)) {
+      return true;
+    }
+    return canSeeMembers();
   }
 
-  public boolean canAddGroup(final AccountGroup.Id id) {
+  public boolean canAddGroup(AccountGroup.Id id) {
     return isOwner();
   }
 
-  public boolean canRemoveGroup(final AccountGroup.Id id) {
+  public boolean canRemoveGroup(AccountGroup.Id id) {
     return isOwner();
   }
 
   public boolean canSeeGroup(AccountGroup.Id id) {
-    return isVisible();
+    return canSeeMembers();
+  }
+
+  private boolean canSeeMembers() {
+    return group.isVisibleToAll() || isOwner();
   }
 }