From 5a2f333882811a27c2a89a73f75e473d476b1cd2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 14 Jun 2011 18:06:21 -0700 Subject: [PATCH] Only automatically create accounts for LDAP systems If the account management is LDAP, try to automatically create accounts by looking up the data in LDAP. Otherwise fail and reject an invalid account reference that was supplied on the command line via SSH. Change-Id: I9a458ba527f7f3cfb2e80b9597e2f0e83f62a0c1 --- .../gerrit/server/account/AuthRequest.java | 9 ++++++ .../gerrit/server/auth/ldap/LdapRealm.java | 2 +- .../gerrit/sshd/args4j/AccountIdHandler.java | 31 ++++++++++++++----- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java index b3ca18a312..2a540297e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java @@ -57,6 +57,7 @@ public class AuthRequest { private String displayName; private String emailAddress; private String userName; + private boolean skipAuthentication; public AuthRequest(final String externalId) { this.externalId = externalId; @@ -108,4 +109,12 @@ public class AuthRequest { public void setUserName(final String user) { userName = user; } + + public boolean isSkipAuthentication() { + return skipAuthentication; + } + + public void setSkipAuthentication(boolean skip) { + skipAuthentication = skip; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index cd3f2c0832..e008e6ccd4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java @@ -193,7 +193,7 @@ class LdapRealm implements Realm { final Helper.LdapSchema schema = helper.getSchema(ctx); final LdapQuery.Result m = helper.findAccount(schema, ctx, username); - if (authConfig.getAuthType() == AuthType.LDAP) { + if (authConfig.getAuthType() == AuthType.LDAP && !who.isSkipAuthentication()) { // We found the user account, but we need to verify // the password matches it before we can continue. // diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountIdHandler.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountIdHandler.java index b8bf0fd4eb..1f6454be9a 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountIdHandler.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountIdHandler.java @@ -15,11 +15,12 @@ package com.google.gerrit.sshd.args4j; import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.reviewdb.AuthType; import com.google.gerrit.server.account.AccountException; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AuthRequest; -import com.google.gerrit.server.account.AuthResult; +import com.google.gerrit.server.config.AuthConfig; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -34,16 +35,19 @@ import org.kohsuke.args4j.spi.Setter; public class AccountIdHandler extends OptionHandler { private final AccountResolver accountResolver; private final AccountManager accountManager; + private final AuthType authType; @SuppressWarnings({"unchecked", "rawtypes"}) @Inject public AccountIdHandler(final AccountResolver accountResolver, final AccountManager accountManager, + final AuthConfig authConfig, @Assisted final CmdLineParser parser, @Assisted final OptionDef option, @Assisted final Setter setter) { super(parser, option, setter); this.accountResolver = accountResolver; this.accountManager = accountManager; + this.authType = authConfig.getAuthType(); } @Override @@ -56,7 +60,15 @@ public class AccountIdHandler extends OptionHandler { if (a != null) { accountId = a.getId(); } else { - accountId = createAccountIfUserCanBeAuthenticated(token); + switch (authType) { + case HTTP_LDAP: + case CLIENT_SSL_CERT_LDAP: + case LDAP: + accountId = createAccountByLdap(token); + break; + default: + throw new CmdLineException(owner, "user \"" + token + "\" not found"); + } } } catch (OrmException e) { throw new CmdLineException(owner, "database is down"); @@ -65,15 +77,18 @@ public class AccountIdHandler extends OptionHandler { return 1; } - private Account.Id createAccountIfUserCanBeAuthenticated(final String username) + private Account.Id createAccountByLdap(String user) throws CmdLineException { + if (!user.matches(Account.USER_NAME_PATTERN)) { + throw new CmdLineException(owner, "user \"" + user + "\" not found"); + } + try { - final AuthRequest areq = AuthRequest.forUser(username); - final AuthResult arsp = accountManager.authenticate(areq); - return arsp.getAccountId(); + AuthRequest req = AuthRequest.forUser(user); + req.setSkipAuthentication(true); + return accountManager.authenticate(req).getAccountId(); } catch (AccountException e) { - throw new CmdLineException(owner, "Unable to authenticate user \"" - + username + "\"", e); + throw new CmdLineException(owner, "user \"" + user + "\" not found"); } }