KEYCLOAK-1545 KEYCLOAK-1551 Ensure that username and email are always saved to DB lowercased

This commit is contained in:
mposolda 2015-07-17 18:38:59 +02:00
parent 38c7ca64cb
commit c71a4ac4e8
11 changed files with 123 additions and 19 deletions

View file

@ -431,13 +431,6 @@ public class LDAPFederationProvider implements UserFederationProvider {
return null; return null;
} }
// KEYCLOAK-808: Should we allow case-sensitivity to be configurable?
String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig());
if (!username.equals(ldapUsername)) {
logger.warnf("User found in LDAP but with different username. LDAP username: %s, Searched username: %s", username, ldapUsername);
return null;
}
return ldapUser; return ldapUser;
} }

View file

@ -23,6 +23,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserFederationMapperModel; import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider; import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.UserModelDelegate; import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.models.utils.reflection.Property; import org.keycloak.models.utils.reflection.Property;
import org.keycloak.models.utils.reflection.PropertyCriteria; import org.keycloak.models.utils.reflection.PropertyCriteria;
@ -138,6 +139,9 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
// throw ModelDuplicateException if there is different user in model with same email // throw ModelDuplicateException if there is different user in model with same email
protected void checkDuplicateEmail(String userModelAttrName, String email, RealmModel realm, KeycloakSession session, UserModel user) { protected void checkDuplicateEmail(String userModelAttrName, String email, RealmModel realm, KeycloakSession session, UserModel user) {
if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName)) { if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName)) {
// lowercase before search
email = KeycloakModelUtils.toLowerCaseSafe(email);
UserModel that = session.userStorage().getUserByEmail(email, realm); UserModel that = session.userStorage().getUserByEmail(email, realm);
if (that != null && !that.getId().equals(user.getId())) { if (that != null && !that.getId().equals(user.getId())) {
session.getTransaction().setRollbackOnly(); session.getTransaction().setRollbackOnly();

View file

@ -1,16 +1,14 @@
package org.keycloak.migration.migrators; package org.keycloak.migration.migrators;
import org.keycloak.migration.ModelVersion; import org.keycloak.migration.ModelVersion;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.AuthenticationFlowModel;
import org.keycloak.models.ImpersonationConstants; import org.keycloak.models.ImpersonationConstants;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.RequiredCredentialModel; import org.keycloak.models.UserModel;
import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.DefaultAuthenticationFlows;
import org.keycloak.models.utils.DefaultRequiredActions; import org.keycloak.models.utils.DefaultRequiredActions;
import org.keycloak.models.utils.KeycloakModelUtils;
import java.util.HashSet;
import java.util.List; import java.util.List;
/** /**
@ -28,7 +26,19 @@ public class MigrateTo1_4_0 {
DefaultRequiredActions.addActions(realm); DefaultRequiredActions.addActions(realm);
} }
ImpersonationConstants.setupImpersonationService(session, realm); ImpersonationConstants.setupImpersonationService(session, realm);
migrateUsers(session, realm);
} }
} }
public void migrateUsers(KeycloakSession session, RealmModel realm) {
List<UserModel> users = session.userStorage().getUsers(realm);
for (UserModel user : users) {
String email = user.getEmail();
email = KeycloakModelUtils.toLowerCaseSafe(email);
if (email != null && !email.equals(user.getEmail())) {
user.setEmail(email);
}
}
}
} }

View file

@ -350,4 +350,7 @@ public final class KeycloakModelUtils {
return mapperModel; return mapperModel;
} }
public static String toLowerCaseSafe(String str) {
return str==null ? str : str.toLowerCase();
}
} }

View file

@ -86,6 +86,8 @@ public class UserAdapter implements UserModel, Comparable {
@Override @Override
public void setUsername(String username) { public void setUsername(String username) {
username = KeycloakModelUtils.toLowerCaseSafe(username);
if (getUsername() == null) { if (getUsername() == null) {
user.setUsername(username); user.setUsername(username);
return; return;
@ -145,6 +147,8 @@ public class UserAdapter implements UserModel, Comparable {
@Override @Override
public void setEmail(String email) { public void setEmail(String email) {
email = KeycloakModelUtils.toLowerCaseSafe(email);
if (email == null) { if (email == null) {
user.setEmail(email); user.setEmail(email);
return; return;

View file

@ -10,6 +10,7 @@ import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserCredentialValueModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.models.cache.entities.CachedUser; import org.keycloak.models.cache.entities.CachedUser;
import org.keycloak.models.utils.KeycloakModelUtils;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
@ -57,6 +58,7 @@ public class UserAdapter implements UserModel {
@Override @Override
public void setUsername(String username) { public void setUsername(String username) {
getDelegateForUpdate(); getDelegateForUpdate();
username = KeycloakModelUtils.toLowerCaseSafe(username);
updated.setUsername(username); updated.setUsername(username);
} }
@ -189,6 +191,7 @@ public class UserAdapter implements UserModel {
@Override @Override
public void setEmail(String email) { public void setEmail(String email) {
getDelegateForUpdate(); getDelegateForUpdate();
email = KeycloakModelUtils.toLowerCaseSafe(email);
updated.setEmail(email); updated.setEmail(email);
} }

View file

@ -74,6 +74,7 @@ public class UserAdapter implements UserModel {
@Override @Override
public void setUsername(String username) { public void setUsername(String username) {
username = KeycloakModelUtils.toLowerCaseSafe(username);
user.setUsername(username); user.setUsername(username);
} }
@ -266,6 +267,7 @@ public class UserAdapter implements UserModel {
@Override @Override
public void setEmail(String email) { public void setEmail(String email) {
email = KeycloakModelUtils.toLowerCaseSafe(email);
user.setEmail(email); user.setEmail(email);
} }

View file

@ -67,6 +67,8 @@ public class UserAdapter extends AbstractMongoAdapter<MongoUserEntity> implement
@Override @Override
public void setUsername(String username) { public void setUsername(String username) {
username = KeycloakModelUtils.toLowerCaseSafe(username);
user.setUsername(username); user.setUsername(username);
updateUser(); updateUser();
} }
@ -121,6 +123,8 @@ public class UserAdapter extends AbstractMongoAdapter<MongoUserEntity> implement
@Override @Override
public void setEmail(String email) { public void setEmail(String email) {
email = KeycloakModelUtils.toLowerCaseSafe(email);
user.setEmail(email); user.setEmail(email);
updateUser(); updateUser();
} }

View file

@ -27,9 +27,13 @@ import static org.junit.Assert.fail;
public class UserTest extends AbstractClientTest { public class UserTest extends AbstractClientTest {
public String createUser() { public String createUser() {
return createUser("user1", "user1@localhost");
}
public String createUser(String username, String email) {
UserRepresentation user = new UserRepresentation(); UserRepresentation user = new UserRepresentation();
user.setUsername("user1"); user.setUsername(username);
user.setEmail("user1@localhost"); user.setEmail(email);
Response response = realm.users().create(user); Response response = realm.users().create(user);
String createdId = ApiUtil.getCreatedId(response); String createdId = ApiUtil.getCreatedId(response);
@ -116,6 +120,19 @@ public class UserTest extends AbstractClientTest {
response.close(); response.close();
} }
@Test
public void createDuplicatedUser7() {
createUser("user1", "USer1@Localhost");
UserRepresentation user = new UserRepresentation();
user.setUsername("user2");
user.setEmail("user1@localhost");
Response response = realm.users().create(user);
assertEquals(409, response.getStatus());
response.close();
}
private void createUsers() { private void createUsers() {
for (int i = 1; i < 10; i++) { for (int i = 1; i < 10; i++) {
UserRepresentation user = new UserRepresentation(); UserRepresentation user = new UserRepresentation();

View file

@ -39,6 +39,7 @@ import org.keycloak.testsuite.rule.WebResource;
import org.keycloak.testsuite.rule.WebRule; import org.keycloak.testsuite.rule.WebRule;
import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebDriver;
import java.util.List;
import java.util.Map; import java.util.Map;
/** /**
@ -113,12 +114,75 @@ public class FederationProvidersIntegrationTest {
@Test @Test
public void caseSensitiveSearch() { public void caseInSensitiveImport() {
loginPage.open(); KeycloakSession session = keycloakRule.startSession();
try {
RealmManager manager = new RealmManager(session);
RealmModel appRealm = manager.getRealm("test");
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
LDAPObject jbrown2 = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "JBrown2", "John", "Brown2", "jbrown2@email.org", null, "1234");
ldapFedProvider.getLdapIdentityStore().updatePassword(jbrown2, "password");
LDAPObject jbrown3 = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "jbrown3", "John", "Brown3", "JBrown3@email.org", null, "1234");
ldapFedProvider.getLdapIdentityStore().updatePassword(jbrown3, "password");
} finally {
keycloakRule.stopSession(session, true);
}
// This should fail for now due to case-sensitivity loginSuccessAndLogout("jbrown2", "password");
loginPage.login("johnKeycloak", "Password1"); loginSuccessAndLogout("JBrown2", "password");
Assert.assertEquals("Invalid username or password.", loginPage.getError()); loginSuccessAndLogout("jbrown2@email.org", "password");
loginSuccessAndLogout("JBrown2@email.org", "password");
loginSuccessAndLogout("jbrown3", "password");
loginSuccessAndLogout("JBrown3", "password");
loginSuccessAndLogout("jbrown3@email.org", "password");
loginSuccessAndLogout("JBrown3@email.org", "password");
}
private void loginSuccessAndLogout(String username, String password) {
loginPage.open();
loginPage.login(username, password);
Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE));
oauth.openLogout();
}
@Test
public void caseInsensitiveSearch() {
KeycloakSession session = keycloakRule.startSession();
try {
RealmManager manager = new RealmManager(session);
RealmModel appRealm = manager.getRealm("test");
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
LDAPObject jbrown2 = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "JBrown4", "John", "Brown4", "jbrown4@email.org", null, "1234");
ldapFedProvider.getLdapIdentityStore().updatePassword(jbrown2, "password");
LDAPObject jbrown3 = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "jbrown5", "John", "Brown5", "JBrown5@Email.org", null, "1234");
ldapFedProvider.getLdapIdentityStore().updatePassword(jbrown3, "password");
} finally {
keycloakRule.stopSession(session, true);
}
session = keycloakRule.startSession();
try {
RealmManager manager = new RealmManager(session);
RealmModel appRealm = manager.getRealm("test");
// search by username
List<UserModel> users = session.users().searchForUser("JBROwn4", appRealm);
Assert.assertEquals(1, users.size());
UserModel user4 = users.get(0);
Assert.assertEquals("jbrown4", user4.getUsername());
Assert.assertEquals("jbrown4@email.org", user4.getEmail());
// search by email
users = session.users().searchForUser("JBROwn5@eMAil.org", appRealm);
Assert.assertEquals(1, users.size());
UserModel user5 = users.get(0);
Assert.assertEquals("jbrown5", user5.getUsername());
Assert.assertEquals("jbrown5@email.org", user5.getEmail());
} finally {
keycloakRule.stopSession(session, true);
}
} }
@Test @Test

View file

@ -151,7 +151,7 @@ public class SyncProvidersTest {
RealmModel testRealm = session.realms().getRealm("test"); RealmModel testRealm = session.realms().getRealm("test");
UserProvider userProvider = session.userStorage(); UserProvider userProvider = session.userStorage();
// Assert users updated in local provider // Assert users updated in local provider
FederationTestUtils.assertUserImported(userProvider, testRealm, "user5", "User5FN", "User5LN", "user5Updated@email.org", "521"); FederationTestUtils.assertUserImported(userProvider, testRealm, "user5", "User5FN", "User5LN", "user5updated@email.org", "521");
FederationTestUtils.assertUserImported(userProvider, testRealm, "user6", "User6FN", "User6LN", "user6@email.org", "126"); FederationTestUtils.assertUserImported(userProvider, testRealm, "user6", "User6FN", "User6LN", "user6@email.org", "126");
} finally { } finally {
keycloakRule.stopSession(session, false); keycloakRule.stopSession(session, false);