Merge changes Ide146a98,Ic61022af
* changes:
Test that message on post review is extended if comments are added
Add capability that allows to call the CheckCodeOwner REST endpoint
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
index 1bc41d7..86adb2b 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
@@ -37,7 +37,6 @@
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwnersResult;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.BranchResource;
@@ -61,6 +60,7 @@
* /projects/<project-name>/branches/<branch-name>/code_owners.check} requests.
*/
public class CheckCodeOwner implements RestReadView<BranchResource> {
+ private final CheckCodeOwnerCapability checkCodeOwnerCapability;
private final PermissionBackend permissionBackend;
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerConfigHierarchy codeOwnerConfigHierarchy;
@@ -76,6 +76,7 @@
@Inject
public CheckCodeOwner(
+ CheckCodeOwnerCapability checkCodeOwnerCapability,
PermissionBackend permissionBackend,
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
@@ -83,6 +84,7 @@
Provider<CodeOwnerResolver> codeOwnerResolverProvider,
CodeOwners codeOwners,
AccountsCollection accountsCollection) {
+ this.checkCodeOwnerCapability = checkCodeOwnerCapability;
this.permissionBackend = permissionBackend;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.codeOwnerConfigHierarchy = codeOwnerConfigHierarchy;
@@ -115,7 +117,7 @@
public Response<CodeOwnerCheckInfo> apply(BranchResource branchResource)
throws BadRequestException, AuthException, IOException, ConfigInvalidException,
PermissionBackendException {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
+ permissionBackend.currentUser().check(checkCodeOwnerCapability.getPermission());
validateInput();
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerCapability.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerCapability.java
new file mode 100644
index 0000000..a127815
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerCapability.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2021 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.plugins.codeowners.restapi;
+
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/** Global capability that allows a user to call the {@link CheckCodeOwner} REST endpoint. */
+@Singleton
+public class CheckCodeOwnerCapability extends CapabilityDefinition {
+ public static final String ID = "checkCodeOwner";
+
+ private final String pluginName;
+
+ @Inject
+ CheckCodeOwnerCapability(@PluginName String pluginName) {
+ this.pluginName = pluginName;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Check Code Owner";
+ }
+
+ public PluginPermission getPermission() {
+ return new PluginPermission(pluginName, ID);
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
index 68d057e..df4e6f7 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
@@ -19,6 +19,8 @@
import static com.google.gerrit.server.project.BranchResource.BRANCH_KIND;
import static com.google.gerrit.server.project.ProjectResource.PROJECT_KIND;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.registration.DynamicMap;
/** Guice module that binds the REST API for the code-owners plugin. */
@@ -32,6 +34,10 @@
get(BRANCH_KIND, "code_owners.config_files").to(GetCodeOwnerConfigFiles.class);
get(BRANCH_KIND, "code_owners.branch_config").to(GetCodeOwnerBranchConfig.class);
post(BRANCH_KIND, "code_owners.rename").to(RenameEmail.class);
+
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named(CheckCodeOwnerCapability.ID))
+ .to(CheckCodeOwnerCapability.class);
get(BRANCH_KIND, "code_owners.check").to(CheckCodeOwner.class);
factory(CodeOwnerJson.Factory.class);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
index ea4fc3a..fc5fddf 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
@@ -16,7 +16,9 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerCheckInfoSubject.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.TestAccount;
@@ -40,6 +42,7 @@
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.config.BackendConfig;
+import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerCapability;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -87,15 +90,33 @@
}
@Test
- public void requiresCallerToBeAdmin() throws Exception {
+ public void requiresCallerToBeAdminOrHaveTheCheckCodeOwnerCapability() throws Exception {
requestScopeOperations.setApiUser(user.id());
AuthException authException =
assertThrows(AuthException.class, () -> checkCodeOwner(ROOT_PATH, user.email()));
- assertThat(authException).hasMessageThat().isEqualTo("administrate server not permitted");
+ assertThat(authException)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format("%s for plugin code-owners not permitted", CheckCodeOwnerCapability.ID));
}
@Test
- public void checkCodeOwner() throws Exception {
+ public void checkCodeOwner_byAdmin() throws Exception {
+ testCheckCodeOwner();
+ }
+
+ @Test
+ public void checkCodeOwner_byUserThatHasTheCheckCodeOwnerCapability() throws Exception {
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability("code-owners-" + CheckCodeOwnerCapability.ID).group(REGISTERED_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ testCheckCodeOwner();
+ }
+
+ private void testCheckCodeOwner() throws Exception {
TestAccount codeOwner =
accountCreator.create(
"codeOwner", "codeOwner@example.com", "Code Owner", /* displayName= */ null);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
index d899268..9160b32 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
@@ -18,12 +18,14 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.LabelDefinitionInput;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
import java.util.Collection;
+import java.util.HashMap;
import org.junit.Test;
/**
@@ -586,4 +588,39 @@
Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review+1");
}
+
+ @Test
+ public void changeMessageListsNewlyApprovedPathsIfCommentsAreAddedOnPostReview()
+ throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ String path = "foo/bar.baz";
+ String changeId = createChange("Test Change", path, "file content").getChangeId();
+
+ ReviewInput.CommentInput commentInput = new ReviewInput.CommentInput();
+ commentInput.line = 1;
+ commentInput.message = "some comment";
+ commentInput.path = path;
+ ReviewInput reviewInput = ReviewInput.recommend();
+ reviewInput.comments = reviewInput.comments = new HashMap<>();
+ reviewInput.comments.put(commentInput.path, Lists.newArrayList(commentInput));
+ gApi.changes().id(changeId).current().review(reviewInput);
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Code-Review+1\n\n"
+ + "(1 comment)\n\n"
+ + "By voting Code-Review+1 the following files are now code-owner approved by"
+ + " %s:\n"
+ + "* %s\n",
+ admin.fullName(), path));
+ }
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 8902d36..c93adc0 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -232,8 +232,8 @@
| `path` | mandatory | Path for which the code ownership should be checked.
| `user` | optional | User for which the code owner visibility should be checked. If not specified the code owner visibility is not checked. Can be used to investigate why a code owner is not shown/suggested to this user.
-Requires that the caller has the [Administrate
-Server](../../../Documentation/access-control.html#capability_administrateServer)
+Requires that the caller has the [Check Code Owner](#checkCodeOwner) or the
+[Administrate Server](../../../Documentation/access-control.html#capability_administrateServer)
global capability.
This REST endpoint is intended to investigate code owner configurations that do
@@ -841,6 +841,19 @@
---
+## <a id="capabilities">Capabilities
+
+### <a id="checkCodeOwner">Check Code Owner
+
+Global capability that allows a user to call the [Check Code
+Owner](#check-code-owner) REST endpoint.
+
+Assigning this capability allows users to inspect code ownerships. This may
+reveal accounts and secondary emails to the user that the user cannot see
+otherwise. Hence this capability should only ge granted to trusted users.
+
+Administrators have this capability implicitly assigned.
+
Back to [@PLUGIN@ documentation index](index.html)
Part of [Gerrit Code Review](../../../Documentation/index.html)
