Revert "Prevent mutation of permissions on refs/groups through RPC"
As discussed in change Id18851f503 group owners are no longer defined by
permissions on refs/groups and hence we don't need to prevent
modifications to permissions on this namespace.
This reverts commit 7c6fd9b183.
Change-Id: I5c51cbe0a19ae741ad9930e23c7c7cc51e9321fb
This commit is contained in:
@@ -23,7 +23,6 @@ import com.google.gerrit.server.CreateGroupPermissionSyncer;
|
|||||||
import com.google.gerrit.server.CurrentUser;
|
import com.google.gerrit.server.CurrentUser;
|
||||||
import com.google.gerrit.server.account.GroupBackend;
|
import com.google.gerrit.server.account.GroupBackend;
|
||||||
import com.google.gerrit.server.config.AllProjectsName;
|
import com.google.gerrit.server.config.AllProjectsName;
|
||||||
import com.google.gerrit.server.config.AllUsersName;
|
|
||||||
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.ProjectConfig;
|
import com.google.gerrit.server.git.ProjectConfig;
|
||||||
@@ -64,7 +63,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
|
|||||||
GroupBackend groupBackend,
|
GroupBackend groupBackend,
|
||||||
MetaDataUpdate.User metaDataUpdateFactory,
|
MetaDataUpdate.User metaDataUpdateFactory,
|
||||||
AllProjectsName allProjects,
|
AllProjectsName allProjects,
|
||||||
AllUsersName allUsers,
|
|
||||||
Provider<SetParent> setParent,
|
Provider<SetParent> setParent,
|
||||||
GitReferenceUpdated gitRefUpdated,
|
GitReferenceUpdated gitRefUpdated,
|
||||||
ContributorAgreementsChecker contributorAgreements,
|
ContributorAgreementsChecker contributorAgreements,
|
||||||
@@ -80,7 +78,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
|
|||||||
groupBackend,
|
groupBackend,
|
||||||
metaDataUpdateFactory,
|
metaDataUpdateFactory,
|
||||||
allProjects,
|
allProjects,
|
||||||
allUsers,
|
|
||||||
setParent,
|
setParent,
|
||||||
user.get(),
|
user.get(),
|
||||||
projectName,
|
projectName,
|
||||||
|
|||||||
@@ -32,12 +32,10 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
|||||||
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||||
import com.google.gerrit.httpd.rpc.Handler;
|
import com.google.gerrit.httpd.rpc.Handler;
|
||||||
import com.google.gerrit.reviewdb.client.Project;
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
import com.google.gerrit.reviewdb.client.RefNames;
|
|
||||||
import com.google.gerrit.server.CurrentUser;
|
import com.google.gerrit.server.CurrentUser;
|
||||||
import com.google.gerrit.server.account.GroupBackend;
|
import com.google.gerrit.server.account.GroupBackend;
|
||||||
import com.google.gerrit.server.account.GroupBackends;
|
import com.google.gerrit.server.account.GroupBackends;
|
||||||
import com.google.gerrit.server.config.AllProjectsName;
|
import com.google.gerrit.server.config.AllProjectsName;
|
||||||
import com.google.gerrit.server.config.AllUsersName;
|
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.ProjectConfig;
|
import com.google.gerrit.server.git.ProjectConfig;
|
||||||
import com.google.gerrit.server.permissions.PermissionBackend;
|
import com.google.gerrit.server.permissions.PermissionBackend;
|
||||||
@@ -67,7 +65,6 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
|
|
||||||
private final MetaDataUpdate.User metaDataUpdateFactory;
|
private final MetaDataUpdate.User metaDataUpdateFactory;
|
||||||
private final AllProjectsName allProjects;
|
private final AllProjectsName allProjects;
|
||||||
private final AllUsersName allUsers;
|
|
||||||
private final Provider<SetParent> setParent;
|
private final Provider<SetParent> setParent;
|
||||||
private final ContributorAgreementsChecker contributorAgreements;
|
private final ContributorAgreementsChecker contributorAgreements;
|
||||||
private final PermissionBackend permissionBackend;
|
private final PermissionBackend permissionBackend;
|
||||||
@@ -83,7 +80,6 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
GroupBackend groupBackend,
|
GroupBackend groupBackend,
|
||||||
MetaDataUpdate.User metaDataUpdateFactory,
|
MetaDataUpdate.User metaDataUpdateFactory,
|
||||||
AllProjectsName allProjects,
|
AllProjectsName allProjects,
|
||||||
AllUsersName allUsers,
|
|
||||||
Provider<SetParent> setParent,
|
Provider<SetParent> setParent,
|
||||||
CurrentUser user,
|
CurrentUser user,
|
||||||
Project.NameKey projectName,
|
Project.NameKey projectName,
|
||||||
@@ -97,7 +93,6 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
this.groupBackend = groupBackend;
|
this.groupBackend = groupBackend;
|
||||||
this.metaDataUpdateFactory = metaDataUpdateFactory;
|
this.metaDataUpdateFactory = metaDataUpdateFactory;
|
||||||
this.allProjects = allProjects;
|
this.allProjects = allProjects;
|
||||||
this.allUsers = allUsers;
|
|
||||||
this.setParent = setParent;
|
this.setParent = setParent;
|
||||||
this.user = user;
|
this.user = user;
|
||||||
|
|
||||||
@@ -142,26 +137,12 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
RefPattern.validate(name);
|
RefPattern.validate(name);
|
||||||
boolean differs = replace(config, toDelete, section);
|
|
||||||
if (differs
|
replace(config, toDelete, section);
|
||||||
&& groupMutationsDisallowed(projectName)
|
|
||||||
&& isGroupMutation(section.getName())) {
|
|
||||||
throw new ConfigInvalidException(
|
|
||||||
String.format(
|
|
||||||
"permissions on %s are managed by gerrit and cannot be modified",
|
|
||||||
RefNames.REFS_GROUPS));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
for (String name : toDelete) {
|
for (String name : toDelete) {
|
||||||
if (groupMutationsDisallowed(projectName) && isGroupMutation(name)) {
|
|
||||||
throw new ConfigInvalidException(
|
|
||||||
String.format(
|
|
||||||
"permissions on %s are managed by gerrit and cannot be modified",
|
|
||||||
RefNames.REFS_GROUPS));
|
|
||||||
}
|
|
||||||
|
|
||||||
if (AccessSection.GLOBAL_CAPABILITIES.equals(name)) {
|
if (AccessSection.GLOBAL_CAPABILITIES.equals(name)) {
|
||||||
if (!checkIfOwner || canWriteConfig()) {
|
if (!checkIfOwner || canWriteConfig()) {
|
||||||
config.remove(config.getAccessSection(name));
|
config.remove(config.getAccessSection(name));
|
||||||
@@ -216,18 +197,15 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
throws IOException, NoSuchProjectException, ConfigInvalidException, OrmException,
|
throws IOException, NoSuchProjectException, ConfigInvalidException, OrmException,
|
||||||
PermissionDeniedException, PermissionBackendException;
|
PermissionDeniedException, PermissionBackendException;
|
||||||
|
|
||||||
/** @return true if the access section differed from the existing one and had to be replaced. */
|
private void replace(ProjectConfig config, Set<String> toDelete, AccessSection section)
|
||||||
private boolean replace(ProjectConfig config, Set<String> toDelete, AccessSection section)
|
|
||||||
throws NoSuchGroupException {
|
throws NoSuchGroupException {
|
||||||
for (Permission permission : section.getPermissions()) {
|
for (Permission permission : section.getPermissions()) {
|
||||||
for (PermissionRule rule : permission.getRules()) {
|
for (PermissionRule rule : permission.getRules()) {
|
||||||
lookupGroup(rule);
|
lookupGroup(rule);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
boolean differs = !section.equals(config.getAccessSection(section.getName()));
|
|
||||||
config.replace(section);
|
config.replace(section);
|
||||||
toDelete.remove(section.getName());
|
toDelete.remove(section.getName());
|
||||||
return differs;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static Set<String> scanSectionNames(ProjectConfig config) {
|
private static Set<String> scanSectionNames(ProjectConfig config) {
|
||||||
@@ -264,13 +242,4 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
|||||||
}
|
}
|
||||||
return canWriteConfig;
|
return canWriteConfig;
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean groupMutationsDisallowed(Project.NameKey projectName) {
|
|
||||||
return (projectName.get().equals(allProjects.get())
|
|
||||||
|| projectName.get().equals(allUsers.get()));
|
|
||||||
}
|
|
||||||
|
|
||||||
private boolean isGroupMutation(String sectionName) {
|
|
||||||
return sectionName.startsWith(RefNames.REFS_GROUPS);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -38,7 +38,6 @@ import com.google.gerrit.server.change.ChangeResource;
|
|||||||
import com.google.gerrit.server.change.ChangesCollection;
|
import com.google.gerrit.server.change.ChangesCollection;
|
||||||
import com.google.gerrit.server.change.PostReviewers;
|
import com.google.gerrit.server.change.PostReviewers;
|
||||||
import com.google.gerrit.server.config.AllProjectsName;
|
import com.google.gerrit.server.config.AllProjectsName;
|
||||||
import com.google.gerrit.server.config.AllUsersName;
|
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.ProjectConfig;
|
import com.google.gerrit.server.git.ProjectConfig;
|
||||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||||
@@ -92,7 +91,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
|
|||||||
Provider<PostReviewers> reviewersProvider,
|
Provider<PostReviewers> reviewersProvider,
|
||||||
ProjectCache projectCache,
|
ProjectCache projectCache,
|
||||||
AllProjectsName allProjects,
|
AllProjectsName allProjects,
|
||||||
AllUsersName allUsers,
|
|
||||||
ChangesCollection changes,
|
ChangesCollection changes,
|
||||||
ChangeInserter.Factory changeInserterFactory,
|
ChangeInserter.Factory changeInserterFactory,
|
||||||
BatchUpdate.Factory updateFactory,
|
BatchUpdate.Factory updateFactory,
|
||||||
@@ -109,7 +107,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
|
|||||||
groupBackend,
|
groupBackend,
|
||||||
metaDataUpdateFactory,
|
metaDataUpdateFactory,
|
||||||
allProjects,
|
allProjects,
|
||||||
allUsers,
|
|
||||||
setParent,
|
setParent,
|
||||||
user.get(),
|
user.get(),
|
||||||
projectName,
|
projectName,
|
||||||
|
|||||||
Reference in New Issue
Block a user