opendev/gerrit
Code Issues Proposed changes

Relax access to ssh set-account command

Browse Source 
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
This commit is contained in:
Richard Christie 2018-06-28 13:08:44 +01:00
parent efa38be397
commit 2a29838da3
2 changed files with 35 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/cmd-set-account.txt
View File

@ -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.

38
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
View File

@ -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<CurrentUser> 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) {