Merge "Introduced a new PermissionRule.Action: BLOCK."
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
index 6deb14f..2ddd8a7 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
@@ -16,7 +16,7 @@
public class PermissionRule implements Comparable<PermissionRule> {
public static enum Action {
- ALLOW, DENY,
+ ALLOW, DENY, BLOCK,
INTERACTIVE, BATCH;
}
@@ -53,6 +53,14 @@
action = Action.DENY;
}
+ public boolean isBlock() {
+ return action == Action.BLOCK;
+ }
+
+ public void setBlock() {
+ action = Action.BLOCK;
+ }
+
public Boolean getForce() {
return force;
}
@@ -97,7 +105,10 @@
void mergeFrom(PermissionRule src) {
if (getAction() != src.getAction()) {
- if (getAction() == Action.DENY || src.getAction() == Action.DENY) {
+ if (getAction() == Action.BLOCK || src.getAction() == Action.BLOCK) {
+ setAction(Action.BLOCK);
+
+ } else if (getAction() == Action.DENY || src.getAction() == Action.DENY) {
setAction(Action.DENY);
} else if (getAction() == Action.BATCH || src.getAction() == Action.BATCH) {
@@ -151,6 +162,10 @@
r.append("deny ");
break;
+ case BLOCK:
+ r.append("block ");
+ break;
+
case INTERACTIVE:
r.append("interactive ");
break;
@@ -189,6 +204,10 @@
rule.setAction(Action.DENY);
src = src.substring("deny ".length()).trim();
+ } else if (src.startsWith("block ")) {
+ rule.setAction(Action.BLOCK);
+ src = src.substring("block ".length()).trim();
+
} else if (src.startsWith("interactive ")) {
rule.setAction(Action.INTERACTIVE);
src = src.substring("interactive ".length()).trim();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
index 98e6283..7f5b573 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
@@ -124,7 +124,8 @@
action.setValue(PermissionRule.Action.ALLOW);
action.setAcceptableValues(Arrays.asList(
PermissionRule.Action.ALLOW,
- PermissionRule.Action.DENY));
+ PermissionRule.Action.DENY,
+ PermissionRule.Action.BLOCK));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
index 1196b02..02f56c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
@@ -99,19 +99,25 @@
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 =
new HashMap<String, List<PermissionRule>>();
for (AccessSection section : sections) {
for (Permission permission : section.getPermissions()) {
- if (exclusiveGroupPermissions.contains(permission.getName())) {
- continue;
- }
+ boolean exclusivePermissionExists =
+ exclusiveGroupPermissions.contains(permission.getName());
for (PermissionRule rule : permission.getRules()) {
SeenRule s = new SeenRule(section, permission, rule);
- if (seen.add(s) && !rule.isDeny()) {
+ boolean addRule = false;
+ if (rule.isBlock()) {
+ addRule = seenBlockingRules.add(s);
+ } else {
+ addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists;
+ }
+ if (addRule) {
List<PermissionRule> r = permissions.get(permission.getName());
if (r == null) {
r = new ArrayList<PermissionRule>(2);
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 1024f9a..ee42833 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
@@ -19,6 +19,7 @@
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.reviewdb.AbstractAgreement;
import com.google.gerrit.reviewdb.AccountAgreement;
import com.google.gerrit.reviewdb.AccountGroup;
@@ -393,7 +394,7 @@
}
for (PermissionRule rule : permission.getRules()) {
- if (rule.isDeny() || !match(rule)) {
+ if (rule.isBlock() || rule.isDeny() || !match(rule)) {
continue;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 984623d..9c60133 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -163,12 +163,16 @@
// granting of powers beyond pushing to the configuration.
return false;
}
+ boolean result = false;
for (PermissionRule rule : access(Permission.PUSH)) {
+ if (rule.isBlock()) {
+ return false;
+ }
if (rule.getForce()) {
- return true;
+ result = true;
}
}
- return false;
+ return result;
}
/**
@@ -313,16 +317,35 @@
List<PermissionRule> ruleList) {
int min = 0;
int max = 0;
+ int blockMin = Integer.MIN_VALUE;
+ int blockMax = Integer.MAX_VALUE;
for (PermissionRule rule : ruleList) {
- min = Math.min(min, rule.getMin());
- max = Math.max(max, rule.getMax());
+ if (rule.isBlock()) {
+ blockMin = Math.max(blockMin, rule.getMin());
+ blockMax = Math.min(blockMax, rule.getMax());
+ } else {
+ min = Math.min(min, rule.getMin());
+ max = Math.max(max, rule.getMax());
+ }
+ }
+ if (blockMin > Integer.MIN_VALUE) {
+ min = Math.max(min, blockMin + 1);
+ }
+ if (blockMax < Integer.MAX_VALUE) {
+ max = Math.min(max, blockMax - 1);
}
return new PermissionRange(permissionName, min, max);
}
/** True if the user has this permission. Works only for non labels. */
boolean canPerform(String permissionName) {
- return !access(permissionName).isEmpty();
+ List<PermissionRule> access = access(permissionName);
+ for (PermissionRule rule : access) {
+ if (rule.isBlock() && !rule.getForce()) {
+ return false;
+ }
+ }
+ return !access.isEmpty();
}
/** Rules for the given permission, or the empty list. */
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 f829ba0..29b2762 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
@@ -14,6 +14,7 @@
package com.google.gerrit.server.project;
+import static com.google.gerrit.common.data.Permission.LABEL;
import static com.google.gerrit.common.data.Permission.OWNER;
import static com.google.gerrit.common.data.Permission.PUSH;
import static com.google.gerrit.common.data.Permission.READ;
@@ -21,6 +22,7 @@
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.AccountProjectWatch;
@@ -221,6 +223,26 @@
assertTrue("d can read", d.controlForRef("refs/heads/foo-QA-bar").isVisible());
}
+ public void testBlockRule_ParentBlocksChild() {
+ grant(local, PUSH, devs, "refs/tags/*");
+ grant(parent, PUSH, anonymous, "refs/tags/*").setBlock();
+
+ ProjectControl u = user(devs);
+ assertFalse("u can't force update tag", u.controlForRef("refs/tags/V10").canForceUpdate());
+ }
+
+ public void testBlockLabelRange_ParentBlocksChild() {
+ grant(local, LABEL + "Code-Review", -2, +2, devs, "refs/heads/*");
+ grant(parent, LABEL + "Code-Review", -2, +2, devs, "refs/heads/*").setBlock();
+
+ ProjectControl u = user(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));
+ }
// -----------------------------------------------------------------------
private final Map<Project.NameKey, ProjectState> all;
@@ -317,7 +339,20 @@
private PermissionRule grant(ProjectConfig project, String permissionName,
AccountGroup.UUID group, String ref) {
+ return grant(project, permissionName, newRule(project, group), ref);
+ }
+
+ private PermissionRule grant(ProjectConfig project, String permissionName,
+ int min, int max, AccountGroup.UUID group, String ref) {
PermissionRule rule = newRule(project, group);
+ rule.setMin(min);
+ rule.setMax(max);
+ return grant(project, permissionName, rule, ref);
+ }
+
+
+ private PermissionRule grant(ProjectConfig project, String permissionName,
+ PermissionRule rule, String ref) {
project.getAccessSection(ref, true) //
.getPermission(permissionName, true) //
.add(rule);
