opendev/gerrit
Code Issues Proposed changes

check.access: allow specifying permissions

Browse Source 
This allows us to verify permissions of a specific user without having
to read through project configurations.

Change-Id: Ibfe988391578dcadd74014ade181e7c15a484016
This commit is contained in:
Han-Wen Nienhuys
2018-01-18 19:27:46 +01:00
parent 033b6b1e51
commit 8a4a682aa1
4 changed files with 143 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
Documentation/rest-api-projects.txt
View File

@@ -1330,8 +1330,8 @@ link:#access-check-input[AccessCheckInput] entity.
----
The result is a link:#access-check-info[AccessCheckInfo] entity
detailing the read access of the given user for the given project (or
project-ref combination).
detailing the access of the given user for the given project,
project-ref, or project-permission-ref combination.
.Response
----
@@ -2752,7 +2752,10 @@ The `AccessCheckInput` entity is either an account or
|=========================================
|Field Name ||Description
|`account` ||The account for which to check access
|`ref` |optional|The refname for which to check access
|`ref` |optional|The refname for which to check
access
|`permission` |optional|The ref permission for which to
check. This defaults to `read`. If given, it `ref` must be given too.
|=========================================
[[ban-input]]

9
java/com/google/gerrit/extensions/api/config/AccessCheckInput.java
View File

@@ -18,13 +18,8 @@ import com.google.gerrit.common.Nullable;
public class AccessCheckInput {
public String account;
@Nullable public String ref;
public AccessCheckInput(String account, @Nullable String ref) {
this.account = account;
this.ref = ref;
}
public AccessCheckInput() {}
// If permission is given, ref must also be given.
@Nullable public String permission;
}

27
java/com/google/gerrit/server/restapi/project/CheckAccess.java
View File

@@ -36,6 +36,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -89,18 +90,38 @@ public class CheckAccess implements RestModifyView<ProjectResource, AccessCheckI
return info;
}
RefPermission refPerm = null;
if (!Strings.isNullOrEmpty(input.permission)) {
if (Strings.isNullOrEmpty(input.ref)) {
throw new BadRequestException("must set 'ref' when specifying 'permission'");
}
Optional<RefPermission> rp = RefPermission.fromName(input.permission);
if (!rp.isPresent()) {
throw new BadRequestException(
String.format("'%s' is not recognized as ref permission", input.permission));
}
refPerm = rp.get();
} else {
refPerm = RefPermission.READ;
}
if (!Strings.isNullOrEmpty(input.ref)) {
try {
permissionBackend
.user(user)
.ref(new Branch.NameKey(rsrc.getNameKey(), input.ref))
.check(RefPermission.READ);
.check(refPerm);
} catch (AuthException | PermissionBackendException e) {
info.status = HttpServletResponse.SC_FORBIDDEN;
info.message =
String.format(
"user %s (%s) cannot see ref %s in project %s",
user.getNameEmail(), user.getAccount().getId(), input.ref, rsrc.getName());
"user %s (%s) lacks permission %s for %s in project %s",
user.getNameEmail(),
user.getAccount().getId(),
input.permission,
input.ref,
rsrc.getName());
return info;
}
}

126
javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
View File

@@ -26,6 +26,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.SystemGroupBackend;
import java.util.List;
@@ -56,7 +57,6 @@ public class CheckAccessIT extends AbstractDaemonTest {
grant(secretProject, "refs/*", Permission.READ, false, privilegedGroup.getGroupUUID());
block(secretProject, "refs/*", Permission.READ, SystemGroupBackend.REGISTERED_USERS);
// deny/grant/block arg ordering is screwy.
deny(secretRefProject, "refs/*", Permission.READ, SystemGroupBackend.ANONYMOUS_USERS);
grant(
secretRefProject,
@@ -75,6 +75,17 @@ public class CheckAccessIT extends AbstractDaemonTest {
Permission.READ,
false,
SystemGroupBackend.REGISTERED_USERS);
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
// Ref permission
grant(
normalProject,
"refs/*",
Permission.VIEW_PRIVATE_CHANGES,
false,
privilegedGroup.getGroupUUID());
grant(normalProject, "refs/*", Permission.FORGE_SERVER, false, privilegedGroup.getGroupUUID());
}
@Test
@@ -84,24 +95,87 @@ public class CheckAccessIT extends AbstractDaemonTest {
gApi.projects().name(normalProject.get()).checkAccess(new AccessCheckInput());
}
@Test
public void nonexistentPermission() throws Exception {
exception.expect(BadRequestException.class);
exception.expectMessage("not recognized");
AccessCheckInput in = new AccessCheckInput();
in.account = user.email;
in.permission = "notapermission";
in.ref = "refs/heads/master";
gApi.projects().name(normalProject.get()).checkAccess(in);
}
@Test
public void permissionLacksRef() throws Exception {
exception.expect(BadRequestException.class);
exception.expectMessage("must set 'ref'");
AccessCheckInput in = new AccessCheckInput();
in.account = user.email;
in.permission = "forge_author";
gApi.projects().name(normalProject.get()).checkAccess(in);
}
@Test
public void changePermission() throws Exception {
exception.expect(BadRequestException.class);
exception.expectMessage("recognized as ref permission");
AccessCheckInput in = new AccessCheckInput();
in.account = user.email;
in.permission = "rebase";
in.ref = "refs/heads/master";
gApi.projects().name(normalProject.get()).checkAccess(in);
}
@Test
public void nonexistentEmail() throws Exception {
exception.expect(UnprocessableEntityException.class);
exception.expectMessage("cannot find account doesnotexist@invalid.com");
gApi.projects()
.name(normalProject.get())
.checkAccess(new AccessCheckInput("doesnotexist@invalid.com", null));
AccessCheckInput in = new AccessCheckInput();
in.account = "doesnotexist@invalid.com";
in.permission = "rebase";
in.ref = "refs/heads/master";
gApi.projects().name(normalProject.get()).checkAccess(in);
}
private static class TestCase {
AccessCheckInput input;
String project;
String permission;
int want;
TestCase(String mail, String project, String ref, int want) {
this.input = new AccessCheckInput(mail, ref);
this.project = project;
this.want = want;
static TestCase project(String mail, String project, int want) {
TestCase t = new TestCase();
t.input = new AccessCheckInput();
t.input.account = mail;
t.project = project;
t.want = want;
return t;
}
static TestCase projectRef(String mail, String project, String ref, int want) {
TestCase t = new TestCase();
t.input = new AccessCheckInput();
t.input.account = mail;
t.input.ref = ref;
t.project = project;
t.want = want;
return t;
}
static TestCase projectRefPerm(
String mail, String project, String ref, String permission, int want) {
TestCase t = new TestCase();
t.input = new AccessCheckInput();
t.input.account = mail;
t.input.ref = ref;
t.input.permission = permission;
t.project = project;
t.want = want;
return t;
}
}
@@ -109,13 +183,33 @@ public class CheckAccessIT extends AbstractDaemonTest {
public void accessible() throws Exception {
List<TestCase> inputs =
ImmutableList.of(
new TestCase(user.email, normalProject.get(), null, 200),
new TestCase(user.email, secretProject.get(), null, 403),
new TestCase(user.email, secretRefProject.get(), "refs/heads/secret/master", 403),
new TestCase(
TestCase.projectRefPerm(
user.email,
normalProject.get(),
"refs/heads/master",
Permission.VIEW_PRIVATE_CHANGES,
403),
TestCase.project(user.email, normalProject.get(), 200),
TestCase.project(user.email, secretProject.get(), 403),
TestCase.projectRef(
user.email, secretRefProject.get(), "refs/heads/secret/master", 403),
TestCase.projectRef(
privilegedUser.email, secretRefProject.get(), "refs/heads/secret/master", 200),
new TestCase(privilegedUser.email, normalProject.get(), null, 200),
new TestCase(privilegedUser.email, secretProject.get(), null, 200));
TestCase.projectRef(privilegedUser.email, normalProject.get(), null, 200),
TestCase.projectRef(privilegedUser.email, secretProject.get(), null, 200),
TestCase.projectRef(privilegedUser.email, secretProject.get(), null, 200),
TestCase.projectRefPerm(
privilegedUser.email,
normalProject.get(),
"refs/heads/master",
Permission.VIEW_PRIVATE_CHANGES,
200),
TestCase.projectRefPerm(
privilegedUser.email,
normalProject.get(),
"refs/heads/master",
Permission.FORGE_SERVER,
200));
for (TestCase tc : inputs) {
String in = newGson().toJson(tc.input);
@@ -135,7 +229,9 @@ public class CheckAccessIT extends AbstractDaemonTest {
switch (want) {
case 403:
assertThat(info.message).contains("cannot see");
if (tc.permission != null) {
assertThat(info.message).contains("lacks permission " + tc.permission);
}
break;
case 404:
assertThat(info.message).contains("does not exist");