Respect the username value format when processing federated users
Closes #31240 Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
parent
b20123dcdc
commit
87c279d645
9 changed files with 120 additions and 15 deletions
|
@ -459,7 +459,8 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
|
|||
|
||||
Stream<String> 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<String, List<String>> implements
|
|||
}
|
||||
};
|
||||
}
|
||||
|
||||
private boolean isLocalUser() {
|
||||
return user == null || user.isLocal();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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.
|
||||
*/
|
||||
|
|
|
@ -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 <a href="mailto:psilva@redhat.com">Pedro Igor</a>
|
||||
*/
|
||||
public class ImmutableAttributeValidator implements SimpleValidator {
|
||||
|
@ -58,7 +59,14 @@ public class ImmutableAttributeValidator implements SimpleValidator {
|
|||
return context;
|
||||
}
|
||||
|
||||
List<String> currentValue = user.getAttributeStream(inputHint).filter(Objects::nonNull).collect(Collectors.toList());
|
||||
Stream<String> 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<String> currentValue = rawValues.collect(Collectors.toList());
|
||||
List<String> values = (List<String>) input;
|
||||
|
||||
if (!collectionEquals(currentValue, values) && isReadOnly(attributeContext)) {
|
||||
|
|
|
@ -61,7 +61,7 @@ public class UserPropertyFileStorage implements UserLookupProvider, UserStorageP
|
|||
protected ComponentModel model;
|
||||
protected KeycloakSession session;
|
||||
protected boolean federatedStorageEnabled;
|
||||
|
||||
|
||||
public static Map<String, List<UserPropertyFileStorageCall>> 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<String, String> 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;
|
||||
|
|
|
@ -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<String, String> 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<String, String> 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);
|
||||
|
|
|
@ -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<String, String> 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");
|
||||
|
|
|
@ -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<UserRepresentation> queried = new HashSet<>();
|
||||
int first = 0;
|
||||
while (queried.size() < 8) {
|
||||
while (queried.size() < 10) {
|
||||
List<UserRepresentation> 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<UserRepresentation> 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<CredentialModel> creds, String... expectedIds) {
|
||||
org.keycloak.testsuite.Assert.assertEquals(expectedIds.length, creds.size());
|
||||
|
|
|
@ -2,3 +2,4 @@ tbrady=goat
|
|||
rob=pw
|
||||
jules=pw
|
||||
danny=pw
|
||||
uppercase=uppercase
|
|
@ -1,4 +1,4 @@
|
|||
tbrady=goat
|
||||
rob=pw
|
||||
jules=pw
|
||||
danny=pw
|
||||
danny=pw
|
Loading…
Reference in a new issue