Block inheritance by default on per-branch permissions.
This change permits a single RefRight to block an ApprovalCategory for a given
ref. In other words, if there happens to be a change going to the project
tools/gerrit and that change is for the ref /refs/heads/master, given
the following rights:
in -- All Projects --:
Administrators Code Review +2 refs/heads/*
in tools/gerrit:
gerrit-devs Code Review +2 refs/heads/*
Dr. No Code Review +2 refs/heads/master
Then the change can only be Code Review +2 by members of the Dr. No group.
Change-Id: Ib72ac3cc7131c40d4209e438c70ae3e2b08e5830
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java
index 53fe746..179a571 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java
@@ -42,7 +42,9 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -126,19 +128,31 @@
private void computeAllowed() {
final Set<AccountGroup.Id> am = user.getEffectiveGroups();
final ProjectState pe = projectCache.get(change.getProject());
- computeAllowed(am, pe.getLocalRights());
- computeAllowed(am, pe.getInheritedRights());
+ List<RefRight> allRights = new ArrayList<RefRight>();
+ allRights.addAll(filterMatching(pe.getLocalRights()));
+ allRights.addAll(filterMatching(pe.getInheritedRights()));
+ Collections.sort(allRights, RefRight.REF_PATTERN_ORDER);
+ allRights = RefControl.filterMostSpecific(allRights);
+ computeAllowed(am, allRights);
+ }
+
+ private List<RefRight> filterMatching(Collection<RefRight> rights) {
+ List<RefRight> result = new ArrayList<RefRight>();
+ for (RefRight right : rights) {
+ if (RefControl.matches(change.getDest().get(), right.getRefPattern())) {
+ result.add(right);
+ }
+ }
+ return result;
}
private void computeAllowed(final Set<AccountGroup.Id> am,
- final Collection<RefRight> list) {
+ final List<RefRight> list) {
+
for (final RefRight r : list) {
if (!am.contains(r.getAccountGroupId())) {
continue;
}
- if (!RefControl.matches(change.getDest().get(), r.getRefPattern())) {
- continue;
- }
Set<ApprovalCategoryValue.Id> s = allowed.get(r.getApprovalCategoryId());
if (s == null) {
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java
index 6994a60..301da87 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java
@@ -18,6 +18,8 @@
import com.google.gwtorm.client.CompoundKey;
import com.google.gwtorm.client.StringKey;
+import java.util.Comparator;
+
/** Grant to use an {@link ApprovalCategory} in the scope of a git ref. */
public final class RefRight {
public static class RefPattern extends
@@ -146,4 +148,20 @@
public void setMaxValue(final short m) {
maxValue = m;
}
+
+ private static class RefPatternOrder implements Comparator<RefRight> {
+
+ @Override
+ public int compare(RefRight a, RefRight b) {
+ int aLength = a.getRefPattern().length();
+ int bLength = b.getRefPattern().length();
+ if ((bLength - aLength) == 0) {
+ return a.getApprovalCategoryId().get()
+ .compareTo(b.getApprovalCategoryId().get());
+ }
+ return bLength - aLength;
+ }
+ }
+
+ public static final RefPatternOrder REF_PATTERN_ORDER = new RefPatternOrder();
}
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 32df41e..74dc7a2 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
@@ -40,6 +40,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
@@ -183,9 +184,19 @@
private boolean canPerform(ApprovalCategory.Id actionId, short level) {
final Set<AccountGroup.Id> groups = getCurrentUser().getEffectiveGroups();
int val = Integer.MIN_VALUE;
- for (final RefRight right : getLocalRights()) {
- if (right.getApprovalCategoryId().equals(actionId)
- && groups.contains(right.getAccountGroupId())) {
+
+ List<RefRight> allRights = new ArrayList<RefRight>();
+ allRights.addAll(getLocalRights(actionId));
+
+ if (actionId.canInheritFromWildProject()) {
+ allRights.addAll(getInheritedRights(actionId));
+ }
+
+ // Sort in descending refPattern length
+ Collections.sort(allRights, RefRight.REF_PATTERN_ORDER);
+
+ for (RefRight right : filterMostSpecific(allRights)) {
+ if (groups.contains(right.getAccountGroupId())) {
if (val < 0 && right.getMaxValue() > 0) {
// If one of the user's groups had denied them access, but
// this group grants them access, prefer the grant over
@@ -200,31 +211,50 @@
}
}
}
-
- if (val == Integer.MIN_VALUE && actionId.canInheritFromWildProject()) {
- for (final RefRight pr : getInheritedRights()) {
- if (actionId.equals(pr.getApprovalCategoryId())
- && groups.contains(pr.getAccountGroupId())) {
- val = Math.max(pr.getMaxValue(), val);
- }
- }
- }
-
return val >= level;
}
- private Collection<RefRight> getLocalRights() {
- return filter(projectControl.getProjectState().getLocalRights());
+ public static List<RefRight> filterMostSpecific(List<RefRight> actionRights) {
+ // Grab the first set of RefRight which have the same refPattern
+ // those are the most specific RefRights we have, and are the
+ // we will consider to verify if this action can be performed.
+ // We do this so that one can override the ref rights for a specific
+ // project on a specific branch
+ boolean sameRefPattern = true;
+ List<RefRight> mostSpecific = new ArrayList<RefRight>();
+ String currentRefPattern = null;
+ int i = 0;
+ while (sameRefPattern && i < actionRights.size()) {
+ if (currentRefPattern == null) {
+ currentRefPattern = actionRights.get(i).getRefPattern();
+ mostSpecific.add(actionRights.get(i));
+ i++;
+ } else {
+ if (currentRefPattern.equals(actionRights.get(i).getRefPattern())) {
+ mostSpecific.add(actionRights.get(i));
+ i++;
+ } else {
+ sameRefPattern = false;
+ }
+ }
+ }
+ return mostSpecific;
}
- private Collection<RefRight> getInheritedRights() {
- return filter(projectControl.getProjectState().getInheritedRights());
+ private List<RefRight> getLocalRights(ApprovalCategory.Id actionId) {
+ return filter(projectControl.getProjectState().getLocalRights(), actionId);
}
- private Collection<RefRight> filter(Collection<RefRight> all) {
+ private List<RefRight> getInheritedRights(ApprovalCategory.Id actionId) {
+ return filter(projectControl.getProjectState().getInheritedRights(), actionId);
+ }
+
+ private List<RefRight> filter(Collection<RefRight> all,
+ ApprovalCategory.Id actionId) {
List<RefRight> mine = new ArrayList<RefRight>(all.size());
for (RefRight right : all) {
- if (matches(getRefName(), right.getRefPattern())) {
+ if (matches(getRefName(), right.getRefPattern()) &&
+ right.getApprovalCategoryId().equals(actionId)) {
mine.add(right);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
index 0b85db1..63b3883 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
@@ -58,10 +58,10 @@
new HashMap<ApprovalCategory.Id, Boolean>();
private final Change change;
private final ProjectState project;
- private final Map<ApprovalCategory.Id, Collection<RefRight>> allRights =
- new HashMap<ApprovalCategory.Id, Collection<RefRight>>();
- private Map<ApprovalCategory.Id, Collection<RefRight>> RefRights;
- private Map<ApprovalCategory.Id, Collection<RefRight>> inheritedRights;
+ private final Map<ApprovalCategory.Id, List<RefRight>> allRights =
+ new HashMap<ApprovalCategory.Id, List<RefRight>>();
+ private Map<ApprovalCategory.Id, List<RefRight>> refRights;
+ private Map<ApprovalCategory.Id, List<RefRight>> inheritedRights;
private Set<PatchSetApproval> modified;
@Inject
@@ -136,53 +136,46 @@
return Collections.emptySet();
}
- public Collection<RefRight> getRefRights(final ApprovalType at) {
- return getRefRights(id(at));
- }
-
- public Collection<RefRight> getRefRights(final ApprovalCategory.Id id) {
- if (RefRights == null) {
- RefRights = index(project.getLocalRights());
+ private List<RefRight> getRefRights(final ApprovalCategory.Id id) {
+ if (refRights == null) {
+ refRights = index(project.getLocalRights());
}
- final Collection<RefRight> l = RefRights.get(id);
- return l != null ? l : Collections.<RefRight> emptySet();
+ final List<RefRight> l = refRights.get(id);
+ return l != null ? l : Collections.<RefRight> emptyList();
}
- public Collection<RefRight> getWildcardRights(final ApprovalType at) {
- return getWildcardRights(id(at));
- }
-
- public Collection<RefRight> getWildcardRights(final ApprovalCategory.Id id) {
+ private List<RefRight> getWildcardRights(final ApprovalCategory.Id id) {
if (inheritedRights == null) {
inheritedRights = index(project.getInheritedRights());
}
- final Collection<RefRight> l = inheritedRights.get(id);
- return l != null ? l : Collections.<RefRight> emptySet();
+ final List<RefRight> l = inheritedRights.get(id);
+ return l != null ? l : Collections.<RefRight> emptyList();
}
public Collection<RefRight> getAllRights(final ApprovalType at) {
return getAllRights(id(at));
}
- public Collection<RefRight> getAllRights(final ApprovalCategory.Id id) {
- Collection<RefRight> l = allRights.get(id);
+ public List<RefRight> getAllRights(final ApprovalCategory.Id id) {
+ List<RefRight> l = allRights.get(id);
if (l == null) {
l = new ArrayList<RefRight>();
l.addAll(getRefRights(id));
l.addAll(getWildcardRights(id));
- l = Collections.unmodifiableCollection(l);
+ Collections.sort(l, RefRight.REF_PATTERN_ORDER);
+ l = Collections.unmodifiableList(RefControl.filterMostSpecific(l));
allRights.put(id, l);
}
return l;
}
- private Map<Id, Collection<RefRight>> index(final Collection<RefRight> rights) {
- final HashMap<ApprovalCategory.Id, Collection<RefRight>> r;
+ private Map<Id, List<RefRight>> index(final Collection<RefRight> rights) {
+ final HashMap<ApprovalCategory.Id, List<RefRight>> r;
- r = new HashMap<ApprovalCategory.Id, Collection<RefRight>>();
+ r = new HashMap<ApprovalCategory.Id, List<RefRight>>();
for (final RefRight pr : rights) {
if (RefControl.matches(change.getDest().get(), pr.getRefPattern())) {
- Collection<RefRight> l = r.get(pr.getApprovalCategoryId());
+ List<RefRight> l = r.get(pr.getApprovalCategoryId());
if (l == null) {
l = new ArrayList<RefRight>();
r.put(pr.getApprovalCategoryId(), l);
