Merge changes from topic 'permission-backend'

* changes:
  Hide ProjectControl.isHidden
  Add ProjectPermission.CREATE_CHANGE for cherry picks
  Add RefPermission.MERGE to check creating merge commits
  Hide RefControl.canSubmit, adding UPDATE_BY_SUBMIT
This commit is contained in:
David Pursehouse
2017-06-14 08:29:21 +00:00
committed by Gerrit Code Review
8 changed files with 76 additions and 24 deletions

View File

@@ -154,7 +154,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
@Test @Test
public void submitOnPushNotAllowed_Error() throws Exception { public void submitOnPushNotAllowed_Error() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master%submit"); PushOneCommit.Result r = pushTo("refs/for/master%submit");
r.assertErrorStatus("submit not allowed"); r.assertErrorStatus("update by submit not permitted");
} }
@Test @Test
@@ -166,7 +166,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
push( push(
"refs/for/master%submit", "refs/for/master%submit",
PushOneCommit.SUBJECT, "a.txt", "other content", r.getChangeId()); PushOneCommit.SUBJECT, "a.txt", "other content", r.getChangeId());
r.assertErrorStatus("submit not allowed"); r.assertErrorStatus("update by submit not permitted");
} }
@Test @Test

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.IntegrationException; import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; 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.permissions.RefPermission;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
@@ -99,10 +100,15 @@ public class CherryPick
} }
@Override @Override
public UiAction.Description getDescription(RevisionResource resource) { public UiAction.Description getDescription(RevisionResource rsrc) {
return new UiAction.Description() return new UiAction.Description()
.setLabel("Cherry Pick") .setLabel("Cherry Pick")
.setTitle("Cherry pick change to a different branch") .setTitle("Cherry pick change to a different branch")
.setVisible(resource.getControl().getProjectControl().canUpload() && resource.isCurrent()); .setVisible(
rsrc.isCurrent()
&& permissionBackend
.user(user)
.project(rsrc.getProject())
.testOrFalse(ProjectPermission.CREATE_CHANGE));
} }
} }

View File

@@ -1545,10 +1545,13 @@ public class ReceiveCommits {
return; return;
} }
if (magicBranch.submit if (magicBranch.submit) {
&& !projectControl.controlForRef(MagicBranch.NEW_CHANGE + ref).canSubmit(true)) { try {
reject(cmd, "submit not allowed"); permissions.ref(ref).check(RefPermission.UPDATE_BY_SUBMIT);
return; } catch (AuthException e) {
reject(cmd, e.getMessage());
return;
}
} }
RevWalk walk = rp.getRevWalk(); RevWalk walk = rp.getRevWalk();

View File

@@ -111,7 +111,7 @@ public class CommitValidators {
IdentifiedUser user = refctl.getUser().asIdentifiedUser(); IdentifiedUser user = refctl.getUser().asIdentifiedUser();
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(refctl), new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl), new CommitterUploaderValidator(user, perm, canonicalWebUrl),
@@ -128,7 +128,7 @@ public class CommitValidators {
IdentifiedUser user = refctl.getUser().asIdentifiedUser(); IdentifiedUser user = refctl.getUser().asIdentifiedUser();
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(refctl), new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()),
@@ -155,7 +155,7 @@ public class CommitValidators {
// formats, so we play it safe and exclude them. // formats, so we play it safe and exclude them.
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(refControl), new UploadMergesPermissionValidator(perm),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl))); new CommitterUploaderValidator(user, perm, canonicalWebUrl)));
} }
@@ -407,21 +407,29 @@ public class CommitValidators {
} }
} }
/** Require permission to upload merges. */ /** Require permission to upload merge commits. */
public static class UploadMergesPermissionValidator implements CommitValidationListener { public static class UploadMergesPermissionValidator implements CommitValidationListener {
private final RefControl refControl; private final PermissionBackend.ForRef perm;
public UploadMergesPermissionValidator(RefControl refControl) { public UploadMergesPermissionValidator(PermissionBackend.ForRef perm) {
this.refControl = refControl; this.perm = perm;
} }
@Override @Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent) public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
throws CommitValidationException { throws CommitValidationException {
if (receiveEvent.commit.getParentCount() > 1 && !refControl.canUploadMerges()) { if (receiveEvent.commit.getParentCount() <= 1) {
throw new CommitValidationException("you are not allowed to upload merges"); 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");
} }
return Collections.emptyList();
} }
} }

View File

@@ -47,7 +47,22 @@ public enum ProjectPermission {
* .check(RefPermission.CREATE); * .check(RefPermission.CREATE);
* </pre> * </pre>
*/ */
CREATE_REF; CREATE_REF,
/**
* Can create at least one change in the project.
*
* <p>This project level permission only validates the user may create a change for some branch
* within the project. The exact reference name must be checked at creation:
*
* <pre>permissionBackend
* .user(user)
* .project(proj)
* .ref(ref)
* .check(RefPermission.CREATE_CHANGE);
* </pre>
*/
CREATE_CHANGE;
private final String name; private final String name;

View File

@@ -28,9 +28,20 @@ public enum RefPermission {
FORGE_AUTHOR(Permission.FORGE_AUTHOR), FORGE_AUTHOR(Permission.FORGE_AUTHOR),
FORGE_COMMITTER(Permission.FORGE_COMMITTER), FORGE_COMMITTER(Permission.FORGE_COMMITTER),
FORGE_SERVER(Permission.FORGE_SERVER), FORGE_SERVER(Permission.FORGE_SERVER),
MERGE,
BYPASS_REVIEW, BYPASS_REVIEW,
CREATE_CHANGE; /** Create a change to code review a commit. */
CREATE_CHANGE,
/**
* Creates changes, then also immediately submits them during {@code push}.
*
* <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, automatic server side rebase, and post-update review are enabled.
*/
UPDATE_BY_SUBMIT;
private final String name; private final String name;

View File

@@ -245,7 +245,7 @@ public class ProjectControl {
} }
/** Returns whether the project is hidden. */ /** Returns whether the project is hidden. */
public boolean isHidden() { private boolean isHidden() {
return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN); return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
} }
@@ -253,7 +253,7 @@ public class ProjectControl {
return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef()); return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef());
} }
public boolean canUpload() { private boolean canCreateChanges() {
for (SectionMatcher matcher : access()) { for (SectionMatcher matcher : access()) {
AccessSection section = matcher.section; AccessSection section = matcher.section;
if (section.getName().startsWith("refs/for/")) { if (section.getName().startsWith("refs/for/")) {
@@ -591,6 +591,8 @@ public class ProjectControl {
case CREATE_REF: case CREATE_REF:
return canAddRefs(); return canAddRefs();
case CREATE_CHANGE:
return canCreateChanges();
} }
throw new PermissionBackendException(perm + " unsupported"); throw new PermissionBackendException(perm + " unsupported");
} }

View File

@@ -165,7 +165,7 @@ public class RefControl {
} }
/** @return true if this user can submit merge patch sets to this ref */ /** @return true if this user can submit merge patch sets to this ref */
public boolean canUploadMerges() { private boolean canUploadMerges() {
return projectControl return projectControl
.controlForRef("refs/for/" + getRefName()) .controlForRef("refs/for/" + getRefName())
.canPerform(Permission.PUSH_MERGE) .canPerform(Permission.PUSH_MERGE)
@@ -178,7 +178,7 @@ public class RefControl {
} }
/** @return true if this user can submit patch sets to this ref */ /** @return true if this user can submit patch sets to this ref */
public boolean canSubmit(boolean isChangeOwner) { boolean canSubmit(boolean isChangeOwner) {
if (RefNames.REFS_CONFIG.equals(refName)) { if (RefNames.REFS_CONFIG.equals(refName)) {
// Always allow project owners to submit configuration changes. // Always allow project owners to submit configuration changes.
// Submitting configuration changes modifies the access control // Submitting configuration changes modifies the access control
@@ -725,15 +725,22 @@ public class RefControl {
return canUpdate(); return canUpdate();
case FORCE_UPDATE: case FORCE_UPDATE:
return canForceUpdate(); return canForceUpdate();
case FORGE_AUTHOR: case FORGE_AUTHOR:
return canForgeAuthor(); return canForgeAuthor();
case FORGE_COMMITTER: case FORGE_COMMITTER:
return canForgeCommitter(); return canForgeCommitter();
case FORGE_SERVER: case FORGE_SERVER:
return canForgeGerritServerIdentity(); return canForgeGerritServerIdentity();
case MERGE:
return canUploadMerges();
case CREATE_CHANGE: case CREATE_CHANGE:
return canUpload(); return canUpload();
case UPDATE_BY_SUBMIT:
return projectControl.controlForRef("refs/for/" + getRefName()).canSubmit(true);
case BYPASS_REVIEW: case BYPASS_REVIEW:
return canForgeAuthor() return canForgeAuthor()
&& canForgeCommitter() && canForgeCommitter()