From a32b58d3374cc40ef164b00b93dd2dc0ebe406c2 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 27 Nov 2023 11:38:14 +0100 Subject: [PATCH] Escape ldap id when using normal attribute syntax (#25) (#25036) Closes https://github.com/keycloak/security/issues/46 Co-authored-by: Ricardo Martin --- .../idm/store/ldap/LDAPOperationManager.java | 3 +- .../AbstractUsernameFormAuthenticator.java | 2 +- .../federation/ldap/LDAPSpecialCharsTest.java | 42 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java index 32503b245a..df039f7a16 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java @@ -24,6 +24,7 @@ import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.idm.model.LDAPDn; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; import org.keycloak.storage.ldap.idm.store.ldap.extended.PasswordModifyRequest; import org.keycloak.storage.ldap.mappers.LDAPOperationDecorator; @@ -386,7 +387,7 @@ public class LDAPOperationManager { ).append(LDAPUtil.convertGUIDToEdirectoryHexString(id)).append(")"); } else { filter.append("(objectClass=*)(").append(getUuidAttributeName()).append(LDAPConstants.EQUAL) - .append(id).append(")"); + .append(EscapeStrategy.DEFAULT.escape(id)).append(")"); } if (config.getCustomUserSearchFilter() != null) { 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 13d218efe9..481df3f3b1 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 @@ -170,7 +170,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth private UserModel getUserFromForm(AuthenticationFlowContext context, MultivaluedMap inputData) { String username = inputData.getFirst(AuthenticationManager.FORM_USERNAME); - if (username == null) { + if (username == null || username.isEmpty()) { context.getEvent().error(Errors.USER_NOT_FOUND); Response challengeResponse = challenge(context, getDefaultChallengeMessage(context), FIELD_USERNAME); context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java index 1f19f03de5..d508ec3fa1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java @@ -31,6 +31,7 @@ import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.mappers.membership.LDAPGroupMapperMode; @@ -198,4 +199,45 @@ public class LDAPSpecialCharsTest extends AbstractLDAPTest { }); } + @Test + public void test04_loginWithSpecialCharacterUsingSameUUIDThanUsernameAttribute() { + // remove users from the ldap to use the new UUID attribute + adminClient.realm(TEST_REALM_NAME).userStorage().removeImportedUsers(ldapModelId); + + // change the UUID attribute to be the username attribute + String origUuidAttrName = testingClient.server().fetch(session -> { + LDAPTestContext ctx = LDAPTestContext.init(session); + + String uidAttrName = ctx.getLdapProvider().getLdapIdentityStore().getConfig().getUsernameLdapAttribute(); + String origUuidAttrNamee = ctx.getLdapModel().get(LDAPConstants.UUID_LDAP_ATTRIBUTE); + ctx.getLdapModel().put(LDAPConstants.UUID_LDAP_ATTRIBUTE, uidAttrName); + ctx.getRealm().updateComponent(ctx.getLdapModel()); + + return origUuidAttrNamee; + }, String.class); + + try { + // assert the user is found and UUID is the name + List users = adminClient.realm(TEST_REALM_NAME).users().search("jamees,key*cložak)ppp", true); + Assert.assertEquals("User not found", 1, users.size()); + UserRepresentation jamees = users.iterator().next(); + Assert.assertEquals("Incorrect user", "jamees,key*cložak)ppp", jamees.getUsername()); + Assert.assertEquals("Incorrect UUID attribute", "jamees,key*cložak)ppp", jamees.firstAttribute(LDAPConstants.LDAP_ID)); + + // Fail login with wildcard + loginPage.open(); + loginPage.login("jamees*", "Password1"); + Assert.assertEquals("Invalid username or password.", loginPage.getInputError()); + + // Success login as username exactly match + loginPage.login("jamees,key*cložak)ppp", "Password1"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + } finally { + // Revert config changes to be back to previous UUID attribute + ComponentRepresentation ldapRep = testRealm().components().component(ldapModelId).toRepresentation(); + ldapRep.getConfig().putSingle(LDAPConstants.UUID_LDAP_ATTRIBUTE, origUuidAttrName); + testRealm().components().component(ldapModelId).update(ldapRep); + } + } }