ADD_PATCH_SET should evaluate against refs/heads/*

Any "ADD_PATCH_SET" on "refs/for/*" will be automatially migrated
when the config is loaded.

Change-Id: I08c8648ae18051357c3a9fdd4c1b84481e8edf5f

Documentation/access-control.txt

@@ -610,10 +610,9 @@ reviews should not grant this category.
 
 This category controls which users are allowed to upload new patch sets to
 existing changes. Irrespective of this permission, change owners are always
-allowed to upload new patch sets for their changes. This permission needs to be
-set on `refs/for/*`.
+allowed to upload new patch sets for their changes.
 
-By default, this permission is granted to `Registered Users` on `refs/for/*`,
+By default, this permission is granted to `Registered Users` on `refs/*`,
 allowing all registered users to upload a new patch set to any change. Revoking
 this permission (by granting it to no groups and setting the "Exclusive" flag)
 will prevent users from uploading a patch set to a change they do not own.

java/com/google/gerrit/server/permissions/RefControl.java

@@ -29,7 +29,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
 import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.util.MagicBranch;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.util.Providers;
 import java.util.Collection;
@@ -97,9 +96,7 @@ class RefControl {
 
   /** @return true if this user can add a new patch set to this ref */
   boolean canAddPatchSet() {
-    return projectControl
-        .controlForRef(MagicBranch.NEW_CHANGE + refName)
-        .canPerform(Permission.ADD_PATCH_SET);
+    return canPerform(Permission.ADD_PATCH_SET);
   }
 
   /** @return true if this user can rebase changes on this ref */

java/com/google/gerrit/server/project/ProjectConfig.java

@@ -773,6 +773,12 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       }
       return true;
     }
+    if (isRefsForExclusively(refName) && perm.getName().equals(Permission.ADD_PATCH_SET)) {
+      // No need to migrate ADD_PATCH_SET on refs/*
+      AccessSection migratedAs = getAccessSection(unRefsFor(refName), true);
+      migratedAs.addPermission(perm);
+      return true;
+    }
     return false;
   }
 

java/com/google/gerrit/server/schema/AllProjectsCreator.java

@@ -162,13 +162,12 @@ public class AllProjectsCreator {
       AccessSection heads = config.getAccessSection(AccessSection.HEADS, true);
       AccessSection tags = config.getAccessSection("refs/tags/*", true);
       AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true);
-      AccessSection refsFor = config.getAccessSection("refs/for/*", true);
       AccessSection magic = config.getAccessSection("refs/for/" + AccessSection.ALL, true);
 
       grant(config, cap, GlobalCapability.ADMINISTRATE_SERVER, admin);
       grant(config, all, Permission.READ, admin, anonymous);
       grant(config, all, Permission.CREATE_REVIEW, registered);
-      grant(config, refsFor, Permission.ADD_PATCH_SET, registered);
+      grant(config, all, Permission.ADD_PATCH_SET, registered);
 
       if (batch != null) {
         Permission priority = cap.getPermission(GlobalCapability.PRIORITY, true);

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -2727,7 +2727,7 @@ public class ChangeIT extends AbstractDaemonTest {
     TestRepository<InMemoryRepository> userTestRepo = cloneProject(p, user);
 
     // Block default permission
-    block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
+    block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
 
     // Create change as admin
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo);
@@ -2771,7 +2771,7 @@ public class ChangeIT extends AbstractDaemonTest {
     TestRepository<?> adminTestRepo = cloneProject(project, admin);
 
     // Block default permission
-    block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
+    block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
 
     // Create change as admin
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo);
@@ -3407,7 +3407,7 @@ public class ChangeIT extends AbstractDaemonTest {
     Project.NameKey p = createProject("addPatchSetEdit");
     TestRepository<InMemoryRepository> userTestRepo = cloneProject(p, user);
     // Block default permission
-    block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
+    block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
     // Create change as user
     PushOneCommit push = pushFactory.create(db, user.getIdent(), userTestRepo);
     PushOneCommit.Result r = push.to("refs/for/master");

javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -694,7 +694,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
     TestRepository<InMemoryRepository> userTestRepo = cloneProject(p, user);
 
     // Block default permission
-    block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
+    block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS);
 
     // Create change as user
     PushOneCommit push = pushFactory.create(db, user.getIdent(), userTestRepo);

javatests/com/google/gerrit/server/project/ProjectConfigTest.java

@@ -672,6 +672,26 @@ public class ProjectConfigTest extends GerritBaseTests {
         .isEqualTo(Lists.newArrayList(rule));
   }
 
+  @Test
+  public void readConfigAddPatchSetRefsForStarIsMigrated() throws Exception {
+    RevCommit rev =
+        tr.commit()
+            .add("groups", group(developers))
+            .add(
+                "project.config",
+                "[access \"refs/for/*\"]\n" + "  addPatchSet = group Developers\n")
+            .create();
+
+    ProjectConfig cfg = read(rev);
+    AccessSection as = cfg.getAccessSection("refs/for/*");
+    assertThat(as).isNull();
+    as = cfg.getAccessSection("refs/*");
+    assertThat(as).isNotNull();
+    PermissionRule rule = new PermissionRule(developers);
+    assertThat(as.getPermission(Permission.ADD_PATCH_SET, false).getRules())
+        .isEqualTo(Lists.newArrayList(rule));
+  }
+
   @Test
   public void readCommentLinkMatchButNoHtmlOrLink() throws Exception {
     RevCommit rev =