Check Administrate Checkers capability when creating/updating checks
As long as we don't have dedicated fine grained permissions to allow
posting checks require the Administrate Checkers capability for this.
Change-Id: I43e7c71c1cf9308df07f594d8ae5041c9e4d3e68
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/checks/PostCheck.java b/java/com/google/gerrit/plugins/checks/PostCheck.java
index d59139f..c5e2c02 100644
--- a/java/com/google/gerrit/plugins/checks/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/PostCheck.java
@@ -22,6 +22,8 @@
import com.google.gerrit.plugins.checks.api.CheckResource;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
import java.util.Optional;
@@ -32,14 +34,21 @@
@Singleton
public class PostCheck
implements RestCollectionModifyView<RevisionResource, CheckResource, CheckInput> {
-
+ private final PermissionBackend permissionBackend;
+ private final AdministrateCheckersPermission permission;
private final Checks checks;
private final Provider<ChecksUpdate> checksUpdate;
private final CheckJson checkJson;
@Inject
PostCheck(
- Checks checks, @UserInitiated Provider<ChecksUpdate> checksUpdate, CheckJson checkJson) {
+ PermissionBackend permissionBackend,
+ AdministrateCheckersPermission permission,
+ Checks checks,
+ @UserInitiated Provider<ChecksUpdate> checksUpdate,
+ CheckJson checkJson) {
+ this.permissionBackend = permissionBackend;
+ this.permission = permission;
this.checks = checks;
this.checksUpdate = checksUpdate;
this.checkJson = checkJson;
@@ -47,7 +56,7 @@
@Override
public CheckInfo apply(RevisionResource rsrc, CheckInput input)
- throws OrmException, IOException, RestApiException {
+ throws OrmException, IOException, RestApiException, PermissionBackendException {
if (input == null) {
input = new CheckInput();
}
@@ -58,6 +67,8 @@
throw new BadRequestException("invalid checker UUID: " + input.checkerUuid);
}
+ permissionBackend.currentUser().check(permission);
+
CheckKey key =
CheckKey.create(
rsrc.getProject(), rsrc.getPatchSet().getId(), CheckerUuid.parse(input.checkerUuid));
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
index e1566d7..259c5ca 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.plugins.checks.PostCheck;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -34,7 +35,7 @@
@Override
public CheckInfo apply(CheckResource checkResource, CheckInput input)
- throws RestApiException, IOException, OrmException {
+ throws RestApiException, IOException, OrmException, PermissionBackendException {
if (input == null) {
input = new CheckInput();
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index 101d6c5..d354ce5 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -17,6 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
@@ -28,6 +30,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.testing.TestTimeUtil;
+import com.google.inject.Inject;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.Map;
@@ -37,6 +40,8 @@
import org.junit.Test;
public class CreateCheckIT extends AbstractCheckersTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+
private PatchSet.Id patchSetId;
private RevId revId;
@@ -92,6 +97,21 @@
checksApiFactory.revision(patchSetId).create(input);
}
+ @Test
+ public void cannotCreateCheckWithoutAdministrateCheckers() throws Exception {
+ requestScopeOperations.setApiUser(user.getId());
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.toString();
+ input.state = CheckState.RUNNING;
+
+ exception.expect(AuthException.class);
+ exception.expectMessage("not permitted");
+ checksApiFactory.revision(patchSetId).create(input);
+ }
+
// TODO(gerrit-team) More tests, especially for multiple checkers and PS and how commits behave
private String noteDbContent(String uuid) {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
index b10ba3c..23c4a0a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -16,6 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
@@ -23,10 +25,13 @@
import com.google.gerrit.plugins.checks.api.CheckInput;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.inject.Inject;
import org.junit.Before;
import org.junit.Test;
public class UpdateCheckIT extends AbstractCheckersTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+
private PatchSet.Id patchSetId;
private CheckKey checkKey;
@@ -47,4 +52,13 @@
CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
assertThat(info.state).isEqualTo(CheckState.FAILED);
}
+
+ @Test
+ public void cannotUpdateCheckWithoutAdministrateCheckers() throws Exception {
+ requestScopeOperations.setApiUser(user.getId());
+
+ exception.expect(AuthException.class);
+ exception.expectMessage("not permitted");
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(new CheckInput());
+ }
}
