Merge remote branch 'korg/master' into exp-nosql

* korg/master:
  Simplify reference level access control.

Conflicts:
	gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

Change-Id: I396c85f1730ea7173f76e7951fcab76ee0ecdbda

Documentation/access-control.txt

@@ -140,26 +140,65 @@ prefix. So a permission with `refs/heads/*` will match
 `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
+the full set of access rights to determine if the user
-is allowed to perform a given action. For example, if a user
+is allowed to perform a given action. For example, if a user is a
-tries to review a change destined for branch `refs/heads/qa`
+member of `Foo Leads`, they are reviewing a change destined for
-in project `tools/gerrit`, and the following ACLs are granted:
+the `refs/heads/qa` branch, and the following ACLs are granted
+on the project:
 
 [grid="all"]
-`---------------`----------------`-------------`-------
+`---------------`---------------`-------------`-------
-Group            Reference Name  Category       Range
+Group            Reference Name Category       Range
--------------------------------------------------------
+------------------------------------------------------
-Anonymous Users  refs/heads/*    Code Review     -1..+1
+Registered Users refs/heads/*   Code Review     -1..+1
-Registered Users refs/heads/*    Code Review     -1..+1
+Foo Leads        refs/heads/*   Code Review     -2..+2
-Foo Leads        refs/heads/*    Code Review     -2..+2
+QA Leads         refs/heads/qa  Code Review     -2..+2
-QA Leads         refs/heads/qa   Code Review     -2..+2
+------------------------------------------------------
--------------------------------------------------------
+
+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
+--------------------------------------------------------
 
-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.
 
 OpenID Authentication
 ~~~~~~~~~~~~~~~~~~~~~

gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectRightsPanel.java

@@ -483,7 +483,7 @@ public class ProjectRightsPanel extends Composite {
             .get()));
       }
 
-      table.setText(row, 4, right.getRefPattern());
+      table.setText(row, 4, right.getRefPatternForDisplay());
 
       {
         final SafeHtmlBuilder m = new SafeHtmlBuilder();

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java

@@ -133,7 +133,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
       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 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
       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++) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddRefRight.java

@@ -135,6 +135,12 @@ class AddRefRight extends Handler<ProjectDetail> {
         refPattern = RefRight.ALL;
       }
     }
+
+    boolean exclusive = refPattern.startsWith("-");
+    if (exclusive) {
+      refPattern = refPattern.substring(1);
+    }
+
     while (refPattern.startsWith("/")) {
       refPattern = refPattern.substring(1);
     }
@@ -152,6 +158,10 @@ class AddRefRight extends Handler<ProjectDetail> {
       }
     }
 
+    if (exclusive) {
+      refPattern = "-" + refPattern;
+    }
+
     if (!controlForRef(projectControl, refPattern).isOwner()) {
       throw new NoSuchRefException(refPattern);
     }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java

@@ -52,6 +52,7 @@ import com.google.inject.spi.Message;
 
 import org.kohsuke.args4j.Option;
 
+import java.io.Console;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -194,6 +195,11 @@ public class Init extends SiteProgram {
           System.err.flush();
         }
 
+        @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) {

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java

@@ -121,6 +121,13 @@ public final class RefRight {
   }
 
   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 @@ public final class RefRight {
     return getKey().getProjectNameKey();
   }
 
+  public boolean isExclusive() {
+    return key.refPattern.get().startsWith("-");
+  }
+
   public ApprovalCategory.Id getApprovalCategoryId() {
     return key.categoryId;
   }
@@ -152,6 +163,25 @@ public final class RefRight {
     maxValue = m;
   }
 
+  @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 @@ public final class RefRight {
     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();
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java

@@ -44,8 +44,11 @@ import java.io.IOException;
 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 @@ public class RefControl {
     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 @@ public class RefControl {
       allRights.addAll(getInheritedRights(actionId));
     }
 
-    // Sort in descending refPattern length
+    SortedMap<String, RefRightsForPattern> perPatternRights =
-    Collections.sort(allRights, RefRight.REF_PATTERN_ORDER);
+      sortedRightsByPattern(allRights);
 
-    for (RefRight right : filterMostSpecific(allRights)) {
+    for (RefRightsForPattern right : perPatternRights.values()) {
-      if (groups.contains(right.getAccountGroupId())) {
+      val = Math.max(val, right.allowedValueForRef(groups));
-        val = Math.max(right.getMaxValue(), val);
+      if (val >= level || right.containsExclusive()) {
+        return val >= level;
       }
     }
     return val >= level;
   }
 
-  public static List<RefRight> filterMostSpecific(List<RefRight> actionRights) {
+  public static final Comparator<String> DESCENDING_SORT =
-    // Grab the first set of RefRight which have the same refPattern
+      new Comparator<String>() {
-    // those are the most specific RefRights we have, and are the
+
-    // we will consider to verify if this action can be performed.
+    @Override
-    // We do this so that one can override the ref rights for a specific
+    public int compare(String a, String b) {
-    // project on a specific branch
+      int aLength = a.length();
-    boolean sameRefPattern = true;
+      int bLength = b.length();
-    List<RefRight> mostSpecific = new ArrayList<RefRight>();
+      if (bLength == aLength) {
-    String currentRefPattern = null;
+        return a.compareTo(b);
-    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 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 @@ public class RefControl {
     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);
+    SortedMap<String, RefRightsForPattern> perPatternRights =
-    return Collections.unmodifiableList(RefControl.filterMostSpecific(l));
+      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) {

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_100.java

@@ -29,7 +29,7 @@ public class Schema_100 extends SchemaVersion {
   private static final int MAX_SCAN_SIZE = 50000;
 
   @Inject
-  Schema_100(Provider<Schema_33> prior) {
+  Schema_100(Provider<Schema_34> prior) {
     super(prior);
   }
 

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/*");
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java

@@ -22,6 +22,8 @@ import java.util.List;
 public interface UpdateUI {
   void message(String msg);
 
+  boolean yesno(boolean def, String msg);
+
   void pruneSchema(StatementExecutor e, List<String> pruneList)
       throws OrmException;
 }

gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java

@@ -92,7 +92,7 @@ public abstract class CategoryFunction {
   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;

gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java

@@ -146,7 +146,7 @@ public class FunctionState {
     // 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 @@ public class FunctionState {
       }
     }
 
-    // Normalize the value into that range, returning true if we changed
+    // Normalize the value into that range.
-    // the value.
     //
     if (a.getValue() < minAllowed) {
       a.setValue(minAllowed);

gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java

@@ -44,7 +44,7 @@ public class SubmitFunction extends CategoryFunction {
       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;

gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java

@@ -66,6 +66,11 @@ public class SchemaUpdaterTest extends TestCase {
       public void message(String msg) {
       }
 
+      @Override
+      public boolean yesno(boolean def, String msg) {
+        return def;
+      }
+
       @Override
       public void pruneSchema(StatementExecutor e, List<String> pruneList)
           throws OrmException {