diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java index 417bd26cac..b9af127f6f 100644 --- a/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/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 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(); diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java index ee6f387260..22335b6385 100644 --- a/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/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; } diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java index 0cdcdecb76..17ab34468b 100644 --- a/java/com/google/gerrit/server/group/db/GroupConfig.java +++ b/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"); diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index bb59183c0e..eeb914a358 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/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 { - pushToGroupBranch(project, null, null); + public void pushToGroupNamesBranchIsRejectedForAllUsersRepo() throws Exception { + 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( - Project.NameKey project, String expectedErrorOnCreate, String expectedErrorOnUpdate) + @Test + 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 repo = cloneProject(project); - // create new branch - PushOneCommit.Result r = - 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(); + if (createRef) { + createGroupBranch(project, groupRefName); } // 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 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");