From f49c2a87ab5d4f630ebb0cdd15e5b70f37a20858 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 19 Feb 2017 20:45:19 -0800 Subject: [PATCH] Convert runAs to PermissionBackend Change-Id: Ia08189d864388b45c7f11b41cc835fda57d7e03d --- .../com/google/gerrit/httpd/RunAsFilter.java | 19 +++++++++++++++++-- .../server/account/CapabilityControl.java | 10 +++------- .../java/com/google/gerrit/sshd/SuExec.java | 17 +++++++++++++++-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RunAsFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RunAsFilter.java index 4862a70cf2..47850c40e8 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RunAsFilter.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RunAsFilter.java @@ -19,11 +19,15 @@ import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.config.AuthConfig; +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; @@ -57,6 +61,7 @@ class RunAsFilter implements Filter { private final Provider db; private final boolean enabled; private final DynamicItem session; + private final PermissionBackend permissionBackend; private final AccountResolver accountResolver; @Inject @@ -64,10 +69,12 @@ class RunAsFilter implements Filter { Provider db, AuthConfig config, DynamicItem session, + PermissionBackend permissionBackend, AccountResolver accountResolver) { this.db = db; this.enabled = config.isRunAsEnabled(); this.session = session; + this.permissionBackend = permissionBackend; this.accountResolver = accountResolver; } @@ -85,12 +92,20 @@ class RunAsFilter implements Filter { } CurrentUser self = session.get().getUser(); - if (!self.getCapabilities().canRunAs() + try { + if (!self.isIdentifiedUser()) { // Always disallow for anonymous users, even if permitted by the ACL, // because that would be crazy. - || !self.isIdentifiedUser()) { + throw new AuthException("denied"); + } + permissionBackend.user(self).check(GlobalPermission.RUN_AS); + } catch (AuthException e) { replyError(req, res, SC_FORBIDDEN, "not permitted to use " + RUN_AS, null); return; + } catch (PermissionBackendException e) { + log.warn("cannot check runAs", e); + replyError(req, res, SC_INTERNAL_SERVER_ERROR, RUN_AS + " unavailable", null); + return; } Account target; 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 5aab085a1d..b4d9195914 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 @@ -112,11 +112,6 @@ public class CapabilityControl { return canPerform(GlobalCapability.ACCESS_DATABASE); } - /** @return true if the user can impersonate another user. */ - public boolean canRunAs() { - return canPerform(GlobalCapability.RUN_AS); - } - /** @return which priority queue the user's tasks should be submitted to. */ public QueueProvider.QueueType getQueueType() { // If a non-generic group (that is not Anonymous Users or Registered Users) @@ -245,8 +240,6 @@ public class CapabilityControl { return canMaintainServer(); case MODIFY_ACCOUNT: return canModifyAccount(); - case RUN_AS: - return canRunAs(); case VIEW_ALL_ACCOUNTS: return canViewAllAccounts(); case VIEW_QUEUE: @@ -265,6 +258,9 @@ public class CapabilityControl { case VIEW_CONNECTIONS: case VIEW_PLUGINS: return canPerform(perm.permissionName()) || canAdministrateServer(); + + case RUN_AS: + return canPerform(perm.permissionName()); } throw new PermissionBackendException(perm + " unsupported"); } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java index 53a98ebeeb..54371c1119 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java @@ -18,11 +18,15 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Throwables; import com.google.common.util.concurrent.Atomics; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PeerDaemonUser; import com.google.gerrit.server.config.AuthConfig; +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.SshScope.Context; import com.google.inject.Inject; import java.io.IOException; @@ -45,6 +49,7 @@ import org.kohsuke.args4j.Option; public final class SuExec extends BaseCommand { private final SshScope sshScope; private final DispatchCommandProvider dispatcher; + private final PermissionBackend permissionBackend; private boolean enableRunAs; private CurrentUser caller; @@ -67,6 +72,7 @@ public final class SuExec extends BaseCommand { SuExec( final SshScope sshScope, @CommandName(Commands.ROOT) final DispatchCommandProvider dispatcher, + PermissionBackend permissionBackend, final CurrentUser caller, final SshSession session, final IdentifiedUser.GenericFactory userFactory, @@ -74,6 +80,7 @@ public final class SuExec extends BaseCommand { AuthConfig config) { this.sshScope = sshScope; this.dispatcher = dispatcher; + this.permissionBackend = permissionBackend; this.caller = caller; this.session = session; this.userFactory = userFactory; @@ -115,8 +122,14 @@ public final class SuExec extends BaseCommand { // OK. } else if (!enableRunAs) { throw die("suexec disabled by auth.enableRunAs = false"); - } else if (!caller.getCapabilities().canRunAs()) { - throw die("suexec not permitted"); + } else { + try { + permissionBackend.user(caller).check(GlobalPermission.RUN_AS); + } catch (AuthException e) { + throw die("suexec not permitted"); + } catch (PermissionBackendException e) { + throw die("suexec not available: " + e); + } } }