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 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()); + } + // -----------------------------------------------------------------------