Prevent updates to refs/meta/group-names through push and submit

Change-Id: Iea92c2b120a96439b3fb1240af0ab484bf02a858

java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -778,7 +778,7 @@ public class CommitValidators {
     }
   }
 
-  /** Rejects updates to group branches. */
+  /** Rejects updates to group branches (refs/groups/* and refs/meta/group-names). */
   public static class GroupCommitValidator implements CommitValidationListener {
     private final AllUsersName allUsers;
 
@@ -790,6 +790,7 @@ public class CommitValidators {
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
       // Groups are stored inside 'refs/groups/' refs inside the 'All-Users' repository.
+      // Group names are stored inside 'refs/meta/group-names' refs inside the 'All-Users' repository.
       if (!allUsers.equals(receiveEvent.project.getNameKey())) {
         return Collections.emptyList();
       }
@@ -800,7 +801,8 @@ public class CommitValidators {
         return Collections.emptyList();
       }
 
-      if (receiveEvent.command.getRefName().startsWith(RefNames.REFS_GROUPS)) {
+      if (receiveEvent.command.getRefName().startsWith(RefNames.REFS_GROUPS)
+          || receiveEvent.command.getRefName().equals(RefNames.REFS_GROUPNAMES)) {
         throw new CommitValidationException("group update not allowed");
       }
       return Collections.emptyList();

java/com/google/gerrit/server/git/validators/MergeValidators.java

@@ -319,8 +319,10 @@ public class MergeValidators {
         IdentifiedUser caller)
         throws MergeValidationException {
       // Groups are stored inside 'refs/groups/' refs inside the 'All-Users' repository.
+      // Group names are stored inside 'refs/meta/group-names' inside the 'All-Users' repository.
       if (!allUsersName.equals(destProject.getNameKey())
-          || !destBranch.get().startsWith(RefNames.REFS_GROUPS)) {
+          || (!destBranch.get().startsWith(RefNames.REFS_GROUPS)
+              && !destBranch.get().equals(RefNames.REFS_GROUPNAMES))) {
         return;
       }
 

java/com/google/gerrit/server/group/db/GroupConfig.java

@@ -49,7 +49,7 @@ import org.eclipse.jgit.revwalk.RevSort;
 
 // TODO(aliceks): Add Javadoc descriptions to this file.
 public class GroupConfig extends VersionedMetaData {
-  private static final String GROUP_CONFIG_FILE = "group.config";
+  public static final String GROUP_CONFIG_FILE = "group.config";
   private static final String MEMBERS_FILE = "members";
   private static final String SUBGROUPS_FILE = "subgroups";
   private static final Pattern LINE_SEPARATOR_PATTERN = Pattern.compile("\\R");

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -35,6 +35,7 @@ import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.data.Permission;
@@ -66,6 +67,7 @@ import com.google.gerrit.server.account.GroupIncludeCache;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.group.db.GroupConfig;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.index.group.StalenessChecker;
 import com.google.gerrit.server.util.MagicBranch;
@@ -832,44 +834,57 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void pushToGroupBranchIsRejectedForAllUsersRepo() throws Exception {
-    pushToGroupBranch(allUsers, "Not allowed to create group branch.", "group update not allowed");
+    String groupRef =
+        RefNames.refsGroups(new AccountGroup.UUID(gApi.groups().create(name("fo")).get().id));
+    assertPushToGroupBranch(allUsers, groupRef, !groupsInNoteDb(), "group update not allowed");
   }
 
   @Test
-  public void pushToGroupBranchForNonAllUsersRepo() throws Exception {
+  public void pushToGroupNamesBranchIsRejectedForAllUsersRepo() throws Exception {
-    pushToGroupBranch(project, null, null);
+    assume().that(groupsInNoteDb()).isTrue(); // branch only exists when groups are in NoteDb
+    // refs/meta/group-names isn't usually available for fetch, so grant ACCESS_DATABASE
+    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    assertPushToGroupBranch(allUsers, RefNames.REFS_GROUPNAMES, false, "group update not allowed");
   }
 
-  private void pushToGroupBranch(
+  @Test
-      Project.NameKey project, String expectedErrorOnCreate, String expectedErrorOnUpdate)
+  public void pushToGroupsBranchForNonAllUsersRepo() throws Exception {
+    assertCreateGroupBranch(project, null);
+    String groupRef =
+        RefNames.refsGroups(new AccountGroup.UUID(gApi.groups().create(name("fo")).get().id));
+    assertPushToGroupBranch(project, groupRef, true, null);
+  }
+
+  @Test
+  public void pushToGroupNamesBranchForNonAllUsersRepo() throws Exception {
+    assertPushToGroupBranch(project, RefNames.REFS_GROUPNAMES, true, null);
+  }
+
+  private void assertPushToGroupBranch(
+      Project.NameKey project, String groupRefName, boolean createRef, String expectedErrorOnUpdate)
       throws Exception {
     grant(project, RefNames.REFS_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
     grant(project, RefNames.REFS_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
+    grant(project, RefNames.REFS_GROUPNAMES, Permission.PUSH, false, REGISTERED_USERS);
 
     TestRepository<InMemoryRepository> repo = cloneProject(project);
 
-    // create new branch
+    if (createRef) {
-    PushOneCommit.Result r =
+      createGroupBranch(project, groupRefName);
-        pushFactory
-            .create(
-                db, admin.getIdent(), repo, "Update group config", "group.config", "some content")
-            .setParents(ImmutableList.of())
-            .to(RefNames.REFS_GROUPS + name("foo"));
-    if (expectedErrorOnCreate != null) {
-      r.assertErrorStatus(expectedErrorOnCreate);
-    } else {
-      r.assertOkStatus();
     }
 
     // update existing branch
-    String groupRefName = RefNames.REFS_GROUPS + name("bar");
-    createGroupBranch(project, groupRefName);
     fetch(repo, groupRefName + ":groupRef");
     repo.reset("groupRef");
-    r =
+    PushOneCommit.Result r =
         pushFactory
             .create(
-                db, admin.getIdent(), repo, "Update group config", "group.config", "some content")
+                db,
+                admin.getIdent(),
+                repo,
+                "Update group config",
+                GroupConfig.GROUP_CONFIG_FILE,
+                "some content")
             .to(groupRefName);
     if (expectedErrorOnUpdate != null) {
       r.assertErrorStatus(expectedErrorOnUpdate);
@@ -878,6 +893,29 @@ public class GroupsIT extends AbstractDaemonTest {
     }
   }
 
+  private void assertCreateGroupBranch(Project.NameKey project, String expectedErrorOnCreate)
+      throws Exception {
+    grant(project, RefNames.REFS_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
+    grant(project, RefNames.REFS_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
+    TestRepository<InMemoryRepository> repo = cloneProject(project);
+    PushOneCommit.Result r =
+        pushFactory
+            .create(
+                db,
+                admin.getIdent(),
+                repo,
+                "Update group config",
+                GroupConfig.GROUP_CONFIG_FILE,
+                "some content")
+            .setParents(ImmutableList.of())
+            .to(RefNames.REFS_GROUPS + name("bar"));
+    if (expectedErrorOnCreate != null) {
+      r.assertErrorStatus(expectedErrorOnCreate);
+    } else {
+      r.assertOkStatus();
+    }
+  }
+
   @Test
   public void pushToGroupBranchForReviewForAllUsersRepoIsRejectedOnSubmit() throws Exception {
     pushToGroupBranchForReviewAndSubmit(allUsers, "group update not allowed");