com.google.gerrit.server.auth.AuthRequest: Let methods return Optional

This makes it more explicit that callers must handle the case where the
returned user name / password is absent.

Change-Id: Iee464f62ac41941d6397702766d0834f37602aff
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-01-19 14:18:38 +01:00
parent a1df95de04
commit ff3823e4eb
4 changed files with 21 additions and 20 deletions

View File

@@ -203,7 +203,7 @@ public class AccountState {
return userName; return userName;
} }
public boolean checkPassword(String password, String username) { public boolean checkPassword(@Nullable String password, String username) {
if (password == null) { if (password == null) {
return false; return false;
} }

View File

@@ -14,16 +14,18 @@
package com.google.gerrit.server.auth; package com.google.gerrit.server.auth;
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import java.util.Optional;
/** Defines an abstract request for user authentication to Gerrit. */ /** Defines an abstract request for user authentication to Gerrit. */
public abstract class AuthRequest { public abstract class AuthRequest {
private final String username; private final Optional<String> username;
private final String password; private final Optional<String> password;
protected AuthRequest(String username, String password) { protected AuthRequest(@Nullable String username, @Nullable String password) {
this.username = username; this.username = Optional.ofNullable(Strings.emptyToNull(username));
this.password = password; this.password = Optional.ofNullable(Strings.emptyToNull(password));
} }
/** /**
@@ -31,8 +33,7 @@ public abstract class AuthRequest {
* *
* @return username for authentication or null for anonymous access. * @return username for authentication or null for anonymous access.
*/ */
@Nullable public final Optional<String> getUsername() {
public final String getUsername() {
return username; return username;
} }
@@ -41,8 +42,7 @@ public abstract class AuthRequest {
* *
* @return user's credentials or null * @return user's credentials or null
*/ */
@Nullable public final Optional<String> getPassword() {
public final String getPassword() {
return password; return password;
} }
} }

View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.auth; package com.google.gerrit.server.auth;
import com.google.common.base.Strings;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
@@ -43,15 +42,15 @@ public class InternalAuthBackend implements AuthBackend {
public AuthUser authenticate(AuthRequest req) public AuthUser authenticate(AuthRequest req)
throws MissingCredentialsException, InvalidCredentialsException, UnknownUserException, throws MissingCredentialsException, InvalidCredentialsException, UnknownUserException,
UserNotAllowedException, AuthException { UserNotAllowedException, AuthException {
if (Strings.isNullOrEmpty(req.getUsername()) || Strings.isNullOrEmpty(req.getPassword())) { if (!req.getUsername().isPresent() || !req.getPassword().isPresent()) {
throw new MissingCredentialsException(); throw new MissingCredentialsException();
} }
String username; String username;
if (authConfig.isUserNameToLowerCase()) { if (authConfig.isUserNameToLowerCase()) {
username = req.getUsername().toLowerCase(Locale.US); username = req.getUsername().map(u -> u.toLowerCase(Locale.US)).get();
} else { } else {
username = req.getUsername(); username = req.getUsername().get();
} }
AccountState who = AccountState who =
@@ -64,7 +63,7 @@ public class InternalAuthBackend implements AuthBackend {
+ ": account inactive or not provisioned in Gerrit"); + ": account inactive or not provisioned in Gerrit");
} }
if (!who.checkPassword(req.getPassword(), username)) { if (!who.checkPassword(req.getPassword().get(), username)) {
throw new InvalidCredentialsException(); throw new InvalidCredentialsException();
} }
return new AuthUser(AuthUser.UUID.create(username), username); return new AuthUser(AuthUser.UUID.create(username), username);

View File

@@ -60,16 +60,18 @@ public class LdapAuthBackend implements AuthBackend {
public AuthUser authenticate(AuthRequest req) public AuthUser authenticate(AuthRequest req)
throws MissingCredentialsException, InvalidCredentialsException, UnknownUserException, throws MissingCredentialsException, InvalidCredentialsException, UnknownUserException,
UserNotAllowedException, AuthException { UserNotAllowedException, AuthException {
if (req.getUsername() == null || req.getPassword() == null) { if (!req.getUsername().isPresent() || !req.getPassword().isPresent()) {
throw new MissingCredentialsException(); throw new MissingCredentialsException();
} }
final String username = String username =
lowerCaseUsername ? req.getUsername().toLowerCase(Locale.US) : req.getUsername(); lowerCaseUsername
? req.getUsername().map(u -> u.toLowerCase(Locale.US)).get()
: req.getUsername().get();
try { try {
final DirContext ctx; final DirContext ctx;
if (authConfig.getAuthType() == AuthType.LDAP_BIND) { if (authConfig.getAuthType() == AuthType.LDAP_BIND) {
ctx = helper.authenticate(username, req.getPassword()); ctx = helper.authenticate(username, req.getPassword().get());
} else { } else {
ctx = helper.open(); ctx = helper.open();
} }
@@ -81,7 +83,7 @@ public class LdapAuthBackend implements AuthBackend {
// We found the user account, but we need to verify // We found the user account, but we need to verify
// the password matches it before we can continue. // the password matches it before we can continue.
// //
helper.close(helper.authenticate(m.getDN(), req.getPassword())); helper.close(helper.authenticate(m.getDN(), req.getPassword().get()));
} }
return new AuthUser(AuthUser.UUID.create(username), username); return new AuthUser(AuthUser.UUID.create(username), username);
} finally { } finally {