opendev/gerrit
Code Issues Proposed changes

Fix a possibility to overcome BLOCK permission

Browse Source 
This patch closes the possibility to overwrite BLOCK
permission from parent project by simply re-adding exactly
the same BLOCK entry followed by ALLOW entry in the child project.

The two new unit tests in RefControlTest show the problem.

Change-Id: I0bf0f0cd549185d72f8753d04f02d87d8709a3b2
Signed-off-by: Eryk Szymanski <eryksz@gmail.com>
This commit is contained in:
Eryk Szymanski
2014-05-15 01:05:18 +02:00
committed by David Pursehouse
parent fa6756cf1d
commit ad7e7a3c74
2 changed files with 27 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
View File

@@ -110,7 +110,6 @@ public class PermissionCollection {
sorter.sort(ref, sections);
Set<SeenRule> seen = new HashSet<SeenRule>();
Set<SeenRule> seenBlockingRules = new HashSet<SeenRule>();
Set<String> exclusiveGroupPermissions = new HashSet<String>();
HashMap<String, List<PermissionRule>> permissions =
@@ -126,7 +125,7 @@ public class PermissionCollection {
SeenRule s = new SeenRule(section, permission, rule);
boolean addRule;
if (rule.isBlock()) {
addRule = seenBlockingRules.add(s);
addRule = true;
} else {
addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists;
}

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

@@ -299,6 +299,15 @@ public class RefControlTest {
public void testBlockRule_ParentBlocksChild() {
grant(local, PUSH, DEVS, "refs/tags/*");
grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock();
ProjectControl u = util.user(local, DEVS);
assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate());
}
@Test
public void testBlockRule_ParentBlocksChildEvenIfAlreadyBlockedInChild() {
grant(local, PUSH, DEVS, "refs/tags/*");
grant(local, PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock();
grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock();
ProjectControl u = util.user(local, DEVS);
assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate());
@@ -318,6 +327,23 @@ public class RefControlTest {
assertFalse("u can't vote 2", range.contains(2));
}
@Test
public void testBlockLabelRange_ParentBlocksChildEvenIfAlreadyBlockedInChild() {
grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*");
grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*").setBlock();
grant(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS,
"refs/heads/*").setBlock();
ProjectControl u = util.user(local, DEVS);
PermissionRange range =
u.controlForRef("refs/heads/master").getRange(LABEL + "Code-Review");
assertTrue("u can vote -1", range.contains(-1));
assertTrue("u can vote +1", range.contains(1));
assertFalse("u can't vote -2", range.contains(-2));
assertFalse("u can't vote 2", range.contains(2));
}
@Test
public void testUnblockNoForce() {
grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/*").setBlock();