use explicit exceptions which are internally caught and generalized f…

…or the end user

Signed-off-by: Max Thonagel <[email protected]>

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<PasswordCredential> getTypePassword(List<CredentialType> credentials) {
-		if(credentials == null) {
-			return Optional.empty();
-		}
-		return credentials.stream()
-			.filter(PasswordCredential.class::isInstance)
-			.map(PasswordCredential.class::cast)
-			.collect(MoreCollectors.toOptional());
-	}
-
 	//////////////////// USER MANAGEMENT ////////////////////
 
 	@Override

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.");
+		}
 	}
 }

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");
 
 	}
 }