Merge "Test that blocking approval for change owners on parent works" into stable-2.14
This commit is contained in:
@@ -848,6 +848,14 @@ public abstract class AbstractDaemonTest {
|
|||||||
return rule;
|
return rule;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected void blockLabel(
|
||||||
|
String label, int min, int max, AccountGroup.UUID id, String ref, Project.NameKey project)
|
||||||
|
throws Exception {
|
||||||
|
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
||||||
|
Util.block(cfg, Permission.LABEL + label, min, max, id, ref);
|
||||||
|
saveProjectConfig(project, cfg);
|
||||||
|
}
|
||||||
|
|
||||||
protected void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws Exception {
|
protected void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws Exception {
|
||||||
try (MetaDataUpdate md = metaDataUpdateFactory.create(p)) {
|
try (MetaDataUpdate md = metaDataUpdateFactory.create(p)) {
|
||||||
md.setAuthor(identifiedUserFactory.create(admin.getId()));
|
md.setAuthor(identifiedUserFactory.create(admin.getId()));
|
||||||
@@ -892,19 +900,22 @@ public abstract class AbstractDaemonTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
protected void grantLabel(
|
protected void grantLabel(
|
||||||
String permission,
|
String label,
|
||||||
int min,
|
int min,
|
||||||
int max,
|
int max,
|
||||||
Project.NameKey project,
|
Project.NameKey project,
|
||||||
String ref,
|
String ref,
|
||||||
boolean force,
|
boolean force,
|
||||||
AccountGroup.UUID groupUUID)
|
AccountGroup.UUID groupUUID,
|
||||||
|
boolean exclusive)
|
||||||
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
|
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
|
||||||
|
String permission = Permission.LABEL + label;
|
||||||
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
|
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
|
||||||
md.setMessage(String.format("Grant %s on %s", permission, ref));
|
md.setMessage(String.format("Grant %s on %s", permission, ref));
|
||||||
ProjectConfig config = ProjectConfig.read(md);
|
ProjectConfig config = ProjectConfig.read(md);
|
||||||
AccessSection s = config.getAccessSection(ref, true);
|
AccessSection s = config.getAccessSection(ref, true);
|
||||||
Permission p = s.getPermission(permission, true);
|
Permission p = s.getPermission(permission, true);
|
||||||
|
p.setExclusiveGroup(exclusive);
|
||||||
PermissionRule rule = Util.newRule(config, groupUUID);
|
PermissionRule rule = Util.newRule(config, groupUUID);
|
||||||
rule.setForce(force);
|
rule.setForce(force);
|
||||||
rule.setMin(min);
|
rule.setMin(min);
|
||||||
|
|||||||
@@ -27,7 +27,6 @@ import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
|
|||||||
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
|
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
|
||||||
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
|
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
|
||||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||||
import static com.google.gerrit.server.project.Util.blockLabel;
|
|
||||||
import static com.google.gerrit.server.project.Util.category;
|
import static com.google.gerrit.server.project.Util.category;
|
||||||
import static com.google.gerrit.server.project.Util.value;
|
import static com.google.gerrit.server.project.Util.value;
|
||||||
import static java.util.concurrent.TimeUnit.SECONDS;
|
import static java.util.concurrent.TimeUnit.SECONDS;
|
||||||
@@ -1812,7 +1811,7 @@ public class ChangeIT extends AbstractDaemonTest {
|
|||||||
assertThat(approval.value).isEqualTo(0);
|
assertThat(approval.value).isEqualTo(0);
|
||||||
|
|
||||||
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
||||||
blockLabel(cfg, "Code-Review", REGISTERED_USERS, "refs/heads/*");
|
Util.blockLabel(cfg, "Code-Review", REGISTERED_USERS, "refs/heads/*");
|
||||||
saveProjectConfig(project, cfg);
|
saveProjectConfig(project, cfg);
|
||||||
c = gApi.changes().id(triplet).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
|
c = gApi.changes().id(triplet).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
|
||||||
codeReview = c.labels.get("Code-Review");
|
codeReview = c.labels.get("Code-Review");
|
||||||
@@ -2435,7 +2434,7 @@ public class ChangeIT extends AbstractDaemonTest {
|
|||||||
@Test
|
@Test
|
||||||
public void maxPermittedValueBlocked() throws Exception {
|
public void maxPermittedValueBlocked() throws Exception {
|
||||||
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
||||||
blockLabel(cfg, "Code-Review", REGISTERED_USERS, "refs/heads/*");
|
Util.blockLabel(cfg, "Code-Review", REGISTERED_USERS, "refs/heads/*");
|
||||||
saveProjectConfig(project, cfg);
|
saveProjectConfig(project, cfg);
|
||||||
|
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
|
|||||||
@@ -19,10 +19,13 @@ import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
|
|||||||
import com.google.gerrit.acceptance.PushOneCommit;
|
import com.google.gerrit.acceptance.PushOneCommit;
|
||||||
import com.google.gerrit.acceptance.TestAccount;
|
import com.google.gerrit.acceptance.TestAccount;
|
||||||
import com.google.gerrit.acceptance.TestProjectInput;
|
import com.google.gerrit.acceptance.TestProjectInput;
|
||||||
import com.google.gerrit.common.data.Permission;
|
|
||||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||||
import com.google.gerrit.extensions.restapi.AuthException;
|
import com.google.gerrit.extensions.restapi.AuthException;
|
||||||
|
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||||
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||||
|
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
|
||||||
|
import org.eclipse.jgit.junit.TestRepository;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
@@ -39,21 +42,68 @@ public class ChangeOwnerIT extends AbstractDaemonTest {
|
|||||||
@Test
|
@Test
|
||||||
@TestProjectInput(cloneAs = "user")
|
@TestProjectInput(cloneAs = "user")
|
||||||
public void testChangeOwner_OwnerACLNotGranted() throws Exception {
|
public void testChangeOwner_OwnerACLNotGranted() throws Exception {
|
||||||
assertApproveFails(user, createMyChange());
|
assertApproveFails(user, createMyChange(testRepo));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestProjectInput(cloneAs = "user")
|
@TestProjectInput(cloneAs = "user")
|
||||||
public void testChangeOwner_OwnerACLGranted() throws Exception {
|
public void testChangeOwner_OwnerACLGranted() throws Exception {
|
||||||
grantApproveToChangeOwner();
|
grantApproveToChangeOwner(project);
|
||||||
approve(user, createMyChange());
|
approve(user, createMyChange(testRepo));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestProjectInput(cloneAs = "user")
|
@TestProjectInput(cloneAs = "user")
|
||||||
public void testChangeOwner_NotOwnerACLGranted() throws Exception {
|
public void testChangeOwner_NotOwnerACLGranted() throws Exception {
|
||||||
grantApproveToChangeOwner();
|
grantApproveToChangeOwner(project);
|
||||||
assertApproveFails(user2, createMyChange());
|
assertApproveFails(user2, createMyChange(testRepo));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testChangeOwner_OwnerACLGrantedOnParentProject() throws Exception {
|
||||||
|
setApiUser(admin);
|
||||||
|
grantApproveToChangeOwner(project);
|
||||||
|
Project.NameKey child = createProject("child", project);
|
||||||
|
|
||||||
|
setApiUser(user);
|
||||||
|
TestRepository<InMemoryRepository> childRepo = cloneProject(child, user);
|
||||||
|
approve(user, createMyChange(childRepo));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testChangeOwner_BlockedOnParentProject() throws Exception {
|
||||||
|
setApiUser(admin);
|
||||||
|
blockApproveForChangeOwner(project);
|
||||||
|
Project.NameKey child = createProject("child", project);
|
||||||
|
|
||||||
|
setApiUser(user);
|
||||||
|
grantApproveToAll(child);
|
||||||
|
TestRepository<InMemoryRepository> childRepo = cloneProject(child, user);
|
||||||
|
String changeId = createMyChange(childRepo);
|
||||||
|
|
||||||
|
// change owner cannot approve because Change-Owner group is blocked on parent
|
||||||
|
assertApproveFails(user, changeId);
|
||||||
|
|
||||||
|
// other user can approve
|
||||||
|
approve(user2, changeId);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testChangeOwner_BlockedOnParentProjectAndExclusiveAllowOnChild() throws Exception {
|
||||||
|
setApiUser(admin);
|
||||||
|
blockApproveForChangeOwner(project);
|
||||||
|
Project.NameKey child = createProject("child", project);
|
||||||
|
|
||||||
|
setApiUser(user);
|
||||||
|
grantExclusiveApproveToAll(child);
|
||||||
|
TestRepository<InMemoryRepository> childRepo = cloneProject(child, user);
|
||||||
|
String changeId = createMyChange(childRepo);
|
||||||
|
|
||||||
|
// change owner cannot approve because Change-Owner group is blocked on parent
|
||||||
|
assertApproveFails(user, changeId);
|
||||||
|
|
||||||
|
// other user can approve
|
||||||
|
approve(user2, changeId);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void approve(TestAccount a, String changeId) throws Exception {
|
private void approve(TestAccount a, String changeId) throws Exception {
|
||||||
@@ -70,18 +120,28 @@ public class ChangeOwnerIT extends AbstractDaemonTest {
|
|||||||
approve(a, changeId);
|
approve(a, changeId);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void grantApproveToChangeOwner() throws Exception {
|
private void grantApproveToChangeOwner(Project.NameKey project) throws Exception {
|
||||||
grantLabel(
|
grantApprove(project, SystemGroupBackend.CHANGE_OWNER, false);
|
||||||
Permission.LABEL + "Code-Review",
|
|
||||||
-2,
|
|
||||||
2,
|
|
||||||
project,
|
|
||||||
"refs/heads/*",
|
|
||||||
false,
|
|
||||||
SystemGroupBackend.CHANGE_OWNER);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private String createMyChange() throws Exception {
|
private void grantApproveToAll(Project.NameKey project) throws Exception {
|
||||||
|
grantApprove(project, SystemGroupBackend.REGISTERED_USERS, false);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void grantExclusiveApproveToAll(Project.NameKey project) throws Exception {
|
||||||
|
grantApprove(project, SystemGroupBackend.REGISTERED_USERS, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void grantApprove(Project.NameKey project, AccountGroup.UUID groupUUID, boolean exclusive)
|
||||||
|
throws Exception {
|
||||||
|
grantLabel("Code-Review", -2, 2, project, "refs/heads/*", false, groupUUID, exclusive);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void blockApproveForChangeOwner(Project.NameKey project) throws Exception {
|
||||||
|
blockLabel("Code-Review", -2, 2, SystemGroupBackend.CHANGE_OWNER, "refs/heads/*", project);
|
||||||
|
}
|
||||||
|
|
||||||
|
private String createMyChange(TestRepository<InMemoryRepository> testRepo) throws Exception {
|
||||||
PushOneCommit push = pushFactory.create(db, user.getIdent(), testRepo);
|
PushOneCommit push = pushFactory.create(db, user.getIdent(), testRepo);
|
||||||
return push.to("refs/for/master").getChangeId();
|
return push.to("refs/for/master").getChangeId();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -160,9 +160,19 @@ public class Util {
|
|||||||
|
|
||||||
public static PermissionRule blockLabel(
|
public static PermissionRule blockLabel(
|
||||||
ProjectConfig project, String labelName, AccountGroup.UUID group, String ref) {
|
ProjectConfig project, String labelName, AccountGroup.UUID group, String ref) {
|
||||||
|
return blockLabel(project, labelName, -1, 1, group, ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
public static PermissionRule blockLabel(
|
||||||
|
ProjectConfig project,
|
||||||
|
String labelName,
|
||||||
|
int min,
|
||||||
|
int max,
|
||||||
|
AccountGroup.UUID group,
|
||||||
|
String ref) {
|
||||||
PermissionRule r = grant(project, Permission.LABEL + labelName, newRule(project, group), ref);
|
PermissionRule r = grant(project, Permission.LABEL + labelName, newRule(project, group), ref);
|
||||||
r.setBlock();
|
r.setBlock();
|
||||||
r.setRange(-1, 1);
|
r.setRange(min, max);
|
||||||
return r;
|
return r;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user