From 5f211dbac2a1fd54f4280bf0798040a3abc0832e Mon Sep 17 00:00:00 2001 From: Marie Daly Date: Wed, 22 Jul 2026 13:58:35 +0100 Subject: [PATCH] Closes #50903, added length check for username Signed-off-by: Marie Daly --- .../AbstractUsernameFormAuthenticator.java | 8 ++ .../directgrant/ValidateUsername.java | 17 +++- .../resetcred/ResetCredentialChooseUser.java | 10 +++ .../browser/OrganizationAuthenticator.java | 10 +++ .../services/validation/Validation.java | 2 + .../org/keycloak/tests/forms/LoginTest.java | 37 ++++++++ .../tests/forms/ResetPasswordTest.java | 20 +++++ .../oauth/DirectGrantInputValidationTest.java | 86 +++++++++++++++++++ .../OID4VCredentialOfferPreAuthTest.java | 1 - .../OrganizationAuthenticationTest.java | 13 +++ 10 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 tests/base/src/test/java/org/keycloak/tests/oauth/DirectGrantInputValidationTest.java diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index 2050169ef2bb..1a20a45df9f1 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -40,6 +40,7 @@ import static org.keycloak.services.validation.Validation.FIELD_PASSWORD; import static org.keycloak.services.validation.Validation.FIELD_USERNAME; +import static org.keycloak.services.validation.Validation.MAX_USERNAME_LENGTH; /** * @author Bill Burke @@ -173,6 +174,13 @@ private UserModel getUserFromForm(AuthenticationFlowContext context, Multivalued // remove leading and trailing whitespace username = username.trim(); + if (username.length() > MAX_USERNAME_LENGTH) { + context.getEvent().error(Errors.USER_NOT_FOUND); + Response challengeResponse = challenge(context, getDefaultChallengeMessage(context), FIELD_USERNAME); + context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); + return null; + } + context.getEvent().detail(Details.USERNAME, username); context.getAuthenticationSession().setAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME, username); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java index 510d94e6060e..b5037e95cbc7 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java @@ -42,6 +42,7 @@ import org.keycloak.services.managers.AuthenticationManager; import static org.keycloak.authentication.authenticators.util.AuthenticatorUtils.getDisabledByBruteForceEventError; +import static org.keycloak.services.validation.Validation.MAX_USERNAME_LENGTH; /** * @author Bill Burke @@ -54,12 +55,26 @@ public class ValidateUsername extends AbstractDirectGrantAuthenticator { @Override public void authenticate(AuthenticationFlowContext context) { String username = retrieveUsername(context); - if (username == null) { + + if (username != null) { + username = username.trim(); + } + + if (username == null || username.isEmpty()) { context.getEvent().error(Errors.USER_NOT_FOUND); Response challengeResponse = errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_request", "Missing parameter: username"); context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse); return; } + + if (username.length() > MAX_USERNAME_LENGTH) { + AuthenticatorUtils.dummyHash(context); + context.getEvent().error(Errors.USER_NOT_FOUND); + Response challengeResponse = errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_request", "Invalid user credentials"); + context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse); + return; + } + context.getEvent().detail(Details.USERNAME, username); context.getAuthenticationSession().setAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME, username); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialChooseUser.java b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialChooseUser.java index 2a81f9444247..ae059f52660b 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialChooseUser.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialChooseUser.java @@ -46,6 +46,8 @@ import org.jboss.logging.Logger; +import static org.keycloak.services.validation.Validation.MAX_USERNAME_LENGTH; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -110,6 +112,14 @@ public void action(AuthenticationFlowContext context) { } username = username.trim(); + if (username.length() > MAX_USERNAME_LENGTH) { + context.getEvent().error(Errors.USER_NOT_FOUND); + Response challengeResponse = context.form() + .addError(new FormMessage(Validation.FIELD_USERNAME, Messages.INVALID_USER)) + .createPasswordReset(); + context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); + return; + } RealmModel realm = context.getRealm(); UserModel user = context.getSession().users().getUserByUsername(realm, username); diff --git a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java index 15d116165494..d73547551fee 100644 --- a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java +++ b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java @@ -70,6 +70,7 @@ import static org.keycloak.organization.utils.Organizations.getMatchingDomain; import static org.keycloak.organization.utils.Organizations.isEnabledAndOrganizationsPresent; import static org.keycloak.organization.utils.Organizations.resolveHomeBroker; +import static org.keycloak.services.validation.Validation.MAX_USERNAME_LENGTH; import static org.keycloak.utils.StringUtil.isBlank; public class OrganizationAuthenticator extends IdentityProviderAuthenticator { @@ -139,7 +140,16 @@ public void action(AuthenticationFlowContext context) { }); return; } + // remove leading and trailing whitespace + username = username.trim(); + if (username.length() > MAX_USERNAME_LENGTH) { + initialChallenge(context, form -> { + form.addError(new FormMessage(UserModel.USERNAME, Messages.INVALID_USERNAME)); + return form.createLoginUsername(); + }); + return; + } action(context, username); } diff --git a/services/src/main/java/org/keycloak/services/validation/Validation.java b/services/src/main/java/org/keycloak/services/validation/Validation.java index 527c0544076d..1e91b2b6ad88 100755 --- a/services/src/main/java/org/keycloak/services/validation/Validation.java +++ b/services/src/main/java/org/keycloak/services/validation/Validation.java @@ -33,6 +33,8 @@ public class Validation { public static final String FIELD_USERNAME = "username"; public static final String FIELD_OTP_CODE = "totp"; public static final String FIELD_OTP_LABEL = "userLabel"; + public static final int MAX_USERNAME_LENGTH = 255; // USER_ENTITY table NVARCHAR(255) + private static final Pattern USERNAME_PATTERN = Pattern.compile("^[\\p{IsLatin}|\\p{IsCommon}]+$"); diff --git a/tests/base/src/test/java/org/keycloak/tests/forms/LoginTest.java b/tests/base/src/test/java/org/keycloak/tests/forms/LoginTest.java index 85e66a692a19..2e04068e08a5 100644 --- a/tests/base/src/test/java/org/keycloak/tests/forms/LoginTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/forms/LoginTest.java @@ -40,6 +40,7 @@ import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.services.managers.AuthenticationSessionManager; +import org.keycloak.services.validation.Validation; import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.testframework.annotations.InjectEvents; import org.keycloak.testframework.annotations.InjectHttpClient; @@ -1147,6 +1148,42 @@ public void testUserDisabledDuringRequiredAction() { assertThat(errorPage.getError(), containsString("Account is disabled")); } + @Test + public void loginMaxLengthUsername() { + oauth.openLoginForm(); + loginPage.fillLogin("a".repeat(Validation.MAX_USERNAME_LENGTH + 1), "invalid"); + loginPage.submit(); + + loginPage.assertCurrent(); + + assertEquals("Invalid username or password.", loginPage.getUsernameInputError()); + + EventAssertion.assertError(events.poll()) + .type(EventType.LOGIN_ERROR) + .userId(null) + .sessionId(null) + .error(Errors.USER_NOT_FOUND) + .withoutDetails(Details.USERNAME); + } + + @Test + public void loginWhitespaceOnlyUsername() { + oauth.openLoginForm(); + loginPage.fillLogin(" ", "invalid"); + loginPage.submit(); + + loginPage.assertCurrent(); + + assertEquals("Invalid username or password.", loginPage.getUsernameInputError()); + + EventAssertion.assertError(events.poll()) + .type(EventType.LOGIN_ERROR) + .userId(null) + .sessionId(null) + .error(Errors.USER_NOT_FOUND) + .withoutDetails(Details.USERNAME); + } + static class DynamicScopeServerConfig implements KeycloakServerConfig { @Override public KeycloakServerConfigBuilder configure(KeycloakServerConfigBuilder config) { diff --git a/tests/base/src/test/java/org/keycloak/tests/forms/ResetPasswordTest.java b/tests/base/src/test/java/org/keycloak/tests/forms/ResetPasswordTest.java index 9572aa33cefb..cea9392a306c 100644 --- a/tests/base/src/test/java/org/keycloak/tests/forms/ResetPasswordTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/forms/ResetPasswordTest.java @@ -6,11 +6,13 @@ import org.keycloak.broker.oidc.OIDCIdentityProviderFactory; import org.keycloak.events.Details; +import org.keycloak.events.Errors; import org.keycloak.events.EventType; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.credential.PasswordCredentialModel; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.services.validation.Validation; import org.keycloak.testframework.annotations.InjectEvents; import org.keycloak.testframework.annotations.InjectRealm; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; @@ -208,6 +210,24 @@ public void resetPasswordEmailLinkWorksAfterNavigatingBackToLoginPage() throws I assertTrue(driver.page().getPageSource().contains("Happy days")); } + @Test + public void resetPasswordMaxLengthUsername() { + + oauth.openLoginForm(); + loginPage.assertCurrent(); + loginPage.resetPassword(); + resetPasswordPage.assertCurrent(); + resetPasswordPage.changePassword("a".repeat(Validation.MAX_USERNAME_LENGTH + 1)); + resetPasswordPage.assertCurrent(); + + EventAssertion.assertError(events.poll()) + .type(EventType.RESET_PASSWORD_ERROR) + .userId(null) + .sessionId(null) + .error(Errors.USER_NOT_FOUND) + .withoutDetails(Details.USERNAME); + } + static class ConsumerRealmConfig implements RealmConfig { @Override public RealmBuilder configure(RealmBuilder realm) { diff --git a/tests/base/src/test/java/org/keycloak/tests/oauth/DirectGrantInputValidationTest.java b/tests/base/src/test/java/org/keycloak/tests/oauth/DirectGrantInputValidationTest.java new file mode 100644 index 000000000000..7c91774cd963 --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/oauth/DirectGrantInputValidationTest.java @@ -0,0 +1,86 @@ +package org.keycloak.tests.oauth; + +import org.keycloak.services.validation.Validation; +import org.keycloak.testframework.annotations.InjectRealm; +import org.keycloak.testframework.annotations.InjectUser; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.oauth.OAuthClient; +import org.keycloak.testframework.oauth.annotations.InjectOAuthClient; +import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.realm.ManagedUser; +import org.keycloak.testframework.realm.UserBuilder; +import org.keycloak.testframework.realm.UserConfig; +import org.keycloak.testsuite.util.oauth.AccessTokenResponse; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +@KeycloakIntegrationTest +public class DirectGrantInputValidationTest { + + @InjectRealm + ManagedRealm realm; + + @InjectOAuthClient + OAuthClient oauth; + + @InjectUser(config = DirectGrantUserConfig.class) + ManagedUser user; + + @Test + public void directGrantRejectsUsernameLongerThanMaxLength() { + AccessTokenResponse response = doGrant("a".repeat(Validation.MAX_USERNAME_LENGTH + 1), "password"); + assertEquals(401, response.getStatusCode()); + assertEquals("invalid_request", response.getError()); + assertNull(response.getAccessToken()); + } + + @Test + public void directGrantAcceptsUsernameLengthAtMaxLength() { + // A username exactly at the limit passes the length check; the user doesn't exist so we get + // the standard user-not-found error rather than the over-length rejection. + AccessTokenResponse response = doGrant("a".repeat(Validation.MAX_USERNAME_LENGTH), "password"); + assertEquals(400, response.getStatusCode()); + assertEquals("invalid_grant", response.getError()); + } + + @Test + public void directGrantRejectsWhitespaceOnlyUsernameAsMissing() { + AccessTokenResponse response = doGrant(" ", "password"); + assertEquals(401, response.getStatusCode()); + assertEquals("invalid_request", response.getError()); + assertEquals("Missing parameter: username", response.getErrorDescription()); + assertNull(response.getAccessToken()); + } + + @Test + public void directGrantAcceptsUsernameWithSurroundingWhitespace() { + AccessTokenResponse response = doGrant(" validuser ", "password"); + assertEquals(200, response.getStatusCode()); + assertNull(response.getError()); + } + + @Test + public void directGrantAcceptsUsernameHappyPath() { + AccessTokenResponse response = doGrant("validuser", "password"); + assertEquals(200, response.getStatusCode()); + assertNull(response.getError()); + } + + private AccessTokenResponse doGrant(String username, String password) { + return oauth.doPasswordGrantRequest(username, password); + } + + public static class DirectGrantUserConfig implements UserConfig { + @Override + public UserBuilder configure(UserBuilder user) { + return user.username("validuser") + .password("password") + .email("validuser@localhost") + .name("Valid", "User") + .emailVerified(true); + } + } +} diff --git a/tests/base/src/test/java/org/keycloak/tests/oid4vc/preauth/OID4VCredentialOfferPreAuthTest.java b/tests/base/src/test/java/org/keycloak/tests/oid4vc/preauth/OID4VCredentialOfferPreAuthTest.java index b71be8a0186e..44c92a83aef5 100644 --- a/tests/base/src/test/java/org/keycloak/tests/oid4vc/preauth/OID4VCredentialOfferPreAuthTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/oid4vc/preauth/OID4VCredentialOfferPreAuthTest.java @@ -35,7 +35,6 @@ import static org.keycloak.constants.OID4VCIConstants.CREDENTIAL_OFFER_CREATE; import static org.keycloak.protocol.oid4vc.issuance.OID4VCIssuerEndpoint.CREDENTIAL_OFFER_LIFESPAN_REALM_ATTRIBUTE_KEY; import static org.keycloak.protocol.oid4vc.issuance.OID4VCIssuerEndpoint.DEFAULT_CREDENTIAL_OFFER_LIFESPAN_S; - import static org.keycloak.tests.oid4vc.CredentialOfferStateUtils.getCredentialOfferStateRecord; import static org.junit.jupiter.api.Assertions.assertEquals; diff --git a/tests/base/src/test/java/org/keycloak/tests/organization/authentication/OrganizationAuthenticationTest.java b/tests/base/src/test/java/org/keycloak/tests/organization/authentication/OrganizationAuthenticationTest.java index 368fe4ed6e93..1bc2a3141780 100644 --- a/tests/base/src/test/java/org/keycloak/tests/organization/authentication/OrganizationAuthenticationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/organization/authentication/OrganizationAuthenticationTest.java @@ -37,6 +37,7 @@ import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.services.validation.Validation; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.oauth.OAuthClient; import org.keycloak.testframework.oauth.annotations.InjectOAuthClient; @@ -144,6 +145,18 @@ public void testEmptyUserNameValidation() { assertEquals("Invalid username.", loginUsernamePage.getUsernameInputError()); } + @Test + public void testMaxLengthUserNameValidation() { + createOrganization(); + + oauth.openLoginForm(); + assertFalse(loginPage.isPasswordInputPresent()); + loginUsernamePage.fillLoginWithUsernameOnly("a".repeat(Validation.MAX_USERNAME_LENGTH + 1)); + loginUsernamePage.submit(); + + assertEquals("Invalid username.", loginUsernamePage.getUsernameInputError()); + } + @Test public void testDefaultAuthenticationMechanismIfNotOrganizationMember() { realm.admin().organizations().get(createOrganization().getId());