diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/basic/LocalAuthenticationRealm.java b/backend/src/main/java/com/bakdata/conquery/models/auth/basic/LocalAuthenticationRealm.java index 016782e7e1..26ad9445c8 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/basic/LocalAuthenticationRealm.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/basic/LocalAuthenticationRealm.java @@ -3,7 +3,6 @@ import java.io.File; import java.io.IOException; import java.util.List; -import java.util.Optional; import java.util.concurrent.Executors; import javax.validation.Validator; @@ -29,7 +28,6 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; -import com.google.common.collect.MoreCollectors; import com.password4j.HashingFunction; import com.password4j.Password; import io.dropwizard.util.Duration; @@ -42,8 +40,8 @@ import lombok.extern.slf4j.Slf4j; import org.apache.shiro.authc.AuthenticationException; import org.apache.shiro.authc.AuthenticationToken; +import org.apache.shiro.authc.CredentialsException; import org.apache.shiro.authc.IncorrectCredentialsException; -import org.apache.shiro.authc.UnknownAccountException; import org.apache.shiro.realm.AuthenticatingRealm; import org.apache.shiro.util.Destroyable; @@ -136,24 +134,24 @@ public ConqueryAuthenticationInfo doGetAuthenticationInfo(AuthenticationToken to //////////////////// FOR USERNAME/PASSWORD public String createAccessToken(String username, String password) { - // Check the password which is afterwards cleared if (username.isEmpty()) { throw new IncorrectCredentialsException("Username was empty"); } if (password.isEmpty()) { throw new IncorrectCredentialsException("Password was empty"); } - HashEntry hashedEntry = passwordStore.get(new UserId(username)); + final UserId userId = new UserId(username); + HashEntry hashedEntry = passwordStore.get(userId); if (hashedEntry == null) { - throw new UnknownAccountException("Provided username or password was not valid."); + throw new CredentialsException("No password hash was found for user: " + username); } final String hash = hashedEntry.getHash(); if (!Password.check(password.getBytes(), hash.getBytes()).with(PasswordHelper.getHashingFunction(hash))) { - throw new UnknownAccountException("Provided username or password was not valid."); + throw new IncorrectCredentialsException("Password was was invalid for user: " + userId); } - return centralTokenRealm.createTokenForUser(new UserId(username), validDuration); + return centralTokenRealm.createTokenForUser(userId, validDuration); } /** @@ -174,25 +172,6 @@ else if (credential instanceof PasswordHashCredential passwordHashCredential) { throw new IllegalArgumentException("CredentialType not supported yet: " + credential.getClass()); } - /** - * Checks the provided credentials for the realm-compatible - * {@link PasswordCredential}. However only one credential of this type is - * allowed to be provided. - * - * @param credentials - * A list of possible credentials. - * @return The password credential. - */ - private static Optional getTypePassword(List credentials) { - if(credentials == null) { - return Optional.empty(); - } - return credentials.stream() - .filter(PasswordCredential.class::isInstance) - .map(PasswordCredential.class::cast) - .collect(MoreCollectors.toOptional()); - } - //////////////////// USER MANAGEMENT //////////////////// @Override diff --git a/backend/src/main/java/com/bakdata/conquery/resources/unprotected/TokenResource.java b/backend/src/main/java/com/bakdata/conquery/resources/unprotected/TokenResource.java index 23cd8c62d6..da4be49818 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/unprotected/TokenResource.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/unprotected/TokenResource.java @@ -1,6 +1,7 @@ package com.bakdata.conquery.resources.unprotected; import javax.ws.rs.Consumes; +import javax.ws.rs.NotAuthorizedException; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.Produces; @@ -10,9 +11,12 @@ import com.bakdata.conquery.apiv1.auth.UsernamePasswordToken; import com.bakdata.conquery.models.auth.basic.AccessTokenCreator; import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.shiro.authc.AuthenticationException; @Path("/") @AllArgsConstructor +@Slf4j public class TokenResource { private final AccessTokenCreator realm; @@ -21,6 +25,12 @@ public class TokenResource { @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) public JwtWrapper getToken(UsernamePasswordToken token) { - return new JwtWrapper(realm.createAccessToken(token.getUser(), token.getPassword())); + try { + return new JwtWrapper(realm.createAccessToken(token.getUser(), token.getPassword())); + } + catch (AuthenticationException e) { + log.warn("Failed to authorize request", e); + throw new NotAuthorizedException("Failed to authenticate request. The cause has been logged."); + } } } diff --git a/backend/src/test/java/com/bakdata/conquery/models/auth/LocalAuthRealmTest.java b/backend/src/test/java/com/bakdata/conquery/models/auth/LocalAuthRealmTest.java index 1460d2be14..fa560e3c01 100644 --- a/backend/src/test/java/com/bakdata/conquery/models/auth/LocalAuthRealmTest.java +++ b/backend/src/test/java/com/bakdata/conquery/models/auth/LocalAuthRealmTest.java @@ -18,8 +18,8 @@ import io.dropwizard.jersey.validation.Validators; import io.dropwizard.util.Duration; import org.apache.commons.io.FileUtils; -import org.apache.shiro.authc.AuthenticationException; import org.apache.shiro.authc.BearerToken; +import org.apache.shiro.authc.CredentialsException; import org.apache.shiro.authc.IncorrectCredentialsException; import org.apache.shiro.util.LifecycleUtils; import org.junit.jupiter.api.AfterAll; @@ -102,13 +102,13 @@ public void testEmptyPassword() { @Test public void testWrongPassword() { assertThatThrownBy(() -> realm.createAccessToken("TestUser", "wrongPassword")) - .isInstanceOf(AuthenticationException.class).hasMessageContaining("Provided username or password was not valid."); + .isInstanceOf(IncorrectCredentialsException.class).hasMessageContaining("Password was was invalid for user"); } @Test public void testWrongUsername() { assertThatThrownBy(() -> realm.createAccessToken("NoTestUser", "testPassword")) - .isInstanceOf(AuthenticationException.class).hasMessageContaining("Provided username or password was not valid."); + .isInstanceOf(CredentialsException.class).hasMessageContaining("No password hash was found for user"); } @Test @@ -127,7 +127,7 @@ public void testUserUpdate() { realm.updateUser(user1, new PasswordCredential("newTestPassword")); // Wrong (old) password assertThatThrownBy(() -> realm.createAccessToken("TestUser", "testPassword")) - .isInstanceOf(AuthenticationException.class).hasMessageContaining("Provided username or password was not valid."); + .isInstanceOf(IncorrectCredentialsException.class).hasMessageContaining("Password was was invalid for user"); // Right (new) password String jwt = realm.createAccessToken("TestUser", "newTestPassword"); @@ -139,7 +139,7 @@ public void testRemoveUser() { realm.removeUser(user1); // Wrong password assertThatThrownBy(() -> realm.createAccessToken("TestUser", "testPassword")) - .isInstanceOf(AuthenticationException.class).hasMessageContaining("Provided username or password was not valid."); + .isInstanceOf(CredentialsException.class).hasMessageContaining("No password hash was found for user"); } }