Add RefPermission.MERGE to check creating merge commits
Hide canUploadMerges() and instead use MERGE permission on the ref.
If the caller has MERGE they can upload merge commits, or create
merges with the REST API.
Change-Id: I2db15347d66c559b0487bf192a8245c2a2e69de7
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 c2b5947..a293ed1 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
@@ -111,7 +111,7 @@
IdentifiedUser user = refctl.getUser().asIdentifiedUser();
return new CommitValidators(
ImmutableList.of(
- new UploadMergesPermissionValidator(refctl),
+ new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl),
@@ -128,7 +128,7 @@
IdentifiedUser user = refctl.getUser().asIdentifiedUser();
return new CommitValidators(
ImmutableList.of(
- new UploadMergesPermissionValidator(refctl),
+ new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()),
@@ -155,7 +155,7 @@
// formats, so we play it safe and exclude them.
return new CommitValidators(
ImmutableList.of(
- new UploadMergesPermissionValidator(refControl),
+ new UploadMergesPermissionValidator(perm),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl)));
}
@@ -408,21 +408,29 @@
}
}
- /** Require permission to upload merges. */
+ /** Require permission to upload merge commits. */
public static class UploadMergesPermissionValidator implements CommitValidationListener {
- private final RefControl refControl;
+ private final PermissionBackend.ForRef perm;
- public UploadMergesPermissionValidator(RefControl refControl) {
- this.refControl = refControl;
+ public UploadMergesPermissionValidator(PermissionBackend.ForRef perm) {
+ this.perm = perm;
}
@Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
throws CommitValidationException {
- if (receiveEvent.commit.getParentCount() > 1 && !refControl.canUploadMerges()) {
- throw new CommitValidationException("you are not allowed to upload merges");
+ if (receiveEvent.commit.getParentCount() <= 1) {
+ return Collections.emptyList();
}
- return Collections.emptyList();
+ try {
+ perm.check(RefPermission.MERGE);
+ return Collections.emptyList();
+ } catch (AuthException e) {
+ throw new CommitValidationException("you are not allowed to upload merges");
+ } catch (PermissionBackendException e) {
+ log.error("cannot check MERGE", e);
+ throw new CommitValidationException("internal auth error");
+ }
}
}
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 4c738bf..e03272b 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 @@
FORGE_AUTHOR(Permission.FORGE_AUTHOR),
FORGE_COMMITTER(Permission.FORGE_COMMITTER),
FORGE_SERVER(Permission.FORGE_SERVER),
+ MERGE,
BYPASS_REVIEW,
/** Create a change to code review a commit. */
@@ -38,7 +39,7 @@
*
* <p>This is similar to {@link #UPDATE} except it constructs changes first, then submits them
* according to the submit strategy, which may include cherry-pick or rebase. By creating changes
- * for each commit, post-submit review is possible.
+ * for each commit, automatic server side rebase, and post-update review are enabled.
*/
UPDATE_BY_SUBMIT;
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 88caa9e..fa5720f 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
@@ -165,7 +165,7 @@
}
/** @return true if this user can submit merge patch sets to this ref */
- public boolean canUploadMerges() {
+ private boolean canUploadMerges() {
return projectControl
.controlForRef("refs/for/" + getRefName())
.canPerform(Permission.PUSH_MERGE)
@@ -725,12 +725,16 @@
return canUpdate();
case FORCE_UPDATE:
return canForceUpdate();
+
case FORGE_AUTHOR:
return canForgeAuthor();
case FORGE_COMMITTER:
return canForgeCommitter();
case FORGE_SERVER:
return canForgeGerritServerIdentity();
+ case MERGE:
+ return canUploadMerges();
+
case CREATE_CHANGE:
return canUpload();