Merge "Consider rule action while constructing local owners list" into stable-2.9

3 files changed, 48 insertions, 5 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
index 50f232a7bf..a7a96f0fd6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.project;
 
+import static com.google.gerrit.common.data.PermissionRule.Action.ALLOW;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.base.Function;
@@ -141,7 +142,7 @@ public class ProjectState {
         if (owner != null) {
           for (PermissionRule rule : owner.getRules()) {
             GroupReference ref = rule.getGroup();
-            if (ref.getUUID() != null) {
+            if (rule.getAction() == ALLOW && ref.getUUID() != null) {
               groups.add(ref.getUUID());
             }
           }
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 74156dd226..059974f50d 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
@@ -25,6 +25,9 @@ import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.ADMIN;
 import static com.google.gerrit.server.project.Util.DEVS;
+import static com.google.gerrit.server.project.Util.allow;
+import static com.google.gerrit.server.project.Util.block;
+import static com.google.gerrit.server.project.Util.deny;
 import static com.google.gerrit.server.project.Util.doNotInherit;
 import static com.google.gerrit.server.project.Util.grant;
 import static com.google.gerrit.testutil.InMemoryRepositoryManager.newRepository;
@@ -70,11 +73,23 @@ public class RefControlTest {
   public void testOwnerProject() {
     grant(local, OWNER, ADMIN, "refs/*");
 
-    ProjectControl uBlah = util.user(local, DEVS);
-    ProjectControl uAdmin = util.user(local, DEVS, ADMIN);
+    assertAdminsAreOwnersAndDevsAreNot();
+  }
 
-    assertFalse("not owner", uBlah.isOwner());
-    assertTrue("is owner", uAdmin.isOwner());
+  @Test
+  public void testDenyOwnerProject() {
+    allow(local, OWNER, ADMIN, "refs/*");
+    deny(local, OWNER, DEVS, "refs/*");
+
+    assertAdminsAreOwnersAndDevsAreNot();
+  }
+
+  @Test
+  public void testBlockOwnerProject() {
+    allow(local, OWNER, ADMIN, "refs/*");
+    block(local, OWNER, DEVS, "refs/*");
+
+    assertAdminsAreOwnersAndDevsAreNot();
   }
 
   @Test
@@ -523,4 +538,12 @@ public class RefControlTest {
     assertFalse("u can vote -2", range.contains(-2));
     assertFalse("u can vote +2", range.contains(2));
   }
+
+  private void assertAdminsAreOwnersAndDevsAreNot() {
+    ProjectControl uBlah = util.user(local, DEVS);
+    ProjectControl uAdmin = util.user(local, DEVS, ADMIN);
+
+    assertFalse("not owner", uBlah.isOwner());
+    assertTrue("is owner", uAdmin.isOwner());
+  }
 }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
index e4de9ed7e8..ba2aeca02d 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
@@ -127,6 +127,25 @@ public class Util {
     return rule;
   }
 
+  public static PermissionRule allow(ProjectConfig project,
+      String permissionName, AccountGroup.UUID group, String ref) {
+    return grant(project, permissionName, newRule(project, group), ref);
+  }
+
+  public static PermissionRule block(ProjectConfig project,
+      String permissionName, AccountGroup.UUID group, String ref) {
+    PermissionRule r = grant(project, permissionName, newRule(project, group), ref);
+    r.setBlock();
+    return r;
+  }
+
+  public static PermissionRule deny(ProjectConfig project,
+      String permissionName, AccountGroup.UUID group, String ref) {
+    PermissionRule r = grant(project, permissionName, newRule(project, group), ref);
+    r.setDeny();
+    return r;
+  }
+
   private final Map<Project.NameKey, ProjectState> all;
   private final ProjectCache projectCache;
   private final CapabilityControl.Factory capabilityControlFactory;