Move CheckAccess endpoint into /projects/

This ensures proper load balancing on multi-master setups with
repo-affine request routing. Since the original REST endpoint was
added by Icf74b0fc40 which is not part of any release yet, this
change will not break backwards compatibility.

Change-Id: Ie1be1173ff8ab2a4c79e568834c6a1d182d79883
This commit is contained in:
Han-Wen Nienhuys
2017-09-04 18:13:23 +02:00
committed by Changcheng Xiao
parent 2def676ee6
commit d1ed08d46a
11 changed files with 156 additions and 157 deletions

View File

@@ -193,45 +193,6 @@ is returned that contains detected consistency problems.
}
----
[[check-access]]
=== Check Access
--
'POST /config/server/check.access'
--
Runs access checks for other users.
Input for the access checks that should be run must be provided in
the request body inside a
link:#access-check-input[AccessCheckInput] entity.
.Request
----
POST /config/server/check HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"project": "medium",
"account": "Kristen.Burns@gerritcodereview.com",
"ref": "refs/heads/secret/bla"
}
----
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).
.Response
----
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
)]}'
{
"message": "user Kristen Burns \u003cKristen.Burns@gerritcodereview.com\u003e (1000098) cannot see ref refs/heads/secret/master in project medium",
"status": 403
}
----
[[confirm-email]]
=== Confirm Email
@@ -1297,33 +1258,6 @@ The ID of the task (hex string).
[[json-entities]]
== JSON Entities
[[access-check-info]]
=== AccessCheckInfo
The `AccessCheckInfo` entity is the result of a
an access check.
[options="header",cols="1,^1,5"]
|=========================================
|Field Name ||Description
|`status`||The HTTP status code for the access.
200 means success, 403 means denied and 404 means the project does not
exist.
|`message`|optional|A clarifying message if `status` is not 200.
|=========================================
[[access-check-input]]
=== AccessCheckInput
The `AccessCheckInput` entity is a tuple of (account, project) or
(account, project, ref) for which we want to check access.
[options="header",cols="1,^1,5"]
|=========================================
|Field Name ||Description
|`account`||The account for which to check access
|`project`||The project for which to check access
|`ref`|optional|The refname for which to check access
|=========================================
[[accounts-config-info]]
=== AccountsConfigInfo
The `AccountsConfigInfo` entity contains information about Gerrit

View File

@@ -1112,7 +1112,6 @@ As result a link:#project-access-info[ProjectAccessInfo] entity is returned.
}
----
[[create-access-change]]
=== Create Access Rights Change for review.
--
@@ -1178,6 +1177,46 @@ a link:#change-info[ChangeInfo] entity describing the resulting change.
}
----
[[check-access]]
=== Check Access
--
'POST /projects/MyProject/check.access'
--
Runs access checks for other users. This requires the
link:access-control.html#capability_administrateServer[Administrate Server]
global capability.
Input for the access checks that should be run must be provided in
the request body inside a
link:#access-check-input[AccessCheckInput] entity.
.Request
----
POST /projects/MyProject/check.access HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"account": "Kristen.Burns@gerritcodereview.com",
"ref": "refs/heads/secret/bla"
}
----
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).
.Response
----
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
)]}'
{
"message": "user Kristen Burns \u003cKristen.Burns@gerritcodereview.com\u003e (1000098) cannot see ref refs/heads/secret/bla in project MyProject",
"status": 403
}
----
[[index]]
=== Index all changes in a project
@@ -2564,6 +2603,31 @@ If the name ends with `.git`, the suffix will be automatically removed.
[[json-entities]]
== JSON Entities
[[access-check-info]]
=== AccessCheckInfo
The `AccessCheckInfo` entity is the result of an access check.
[options="header",cols="1,^1,5"]
|=========================================
|Field Name ||Description
|`status` ||The HTTP status code for the access.
200 means success, 403 means denied and 404 means the project does not exist.
|`message` |optional|A clarifying message if `status` is not 200.
|=========================================
[[access-check-input]]
=== AccessCheckInput
The `AccessCheckInput` entity is either an account or
(account, ref) tuple for which we want to check access.
[options="header",cols="1,^1,5"]
|=========================================
|Field Name ||Description
|`account` ||The account for which to check access
|`ref` |optional|The refname for which to check access
|=========================================
[[ban-input]]
=== BanInput
The `BanInput` entity contains information for banning commits in a

View File

@@ -12,24 +12,24 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.rest.account;
package com.google.gerrit.acceptance.api.project;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
import com.google.gerrit.extensions.api.config.AccessCheckInput;
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.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.group.SystemGroupBackend;
import java.util.List;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
@@ -79,46 +79,59 @@ public class CheckAccessIT extends AbstractDaemonTest {
}
@Test
public void invalidInputs() {
List<AccessCheckInput> inputs =
ImmutableList.of(
new AccessCheckInput(),
new AccessCheckInput(user.email, null, null),
new AccessCheckInput(null, normalProject.toString(), null),
new AccessCheckInput("doesnotexist@invalid.com", normalProject.toString(), null));
for (AccessCheckInput input : inputs) {
try {
gApi.config().server().checkAccess(input);
fail(String.format("want RestApiException for %s", newGson().toJson(input)));
} catch (RestApiException e) {
public void emptyInput() throws Exception {
exception.expect(BadRequestException.class);
exception.expectMessage("input requires 'account'");
gApi.projects().name(normalProject.get()).checkAccess(new AccessCheckInput());
}
}
@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));
}
private static class TestCase {
AccessCheckInput input;
String project;
int want;
TestCase(String mail, String project, String ref, int want) {
this.input = new AccessCheckInput(mail, ref);
this.project = project;
this.want = want;
}
}
@Test
public void accessible() {
Map<AccessCheckInput, Integer> inputs =
ImmutableMap.of(
new AccessCheckInput(user.email, normalProject.get(), null), 200,
new AccessCheckInput(user.email, secretProject.get(), null), 403,
new AccessCheckInput(user.email, "nonexistent", null), 404,
new AccessCheckInput(privilegedUser.email, normalProject.get(), null), 200,
new AccessCheckInput(privilegedUser.email, secretProject.get(), null), 200);
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(
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));
for (Map.Entry<AccessCheckInput, Integer> entry : inputs.entrySet()) {
String in = newGson().toJson(entry.getKey());
for (TestCase tc : inputs) {
String in = newGson().toJson(tc.input);
AccessCheckInfo info = null;
try {
info = gApi.config().server().checkAccess(entry.getKey());
info = gApi.projects().name(tc.project).checkAccess(tc.input);
} catch (RestApiException e) {
fail(String.format("check.check(%s): exception %s", in, e));
fail(String.format("check.access(%s, %s): exception %s", tc.project, in, e));
}
int want = entry.getValue();
int want = tc.want;
if (want != info.status) {
fail(String.format("check.access(%s) = %d, want %d", in, info.status, want));
fail(
String.format("check.access(%s, %s) = %d, want %d", tc.project, in, info.status, want));
}
switch (want) {

View File

@@ -18,13 +18,11 @@ import com.google.gerrit.common.Nullable;
public class AccessCheckInput {
public String account;
public String project;
@Nullable public String ref;
public AccessCheckInput(String account, String project, @Nullable String ref) {
public AccessCheckInput(String account, @Nullable String ref) {
this.account = account;
this.project = project;
this.ref = ref;
}

View File

@@ -36,8 +36,6 @@ public interface Server {
ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException;
AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -79,10 +77,5 @@ public interface Server {
public ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
throw new NotImplementedException();
}
}
}

View File

@@ -16,6 +16,8 @@ package com.google.gerrit.extensions.api.projects;
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@@ -39,6 +41,8 @@ public interface ProjectApi {
ChangeInfo accessChange(ProjectAccessInput p) throws RestApiException;
AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException;
ConfigInfo config() throws RestApiException;
ConfigInfo config(ConfigInput in) throws RestApiException;
@@ -180,11 +184,21 @@ public interface ProjectApi {
throw new NotImplementedException();
}
@Override
public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
throw new NotImplementedException();
}
@Override
public ChangeInfo accessChange(ProjectAccessInput input) throws RestApiException {
throw new NotImplementedException();
}
@Override
public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
public ConfigInfo config() throws RestApiException {
throw new NotImplementedException();
@@ -195,11 +209,6 @@ public interface ProjectApi {
throw new NotImplementedException();
}
@Override
public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
throw new NotImplementedException();
}
@Override
public void description(DescriptionInput in) throws RestApiException {
throw new NotImplementedException();

View File

@@ -17,8 +17,6 @@ package com.google.gerrit.server.api.config;
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.gerrit.common.Version;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInput;
import com.google.gerrit.extensions.api.config.Server;
@@ -26,7 +24,6 @@ import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.config.CheckAccess;
import com.google.gerrit.server.config.CheckConsistency;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.GetDiffPreferences;
@@ -46,7 +43,6 @@ public class ServerImpl implements Server {
private final SetDiffPreferences setDiffPreferences;
private final GetServerInfo getServerInfo;
private final Provider<CheckConsistency> checkConsistency;
private final Provider<CheckAccess> checkAccess;
@Inject
ServerImpl(
@@ -55,15 +51,13 @@ public class ServerImpl implements Server {
GetDiffPreferences getDiffPreferences,
SetDiffPreferences setDiffPreferences,
GetServerInfo getServerInfo,
Provider<CheckConsistency> checkConsistency,
Provider<CheckAccess> checkAccess) {
Provider<CheckConsistency> checkConsistency) {
this.getPreferences = getPreferences;
this.setPreferences = setPreferences;
this.getDiffPreferences = getDiffPreferences;
this.setDiffPreferences = setDiffPreferences;
this.getServerInfo = getServerInfo;
this.checkConsistency = checkConsistency;
this.checkAccess = checkAccess;
}
@Override
@@ -126,13 +120,4 @@ public class ServerImpl implements Server {
throw asRestApiException("Cannot check consistency", e);
}
}
@Override
public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
try {
return checkAccess.get().apply(new ConfigResource(), in);
} catch (Exception e) {
throw asRestApiException("Cannot check access", e);
}
}
}

View File

@@ -19,6 +19,8 @@ import static com.google.gerrit.server.project.DashboardsCollection.DEFAULT_DASH
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.ChildProjectApi;
@@ -44,6 +46,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.CheckAccess;
import com.google.gerrit.server.project.ChildProjectsCollection;
import com.google.gerrit.server.project.CommitsCollection;
import com.google.gerrit.server.project.CreateAccessChange;
@@ -100,6 +103,7 @@ public class ProjectApiImpl implements ProjectApi {
private final CommitsCollection commitsCollection;
private final CommitApiImpl.Factory commitApi;
private final DashboardApiImpl.Factory dashboardApi;
private final CheckAccess checkAccess;
@AssistedInject
ProjectApiImpl(
@@ -127,6 +131,7 @@ public class ProjectApiImpl implements ProjectApi {
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
CheckAccess checkAccess,
@Assisted ProjectResource project) {
this(
user,
@@ -154,6 +159,7 @@ public class ProjectApiImpl implements ProjectApi {
commitsCollection,
commitApi,
dashboardApi,
checkAccess,
null);
}
@@ -183,6 +189,7 @@ public class ProjectApiImpl implements ProjectApi {
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
CheckAccess checkAccess,
@Assisted String name) {
this(
user,
@@ -210,6 +217,7 @@ public class ProjectApiImpl implements ProjectApi {
commitsCollection,
commitApi,
dashboardApi,
checkAccess,
name);
}
@@ -239,6 +247,7 @@ public class ProjectApiImpl implements ProjectApi {
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
CheckAccess checkAccess,
String name) {
this.user = user;
this.permissionBackend = permissionBackend;
@@ -251,7 +260,6 @@ public class ProjectApiImpl implements ProjectApi {
this.children = children;
this.projectJson = projectJson;
this.project = project;
this.name = name;
this.branchApi = branchApiFactory;
this.tagApi = tagApiFactory;
this.getAccess = getAccess;
@@ -266,6 +274,8 @@ public class ProjectApiImpl implements ProjectApi {
this.commitApi = commitApi;
this.createAccessChange = createAccessChange;
this.dashboardApi = dashboardApi;
this.checkAccess = checkAccess;
this.name = name;
}
@Override
@@ -313,6 +323,15 @@ public class ProjectApiImpl implements ProjectApi {
}
}
@Override
public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
try {
return checkAccess.apply(checkExists(), in);
} catch (Exception e) {
throw asRestApiException("Cannot check access rights", e);
}
}
@Override
public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
try {

View File

@@ -37,7 +37,6 @@ public class Module extends RestApiModule {
get(CONFIG_KIND, "version").to(GetVersion.class);
get(CONFIG_KIND, "info").to(GetServerInfo.class);
post(CONFIG_KIND, "check.consistency").to(CheckConsistency.class);
post(CONFIG_KIND, "check.access").to(CheckAccess.class);
get(CONFIG_KIND, "preferences").to(GetPreferences.class);
put(CONFIG_KIND, "preferences").to(SetPreferences.class);
get(CONFIG_KIND, "preferences.diff").to(GetDiffPreferences.class);

View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.config;
package com.google.gerrit.server.project;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
@@ -21,9 +21,9 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver;
@@ -32,7 +32,6 @@ import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -42,35 +41,29 @@ import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckInput> {
private final Provider<IdentifiedUser> currentUser;
public class CheckAccess implements RestModifyView<ProjectResource, AccessCheckInput> {
private final AccountResolver accountResolver;
private final Provider<ReviewDb> db;
private final IdentifiedUser.GenericFactory userFactory;
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
@Inject
CheckAccess(
Provider<IdentifiedUser> currentUser,
AccountResolver resolver,
Provider<ReviewDb> db,
IdentifiedUser.GenericFactory userFactory,
ProjectCache projectCache,
PermissionBackend permissionBackend) {
this.currentUser = currentUser;
this.accountResolver = resolver;
this.db = db;
this.userFactory = userFactory;
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
}
@Override
public AccessCheckInfo apply(ConfigResource unused, AccessCheckInput input)
public AccessCheckInfo apply(ProjectResource rsrc, AccessCheckInput input)
throws OrmException, PermissionBackendException, RestApiException, IOException,
ConfigInvalidException {
permissionBackend.user(currentUser.get()).check(GlobalPermission.ADMINISTRATE_SERVER);
permissionBackend.user(rsrc.getUser()).check(GlobalPermission.ADMINISTRATE_SERVER);
if (input == null) {
throw new BadRequestException("input is required");
@@ -78,32 +71,23 @@ public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckIn
if (Strings.isNullOrEmpty(input.account)) {
throw new BadRequestException("input requires 'account'");
}
if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("input requires 'project'");
}
Account match = accountResolver.find(db.get(), input.account);
if (match == null) {
throw new BadRequestException(String.format("cannot find account %s", input.account));
throw new UnprocessableEntityException(
String.format("cannot find account %s", input.account));
}
AccessCheckInfo info = new AccessCheckInfo();
Project.NameKey key = new Project.NameKey(input.project);
if (projectCache.get(key) == null) {
info.message = String.format("project %s does not exist", key);
info.status = HttpServletResponse.SC_NOT_FOUND;
return info;
}
IdentifiedUser user = userFactory.create(match.getId());
try {
permissionBackend.user(user).project(key).check(ProjectPermission.ACCESS);
permissionBackend.user(user).project(rsrc.getNameKey()).check(ProjectPermission.ACCESS);
} catch (AuthException | PermissionBackendException e) {
info.message =
String.format(
"user %s (%s) cannot see project %s",
user.getNameEmail(), user.getAccount().getId(), key);
user.getNameEmail(), user.getAccount().getId(), rsrc.getName());
info.status = HttpServletResponse.SC_FORBIDDEN;
return info;
}
@@ -112,14 +96,14 @@ public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckIn
try {
permissionBackend
.user(user)
.ref(new Branch.NameKey(key, input.ref))
.ref(new Branch.NameKey(rsrc.getNameKey(), input.ref))
.check(RefPermission.READ);
} 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, key);
user.getNameEmail(), user.getAccount().getId(), input.ref, rsrc.getName());
return info;
}
}

View File

@@ -49,6 +49,7 @@ public class Module extends RestApiModule {
get(PROJECT_KIND, "access").to(GetAccess.class);
post(PROJECT_KIND, "access").to(SetAccess.class);
put(PROJECT_KIND, "access:review").to(CreateAccessChange.class);
post(PROJECT_KIND, "check.access").to(CheckAccess.class);
get(PROJECT_KIND, "parent").to(GetParent.class);
put(PROJECT_KIND, "parent").to(SetParent.class);