From cddd84b60cc65aaa21a88b9092035f0bf58139f3 Mon Sep 17 00:00:00 2001 From: Nico Sallembien Date: Wed, 27 Jan 2010 17:34:26 -0800 Subject: [PATCH] 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 --- .../PatchSetPublishDetailFactory.java | 26 +++++-- .../com/google/gerrit/reviewdb/RefRight.java | 18 +++++ .../gerrit/server/project/RefControl.java | 68 +++++++++++++------ .../gerrit/server/workflow/FunctionState.java | 47 ++++++------- 4 files changed, 107 insertions(+), 52 deletions(-) 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 53fe746245..179a571562 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.gwtorm.client.OrmException; 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 @@ final class PatchSetPublishDetailFactory extends Handler private void computeAllowed() { final Set am = user.getEffectiveGroups(); final ProjectState pe = projectCache.get(change.getProject()); - computeAllowed(am, pe.getLocalRights()); - computeAllowed(am, pe.getInheritedRights()); + List allRights = new ArrayList(); + 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 filterMatching(Collection rights) { + List result = new ArrayList(); + for (RefRight right : rights) { + if (RefControl.matches(change.getDest().get(), right.getRefPattern())) { + result.add(right); + } + } + return result; } private void computeAllowed(final Set am, - final Collection list) { + final List list) { + for (final RefRight r : list) { if (!am.contains(r.getAccountGroupId())) { continue; } - if (!RefControl.matches(change.getDest().get(), r.getRefPattern())) { - continue; - } Set 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 6994a60115..301da87cc1 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.Column; 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 final class RefRight { public void setMaxValue(final short m) { maxValue = m; } + + private static class RefPatternOrder implements Comparator { + + @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 32df41e374..74dc7a241d 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 org.eclipse.jgit.revwalk.RevWalk; 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 @@ public class RefControl { private boolean canPerform(ApprovalCategory.Id actionId, short level) { final Set groups = getCurrentUser().getEffectiveGroups(); int val = Integer.MIN_VALUE; - for (final RefRight right : getLocalRights()) { - if (right.getApprovalCategoryId().equals(actionId) - && groups.contains(right.getAccountGroupId())) { + + List allRights = new ArrayList(); + 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 @@ public class RefControl { } } } - - 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 getLocalRights() { - return filter(projectControl.getProjectState().getLocalRights()); + public static List filterMostSpecific(List 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 mostSpecific = new ArrayList(); + 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 getInheritedRights() { - return filter(projectControl.getProjectState().getInheritedRights()); + private List getLocalRights(ApprovalCategory.Id actionId) { + return filter(projectControl.getProjectState().getLocalRights(), actionId); } - private Collection filter(Collection all) { + private List getInheritedRights(ApprovalCategory.Id actionId) { + return filter(projectControl.getProjectState().getInheritedRights(), actionId); + } + + private List filter(Collection all, + ApprovalCategory.Id actionId) { List mine = new ArrayList(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 0b85db1ce5..63b3883f5c 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 @@ public class FunctionState { new HashMap(); private final Change change; private final ProjectState project; - private final Map> allRights = - new HashMap>(); - private Map> RefRights; - private Map> inheritedRights; + private final Map> allRights = + new HashMap>(); + private Map> refRights; + private Map> inheritedRights; private Set modified; @Inject @@ -136,53 +136,46 @@ public class FunctionState { return Collections.emptySet(); } - public Collection getRefRights(final ApprovalType at) { - return getRefRights(id(at)); - } - - public Collection getRefRights(final ApprovalCategory.Id id) { - if (RefRights == null) { - RefRights = index(project.getLocalRights()); + private List getRefRights(final ApprovalCategory.Id id) { + if (refRights == null) { + refRights = index(project.getLocalRights()); } - final Collection l = RefRights.get(id); - return l != null ? l : Collections. emptySet(); + final List l = refRights.get(id); + return l != null ? l : Collections. emptyList(); } - public Collection getWildcardRights(final ApprovalType at) { - return getWildcardRights(id(at)); - } - - public Collection getWildcardRights(final ApprovalCategory.Id id) { + private List getWildcardRights(final ApprovalCategory.Id id) { if (inheritedRights == null) { inheritedRights = index(project.getInheritedRights()); } - final Collection l = inheritedRights.get(id); - return l != null ? l : Collections. emptySet(); + final List l = inheritedRights.get(id); + return l != null ? l : Collections. emptyList(); } public Collection getAllRights(final ApprovalType at) { return getAllRights(id(at)); } - public Collection getAllRights(final ApprovalCategory.Id id) { - Collection l = allRights.get(id); + public List getAllRights(final ApprovalCategory.Id id) { + List l = allRights.get(id); if (l == null) { l = new ArrayList(); 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> index(final Collection rights) { - final HashMap> r; + private Map> index(final Collection rights) { + final HashMap> r; - r = new HashMap>(); + r = new HashMap>(); for (final RefRight pr : rights) { if (RefControl.matches(change.getDest().get(), pr.getRefPattern())) { - Collection l = r.get(pr.getApprovalCategoryId()); + List l = r.get(pr.getApprovalCategoryId()); if (l == null) { l = new ArrayList(); r.put(pr.getApprovalCategoryId(), l);