Add a Change Owner group

Add a new system 'CHANGE_OWNER' (the patch set owner) group to allow
configuration of this group with gerrit label permissions.

To keeps it consistent with how the existing system groups
(Project Owner, Admin, etc..) are configured the Change Owner
group's logic for existing change context permissions like
Abandon, Rebase, View Drafts, etc. is untouched by this patch.
In other words an Admin can only apply this groups to a label
permission.

Bug: issue 1993
Change-Id: Ibb35b8172ce50319cfc2fca3b661fdaf7e245e5d

Documentation/access-control.txt

@@ -10,10 +10,19 @@ users.
 System Groups
 -------------
 
-Gerrit comes with 4 system groups, with special access privileges
-and membership management.  The identity of these groups is set
-in the `system_config` table within the database, so the groups
-can be renamed after installation if desired.
+Gerrit comes with following system groups:
+
+* Administrators
+* Anonymous Users
+* Change Owner
+* Non-Interactive Users
+* Project Owners
+* Registered Users
+
+The system groups are assigned special access and membership management
+privileges.  The identity of these groups is set in the `system_config`
+table within the database, so the groups can be renamed after installation
+if desired.
 
 
 [[administrators]]
@@ -85,6 +94,18 @@ avoid the need to initially configure access rights for
 newly created child projects.
 
 
+[[change_owner]]
+Change Owner
+~~~~~~~~~~~~
+
+Access rights assigned to this group are always evaluated within the
+context of a change to which the access rights apply. These rights
+therefore apply to the user who is the owner of this change.
+
+It is typical to assign a label to this group, allowing the change
+owner to vote on his change, but not actually cause it to become
+approved or rejected.
+
 [[registered_users]]
 Registered Users
 ~~~~~~~~~~~~~~~~

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AccountCreator.java

@@ -122,6 +122,16 @@ public class AccountCreator {
       "Administrators");
   }
 
+  public TestAccount user()
+      throws UnsupportedEncodingException, OrmException, JSchException {
+    return create("user", "user@example.com", "User");
+  }
+
+  public TestAccount user2()
+      throws UnsupportedEncodingException, OrmException, JSchException {
+    return create("user2", "user2@example.com", "User2");
+  }
+
   private AccountExternalId.Key getEmailKey(String email) {
     return new AccountExternalId.Key(AccountExternalId.SCHEME_MAILTO, email);
   }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java

@@ -0,0 +1,167 @@
+// Copyright (C) 2013 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.acceptance.rest.change;
+
+import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
+import static com.google.gerrit.acceptance.git.GitUtil.createProject;
+import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
+import static com.google.gerrit.common.data.Permission.LABEL;
+import static org.junit.Assert.assertEquals;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.RestSession;
+import com.google.gerrit.acceptance.SshSession;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.git.PushOneCommit;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+
+import com.jcraft.jsch.JSchException;
+
+import org.apache.http.HttpStatus;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+
+public class ChangeOwnerIT extends AbstractDaemonTest {
+
+  @Inject
+  private AccountCreator accounts;
+
+  @Inject
+  private SchemaFactory<ReviewDb> reviewDbProvider;
+
+  @Inject
+  private MetaDataUpdate.Server metaDataUpdateFactory;
+
+  @Inject
+  private ProjectCache projectCache;
+
+  @Inject
+  private GroupCache groupCache;
+
+  private TestAccount owner;
+  private TestAccount dev;
+
+  private RestSession sessionOwner;
+  private RestSession sessionDev;
+  private Git git;
+  private ReviewDb db;
+  private Project.NameKey project;
+
+  @Before
+  public void setUp() throws Exception {
+    newProject();
+    owner = accounts.user();
+    sessionOwner = new RestSession(server, owner);
+    SshSession sshSession = new SshSession(server, owner);
+    initSsh(owner);
+    // need to initialize intern session
+    createProject(sshSession, "foo");
+    git = cloneProject(sshSession.getUrl() + "/" + project.get());
+    sshSession.close();
+    dev = accounts.user2();
+    sessionDev = new RestSession(server, dev);
+    db = reviewDbProvider.open();
+  }
+
+  @After
+  public void cleanup() {
+    db.close();
+  }
+
+  @Test
+  public void testChangeOwner_OwnerACLNotGranted() throws GitAPIException,
+      IOException, OrmException, ConfigInvalidException {
+    approve(sessionOwner, createChange(), HttpStatus.SC_FORBIDDEN);
+  }
+
+  @Test
+  public void testChangeOwner_OwnerACLGranted() throws GitAPIException,
+      IOException, OrmException, ConfigInvalidException {
+    grantApproveToChangeOwner();
+    approve(sessionOwner, createChange(), HttpStatus.SC_OK);
+  }
+
+  @Test
+  public void testChangeOwner_NotOwnerACLGranted() throws GitAPIException,
+      IOException, OrmException, ConfigInvalidException {
+    grantApproveToChangeOwner();
+    approve(sessionDev, createChange(), HttpStatus.SC_FORBIDDEN);
+  }
+
+  private void approve(RestSession s, String changeId, int expected)
+      throws IOException {
+    RestResponse r =
+        s.post("/changes/" + changeId + "/revisions/current/review",
+            new ReviewInput().label("Code-Review", 2));
+    assertEquals(expected, r.getStatusCode());
+    r.consume();
+  }
+
+  private void grantApproveToChangeOwner() throws IOException,
+      ConfigInvalidException {
+    MetaDataUpdate md = metaDataUpdateFactory.create(project);
+    md.setMessage(String.format("Grant approve to change owner"));
+    ProjectConfig config = ProjectConfig.read(md);
+    AccessSection s = config.getAccessSection("refs/heads/*", true);
+    Permission p = s.getPermission(LABEL + "Code-Review", true);
+    AccountGroup changeOwnerGroup = groupCache
+        .get(new AccountGroup.NameKey("Change Owner"));
+    PermissionRule rule = new PermissionRule(config
+        .resolve(changeOwnerGroup));
+    rule.setMin(-2);
+    rule.setMax(+2);
+    p.add(rule);
+    config.commit(md);
+    projectCache.evict(config.getProject());
+  }
+
+  private String createChange() throws GitAPIException,
+      IOException {
+    PushOneCommit push = new PushOneCommit(db, owner.getIdent());
+    return push.to(git, "refs/for/master").getChangeId();
+  }
+
+  private void newProject() throws UnsupportedEncodingException,
+      OrmException, JSchException, IOException {
+    TestAccount admin = accounts.admin();
+    initSsh(admin);
+    project = new Project.NameKey("p");
+    SshSession sshSession = new SshSession(server, admin);
+    createProject(sshSession, project.get());
+    sshSession.close();
+  }
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/SystemGroupsIT.java

@@ -69,6 +69,7 @@ public class SystemGroupsIT extends AbstractDaemonTest {
     String result = session.exec("gerrit ls-groups");
     assertTrue(result.contains("Administrators"));
     assertTrue(result.contains("Anonymous Users"));
+    assertTrue(result.contains("Change Owner"));
     assertTrue(result.contains("Non-Interactive Users"));
     assertTrue(result.contains("Project Owners"));
     assertTrue(result.contains("Registered Users"));
@@ -85,6 +86,7 @@ public class SystemGroupsIT extends AbstractDaemonTest {
     Set<String> names = result.keySet();
     assertTrue(names.contains("Administrators"));
     assertTrue(names.contains("Anonymous Users"));
+    assertTrue(names.contains("Change Owner"));
     assertTrue(names.contains("Non-Interactive Users"));
     assertTrue(names.contains("Project Owners"));
     assertTrue(names.contains("Registered Users"));
@@ -100,6 +102,7 @@ public class SystemGroupsIT extends AbstractDaemonTest {
       }
       assertTrue(names.contains("Administrators"));
       assertTrue(names.contains("Anonymous Users"));
+      assertTrue(names.contains("Change Owner"));
       assertTrue(names.contains("Non-Interactive Users"));
       assertTrue(names.contains("Project Owners"));
       assertTrue(names.contains("Registered Users"));

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java

@@ -149,6 +149,10 @@ public final class AccountGroup {
   public static final AccountGroup.UUID PROJECT_OWNERS =
       new AccountGroup.UUID("global:Project-Owners");
 
+  /** Common UUID assigned to the "Change Owner" placeholder group. */
+  public static final AccountGroup.UUID CHANGE_OWNER =
+      new AccountGroup.UUID("global:Change-Owner");
+
   /** Common UUID assigned to the "Anonymous Users" group. */
   public static final AccountGroup.UUID ANONYMOUS_USERS =
       new AccountGroup.UUID("global:Anonymous-Users");

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

@@ -292,12 +292,12 @@ public class ChangeControl {
 
   /** All value ranges of any allowed label permission. */
   public List<PermissionRange> getLabelRanges() {
-    return getRefControl().getLabelRanges();
+    return getRefControl().getLabelRanges(isOwner());
   }
 
   /** The range of permitted values associated with a label permission. */
   public PermissionRange getRange(String permission) {
-    return getRefControl().getRange(permission);
+    return getRefControl().getRange(permission, isOwner());
   }
 
   /** Can this user add a patch set to this change? */

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

@@ -462,9 +462,19 @@ public class ProjectControl {
     return match(rule.getGroup().getUUID());
   }
 
+  boolean match(PermissionRule rule, boolean isChangeOwner) {
+    return match(rule.getGroup().getUUID(), isChangeOwner);
+  }
+
   boolean match(AccountGroup.UUID uuid) {
+    return match(uuid, false);
+  }
+
+  boolean match(AccountGroup.UUID uuid, boolean isChangeOwner) {
     if (AccountGroup.PROJECT_OWNERS.equals(uuid)) {
       return isDeclaredOwner();
+    } else if (AccountGroup.CHANGE_OWNER.equals(uuid)) {
+      return isChangeOwner;
     } else {
       return user.getEffectiveGroups().contains(uuid);
     }

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

@@ -377,14 +377,14 @@ public class RefControl {
   }
 
   /** All value ranges of any allowed label permission. */
-  public List<PermissionRange> getLabelRanges() {
+  public List<PermissionRange> getLabelRanges(boolean isChangeOwner) {
     List<PermissionRange> r = new ArrayList<PermissionRange>();
     for (Map.Entry<String, List<PermissionRule>> e : relevant.getDeclaredPermissions()) {
       if (Permission.isLabel(e.getKey())) {
         int min = 0;
         int max = 0;
         for (PermissionRule rule : e.getValue()) {
-          if (projectControl.match(rule)) {
+          if (projectControl.match(rule, isChangeOwner)) {
             min = Math.min(min, rule.getMin());
             max = Math.max(max, rule.getMax());
           }
@@ -399,8 +399,13 @@ public class RefControl {
 
   /** The range of permitted values associated with a label permission. */
   public PermissionRange getRange(String permission) {
+    return getRange(permission, false);
+  }
+
+  /** The range of permitted values associated with a label permission. */
+  public PermissionRange getRange(String permission, boolean isChangeOwner) {
     if (Permission.hasRange(permission)) {
-      return toRange(permission, access(permission));
+      return toRange(permission, access(permission, isChangeOwner));
     }
     return null;
   }
@@ -516,6 +521,12 @@ public class RefControl {
 
   /** Rules for the given permission, or the empty list. */
   private List<PermissionRule> access(String permissionName) {
+    return access(permissionName, false);
+  }
+
+  /** Rules for the given permission, or the empty list. */
+  private List<PermissionRule> access(String permissionName,
+      boolean isChangeOwner) {
     List<PermissionRule> rules = effective.get(permissionName);
     if (rules != null) {
       return rules;
@@ -529,7 +540,7 @@ public class RefControl {
     }
 
     if (rules.size() == 1) {
-      if (!projectControl.match(rules.get(0))) {
+      if (!projectControl.match(rules.get(0), isChangeOwner)) {
         rules = Collections.emptyList();
       }
       effective.put(permissionName, rules);
@@ -538,7 +549,7 @@ public class RefControl {
 
     List<PermissionRule> mine = new ArrayList<PermissionRule>(rules.size());
     for (PermissionRule rule : rules) {
-      if (projectControl.match(rule)) {
+      if (projectControl.match(rule, isChangeOwner)) {
         mine.add(rule);
       }
     }

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

@@ -52,6 +52,7 @@ public class SchemaCreator {
   private AccountGroup registered;
   private AccountGroup owners;
   private AccountGroup batch;
+  private AccountGroup changeOwner;
 
   @Inject
   public SchemaCreator(SitePaths site,
@@ -149,6 +150,14 @@ public class SchemaCreator {
     c.accountGroupNames().insert(
         Collections.singleton(new AccountGroupName(owners)));
 
+    changeOwner = newGroup(c, "Change Owner", AccountGroup.CHANGE_OWNER);
+    changeOwner.setDescription("The owner of a change");
+    changeOwner.setOwnerGroupUUID(admin.getGroupUUID());
+    changeOwner.setType(AccountGroup.Type.SYSTEM);
+    c.accountGroups().insert(Collections.singleton(changeOwner));
+    c.accountGroupNames().insert(
+        Collections.singleton(new AccountGroupName(changeOwner)));
+
     final SystemConfig s = SystemConfig.create();
     try {
       s.sitePath = site_path.getCanonicalPath();

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

@@ -32,7 +32,7 @@ import java.util.List;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_85> C = Schema_85.class;
+  public static final Class<Schema_86> C = Schema_86.class;
 
   public static class Module extends AbstractModule {
     @Override

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

@@ -0,0 +1,118 @@
+// Copyright (C) 2013 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.common.data.AccessSection;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.common.data.PermissionRule.Action;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountGroupName;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.GroupUUID;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+public class Schema_86 extends SchemaVersion {
+  private final AllProjectsName allProjects;
+  private final GitRepositoryManager mgr;
+  private final PersonIdent serverUser;
+
+  @Inject
+  Schema_86(Provider<Schema_85> prior,
+      AllProjectsName allProjects,
+      GitRepositoryManager mgr,
+      @GerritPersonIdent PersonIdent serverUser) {
+    super(prior);
+    this.allProjects = allProjects;
+    this.mgr = mgr;
+    this.serverUser = serverUser;
+  }
+
+  @Override
+  protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
+    Repository git;
+    try {
+      git = mgr.openRepository(allProjects);
+    } catch (IOException e) {
+      throw new OrmException(e);
+    }
+
+    try {
+      MetaDataUpdate md =
+          new MetaDataUpdate(GitReferenceUpdated.DISABLED, allProjects, git);
+      ProjectConfig config = ProjectConfig.read(md);
+
+      // Create the CHANGE OWNER group.
+      AccountGroup.UUID adminGroupUUID = findAdminGroup(db, config);
+      createGroup(db, "Change Owner", adminGroupUUID,
+            "The owner of a change");
+    } catch (IOException e) {
+      throw new OrmException(e);
+    } catch (ConfigInvalidException e) {
+      throw new OrmException(e);
+    } finally {
+      git.close();
+    }
+  }
+
+  private AccountGroup createGroup(ReviewDb db, String groupName,
+      AccountGroup.UUID adminGroupUUID, String description) throws OrmException {
+    AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId());
+    AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName);
+    AccountGroup.UUID uuid = GroupUUID.make(groupName, serverUser);
+    AccountGroup group = new AccountGroup(nameKey, groupId, uuid);
+    group.setOwnerGroupUUID(adminGroupUUID);
+    group.setDescription(description);
+    group.setType(AccountGroup.Type.SYSTEM);
+
+    AccountGroupName gn = new AccountGroupName(group);
+    // first insert the group name to validate that the group name hasn't
+    // already been used to create another group
+    db.accountGroupNames().insert(Collections.singleton(gn));
+    db.accountGroups().insert(Collections.singleton(group));
+    return group;
+  }
+
+  private static AccountGroup.UUID findAdminGroup(
+      ReviewDb db, ProjectConfig cfg) {
+    List<PermissionRule> rules = cfg
+        .getAccessSection(AccessSection.GLOBAL_CAPABILITIES)
+        .getPermission(GlobalCapability.ADMINISTRATE_SERVER)
+        .getRules();
+
+    for (PermissionRule rule : rules) {
+      if (rule.getAction() == Action.ALLOW) {
+        return rule.getGroup().getUUID();
+      }
+    }
+    throw new IllegalStateException("no administrator group found");
+  }
+}

gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

@@ -22,6 +22,7 @@ import static com.google.gerrit.common.data.Permission.READ;
 import static com.google.gerrit.common.data.Permission.SUBMIT;
 import static com.google.gerrit.server.project.Util.ANONYMOUS;
 import static com.google.gerrit.server.project.Util.REGISTERED;
+import static com.google.gerrit.server.project.Util.CHANGE_OWNER;
 import static com.google.gerrit.server.project.Util.ADMIN;
 import static com.google.gerrit.server.project.Util.DEVS;
 import static com.google.gerrit.server.project.Util.grant;
@@ -428,4 +429,24 @@ public class RefControlTest {
     assertFalse("u can't vote -2", range.contains(-2));
     assertFalse("u can't vote 2", range.contains(2));
   }
+
+  public void testUnblockRangeForChangeOwner() {
+    grant(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*");
+
+    ProjectControl u = util.user(local, DEVS);
+    PermissionRange range = u.controlForRef("refs/heads/master")
+        .getRange(LABEL + "Code-Review", true);
+    assertTrue("u can vote -2", range.contains(-2));
+    assertTrue("u can vote +2", range.contains(2));
+  }
+
+  public void testUnblockRangeForNotChangeOwner() {
+    grant(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*");
+
+    ProjectControl u = util.user(local, DEVS);
+    PermissionRange range = u.controlForRef("refs/heads/master")
+        .getRange(LABEL + "Code-Review");
+    assertFalse("u can vote -2", range.contains(-2));
+    assertFalse("u can vote +2", range.contains(2));
+  }
 }

gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java

@@ -54,10 +54,11 @@ import java.util.Map;
 import java.util.Set;
 
 public class Util {
-  public static final AccountGroup.UUID ANONYMOUS = AccountGroup.ANONYMOUS_USERS;
-  public static final AccountGroup.UUID REGISTERED = AccountGroup.REGISTERED_USERS;
-  public static final AccountGroup.UUID ADMIN = new AccountGroup.UUID("test.admin");
-  public static final AccountGroup.UUID DEVS = new AccountGroup.UUID("test.devs");
+  public static AccountGroup.UUID ANONYMOUS = AccountGroup.ANONYMOUS_USERS;
+  public static AccountGroup.UUID CHANGE_OWNER = AccountGroup.CHANGE_OWNER;
+  public static AccountGroup.UUID REGISTERED = AccountGroup.REGISTERED_USERS;
+  public static AccountGroup.UUID ADMIN = new AccountGroup.UUID("test.admin");
+  public static AccountGroup.UUID DEVS = new AccountGroup.UUID("test.devs");
 
   public static final LabelType CR = category("Code-Review",
       value(2, "Looks good to me, approved"),