commit 1199891854bfce499b40c3611b026c025cc30816
author Edwin Kempin <edwin.kempin@sap.com> Fri Jun 03 09:12:13 2011 +0200
committer Edwin Kempin <edwin.kempin@sap.com> Fri Jun 03 09:12:13 2011 +0200
tree 4d358ba29f25b25dc90eb7e84b66a7d5fa6a2b7a
parent e43d3c46693564119ab10b232ac0ccdc24d598f5

Resolve Project Owners when checking access right on any ref

A project is visible to a user when the user has Read Access on any
ref of the project. This check whether an access right is granted on
any ref is not taking the access rights granted to the
'Project Owners' group into account. As a result a user does not see
a project if he is project owner and Read Access is only granted to
the 'Project Owners' group. This change ensures that the
'Project Owners' group is properly resolved in this case.

Bug: issue 997
Change-Id: I27cd8293e5c4a01c867a4e076073bf587294e0ba
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 25778a6..2a55019 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -21,6 +21,7 @@
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.reviewdb.RefRight;
+import com.google.gerrit.reviewdb.SystemConfig;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.ReplicationUser;
 import com.google.gerrit.server.config.GitReceivePackGroups;
@@ -29,6 +30,7 @@
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -101,6 +103,7 @@
     ProjectControl create(CurrentUser who, ProjectState ps);
   }
 
+  private final SystemConfig systemConfig;
   private final Set<AccountGroup.Id> uploadGroups;
   private final Set<AccountGroup.Id> receiveGroups;
 
@@ -109,10 +112,12 @@
   private final ProjectState state;
 
   @Inject
-  ProjectControl(@GitUploadPackGroups Set<AccountGroup.Id> uploadGroups,
+  ProjectControl(final SystemConfig systemConfig,
+      @GitUploadPackGroups Set<AccountGroup.Id> uploadGroups,
       @GitReceivePackGroups Set<AccountGroup.Id> receiveGroups,
       final RefControl.Factory refControlFactory,
       @Assisted CurrentUser who, @Assisted ProjectState ps) {
+    this.systemConfig = systemConfig;
     this.uploadGroups = uploadGroups;
     this.receiveGroups = receiveGroups;
     this.refControlFactory = refControlFactory;
@@ -197,7 +202,7 @@
   // TODO (anatol.pomazau): Try to merge this method with similar RefRightsForPattern#canPerform
   private boolean canPerformOnAnyRef(ApprovalCategory.Id actionId,
       short requireValue) {
-    final Set<AccountGroup.Id> groups = user.getEffectiveGroups();
+    final Set<AccountGroup.Id> groups = getEffectiveUserGroups();
 
     for (final RefRight pr : state.getAllRights(actionId, true)) {
       if (groups.contains(pr.getAccountGroupId())
@@ -209,6 +214,22 @@
     return false;
   }
 
+  /**
+   * @return the effective groups of the current user for this project
+   */
+  private Set<AccountGroup.Id> getEffectiveUserGroups() {
+    final Set<AccountGroup.Id> userGroups = user.getEffectiveGroups();
+    if (isOwner()) {
+      final Set<AccountGroup.Id> userGroupsOnProject =
+          new HashSet<AccountGroup.Id>(userGroups.size() + 1);
+      userGroupsOnProject.addAll(userGroups);
+      userGroupsOnProject.add(systemConfig.ownerGroupId);
+      return Collections.unmodifiableSet(userGroupsOnProject);
+    } else {
+      return userGroups;
+    }
+  }
+
   private boolean canPerformOnAllRefs(ApprovalCategory.Id actionId,
       short requireValue) {
     boolean canPerform = false;
@@ -238,10 +259,10 @@
   }
 
   public boolean canRunUploadPack() {
-    return isAnyIncludedIn(uploadGroups, user.getEffectiveGroups());
+    return isAnyIncludedIn(uploadGroups, getEffectiveUserGroups());
   }
 
   public boolean canRunReceivePack() {
-    return isAnyIncludedIn(receiveGroups, user.getEffectiveGroups());
+    return isAnyIncludedIn(receiveGroups, getEffectiveUserGroups());
   }
 }

gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index cf5d264..6b0f13a 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -277,7 +277,8 @@
         return new RefControl(systemConfig, projectControl, ref);
       }
     };
-    return new ProjectControl(Collections.<AccountGroup.Id> emptySet(),
+    return new ProjectControl(systemConfig,
+        Collections.<AccountGroup.Id> emptySet(),
         Collections.<AccountGroup.Id> emptySet(), refControlFactory,
         new MockUser(memberOf), newProjectState());
   }