diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 2d6ee43df11..2ba529491f6 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -119,8 +119,6 @@ import java.util.stream.Collectors; import static org.labkey.api.action.SpringActionController.ERROR_MSG; -import static org.labkey.api.security.AuthenticationProvider.FailureReason.complexity; -import static org.labkey.api.security.AuthenticationProvider.FailureReason.expired; public class AuthenticationManager { @@ -668,6 +666,14 @@ public void addUserErrorMessage(BindException errors, PrimaryAuthenticationResul throw new IllegalStateException("Shouldn't be adding an error message in success case"); } }, + BadApiKey + { + @Override + public void addUserErrorMessage(BindException errors, PrimaryAuthenticationResult result, @Nullable String fullEmailAddress, @Nullable URLHelper returnUrl, DisplayLocation location) + { + errors.reject(ERROR_MSG, "The API key you provided is invalid."); + } + }, BadCredentials { @Override @@ -1031,24 +1037,22 @@ else if (null != emailAddress) { // Funny audit case -- user doesn't exist, so there's no user to associate with the event. Use guest. addAuditEvent(User.guest, request, "Unknown user" + message); - _log.warn("Unknown user" + message); + _log.warn("Unknown user{}", message); } // For now, redirectURL is only checked in the failure case, see #19778 for some history on redirect handling ActionURL redirectURL = firstFailure.getRedirectURL(); - // if labkey db authentication determines password has expired or does not meet requirements then return - // result with appropriate status. Note that redirectUrl might be null (e.g., API case). + // If labkey db authentication determines password has expired or does not meet requirements then return + // result with appropriate status. Same with a bad API key. Note that redirectUrl might be null (e.g., API case). FailureReason firstFailureReason = firstFailure.getFailureReason(); - if (firstFailureReason == expired) - { - return new PrimaryAuthenticationResult(redirectURL, AuthenticationStatus.PasswordExpired); - } - else if (firstFailureReason == complexity) + AuthenticationStatus status = firstFailureReason.getAssociatedStatus(); + if (null != status) { - return new PrimaryAuthenticationResult(redirectURL, AuthenticationStatus.Complexity); + return new PrimaryAuthenticationResult(redirectURL, status); } - else if (null != redirectURL) + + if (null != redirectURL) { throw new RedirectException(redirectURL); } @@ -1145,7 +1149,7 @@ private static PrimaryAuthenticationResult _beforeAuthenticate(HttpServletReques long delay = 0; // slow down login attempts when we detect more than 20/minute bad attempts per user, password, or ip address - rl = addrLimiter.get(_toKey(request==null?null:request.getRemoteAddr())); + rl = addrLimiter.get(_toKey(request == null ? null : request.getRemoteAddr())); if (null != rl) delay = Math.max(delay,rl.add(0, false)); rl = pwdLimiter.get(_toKey(pwd)); diff --git a/api/src/org/labkey/api/security/AuthenticationProvider.java b/api/src/org/labkey/api/security/AuthenticationProvider.java index 316eb539174..6d827947235 100644 --- a/api/src/org/labkey/api/security/AuthenticationProvider.java +++ b/api/src/org/labkey/api/security/AuthenticationProvider.java @@ -27,6 +27,7 @@ import org.labkey.api.security.AuthenticationConfiguration.PrimaryAuthenticationConfiguration; import org.labkey.api.security.AuthenticationConfiguration.SSOAuthenticationConfiguration; import org.labkey.api.security.AuthenticationConfiguration.SecondaryAuthenticationConfiguration; +import org.labkey.api.security.AuthenticationManager.AuthenticationStatus; import org.labkey.api.security.AuthenticationManager.AuthenticationValidator; import org.labkey.api.security.ValidEmail.InvalidEmailException; import org.labkey.api.settings.StandardStartupPropertyHandler; @@ -452,14 +453,14 @@ public AuthenticationResponse setRequireSecondary(boolean requireSecondary) // hackers). We try to be as specific as possible. enum FailureReason { - userDoesNotExist(ReportType.onFailure, "user does not exist"), - badPassword(ReportType.onFailure, "incorrect password"), - badCredentials(ReportType.onFailure, "invalid credentials"), // Use for cases where we can't distinguish between userDoesNotExist and badPassword - complexity(ReportType.onFailure, "password does not meet the complexity requirements"), - expired(ReportType.onFailure, "password has expired"), - configurationError(ReportType.always, "configuration problem"), - notApplicable(ReportType.never, "not applicable"), - badApiKey(ReportType.onFailure, "invalid API key") { + userDoesNotExist(ReportType.onFailure, "user does not exist", null), + badPassword(ReportType.onFailure, "incorrect password", null), + badCredentials(ReportType.onFailure, "invalid credentials", null), // Use for cases where we can't distinguish between userDoesNotExist and badPassword + complexity(ReportType.onFailure, "password does not meet the complexity requirements", AuthenticationStatus.Complexity), + expired(ReportType.onFailure, "password has expired", AuthenticationStatus.PasswordExpired), + configurationError(ReportType.always, "configuration problem", null), + notApplicable(ReportType.never, "not applicable", null), + badApiKey(ReportType.onFailure, "invalid API key", AuthenticationStatus.BadApiKey) { @Override public @Nullable String getEmailAddress(ValidEmail email) throws InvalidEmailException { @@ -471,11 +472,13 @@ enum FailureReason private final ReportType _type; private final String _message; + private final @Nullable AuthenticationStatus _associatedStatus; - FailureReason(ReportType type, String message) + FailureReason(ReportType type, String message, @Nullable AuthenticationStatus associatedStatus) { _type = type; _message = message; + _associatedStatus = associatedStatus; } public ReportType getReportType() @@ -492,6 +495,11 @@ public String getMessage() { return email.getEmailAddress(); } + + public @Nullable AuthenticationStatus getAssociatedStatus() + { + return _associatedStatus; + } } enum ReportType diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 410924060ae..4114d563504 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -317,7 +317,8 @@ private static boolean authenticate(LoginForm form, BindException errors, HttpSe try { // Attempt authentication with all active form providers - PrimaryAuthenticationResult result = AuthenticationManager.authenticate(request, form.getEmail(), form.getPassword(), form.getReturnUrlHelper(), true); + String formEmail = form.getEmail(); + PrimaryAuthenticationResult result = AuthenticationManager.authenticate(request, formEmail, form.getPassword(), form.getReturnUrlHelper(), true); AuthenticationStatus status = result.getStatus(); if (Success == status) @@ -328,7 +329,7 @@ private static boolean authenticate(LoginForm form, BindException errors, HttpSe else { // Explicit test for valid email - ValidEmail email = new ValidEmail(form.getEmail()); + ValidEmail email = SecurityManager.API_KEY.equals(formEmail) ? null : new ValidEmail(formEmail); if (status.handleRedirect()) { @@ -337,7 +338,7 @@ private static boolean authenticate(LoginForm form, BindException errors, HttpSe else { // Pass in normalized email address, but only if user provided a full email address - status.addUserErrorMessage(errors, result, form.getEmail().contains("@") ? email.getEmailAddress() : null, form.getReturnUrlHelper()); + status.addUserErrorMessage(errors, result, formEmail.contains("@") ? email.getEmailAddress() : null, form.getReturnUrlHelper()); } } } diff --git a/core/src/org/labkey/core/user/UserController.java b/core/src/org/labkey/core/user/UserController.java index 0c479e1c0f8..49d9ef0d694 100644 --- a/core/src/org/labkey/core/user/UserController.java +++ b/core/src/org/labkey/core/user/UserController.java @@ -628,6 +628,7 @@ public boolean isActivate() return activate; } + @SuppressWarnings("unused") public void setActivate(boolean activate) { this.activate = activate; @@ -638,6 +639,7 @@ public boolean isDelete() return delete; } + @SuppressWarnings("unused") public void setDelete(boolean delete) { this.delete = delete;