diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java index 6e9a11915e..8ed1ef0860 100644 --- a/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/java/com/google/gerrit/server/change/ChangeInserter.java @@ -569,8 +569,7 @@ public class ChangeInserter implements InsertChangeOp { new Branch.NameKey(ctx.getProject(), refName), ctx.getIdentifiedUser(), new NoSshInfo(), - ctx.getRevWalk(), - projectState) + ctx.getRevWalk()) .validate(event); } } catch (CommitValidationException e) { diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java index bbb41e0ffa..d298730a42 100644 --- a/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -344,8 +344,7 @@ public class PatchSetInserter implements BatchUpdateOp { origNotes.getChange().getDest(), ctx.getIdentifiedUser(), new NoSshInfo(), - ctx.getRevWalk(), - projectCache.checkedGet(ctx.getProject())) + ctx.getRevWalk()) .validate(event); } catch (CommitValidationException e) { throw new ResourceConflictException(e.getFullMessage()); diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java index f6a9d8d156..417bd26cac 100644 --- a/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -21,11 +21,9 @@ import static java.util.stream.Collectors.toList; import com.google.common.base.CharMatcher; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Sets; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.PageLinks; -import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.AuthException; @@ -60,9 +58,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.regex.Pattern; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; @@ -137,7 +133,7 @@ public class CommitValidators { new SignedOffByValidator(user, perm, projectState), new ChangeIdValidator( projectState, user, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(branch, user, rw, allUsers, allProjects, projectState), + new ConfigValidator(branch, user, rw, allUsers, allProjects), new BannedCommitsValidator(rejectCommits), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), @@ -150,8 +146,7 @@ public class CommitValidators { Branch.NameKey branch, IdentifiedUser user, SshInfo sshInfo, - RevWalk rw, - ProjectState projectState) + RevWalk rw) throws IOException { return new CommitValidators( ImmutableList.of( @@ -165,7 +160,7 @@ public class CommitValidators { canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(branch, user, rw, allUsers, allProjects, projectState), + new ConfigValidator(branch, user, rw, allUsers, allProjects), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), new AccountCommitValidator(allUsers, accountValidator), @@ -371,21 +366,18 @@ public class CommitValidators { private final RevWalk rw; private final AllUsersName allUsers; private final AllProjectsName allProjects; - private final ProjectState projectState; public ConfigValidator( Branch.NameKey branch, IdentifiedUser user, RevWalk rw, AllUsersName allUsers, - AllProjectsName allProjects, - ProjectState projectState) { + AllProjectsName allProjects) { this.branch = branch; this.user = user; this.rw = rw; - this.allUsers = allUsers; this.allProjects = allProjects; - this.projectState = projectState; + this.allUsers = allUsers; } @Override @@ -404,29 +396,6 @@ public class CommitValidators { } throw new ConfigInvalidException("invalid project configuration"); } - if (allUsers.equals(receiveEvent.project.getNameKey()) - || allProjects.equals(receiveEvent.project.getNameKey())) { - // Check if the new config modifies any access sections for refs/groups/. These are - // managed by Gerrit and modifications are not allowed. - Set diff = - Sets.symmetricDifference( - new HashSet<>(projectState.getConfig().getAccessSections()), - new HashSet<>(cfg.getAccessSections())); - boolean modifiesGroupsAccessSection = - diff.stream() - .filter(as -> as.getName().startsWith(RefNames.REFS_GROUPS)) - .findAny() - .isPresent(); - if (modifiesGroupsAccessSection) { - addError("Invalid project configuration:", messages); - addError( - String.format( - " permissions on %s are managed by gerrit and cannot be modified", - RefNames.REFS_GROUPS), - messages); - throw new ConfigInvalidException("invalid project configuration"); - } - } if (allUsers.equals(receiveEvent.project.getNameKey()) && !allProjects.equals(cfg.getProject().getParent(allProjects))) { addError("Invalid project configuration:", messages); diff --git a/java/com/google/gerrit/server/project/SetAccessUtil.java b/java/com/google/gerrit/server/project/SetAccessUtil.java index 1e7b6b1ca3..cc97695cf9 100644 --- a/java/com/google/gerrit/server/project/SetAccessUtil.java +++ b/java/com/google/gerrit/server/project/SetAccessUtil.java @@ -139,7 +139,7 @@ public class SetAccessUtil { && section.getName().startsWith(RefNames.REFS_GROUPS)) { throw new BadRequestException( String.format( - "permissions on %s are managed by gerrit and cannot be modified", + "Permissions on %s is managed by Gerrit and cannot be modified", RefNames.REFS_GROUPS)); } } diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index dd4ed2303f..ae3e63534c 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java @@ -25,7 +25,6 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -34,10 +33,8 @@ import com.google.gerrit.acceptance.PushOneCommit; 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.GroupReference; 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.extensions.api.groups.GroupApi; import com.google.gerrit.extensions.api.groups.GroupInput; @@ -60,7 +57,6 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; 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.Groups; @@ -824,96 +820,6 @@ public class GroupsIT extends AbstractDaemonTest { pushToGroupBranchForReviewAndSubmit(project, null); } - @Test - public void pushGroupsAccessSectionChangeToAllUsersFails() throws Exception { - TestRepository repo = cloneProject(allUsers, RefNames.REFS_CONFIG); - - String config = - gApi.projects() - .name(allUsers.get()) - .branch(RefNames.REFS_CONFIG) - .file(ProjectConfig.PROJECT_CONFIG) - .asString(); - - Config cfg = new Config(); - cfg.fromText(config); - cfg.setString("access", RefNames.REFS_GROUPS + "foo", "push", "group Registered Users"); - config = cfg.toText(); - - PushOneCommit.Result r = - pushFactory - .create(db, admin.getIdent(), repo, "Subject", ProjectConfig.PROJECT_CONFIG, config) - .to(RefNames.REFS_CONFIG); - r.assertErrorStatus("invalid project configuration"); - r.assertMessage("permissions on refs/groups/ are managed by gerrit and cannot be modified"); - } - - @Test - public void pushNonGroupsAccessSectionChangeToAllUsersSucceeds() throws Exception { - // Add an access section for refs/groups manually to see that mutation other data does not - // trigger a validation error. - ProjectConfig projectConfig = projectCache.checkedGet(allUsers).getConfig(); - AccessSection as = new AccessSection(RefNames.REFS_GROUPS + "foo"); - Permission perm = new Permission("push"); - perm.add(new PermissionRule(systemGroupBackend.getGroup(ANONYMOUS_USERS))); - as.addPermission(perm); - projectConfig.replace(as); - saveProjectConfig(allUsers, projectConfig); - - TestRepository repo = cloneProject(allUsers, RefNames.REFS_CONFIG); - - String config = - gApi.projects() - .name(allUsers.get()) - .branch(RefNames.REFS_CONFIG) - .file(ProjectConfig.PROJECT_CONFIG) - .asString(); - assertThat(config).contains("[access \"refs/groups/foo\"]"); - - Config cfg = new Config(); - cfg.fromText(config); - cfg.setString("access", RefNames.REFS_CHANGES + "foo", "push", "group Registered Users"); - config = cfg.toText(); - - PushOneCommit.Result r = - pushFactory - .create(db, admin.getIdent(), repo, "Subject", ProjectConfig.PROJECT_CONFIG, config) - .to(RefNames.REFS_CONFIG); - r.assertOkStatus(); - } - - @Test - public void pushGroupsAccessSectionChangeToCustomProjectSucceeds() throws Exception { - TestRepository repo = cloneProject(project, RefNames.REFS_CONFIG); - - String config = - gApi.projects() - .name(project.get()) - .branch(RefNames.REFS_CONFIG) - .file(ProjectConfig.PROJECT_CONFIG) - .asString(); - - Config cfg = new Config(); - cfg.fromText(config); - cfg.setString("access", RefNames.REFS_GROUPS + "foo", "push", "group Registered Users"); - config = cfg.toText(); - - PushOneCommit.Result r = - pushFactory - .create( - db, - admin.getIdent(), - repo, - "Subject", - ImmutableMap.of( - "groups", - "global:Registered-Users\tRegistered Users", - ProjectConfig.PROJECT_CONFIG, - config)) - .to(RefNames.REFS_CONFIG); - r.assertOkStatus(); - } - @Test public void pushCustomInheritanceForAllUsersFails() throws Exception { TestRepository repo = cloneProject(allUsers, RefNames.REFS_CONFIG); diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java index 2f5960dd50..1753cfc4c5 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java @@ -531,7 +531,7 @@ public class AccessIT extends AbstractDaemonTest { accessInput.add.put(RefNames.REFS_GROUPS + "*", createDefaultAccessSectionInfo()); exception.expect(BadRequestException.class); exception.expectMessage( - "permissions on refs/groups/ are managed by gerrit and cannot be modified"); + "Permissions on refs/groups/ is managed by Gerrit and cannot be modified"); gApi.projects().name(allProjects.get()).access(accessInput); }