Merge pull request #1467 from mposolda/master

LDAP and case-sensitivity fixes
This commit is contained in:
Marek Posolda 2015-07-21 08:58:07 +02:00
commit 23b69bb7a7
14 changed files with 208 additions and 26 deletions

View file

@ -19,7 +19,7 @@ cp http.keytab /tmp/http.keytab
```
Alternative is to configure different location for `keyTab` property in `kerberosrealm.json` configuration file (On Windows this will be needed).
WARNING: In production, keytab file should be in secured location accessible just to the user under which is Keycloak server running.
**WARNING**: In production, keytab file should be in secured location accessible just to the user under which is Keycloak server running.
**3)** Run Keycloak server and import `kerberosrealm.json` into it through admin console. This will import realm with sample application
@ -47,6 +47,8 @@ See [this file](https://github.com/keycloak/keycloak/blob/master/testsuite/integ
with these commands (assuming you're in `kerberos` directory with this example)
```
cd ../ldap
mvn clean install
cd ..
java -jar ldap/embedded-ldap/target/embedded-ldap.jar kerberos
```

View file

@ -431,13 +431,6 @@ public class LDAPFederationProvider implements UserFederationProvider {
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;
}

View file

@ -10,6 +10,7 @@ import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.TreeSet;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
@ -130,7 +131,7 @@ public class LDAPIdentityStore implements IdentityStore {
.lookupById(baseDN, equalCondition.getValue().toString(), identityQuery.getReturningLdapAttributes());
if (search != null) {
results.add(populateAttributedType(search, identityQuery.getReturningReadOnlyLdapAttributes()));
results.add(populateAttributedType(search, identityQuery));
}
}
@ -150,7 +151,7 @@ public class LDAPIdentityStore implements IdentityStore {
for (SearchResult result : search) {
if (!result.getNameInNamespace().equalsIgnoreCase(baseDN)) {
results.add(populateAttributedType(result, identityQuery.getReturningReadOnlyLdapAttributes()));
results.add(populateAttributedType(result, identityQuery));
}
}
} catch (Exception e) {
@ -371,7 +372,13 @@ public class LDAPIdentityStore implements IdentityStore {
}
private LDAPObject populateAttributedType(SearchResult searchResult, Collection<String> readOnlyAttrNames) {
private LDAPObject populateAttributedType(SearchResult searchResult, LDAPQuery ldapQuery) {
Set<String> readOnlyAttrNames = ldapQuery.getReturningReadOnlyLdapAttributes();
Set<String> lowerCasedAttrNames = new TreeSet<>();
for (String attrName : ldapQuery.getReturningLdapAttributes()) {
lowerCasedAttrNames.add(attrName.toLowerCase());
}
try {
String entryDN = searchResult.getNameInNamespace();
Attributes attributes = searchResult.getAttributes();
@ -397,7 +404,10 @@ public class LDAPIdentityStore implements IdentityStore {
if (ldapAttributeName.equalsIgnoreCase(getConfig().getUuidLDAPAttributeName())) {
Object uuidValue = ldapAttribute.get();
ldapObject.setUuid(this.operationManager.decodeEntryUUID(uuidValue));
} else {
}
// Note: UUID is normally not populated here. It's populated just in case that it's used for name of other attribute as well
if (!ldapAttributeName.equalsIgnoreCase(getConfig().getUuidLDAPAttributeName()) || (lowerCasedAttrNames.contains(ldapAttributeName.toLowerCase()))) {
Set<String> attrValues = new LinkedHashSet<>();
NamingEnumeration<?> enumm = ldapAttribute.getAll();
while (enumm.hasMoreElements()) {

View file

@ -240,7 +240,7 @@ public class LDAPOperationManager {
filter = "(&(objectClass=*)(" + getUuidAttributeName() + LDAPConstants.EQUAL + LDAPUtil.convertObjectGUIToByteString(objectGUID) + "))";
} catch (NamingException ne) {
return filter;
filter = null;
}
}
@ -435,7 +435,7 @@ public class LDAPOperationManager {
public String decodeEntryUUID(final Object entryUUID) {
String id;
if (this.config.isActiveDirectory()) {
if (this.config.isActiveDirectory() && entryUUID instanceof byte[]) {
id = LDAPUtil.decodeObjectGUID((byte[]) entryUUID);
} else {
id = entryUUID.toString();

View file

@ -23,6 +23,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.models.utils.reflection.Property;
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
protected void checkDuplicateEmail(String userModelAttrName, String email, RealmModel realm, KeycloakSession session, UserModel user) {
if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName)) {
// lowercase before search
email = KeycloakModelUtils.toLowerCaseSafe(email);
UserModel that = session.userStorage().getUserByEmail(email, realm);
if (that != null && !that.getId().equals(user.getId())) {
session.getTransaction().setRollbackOnly();

View file

@ -1,16 +1,14 @@
package org.keycloak.migration.migrators;
import org.keycloak.migration.ModelVersion;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.AuthenticationFlowModel;
import org.keycloak.models.ImpersonationConstants;
import org.keycloak.models.KeycloakSession;
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.DefaultRequiredActions;
import org.keycloak.models.utils.KeycloakModelUtils;
import java.util.HashSet;
import java.util.List;
/**
@ -28,7 +26,19 @@ public class MigrateTo1_4_0 {
DefaultRequiredActions.addActions(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;
}
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
public void setUsername(String username) {
username = KeycloakModelUtils.toLowerCaseSafe(username);
if (getUsername() == null) {
user.setUsername(username);
return;
@ -145,6 +147,8 @@ public class UserAdapter implements UserModel, Comparable {
@Override
public void setEmail(String email) {
email = KeycloakModelUtils.toLowerCaseSafe(email);
if (email == null) {
user.setEmail(email);
return;

View file

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

View file

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

View file

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

View file

@ -27,9 +27,13 @@ import static org.junit.Assert.fail;
public class UserTest extends AbstractClientTest {
public String createUser() {
return createUser("user1", "user1@localhost");
}
public String createUser(String username, String email) {
UserRepresentation user = new UserRepresentation();
user.setUsername("user1");
user.setEmail("user1@localhost");
user.setUsername(username);
user.setEmail(email);
Response response = realm.users().create(user);
String createdId = ApiUtil.getCreatedId(response);
@ -115,6 +119,19 @@ public class UserTest extends AbstractClientTest {
assertEquals(409, response.getStatus());
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() {
for (int i = 1; i < 10; i++) {

View file

@ -39,6 +39,7 @@ import org.keycloak.testsuite.rule.WebResource;
import org.keycloak.testsuite.rule.WebRule;
import org.openqa.selenium.WebDriver;
import java.util.List;
import java.util.Map;
/**
@ -113,12 +114,75 @@ public class FederationProvidersIntegrationTest {
@Test
public void caseSensitiveSearch() {
loginPage.open();
public void caseInSensitiveImport() {
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
loginPage.login("johnKeycloak", "Password1");
Assert.assertEquals("Invalid username or password.", loginPage.getError());
loginSuccessAndLogout("jbrown2", "password");
loginSuccessAndLogout("JBrown2", "password");
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

View file

@ -7,6 +7,7 @@ import org.junit.Test;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
import org.junit.runners.MethodSorters;
import org.keycloak.federation.ldap.LDAPConfig;
import org.keycloak.federation.ldap.LDAPFederationProvider;
import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
import org.keycloak.federation.ldap.idm.model.LDAPObject;
@ -17,9 +18,12 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserFederationSyncResult;
import org.keycloak.models.UserModel;
import org.keycloak.models.UserProvider;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.services.managers.UsersSyncManager;
import org.keycloak.testsuite.rule.AbstractKeycloakRule;
import org.keycloak.testsuite.rule.KeycloakRule;
import org.keycloak.testsuite.rule.LDAPRule;
import org.keycloak.testsuite.DummyUserFederationProviderFactory;
@ -147,7 +151,7 @@ public class SyncProvidersTest {
RealmModel testRealm = session.realms().getRealm("test");
UserProvider userProvider = session.userStorage();
// 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");
} finally {
keycloakRule.stopSession(session, false);
@ -213,6 +217,68 @@ public class SyncProvidersTest {
}
}
// KEYCLOAK-1571
@Test
public void test03SameUUIDAndUsernameSync() {
KeycloakSession session = keycloakRule.startSession();
String origUuidAttrName;
try {
RealmModel testRealm = session.realms().getRealm("test");
// Remove all users from model
for (UserModel user : session.userStorage().getUsers(testRealm)) {
session.userStorage().removeUser(testRealm, user);
}
UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
// Change name of UUID attribute to same like usernameAttribute
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
String uidAttrName = ldapFedProvider.getLdapIdentityStore().getConfig().getUsernameLdapAttribute();
origUuidAttrName = providerModel.getConfig().get(LDAPConstants.UUID_LDAP_ATTRIBUTE);
providerModel.getConfig().put(LDAPConstants.UUID_LDAP_ATTRIBUTE, uidAttrName);
// Need to change this due to ApacheDS pagination bug (For other LDAP servers, pagination works fine) TODO: Remove once ApacheDS upgraded and pagination is fixed
providerModel.getConfig().put(LDAPConstants.BATCH_SIZE_FOR_SYNC, "10");
testRealm.updateUserFederationProvider(providerModel);
} finally {
keycloakRule.stopSession(session, true);
}
session = keycloakRule.startSession();
try {
RealmModel testRealm = session.realms().getRealm("test");
UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
UserFederationSyncResult syncResult = new UsersSyncManager().syncAllUsers(sessionFactory, "test", providerModel);
Assert.assertEquals(0, syncResult.getFailed());
} finally {
keycloakRule.stopSession(session, false);
}
session = keycloakRule.startSession();
try {
RealmModel testRealm = session.realms().getRealm("test");
// Assert users imported with correct LDAP_ID
FederationTestUtils.assertUserImported(session.users(), testRealm, "user1", "User1FN", "User1LN", "user1@email.org", "121");
FederationTestUtils.assertUserImported(session.users(), testRealm, "user2", "User2FN", "User2LN", "user2@email.org", "122");
UserModel user1 = session.users().getUserByUsername("user1", testRealm);
Assert.assertEquals("user1", user1.getFirstAttribute(LDAPConstants.LDAP_ID));
// Revert config changes
UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
providerModel.getConfig().put(LDAPConstants.UUID_LDAP_ATTRIBUTE, origUuidAttrName);
testRealm.updateUserFederationProvider(providerModel);
} finally {
keycloakRule.stopSession(session, true);
}
}
@Test
public void testPeriodicSync() {
KeycloakSession session = keycloakRule.startSession();