Skip to content

Provide more appropriate error message for invalid API key #6771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-adam merged 2 commits into from
Jun 19, 2025
Merged

Provide more appropriate error message for invalid API key #6771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
289764e
Provide more appropriate error message for invalid API keys
labkey-adam Jun 18, 2025
a2059ad
Use constant
labkey-adam Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions api/src/org/labkey/api/security/AuthenticationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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));
Expand Down
26 changes: 17 additions & 9 deletions api/src/org/labkey/api/security/AuthenticationProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand All @@ -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()
Expand All @@ -492,6 +495,11 @@ public String getMessage()
{
return email.getEmailAddress();
}

public @Nullable AuthenticationStatus getAssociatedStatus()
{
return _associatedStatus;
}
}

enum ReportType
Expand Down
7 changes: 4 additions & 3 deletions core/src/org/labkey/core/login/LoginController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
{
Expand All @@ -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());
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions core/src/org/labkey/core/user/UserController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ public boolean isActivate()
return activate;
}

@SuppressWarnings("unused")
public void setActivate(boolean activate)
{
this.activate = activate;
Expand All @@ -638,6 +639,7 @@ public boolean isDelete()
return delete;
}

@SuppressWarnings("unused")
public void setDelete(boolean delete)
{
this.delete = delete;
Expand Down