Simplify reference level access control.
The initial implementation of reference level access control only
supports a corner case, that of "locking down" access for a specific
branch.
Upon further discussion, we've decided that this is not the more
general need. Most Gerrit configurations prefer to have a more "open"
access model, where access rights on a reference specified with a
wildcard, such as "refs/heads/*" aren't overridden by a more specific
access right. So this change makes the default behavior to evaluate
all rights, including the wild card ones.
However, in order to accomodate the corner case we were supporting
before, this change also introduces a new way to specify exclusive
reference level access rights. All access rights that start with the
'-' prefix are considered exclusive, and will prevent all wild card
rights from being considered.
Change-Id: I629f5439967b2141e46098614fadb25ff28e5f45
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index b74fc1d..66b4082 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -140,26 +140,65 @@
`refs/heads/master` and `refs/heads/experimental`, etc.
When evaluating a reference-level access right, Gerrit will use
-the most specific set of access rights to determine if the user
-is allowed to perform a given action. For example, if a user
-tries to review a change destined for branch `refs/heads/qa`
-in project `tools/gerrit`, and the following ACLs are granted:
+the full set of access rights to determine if the user
+is allowed to perform a given action. For example, if a user is a
+member of `Foo Leads`, they are reviewing a change destined for
+the `refs/heads/qa` branch, and the following ACLs are granted
+on the project:
[grid="all"]
-`---------------`----------------`-------------`-------
-Group Reference Name Category Range
--------------------------------------------------------
-Anonymous Users refs/heads/* Code Review -1..+1
-Registered Users refs/heads/* Code Review -1..+1
-Foo Leads refs/heads/* Code Review -2..+2
-QA Leads refs/heads/qa Code Review -2..+2
--------------------------------------------------------
+`---------------`---------------`-------------`-------
+Group Reference Name Category Range
+------------------------------------------------------
+Registered Users refs/heads/* Code Review -1..+1
+Foo Leads refs/heads/* Code Review -2..+2
+QA Leads refs/heads/qa Code Review -2..+2
+------------------------------------------------------
-Then this user will have `Code Review -2..+2` if he is a member
-of `QA Leads`, and will not have any rights if not. Inherited ACLs
-from the `\-- All Projects \--` project thus allow system wide
-lock-down of a branch, by granting a permission to a limited group
-of users on that branch.
+Then the effective range permitted to be used by the user is
+`-2..+2`, as the user's membership of `Foo Leads` effectively grant
+them access to the entire reference space, thanks to the wildcard.
+
+Gerrit also supports exclusive reference-level access control.
+
+It is possible to configure Gerrit to grant an exclusive ref level
+access control so that only users of a specific group can perform
+an operation on a project/reference pair. This is done by prefixing
+the reference specified with a `'-'`.
+
+For example, if a user who is a member of `Foo Leads` tries to
+review a change destined for branch `refs/heads/qa` in a project,
+and the following ACLs are granted:
+
+[grid="all"]
+`---------------`----------------`--------------`-------
+Group Reference Name Category Range
+--------------------------------------------------------
+Registered Users refs/heads/* Code Review -1..+1
+Foo Leads refs/heads/* Code Review -2..+2
+QA Leads -refs/heads/qa Code Review -2..+2
+--------------------------------------------------------
+
+Then this user will not have `Code Review` rights on that change,
+since there is an exclusive access right in place for the
+`refs/heads/qa` branch. This allows locking down access for a
+particular branch to a limited set of users, bypassing inherited
+rights and wildcards.
+
+In order to grant the ability to `Code Review` to the members of
+`Foo Leads`, in `refs/heads/qa` then the following access rights
+would be needed:
+
+[grid="all"]
+`---------------`----------------`--------------`-------
+Group Reference Name Category Range
+--------------------------------------------------------
+Registered Users refs/heads/* Code Review -1..+1
+Foo Leads refs/heads/* Code Review -2..+2
+QA Leads -refs/heads/qa Code Review -2..+2
+Foo Leads refs/heads/qa Code Review -2..+2
+--------------------------------------------------------
+
OpenID Authentication
~~~~~~~~~~~~~~~~~~~~~
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectRightsPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectRightsPanel.java
index 80f1c38..75d6931 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectRightsPanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectRightsPanel.java
@@ -483,7 +483,7 @@
.get()));
}
- table.setText(row, 4, right.getRefPattern());
+ table.setText(row, 4, right.getRefPatternForDisplay());
{
final SafeHtmlBuilder m = new SafeHtmlBuilder();
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 d4a8cf5..c98d129 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
@@ -133,7 +133,7 @@
categoryRights.addAll(filterMatching(pe.getLocalRights(category)));
categoryRights.addAll(filterMatching(pe.getInheritedRights(category)));
Collections.sort(categoryRights, RefRight.REF_PATTERN_ORDER);
- categoryRights = RefControl.filterMostSpecific(categoryRights);
+
computeAllowed(am, categoryRights, category);
}
}
@@ -157,11 +157,27 @@
allowed.put(category, s);
}
+ boolean foundExclusive = false;
+ String previousPattern = "";
for (final RefRight r : list) {
+
if (!am.contains(r.getAccountGroupId())) {
+ if (r.isExclusive()) {
+ foundExclusive = true;
+ }
continue;
}
+ if (foundExclusive && !previousPattern.equals(r.getRefPattern())) {
+ break;
+ }
+
+ if (r.isExclusive()) {
+ foundExclusive = true;
+ }
+
+ previousPattern = r.getRefPattern();
+
final ApprovalType at =
approvalTypes.getApprovalType(r.getApprovalCategoryId());
for (short m = r.getMinValue(); m <= r.getMaxValue(); m++) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddRefRight.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddRefRight.java
index 1e39d5b..06f15c22 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddRefRight.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddRefRight.java
@@ -135,6 +135,12 @@
refPattern = RefRight.ALL;
}
}
+
+ boolean exclusive = refPattern.startsWith("-");
+ if (exclusive) {
+ refPattern = refPattern.substring(1);
+ }
+
while (refPattern.startsWith("/")) {
refPattern = refPattern.substring(1);
}
@@ -152,6 +158,10 @@
}
}
+ if (exclusive) {
+ refPattern = "-" + refPattern;
+ }
+
if (!controlForRef(projectControl, refPattern).isOwner()) {
throw new NoSuchRefException(refPattern);
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java
index 8e5445a..445c10a 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java
@@ -52,6 +52,7 @@
import org.kohsuke.args4j.Option;
+import java.io.Console;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
@@ -195,6 +196,11 @@
}
@Override
+ public boolean yesno(boolean def, String msg) {
+ return ui.yesno(def, msg);
+ }
+
+ @Override
public void pruneSchema(StatementExecutor e, List<String> prune) {
for (String p : prune) {
if (!pruneList.contains(p)) {
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 a3ba2f9..0db09e4 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
@@ -121,6 +121,13 @@
}
public String getRefPattern() {
+ if (isExclusive()) {
+ return key.refPattern.get().substring(1);
+ }
+ return key.refPattern.get();
+ }
+
+ public String getRefPatternForDisplay() {
return key.refPattern.get();
}
@@ -128,6 +135,10 @@
return getKey().getProjectNameKey();
}
+ public boolean isExclusive() {
+ return key.refPattern.get().startsWith("-");
+ }
+
public ApprovalCategory.Id getApprovalCategoryId() {
return key.categoryId;
}
@@ -153,6 +164,25 @@
}
@Override
+ public String toString() {
+ StringBuilder s = new StringBuilder();
+ s.append("{group :");
+ s.append(getAccountGroupId().get());
+ s.append(", proj :");
+ s.append(getProjectNameKey().get());
+ s.append(", cat :");
+ s.append(getApprovalCategoryId().get());
+ s.append(", pattern :");
+ s.append(getRefPatternForDisplay());
+ s.append(", min :");
+ s.append(getMinValue());
+ s.append(", max :");
+ s.append(getMaxValue());
+ s.append("}");
+ return s.toString();
+ }
+
+ @Override
public int hashCode() {
return getKey().hashCode();
}
@@ -169,19 +199,23 @@
return false;
}
- private static class RefPatternOrder implements Comparator<RefRight> {
+ public static final Comparator<RefRight> REF_PATTERN_ORDER =
+ new Comparator<RefRight>() {
@Override
public int compare(RefRight a, RefRight b) {
int aLength = a.getRefPattern().length();
int bLength = b.getRefPattern().length();
- if ((bLength - aLength) == 0) {
+ if (bLength == aLength) {
+ ApprovalCategory.Id aCat = a.getApprovalCategoryId();
+ ApprovalCategory.Id bCat = b.getApprovalCategoryId();
+ if (aCat.get().equals(bCat.get())) {
+ return a.getRefPattern().compareTo(b.getRefPattern());
+ }
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 973302c..afeb422 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
@@ -44,8 +44,11 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
/** Manages access control for Git references (aka branches, tags). */
@@ -225,6 +228,52 @@
return canPerform(FORGE_IDENTITY, FORGE_SERVER);
}
+ /**
+ * Convenience holder class used to map a ref pattern to the list of
+ * {@code RefRight}s that use it in the database.
+ */
+ public final static class RefRightsForPattern {
+ private final List<RefRight> rights;
+ private boolean containsExclusive;
+
+ public RefRightsForPattern() {
+ rights = new ArrayList<RefRight>();
+ containsExclusive = false;
+ }
+
+ public void addRight(RefRight right) {
+ rights.add(right);
+ if (right.isExclusive()) {
+ containsExclusive = true;
+ }
+ }
+
+ public List<RefRight> getRights() {
+ return Collections.unmodifiableList(rights);
+ }
+
+ public boolean containsExclusive() {
+ return containsExclusive;
+ }
+
+ /**
+ * Returns The max allowed value for this ref pattern for all specified
+ * groups.
+ *
+ * @param groups The groups of the user
+ * @return The allowed value for this ref for all the specified groups
+ */
+ public int allowedValueForRef(Set<AccountGroup.Id> groups) {
+ int val = Integer.MIN_VALUE;
+ for (RefRight right : rights) {
+ if (groups.contains(right.getAccountGroupId())) {
+ val = Math.max(right.getMaxValue(), val);
+ }
+ }
+ return val;
+ }
+ }
+
boolean canPerform(ApprovalCategory.Id actionId, short level) {
final Set<AccountGroup.Id> groups = getCurrentUser().getEffectiveGroups();
int val = Integer.MIN_VALUE;
@@ -236,42 +285,68 @@
allRights.addAll(getInheritedRights(actionId));
}
- // Sort in descending refPattern length
- Collections.sort(allRights, RefRight.REF_PATTERN_ORDER);
+ SortedMap<String, RefRightsForPattern> perPatternRights =
+ sortedRightsByPattern(allRights);
- for (RefRight right : filterMostSpecific(allRights)) {
- if (groups.contains(right.getAccountGroupId())) {
- val = Math.max(right.getMaxValue(), val);
+ for (RefRightsForPattern right : perPatternRights.values()) {
+ val = Math.max(val, right.allowedValueForRef(groups));
+ if (val >= level || right.containsExclusive()) {
+ return val >= level;
}
}
return val >= level;
}
- 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;
- }
+ public static final Comparator<String> DESCENDING_SORT =
+ new Comparator<String>() {
+
+ @Override
+ public int compare(String a, String b) {
+ int aLength = a.length();
+ int bLength = b.length();
+ if (bLength == aLength) {
+ return a.compareTo(b);
}
+ return bLength - aLength;
}
- return mostSpecific;
+ };
+
+ /**
+ * Sorts all given rights into a map, ordered by descending length of
+ * ref pattern.
+ *
+ * For example, if given the following rights in argument:
+ *
+ * ["refs/heads/master", group1, -1, +1],
+ * ["refs/heads/master", group2, -2, +2],
+ * ["refs/heads/*", group3, -1, +1]
+ * ["refs/heads/stable", group2, -1, +1]
+ *
+ * Then the following map is returned:
+ * "refs/heads/master" => {
+ * ["refs/heads/master", group1, -1, +1],
+ * ["refs/heads/master", group2, -2, +2]
+ * }
+ * "refs/heads/stable" => {["refs/heads/stable", group2, -1, +1]}
+ * "refs/heads/*" => {["refs/heads/*", group3, -1, +1]}
+ *
+ * @param actionRights
+ * @return A sorted map keyed off the ref pattern of all rights.
+ */
+ private static SortedMap<String, RefRightsForPattern> sortedRightsByPattern(
+ List<RefRight> actionRights) {
+ SortedMap<String, RefRightsForPattern> rights =
+ new TreeMap<String, RefRightsForPattern>(DESCENDING_SORT);
+ for (RefRight actionRight : actionRights) {
+ RefRightsForPattern patternRights =
+ rights.get(actionRight.getRefPattern());
+ if (patternRights == null) {
+ patternRights = new RefRightsForPattern();
+ rights.put(actionRight.getRefPattern(), patternRights);
+ }
+ patternRights.addRight(actionRight);
+ }
+ return rights;
}
private List<RefRight> getLocalRights(ApprovalCategory.Id actionId) {
@@ -282,12 +357,30 @@
return filter(getProjectState().getInheritedRights(actionId));
}
- public List<RefRight> getAllRights(final ApprovalCategory.Id id) {
+ /**
+ * Returns all applicable rights for a given approval category.
+ *
+ * Applicable rights are defined as the list of {@code RefRight}s which match
+ * the ref for which this object was created, stopping the ref wildcard
+ * matching when an exclusive ref right was encountered, for the given
+ * approval category.
+ * @param id The {@link ApprovalCategory.Id}.
+ * @return All applicalbe rights.
+ */
+ public List<RefRight> getApplicableRights(final ApprovalCategory.Id id) {
List<RefRight> l = new ArrayList<RefRight>();
l.addAll(getLocalRights(id));
l.addAll(getInheritedRights(id));
- Collections.sort(l, RefRight.REF_PATTERN_ORDER);
- return Collections.unmodifiableList(RefControl.filterMostSpecific(l));
+ SortedMap<String, RefRightsForPattern> perPatternRights =
+ sortedRightsByPattern(l);
+ List<RefRight> applicable = new ArrayList<RefRight>();
+ for (RefRightsForPattern patternRights : perPatternRights.values()) {
+ applicable.addAll(patternRights.getRights());
+ if (patternRights.containsExclusive()) {
+ break;
+ }
+ }
+ return Collections.unmodifiableList(applicable);
}
private List<RefRight> filter(Collection<RefRight> all) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index 2be0d49..9f578bc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -32,7 +32,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- private static final Class<? extends SchemaVersion> C = Schema_33.class;
+ private static final Class<? extends SchemaVersion> C = Schema_34.class;
public static class Module extends AbstractModule {
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_34.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_34.java
new file mode 100644
index 0000000..2cdb47e
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_34.java
@@ -0,0 +1,110 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.schema;
+
+import com.google.gerrit.reviewdb.ApprovalCategory;
+import com.google.gerrit.reviewdb.Project;
+import com.google.gerrit.reviewdb.RefRight;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.reviewdb.RefRight.RefPattern;
+import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.project.RefControl.RefRightsForPattern;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+
+public class Schema_34 extends SchemaVersion {
+ @Inject
+ Schema_34(Provider<Schema_33> prior) {
+ super(prior);
+ }
+
+
+ @Override
+ protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
+ Iterable<Project> projects = db.projects().all();
+ boolean showedBanner = false;
+
+ List<RefRight> toUpdate = new ArrayList<RefRight>();
+ List<RefRight> toDelete = new ArrayList<RefRight>();
+ for (Project p : projects) {
+ boolean showedProject = false;
+ List<RefRight> pr = db.refRights().byProject(p.getNameKey()).toList();
+ Map<ApprovalCategory.Id, Map<String, RefRightsForPattern>> r =
+ new HashMap<ApprovalCategory.Id, Map<String, RefRightsForPattern>>();
+ for (RefRight right : pr) {
+ ui.message(right.toString());
+ ApprovalCategory.Id cat = right.getApprovalCategoryId();
+ if (r.get(cat) == null) {
+ Map<String, RefRightsForPattern> m =
+ new TreeMap<String, RefRightsForPattern>(RefControl.DESCENDING_SORT);
+ r.put(cat, m);
+ }
+ if (r.get(cat).get(right.getRefPattern()) == null) {
+ RefRightsForPattern s = new RefRightsForPattern();
+ r.get(cat).put(right.getRefPattern(), s);
+ }
+ r.get(cat).get(right.getRefPattern()).addRight(right);
+ }
+
+ for (Map<String, RefRightsForPattern> categoryRights : r.values()) {
+ for (RefRightsForPattern rrp : categoryRights.values()) {
+ RefRight oldRight = rrp.getRights().get(0);
+ if (shouldPrompt(oldRight)) {
+ if (!showedBanner) {
+ ui.message("Entering interactive reference rights migration tool...");
+ showedBanner = true;
+ }
+ if (!showedProject) {
+ ui.message("In project " + p.getName());
+ showedProject = true;
+ }
+ ui.message("For category " + oldRight.getApprovalCategoryId());
+ boolean isWildcard = oldRight.getRefPattern().endsWith("/*");
+ boolean shouldUpdate = ui.yesno(!isWildcard,
+ "Should rights for pattern "
+ + oldRight.getRefPattern()
+ + " be considered exclusive?");
+ if (shouldUpdate) {
+ RefRight.Key newKey = new RefRight.Key(oldRight.getProjectNameKey(),
+ new RefPattern("-" + oldRight.getRefPattern()),
+ oldRight.getApprovalCategoryId(),
+ oldRight.getAccountGroupId());
+ RefRight newRight = new RefRight(newKey);
+ newRight.setMaxValue(oldRight.getMaxValue());
+ newRight.setMinValue(oldRight.getMinValue());
+ toUpdate.add(newRight);
+ toDelete.add(oldRight);
+ }
+ }
+ }
+ }
+ }
+ db.refRights().insert(toUpdate);
+ db.refRights().delete(toDelete);
+ }
+
+ private boolean shouldPrompt(RefRight right) {
+ return !right.getRefPattern().equals("refs/*")
+ && !right.getRefPattern().equals("refs/heads/*")
+ && !right.getRefPattern().equals("refs/tags/*");
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java
index f0109db..ac07efd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java
@@ -22,6 +22,8 @@
public interface UpdateUI {
void message(String msg);
+ boolean yesno(boolean def, String msg);
+
void pruneSchema(StatementExecutor e, List<String> pruneList)
throws OrmException;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
index 649a502..dd29f04 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
@@ -92,7 +92,7 @@
public boolean isValid(final CurrentUser user, final ApprovalType at,
final FunctionState state) {
RefControl rc = state.controlFor(user);
- for (final RefRight pr : rc.getAllRights(at.getCategory().getId())) {
+ for (final RefRight pr : rc.getApplicableRights(at.getCategory().getId())) {
if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
&& (pr.getMinValue() < 0 || pr.getMaxValue() > 0)) {
return true;
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 5bf39e1..36a52e2 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
@@ -146,7 +146,7 @@
// Find the maximal range actually granted to the user.
//
short minAllowed = 0, maxAllowed = 0;
- for (final RefRight r : rc.getAllRights(a.getCategoryId())) {
+ for (final RefRight r : rc.getApplicableRights(a.getCategoryId())) {
final AccountGroup.Id grp = r.getAccountGroupId();
if (user.getEffectiveGroups().contains(grp)) {
minAllowed = (short) Math.min(minAllowed, r.getMinValue());
@@ -154,8 +154,7 @@
}
}
- // Normalize the value into that range, returning true if we changed
- // the value.
+ // Normalize the value into that range.
//
if (a.getValue() < minAllowed) {
a.setValue(minAllowed);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java
index e097eeb..f0a00ff 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java
@@ -44,7 +44,7 @@
final FunctionState state) {
if (valid(at, state)) {
RefControl rc = state.controlFor(user);
- for (final RefRight pr : rc.getAllRights(at.getCategory().getId())) {
+ for (final RefRight pr : rc.getApplicableRights(at.getCategory().getId())) {
if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
&& pr.getMaxValue() > 0) {
return true;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java
index bbd8d77..1e01674 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java
@@ -67,6 +67,11 @@
}
@Override
+ public boolean yesno(boolean def, String msg) {
+ return def;
+ }
+
+ @Override
public void pruneSchema(StatementExecutor e, List<String> pruneList)
throws OrmException {
for (String sql : pruneList) {
