From 8e06ccb7e1c18d95a1c720b8fb236074971d448a Mon Sep 17 00:00:00 2001 From: Robert von Burg Date: Thu, 9 Mar 2017 11:07:39 +0100 Subject: [PATCH] [Minor] Don't log exceptions if user fails to auth --- .../privilege/base/AccessDeniedException.java | 10 +++ .../base/InvalidCredentialsException.java | 10 +++ .../handler/DefaultPrivilegeHandler.java | 5 +- .../handler/UserChallengeHandler.java | 50 +++++++------ .../rest/endpoint/AuthenticationService.java | 73 ++++++++++++------- 5 files changed, 98 insertions(+), 50 deletions(-) diff --git a/li.strolch.privilege/src/main/java/li/strolch/privilege/base/AccessDeniedException.java b/li.strolch.privilege/src/main/java/li/strolch/privilege/base/AccessDeniedException.java index c59b9a04e..7fb352453 100644 --- a/li.strolch.privilege/src/main/java/li/strolch/privilege/base/AccessDeniedException.java +++ b/li.strolch.privilege/src/main/java/li/strolch/privilege/base/AccessDeniedException.java @@ -31,4 +31,14 @@ public class AccessDeniedException extends PrivilegeException { public AccessDeniedException(String msg) { super(msg); } + + /** + * @param msg + * detail on why and where access was denied + * @param e + * root exception + */ + public AccessDeniedException(String msg, Exception e) { + super(msg, e); + } } diff --git a/li.strolch.privilege/src/main/java/li/strolch/privilege/base/InvalidCredentialsException.java b/li.strolch.privilege/src/main/java/li/strolch/privilege/base/InvalidCredentialsException.java index 9dcca5b74..8eced97e1 100644 --- a/li.strolch.privilege/src/main/java/li/strolch/privilege/base/InvalidCredentialsException.java +++ b/li.strolch.privilege/src/main/java/li/strolch/privilege/base/InvalidCredentialsException.java @@ -16,4 +16,14 @@ public class InvalidCredentialsException extends AccessDeniedException { public InvalidCredentialsException(String msg) { super(msg); } + + /** + * @param msg + * detail on why and where access was denied + * @param e + * root exception + */ + public InvalidCredentialsException(String msg, Exception e) { + super(msg, e); + } } diff --git a/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/DefaultPrivilegeHandler.java b/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/DefaultPrivilegeHandler.java index f933f5cba..009fae6a2 100644 --- a/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/DefaultPrivilegeHandler.java +++ b/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/DefaultPrivilegeHandler.java @@ -1119,10 +1119,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { return certificate; } catch (RuntimeException e) { - String msg = "User {0} Failed to authenticate: {1}"; //$NON-NLS-1$ + String msg = "User {0} failed to authenticate: {1}"; //$NON-NLS-1$ msg = MessageFormat.format(msg, username, e.getMessage()); - logger.error(msg); - throw e; + throw new InvalidCredentialsException(msg, e); } finally { clearPassword(password); } diff --git a/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/UserChallengeHandler.java b/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/UserChallengeHandler.java index dcbd4d971..b56533469 100644 --- a/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/UserChallengeHandler.java +++ b/li.strolch.privilege/src/main/java/li/strolch/privilege/handler/UserChallengeHandler.java @@ -1,6 +1,5 @@ package li.strolch.privilege.handler; -import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -27,24 +26,7 @@ public abstract class UserChallengeHandler { * a map containing configuration properties */ public void initialize(Map parameterMap) { - this.challenges = Collections.synchronizedMap(new HashMap<>()); - } - - /** - * Initiate a password reset challenge for the given user - * - * @param usage - * the {@link Usage} for this certificate - * @param user - * the user for which to initiate the challenge for - */ - public void initiateChallengeFor(Usage usage, User user) { - - String challenge = generateChallenge(); - UserChallenge userChallenge = new UserChallenge(usage, user, challenge); - this.challenges.put(user, userChallenge); - - sendChallengeToUser(user, challenge); + this.challenges = new HashMap<>(); } /** @@ -57,6 +39,23 @@ public abstract class UserChallengeHandler { return challenge; } + /** + * Initiate a password reset challenge for the given user + * + * @param usage + * the {@link Usage} for this certificate + * @param user + * the user for which to initiate the challenge for + */ + public synchronized void initiateChallengeFor(Usage usage, User user) { + + String challenge = generateChallenge(); + UserChallenge userChallenge = new UserChallenge(usage, user, challenge); + this.challenges.put(user, userChallenge); + + sendChallengeToUser(user, challenge); + } + /** * Validate the response of a challenge for the given username * @@ -70,18 +69,25 @@ public abstract class UserChallengeHandler { * * @return the challenge */ - public UserChallenge validateResponse(User user, String challenge) throws PrivilegeException { + public synchronized UserChallenge validateResponse(User user, String challenge) throws PrivilegeException { - UserChallenge userChallenge = this.challenges.remove(user); + // get the challenge + UserChallenge userChallenge = this.challenges.get(user); if (userChallenge == null) throw new PrivilegeException("No challenge exists for user " + user.getUsername()); + + // validate the challenge if (!userChallenge.getUser().equals(user)) throw new PrivilegeException("UserChallenge invalid: Wrong user!"); - if (!userChallenge.getChallenge().equals(challenge)) throw new PrivilegeException("Challenge is invalid!"); + // then remove it + this.challenges.remove(user); + + // it's full filled userChallenge.fulfilled(); + return userChallenge; } diff --git a/li.strolch.rest/src/main/java/li/strolch/rest/endpoint/AuthenticationService.java b/li.strolch.rest/src/main/java/li/strolch/rest/endpoint/AuthenticationService.java index 6f0fc645a..7beec3587 100644 --- a/li.strolch.rest/src/main/java/li/strolch/rest/endpoint/AuthenticationService.java +++ b/li.strolch.rest/src/main/java/li/strolch/rest/endpoint/AuthenticationService.java @@ -59,6 +59,7 @@ import li.strolch.rest.StrolchRestfulConstants; import li.strolch.rest.StrolchSessionHandler; import li.strolch.rest.model.Result; import li.strolch.runtime.privilege.PrivilegeHandler; +import li.strolch.utils.helper.ExceptionHelper; /** * @author Robert von Burg @@ -168,11 +169,11 @@ public class AuthenticationService { .header(HttpHeaders.AUTHORIZATION, certificate.getAuthToken()).cookie(cookie).build(); } catch (InvalidCredentialsException e) { - logger.error(e.getMessage(), e); + logger.error("Authentication failed due to: " + e.getMessage()); loginResult.addProperty("msg", "Could not log in as the given credentials are invalid"); //$NON-NLS-1$ return Response.status(Status.UNAUTHORIZED).entity(loginResult.toString()).build(); } catch (AccessDeniedException e) { - logger.error(e.getMessage(), e); + logger.error("Authentication failed due to: " + e.getMessage()); loginResult.addProperty("msg", MessageFormat.format("Could not log in due to: {0}", e.getMessage())); //$NON-NLS-1$ return Response.status(Status.UNAUTHORIZED).entity(loginResult.toString()).build(); } catch (StrolchException | PrivilegeException e) { @@ -208,7 +209,7 @@ public class AuthenticationService { return Response.ok().entity(logoutResult.toString()).build(); } catch (StrolchException | PrivilegeException e) { - logger.error(e.getMessage(), e); + logger.error("Failed to invalidate session due to: " + e.getMessage()); logoutResult.addProperty("msg", MessageFormat.format("Could not logout due to: {0}", e.getMessage())); //$NON-NLS-1$ return Response.status(Status.UNAUTHORIZED).entity(logoutResult.toString()).build(); } catch (Exception e) { @@ -252,39 +253,61 @@ public class AuthenticationService { @Produces(MediaType.APPLICATION_JSON) @Path("challenge") public Response initiateChallenge(String data) { - JsonObject jsonObject = new JsonParser().parse(data).getAsJsonObject(); - String username = jsonObject.get("username").getAsString(); - String usage = jsonObject.get("usage").getAsString(); - StrolchSessionHandler sessionHandler = RestfulStrolchComponent.getInstance().getSessionHandler(); - sessionHandler.initiateChallengeFor(Usage.byValue(usage), username); + try { + JsonObject jsonObject = new JsonParser().parse(data).getAsJsonObject(); + String username = jsonObject.get("username").getAsString(); + String usage = jsonObject.get("usage").getAsString(); - return Response.ok(new Result(), MediaType.APPLICATION_JSON).build(); + StrolchSessionHandler sessionHandler = RestfulStrolchComponent.getInstance().getSessionHandler(); + sessionHandler.initiateChallengeFor(Usage.byValue(usage), username); + + return Response.ok(new Result(), MediaType.APPLICATION_JSON).build(); + + } catch (PrivilegeException e) { + logger.error("Challenge initialization failed: " + e.getMessage()); + JsonObject root = new JsonObject(); + root.addProperty("msg", ExceptionHelper.getExceptionMessage(e)); + String json = new Gson().toJson(root); + return Response.status(Status.UNAUTHORIZED).entity(json).build(); + } } @PUT @Produces(MediaType.APPLICATION_JSON) @Path("challenge") public Response validateChallenge(@Context HttpServletRequest request, String data) { - JsonObject jsonObject = new JsonParser().parse(data).getAsJsonObject(); - String username = jsonObject.get("username").getAsString(); - String challenge = jsonObject.get("challenge").getAsString(); - StrolchSessionHandler sessionHandler = RestfulStrolchComponent.getInstance().getSessionHandler(); - Certificate certificate = sessionHandler.validateChallenge(username, challenge); + try { - jsonObject = new JsonObject(); - jsonObject.addProperty("authToken", certificate.getAuthToken()); + JsonObject jsonObject = new JsonParser().parse(data).getAsJsonObject(); + String username = jsonObject.get("username").getAsString(); + String challenge = jsonObject.get("challenge").getAsString(); - boolean secureCookie = RestfulStrolchComponent.getInstance().isSecureCookie(); - if (secureCookie && !request.getScheme().equals("https")) { - String msg = "Authorization cookie is secure, but connection is not secure! Cookie won't be passed to client!"; - logger.warn(msg); + StrolchSessionHandler sessionHandler = RestfulStrolchComponent.getInstance().getSessionHandler(); + Certificate certificate = sessionHandler.validateChallenge(username, challenge); + + jsonObject = new JsonObject(); + jsonObject.addProperty("authToken", certificate.getAuthToken()); + + boolean secureCookie = RestfulStrolchComponent.getInstance().isSecureCookie(); + if (secureCookie && !request.getScheme().equals("https")) { + String msg = "Authorization cookie is secure, but connection is not secure! Cookie won't be passed to client!"; + logger.warn(msg); + } + + NewCookie cookie = new NewCookie(StrolchRestfulConstants.STROLCH_AUTHORIZATION, certificate.getAuthToken(), + "/", null, "Authorization header", (int) TimeUnit.DAYS.toSeconds(1), secureCookie); + + return Response.ok().entity(jsonObject.toString())// + .header(HttpHeaders.AUTHORIZATION, certificate.getAuthToken()).cookie(cookie).build(); + + } catch (PrivilegeException e) { + logger.error("Challenge validation failed: " + e.getMessage()); + JsonObject root = new JsonObject(); + root.addProperty("msg", ExceptionHelper.getExceptionMessage(e)); + String json = new Gson().toJson(root); + return Response.status(Status.UNAUTHORIZED).entity(json).build(); } - NewCookie cookie = new NewCookie(StrolchRestfulConstants.STROLCH_AUTHORIZATION, certificate.getAuthToken(), "/", - null, "Authorization header", (int) TimeUnit.DAYS.toSeconds(1), secureCookie); - - return Response.ok().entity(jsonObject.toString())// - .header(HttpHeaders.AUTHORIZATION, certificate.getAuthToken()).cookie(cookie).build(); } }