opendev/gerrit
Code Issues Proposed changes

Fix inherited Read Access +2 not inheriting

Browse Source 
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>
This commit is contained in:
Shawn O. Pearce
2010-08-21 17:43:54 -07:00
parent ccecfe4d5d
commit cbfb04f081
2 changed files with 43 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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);

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

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