libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

KEYCLOAK-14870: Fix bug where user is incorrectly imported

Browse source 
Bug: SerializedBrokeredIdentityContext was changed to mirror
UserModel changes. However, when creating the user in LDAP,
the username must be provided first (everything else can
be handled via attributes).
This commit is contained in:
Martin Idel 2020-07-27 16:34:39 +02:00 committed by Marek Posolda
parent 6806dfa4d3
commit 97400827d2
6 changed files with 59 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java
View file

@ -22,6 +22,7 @@ import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
@ -43,8 +44,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.keycloak.models.ModelException;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@ -192,24 +191,21 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
@Override
public void setSingleAttribute(String name, String value) {
if (UserModel.USERNAME.equals(name)) {
checkDuplicateUsername(userModelAttrName, value, realm, ldapProvider.getSession(), this);
setUsername(value);
} else if (UserModel.EMAIL.equals(name)) {
checkDuplicateEmail(userModelAttrName, value, realm, ldapProvider.getSession(), this);
}
if (setLDAPAttribute(name, value)) {
setEmail(value);
} else if (setLDAPAttribute(name, value)) {
super.setSingleAttribute(name, value);
}
}
@Override
public void setAttribute(String name, List<String> values) {
String valueToSet = (values != null && values.size() > 0) ? values.get(0) : null;
if (UserModel.USERNAME.equals(name)) {
checkDuplicateUsername(userModelAttrName, valueToSet, realm, ldapProvider.getSession(), this);
setUsername((values != null && values.size() > 0) ? values.get(0) : null);
} else if (UserModel.EMAIL.equals(name)) {
checkDuplicateEmail(userModelAttrName, valueToSet, realm, ldapProvider.getSession(), this);
}
if (setLDAPAttribute(name, values)) {
setEmail((values != null && values.size() > 0) ? values.get(0) : null);
} else if (setLDAPAttribute(name, values)) {
super.setAttribute(name, values);
}
}
@ -223,17 +219,19 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
@Override
public void setUsername(String username) {
checkDuplicateUsername(userModelAttrName, username, realm, ldapProvider.getSession(), this);
setLDAPAttribute(UserModel.USERNAME, username);
super.setUsername(username);
String lowercaseUsername = KeycloakModelUtils.toLowerCaseSafe(username);
checkDuplicateUsername(userModelAttrName, lowercaseUsername, realm, ldapProvider.getSession(), this);
setLDAPAttribute(UserModel.USERNAME, lowercaseUsername);
super.setUsername(lowercaseUsername);
}
@Override
public void setEmail(String email) {
String lowercaseEmail = KeycloakModelUtils.toLowerCaseSafe(email);
checkDuplicateEmail(userModelAttrName, email, realm, ldapProvider.getSession(), this);
setLDAPAttribute(UserModel.EMAIL, email);
super.setEmail(email);
super.setEmail(lowercaseEmail);
}
@Override

7
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java vendored
View file

@ -167,12 +167,19 @@ public class UserAdapter implements CachedUserModel {
@Override
public void setSingleAttribute(String name, String value) {
getDelegateForUpdate();
if (UserModel.USERNAME.equals(name) || UserModel.EMAIL.equals(name)) {
value = KeycloakModelUtils.toLowerCaseSafe(value);
}
updated.setSingleAttribute(name, value);
}
@Override
public void setAttribute(String name, List<String> values) {
getDelegateForUpdate();
if (UserModel.USERNAME.equals(name) || UserModel.EMAIL.equals(name)) {
String lowerCasedFirstValue = KeycloakModelUtils.toLowerCaseSafe((values != null && values.size() > 0) ? values.get(0) : null);
if (lowerCasedFirstValue != null) values.set(0, lowerCasedFirstValue);
}
updated.setAttribute(name, values);
}

8
server-spi-private/src/main/java/org/keycloak/storage/adapter/InMemoryUserAdapter.java
View file

@ -26,6 +26,7 @@ import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.UserModelDefaultMethods;
import org.keycloak.models.utils.DefaultRoles;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.RoleUtils;
import org.keycloak.storage.ReadOnlyException;
@ -123,6 +124,9 @@ public class InMemoryUserAdapter extends UserModelDefaultMethods {
@Override
public void setSingleAttribute(String name, String value) {
checkReadonly();
if (UserModel.USERNAME.equals(name) || UserModel.EMAIL.equals(name)) {
value = KeycloakModelUtils.toLowerCaseSafe(value);
}
attributes.putSingle(name, value);
}
@ -130,6 +134,10 @@ public class InMemoryUserAdapter extends UserModelDefaultMethods {
@Override
public void setAttribute(String name, List<String> values) {
checkReadonly();
if (UserModel.USERNAME.equals(name) || UserModel.EMAIL.equals(name)) {
String lowerCasedFirstValue = KeycloakModelUtils.toLowerCaseSafe((values != null && values.size() > 0) ? values.get(0) : null);
if (lowerCasedFirstValue != null) values.set(0, lowerCasedFirstValue);
}
attributes.put(name, values);
}

8
server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java
View file

@ -340,9 +340,9 @@ public abstract class AbstractUserAdapterFederatedStorage extends UserModelDefau
public void setSingleAttribute(String name, String value) {
if (UserModel.USERNAME.equals(name)) {
setUsername(value);
} else {
getFederatedStorage().setSingleAttribute(realm, this.getId(), mapAttribute(name), value);
}
getFederatedStorage().setSingleAttribute(realm, this.getId(), mapAttribute(name), value);
}
@Override
@ -355,9 +355,9 @@ public abstract class AbstractUserAdapterFederatedStorage extends UserModelDefau
public void setAttribute(String name, List<String> values) {
if (UserModel.USERNAME.equals(name)) {
setUsername((values != null && values.size() > 0) ? values.get(0) : null);
} else {
getFederatedStorage().setAttribute(realm, this.getId(), mapAttribute(name), values);
}
getFederatedStorage().setAttribute(realm, this.getId(), mapAttribute(name), values);
}
@Override

7
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
View file

@ -74,12 +74,11 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
UserModel federatedUser = session.users().addUser(realm, username);
federatedUser.setEnabled(true);
federatedUser.setEmail(brokerContext.getEmail());
federatedUser.setFirstName(brokerContext.getFirstName());
federatedUser.setLastName(brokerContext.getLastName());
for (Map.Entry<String, List<String>> attr : serializedCtx.getAttributes().entrySet()) {
federatedUser.setAttribute(attr.getKey(), attr.getValue());
if (!UserModel.USERNAME.equalsIgnoreCase(attr.getKey())) {
federatedUser.setAttribute(attr.getKey(), attr.getValue());
}
}
AuthenticatorConfigModel config = context.getAuthenticatorConfig();

24
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java
View file

@ -39,6 +39,7 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.storage.ReadOnlyException;
@ -1143,4 +1144,27 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
Assert.assertFalse(userNotVerified.get(0).isEmailVerified());
});
}
@Test
public void testUserAttributeLDAPStorageMapperHandlingUsernameLowercasing() {
setEditingUsernameAllowed(false);
testingClient.server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
UserModel johnkeycloak = session.users().getUserByUsername("johnkeycloak", appRealm);
// If the username was case sensitive in the username-cn mapper, then this would throw an exception
johnkeycloak.setSingleAttribute(UserModel.USERNAME, "JohnKeycloak");
});
// Cleanup
setEditingUsernameAllowed(true);
}
private void setEditingUsernameAllowed(boolean allowed) {
RealmRepresentation realmRepresentation = testRealm().toRepresentation();
realmRepresentation.setEditUsernameAllowed(allowed);
testRealm().update(realmRepresentation);
}
}