diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index 578949a506..afd20bbf92 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -42,9 +42,9 @@ import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.CommitInfo; import com.google.gerrit.extensions.common.MergeInput; import com.google.gerrit.extensions.common.RevisionInfo; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.reviewdb.client.Branch; @@ -192,7 +192,7 @@ public class CreateChangeIT extends AbstractDaemonTest { ChangeInput in = newChangeInput(ChangeStatus.NEW); in.branch = "invisible-branch"; - assertCreateFails(in, AuthException.class, "cannot upload review"); + assertCreateFails(in, ResourceNotFoundException.class, ""); } @Test diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java index 3b620f1e20..afec3b634d 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java @@ -14,6 +14,10 @@ package com.google.gerrit.httpd.rpc.project; +import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; +import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; +import static com.google.gerrit.server.permissions.RefPermission.READ; + import com.google.common.collect.Maps; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.GroupDescription; @@ -39,10 +43,10 @@ import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.ProjectPermission; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; @@ -122,10 +126,11 @@ class ProjectAccessFactory extends Handler { } } - final RefControl metaConfigControl = pc.controlForRef(RefNames.REFS_CONFIG); List local = new ArrayList<>(); Set ownerOf = new HashSet<>(); Map visibleGroups = new HashMap<>(); + PermissionBackend.ForProject perm = permissionBackend.user(user).project(projectName); + boolean checkReadConfig = check(perm, RefNames.REFS_CONFIG, READ); for (AccessSection section : config.getAccessSections()) { String name = section.getName(); @@ -134,20 +139,19 @@ class ProjectAccessFactory extends Handler { local.add(section); ownerOf.add(name); - } else if (metaConfigControl.isVisible()) { + } else if (checkReadConfig) { local.add(section); } } else if (RefConfigSection.isValid(name)) { - RefControl rc = pc.controlForRef(name); - if (rc.isOwner()) { + if (pc.controlForRef(name).isOwner()) { local.add(section); ownerOf.add(name); - } else if (metaConfigControl.isVisible()) { + } else if (checkReadConfig) { local.add(section); - } else if (rc.isVisible()) { + } else if (check(perm, name, READ)) { // Filter the section to only add rules describing groups that // are visible to the current-user. This includes any group the // user is a member of, as well as groups they own or that @@ -205,17 +209,17 @@ class ProjectAccessFactory extends Handler { detail.setInheritsFrom(config.getProject().getParent(allProjectsName)); - if (projectName.equals(allProjectsName)) { - if (pc.isOwner()) { - ownerOf.add(AccessSection.GLOBAL_CAPABILITIES); - } + if (projectName.equals(allProjectsName) + && permissionBackend.user(user).testOrFalse(ADMINISTRATE_SERVER)) { + ownerOf.add(AccessSection.GLOBAL_CAPABILITIES); } detail.setLocal(local); detail.setOwnerOf(ownerOf); detail.setCanUpload( - metaConfigControl.isVisible() && (pc.isOwner() || metaConfigControl.canUpload())); - detail.setConfigVisible(pc.isOwner() || metaConfigControl.isVisible()); + pc.isOwner() + || (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE))); + detail.setConfigVisible(pc.isOwner() || checkReadConfig); detail.setGroupInfo(buildGroupInfo(local)); detail.setLabelTypes(pc.getLabelTypes()); detail.setFileHistoryLinks(getConfigFileLogLinks(projectName.get())); @@ -257,4 +261,14 @@ class ProjectAccessFactory extends Handler { } return pc; } + + private static boolean check(PermissionBackend.ForProject ctx, String ref, RefPermission perm) + throws PermissionBackendException { + try { + ctx.ref(ref).check(perm); + return true; + } catch (AuthException denied) { + return false; + } + } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java index 1a79d57518..4e2a4d3e6e 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java @@ -22,6 +22,7 @@ import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.errors.PermissionDeniedException; import com.google.gerrit.extensions.api.changes.AddReviewerInput; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; @@ -39,9 +40,11 @@ import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.group.SystemGroupBackend; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.project.SetParent; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.UpdateException; @@ -68,6 +71,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { } private final ReviewDb db; + private final PermissionBackend permissionBackend; private final Sequences seq; private final Provider reviewersProvider; private final ProjectCache projectCache; @@ -78,6 +82,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { @Inject ReviewProjectAccess( final ProjectControl.Factory projectControlFactory, + PermissionBackend permissionBackend, GroupBackend groupBackend, MetaDataUpdate.User metaDataUpdateFactory, ReviewDb db, @@ -107,6 +112,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { message, false); this.db = db; + this.permissionBackend = permissionBackend; this.seq = seq; this.reviewersProvider = reviewersProvider; this.projectCache = projectCache; @@ -124,13 +130,23 @@ public class ReviewProjectAccess extends ProjectAccessHandler { ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate) - throws IOException, OrmException, PermissionDeniedException { - RefControl refsMetaConfigControl = projectControl.controlForRef(RefNames.REFS_CONFIG); - if (!refsMetaConfigControl.isVisible()) { + throws IOException, OrmException, PermissionDeniedException, PermissionBackendException { + PermissionBackend.ForRef metaRef = + permissionBackend + .user(projectControl.getUser()) + .project(projectControl.getProject().getNameKey()) + .ref(RefNames.REFS_CONFIG); + try { + metaRef.check(RefPermission.READ); + } catch (AuthException denied) { throw new PermissionDeniedException(RefNames.REFS_CONFIG + " not visible"); } - if (!projectControl.isOwner() && !refsMetaConfigControl.canUpload()) { - throw new PermissionDeniedException("cannot upload to " + RefNames.REFS_CONFIG); + if (!projectControl.isOwner()) { + try { + metaRef.check(RefPermission.CREATE_CHANGE); + } catch (AuthException denied) { + throw new PermissionDeniedException("cannot create change for " + RefNames.REFS_CONFIG); + } } md.setInsertChangeId(true); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java index 024c6107c9..492d0e8c4a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java @@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.GetAccess; import com.google.inject.Inject; import java.io.IOException; @@ -48,11 +49,11 @@ public class ListAccess implements RestReadView { @Override public Map apply(TopLevelResource resource) - throws ResourceNotFoundException, ResourceConflictException, IOException { + throws ResourceNotFoundException, ResourceConflictException, IOException, + PermissionBackendException { Map access = new TreeMap<>(); for (String p : projects) { - Project.NameKey projectName = new Project.NameKey(p); - access.put(p, getAccess.apply(projectName)); + access.put(p, getAccess.apply(new Project.NameKey(p))); } return access; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 80d644f0fb..6a692d7e94 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -53,6 +53,7 @@ import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.PatchSetInfoFactory; +import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectControl; @@ -90,6 +91,7 @@ public class ChangeInserter implements InsertChangeOp { private static final Logger log = LoggerFactory.getLogger(ChangeInserter.class); + private final PermissionBackend permissionBackend; private final ProjectControl.GenericFactory projectControlFactory; private final IdentifiedUser.GenericFactory userFactory; private final ChangeControl.GenericFactory changeControlFactory; @@ -138,6 +140,7 @@ public class ChangeInserter implements InsertChangeOp { @Inject ChangeInserter( + PermissionBackend permissionBackend, ProjectControl.GenericFactory projectControlFactory, IdentifiedUser.GenericFactory userFactory, ChangeControl.GenericFactory changeControlFactory, @@ -154,6 +157,7 @@ public class ChangeInserter implements InsertChangeOp { @Assisted Change.Id changeId, @Assisted ObjectId commitId, @Assisted String refName) { + this.permissionBackend = permissionBackend; this.projectControlFactory = projectControlFactory; this.userFactory = userFactory; this.changeControlFactory = changeControlFactory; @@ -543,6 +547,8 @@ public class ChangeInserter implements InsertChangeOp { return; } + PermissionBackend.ForRef perm = + permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName); try { RefControl refControl = projectControlFactory.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName); @@ -555,7 +561,7 @@ public class ChangeInserter implements InsertChangeOp { commitId, ctx.getIdentifiedUser())) { commitValidatorsFactory - .forGerritCommits(refControl, new NoSshInfo(), ctx.getRevWalk()) + .forGerritCommits(perm, refControl, new NoSshInfo(), ctx.getRevWalk()) .validate(event); } } catch (CommitValidationException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 95491f5f4e..ae937c96d2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java @@ -14,23 +14,21 @@ package com.google.gerrit.server.change; -import com.google.gerrit.common.data.Capable; import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.RefNames; -import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.git.IntegrationException; -import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryingRestModifyView; @@ -45,59 +43,54 @@ import java.io.IOException; public class CherryPick extends RetryingRestModifyView implements UiAction { - private final Provider dbProvider; + private final PermissionBackend permissionBackend; + private final Provider user; private final CherryPickChange cherryPickChange; private final ChangeJson.Factory json; @Inject CherryPick( + PermissionBackend permissionBackend, + Provider user, RetryHelper retryHelper, - Provider dbProvider, CherryPickChange cherryPickChange, ChangeJson.Factory json) { super(retryHelper); - this.dbProvider = dbProvider; + this.permissionBackend = permissionBackend; + this.user = user; this.cherryPickChange = cherryPickChange; this.json = json; } @Override - protected ChangeInfo applyImpl( - BatchUpdate.Factory updateFactory, RevisionResource revision, CherryPickInput input) - throws OrmException, IOException, UpdateException, RestApiException { - final ChangeControl control = revision.getControl(); + public ChangeInfo applyImpl( + BatchUpdate.Factory updateFactory, RevisionResource rsrc, CherryPickInput input) + throws OrmException, IOException, UpdateException, RestApiException, + PermissionBackendException { input.parent = input.parent == null ? 1 : input.parent; - if (input.message == null || input.message.trim().isEmpty()) { throw new BadRequestException("message must be non-empty"); } else if (input.destination == null || input.destination.trim().isEmpty()) { throw new BadRequestException("destination must be non-empty"); } - if (!control.isVisible(dbProvider.get())) { - throw new AuthException("Cherry pick not permitted"); - } - - ProjectControl projectControl = control.getProjectControl(); - Capable capable = projectControl.canPushToAtLeastOneRef(); - if (capable != Capable.OK) { - throw new AuthException(capable.getMessage()); - } - - RefControl refControl = projectControl.controlForRef(RefNames.fullName(input.destination)); - if (!refControl.canUpload()) { - throw new AuthException( - "Not allowed to cherry pick " - + revision.getChange().getId().toString() - + " to " - + input.destination); - } + String refName = RefNames.fullName(input.destination); + CreateChange.checkValidCLA(rsrc.getControl().getProjectControl()); + permissionBackend + .user(user) + .project(rsrc.getChange().getProject()) + .ref(refName) + .check(RefPermission.CREATE_CHANGE); try { Change.Id cherryPickedChangeId = cherryPickChange.cherryPick( - updateFactory, revision.getChange(), revision.getPatchSet(), input, refControl); - return json.noOptions().format(revision.getProject(), cherryPickedChangeId); + updateFactory, + rsrc.getChange(), + rsrc.getPatchSet(), + input, + rsrc.getControl().getProjectControl().controlForRef(refName)); + return json.noOptions().format(rsrc.getProject(), cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage()); } catch (IntegrationException | NoSuchChangeException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java index 1b63cb503c..41f0463e59 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java @@ -15,27 +15,28 @@ package com.google.gerrit.server.change; import com.google.common.base.Strings; -import com.google.gerrit.common.data.Capable; import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; 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.git.IntegrationException; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.CommitResource; import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryingRestModifyView; import com.google.gerrit.server.update.UpdateException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; import org.eclipse.jgit.revwalk.RevCommit; @@ -43,14 +44,21 @@ import org.eclipse.jgit.revwalk.RevCommit; @Singleton public class CherryPickCommit extends RetryingRestModifyView { - + private final PermissionBackend permissionBackend; + private final Provider user; private final CherryPickChange cherryPickChange; private final ChangeJson.Factory json; @Inject CherryPickCommit( - RetryHelper retryHelper, CherryPickChange cherryPickChange, ChangeJson.Factory json) { + RetryHelper retryHelper, + Provider user, + CherryPickChange cherryPickChange, + ChangeJson.Factory json, + PermissionBackend permissionBackend) { super(retryHelper); + this.permissionBackend = permissionBackend; + this.user = user; this.cherryPickChange = cherryPickChange; this.json = json; } @@ -58,35 +66,40 @@ public class CherryPickCommit @Override public ChangeInfo applyImpl( BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input) - throws OrmException, IOException, UpdateException, RestApiException { + throws OrmException, IOException, UpdateException, RestApiException, + PermissionBackendException { RevCommit commit = rsrc.getCommit(); String message = Strings.nullToEmpty(input.message).trim(); input.message = message.isEmpty() ? commit.getFullMessage() : message; String destination = Strings.nullToEmpty(input.destination).trim(); input.parent = input.parent == null ? 1 : input.parent; + Project.NameKey projectName = rsrc.getProject().getProject().getNameKey(); if (destination.isEmpty()) { throw new BadRequestException("destination must be non-empty"); } - ProjectControl projectControl = rsrc.getProject(); - Capable capable = projectControl.canPushToAtLeastOneRef(); - if (capable != Capable.OK) { - throw new AuthException(capable.getMessage()); - } - String refName = RefNames.fullName(destination); - RefControl refControl = projectControl.controlForRef(refName); - if (!refControl.canUpload()) { - throw new AuthException("Not allowed to cherry pick " + commit + " to " + destination); - } + CreateChange.checkValidCLA(rsrc.getProject()); + permissionBackend + .user(user) + .project(projectName) + .ref(refName) + .check(RefPermission.CREATE_CHANGE); - Project.NameKey project = projectControl.getProject().getNameKey(); try { Change.Id cherryPickedChangeId = cherryPickChange.cherryPick( - updateFactory, null, null, null, null, project, commit, input, refControl); - return json.noOptions().format(project, cherryPickedChangeId); + updateFactory, + null, + null, + null, + null, + projectName, + commit, + input, + rsrc.getProject().controlForRef(refName)); + return json.noOptions().format(projectName, cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage()); } catch (IntegrationException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java index 599ce5efdb..cca9cb6e48 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java @@ -52,13 +52,14 @@ import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectResource; import com.google.gerrit.server.project.ProjectsCollection; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryingRestModifyView; @@ -89,13 +90,13 @@ import org.eclipse.jgit.util.ChangeIdUtil; @Singleton public class CreateChange extends RetryingRestModifyView> { - private final String anonymousCowardName; private final Provider db; private final GitRepositoryManager gitManager; private final AccountCache accountCache; private final Sequences seq; private final TimeZone serverTimeZone; + private final PermissionBackend permissionBackend; private final Provider user; private final ProjectsCollection projectsCollection; private final ChangeInserter.Factory changeInserterFactory; @@ -115,6 +116,7 @@ public class CreateChange AccountCache accountCache, Sequences seq, @GerritPersonIdent PersonIdent myIdent, + PermissionBackend permissionBackend, Provider user, ProjectsCollection projectsCollection, ChangeInserter.Factory changeInserterFactory, @@ -132,6 +134,7 @@ public class CreateChange this.accountCache = accountCache; this.seq = seq; this.serverTimeZone = myIdent.getTimeZone(); + this.permissionBackend = permissionBackend; this.user = user; this.projectsCollection = projectsCollection; this.changeInserterFactory = changeInserterFactory; @@ -165,26 +168,18 @@ public class CreateChange if (input.status != ChangeStatus.NEW && input.status != ChangeStatus.DRAFT) { throw new BadRequestException("unsupported change status"); } - if (!allowDrafts && input.status == ChangeStatus.DRAFT) { throw new MethodNotAllowedException("draft workflow is disabled"); } } - String refName = RefNames.fullName(input.branch); ProjectResource rsrc = projectsCollection.parse(input.project); - - Capable r = rsrc.getControl().canPushToAtLeastOneRef(); - if (r != Capable.OK) { - throw new AuthException(r.getMessage()); - } - - RefControl refControl = rsrc.getControl().controlForRef(refName); - if (!refControl.canUpload() || !refControl.isVisible()) { - throw new AuthException("cannot upload review"); - } + checkValidCLA(rsrc.getControl()); Project.NameKey project = rsrc.getNameKey(); + String refName = RefNames.fullName(input.branch); + permissionBackend.user(user).project(project).ref(refName).check(RefPermission.CREATE_CHANGE); + try (Repository git = gitManager.openRepository(project); ObjectInserter oi = git.newObjectInserter(); ObjectReader reader = oi.newReader(); @@ -345,4 +340,11 @@ public class CreateChange private static ObjectId emptyTreeId(ObjectInserter inserter) throws IOException { return inserter.insert(new TreeFormatter()); } + + static void checkValidCLA(ProjectControl ctl) throws AuthException { + Capable capable = ctl.canPushToAtLeastOneRef(); + if (capable != Capable.OK) { + throw new AuthException(capable.getMessage()); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index 581f2bae35..a5fe978945 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -320,6 +320,9 @@ public class PatchSetInserter implements BatchUpdateOp { return; } + PermissionBackend.ForRef perm = + permissionBackend.user(ctx.getUser()).ref(origCtl.getChange().getDest()); + String refName = getPatchSetId().toRefName(); try (CommitReceivedEvent event = new CommitReceivedEvent( @@ -333,7 +336,7 @@ public class PatchSetInserter implements BatchUpdateOp { commitId, ctx.getIdentifiedUser())) { commitValidatorsFactory - .forGerritCommits(origCtl.getRefControl(), new NoSshInfo(), ctx.getRevWalk()) + .forGerritCommits(perm, origCtl.getRefControl(), new NoSshInfo(), ctx.getRevWalk()) .validate(event); } catch (CommitValidationException e) { throw new ResourceConflictException(e.getFullMessage()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java index 658b87b603..eab06fb0d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java @@ -14,11 +14,9 @@ package com.google.gerrit.server.change; -import com.google.gerrit.common.data.Capable; import com.google.gerrit.extensions.api.changes.PublishChangeEditInput; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsPost; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ChildCollection; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.NotImplementedException; @@ -88,11 +86,7 @@ public class PublishChangeEdit protected Response applyImpl( BatchUpdate.Factory updateFactory, ChangeResource rsrc, PublishChangeEditInput in) throws IOException, OrmException, RestApiException, UpdateException { - Capable r = rsrc.getControl().getProjectControl().canPushToAtLeastOneRef(); - if (r != Capable.OK) { - throw new AuthException(r.getMessage()); - } - + CreateChange.checkValidCLA(rsrc.getControl().getProjectControl()); Optional edit = editUtil.byChange(rsrc.getChange()); if (!edit.isPresent()) { throw new ResourceConflictException( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java index 56d54ee3ed..af06054e03 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java @@ -14,19 +14,18 @@ package com.google.gerrit.server.change; +import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; + import com.google.common.base.Strings; import com.google.gerrit.common.TimeUtil; -import com.google.gerrit.common.data.Capable; import com.google.gerrit.extensions.api.changes.RevertInput; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; @@ -43,10 +42,10 @@ import com.google.gerrit.server.extensions.events.ChangeReverted; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.mail.send.RevertedSender; import com.google.gerrit.server.notedb.ReviewerStateInternal; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.ChangeContext; @@ -82,6 +81,8 @@ public class Revert extends RetryingRestModifyView db; + private final PermissionBackend permissionBackend; + private final Provider user; private final GitRepositoryManager repoManager; private final ChangeInserter.Factory changeInserterFactory; private final ChangeMessagesUtil cmUtil; @@ -96,6 +97,8 @@ public class Revert extends RetryingRestModifyView db, + PermissionBackend permissionBackend, + Provider user, GitRepositoryManager repoManager, ChangeInserter.Factory changeInserterFactory, ChangeMessagesUtil cmUtil, @@ -109,6 +112,8 @@ public class Revert extends RetryingRestModifyView reviewer = Sets.newLinkedHashSet(); Set cc = Sets.newLinkedHashSet(); Map labels = new HashMap<>(); @@ -1437,7 +1445,7 @@ public class ReceiveCommits { return ImmutableListMultimap.copyOf(pushOptions); } - private void parseMagicBranch(ReceiveCommand cmd) { + private void parseMagicBranch(ReceiveCommand cmd) throws PermissionBackendException { // Permit exactly one new change request per push. if (magicBranch != null) { reject(cmd, "duplicate request"); @@ -1488,6 +1496,7 @@ public class ReceiveCommits { magicBranch.dest = new Branch.NameKey(project.getNameKey(), ref); magicBranch.ctl = projectControl.controlForRef(ref); + magicBranch.perm = permissions.ref(ref); if (projectControl.getProject().getState() != com.google.gerrit.extensions.client.ProjectState.ACTIVE) { reject(cmd, "project is read only"); @@ -1508,9 +1517,11 @@ public class ReceiveCommits { } } - if (!magicBranch.ctl.canUpload()) { + try { + magicBranch.perm.check(RefPermission.CREATE_CHANGE); + } catch (AuthException denied) { errors.put(Error.CODE_REVIEW, ref); - reject(cmd, "cannot upload review"); + reject(cmd, denied.getMessage()); return; } @@ -1829,7 +1840,7 @@ public class ReceiveCommits { logDebug("Creating new change for {} even though it is already tracked", name); } - if (!validCommit(rp.getRevWalk(), magicBranch.ctl, magicBranch.cmd, c)) { + if (!validCommit(rp.getRevWalk(), magicBranch.perm, magicBranch.ctl, magicBranch.cmd, c)) { // Not a change the user can propose? Abort as early as possible. newChanges = Collections.emptyList(); logDebug("Aborting early due to invalid commit"); @@ -2407,8 +2418,9 @@ public class ReceiveCommits { } } - ChangeControl changeCtl = projectControl.controlFor(notes); - if (!validCommit(rp.getRevWalk(), changeCtl.getRefControl(), inputCommand, newCommit)) { + PermissionBackend.ForRef perm = permissions.ref(change.getDest().get()); + RefControl refctl = projectControl.controlForRef(change.getDest()); + if (!validCommit(rp.getRevWalk(), perm, refctl, inputCommand, newCommit)) { return false; } rp.getRevWalk().parseBody(priorCommit); @@ -2704,17 +2716,22 @@ public class ReceiveCommits { return true; } - private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) { - if (ctl.canForgeAuthor() - && ctl.canForgeCommitter() - && ctl.canForgeGerritServerIdentity() - && ctl.canUploadMerges() - && !projectControl.getProjectState().isUseSignedOffBy() - && Iterables.isEmpty(rejectCommits) - && !RefNames.REFS_CONFIG.equals(ctl.getRefName()) + private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) + throws PermissionBackendException { + PermissionBackend.ForRef perm = permissions.ref(ctl.getRefName()); + if (!RefNames.REFS_CONFIG.equals(cmd.getRefName()) && !(MagicBranch.isMagicBranch(cmd.getRefName()) - || NEW_PATCHSET.matcher(cmd.getRefName()).matches())) { - logDebug("Short-circuiting new commit validation"); + || NEW_PATCHSET.matcher(cmd.getRefName()).matches()) + && pushOptions.containsKey(BYPASS_REVIEW)) { + try { + perm.check(RefPermission.BYPASS_REVIEW); + if (!Iterables.isEmpty(rejectCommits)) { + throw new AuthException("reject-commits prevents " + BYPASS_REVIEW); + } + logDebug("Short-circuiting new commit validation"); + } catch (AuthException denied) { + reject(cmd, denied.getMessage()); + } return; } @@ -2735,7 +2752,7 @@ public class ReceiveCommits { i++; if (existing.keySet().contains(c)) { continue; - } else if (!validCommit(walk, ctl, cmd, c)) { + } else if (!validCommit(walk, perm, ctl, cmd, c)) { break; } @@ -2761,7 +2778,8 @@ public class ReceiveCommits { } } - private boolean validCommit(RevWalk rw, RefControl ctl, ReceiveCommand cmd, ObjectId id) + private boolean validCommit( + RevWalk rw, PermissionBackend.ForRef perm, RefControl ctl, ReceiveCommand cmd, ObjectId id) throws IOException { if (validCommits.contains(id)) { @@ -2779,8 +2797,8 @@ public class ReceiveCommits { && magicBranch.merged; CommitValidators validators = isMerged - ? commitValidatorsFactory.forMergedCommits(ctl) - : commitValidatorsFactory.forReceiveCommits(ctl, sshInfo, repo, rw); + ? commitValidatorsFactory.forMergedCommits(perm, ctl) + : commitValidatorsFactory.forReceiveCommits(perm, ctl, sshInfo, repo, rw); messages.addAll(validators.validate(receiveEvent)); } catch (CommitValidationException e) { logDebug("Commit validation failed on {}", c.name()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidationException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidationException.java index 07f3b218a7..bffe382d89 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidationException.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidationException.java @@ -22,6 +22,11 @@ public class CommitValidationException extends ValidationException { private static final long serialVersionUID = 1L; private final ImmutableList messages; + public CommitValidationException(String reason, CommitValidationMessage message) { + super(reason); + this.messages = ImmutableList.of(message); + } + public CommitValidationException(String reason, List messages) { super(reason); this.messages = ImmutableList.copyOf(messages); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java index c086f1cf7b..c2b5947155 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -26,6 +26,7 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo; import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.GerritPersonIdent; @@ -39,7 +40,11 @@ import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.git.BanCommit; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.ValidationError; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.ProjectControl; +import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.util.MagicBranch; @@ -96,38 +101,45 @@ public class CommitValidators { } public CommitValidators forReceiveCommits( - RefControl refControl, SshInfo sshInfo, Repository repo, RevWalk rw) throws IOException { + PermissionBackend.ForRef perm, + RefControl refctl, + SshInfo sshInfo, + Repository repo, + RevWalk rw) + throws IOException { NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw); + IdentifiedUser user = refctl.getUser().asIdentifiedUser(); return new CommitValidators( ImmutableList.of( - new UploadMergesPermissionValidator(refControl), - new AmendedGerritMergeCommitValidationListener(refControl, gerritIdent), - new AuthorUploaderValidator(refControl, canonicalWebUrl), - new CommitterUploaderValidator(refControl, canonicalWebUrl), - new SignedOffByValidator(refControl), - new ChangeIdValidator( - refControl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(refControl, rw, allUsers), + new UploadMergesPermissionValidator(refctl), + new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), + new AuthorUploaderValidator(user, perm, canonicalWebUrl), + new CommitterUploaderValidator(user, perm, canonicalWebUrl), + new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), + new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), + new ConfigValidator(refctl, rw, allUsers), new BannedCommitsValidator(rejectCommits), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker))); } - public CommitValidators forGerritCommits(RefControl refControl, SshInfo sshInfo, RevWalk rw) { + public CommitValidators forGerritCommits( + PermissionBackend.ForRef perm, RefControl refctl, SshInfo sshInfo, RevWalk rw) { + IdentifiedUser user = refctl.getUser().asIdentifiedUser(); return new CommitValidators( ImmutableList.of( - new UploadMergesPermissionValidator(refControl), - new AmendedGerritMergeCommitValidationListener(refControl, gerritIdent), - new AuthorUploaderValidator(refControl, canonicalWebUrl), - new SignedOffByValidator(refControl), - new ChangeIdValidator( - refControl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(refControl, rw, allUsers), + new UploadMergesPermissionValidator(refctl), + new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), + new AuthorUploaderValidator(user, perm, canonicalWebUrl), + new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), + new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), + new ConfigValidator(refctl, rw, allUsers), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker))); } - public CommitValidators forMergedCommits(RefControl refControl) { + public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, RefControl refControl) { + IdentifiedUser user = refControl.getUser().asIdentifiedUser(); // Generally only include validators that are based on permissions of the // user creating a change for a merged commit; generally exclude // validators that would require amending the change in order to correct. @@ -144,8 +156,8 @@ public class CommitValidators { return new CommitValidators( ImmutableList.of( new UploadMergesPermissionValidator(refControl), - new AuthorUploaderValidator(refControl, canonicalWebUrl), - new CommitterUploaderValidator(refControl, canonicalWebUrl))); + new AuthorUploaderValidator(user, perm, canonicalWebUrl), + new CommitterUploaderValidator(user, perm, canonicalWebUrl))); } } @@ -441,37 +453,50 @@ public class CommitValidators { } public static class SignedOffByValidator implements CommitValidationListener { - private final RefControl refControl; + private final IdentifiedUser user; + private final PermissionBackend.ForRef perm; + private final ProjectState state; - public SignedOffByValidator(RefControl refControl) { - this.refControl = refControl; + public SignedOffByValidator( + IdentifiedUser user, PermissionBackend.ForRef perm, ProjectState state) { + this.user = user; + this.perm = perm; + this.state = state; } @Override public List onCommitReceived(CommitReceivedEvent receiveEvent) throws CommitValidationException { - IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser(); - final PersonIdent committer = receiveEvent.commit.getCommitterIdent(); - final PersonIdent author = receiveEvent.commit.getAuthorIdent(); - final ProjectControl projectControl = refControl.getProjectControl(); + if (!state.isUseSignedOffBy()) { + return Collections.emptyList(); + } - if (projectControl.getProjectState().isUseSignedOffBy()) { - boolean sboAuthor = false; - boolean sboCommitter = false; - boolean sboMe = false; - for (final FooterLine footer : receiveEvent.commit.getFooterLines()) { - if (footer.matches(FooterKey.SIGNED_OFF_BY)) { - final String e = footer.getEmailAddress(); - if (e != null) { - sboAuthor |= author.getEmailAddress().equals(e); - sboCommitter |= committer.getEmailAddress().equals(e); - sboMe |= currentUser.hasEmailAddress(e); - } + RevCommit commit = receiveEvent.commit; + PersonIdent committer = commit.getCommitterIdent(); + PersonIdent author = commit.getAuthorIdent(); + + boolean sboAuthor = false; + boolean sboCommitter = false; + boolean sboMe = false; + for (FooterLine footer : commit.getFooterLines()) { + if (footer.matches(FooterKey.SIGNED_OFF_BY)) { + String e = footer.getEmailAddress(); + if (e != null) { + sboAuthor |= author.getEmailAddress().equals(e); + sboCommitter |= committer.getEmailAddress().equals(e); + sboMe |= user.hasEmailAddress(e); } } - if (!sboAuthor && !sboCommitter && !sboMe && !refControl.canForgeCommitter()) { + } + if (!sboAuthor && !sboCommitter && !sboMe) { + try { + perm.check(RefPermission.FORGE_COMMITTER); + } catch (AuthException denied) { throw new CommitValidationException( "not Signed-off-by author/committer/uploader in commit message footer"); + } catch (PermissionBackendException e) { + log.error("cannot check FORGE_COMMITTER", e); + throw new CommitValidationException("internal auth error"); } } return Collections.emptyList(); @@ -480,56 +505,69 @@ public class CommitValidators { /** Require that author matches the uploader. */ public static class AuthorUploaderValidator implements CommitValidationListener { - private final RefControl refControl; + private final IdentifiedUser user; + private final PermissionBackend.ForRef perm; private final String canonicalWebUrl; - public AuthorUploaderValidator(RefControl refControl, String canonicalWebUrl) { - this.refControl = refControl; + public AuthorUploaderValidator( + IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) { + this.user = user; + this.perm = perm; this.canonicalWebUrl = canonicalWebUrl; } @Override public List onCommitReceived(CommitReceivedEvent receiveEvent) throws CommitValidationException { - IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser(); - final PersonIdent author = receiveEvent.commit.getAuthorIdent(); - - if (!currentUser.hasEmailAddress(author.getEmailAddress()) && !refControl.canForgeAuthor()) { - List messages = new ArrayList<>(); - - messages.add( - getInvalidEmailError( - receiveEvent.commit, "author", author, currentUser, canonicalWebUrl)); - throw new CommitValidationException("invalid author", messages); + PersonIdent author = receiveEvent.commit.getAuthorIdent(); + if (user.hasEmailAddress(author.getEmailAddress())) { + return Collections.emptyList(); + } + try { + perm.check(RefPermission.FORGE_AUTHOR); + return Collections.emptyList(); + } catch (AuthException e) { + throw new CommitValidationException( + "invalid author", + invalidEmail(receiveEvent.commit, "author", author, user, canonicalWebUrl)); + } catch (PermissionBackendException e) { + log.error("cannot check FORGE_AUTHOR", e); + throw new CommitValidationException("internal auth error"); } - return Collections.emptyList(); } } /** Require that committer matches the uploader. */ public static class CommitterUploaderValidator implements CommitValidationListener { - private final RefControl refControl; + private final IdentifiedUser user; + private final PermissionBackend.ForRef perm; private final String canonicalWebUrl; - public CommitterUploaderValidator(RefControl refControl, String canonicalWebUrl) { - this.refControl = refControl; + public CommitterUploaderValidator( + IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) { + this.user = user; + this.perm = perm; this.canonicalWebUrl = canonicalWebUrl; } @Override public List onCommitReceived(CommitReceivedEvent receiveEvent) throws CommitValidationException { - IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser(); - final PersonIdent committer = receiveEvent.commit.getCommitterIdent(); - if (!currentUser.hasEmailAddress(committer.getEmailAddress()) - && !refControl.canForgeCommitter()) { - List messages = new ArrayList<>(); - messages.add( - getInvalidEmailError( - receiveEvent.commit, "committer", committer, currentUser, canonicalWebUrl)); - throw new CommitValidationException("invalid committer", messages); + PersonIdent committer = receiveEvent.commit.getCommitterIdent(); + if (user.hasEmailAddress(committer.getEmailAddress())) { + return Collections.emptyList(); + } + try { + perm.check(RefPermission.FORGE_COMMITTER); + return Collections.emptyList(); + } catch (AuthException e) { + throw new CommitValidationException( + "invalid committer", + invalidEmail(receiveEvent.commit, "committer", committer, user, canonicalWebUrl)); + } catch (PermissionBackendException e) { + log.error("cannot check FORGE_COMMITTER", e); + throw new CommitValidationException("internal auth error"); } - return Collections.emptyList(); } } @@ -539,25 +577,30 @@ public class CommitValidators { */ public static class AmendedGerritMergeCommitValidationListener implements CommitValidationListener { + private final PermissionBackend.ForRef perm; private final PersonIdent gerritIdent; - private final RefControl refControl; public AmendedGerritMergeCommitValidationListener( - final RefControl refControl, final PersonIdent gerritIdent) { - this.refControl = refControl; + PermissionBackend.ForRef perm, PersonIdent gerritIdent) { + this.perm = perm; this.gerritIdent = gerritIdent; } @Override public List onCommitReceived(CommitReceivedEvent receiveEvent) throws CommitValidationException { - final PersonIdent author = receiveEvent.commit.getAuthorIdent(); - + PersonIdent author = receiveEvent.commit.getAuthorIdent(); if (receiveEvent.commit.getParentCount() > 1 && author.getName().equals(gerritIdent.getName()) - && author.getEmailAddress().equals(gerritIdent.getEmailAddress()) - && !refControl.canForgeGerritServerIdentity()) { - throw new CommitValidationException("do not amend merges not made by you"); + && author.getEmailAddress().equals(gerritIdent.getEmailAddress())) { + try { + perm.check(RefPermission.FORGE_SERVER); + } catch (AuthException denied) { + throw new CommitValidationException("do not amend merges not made by you"); + } catch (PermissionBackendException e) { + log.error("cannot check FORGE_SERVER", e); + throw new CommitValidationException("internal auth error"); + } } return Collections.emptyList(); } @@ -629,7 +672,7 @@ public class CommitValidators { } } - private static CommitValidationMessage getInvalidEmailError( + private static CommitValidationMessage invalidEmail( RevCommit c, String type, PersonIdent who, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/ProjectPermission.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/ProjectPermission.java index 85b66c4a16..098b07a439 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/ProjectPermission.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/ProjectPermission.java @@ -32,7 +32,22 @@ public enum ProjectPermission { * *

This is a stronger form of {@link #ACCESS} where no filtering is required. */ - READ(Permission.READ); + READ(Permission.READ), + + /** + * Can create at least one reference in the project. + * + *

This project level permission only validates the user may create some type of reference + * within the project. The exact reference name must be checked at creation: + * + * 

permissionBackend
+   *    .user(user)
+   *    .project(proj)
+   *    .ref(ref)
+   *    .check(RefPermission.CREATE);
+   *
+ */ + CREATE_REF; private final String name; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefPermission.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefPermission.java index 37744b034f..127603fc12 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefPermission.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefPermission.java @@ -28,6 +28,7 @@ public enum RefPermission { FORGE_AUTHOR(Permission.FORGE_AUTHOR), FORGE_COMMITTER(Permission.FORGE_COMMITTER), FORGE_SERVER(Permission.FORGE_SERVER), + BYPASS_REVIEW, CREATE_CHANGE; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 2cb8d96ff6..65ba1ab056 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -42,6 +42,7 @@ import com.google.gerrit.server.permissions.ChangePermissionOrLabel; import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.PermissionBackend.ForChange; import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -281,14 +282,14 @@ public class ChangeControl { /** Can this user rebase this change? */ private boolean canRebase(ReviewDb db) throws OrmException { return (isOwner() || getRefControl().canSubmit(isOwner()) || getRefControl().canRebase()) - && getRefControl().canUpload() + && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) && !isPatchSetLocked(db); } /** Can this user restore this change? */ private boolean canRestore(ReviewDb db) throws OrmException { - return canAbandon(db) // Anyone who can abandon the change can restore it back - && getRefControl().canUpload(); // as long as you can upload too + // Anyone who can abandon the change can restore it, as long as they can create changes. + return canAbandon(db) && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE); } /** All available label types for this change. */ @@ -326,7 +327,7 @@ public class ChangeControl { /** Can this user add a patch set to this change? */ private boolean canAddPatchSet(ReviewDb db) throws OrmException { - if (!getRefControl().canUpload() + if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) || isPatchSetLocked(db) || !isPatchVisible(patchSetUtil.current(db, notes), db)) { return false; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java index 997239dddf..7c0795ee71 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java @@ -14,6 +14,11 @@ package com.google.gerrit.server.project; +import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; +import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_REF; +import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; +import static com.google.gerrit.server.permissions.RefPermission.READ; + import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.Iterables; import com.google.gerrit.common.data.AccessSection; @@ -25,6 +30,7 @@ import com.google.gerrit.extensions.api.access.AccessSectionInfo; import com.google.gerrit.extensions.api.access.PermissionInfo; import com.google.gerrit.extensions.api.access.PermissionRuleInfo; import com.google.gerrit.extensions.api.access.ProjectAccessInfo; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestReadView; @@ -37,6 +43,9 @@ import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.permissions.RefPermission; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -63,7 +72,8 @@ public class GetAccess implements RestReadView { PermissionRule.Action.INTERACTIVE, PermissionRuleInfo.Action.INTERACTIVE); - private final Provider self; + private final Provider user; + private final PermissionBackend permissionBackend; private final GroupControl.Factory groupControlFactory; private final AllProjectsName allProjectsName; private final ProjectJson projectJson; @@ -75,6 +85,7 @@ public class GetAccess implements RestReadView { @Inject public GetAccess( Provider self, + PermissionBackend permissionBackend, GroupControl.Factory groupControlFactory, AllProjectsName allProjectsName, ProjectCache projectCache, @@ -82,7 +93,8 @@ public class GetAccess implements RestReadView { ProjectJson projectJson, ProjectControl.GenericFactory projectControlFactory, GroupBackend groupBackend) { - this.self = self; + this.user = self; + this.permissionBackend = permissionBackend; this.groupControlFactory = groupControlFactory; this.allProjectsName = allProjectsName; this.projectJson = projectJson; @@ -93,9 +105,10 @@ public class GetAccess implements RestReadView { } public ProjectAccessInfo apply(Project.NameKey nameKey) - throws ResourceNotFoundException, ResourceConflictException, IOException { + throws ResourceNotFoundException, ResourceConflictException, IOException, + PermissionBackendException { try { - return apply(new ProjectResource(projectControlFactory.controlFor(nameKey, self.get()))); + return apply(new ProjectResource(projectControlFactory.controlFor(nameKey, user.get()))); } catch (NoSuchProjectException e) { throw new ResourceNotFoundException(nameKey.get()); } @@ -103,16 +116,18 @@ public class GetAccess implements RestReadView { @Override public ProjectAccessInfo apply(ProjectResource rsrc) - throws ResourceNotFoundException, ResourceConflictException, IOException { + throws ResourceNotFoundException, ResourceConflictException, IOException, + PermissionBackendException { // Load the current configuration from the repository, ensuring it's the most // recent version available. If it differs from what was in the project // state, force a cache flush now. - // + Project.NameKey projectName = rsrc.getNameKey(); ProjectAccessInfo info = new ProjectAccessInfo(); - ProjectConfig config; ProjectControl pc = createProjectControl(projectName); - RefControl metaConfigControl = pc.controlForRef(RefNames.REFS_CONFIG); + PermissionBackend.ForProject perm = permissionBackend.user(user).project(projectName); + + ProjectConfig config; try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) { config = ProjectConfig.read(md); @@ -121,10 +136,12 @@ public class GetAccess implements RestReadView { config.commit(md); projectCache.evict(config.getProject()); pc = createProjectControl(projectName); + perm = permissionBackend.user(user).project(projectName); } else if (config.getRevision() != null && !config.getRevision().equals(pc.getProjectState().getConfig().getRevision())) { projectCache.evict(config.getProject()); pc = createProjectControl(projectName); + perm = permissionBackend.user(user).project(projectName); } } catch (ConfigInvalidException e) { throw new ResourceConflictException(e.getMessage()); @@ -135,6 +152,7 @@ public class GetAccess implements RestReadView { info.local = new HashMap<>(); info.ownerOf = new HashSet<>(); Map visibleGroups = new HashMap<>(); + boolean checkReadConfig = check(perm, RefNames.REFS_CONFIG, READ); for (AccessSection section : config.getAccessSections()) { String name = section.getName(); @@ -143,20 +161,19 @@ public class GetAccess implements RestReadView { info.local.put(name, createAccessSection(section)); info.ownerOf.add(name); - } else if (metaConfigControl.isVisible()) { + } else if (checkReadConfig) { info.local.put(section.getName(), createAccessSection(section)); } } else if (RefConfigSection.isValid(name)) { - RefControl rc = pc.controlForRef(name); - if (rc.isOwner()) { + if (pc.controlForRef(name).isOwner()) { info.local.put(name, createAccessSection(section)); info.ownerOf.add(name); - } else if (metaConfigControl.isVisible()) { + } else if (checkReadConfig) { info.local.put(name, createAccessSection(section)); - } else if (rc.isVisible()) { + } else if (check(perm, name, READ)) { // Filter the section to only add rules describing groups that // are visible to the current-user. This includes any group the // user is a member of, as well as groups they own or that @@ -214,21 +231,32 @@ public class GetAccess implements RestReadView { info.inheritsFrom = projectJson.format(parent.getProject()); } - if (pc.getProject().getNameKey().equals(allProjectsName)) { - if (pc.isOwner()) { - info.ownerOf.add(AccessSection.GLOBAL_CAPABILITIES); - } + if (projectName.equals(allProjectsName) + && permissionBackend.user(user).testOrFalse(ADMINISTRATE_SERVER)) { + info.ownerOf.add(AccessSection.GLOBAL_CAPABILITIES); } info.isOwner = toBoolean(pc.isOwner()); info.canUpload = - toBoolean(pc.isOwner() || (metaConfigControl.isVisible() && metaConfigControl.canUpload())); - info.canAdd = toBoolean(pc.canAddRefs()); - info.configVisible = pc.isOwner() || metaConfigControl.isVisible(); + toBoolean( + pc.isOwner() + || (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE))); + info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF)); + info.configVisible = checkReadConfig || pc.isOwner(); return info; } + private static boolean check(PermissionBackend.ForProject ctx, String ref, RefPermission perm) + throws PermissionBackendException { + try { + ctx.ref(ref).check(perm); + return true; + } catch (AuthException denied) { + return false; + } + } + private AccessSectionInfo createAccessSection(AccessSection section) { AccessSectionInfo accessSectionInfo = new AccessSectionInfo(); accessSectionInfo.permissions = new HashMap<>(); @@ -255,7 +283,7 @@ public class GetAccess implements RestReadView { private ProjectControl createProjectControl(Project.NameKey projectName) throws IOException, ResourceNotFoundException { try { - return projectControlFactory.controlFor(projectName, self.get()); + return projectControlFactory.controlFor(projectName, user.get()); } catch (NoSuchProjectException e) { throw new ResourceNotFoundException(projectName.get()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java index 8773bad09f..44a3405c2a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java @@ -249,7 +249,7 @@ public class ProjectControl { return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN); } - public boolean canAddRefs() { + private boolean canAddRefs() { return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef()); } @@ -588,6 +588,9 @@ public class ProjectControl { case READ: return !isHidden() && allRefsAreVisible(Collections.emptySet()); + + case CREATE_REF: + return canAddRefs(); } throw new PermissionBackendException(perm + " unsupported"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index bf53ab19c9..c9e4989d9d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -151,12 +151,7 @@ public class RefControl { return blocks.isEmpty() && !allows.isEmpty(); } - /** - * Determines whether the user can upload a change to the ref controlled by this object. - * - * @return {@code true} if the user specified can upload a change to the Git ref - */ - public boolean canUpload() { + private boolean canUpload() { return projectControl.controlForRef("refs/for/" + getRefName()).canPerform(Permission.PUSH) && isProjectStatePermittingWrite(); } @@ -391,7 +386,7 @@ public class RefControl { } /** @return true if this user can forge the author line in a commit. */ - public boolean canForgeAuthor() { + private boolean canForgeAuthor() { if (canForgeAuthor == null) { canForgeAuthor = canPerform(Permission.FORGE_AUTHOR); } @@ -399,7 +394,7 @@ public class RefControl { } /** @return true if this user can forge the committer line in a commit. */ - public boolean canForgeCommitter() { + private boolean canForgeCommitter() { if (canForgeCommitter == null) { canForgeCommitter = canPerform(Permission.FORGE_COMMITTER); } @@ -407,7 +402,7 @@ public class RefControl { } /** @return true if this user can forge the server on the committer line. */ - public boolean canForgeGerritServerIdentity() { + private boolean canForgeGerritServerIdentity() { return canPerform(Permission.FORGE_SERVER); } @@ -738,6 +733,13 @@ public class RefControl { return canForgeGerritServerIdentity(); case CREATE_CHANGE: return canUpload(); + + case BYPASS_REVIEW: + return canForgeAuthor() + && canForgeCommitter() + && canForgeGerritServerIdentity() + && canUploadMerges() + && !projectControl.getProjectState().isUseSignedOffBy(); } throw new PermissionBackendException(perm + " unsupported"); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index ebd5b49629..e377574004 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -141,16 +141,18 @@ public class RefControlTest { assertThat(u.canPushToAtLeastOneRef()).named("can upload").isEqualTo(Capable.OK); } - private void assertCanUpload(String ref, ProjectControl u) { - assertThat(u.controlForRef(ref).canUpload()).named("can upload " + ref).isTrue(); + private void assertCreateChange(String ref, ProjectControl u) { + boolean create = u.asForProject().ref(ref).testOrFalse(RefPermission.CREATE_CHANGE); + assertThat(create).named("can create change " + ref).isTrue(); } private void assertCannotUpload(ProjectControl u) { assertThat(u.canPushToAtLeastOneRef()).named("cannot upload").isNotEqualTo(Capable.OK); } - private void assertCannotUpload(String ref, ProjectControl u) { - assertThat(u.controlForRef(ref).canUpload()).named("cannot upload " + ref).isFalse(); + private void assertCannotCreateChange(String ref, ProjectControl u) { + boolean create = u.asForProject().ref(ref).testOrFalse(RefPermission.CREATE_CHANGE); + assertThat(create).named("cannot create change " + ref).isFalse(); } private void assertBlocked(String p, String ref, ProjectControl u) { @@ -405,8 +407,8 @@ public class RefControlTest { ProjectControl u = user(local); assertCanUpload(u); - assertCanUpload("refs/heads/master", u); - assertCannotUpload("refs/heads/foobar", u); + assertCreateChange("refs/heads/master", u); + assertCannotCreateChange("refs/heads/foobar", u); } @Test @@ -415,7 +417,7 @@ public class RefControlTest { block(parent, PUSH, ANONYMOUS_USERS, "refs/drafts/*"); ProjectControl u = user(local); - assertCanUpload("refs/heads/master", u); + assertCreateChange("refs/heads/master", u); assertBlocked(PUSH, "refs/drafts/refs/heads/master", u); } @@ -438,8 +440,8 @@ public class RefControlTest { ProjectControl u = user(local); assertCanUpload(u); - assertCanUpload("refs/heads/master", u); - assertCanUpload("refs/heads/foobar", u); + assertCreateChange("refs/heads/master", u); + assertCreateChange("refs/heads/foobar", u); } @Test @@ -508,7 +510,7 @@ public class RefControlTest { ProjectControl u = user(local); assertCannotUpload(u); - assertCannotUpload("refs/heads/master", u); + assertCannotCreateChange("refs/heads/master", u); } @Test