From 2a29838da3cda5114ac0f158e12072c03b7c50e8 Mon Sep 17 00:00:00 2001 From: Richard Christie Date: Thu, 28 Jun 2018 13:08:44 +0100 Subject: [PATCH] Relax access to ssh set-account command This commit modifies access to the set-account to enable users to set their own account details without needing MODIFY_ACCOUNT. This makes it similar to the REST API. All other access should be the same as before, though we now check some settings we know require admin and catch those early to give clearer error messages to the user. Previously things failed with: fatal: administrate server not permitted without saying which out of potentially multiple settings being changed required that. Change-Id: I7e07743209b6cd6c7ececa21e4e5f65ee629bedf --- Documentation/cmd-set-account.txt | 5 ++- .../sshd/commands/SetAccountCommand.java | 38 ++++++++++++++++--- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Documentation/cmd-set-account.txt b/Documentation/cmd-set-account.txt index adbb6b13cf..276306eadc 100644 --- a/Documentation/cmd-set-account.txt +++ b/Documentation/cmd-set-account.txt @@ -26,8 +26,9 @@ It also allows managing email addresses, which bypasses the verification step we force within the UI. == ACCESS -Caller must be a member of the privileged 'Administrators' group, -or have been granted +Users can call this to update their own accounts. To update a different +account, a caller must be a member of the privileged 'Administrators' +group, or have been granted link:access-control.html#capability_modifyAccount[the 'Modify Account' global capability]. For security reasons only the members of the privileged 'Administrators' group can add or delete SSH keys for a user. 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 6e1a3b8bdc..3fbf81d77f 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 @@ -18,9 +18,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Strings; import com.google.gerrit.common.RawInputUtil; -import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.errors.EmailException; -import com.google.gerrit.extensions.annotations.RequiresCapability; import com.google.gerrit.extensions.api.accounts.EmailInput; import com.google.gerrit.extensions.common.EmailInfo; import com.google.gerrit.extensions.common.SshKeyInfo; @@ -30,6 +28,7 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountSshKey; +import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.AddSshKey; @@ -43,11 +42,14 @@ 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.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.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -62,7 +64,6 @@ import org.kohsuke.args4j.Option; /** Set a user's account settings. * */ @CommandMetaData(name = "set-account", description = "Change an account's settings") -@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT) final class SetAccountCommand extends SshCommand { @Argument( @@ -141,23 +142,49 @@ final class SetAccountCommand extends SshCommand { @Inject private DeleteSshKey deleteSshKey; + @Inject private PermissionBackend permissionBackend; + + @Inject private Provider userProvider; + private IdentifiedUser user; private AccountResource rsrc; @Override public void run() throws Exception { + user = genericUserFactory.create(id); + validate(); setAccount(); } private void validate() throws UnloggedFailure { - if (active && inactive) { - throw die("--active and --inactive options are mutually exclusive."); + PermissionBackend.WithUser userPermission = permissionBackend.user(userProvider); + + boolean isAdmin = userPermission.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER); + boolean canModifyAccount = + isAdmin || userPermission.testOrFalse(GlobalPermission.MODIFY_ACCOUNT); + + if (!user.hasSameAccountId(userProvider.get()) && !canModifyAccount) { + throw die( + "Setting another user's account information requries 'modify account' or 'administrate server' capabilities."); } + if (active || inactive) { + if (!canModifyAccount) { + throw die( + "--active and --inactive require 'modify account' or 'administrate server' capabilities."); + } + if (active && inactive) { + throw die("--active and --inactive options are mutually exclusive."); + } + } + if (generateHttpPassword && clearHttpPassword) { throw die("--generate-http-password and --clear-http-password are mutually exclusive."); } if (!Strings.isNullOrEmpty(httpPassword)) { // gave --http-password + if (!isAdmin) { + throw die("--http-password requires 'administrate server' capabilities."); + } if (generateHttpPassword) { throw die("--http-password and --generate-http-password options are mutually exclusive."); } @@ -184,7 +211,6 @@ final class SetAccountCommand extends SshCommand { private void setAccount() throws OrmException, IOException, UnloggedFailure, ConfigInvalidException, PermissionBackendException { - user = genericUserFactory.create(id); rsrc = new AccountResource(user); try { for (String email : addEmails) {