KEYCLOAK-1533 Handle importing LDAP user with duplicate email during authentication
This commit is contained in:
parent
94c0a436b5
commit
bde65a6c57
9 changed files with 88 additions and 8 deletions
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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");
|
||||
|
|
|
@ -437,7 +437,7 @@ public class AccountService {
|
|||
String username = formData.getFirst("username");
|
||||
|
||||
UserModel existing = session.users().getUserByUsername(username, realm);
|
||||
if (existing != null && !existing.equals(user)) {
|
||||
if (existing != null && !existing.getId().equals(user.getId())) {
|
||||
throw new ModelDuplicateException(Messages.USERNAME_EXISTS);
|
||||
}
|
||||
|
||||
|
@ -451,7 +451,7 @@ public class AccountService {
|
|||
boolean emailChanged = oldEmail != null ? !oldEmail.equals(email) : email != null;
|
||||
if (emailChanged) {
|
||||
UserModel existing = session.users().getUserByEmail(email, realm);
|
||||
if (existing != null && !existing.equals(user)) {
|
||||
if (existing != null && !existing.getId().equals(user.getId())) {
|
||||
throw new ModelDuplicateException(Messages.EMAIL_EXISTS);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -416,6 +416,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();
|
||||
|
|
Loading…
Reference in a new issue