diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index e8d5c15dea..888edc17c5 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -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 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 diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK index 6014118f43..d8e839a5ea 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK @@ -1,7 +1,7 @@ include_defs('//gerrit-acceptance-tests/tests.defs') acceptance_tests( - srcs = ['SubmitOnPushIT.java'], + srcs = ['DraftChangeBlockedIT.java', 'SubmitOnPushIT.java'], deps = [':util'], ) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java new file mode 100644 index 0000000000..578d48b7b0 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java @@ -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 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(); + } + } +} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java new file mode 100644 index 0000000000..086cfa46f8 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java @@ -0,0 +1,190 @@ +// 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.rest.project; + +import static com.google.gerrit.acceptance.git.GitUtil.createProject; +import static org.junit.Assert.assertEquals; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.AccountCreator; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.acceptance.RestSession; +import com.google.gerrit.acceptance.SshSession; +import com.google.gerrit.acceptance.TestAccount; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.config.AllProjectsNameProvider; +import com.google.gerrit.server.git.MetaDataUpdate; +import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.project.ProjectCache; +import com.google.inject.Inject; + +import org.apache.http.HttpStatus; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; + +public class CreateBranchIT extends AbstractDaemonTest { + + @Inject + private AccountCreator accounts; + + @Inject + private MetaDataUpdate.Server metaDataUpdateFactory; + + @Inject + private ProjectCache projectCache; + + @Inject + private GroupCache groupCache; + + @Inject + private AllProjectsNameProvider allProjects; + + private RestSession adminSession; + private RestSession userSession; + + private Project.NameKey project; + private Branch.NameKey branch; + + @Before + public void setUp() throws Exception { + TestAccount admin = accounts.admin(); + adminSession = new RestSession(server, admin); + + TestAccount user = accounts.create("user", "user@example.com", "User"); + userSession = new RestSession(server, user); + + project = new Project.NameKey("p"); + branch = new Branch.NameKey(project, "test"); + + SshSession sshSession = new SshSession(server, admin); + try { + createProject(sshSession, project.get(), null, true); + } finally { + sshSession.close(); + } + } + + @Test + public void createBranch_Forbidden() throws IOException { + RestResponse r = + userSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_FORBIDDEN, r.getStatusCode()); + } + + @Test + public void createBranchByAdmin() throws IOException { + RestResponse r = + adminSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_CREATED, r.getStatusCode()); + r.consume(); + + r = adminSession.get("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_OK, r.getStatusCode()); + } + + @Test + public void branchAlreadyExists_Conflict() throws IOException { + RestResponse r = + adminSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_CREATED, r.getStatusCode()); + r.consume(); + + r = adminSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_CONFLICT, r.getStatusCode()); + } + + @Test + public void createBranchByProjectOwner() throws IOException, + ConfigInvalidException { + grantOwner(); + + RestResponse r = + userSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_CREATED, r.getStatusCode()); + r.consume(); + + r = adminSession.get("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_OK, r.getStatusCode()); + } + + @Test + public void createBranchByAdminCreateReferenceBlocked() throws IOException, + ConfigInvalidException { + blockCreateReference(); + RestResponse r = + adminSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_CREATED, r.getStatusCode()); + r.consume(); + + r = adminSession.get("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_OK, r.getStatusCode()); + } + + @Test + public void createBranchByProjectOwnerCreateReferenceBlocked_Forbidden() + throws IOException, ConfigInvalidException { + grantOwner(); + blockCreateReference(); + RestResponse r = + userSession.put("/projects/" + project.get() + + "/branches/" + branch.getShortName()); + assertEquals(HttpStatus.SC_FORBIDDEN, r.getStatusCode()); + } + + private void blockCreateReference() throws IOException, ConfigInvalidException { + MetaDataUpdate md = metaDataUpdateFactory.create(allProjects.get()); + md.setMessage(String.format("Block %s", Permission.CREATE)); + ProjectConfig config = ProjectConfig.read(md); + AccessSection s = config.getAccessSection("refs/*", true); + Permission p = s.getPermission(Permission.CREATE, true); + PermissionRule rule = new PermissionRule(config.resolve( + groupCache.get(AccountGroup.ANONYMOUS_USERS))); + rule.setBlock(); + p.add(rule); + config.commit(md); + projectCache.evict(config.getProject()); + } + + private void grantOwner() throws IOException, ConfigInvalidException { + MetaDataUpdate md = metaDataUpdateFactory.create(project); + md.setMessage(String.format("Grant %s", Permission.OWNER)); + ProjectConfig config = ProjectConfig.read(md); + AccessSection s = config.getAccessSection("refs/*", true); + Permission p = s.getPermission(Permission.OWNER, true); + PermissionRule rule = new PermissionRule(config.resolve( + groupCache.get(AccountGroup.REGISTERED_USERS))); + p.add(rule); + config.commit(md); + projectCache.evict(config.getProject()); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 46afc52809..b4e96f36e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -50,6 +50,7 @@ import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.Capable; 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.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; @@ -1112,6 +1113,15 @@ public class ReceiveCommits { reject(cmd, "project is read only"); 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()) { errors.put(Error.CODE_REVIEW, ref); reject(cmd, "cannot upload review"); 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 849d6f2e3e..70a8d8bd0e 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 @@ -247,7 +247,8 @@ public class RefControl { } if (object instanceof RevCommit) { - return owner + return getCurrentUser().getCapabilities().canAdministrateServer() + || (owner && !isBlocked(Permission.CREATE)) || (canPerform(Permission.CREATE) && projectControl.canReadCommit(rw, (RevCommit) object)); } else if (object instanceof RevTag) { @@ -468,6 +469,15 @@ public class RefControl { /** True if the user has this permission. Works only for non labels. */ boolean canPerform(String permissionName) { + return doCanPerform(permissionName, false); + } + + /** True if the user is blocked from using this permission. */ + public boolean isBlocked(String permissionName) { + return !doCanPerform(permissionName, true); + } + + private boolean doCanPerform(String permissionName, boolean blockOnly) { List access = access(permissionName); Set allows = Sets.newHashSet(); Set blocks = Sets.newHashSet(); @@ -479,7 +489,7 @@ public class RefControl { } } blocks.removeAll(allows); - return blocks.isEmpty() && !allows.isEmpty(); + return blocks.isEmpty() && (!allows.isEmpty() || blockOnly); } /** True if the user has force this permission. Works only for non labels. */ 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 8f4e058139..fb9165e020 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 @@ -133,6 +133,31 @@ public class RefControlTest extends TestCase { 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() { grant(util.getParentConfig(), READ, REGISTERED, "refs/*"); grant(util.getParentConfig(), PUSH, REGISTERED, "refs/for/refs/*");