opendev/gerrit
Code Issues Proposed changes

Merge "Fix: Permission.PUSH_MERGE checking for push bypassing Gerrit"

Browse Source
This commit is contained in:
Martin Fick
2013-09-25 19:39:12 +00:00
committed by Gerrit Code Review
parent 0ca10dd387 f6159bed86
commit 4f7634ee0f
5 changed files with 37 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
Documentation/access-control.txt
View File

@@ -609,12 +609,11 @@ push to happen. Some projects wish to restrict merges to being created
by Gerrit. By granting `Push` without `Push Merge Commit`, the only
merges that enter the system will be those created by Gerrit.
The reference name connected to a `Push Merge Commit` entry must always
be prefixed with `refs/for/`, for example `refs/for/refs/heads/BRANCH`.
This applies even for an entry that complements a `Push` entry for
`refs/heads/BRANCH` that allows direct pushes of non-merge commits, and
the intention of the `Push Merge Commit` entry is to allow direct pushes
of merge commits.
For pushing a merge commit for review, the reference name connected to
a `Push Merge Commit` entry must be prefixed with `refs/for/`,
for example `refs/for/refs/heads/BRANCH`. For pushing a merge commit
directly into the repository bypassing review, the reference name
must be `refs/heads/BRANCH`.
[[category_push_annotated]]

3
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -1986,7 +1986,8 @@ public class ReceiveCommits {
if (ctl.canForgeAuthor()
&& ctl.canForgeCommitter()
&& ctl.canForgeGerritServerIdentity()
&& ctl.canUploadMerges()
&& (MagicBranch.isForReview(cmd.getRefName()) ? ctl.canUploadMerges()
: ctl.canPushMerges())
&& !projectControl.getProjectState().isUseSignedOffBy()
&& Iterables.isEmpty(rejectCommits)
&& !GitRepositoryManager.REF_CONFIG.equals(ctl.getRefName())

18
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@@ -355,11 +355,21 @@ public class CommitValidators {
@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) {
if (MagicBranch.isForReview(receiveEvent.command.getRefName())) {
if (!refControl.canUploadMerges()) {
throw new CommitValidationException(
"you are not allowed to upload merges for review");
}
} else {
if (!refControl.canPushMerges()) {
throw new CommitValidationException(
"you are not allowed to push merges directly bypassing code review");
}
}
}
return Collections.<CommitValidationMessage>emptyList();
return Collections.<CommitValidationMessage> emptyList();
}
}

12
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@@ -147,7 +147,17 @@ public class RefControl {
&& canWrite();
}
/** @return true if this user can submit merge patch sets to this ref */
/**
* @return true if this user can push merge commits directly bypassing code
* review to this ref.
*/
public boolean canPushMerges() {
return projectControl.controlForRef(getRefName()).canPerform(
Permission.PUSH_MERGE)
&& canWrite();
}
/** @return true if this user can upload merge commits to this ref for review */
public boolean canUploadMerges() {
return projectControl.controlForRef("refs/for/" + getRefName())
.canPerform(Permission.PUSH_MERGE)

5
gerrit-server/src/main/java/com/google/gerrit/server/util/MagicBranch.java
View File

@@ -51,6 +51,11 @@ public final class MagicBranch {
|| refName.startsWith(NEW_CHANGE);
}
/** Checks if the supplied ref name is a magic branch for review */
public static boolean isForReview(String refName) {
return isMagicBranch(refName);
}
/** Returns the ref name prefix for a magic branch, <code>null</code> if the branch is not magic */
public static String getMagicRefNamePrefix(String refName) {
if (refName.startsWith(NEW_DRAFT_CHANGE)) {