From a3efaba361208d7538279f0ef4a71d940c65e86a Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 20 Feb 2017 11:42:27 -0800 Subject: [PATCH] Convert maintainServer to PermissionBackend Change-Id: I0b8bbd2df1f72f8e76b19b8baae01713403e628a --- .../server/account/CapabilityControl.java | 12 ++++------- .../server/api/changes/ChangeApiImpl.java | 4 ++-- .../google/gerrit/server/change/Check.java | 20 ++++++++++++------- .../google/gerrit/server/change/Index.java | 20 +++++++++++++++---- .../gerrit/server/config/FlushCache.java | 14 +++++++++---- .../gerrit/server/config/PostCaches.java | 9 ++++++--- .../gerrit/sshd/ChangeArgumentParser.java | 19 ++++++++++++++---- .../gerrit/sshd/commands/FlushCaches.java | 3 +++ .../sshd/commands/IndexChangesCommand.java | 3 ++- .../gerrit/sshd/commands/ShowCaches.java | 18 +++++++++++++---- 10 files changed, 85 insertions(+), 37 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java index 4b3b72bd6e..510038ec2c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java @@ -97,11 +97,6 @@ public class CapabilityControl { return canPerform(GlobalCapability.VIEW_ALL_ACCOUNTS) || canAdministrateServer(); } - /** @return true if the user can perform basic server maintenance. */ - public boolean canMaintainServer() { - return canPerform(GlobalCapability.MAINTAIN_SERVER) || canAdministrateServer(); - } - /** @return true if the user can access the database (with gsql). */ public boolean canAccessDatabase() { try { @@ -233,8 +228,6 @@ public class CapabilityControl { return canAdministrateServer(); case EMAIL_REVIEWERS: return canEmailReviewers(); - case MAINTAIN_SERVER: - return canMaintainServer(); case MODIFY_ACCOUNT: return canModifyAccount(); case VIEW_ALL_ACCOUNTS: @@ -245,11 +238,14 @@ public class CapabilityControl { case RUN_GC: case VIEW_CACHES: case VIEW_QUEUE: - return canPerform(perm.permissionName()) || canMaintainServer(); + return canPerform(perm.permissionName()) + || canPerform(GlobalCapability.MAINTAIN_SERVER) + || canAdministrateServer(); case CREATE_ACCOUNT: case CREATE_GROUP: case CREATE_PROJECT: + case MAINTAIN_SERVER: case STREAM_EVENTS: case VIEW_CONNECTIONS: case VIEW_PLUGINS: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index d534c5a130..cee2403a15 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -572,7 +572,7 @@ class ChangeApiImpl implements ChangeApi { public ChangeInfo check(FixInput fix) throws RestApiException { try { return check.apply(change, fix).value(); - } catch (OrmException e) { + } catch (OrmException | PermissionBackendException e) { throw new RestApiException("Cannot check change", e); } } @@ -581,7 +581,7 @@ class ChangeApiImpl implements ChangeApi { public void index() throws RestApiException { try { index.apply(change, new Index.Input()); - } catch (IOException | OrmException e) { + } catch (IOException | OrmException | PermissionBackendException e) { throw new RestApiException("Cannot index change", e); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java index 3b6793021c..5f6923e2e7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java @@ -17,21 +17,29 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.restapi.AuthException; 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.CurrentUser; +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.ChangeControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; public class Check implements RestReadView, RestModifyView { + private final PermissionBackend permissionBackend; + private final Provider user; private final ChangeJson.Factory jsonFactory; @Inject - Check(ChangeJson.Factory json) { + Check(PermissionBackend permissionBackend, Provider user, ChangeJson.Factory json) { + this.permissionBackend = permissionBackend; + this.user = user; this.jsonFactory = json; } @@ -42,12 +50,10 @@ public class Check @Override public Response apply(ChangeResource rsrc, FixInput input) - throws RestApiException, OrmException { + throws RestApiException, OrmException, PermissionBackendException { ChangeControl ctl = rsrc.getControl(); - if (!ctl.isOwner() - && !ctl.getProjectControl().isOwner() - && !ctl.getUser().getCapabilities().canMaintainServer()) { - throw new AuthException("Cannot fix change"); + if (!ctl.isOwner() && !ctl.getProjectControl().isOwner()) { + permissionBackend.user(user).check(GlobalPermission.MAINTAIN_SERVER); } return Response.withMustRevalidate(newChangeJson().fix(input).format(rsrc)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java index 9257445972..ab922814c9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java @@ -18,8 +18,12 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.Index.Input; import com.google.gerrit.server.index.change.ChangeIndexer; +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.ChangeControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -32,20 +36,28 @@ public class Index implements RestModifyView { public static class Input {} private final Provider db; + private final PermissionBackend permissionBackend; + private final Provider user; private final ChangeIndexer indexer; @Inject - Index(Provider db, ChangeIndexer indexer) { + Index( + Provider db, + PermissionBackend permissionBackend, + Provider user, + ChangeIndexer indexer) { this.db = db; + this.permissionBackend = permissionBackend; + this.user = user; this.indexer = indexer; } @Override public Response apply(ChangeResource rsrc, Input input) - throws IOException, AuthException, OrmException { + throws IOException, AuthException, OrmException, PermissionBackendException { ChangeControl ctl = rsrc.getControl(); - if (!ctl.isOwner() && !ctl.getUser().getCapabilities().canMaintainServer()) { - throw new AuthException("Only change owner or server maintainer can reindex"); + if (!ctl.isOwner()) { + permissionBackend.user(user).check(GlobalPermission.MAINTAIN_SERVER); } indexer.index(db.get(), rsrc.getChange()); return Response.none(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/FlushCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/FlushCache.java index 5e190910a9..366dae168e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/FlushCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/FlushCache.java @@ -23,6 +23,9 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.FlushCache.Input; +import com.google.gerrit.server.permissions.GlobalPermission; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -34,17 +37,20 @@ public class FlushCache implements RestModifyView { public static final String WEB_SESSIONS = "web_sessions"; + private final PermissionBackend permissionBackend; private final Provider self; @Inject - public FlushCache(Provider self) { + public FlushCache(PermissionBackend permissionBackend, Provider self) { + this.permissionBackend = permissionBackend; this.self = self; } @Override - public Response apply(CacheResource rsrc, Input input) throws AuthException { - if (WEB_SESSIONS.equals(rsrc.getName()) && !self.get().getCapabilities().canMaintainServer()) { - throw new AuthException(String.format("only site maintainers can flush %s", WEB_SESSIONS)); + public Response apply(CacheResource rsrc, Input input) + throws AuthException, PermissionBackendException { + if (WEB_SESSIONS.equals(rsrc.getName())) { + permissionBackend.user(self).check(GlobalPermission.MAINTAIN_SERVER); } rsrc.getCache().invalidateAll(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/PostCaches.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/PostCaches.java index 3cfa2b911b..d08f0a9197 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/PostCaches.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/PostCaches.java @@ -26,6 +26,7 @@ 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.server.config.PostCaches.Input; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import com.google.inject.Singleton; import java.util.ArrayList; @@ -66,7 +67,8 @@ public class PostCaches implements RestModifyView { @Override public Response apply(ConfigResource rsrc, Input input) - throws AuthException, BadRequestException, UnprocessableEntityException { + throws AuthException, BadRequestException, UnprocessableEntityException, + PermissionBackendException { if (input == null || input.operation == null) { throw new BadRequestException("operation must be specified"); } @@ -90,7 +92,7 @@ public class PostCaches implements RestModifyView { } } - private void flushAll() throws AuthException { + private void flushAll() throws AuthException, PermissionBackendException { for (DynamicMap.Entry> e : cacheMap) { CacheResource cacheResource = new CacheResource(e.getPluginName(), e.getExportName(), e.getProvider()); @@ -101,7 +103,8 @@ public class PostCaches implements RestModifyView { } } - private void flush(List cacheNames) throws UnprocessableEntityException, AuthException { + private void flush(List cacheNames) + throws UnprocessableEntityException, AuthException, PermissionBackendException { List cacheResources = new ArrayList<>(cacheNames.size()); for (String n : cacheNames) { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java index 4488c711c2..e64ab0ec1b 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java @@ -16,6 +16,7 @@ package com.google.gerrit.sshd; import static java.util.stream.Collectors.toList; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -24,6 +25,9 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.notedb.ChangeNotes; +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.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectControl; @@ -43,6 +47,7 @@ public class ChangeArgumentParser { private final ReviewDb db; private final ChangeNotes.Factory changeNotesFactory; private final ChangeControl.GenericFactory changeControlFactory; + private final PermissionBackend permissionBackend; @Inject ChangeArgumentParser( @@ -51,13 +56,15 @@ public class ChangeArgumentParser { ChangeFinder changeFinder, ReviewDb db, ChangeNotes.Factory changeNotesFactory, - ChangeControl.GenericFactory changeControlFactory) { + ChangeControl.GenericFactory changeControlFactory, + PermissionBackend permissionBackend) { this.currentUser = currentUser; this.changesCollection = changesCollection; this.changeFinder = changeFinder; this.db = db; this.changeNotesFactory = changeNotesFactory; this.changeControlFactory = changeControlFactory; + this.permissionBackend = permissionBackend; } public void addChange(String id, Map changes) @@ -80,9 +87,13 @@ public class ChangeArgumentParser { List matched = useIndex ? changeFinder.find(id, currentUser) : changeFromNotesFactory(id, currentUser); List toAdd = new ArrayList<>(changes.size()); - boolean canMaintainServer = - currentUser.isIdentifiedUser() - && currentUser.asIdentifiedUser().getCapabilities().canMaintainServer(); + boolean canMaintainServer; + try { + permissionBackend.user(currentUser).check(GlobalPermission.MAINTAIN_SERVER); + canMaintainServer = true; + } catch (AuthException | PermissionBackendException e) { + canMaintainServer = false; + } for (ChangeControl ctl : matched) { if (!changes.containsKey(ctl.getId()) && inProject(projectControl, ctl.getProject()) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java index 21bfe9bddf..392fd29d7e 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java @@ -26,6 +26,7 @@ import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.config.ListCaches; import com.google.gerrit.server.config.ListCaches.OutputFormat; import com.google.gerrit.server.config.PostCaches; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; import com.google.inject.Inject; @@ -81,6 +82,8 @@ final class FlushCaches extends SshCommand { } } catch (RestApiException e) { throw die(e.getMessage()); + } catch (PermissionBackendException e) { + throw new Failure(1, "unavailable", e); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java index fc65cf3733..0ee1c28683 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java @@ -18,6 +18,7 @@ import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.Index; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.sshd.ChangeArgumentParser; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; @@ -57,7 +58,7 @@ final class IndexChangesCommand extends SshCommand { for (ChangeResource rsrc : changes.values()) { try { index.apply(rsrc, new Index.Input()); - } catch (IOException | RestApiException | OrmException e) { + } catch (IOException | RestApiException | OrmException | PermissionBackendException e) { ok = false; writeError( "error", String.format("failed to index change %s: %s", rsrc.getId(), e.getMessage())); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowCaches.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowCaches.java index e16f270119..1ed7db3a44 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowCaches.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowCaches.java @@ -23,6 +23,7 @@ import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.Version; import com.google.gerrit.extensions.annotations.RequiresAnyCapability; import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.config.GetSummary; @@ -34,6 +35,9 @@ import com.google.gerrit.server.config.GetSummary.ThreadSummaryInfo; import com.google.gerrit.server.config.ListCaches; import com.google.gerrit.server.config.ListCaches.CacheInfo; import com.google.gerrit.server.config.ListCaches.CacheType; +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.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; import com.google.gerrit.sshd.SshDaemon; @@ -80,12 +84,10 @@ final class ShowCaches extends SshCommand { private boolean showThreads; @Inject private SshDaemon daemon; - @Inject private ListCaches listCaches; - @Inject private GetSummary getSummary; - @Inject private CurrentUser self; + @Inject private PermissionBackend permissionBackend; @Option( name = "--width", @@ -168,7 +170,15 @@ final class ShowCaches extends SshCommand { printDiskCaches(caches); stdout.print('\n'); - if (self.getCapabilities().canMaintainServer()) { + boolean showJvm; + try { + permissionBackend.user(self).check(GlobalPermission.MAINTAIN_SERVER); + showJvm = true; + } catch (AuthException | PermissionBackendException e) { + // Silently ignore and do not display detailed JVM information. + showJvm = false; + } + if (showJvm) { sshSummary(); SummaryInfo summary = getSummary.setGc(gc).setJvm(showJVM).apply(new ConfigResource());