diff options
author | Shawn O. Pearce <sop@google.com> | 2010-08-21 17:43:54 -0700 |
---|---|---|
committer | Shawn O. Pearce <sop@google.com> | 2010-08-21 17:43:54 -0700 |
commit | cbfb04f0810f47fd4390c2aff0761f457479cd79 (patch) | |
tree | cdcbe24c870c92e92a9f8da43060ef19b9d180ce | |
parent | ccecfe4d5d407b0757dae6a95d03b7560b02ee59 (diff) |
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>
-rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java | 7 | ||||
-rw-r--r-- | gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java | 39 |
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()); + } + // ----------------------------------------------------------------------- |