Provide more appropriate error message for invalid API key (#6771)

Author: labkey-adam

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

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

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

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;