@@ -94,13 +94,12 @@ public class BasicAuthentication { | |||
} | |||
private UserDto authenticate(Credentials credentials, HttpServletRequest request) { | |||
if (credentials.getPassword().isEmpty()) { | |||
if (!credentials.getPassword().isPresent()) { | |||
UserDto userDto = authenticateFromUserToken(credentials.getLogin()); | |||
authenticationEvent.loginSuccess(request, userDto.getLogin(), Source.local(Method.BASIC_TOKEN)); | |||
return userDto; | |||
} else { | |||
return credentialsAuthentication.authenticate(credentials, request, Method.BASIC); | |||
} | |||
return credentialsAuthentication.authenticate(credentials, request, Method.BASIC); | |||
} | |||
private UserDto authenticateFromUserToken(String token) { |
@@ -20,9 +20,12 @@ | |||
package org.sonar.server.authentication; | |||
import java.util.Objects; | |||
import java.util.Optional; | |||
import javax.annotation.Nullable; | |||
import javax.annotation.concurrent.Immutable; | |||
import org.apache.commons.lang.StringUtils; | |||
import static com.google.common.base.Preconditions.checkArgument; | |||
import static org.apache.commons.lang.StringUtils.defaultIfEmpty; | |||
@Immutable | |||
public class Credentials { | |||
@@ -31,8 +34,9 @@ public class Credentials { | |||
private final String password; | |||
public Credentials(String login, @Nullable String password) { | |||
this.login = Objects.requireNonNull(login, "login must not be null"); | |||
this.password = StringUtils.defaultString(password, ""); | |||
checkArgument(login != null && !login.isEmpty(), "login must not be null nor empty"); | |||
this.login = login; | |||
this.password = defaultIfEmpty(password, null); | |||
} | |||
/** | |||
@@ -43,10 +47,11 @@ public class Credentials { | |||
} | |||
/** | |||
* Non-null password. Can be empty. | |||
* Non-empty password. {@code Optional.empty()} is returned if the password is not set | |||
* or initially empty. {@code Optional.of("")} is never returned. | |||
*/ | |||
public String getPassword() { | |||
return password; | |||
public Optional<String> getPassword() { | |||
return Optional.ofNullable(password); | |||
} | |||
@Override |
@@ -58,7 +58,7 @@ public class CredentialsAuthentication { | |||
private UserDto authenticate(DbSession dbSession, Credentials credentials, HttpServletRequest request, Method method) { | |||
UserDto localUser = dbClient.userDao().selectActiveUserByLogin(dbSession, credentials.getLogin()); | |||
if (localUser != null && localUser.isLocal()) { | |||
localAuthentication.authenticate(dbSession, localUser, credentials.getPassword(), method); | |||
localAuthentication.authenticate(dbSession, localUser, credentials.getPassword().orElse(null), method); | |||
dbSession.commit(); | |||
authenticationEvent.loginSuccess(request, localUser.getLogin(), Source.local(method)); | |||
return localUser; |
@@ -67,7 +67,7 @@ public class CredentialsExternalAuthentication implements Startable { | |||
private ExternalGroupsProvider externalGroupsProvider; | |||
public CredentialsExternalAuthentication(Configuration config, SecurityRealmFactory securityRealmFactory, | |||
UserRegistrar userRegistrar, AuthenticationEvent authenticationEvent) { | |||
UserRegistrar userRegistrar, AuthenticationEvent authenticationEvent) { | |||
this.config = config; | |||
this.securityRealmFactory = securityRealmFactory; | |||
this.userRegistrar = userRegistrar; | |||
@@ -102,7 +102,7 @@ public class CredentialsExternalAuthentication implements Startable { | |||
.setMessage("No user details") | |||
.build(); | |||
} | |||
Authenticator.Context authenticatorContext = new Authenticator.Context(credentials.getLogin(), credentials.getPassword(), request); | |||
Authenticator.Context authenticatorContext = new Authenticator.Context(credentials.getLogin(), credentials.getPassword().orElse(null), request); | |||
boolean status = authenticator.doAuthenticate(authenticatorContext); | |||
if (!status) { | |||
throw AuthenticationException.newBuilder() | |||
@@ -155,7 +155,7 @@ public class CredentialsExternalAuthentication implements Startable { | |||
private Credentials fixCase(Credentials credentials) { | |||
if (config.getBoolean("sonar.authenticator.downcase").orElse(false)) { | |||
return new Credentials(credentials.getLogin().toLowerCase(Locale.ENGLISH), credentials.getPassword()); | |||
return new Credentials(credentials.getLogin().toLowerCase(Locale.ENGLISH), credentials.getPassword().orElse(null)); | |||
} | |||
return credentials; | |||
} |
@@ -21,6 +21,7 @@ | |||
package org.sonar.server.authentication; | |||
import java.security.SecureRandom; | |||
import javax.annotation.Nullable; | |||
import org.apache.commons.codec.digest.DigestUtils; | |||
import org.mindrot.jbcrypt.BCrypt; | |||
import org.sonar.db.DbClient; | |||
@@ -55,7 +56,7 @@ public class CredentialsLocalAuthentication { | |||
* If the password must be updated because an old algorithm is used, the UserDto is updated but the session | |||
* is not committed | |||
*/ | |||
public void authenticate(DbSession session, UserDto user, String password, Method method) { | |||
public void authenticate(DbSession session, UserDto user, @Nullable String password, Method method) { | |||
if (user.getHashMethod() == null) { | |||
throw AuthenticationException.newBuilder() | |||
.setSource(Source.local(method)) |
@@ -22,22 +22,39 @@ package org.sonar.server.authentication; | |||
import org.junit.Test; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
import static org.assertj.core.api.Assertions.catchThrowable; | |||
public class CredentialsTest { | |||
@Test | |||
public void password_can_be_empty_but_not_null() { | |||
public void login_cant_be_empty() { | |||
Throwable thrown = catchThrowable(() -> new Credentials("", "bar")); | |||
assertThat(thrown) | |||
.isInstanceOf(IllegalArgumentException.class) | |||
.hasMessage("login must not be null nor empty"); | |||
thrown = catchThrowable(() -> new Credentials(null, "bar")); | |||
assertThat(thrown) | |||
.isInstanceOf(IllegalArgumentException.class) | |||
.hasMessage("login must not be null nor empty"); | |||
Credentials underTest = new Credentials("foo", "bar"); | |||
assertThat(underTest.getLogin()).isEqualTo("foo"); | |||
} | |||
@Test | |||
public void password_cant_be_empty_string() { | |||
Credentials underTest = new Credentials("foo", ""); | |||
assertThat(underTest.getPassword()).isEqualTo(""); | |||
assertThat(underTest.getPassword()).isEmpty(); | |||
underTest = new Credentials("foo", null); | |||
assertThat(underTest.getPassword()).isEqualTo(""); | |||
assertThat(underTest.getPassword()).isEmpty(); | |||
underTest = new Credentials("foo", " "); | |||
assertThat(underTest.getPassword()).isEqualTo(" "); | |||
assertThat(underTest.getPassword()).hasValue(" "); | |||
underTest = new Credentials("foo", "bar"); | |||
assertThat(underTest.getPassword()).isEqualTo("bar"); | |||
assertThat(underTest.getPassword()).hasValue("bar"); | |||
} | |||
@Test |