Merge changes I236a2969,I1c94aade,I448d9591 into stable-2.8
* changes: ReceiveCommits: Fix PUSH permission check for draft changes Don't allow project owners to create branches if create is blocked Add acceptance test for branch creation
This commit is contained in:
commit
28b367b126
@ -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
|
||||
|
@ -1,7 +1,7 @@
|
||||
include_defs('//gerrit-acceptance-tests/tests.defs')
|
||||
|
||||
acceptance_tests(
|
||||
srcs = ['SubmitOnPushIT.java'],
|
||||
srcs = ['DraftChangeBlockedIT.java', 'SubmitOnPushIT.java'],
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
@ -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());
|
||||
}
|
||||
}
|
@ -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");
|
||||
|
@ -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<PermissionRule> access = access(permissionName);
|
||||
Set<ProjectRef> allows = Sets.newHashSet();
|
||||
Set<ProjectRef> 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. */
|
||||
|
@ -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/*");
|
||||
|
Loading…
Reference in New Issue
Block a user