ReceiveCommits: Fix PUSH permission check for draft changes
This change fixes PUSH permission check for blocking draft changes:
[access "refs/drafts/*"]
push = block group Anonymous Users
MagicBranchInput.parse() method updates the ref name, converting
the refs/drafts/master to refs/heads/master. All checks happens
with refs/heads/master. Only draft flag is set to true to mark the
change as draft. Use this flag and explicitly call
controlForRef("refs/drafts/refs/heads/master")
to check PUSH permission is not blocked for the ref.
Change-Id: I236a2969cf16e39198c26ebe1628de5313b660bd
This commit is contained in:
@@ -401,6 +401,14 @@ of adding individual reviewers before making the change public to all. The
|
|||||||
change page will have a 'Publish' button which allows you to convert individual
|
change page will have a 'Publish' button which allows you to convert individual
|
||||||
draft patch sets of a change into public patch sets for review.
|
draft patch sets of a change into public patch sets for review.
|
||||||
|
|
||||||
|
To block push permission to `refs/drafts/*` the following permission rule can
|
||||||
|
be configured:
|
||||||
|
|
||||||
|
====
|
||||||
|
[access "refs/drafts/*"]
|
||||||
|
push = block group Anonymous Users
|
||||||
|
====
|
||||||
|
|
||||||
|
|
||||||
[[access_categories]]
|
[[access_categories]]
|
||||||
Access Categories
|
Access Categories
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
include_defs('//gerrit-acceptance-tests/tests.defs')
|
include_defs('//gerrit-acceptance-tests/tests.defs')
|
||||||
|
|
||||||
acceptance_tests(
|
acceptance_tests(
|
||||||
srcs = ['SubmitOnPushIT.java'],
|
srcs = ['DraftChangeBlockedIT.java', 'SubmitOnPushIT.java'],
|
||||||
deps = [':util'],
|
deps = [':util'],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,115 @@
|
|||||||
|
// Copyright (C) 2014 The Android Open Source Project
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
package com.google.gerrit.acceptance.git;
|
||||||
|
|
||||||
|
import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
|
||||||
|
import static com.google.gerrit.acceptance.git.GitUtil.createProject;
|
||||||
|
import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
|
||||||
|
import static com.google.gerrit.server.project.Util.ANONYMOUS;
|
||||||
|
import static com.google.gerrit.server.project.Util.grant;
|
||||||
|
|
||||||
|
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||||
|
import com.google.gerrit.acceptance.AccountCreator;
|
||||||
|
import com.google.gerrit.acceptance.SshSession;
|
||||||
|
import com.google.gerrit.acceptance.TestAccount;
|
||||||
|
import com.google.gerrit.common.data.Permission;
|
||||||
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
|
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.project.ProjectCache;
|
||||||
|
import com.google.gwtorm.server.OrmException;
|
||||||
|
import com.google.gwtorm.server.SchemaFactory;
|
||||||
|
import com.google.inject.Inject;
|
||||||
|
|
||||||
|
import org.eclipse.jgit.api.Git;
|
||||||
|
import org.eclipse.jgit.api.errors.GitAPIException;
|
||||||
|
import org.junit.Before;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
import java.io.IOException;
|
||||||
|
|
||||||
|
public class DraftChangeBlockedIT extends AbstractDaemonTest {
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
private AccountCreator accounts;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
private SchemaFactory<ReviewDb> reviewDbProvider;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
private ProjectCache projectCache;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
private AllProjectsName allProjects;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
private MetaDataUpdate.Server metaDataUpdateFactory;
|
||||||
|
|
||||||
|
private TestAccount admin;
|
||||||
|
private Project.NameKey project;
|
||||||
|
private Git git;
|
||||||
|
private ReviewDb db;
|
||||||
|
|
||||||
|
@Before
|
||||||
|
public void setUp() throws Exception {
|
||||||
|
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
|
||||||
|
grant(cfg, Permission.PUSH, ANONYMOUS,
|
||||||
|
"refs/drafts/*").setBlock();
|
||||||
|
saveProjectConfig(cfg);
|
||||||
|
|
||||||
|
project = new Project.NameKey("p");
|
||||||
|
admin = accounts.admin();
|
||||||
|
initSsh(admin);
|
||||||
|
SshSession sshSession = new SshSession(server, admin);
|
||||||
|
createProject(sshSession, project.get());
|
||||||
|
|
||||||
|
db = reviewDbProvider.open();
|
||||||
|
git = cloneProject(sshSession.getUrl() + "/" + project.get());
|
||||||
|
sshSession.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPushDraftChange_Blocked() throws GitAPIException,
|
||||||
|
OrmException, IOException {
|
||||||
|
// create draft by pushing to 'refs/drafts/'
|
||||||
|
PushOneCommit.Result r = pushTo("refs/drafts/master");
|
||||||
|
r.assertErrorStatus("cannot upload drafts");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPushDraftChangeMagic_Blocked() throws GitAPIException,
|
||||||
|
OrmException, IOException {
|
||||||
|
// create draft by using 'draft' option
|
||||||
|
PushOneCommit.Result r = pushTo("refs/for/master%draft");
|
||||||
|
r.assertErrorStatus("cannot upload drafts");
|
||||||
|
}
|
||||||
|
|
||||||
|
private PushOneCommit.Result pushTo(String ref) throws GitAPIException,
|
||||||
|
IOException {
|
||||||
|
PushOneCommit push = new PushOneCommit(db, admin.getIdent());
|
||||||
|
return push.to(git, ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void saveProjectConfig(ProjectConfig cfg) throws IOException {
|
||||||
|
MetaDataUpdate md = metaDataUpdateFactory.create(allProjects);
|
||||||
|
try {
|
||||||
|
cfg.commit(md);
|
||||||
|
} finally {
|
||||||
|
md.close();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -50,6 +50,7 @@ import com.google.gerrit.common.ChangeHooks;
|
|||||||
import com.google.gerrit.common.Nullable;
|
import com.google.gerrit.common.Nullable;
|
||||||
import com.google.gerrit.common.data.Capable;
|
import com.google.gerrit.common.data.Capable;
|
||||||
import com.google.gerrit.common.data.LabelTypes;
|
import com.google.gerrit.common.data.LabelTypes;
|
||||||
|
import com.google.gerrit.common.data.Permission;
|
||||||
import com.google.gerrit.common.data.PermissionRule;
|
import com.google.gerrit.common.data.PermissionRule;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
import com.google.gerrit.reviewdb.client.Branch;
|
import com.google.gerrit.reviewdb.client.Branch;
|
||||||
@@ -1112,6 +1113,15 @@ public class ReceiveCommits {
|
|||||||
reject(cmd, "project is read only");
|
reject(cmd, "project is read only");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (magicBranch.isDraft()
|
||||||
|
&& projectControl.controlForRef("refs/drafts/" + ref)
|
||||||
|
.isBlocked(Permission.PUSH)) {
|
||||||
|
errors.put(Error.CODE_REVIEW, ref);
|
||||||
|
reject(cmd, "cannot upload drafts");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (!magicBranch.ctl.canUpload()) {
|
if (!magicBranch.ctl.canUpload()) {
|
||||||
errors.put(Error.CODE_REVIEW, ref);
|
errors.put(Error.CODE_REVIEW, ref);
|
||||||
reject(cmd, "cannot upload review");
|
reject(cmd, "cannot upload review");
|
||||||
|
|||||||
@@ -133,6 +133,31 @@ public class RefControlTest extends TestCase {
|
|||||||
u.controlForRef("refs/heads/foobar").canUpload());
|
u.controlForRef("refs/heads/foobar").canUpload());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void testBlockPushDrafts() {
|
||||||
|
grant(util.getParentConfig(), PUSH, REGISTERED, "refs/for/refs/*");
|
||||||
|
grant(util.getParentConfig(), PUSH, ANONYMOUS, "refs/drafts/*")
|
||||||
|
.setBlock();
|
||||||
|
|
||||||
|
ProjectControl u = util.user(local);
|
||||||
|
assertTrue("can upload refs/heads/master",
|
||||||
|
u.controlForRef("refs/heads/master").canUpload());
|
||||||
|
assertTrue("push is blocked to refs/drafts/master",
|
||||||
|
u.controlForRef("refs/drafts/refs/heads/master").isBlocked(PUSH));
|
||||||
|
}
|
||||||
|
|
||||||
|
public void testBlockPushDraftsUnblockAdmin() {
|
||||||
|
grant(util.getParentConfig(), PUSH, ANONYMOUS, "refs/drafts/*")
|
||||||
|
.setBlock();
|
||||||
|
grant(util.getParentConfig(), PUSH, ADMIN, "refs/drafts/*");
|
||||||
|
|
||||||
|
assertTrue("push is blocked for anonymous to refs/drafts/master",
|
||||||
|
util.user(local).controlForRef("refs/drafts/refs/heads/master")
|
||||||
|
.isBlocked(PUSH));
|
||||||
|
assertFalse("push is blocked for admin refs/drafts/master",
|
||||||
|
util.user(local, "a", ADMIN).controlForRef("refs/drafts/refs/heads/master")
|
||||||
|
.isBlocked(PUSH));
|
||||||
|
}
|
||||||
|
|
||||||
public void testInheritRead_SingleBranchDoesNotOverrideInherited() {
|
public void testInheritRead_SingleBranchDoesNotOverrideInherited() {
|
||||||
grant(util.getParentConfig(), READ, REGISTERED, "refs/*");
|
grant(util.getParentConfig(), READ, REGISTERED, "refs/*");
|
||||||
grant(util.getParentConfig(), PUSH, REGISTERED, "refs/for/refs/*");
|
grant(util.getParentConfig(), PUSH, REGISTERED, "refs/for/refs/*");
|
||||||
|
|||||||
Reference in New Issue
Block a user