From 53417fc6058cb530c8a8bb4fce82fbf656ba4e4e Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 19 Feb 2017 21:27:50 -0800 Subject: [PATCH 1/3] Convert viewQueue to PermissionBackend Change-Id: I993701da4a13275ade280674923a57c32cf145e6 --- .../server/account/CapabilityControl.java | 8 +--- .../gerrit/server/config/ListTasks.java | 24 +++++++--- .../gerrit/server/config/TasksCollection.java | 48 ++++++++++++------- .../gerrit/sshd/commands/KillCommand.java | 3 +- .../gerrit/sshd/commands/ShowQueue.java | 14 ++++-- 5 files changed, 60 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 f678379e13..4b3b72bd6e 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 @@ -102,11 +102,6 @@ public class CapabilityControl { return canPerform(GlobalCapability.MAINTAIN_SERVER) || canAdministrateServer(); } - /** @return true if the user can view the entire queue. */ - public boolean canViewQueue() { - return canPerform(GlobalCapability.VIEW_QUEUE) || canMaintainServer(); - } - /** @return true if the user can access the database (with gsql). */ public boolean canAccessDatabase() { try { @@ -244,13 +239,12 @@ public class CapabilityControl { return canModifyAccount(); case VIEW_ALL_ACCOUNTS: return canViewAllAccounts(); - case VIEW_QUEUE: - return canViewQueue(); case FLUSH_CACHES: case KILL_TASK: case RUN_GC: case VIEW_CACHES: + case VIEW_QUEUE: return canPerform(perm.permissionName()) || canMaintainServer(); case CREATE_ACCOUNT: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ListTasks.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ListTasks.java index 7e9bd71494..be2edfd394 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ListTasks.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ListTasks.java @@ -19,11 +19,13 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.TaskInfoFactory; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.git.WorkQueue.ProjectTask; import com.google.gerrit.server.git.WorkQueue.Task; +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.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.util.IdGenerator; @@ -41,30 +43,40 @@ import java.util.concurrent.TimeUnit; @Singleton public class ListTasks implements RestReadView { + private final PermissionBackend permissionBackend; private final WorkQueue workQueue; private final ProjectCache projectCache; - private final Provider self; + private final Provider self; @Inject - public ListTasks(WorkQueue workQueue, ProjectCache projectCache, Provider self) { + public ListTasks( + PermissionBackend permissionBackend, + WorkQueue workQueue, + ProjectCache projectCache, + Provider self) { + this.permissionBackend = permissionBackend; this.workQueue = workQueue; this.projectCache = projectCache; this.self = self; } @Override - public List apply(ConfigResource resource) throws AuthException { + public List apply(ConfigResource resource) + throws AuthException, PermissionBackendException { CurrentUser user = self.get(); if (!user.isIdentifiedUser()) { throw new AuthException("Authentication required"); } List allTasks = getTasks(); - if (user.getCapabilities().canViewQueue()) { + try { + permissionBackend.user(user).check(GlobalPermission.VIEW_QUEUE); return allTasks; + } catch (AuthException e) { + // Fall through to filter tasks. } - Map visibilityCache = new HashMap<>(); + Map visibilityCache = new HashMap<>(); List visibleTasks = new ArrayList<>(); for (TaskInfo task : allTasks) { if (task.projectName != null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/TasksCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/TasksCollection.java index b239856776..b33b1c5c66 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/TasksCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/TasksCollection.java @@ -21,10 +21,12 @@ import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.git.WorkQueue.ProjectTask; import com.google.gerrit.server.git.WorkQueue.Task; +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.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.inject.Inject; @@ -36,7 +38,8 @@ public class TasksCollection implements ChildCollection> views; private final ListTasks list; private final WorkQueue workQueue; - private final Provider self; + private final Provider self; + private final PermissionBackend permissionBackend; private final ProjectCache projectCache; @Inject @@ -44,12 +47,14 @@ public class TasksCollection implements ChildCollection> views, ListTasks list, WorkQueue workQueue, - Provider self, + Provider self, + PermissionBackend permissionBackend, ProjectCache projectCache) { this.views = views; this.list = list; this.workQueue = workQueue; this.self = self; + this.permissionBackend = permissionBackend; this.projectCache = projectCache; } @@ -60,30 +65,37 @@ public class TasksCollection implements ChildCollection task = workQueue.getTask(taskId); - if (task != null) { - if (self.get().getCapabilities().canViewQueue()) { - return new TaskResource(task); - } else if (task instanceof ProjectTask) { - ProjectTask projectTask = ((ProjectTask) task); - ProjectState e = projectCache.get(projectTask.getProjectNameKey()); - if (e != null && e.controlFor(user).isVisible()) { - return new TaskResource(task); - } - } - } - throw new ResourceNotFoundException(id); + taskId = (int) Long.parseLong(id.get(), 16); } catch (NumberFormatException e) { throw new ResourceNotFoundException(id); } + + Task task = workQueue.getTask(taskId); + if (task != null) { + try { + permissionBackend.user(user).check(GlobalPermission.VIEW_QUEUE); + return new TaskResource(task); + } catch (AuthException e) { + // Fall through and try filtering. + } + + if (task instanceof ProjectTask) { + ProjectTask projectTask = ((ProjectTask) task); + ProjectState e = projectCache.get(projectTask.getProjectNameKey()); + if (e != null && e.controlFor(user).isVisible()) { + return new TaskResource(task); + } + } + } + throw new ResourceNotFoundException(id); } @Override diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/KillCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/KillCommand.java index 4ebc5683ac..3465a9c13e 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/KillCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/KillCommand.java @@ -25,6 +25,7 @@ import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.config.DeleteTask; import com.google.gerrit.server.config.TaskResource; import com.google.gerrit.server.config.TasksCollection; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.sshd.AdminHighPriorityCommand; import com.google.gerrit.sshd.SshCommand; import com.google.inject.Inject; @@ -50,7 +51,7 @@ final class KillCommand extends SshCommand { try { TaskResource taskRsrc = tasksCollection.parse(cfgRsrc, IdString.fromDecoded(id)); deleteTask.apply(taskRsrc, null); - } catch (AuthException | ResourceNotFoundException e) { + } catch (AuthException | ResourceNotFoundException | PermissionBackendException e) { stderr.print("kill: " + id + ": No such task\n"); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java index 13db697001..dfb9c9c920 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java @@ -27,6 +27,9 @@ import com.google.gerrit.server.config.ListTasks; import com.google.gerrit.server.config.ListTasks.TaskInfo; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.git.WorkQueue.Task; +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.AdminHighPriorityCommand; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; @@ -60,10 +63,9 @@ final class ShowQueue extends SshCommand { ) private boolean groupByQueue; + @Inject private PermissionBackend permissionBackend; @Inject private ListTasks listTasks; - @Inject private IdentifiedUser currentUser; - @Inject private WorkQueue workQueue; private int columns = 80; @@ -83,7 +85,7 @@ final class ShowQueue extends SshCommand { } @Override - protected void run() throws UnloggedFailure { + protected void run() throws Failure { maxCommandWidth = wide ? Integer.MAX_VALUE : columns - 8 - 12 - 12 - 4 - 4; stdout.print( String.format( @@ -97,10 +99,12 @@ final class ShowQueue extends SshCommand { tasks = listTasks.apply(new ConfigResource()); } catch (AuthException e) { throw die(e); + } catch (PermissionBackendException e) { + throw new Failure(1, "permission backend unavailable", e); } - boolean viewAll = currentUser.getCapabilities().canViewQueue(); - long now = TimeUtil.nowMs(); + boolean viewAll = permissionBackend.user(currentUser).testOrFalse(GlobalPermission.VIEW_QUEUE); + long now = TimeUtil.nowMs(); if (groupByQueue) { ListMultimap byQueue = byQueue(tasks); for (String queueName : byQueue.keySet()) { From a3efaba361208d7538279f0ef4a71d940c65e86a Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 20 Feb 2017 11:42:27 -0800 Subject: [PATCH 2/3] 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()); From b16851133532fc786fdcc20becbf979fbe226530 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 20 Feb 2017 12:20:01 -0800 Subject: [PATCH 3/3] Convert modifyAccount to PermissionBackend Update a few test messages to reflect check throwing a generic AuthException("modify account not permitted") instead of the prior custom text. Change-Id: Ie3ddd250289618a43d2708264863f2e850fd54cb --- .../acceptance/api/accounts/AccountIT.java | 4 +-- .../server/account/CapabilityControl.java | 8 +---- .../gerrit/server/account/CreateEmail.java | 18 ++++++----- .../gerrit/server/account/DeleteEmail.java | 13 ++++++-- .../server/account/GetEditPreferences.java | 16 +++++++--- .../gerrit/server/account/GetPreferences.java | 15 ++++++--- .../gerrit/server/account/GetSshKeys.java | 16 +++++++--- .../google/gerrit/server/account/Index.java | 15 ++++++--- .../google/gerrit/server/account/PutName.java | 12 +++++-- .../gerrit/server/account/PutPreferred.java | 18 ++++++++--- .../gerrit/server/account/PutStatus.java | 18 ++++++++--- .../server/account/SetDiffPreferences.java | 12 +++++-- .../server/account/SetEditPreferences.java | 12 +++++-- .../gerrit/server/account/SetPreferences.java | 13 ++++++-- .../google/gerrit/server/account/SshKeys.java | 19 ++++++++++-- .../server/api/accounts/AccountApiImpl.java | 31 ++++++++++++------- .../sshd/commands/SetAccountCommand.java | 15 ++++++--- 17 files changed, 180 insertions(+), 75 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index 6c4624a240..1a72eda582 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -511,7 +511,7 @@ public class AccountIT extends AbstractDaemonTest { // user cannot delete email of admin exception.expect(AuthException.class); - exception.expectMessage("not allowed to delete email address"); + exception.expectMessage("modify account not permitted"); gApi.accounts().id(admin.id.get()).deleteEmail(admin.email); } @@ -909,7 +909,7 @@ public class AccountIT extends AbstractDaemonTest { // user cannot reindex any account exception.expect(AuthException.class); - exception.expectMessage("not allowed to index account"); + exception.expectMessage("modify account not permitted"); gApi.accounts().id(admin.username).index(); } 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 510038ec2c..791b2e2657 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 @@ -87,11 +87,6 @@ public class CapabilityControl { return canEmailReviewers; } - /** @return true if the user can modify an account for another user. */ - public boolean canModifyAccount() { - return canPerform(GlobalCapability.MODIFY_ACCOUNT) || canAdministrateServer(); - } - /** @return true if the user can view all accounts. */ public boolean canViewAllAccounts() { return canPerform(GlobalCapability.VIEW_ALL_ACCOUNTS) || canAdministrateServer(); @@ -228,8 +223,6 @@ public class CapabilityControl { return canAdministrateServer(); case EMAIL_REVIEWERS: return canEmailReviewers(); - case MODIFY_ACCOUNT: - return canModifyAccount(); case VIEW_ALL_ACCOUNTS: return canViewAllAccounts(); @@ -246,6 +239,7 @@ public class CapabilityControl { case CREATE_GROUP: case CREATE_PROJECT: case MAINTAIN_SERVER: + case MODIFY_ACCOUNT: case STREAM_EVENTS: case VIEW_CONNECTIONS: case VIEW_PLUGINS: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java index b1a5d3b8d9..4af71620d0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java @@ -32,6 +32,9 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.mail.send.OutgoingEmailValidator; import com.google.gerrit.server.mail.send.RegisterNewEmailSender; +import com.google.gerrit.server.permissions.GlobalPermission; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -50,6 +53,7 @@ public class CreateEmail implements RestModifyView private final Provider self; private final Realm realm; + private final PermissionBackend permissionBackend; private final AccountManager accountManager; private final RegisterNewEmailSender.Factory registerNewEmailFactory; private final PutPreferred putPreferred; @@ -60,6 +64,7 @@ public class CreateEmail implements RestModifyView CreateEmail( Provider self, Realm realm, + PermissionBackend permissionBackend, AuthConfig authConfig, AccountManager accountManager, RegisterNewEmailSender.Factory registerNewEmailFactory, @@ -67,6 +72,7 @@ public class CreateEmail implements RestModifyView @Assisted String email) { this.self = self; this.realm = realm; + this.permissionBackend = permissionBackend; this.accountManager = accountManager; this.registerNewEmailFactory = registerNewEmailFactory; this.putPreferred = putPreferred; @@ -78,9 +84,9 @@ public class CreateEmail implements RestModifyView public Response apply(AccountResource rsrc, EmailInput input) throws AuthException, BadRequestException, ResourceConflictException, ResourceNotFoundException, OrmException, EmailException, MethodNotAllowedException, - IOException, ConfigInvalidException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to add email address"); + IOException, ConfigInvalidException, PermissionBackendException { + if (self.get() != rsrc.getUser() || input.noConfirmation) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } if (input == null) { @@ -91,10 +97,6 @@ public class CreateEmail implements RestModifyView throw new BadRequestException("invalid email address"); } - if (input.noConfirmation && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to use no_confirmation"); - } - if (!realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) { throw new MethodNotAllowedException("realm does not allow adding emails"); } @@ -105,7 +107,7 @@ public class CreateEmail implements RestModifyView public Response apply(IdentifiedUser user, EmailInput input) throws AuthException, BadRequestException, ResourceConflictException, ResourceNotFoundException, OrmException, EmailException, MethodNotAllowedException, - IOException, ConfigInvalidException { + IOException, ConfigInvalidException, PermissionBackendException { if (input.email != null && !email.equals(input.email)) { throw new BadRequestException("email address must match URL"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java index bfdf06c33d..b4e2bdb1d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java @@ -29,6 +29,9 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.DeleteEmail.Input; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; +import com.google.gerrit.server.permissions.GlobalPermission; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -43,6 +46,7 @@ public class DeleteEmail implements RestModifyView private final Provider self; private final Realm realm; + private final PermissionBackend permissionBackend; private final Provider dbProvider; private final AccountManager accountManager; private final ExternalIds externalIds; @@ -51,11 +55,13 @@ public class DeleteEmail implements RestModifyView DeleteEmail( Provider self, Realm realm, + PermissionBackend permissionBackend, Provider dbProvider, AccountManager accountManager, ExternalIds externalIds) { this.self = self; this.realm = realm; + this.permissionBackend = permissionBackend; this.dbProvider = dbProvider; this.accountManager = accountManager; this.externalIds = externalIds; @@ -64,9 +70,10 @@ public class DeleteEmail implements RestModifyView @Override public Response apply(AccountResource.Email rsrc, Input input) throws AuthException, ResourceNotFoundException, ResourceConflictException, - MethodNotAllowedException, OrmException, IOException, ConfigInvalidException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to delete email address"); + MethodNotAllowedException, OrmException, IOException, ConfigInvalidException, + PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return apply(rsrc.getUser(), rsrc.getEmail()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java index e385020182..bb207f097f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java @@ -24,6 +24,9 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UserConfigSections; +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; @@ -35,22 +38,27 @@ import org.eclipse.jgit.lib.Repository; @Singleton public class GetEditPreferences implements RestReadView { private final Provider self; + private final PermissionBackend permissionBackend; private final AllUsersName allUsersName; private final GitRepositoryManager gitMgr; @Inject GetEditPreferences( - Provider self, AllUsersName allUsersName, GitRepositoryManager gitMgr) { + Provider self, + PermissionBackend permissionBackend, + AllUsersName allUsersName, + GitRepositoryManager gitMgr) { this.self = self; + this.permissionBackend = permissionBackend; this.allUsersName = allUsersName; this.gitMgr = gitMgr; } @Override public EditPreferencesInfo apply(AccountResource rsrc) - throws AuthException, IOException, ConfigInvalidException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("requires Modify Account capability"); + throws AuthException, IOException, ConfigInvalidException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return readFromGit(rsrc.getUser().getAccountId(), gitMgr, allUsersName, null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java index 77cdbd451d..3ebf864299 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java @@ -19,6 +19,9 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Account; 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.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -26,18 +29,22 @@ import com.google.inject.Singleton; @Singleton public class GetPreferences implements RestReadView { private final Provider self; + private final PermissionBackend permissionBackend; private final AccountCache accountCache; @Inject - GetPreferences(Provider self, AccountCache accountCache) { + GetPreferences( + Provider self, PermissionBackend permissionBackend, AccountCache accountCache) { this.self = self; + this.permissionBackend = permissionBackend; this.accountCache = accountCache; } @Override - public GeneralPreferencesInfo apply(AccountResource rsrc) throws AuthException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("requires Modify Account capability"); + public GeneralPreferencesInfo apply(AccountResource rsrc) + throws AuthException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } Account.Id id = rsrc.getUser().getAccountId(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java index 980d880173..9f5b9d517a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java @@ -22,6 +22,9 @@ import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.AccountSshKey; import com.google.gerrit.server.CurrentUser; 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.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -35,20 +38,25 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException; public class GetSshKeys implements RestReadView { private final Provider self; + private final PermissionBackend permissionBackend; private final VersionedAuthorizedKeys.Accessor authorizedKeys; @Inject - GetSshKeys(Provider self, VersionedAuthorizedKeys.Accessor authorizedKeys) { + GetSshKeys( + Provider self, + PermissionBackend permissionBackend, + VersionedAuthorizedKeys.Accessor authorizedKeys) { this.self = self; + this.permissionBackend = permissionBackend; this.authorizedKeys = authorizedKeys; } @Override public List apply(AccountResource rsrc) throws AuthException, OrmException, RepositoryNotFoundException, IOException, - ConfigInvalidException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to get SSH keys"); + ConfigInvalidException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return apply(rsrc.getUser()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Index.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Index.java index 6943dca47e..ecc6b8caea 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Index.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Index.java @@ -19,6 +19,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.account.Index.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; @@ -29,18 +32,22 @@ public class Index implements RestModifyView { public static class Input {} private final AccountCache accountCache; + private final PermissionBackend permissionBackend; private final Provider self; @Inject - Index(AccountCache accountCache, Provider self) { + Index( + AccountCache accountCache, PermissionBackend permissionBackend, Provider self) { this.accountCache = accountCache; + this.permissionBackend = permissionBackend; this.self = self; } @Override - public Response apply(AccountResource rsrc, Input input) throws IOException, AuthException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to index account"); + public Response apply(AccountResource rsrc, Input input) + throws IOException, AuthException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } // evicting the account from the cache, reindexes the account diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java index 443a549729..7a2868edad 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java @@ -27,6 +27,9 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.PutName.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.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -42,6 +45,7 @@ public class PutName implements RestModifyView { private final Provider self; private final Realm realm; + private final PermissionBackend permissionBackend; private final Provider dbProvider; private final AccountCache byIdCache; @@ -49,10 +53,12 @@ public class PutName implements RestModifyView { PutName( Provider self, Realm realm, + PermissionBackend permissionBackend, Provider dbProvider, AccountCache byIdCache) { this.self = self; this.realm = realm; + this.permissionBackend = permissionBackend; this.dbProvider = dbProvider; this.byIdCache = byIdCache; } @@ -60,9 +66,9 @@ public class PutName implements RestModifyView { @Override public Response apply(AccountResource rsrc, Input input) throws AuthException, MethodNotAllowedException, ResourceNotFoundException, OrmException, - IOException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to change name"); + IOException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return apply(rsrc.getUser(), input); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java index fb87e1eab6..4941cc8e08 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java @@ -23,6 +23,9 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.PutPreferred.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.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -37,20 +40,27 @@ public class PutPreferred implements RestModifyView self; private final Provider dbProvider; + private final PermissionBackend permissionBackend; private final AccountCache byIdCache; @Inject - PutPreferred(Provider self, Provider dbProvider, AccountCache byIdCache) { + PutPreferred( + Provider self, + Provider dbProvider, + PermissionBackend permissionBackend, + AccountCache byIdCache) { this.self = self; this.dbProvider = dbProvider; + this.permissionBackend = permissionBackend; this.byIdCache = byIdCache; } @Override public Response apply(AccountResource.Email rsrc, Input input) - throws AuthException, ResourceNotFoundException, OrmException, IOException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to set preferred email address"); + throws AuthException, ResourceNotFoundException, OrmException, IOException, + PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return apply(rsrc.getUser(), rsrc.getEmail()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java index ff541fdae4..73a720ba3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java @@ -25,6 +25,9 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.PutStatus.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.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -46,20 +49,27 @@ public class PutStatus implements RestModifyView { private final Provider self; private final Provider dbProvider; + private final PermissionBackend permissionBackend; private final AccountCache byIdCache; @Inject - PutStatus(Provider self, Provider dbProvider, AccountCache byIdCache) { + PutStatus( + Provider self, + Provider dbProvider, + PermissionBackend permissionBackend, + AccountCache byIdCache) { this.self = self; this.dbProvider = dbProvider; + this.permissionBackend = permissionBackend; this.byIdCache = byIdCache; } @Override public Response apply(AccountResource rsrc, Input input) - throws AuthException, ResourceNotFoundException, OrmException, IOException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("not allowed to set status"); + throws AuthException, ResourceNotFoundException, OrmException, IOException, + PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } return apply(rsrc.getUser(), input); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java index ac0cc963fb..88e9e205cb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java @@ -29,6 +29,9 @@ import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.UserConfigSections; +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; @@ -41,6 +44,7 @@ public class SetDiffPreferences implements RestModifyView self; private final Provider metaDataUpdateFactory; private final AllUsersName allUsersName; + private final PermissionBackend permissionBackend; private final GitRepositoryManager gitMgr; @Inject @@ -48,19 +52,21 @@ public class SetDiffPreferences implements RestModifyView self, Provider metaDataUpdateFactory, AllUsersName allUsersName, + PermissionBackend permissionBackend, GitRepositoryManager gitMgr) { this.self = self; this.metaDataUpdateFactory = metaDataUpdateFactory; this.allUsersName = allUsersName; + this.permissionBackend = permissionBackend; this.gitMgr = gitMgr; } @Override public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo in) throws AuthException, BadRequestException, ConfigInvalidException, - RepositoryNotFoundException, IOException { - if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) { - throw new AuthException("requires Modify Account capability"); + RepositoryNotFoundException, IOException, PermissionBackendException { + if (self.get() != rsrc.getUser()) { + permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT); } if (in == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java index ca981b8d5f..53285db757 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java @@ -28,6 +28,9 @@ import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.UserConfigSections; +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; @@ -40,6 +43,7 @@ public class SetEditPreferences implements RestModifyView self; private final Provider metaDataUpdateFactory; + private final PermissionBackend permissionBackend; private final GitRepositoryManager gitMgr; private final AllUsersName allUsersName; @@ -47,10 +51,12 @@ public class SetEditPreferences implements RestModifyView self, Provider metaDataUpdateFactory, + PermissionBackend permissionBackend, GitRepositoryManager gitMgr, AllUsersName allUsersName) { this.self = self; this.metaDataUpdateFactory = metaDataUpdateFactory; + this.permissionBackend = permissionBackend; this.gitMgr = gitMgr; this.allUsersName = allUsersName; } @@ -58,9 +64,9 @@ public class SetEditPreferences implements RestModifyView { private final Provider self; private final AccountCache cache; + private final PermissionBackend permissionBackend; private final GeneralPreferencesLoader loader; private final Provider metaDataUpdateFactory; private final AllUsersName allUsersName; @@ -60,6 +64,7 @@ public class SetPreferences implements RestModifyView self, AccountCache cache, + PermissionBackend permissionBackend, GeneralPreferencesLoader loader, Provider metaDataUpdateFactory, AllUsersName allUsersName, @@ -67,6 +72,7 @@ public class SetPreferences implements RestModifyView> views; private final GetSshKeys list; private final Provider self; + private final PermissionBackend permissionBackend; private final VersionedAuthorizedKeys.Accessor authorizedKeys; @Inject @@ -41,10 +46,12 @@ public class SshKeys implements ChildCollection> views, GetSshKeys list, Provider self, + PermissionBackend permissionBackend, VersionedAuthorizedKeys.Accessor authorizedKeys) { this.views = views; this.list = list; this.self = self; + this.permissionBackend = permissionBackend; this.authorizedKeys = authorizedKeys; } @@ -55,9 +62,15 @@ public class SshKeys implements ChildCollection listSshKeys() throws RestApiException { try { return getSshKeys.apply(account); - } catch (OrmException | IOException | ConfigInvalidException e) { + } catch (OrmException | IOException | ConfigInvalidException | PermissionBackendException e) { throw new RestApiException("Cannot list SSH keys", e); } } @@ -423,7 +432,7 @@ public class AccountApiImpl implements AccountApi { AccountResource.SshKey sshKeyRes = sshKeys.parse(account, IdString.fromDecoded(Integer.toString(seq))); deleteSshKey.apply(sshKeyRes, null); - } catch (OrmException | IOException | ConfigInvalidException e) { + } catch (OrmException | IOException | ConfigInvalidException | PermissionBackendException e) { throw new RestApiException("Cannot delete SSH key", e); } } @@ -476,7 +485,7 @@ public class AccountApiImpl implements AccountApi { public void index() throws RestApiException { try { index.apply(account, new Index.Input()); - } catch (IOException e) { + } catch (IOException | PermissionBackendException e) { throw new RestApiException("Cannot index account", e); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java index 21591dd038..bfa1a8188d 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java @@ -42,6 +42,7 @@ import com.google.gerrit.server.account.PutActive; import com.google.gerrit.server.account.PutHttpPassword; import com.google.gerrit.server.account.PutName; import com.google.gerrit.server.account.PutPreferred; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; import com.google.gwtorm.server.OrmException; @@ -174,7 +175,8 @@ final class SetAccountCommand extends SshCommand { } private void setAccount() - throws OrmException, IOException, UnloggedFailure, ConfigInvalidException { + throws OrmException, IOException, UnloggedFailure, ConfigInvalidException, + PermissionBackendException { user = genericUserFactory.create(id); rsrc = new AccountResource(user); try { @@ -237,7 +239,7 @@ final class SetAccountCommand extends SshCommand { private void deleteSshKeys(List sshKeys) throws RestApiException, OrmException, RepositoryNotFoundException, IOException, - ConfigInvalidException { + ConfigInvalidException, PermissionBackendException { List infos = getSshKeys.apply(rsrc); if (sshKeys.contains("ALL")) { for (SshKeyInfo i : infos) { @@ -263,7 +265,8 @@ final class SetAccountCommand extends SshCommand { } private void addEmail(String email) - throws UnloggedFailure, RestApiException, OrmException, IOException, ConfigInvalidException { + throws UnloggedFailure, RestApiException, OrmException, IOException, ConfigInvalidException, + PermissionBackendException { EmailInput in = new EmailInput(); in.email = email; in.noConfirmation = true; @@ -275,7 +278,8 @@ final class SetAccountCommand extends SshCommand { } private void deleteEmail(String email) - throws RestApiException, OrmException, IOException, ConfigInvalidException { + throws RestApiException, OrmException, IOException, ConfigInvalidException, + PermissionBackendException { if (email.equals("ALL")) { List emails = getEmails.apply(rsrc); for (EmailInfo e : emails) { @@ -286,7 +290,8 @@ final class SetAccountCommand extends SshCommand { } } - private void putPreferred(String email) throws RestApiException, OrmException, IOException { + private void putPreferred(String email) + throws RestApiException, OrmException, IOException, PermissionBackendException { for (EmailInfo e : getEmails.apply(rsrc)) { if (e.email.equals(email)) { putPreferred.apply(new AccountResource.Email(user, email), null);