keycloak-aplcache
Changes
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java 21(+20 -1)
forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html 3(+2 -1)
model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java 2(+1 -1)
model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java 2(+1 -1)
model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java 2(+1 -1)
model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java 2(+1 -1)
Details
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java
index 283089a..438c57d 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java
@@ -16,7 +16,9 @@ import org.keycloak.federation.ldap.idm.model.LDAPObject;
import org.keycloak.federation.ldap.idm.query.Condition;
import org.keycloak.federation.ldap.idm.query.QueryParameter;
import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery;
+import org.keycloak.models.KeycloakSession;
import org.keycloak.models.LDAPConstants;
+import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider;
@@ -74,6 +76,9 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
// we have java property on UserModel
String ldapAttrValue = ldapUser.getAttributeAsString(ldapAttrName);
+
+ checkDuplicateEmail(userModelAttrName, ldapAttrValue, realm, ldapProvider.getSession(), user);
+
setPropertyOnUserModel(userModelProperty, user, ldapAttrValue);
} else {
@@ -130,8 +135,20 @@ 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)) {
+ UserModel that = session.userStorage().getUserByEmail(email, realm);
+ if (that != null && !that.getId().equals(user.getId())) {
+ session.getTransaction().setRollbackOnly();
+ String exceptionMessage = String.format("Can't import user '%s' from LDAP because email '%s' already exists in Keycloak. Existing user with this email is '%s'", user.getUsername(), email, that.getUsername());
+ throw new ModelDuplicateException(exceptionMessage, UserModel.EMAIL);
+ }
+ }
+ }
+
@Override
- public UserModel proxy(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, final LDAPObject ldapUser, UserModel delegate, RealmModel realm) {
+ public UserModel proxy(UserFederationMapperModel mapperModel, final LDAPFederationProvider ldapProvider, final LDAPObject ldapUser, UserModel delegate, final RealmModel realm) {
final String userModelAttrName = mapperModel.getConfig().get(USER_MODEL_ATTRIBUTE);
final String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE);
boolean isAlwaysReadValueFromLDAP = parseBooleanParameter(mapperModel, ALWAYS_READ_VALUE_FROM_LDAP);
@@ -162,6 +179,8 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
@Override
public void setEmail(String email) {
+ checkDuplicateEmail(userModelAttrName, email, realm, ldapProvider.getSession(), this);
+
setLDAPAttribute(UserModel.EMAIL, email);
super.setEmail(email);
}
diff --git a/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties b/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
index f783b49..2c3b8c7 100755
--- a/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
+++ b/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
@@ -109,6 +109,9 @@ invalidPasswordExistingMessage=Invalid existing password.
invalidPasswordConfirmMessage=Password confirmation doesn''t match.
invalidTotpMessage=Invalid authenticator code.
+usernameExistsMessage=Username already exists.
+emailExistsMessage=Email already exists.
+
readOnlyUserMessage=You can''t update your account as it is read only.
readOnlyPasswordMessage=You can''t update your password as your account is read only.
diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
index 7ebaa74..51f3f98 100755
--- a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
+++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
@@ -98,7 +98,8 @@
<div class="col-md-6">
<input class="form-control" id="userObjectClasses" type="text" ng-model="instance.config.userObjectClasses" placeholder="LDAP User Object Classes (div. by comma)" required>
</div>
- <kc-tooltip>All values of LDAP objectClass attribute for users in LDAP divided by comma</kc-tooltip>
+ <kc-tooltip>All values of LDAP objectClass attribute for users in LDAP divided by comma. For example: 'inetOrgPerson, organizationalPerson' . Newly created Keycloak users will be written to LDAP
+ with all those object classes and existing LDAP user records are found just if they contain all those object classes. </kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="ldapConnectionUrl"><span class="required">*</span> Connection URL</label>
diff --git a/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java b/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java
index 1d6e69a..eb4776d 100644
--- a/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java
+++ b/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java
@@ -5,6 +5,8 @@ package org.keycloak.models;
*/
public class ModelDuplicateException extends ModelException {
+ private String duplicateFieldName;
+
public ModelDuplicateException() {
}
@@ -12,6 +14,11 @@ public class ModelDuplicateException extends ModelException {
super(message);
}
+ public ModelDuplicateException(String message, String duplicateFieldName) {
+ super(message);
+ this.duplicateFieldName = duplicateFieldName;
+ }
+
public ModelDuplicateException(String message, Throwable cause) {
super(message, cause);
}
@@ -20,4 +27,7 @@ public class ModelDuplicateException extends ModelException {
super(cause);
}
+ public String getDuplicateFieldName() {
+ return duplicateFieldName;
+ }
}
diff --git a/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java b/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java
index 66b7592..52ff887 100755
--- a/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java
+++ b/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java
@@ -223,7 +223,7 @@ public class ClientSessionAdapter implements ClientSessionModel {
@Override
public UserModel getAuthenticatedUser() {
- return session.users().getUserById(entity.getAuthUserId(), realm); }
+ return entity.getAuthUserId() == null ? null : session.users().getUserById(entity.getAuthUserId(), realm); }
@Override
public void setAuthenticatedUser(UserModel user) {
diff --git a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java
index e76811e..1de321e 100755
--- a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java
+++ b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java
@@ -301,7 +301,7 @@ public class ClientSessionAdapter implements ClientSessionModel {
@Override
public UserModel getAuthenticatedUser() {
- return session.users().getUserById(entity.getUserId(), realm);
+ return entity.getUserId() == null ? null : session.users().getUserById(entity.getUserId(), realm);
}
@Override
diff --git a/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java b/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java
index 458278a..c54d341 100755
--- a/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java
+++ b/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java
@@ -189,7 +189,7 @@ public class ClientSessionAdapter implements ClientSessionModel {
@Override
public UserModel getAuthenticatedUser() {
- return session.users().getUserById(entity.getAuthUserId(), realm); }
+ return entity.getAuthUserId() == null ? null : session.users().getUserById(entity.getAuthUserId(), realm); }
@Override
public void setAuthenticatedUser(UserModel user) {
diff --git a/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java b/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java
index 55013ed..7545dd0 100755
--- a/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java
+++ b/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java
@@ -205,7 +205,7 @@ public class ClientSessionAdapter extends AbstractMongoAdapter<MongoClientSessio
@Override
public UserModel getAuthenticatedUser() {
- return session.users().getUserById(entity.getAuthUserId(), realm);
+ return entity.getAuthUserId() == null ? null : session.users().getUserById(entity.getAuthUserId(), realm);
}
@Override
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/AbstractFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/AbstractFormAuthenticator.java
index 957c540..98b906c 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/AbstractFormAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/AbstractFormAuthenticator.java
@@ -1,5 +1,6 @@
package org.keycloak.authentication.authenticators;
+import org.jboss.logging.Logger;
import org.keycloak.OAuth2Constants;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.authentication.Authenticator;
@@ -7,6 +8,7 @@ import org.keycloak.authentication.AuthenticatorContext;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.login.LoginFormsProvider;
+import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
@@ -27,6 +29,8 @@ import java.util.List;
*/
public abstract class AbstractFormAuthenticator implements Authenticator {
+ private static final Logger logger = Logger.getLogger(AbstractFormAuthenticator.class);
+
public static final String REGISTRATION_FORM_ACTION = "registration_form";
public static final String EXECUTION = "execution";
public static final String ATTEMPTED_USERNAME = "ATTEMPTED_USERNAME";
@@ -82,6 +86,14 @@ public abstract class AbstractFormAuthenticator implements Authenticator {
.setError(Messages.INVALID_USER).createLogin();
}
+ protected Response setDuplicateUserChallenge(AuthenticatorContext context, String eventError, String loginFormError, AuthenticationProcessor.Error authenticatorError) {
+ context.getEvent().error(eventError);
+ Response challengeResponse = loginForm(context)
+ .setError(loginFormError).createLogin();
+ context.failureChallenge(authenticatorError, challengeResponse);
+ return challengeResponse;
+ }
+
public boolean invalidUser(AuthenticatorContext context, UserModel user) {
if (user == null) {
context.getEvent().error(Errors.USER_NOT_FOUND);
@@ -118,7 +130,23 @@ public abstract class AbstractFormAuthenticator implements Authenticator {
}
context.getEvent().detail(Details.USERNAME, username);
context.getClientSession().setNote(AbstractFormAuthenticator.ATTEMPTED_USERNAME, username);
- UserModel user = KeycloakModelUtils.findUserByNameOrEmail(context.getSession(), context.getRealm(), username);
+
+ UserModel user = null;
+ try {
+ user = KeycloakModelUtils.findUserByNameOrEmail(context.getSession(), context.getRealm(), username);
+ } catch (ModelDuplicateException mde) {
+ logger.error(mde.getMessage(), mde);
+
+ // Could happen during federation import
+ if (mde.getDuplicateFieldName() != null && mde.getDuplicateFieldName().equals(UserModel.EMAIL)) {
+ setDuplicateUserChallenge(context, Errors.EMAIL_IN_USE, Messages.EMAIL_EXISTS, AuthenticationProcessor.Error.INVALID_USER);
+ } else {
+ setDuplicateUserChallenge(context, Errors.USERNAME_IN_USE, Messages.USERNAME_EXISTS, AuthenticationProcessor.Error.INVALID_USER);
+ }
+
+ return false;
+ }
+
if (invalidUser(context, user)) return false;
String rememberMe = inputData.getFirst("rememberMe");
boolean remember = rememberMe != null && rememberMe.equalsIgnoreCase("on");
diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java
index 4ceebb3..02762af 100755
--- a/services/src/main/java/org/keycloak/services/resources/AccountService.java
+++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java
@@ -42,6 +42,7 @@ import org.keycloak.models.Constants;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
import org.keycloak.models.ModelReadOnlyException;
import org.keycloak.models.RealmModel;
@@ -433,7 +434,14 @@ public class AccountService {
try {
if (realm.isEditUsernameAllowed()) {
- user.setUsername(formData.getFirst("username"));
+ String username = formData.getFirst("username");
+
+ UserModel existing = session.users().getUserByUsername(username, realm);
+ if (existing != null && !existing.getId().equals(user.getId())) {
+ throw new ModelDuplicateException(Messages.USERNAME_EXISTS);
+ }
+
+ user.setUsername(username);
}
user.setFirstName(formData.getFirst("firstName"));
user.setLastName(formData.getFirst("lastName"));
@@ -441,8 +449,14 @@ public class AccountService {
String email = formData.getFirst("email");
String oldEmail = user.getEmail();
boolean emailChanged = oldEmail != null ? !oldEmail.equals(email) : email != null;
+ if (emailChanged) {
+ UserModel existing = session.users().getUserByEmail(email, realm);
+ if (existing != null && !existing.getId().equals(user.getId())) {
+ throw new ModelDuplicateException(Messages.EMAIL_EXISTS);
+ }
+ }
- user.setEmail(formData.getFirst("email"));
+ user.setEmail(email);
AttributeFormDataProcessor.process(formData, realm, user);
@@ -457,6 +471,9 @@ public class AccountService {
} catch (ModelReadOnlyException roe) {
setReferrerOnPage();
return account.setError(Messages.READ_ONLY_USER).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT);
+ } catch (ModelDuplicateException mde) {
+ setReferrerOnPage();
+ return account.setError(mde.getMessage()).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT);
}
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java
index d621e46..1ea081f 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java
@@ -82,6 +82,7 @@ public class AccountTest {
UserModel user2 = manager.getSession().users().addUser(appRealm, "test-user-no-access@localhost");
user2.setEnabled(true);
+ user2.setEmail("test-user-no-access@localhost");
for (String r : accountApp.getDefaultRoles()) {
user2.deleteRoleMapping(accountApp.getRole(r));
}
@@ -395,6 +396,10 @@ public class AccountTest {
events.expectAccount(EventType.UPDATE_PROFILE).assertEvent();
events.expectAccount(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent();
+
+ // reset user for other tests
+ profilePage.updateProfile("Tom", "Brady", "test-user@localhost");
+ events.clear();
}
@Test
@@ -429,6 +434,17 @@ public class AccountTest {
events.assertEmpty();
+ // Change to the username already occupied by other user
+ profilePage.updateProfile("test-user-no-access@localhost", "New first", "New last", "new@email.com");
+
+ Assert.assertEquals("Username already exists.", profilePage.getError());
+ Assert.assertEquals("test-user-no-access@localhost", profilePage.getUsername());
+ Assert.assertEquals("New first", profilePage.getFirstName());
+ Assert.assertEquals("New last", profilePage.getLastName());
+ Assert.assertEquals("new@email.com", profilePage.getEmail());
+
+ events.assertEmpty();
+
profilePage.updateProfile("test-user-new@localhost", "New first", "New last", "new@email.com");
Assert.assertEquals("Your account has been updated.", profilePage.getSuccess());
@@ -452,6 +468,43 @@ public class AccountTest {
}
}
+ // KEYCLOAK-1534
+ @Test
+ public void changeEmailToExisting() {
+ profilePage.open();
+ loginPage.login("test-user@localhost", "password");
+
+ events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT).assertEvent();
+
+ Assert.assertEquals("test-user@localhost", profilePage.getUsername());
+ Assert.assertEquals("test-user@localhost", profilePage.getEmail());
+
+ // Change to the email, which some other user has
+ profilePage.updateProfile("New first", "New last", "test-user-no-access@localhost");
+
+ profilePage.assertCurrent();
+ Assert.assertEquals("Email already exists.", profilePage.getError());
+ Assert.assertEquals("New first", profilePage.getFirstName());
+ Assert.assertEquals("New last", profilePage.getLastName());
+ Assert.assertEquals("test-user-no-access@localhost", profilePage.getEmail());
+
+ events.assertEmpty();
+
+ // Change some other things, but not email
+ profilePage.updateProfile("New first", "New last", "test-user@localhost");
+
+ Assert.assertEquals("Your account has been updated.", profilePage.getSuccess());
+ Assert.assertEquals("New first", profilePage.getFirstName());
+ Assert.assertEquals("New last", profilePage.getLastName());
+ Assert.assertEquals("test-user@localhost", profilePage.getEmail());
+
+ events.expectAccount(EventType.UPDATE_PROFILE).assertEvent();
+
+ // Change email and other things to original values
+ profilePage.updateProfile("Tom", "Brady", "test-user@localhost");
+ events.expectAccount(EventType.UPDATE_PROFILE).assertEvent();
+ }
+
@Test
public void setupTotp() {
totpPage.open();
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java
index 1945701..708da5b 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java
@@ -417,6 +417,29 @@ public class FederationProvidersIntegrationTest {
}
@Test
+ public void testImportExistingUserFromLDAP() throws Exception {
+ // Add LDAP user with same email like existing model user
+ keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+ @Override
+ public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+ LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+ FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "mary", "Mary1", "Kelly1", "mary1@email.org", null, "123");
+ FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "mary-duplicatemail", "Mary2", "Kelly2", "mary@test.com", null, "123");
+ }
+
+ });
+
+ // Try to import the duplicated LDAP user into Keycloak
+ loginPage.open();
+ loginPage.login("mary-duplicatemail", "password");
+ Assert.assertEquals("Email already exists.", loginPage.getError());
+
+ loginPage.login("mary1@email.org", "password");
+ Assert.assertEquals("Username already exists.", loginPage.getError());
+ }
+
+ @Test
public void testReadonly() {
KeycloakSession session = keycloakRule.startSession();
try {