Merge changes I7e6fbabb,Iddf6f9b3,Iaa258788,I38ae31cd
* changes:
CheckAccess: print a useful message if no rules apply
Clarify some methods in the permission APIs
restapi/project: move CheckAccessReadView into CheckAccess
restapi/project: add some comments
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 50ebad7..a809eab 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1848,8 +1848,9 @@
Gets whether the source is mergeable with the target branch.
-The `source` query parameter is required, which can be anything that could be
-resolved to a commit, see examples of the `source` attribute in
+The `source` query parameter is required, which can be anything that
+could be resolved to a commit, and is visible to the caller. See
+examples of the `source` attribute in
link:rest-api-changes.html#merge-input[MergeInput].
Also takes an optional parameter `strategy`, which can be `recursive`, `resolve`,
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index 3f04fa5..322c79e 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -140,6 +140,7 @@
return true;
}
+ /** The permission name, eg. {@code Permission.SUBMIT} */
public abstract String getName();
protected abstract boolean isExclusiveGroup();
diff --git a/java/com/google/gerrit/server/permissions/GlobalPermission.java b/java/com/google/gerrit/server/permissions/GlobalPermission.java
index 07c9e84..d4f22e6 100644
--- a/java/com/google/gerrit/server/permissions/GlobalPermission.java
+++ b/java/com/google/gerrit/server/permissions/GlobalPermission.java
@@ -18,6 +18,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.CapabilityScope;
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
@@ -31,7 +32,12 @@
import java.util.Optional;
import java.util.Set;
-/** Global server permissions built into Gerrit. */
+/**
+ * Global server permissions built into Gerrit.
+ *
+ * <p>See also {@link GlobalCapability} which lists the equivalent strings used in the
+ * refs/meta/config settings in All-Projects.
+ */
public enum GlobalPermission implements GlobalOrPluginPermission {
ACCESS_DATABASE,
ADMINISTRATE_SERVER,
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 89038e2..a1c48bc 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -385,6 +385,7 @@
this.accountsSection = accountsSection;
}
+ /** Returns an access section, {@code name} typically is a ref pattern. */
public AccessSection getAccessSection(String name) {
return accessSections.get(name);
}
diff --git a/java/com/google/gerrit/server/restapi/project/CheckAccess.java b/java/com/google/gerrit/server/restapi/project/CheckAccess.java
index 4ef724a..37616cd 100644
--- a/java/com/google/gerrit/server/restapi/project/CheckAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/CheckAccess.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2017 The Android Open Source Project
+// Copyright (C) 2018 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.
@@ -17,6 +17,7 @@
import static com.google.gerrit.entities.RefNames.REFS_HEADS;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
@@ -25,7 +26,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.logging.TraceContext;
@@ -37,15 +38,14 @@
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectResource;
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;
import org.eclipse.jgit.lib.Repository;
+import org.kohsuke.args4j.Option;
-@Singleton
-public class CheckAccess implements RestModifyView<ProjectResource, AccessCheckInput> {
+public class CheckAccess implements RestReadView<ProjectResource> {
private final AccountResolver accountResolver;
private final PermissionBackend permissionBackend;
private final GitRepositoryManager gitRepositoryManager;
@@ -60,7 +60,15 @@
this.gitRepositoryManager = gitRepositoryManager;
}
- @Override
+ @Option(name = "--ref", usage = "ref name to check permission for")
+ String refName;
+
+ @Option(name = "--account", usage = "account to check acccess for")
+ String account;
+
+ @Option(name = "--perm", usage = "permission to check; default: read of any ref.")
+ String permission;
+
public Response<AccessCheckInfo> apply(ProjectResource rsrc, AccessCheckInput input)
throws PermissionBackendException, RestApiException, IOException, ConfigInvalidException {
permissionBackend.user(rsrc.getUser()).check(GlobalPermission.VIEW_ACCESS);
@@ -142,6 +150,22 @@
info.status = statusCode;
info.message = message;
info.debugLogs = traceContext.getAclLogRecords();
+ if (info.debugLogs.isEmpty()) {
+ info.debugLogs =
+ ImmutableList.of("Found no rules that apply, so defaulting to no permission");
+ }
return info;
}
+
+ @Override
+ public Response<AccessCheckInfo> apply(ProjectResource rsrc)
+ throws PermissionBackendException, RestApiException, IOException, ConfigInvalidException {
+
+ AccessCheckInput input = new AccessCheckInput();
+ input.ref = refName;
+ input.account = account;
+ input.permission = permission;
+
+ return apply(rsrc, input);
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/project/CheckAccessReadView.java b/java/com/google/gerrit/server/restapi/project/CheckAccessReadView.java
deleted file mode 100644
index 6aaa678..0000000
--- a/java/com/google/gerrit/server/restapi/project/CheckAccessReadView.java
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright (C) 2018 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.server.restapi.project;
-
-import com.google.gerrit.extensions.api.config.AccessCheckInfo;
-import com.google.gerrit.extensions.api.config.AccessCheckInput;
-import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ProjectResource;
-import com.google.inject.Inject;
-import java.io.IOException;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.kohsuke.args4j.Option;
-
-public class CheckAccessReadView implements RestReadView<ProjectResource> {
- String refName;
- String account;
- String permission;
-
- @Inject CheckAccess checkAccess;
-
- @Option(name = "--ref", usage = "ref name to check permission for")
- void addOption(String refName) {
- this.refName = refName;
- }
-
- @Option(name = "--account", usage = "account to check acccess for")
- void setAccount(String account) {
- this.account = account;
- }
-
- @Option(name = "--perm", usage = "permission to check; default: read of any ref.")
- void setPermission(String perm) {
- this.permission = perm;
- }
-
- @Override
- public Response<AccessCheckInfo> apply(ProjectResource rsrc)
- throws PermissionBackendException, RestApiException, IOException, ConfigInvalidException {
-
- AccessCheckInput input = new AccessCheckInput();
- input.ref = refName;
- input.account = account;
- input.permission = permission;
-
- return checkAccess.apply(rsrc, input);
- }
-}
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index 5418876..8a2ab0c 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -90,7 +90,11 @@
import org.eclipse.jgit.lib.Repository;
import org.kohsuke.args4j.Option;
-/** List projects visible to the calling user. */
+/**
+ * List projects visible to the calling user.
+ *
+ * <p>This is like {@link QueryProjects} but with a slightly different calling convention.
+ */
public class ListProjects implements RestReadView<TopLevelResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/java/com/google/gerrit/server/restapi/project/Module.java b/java/com/google/gerrit/server/restapi/project/Module.java
index ee3914d..70fb6ea 100644
--- a/java/com/google/gerrit/server/restapi/project/Module.java
+++ b/java/com/google/gerrit/server/restapi/project/Module.java
@@ -57,7 +57,7 @@
get(PROJECT_KIND, "access").to(GetAccess.class);
post(PROJECT_KIND, "access").to(SetAccess.class);
put(PROJECT_KIND, "access:review").to(CreateAccessChange.class);
- get(PROJECT_KIND, "check.access").to(CheckAccessReadView.class);
+ get(PROJECT_KIND, "check.access").to(CheckAccess.class);
post(PROJECT_KIND, "check").to(Check.class);
diff --git a/java/com/google/gerrit/server/restapi/project/QueryProjects.java b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
index e4f7df5..a9d818d 100644
--- a/java/com/google/gerrit/server/restapi/project/QueryProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
@@ -36,6 +36,7 @@
import java.util.List;
import org.kohsuke.args4j.Option;
+/** Implements the {@code GET /projects/?query=QUERY} endpoint. */
public class QueryProjects implements RestReadView<TopLevelResource> {
private final ProjectIndexCollection indexes;
private final ProjectQueryBuilder queryBuilder;
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
index 709facc..45a895a 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
@@ -19,14 +19,17 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.deny;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
@@ -36,9 +39,11 @@
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.server.config.AllProjectsName;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
import java.util.List;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
@@ -48,6 +53,7 @@
public class CheckAccessIT extends AbstractDaemonTest {
@Inject private ProjectOperations projectOperations;
@Inject private GroupOperations groupOperations;
+ @Inject private AllProjectsName allProjectsName;
private Project.NameKey normalProject;
private Project.NameKey secretProject;
@@ -391,4 +397,34 @@
assertThat(info.status).isEqualTo(200);
assertThat(info.message).contains("no branches");
}
+
+ @Test
+ @Sandboxed
+ public void noRules() throws Exception {
+ normalProject = projectOperations.newProject().create();
+
+ for (AccessSection section :
+ projectOperations.project(allProjectsName).getProjectConfig().getAccessSections()) {
+ if (!section.getName().startsWith(Constants.R_REFS)) {
+ continue;
+ }
+ for (Permission permission : section.getPermissions()) {
+ projectOperations
+ .project(allProjectsName)
+ .forUpdate()
+ .remove(permissionKey(permission.getName()).ref(section.getName()).build())
+ .update();
+ }
+ }
+ AccessCheckInput input = new AccessCheckInput();
+ input.account = privilegedUser.email();
+ input.permission = Permission.READ;
+ input.ref = "refs/heads/main";
+
+ AccessCheckInfo info = gApi.projects().name(normalProject.get()).checkAccess(input);
+ assertThat(info.status).isEqualTo(403);
+
+ assertThat(info.debugLogs).isNotEmpty();
+ assertThat(info.debugLogs.get(0)).contains("Found no rules");
+ }
}
