Fix inherited Read Access +2 not inheriting

Upload was rejected to a project if the user has upload permission
inherited from a parent, and there is a local reference right that
applied to a different branch and refused upload:

  --All Projects--  refs/*             Registered  1..2
  test-project      refs/heads/foobar  Registered  1..1

Uploads to "refs/heads/master" in test-project should still work,
but should deny to "refs/heads/foobar".

Bug: issue 668
Change-Id: I4ff6c02918990b36447186c569ec95f0db21e3ac
Signed-off-by: Shawn O. Pearce <sop@google.com>

2 files changed, 43 insertions, 3 deletions

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 67f543b947..ff788dd089 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
@@ -177,16 +177,17 @@ public class ProjectControl {
       short requireValue) {
     final Set<AccountGroup.Id> groups = user.getEffectiveGroups();
     int val = Integer.MIN_VALUE;
-    boolean local = false;
 
     for (final RefRight pr : state.getLocalRights(actionId)) {
       if (groups.contains(pr.getAccountGroupId())) {
         val = Math.max(pr.getMaxValue(), val);
-        local = true;
       }
     }
+    if (val >= requireValue) {
+      return true;
+    }
 
-    if (!local && actionId.canInheritFromWildProject()) {
+    if (actionId.canInheritFromWildProject()) {
       for (final RefRight pr : state.getInheritedRights(actionId)) {
         if (groups.contains(pr.getAccountGroupId())) {
           val = Math.max(pr.getMaxValue(), val);
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 c789feae45..8c26705226 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
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.project;
 
 import static com.google.gerrit.reviewdb.ApprovalCategory.OWN;
+import static com.google.gerrit.reviewdb.ApprovalCategory.READ;
 
 import com.google.gerrit.reviewdb.AccountGroup;
 import com.google.gerrit.reviewdb.AccountProjectWatch;
@@ -101,6 +102,44 @@ public class RefControlTest extends TestCase {
     assertNotOwner("refs/heads/master", uFix);
   }
 
+  public void testInheritRead_SingleBranchDeniesUpload() {
+    inherited.add(grant(READ, registered, "refs/*", 1, 2));
+    local.add(grant(READ, registered, "-refs/heads/foobar", 1, 1));
+
+    ProjectControl u = user();
+    assertTrue("can upload", u.canUploadToAtLeastOneRef());
+
+    assertTrue("can upload refs/heads/master", //
+        u.controlForRef("refs/heads/master").canUpload());
+
+    assertFalse("deny refs/heads/foobar", //
+        u.controlForRef("refs/heads/foobar").canUpload());
+  }
+
+  public void testInheritRead_SingleBranchDoesNotOverrideInherited() {
+    inherited.add(grant(READ, registered, "refs/*", 1, 2));
+    local.add(grant(READ, registered, "refs/heads/foobar", 1, 1));
+
+    ProjectControl u = user();
+    assertTrue("can upload", u.canUploadToAtLeastOneRef());
+
+    assertTrue("can upload refs/heads/master", //
+        u.controlForRef("refs/heads/master").canUpload());
+
+    assertTrue("can upload refs/heads/foobar", //
+        u.controlForRef("refs/heads/foobar").canUpload());
+  }
+
+  public void testCannotUploadToAnyRef() {
+    inherited.add(grant(READ, registered, "refs/*", 1, 1));
+    local.add(grant(READ, devs, "refs/heads/*",1,2));
+
+    ProjectControl u = user();
+    assertFalse("cannot upload", u.canUploadToAtLeastOneRef());
+    assertFalse("cannot upload refs/heads/master", //
+        u.controlForRef("refs/heads/master").canUpload());
+  }
+
 
   // -----------------------------------------------------------------------