Simplify permission checks using RequiresCapability annotation
Change-Id: I5da0ad29a38d527e9891a5fe797c2822e77ab3d8
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/PostVerification.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/PostVerification.java
index 8198981..5e3242e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/PostVerification.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/PostVerification.java
@@ -18,22 +18,15 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.errors.PermissionDeniedException;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.LabelId;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.verifystatus.common.VerificationInfo;
@@ -44,42 +37,27 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
import java.util.UUID;
@Singleton
-@RequiresCapability(SaveReportCapability.ID)
+@RequiresCapability(value = SaveReportCapability.ID, fallBackToAdmin = false)
public class PostVerification
implements RestModifyView<RevisionResource, VerifyInput> {
private static final Logger log =
LoggerFactory.getLogger(PostVerification.class);
private final SchemaFactory<CiDb> schemaFactory;
- private final String pluginName;
- private final Provider<CurrentUser> userProvider;
@Inject
- PostVerification(@PluginName String pluginName,
- Provider<CurrentUser> userProvider,
- SchemaFactory<CiDb> schemaFactory) {
- this.pluginName = pluginName;
- this.userProvider = userProvider;
+ PostVerification(SchemaFactory<CiDb> schemaFactory) {
this.schemaFactory = schemaFactory;
}
@Override
public Response<?> apply(RevisionResource revision, VerifyInput input)
- throws AuthException, BadRequestException, UnprocessableEntityException,
- OrmException, IOException {
-
- try {
- checkPermission();
- } catch (PermissionDeniedException err) {
- throw new BadRequestException("fatal: " + err.getMessage());
- }
-
+ throws BadRequestException, OrmException {
if (input.verifications == null) {
throw new BadRequestException("Missing verifications field");
}
@@ -191,25 +169,4 @@
}
return current;
}
-
- /**
- * Assert that the current user is permitted to perform saving of verification
- * reports.
- * <p>
- * As the @RequireCapability guards at various entry points of internal
- * commands implicitly add administrators (which we want to avoid), we also
- * check permissions within QueryShell and grant access only to those who
- * canPerformRawQuery, regardless of whether they are administrators or not.
- *
- * @throws PermissionDeniedException
- */
- private void checkPermission() throws PermissionDeniedException {
- CapabilityControl ctl = userProvider.get().getCapabilities();
- if (!ctl.canPerform(SaveReportCapability.getName(pluginName))) {
- throw new PermissionDeniedException(String.format(
- "%s does not have \"%s\" capability.",
- userProvider.get().getUserName(),
- new SaveReportCapability().getDescription()));
- }
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/SaveCommand.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/SaveCommand.java
index b2da1de..40eb99e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/SaveCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/SaveCommand.java
@@ -19,14 +19,10 @@
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
-import com.google.gerrit.common.errors.PermissionDeniedException;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
@@ -36,7 +32,6 @@
import com.google.gerrit.sshd.commands.PatchSetParser;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.verifystatus.common.VerificationInfo;
import com.googlesource.gerrit.plugins.verifystatus.common.VerifyInput;
@@ -51,22 +46,13 @@
import java.util.Map;
import java.util.Set;
-@RequiresCapability(SaveReportCapability.ID)
+@RequiresCapability(value = SaveReportCapability.ID, fallBackToAdmin = false)
@CommandMetaData(name = "save", description = "Save patchset verification data")
public class SaveCommand extends SshCommand {
private static final Logger log =
LoggerFactory.getLogger(SaveCommand.class);
private final Set<PatchSet> patchSets = new HashSet<>();
- private final String pluginName;
- private final Provider<CurrentUser> userProvider;
-
- @Inject
- SaveCommand(@PluginName String pluginName,
- Provider<CurrentUser> userProvider) {
- this.pluginName = pluginName;
- this.userProvider = userProvider;
- }
@Argument(index = 0, required = true, multiValued = true,
metaVar = "{COMMIT | CHANGE,PATCHSET}",
@@ -145,11 +131,6 @@
@Override
protected void run() throws UnloggedFailure {
boolean ok = true;
- try {
- checkPermission();
- } catch (PermissionDeniedException err) {
- throw new UnloggedFailure("fatal: " + err.getMessage());
- }
for (PatchSet patchSet : patchSets) {
try {
@@ -167,8 +148,7 @@
}
private void applyVerification(PatchSet patchSet, VerifyInput verify)
- throws RestApiException, OrmException,
- IOException {
+ throws RestApiException, OrmException, IOException {
RevisionResource revResource = revisions.parse(
changes.parse(patchSet.getId().getParentKey()),
IdString.fromUrl(patchSet.getId().getId()));
@@ -192,25 +172,4 @@
} catch (IOException e) {
}
}
-
- /**
- * Assert that the current user is permitted to perform saving of verification
- * reports.
- * <p>
- * As the @RequireCapability guards at various entry points of internal
- * commands implicitly add administrators (which we want to avoid), we also
- * check permissions within QueryShell and grant access only to those who
- * canPerformRawQuery, regardless of whether they are administrators or not.
- *
- * @throws PermissionDeniedException
- */
- private void checkPermission() throws PermissionDeniedException {
- CapabilityControl ctl = userProvider.get().getCapabilities();
- if (!ctl.canPerform(pluginName + "-" + SaveReportCapability.ID)) {
- throw new PermissionDeniedException(String.format(
- "%s does not have \"%s\" capability.",
- userProvider.get().getUserName(),
- new SaveReportCapability().getDescription()));
- }
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/VerifyStatusAdminQueryShell.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/VerifyStatusAdminQueryShell.java
index 3bf9ea3..0ce6d2a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/VerifyStatusAdminQueryShell.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/VerifyStatusAdminQueryShell.java
@@ -14,26 +14,22 @@
package com.googlesource.gerrit.plugins.verifystatus;
-import com.google.gerrit.common.errors.PermissionDeniedException;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.sshd.AdminHighPriorityCommand;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import org.kohsuke.args4j.Option;
/** Opens a query processor. */
@AdminHighPriorityCommand
-@RequiresCapability(AccessCiDatabaseCapability.ID)
+@RequiresCapability(
+ value = AccessCiDatabaseCapability.ID,
+ fallBackToAdmin = false
+)
@CommandMetaData(name = "gsql", description = "Administrative interface to CI database")
public class VerifyStatusAdminQueryShell extends SshCommand {
- private final String pluginName;
- private final Provider<CurrentUser> userProvider;
@Inject
private VerifyStatusQueryShell.Factory factory;
@@ -44,21 +40,8 @@
@Option(name = "-c", metaVar = "SQL QUERY", usage = "Query to execute")
private String query;
- @Inject
- VerifyStatusAdminQueryShell(@PluginName String pluginName,
- Provider<CurrentUser> userProvider) {
- this.pluginName = pluginName;
- this.userProvider = userProvider;
- }
-
@Override
- protected void run() throws UnloggedFailure {
- try {
- checkPermission();
- } catch (PermissionDeniedException err) {
- throw new UnloggedFailure("fatal: " + err.getMessage());
- }
-
+ protected void run() {
final VerifyStatusQueryShell shell = factory.create(in, out);
shell.setOutputFormat(format);
if (query != null) {
@@ -67,24 +50,4 @@
shell.run();
}
}
-
- /**
- * Assert that the current user is permitted to perform raw queries.
- * <p>
- * As the @RequireCapability guards at various entry points of internal
- * commands implicitly add administrators (which we want to avoid), we also
- * check permissions within QueryShell and grant access only to those who
- * canPerformRawQuery, regardless of whether they are administrators or not.
- *
- * @throws PermissionDeniedException
- */
- private void checkPermission() throws PermissionDeniedException {
- CapabilityControl ctl = userProvider.get().getCapabilities();
- if (!ctl.canPerform(pluginName + "-" + AccessCiDatabaseCapability.ID)) {
- throw new PermissionDeniedException(String.format(
- "%s does not have \"%s\" capability.",
- userProvider.get().getUserName(),
- new AccessCiDatabaseCapability().getDescription()));
- }
- }
}
