Prevent mutation of permissions on refs/groups through RPC
For groups in NoteDb, we manage group ownership through permissions on the group ref. During the migration, we prevent the mutation of Gerrit-managed permissions for group refs to ensure the ReviewDb and NoteDb data does not get out of sync. This commit prevents the mutation of access sections through RPCs which are used in the GWT UI. Change-Id: I3f124f6aa9c431a6d5bae72af5146fa835e3fd6e
This commit is contained in:
@@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.account.GroupBackend;
|
||||
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.git.MetaDataUpdate;
|
||||
import com.google.gerrit.server.git.ProjectConfig;
|
||||
@@ -61,6 +62,7 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
|
||||
GroupBackend groupBackend,
|
||||
MetaDataUpdate.User metaDataUpdateFactory,
|
||||
AllProjectsName allProjects,
|
||||
AllUsersName allUsers,
|
||||
Provider<SetParent> setParent,
|
||||
GitReferenceUpdated gitRefUpdated,
|
||||
ContributorAgreementsChecker contributorAgreements,
|
||||
@@ -75,6 +77,7 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
|
||||
groupBackend,
|
||||
metaDataUpdateFactory,
|
||||
allProjects,
|
||||
allUsers,
|
||||
setParent,
|
||||
user.get(),
|
||||
projectName,
|
||||
|
||||
@@ -31,10 +31,12 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
||||
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||
import com.google.gerrit.httpd.rpc.Handler;
|
||||
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.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.GroupBackends;
|
||||
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.ProjectConfig;
|
||||
import com.google.gerrit.server.permissions.PermissionBackend;
|
||||
@@ -64,6 +66,7 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
|
||||
private final MetaDataUpdate.User metaDataUpdateFactory;
|
||||
private final AllProjectsName allProjects;
|
||||
private final AllUsersName allUsers;
|
||||
private final Provider<SetParent> setParent;
|
||||
private final ContributorAgreementsChecker contributorAgreements;
|
||||
private final PermissionBackend permissionBackend;
|
||||
@@ -79,6 +82,7 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
GroupBackend groupBackend,
|
||||
MetaDataUpdate.User metaDataUpdateFactory,
|
||||
AllProjectsName allProjects,
|
||||
AllUsersName allUsers,
|
||||
Provider<SetParent> setParent,
|
||||
CurrentUser user,
|
||||
Project.NameKey projectName,
|
||||
@@ -92,6 +96,7 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
this.groupBackend = groupBackend;
|
||||
this.metaDataUpdateFactory = metaDataUpdateFactory;
|
||||
this.allProjects = allProjects;
|
||||
this.allUsers = allUsers;
|
||||
this.setParent = setParent;
|
||||
this.user = user;
|
||||
|
||||
@@ -136,12 +141,26 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
}
|
||||
|
||||
RefPattern.validate(name);
|
||||
|
||||
replace(config, toDelete, section);
|
||||
boolean differs = replace(config, toDelete, section);
|
||||
if (differs
|
||||
&& 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) {
|
||||
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 (!checkIfOwner || canWriteConfig()) {
|
||||
config.remove(config.getAccessSection(name));
|
||||
@@ -196,15 +215,18 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
throws IOException, NoSuchProjectException, ConfigInvalidException, OrmException,
|
||||
PermissionDeniedException, PermissionBackendException;
|
||||
|
||||
private void replace(ProjectConfig config, Set<String> toDelete, AccessSection section)
|
||||
/** @return true if the access section differed from the existing one and had to be replaced. */
|
||||
private boolean replace(ProjectConfig config, Set<String> toDelete, AccessSection section)
|
||||
throws NoSuchGroupException {
|
||||
for (Permission permission : section.getPermissions()) {
|
||||
for (PermissionRule rule : permission.getRules()) {
|
||||
lookupGroup(rule);
|
||||
}
|
||||
}
|
||||
boolean differs = !section.equals(config.getAccessSection(section.getName()));
|
||||
config.replace(section);
|
||||
toDelete.remove(section.getName());
|
||||
return differs;
|
||||
}
|
||||
|
||||
private static Set<String> scanSectionNames(ProjectConfig config) {
|
||||
@@ -241,4 +263,13 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
|
||||
}
|
||||
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,6 +38,7 @@ import com.google.gerrit.server.change.ChangeResource;
|
||||
import com.google.gerrit.server.change.ChangesCollection;
|
||||
import com.google.gerrit.server.change.PostReviewers;
|
||||
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.ProjectConfig;
|
||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||
@@ -90,6 +91,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
|
||||
Provider<PostReviewers> reviewersProvider,
|
||||
ProjectCache projectCache,
|
||||
AllProjectsName allProjects,
|
||||
AllUsersName allUsers,
|
||||
ChangesCollection changes,
|
||||
ChangeInserter.Factory changeInserterFactory,
|
||||
BatchUpdate.Factory updateFactory,
|
||||
@@ -106,6 +108,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
|
||||
groupBackend,
|
||||
metaDataUpdateFactory,
|
||||
allProjects,
|
||||
allUsers,
|
||||
setParent,
|
||||
user.get(),
|
||||
projectName,
|
||||
|
||||
Reference in New Issue
Block a user