diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java index d752bbe79e9..030a6ed10ce 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java @@ -459,7 +459,8 @@ public class DefaultAttributes extends HashMap> implements Stream valuesStream = Optional.ofNullable(values).orElse(EMPTY_VALUE).stream().filter(Objects::nonNull); - if (UserModel.USERNAME.equals(name) || UserModel.EMAIL.equals(name)) { + // do not normalize the username if a federated user because we need to respect the format from the external identity store + if ((UserModel.USERNAME.equals(name) && isLocalUser()) || UserModel.EMAIL.equals(name)) { valuesStream = valuesStream.map(KeycloakModelUtils::toLowerCaseSafe); } @@ -569,4 +570,8 @@ public class DefaultAttributes extends HashMap> implements } }; } + + private boolean isLocalUser() { + return user == null || user.isLocal(); + } } diff --git a/server-spi/src/main/java/org/keycloak/models/UserModel.java b/server-spi/src/main/java/org/keycloak/models/UserModel.java index e780c2fad5e..18f3d21ea23 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserModel.java @@ -17,7 +17,11 @@ package org.keycloak.models; +import static org.keycloak.utils.StringUtil.isBlank; + import org.keycloak.provider.ProviderEvent; +import org.keycloak.storage.StorageId; +import org.keycloak.utils.StringUtil; import java.util.Comparator; import java.util.List; @@ -207,6 +211,16 @@ public interface UserModel extends RoleMapperModel { String getServiceAccountClientLink(); void setServiceAccountClientLink(String clientInternalId); + /** + * Indicates if this {@link UserModel} maps to a local account or an account + * federated from an external user storage. + * + * @return {@code true} if a local account. Otherwise, {@code false}. + */ + default boolean isLocal() { + return isBlank(getFederationLink()) && StorageId.isLocalStorage(getId()); + } + /** * Instance of a user credential manager to validate and update the credentials of this user. */ diff --git a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java index a753ccb5c8e..7a61218fc71 100644 --- a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java +++ b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java @@ -22,6 +22,7 @@ import static org.keycloak.validate.BuiltinValidators.notBlankValidator; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -34,7 +35,7 @@ import org.keycloak.validate.ValidatorConfig; /** * A validator that fails when the attribute is marked as read only and its value has changed. - * + * * @author Pedro Igor */ public class ImmutableAttributeValidator implements SimpleValidator { @@ -58,7 +59,14 @@ public class ImmutableAttributeValidator implements SimpleValidator { return context; } - List currentValue = user.getAttributeStream(inputHint).filter(Objects::nonNull).collect(Collectors.toList()); + Stream rawValues = user.getAttributeStream(inputHint).filter(Objects::nonNull); + + // force usernames to lower-case to avoid validation errors if the external storage is using a different format + if (user.isLocal() && UserModel.USERNAME.equals(inputHint)) { + rawValues = rawValues.map(String::toLowerCase); + } + + List currentValue = rawValues.collect(Collectors.toList()); List values = (List) input; if (!collectionEquals(currentValue, values) && isReadOnly(attributeContext)) { diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/UserPropertyFileStorage.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/UserPropertyFileStorage.java index 2279e5c0b99..59a86989b01 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/UserPropertyFileStorage.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/UserPropertyFileStorage.java @@ -61,7 +61,7 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP protected ComponentModel model; protected KeycloakSession session; protected boolean federatedStorageEnabled; - + public static Map> storageCalls = new HashMap<>(); public static class UserPropertyFileStorageCall implements Serializable { @@ -87,7 +87,7 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP return max; } } - + public UserPropertyFileStorage(KeycloakSession session, ComponentModel model, Properties userPasswords) { this.session = session; this.model = model; @@ -109,14 +109,17 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP @Override public int getUsersCount(RealmModel realm, Map params) { addCall(COUNT_SEARCH_METHOD); - + return (int) searchForUser(realm, params.get(UserModel.SEARCH), null, null, username -> username.contains(params.get(UserModel.SEARCH))).count(); } @Override public UserModel getUserById(RealmModel realm, String id) { StorageId storageId = new StorageId(id); - final String username = storageId.getExternalId(); + String username = storageId.getExternalId(); + if ("uppercase".equalsIgnoreCase(username)) { + username = username.toLowerCase(); + } if (!userPasswords.containsKey(username)) return null; return createUser(realm, username); @@ -127,6 +130,9 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP return new AbstractUserAdapterFederatedStorage.Streams(session, realm, model) { @Override public String getUsername() { + if ("uppercase".equalsIgnoreCase(username)) { + return username.toUpperCase(); + } return username; } @@ -139,6 +145,9 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP return new AbstractUserAdapter.Streams(session, realm, model) { @Override public String getUsername() { + if ("uppercase".equalsIgnoreCase(username)) { + return username.toUpperCase(); + } return username; } @@ -191,7 +200,11 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP public boolean isValid(RealmModel realm, UserModel user, CredentialInput input) { if (!(input instanceof UserCredentialModel)) return false; if (input.getType().equals(PasswordCredentialModel.TYPE)) { - String pw = (String)userPasswords.get(user.getUsername()); + String username = user.getUsername(); + if ("uppercase".equalsIgnoreCase(username)) { + username = user.getUsername().toLowerCase(); + } + String pw = (String)userPasswords.get(username); return pw != null && pw.equals(input.getChallengeResponse()); } else { return false; diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java index e9a62b15841..79e2475fba6 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java @@ -93,6 +93,12 @@ public class LDAPTestUtils { return addLDAPUser(ldapProvider, realm, username, firstName, lastName, email, street, new MultivaluedHashMap<>(), postalCode); } + public static LDAPObject addLDAPUser(LDAPStorageProvider ldapProvider, RealmModel realm, final String username, + final String firstName, final String lastName, final String email, + final MultivaluedHashMap otherAttrs) { + return addLDAPUser(ldapProvider, realm, username, firstName, lastName, email, null, otherAttrs); + } + public static LDAPObject addLDAPUser(LDAPStorageProvider ldapProvider, RealmModel realm, final String username, final String firstName, final String lastName, final String email, final String street, final MultivaluedHashMap otherAttrs, final String... postalCode) { @@ -355,7 +361,7 @@ public class LDAPTestUtils { } } } - + public static void removeLDAPUserByUsername(LDAPStorageProvider ldapProvider, RealmModel realm, LDAPConfig config, String username) { LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore(); try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm)) { @@ -369,7 +375,7 @@ public class LDAPTestUtils { } } } - + public static void removeAllLDAPRoles(KeycloakSession session, RealmModel appRealm, ComponentModel ldapModel, String mapperName) { ComponentModel mapperModel = getSubcomponentByName(appRealm, ldapModel, mapperName); LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java index 5938be3fa4d..b7b1149077c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java @@ -20,6 +20,7 @@ package org.keycloak.testsuite.federation.ldap; import java.io.IOException; +import java.util.List; import java.util.Set; import org.jboss.arquillian.graphene.page.Page; @@ -29,6 +30,7 @@ import org.junit.FixMethodOrder; import org.junit.Test; import org.junit.runners.MethodSorters; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.component.ComponentModel; import org.keycloak.component.PrioritizedComponentModel; import org.keycloak.models.LDAPConstants; @@ -43,6 +45,8 @@ import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.idm.model.LDAPObject; +import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper; +import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapperFactory; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.util.LDAPRule; @@ -298,6 +302,51 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { } } + @Test + public void testUsernameRespectFormatFromExternalStore() { + String upperCaseUsername = "JOHNKEYCLOAK3"; + testingClient.server().run(session -> { + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); + RealmModel appRealm = ctx.getRealm(); + + ctx.getLdapModel().getConfig().put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, List.of(LDAPConstants.GIVENNAME)); + ctx.getLdapModel().getConfig().put(LDAPConstants.RDN_LDAP_ATTRIBUTE, List.of(LDAPConstants.GIVENNAME)); + + ComponentModel ldapComponentMapper = LDAPTestUtils.addUserAttributeMapper(appRealm, ctx.getLdapModel(), "givename-mapper", "username", LDAPConstants.GIVENNAME); + ldapComponentMapper.put(UserAttributeLDAPStorageMapper.ALWAYS_READ_VALUE_FROM_LDAP, true); + appRealm.updateComponent(ldapComponentMapper); + + appRealm.removeComponent(appRealm.getComponentsStream(ctx.getLdapModel().getId()) + .filter(mapper -> UserAttributeLDAPStorageMapperFactory.PROVIDER_ID.equals(mapper.getProviderId())) + .filter((mapper) -> mapper.getName().equals(UserModel.USERNAME)) + .findAny().orElse(null)); + + appRealm.updateComponent(ctx.getLdapModel()); + + MultivaluedHashMap otherAttrs = new MultivaluedHashMap<>(); + otherAttrs.put(LDAPConstants.GIVENNAME, List.of(upperCaseUsername)); + + LDAPObject john3 = LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, upperCaseUsername, "John", "Doe", "john3@email.org", otherAttrs); + LDAPTestUtils.updateLDAPPassword(ctx.getLdapProvider(), john3, "Password1"); + }); + + UserResource johnResource = ApiUtil.findUserByUsernameId(testRealm(), upperCaseUsername); + UserRepresentation john = johnResource.toRepresentation(true); + Assert.assertEquals(upperCaseUsername, john.getUsername()); + + johnResource = ApiUtil.findUserByUsernameId(testRealm(), upperCaseUsername.toLowerCase()); + john = johnResource.toRepresentation(true); + Assert.assertEquals(upperCaseUsername, john.getUsername()); + + loginPage.open(); + loginPage.login(upperCaseUsername, "Password1"); + appPage.assertCurrent(); + testRealm().users().get(john.getId()).logout(); + loginPage.open(); + loginPage.login(upperCaseUsername.toLowerCase(), "Password1"); + appPage.assertCurrent(); + } + private void setLDAPReadOnly() { testingClient.server().run(session -> { LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java index 3e618420752..0f5b576bf38 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java @@ -85,6 +85,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.hamcrest.MatcherAssert.assertThat; @@ -442,7 +443,7 @@ public class UserStorageTest extends AbstractAuthTest { public void testQuery() { Set queried = new HashSet<>(); int first = 0; - while (queried.size() < 8) { + while (queried.size() < 10) { List results = testRealmResource().users().search("", first, 3); log.debugf("first=%s, results: %s", first, results.size()); if (results.isEmpty()) { @@ -456,7 +457,7 @@ public class UserStorageTest extends AbstractAuthTest { usernames.add(user.getUsername()); log.info(user.getUsername()); } - Assert.assertEquals(9, queried.size()); + Assert.assertEquals(10, queried.size()); Assert.assertTrue(usernames.contains("thor")); Assert.assertTrue(usernames.contains("zeus")); Assert.assertTrue(usernames.contains("apollo")); @@ -466,6 +467,7 @@ public class UserStorageTest extends AbstractAuthTest { Assert.assertTrue(usernames.contains("rob")); Assert.assertTrue(usernames.contains("jules")); Assert.assertTrue(usernames.contains("danny")); + Assert.assertTrue(usernames.contains("UPPERCASE")); // test searchForUser List users = testRealmResource().users().search("tbrady", 0, -1); @@ -544,7 +546,7 @@ public class UserStorageTest extends AbstractAuthTest { UserModel userModel = session.users().getUserByUsername(realm, "thor"); userModel.setSingleAttribute("weapon", longValue); - assertThat(session.users().searchForUserByUserAttributeStream(realm, "weapon", longValue).map(UserModel::getUsername).collect(Collectors.toList()), + assertThat(session.users().searchForUserByUserAttributeStream(realm, "weapon", longValue).map(UserModel::getUsername).collect(Collectors.toList()), containsInAnyOrder("thor")); // searching here is always case sensitive @@ -779,7 +781,7 @@ public class UserStorageTest extends AbstractAuthTest { }); setTimeOffset(1/2 * 60 * 60); // 1/2 hour in future - + testingClient.server().run(session -> { RealmModel realm = session.realms().getRealmByName("test"); UserModel user = session.users().getUserByUsername(realm, "thor"); @@ -1158,6 +1160,13 @@ public class UserStorageTest extends AbstractAuthTest { }); } + @Test + public void testRespectUsernameFormatFromStorage() { + loginSuccessAndLogout("uppercase", "uppercase"); + UserResource user = ApiUtil.findUserByUsernameId(testRealmResource(), "uppercase"); + UserRepresentation rep = user.toRepresentation(); + assertEquals("uppercase".toUpperCase(), rep.getUsername()); + } private void assertOrder(List creds, String... expectedIds) { org.keycloak.testsuite.Assert.assertEquals(expectedIds.length, creds.size()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/storage-test/read-only-user-password.properties b/testsuite/integration-arquillian/tests/base/src/test/resources/storage-test/read-only-user-password.properties index c0b76abe79c..40807e98ae4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/storage-test/read-only-user-password.properties +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/storage-test/read-only-user-password.properties @@ -2,3 +2,4 @@ tbrady=goat rob=pw jules=pw danny=pw +uppercase=uppercase \ No newline at end of file diff --git a/testsuite/model/src/test/resources/file-storage-provider/read-only-user-password.properties b/testsuite/model/src/test/resources/file-storage-provider/read-only-user-password.properties index c0b76abe79c..665b4a2e0a0 100644 --- a/testsuite/model/src/test/resources/file-storage-provider/read-only-user-password.properties +++ b/testsuite/model/src/test/resources/file-storage-provider/read-only-user-password.properties @@ -1,4 +1,4 @@ tbrady=goat rob=pw jules=pw -danny=pw +danny=pw \ No newline at end of file