KEYCLOAK-4266 MSAD: User is disabled after registration

This commit is contained in:
mposolda 2017-01-19 21:32:10 +01:00
parent 536b88790e
commit 3444fb62f1
8 changed files with 418 additions and 95 deletions

View file

@ -92,6 +92,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
protected LDAPProviderKerberosConfig kerberosConfig;
protected PasswordUpdateCallback updater;
protected LDAPStorageMapperManager mapperManager;
protected LDAPStorageUserManager userManager;
protected final Set<String> supportedCredentialTypes = new HashSet<>();
@ -103,6 +104,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
this.kerberosConfig = new LDAPProviderKerberosConfig(model);
this.editMode = ldapIdentityStore.getConfig().getEditMode();
this.mapperManager = new LDAPStorageMapperManager(this);
this.userManager = new LDAPStorageUserManager(this);
supportedCredentialTypes.add(UserCredentialModel.PASSWORD);
if (kerberosConfig.isAllowKerberosAuthentication()) {
@ -134,6 +136,11 @@ public class LDAPStorageProvider implements UserStorageProvider,
return mapperManager;
}
public LDAPStorageUserManager getUserManager() {
return userManager;
}
@Override
public UserModel validate(RealmModel realm, UserModel local) {
LDAPObject ldapObject = loadAndValidateUser(realm, local);
@ -145,6 +152,11 @@ public class LDAPStorageProvider implements UserStorageProvider,
}
protected UserModel proxy(RealmModel realm, UserModel local, LDAPObject ldapObject) {
UserModel existing = userManager.getManagedProxiedUser(local.getId());
if (existing != null) {
return existing;
}
UserModel proxied = local;
checkDNChanged(realm, local, ldapObject);
@ -167,6 +179,8 @@ public class LDAPStorageProvider implements UserStorageProvider,
proxied = ldapMapper.proxy(ldapObject, proxied, realm);
}
userManager.setManagedProxiedUser(proxied, ldapObject);
return proxied;
}
@ -227,6 +241,8 @@ public class LDAPStorageProvider implements UserStorageProvider,
}
ldapIdentityStore.remove(ldapObject);
userManager.removeManagedUserEntry(user.getId());
return true;
}
@ -385,6 +401,11 @@ public class LDAPStorageProvider implements UserStorageProvider,
* @return ldapUser corresponding to local user or null if user is no longer in LDAP
*/
protected LDAPObject loadAndValidateUser(RealmModel realm, UserModel local) {
LDAPObject existing = userManager.getManagedLDAPUser(local.getId());
if (existing != null) {
return existing;
}
LDAPObject ldapUser = loadLDAPUserByUsername(realm, local.getUsername());
if (ldapUser == null) {
return null;

View file

@ -0,0 +1,103 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.storage.ldap;
import java.util.HashMap;
import java.util.Map;
import org.keycloak.models.UserModel;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
import org.keycloak.storage.ldap.mappers.LDAPTransaction;
/**
* Track which LDAP users were already enlisted during this transaction
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPStorageUserManager {
private final Map<String, ManagedUserEntry> managedUsers = new HashMap<>();
private final LDAPStorageProvider provider;
public LDAPStorageUserManager(LDAPStorageProvider provider) {
this.provider = provider;
}
public UserModel getManagedProxiedUser(String userId) {
ManagedUserEntry entry = managedUsers.get(userId);
return entry==null ? null : entry.getManagedProxiedUser();
}
public LDAPObject getManagedLDAPUser(String userId) {
ManagedUserEntry entry = managedUsers.get(userId);
return entry==null ? null : entry.getLdapUser();
}
public LDAPTransaction getTransaction(String userId) {
ManagedUserEntry entry = managedUsers.get(userId);
if (entry == null) {
throw new IllegalStateException("Shouldn't happen to not have entry for userId: " + userId);
}
return entry.getLdapTransaction();
}
public void setManagedProxiedUser(UserModel proxiedUser, LDAPObject ldapObject) {
String userId = proxiedUser.getId();
ManagedUserEntry entry = managedUsers.get(userId);
if (entry != null) {
throw new IllegalStateException("Don't expect to have entry for user " + userId);
}
LDAPTransaction ldapTransaction = new LDAPTransaction(provider, ldapObject);
ManagedUserEntry newEntry = new ManagedUserEntry(proxiedUser, ldapObject, ldapTransaction);
managedUsers.put(userId, newEntry);
}
public void removeManagedUserEntry(String userId) {
managedUsers.remove(userId);
}
private static class ManagedUserEntry {
private final UserModel managedProxiedUser;
private final LDAPObject ldapUser;
private final LDAPTransaction ldapTransaction;
public ManagedUserEntry(UserModel managedProxiedUser, LDAPObject ldapUser, LDAPTransaction ldapTransaction) {
this.managedProxiedUser = managedProxiedUser;
this.ldapUser = ldapUser;
this.ldapTransaction = ldapTransaction;
}
public UserModel getManagedProxiedUser() {
return managedProxiedUser;
}
public LDAPObject getLdapUser() {
return ldapUser;
}
public LDAPTransaction getLdapTransaction() {
return ldapTransaction;
}
}
}

View file

@ -0,0 +1,95 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.storage.ldap.mappers;
import org.jboss.logging.Logger;
import org.keycloak.models.KeycloakTransaction;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPTransaction implements KeycloakTransaction {
public static final Logger logger = Logger.getLogger(LDAPTransaction.class);
protected TransactionState state = TransactionState.NOT_STARTED;
private final LDAPStorageProvider ldapProvider;
private final LDAPObject ldapUser;
public LDAPTransaction(LDAPStorageProvider ldapProvider, LDAPObject ldapUser) {
this.ldapProvider = ldapProvider;
this.ldapUser = ldapUser;
}
@Override
public void begin() {
if (state != TransactionState.NOT_STARTED) {
throw new IllegalStateException("Transaction already started");
}
state = TransactionState.STARTED;
}
@Override
public void commit() {
if (state != TransactionState.STARTED) {
throw new IllegalStateException("Transaction in illegal state for commit: " + state);
}
if (logger.isTraceEnabled()) {
logger.trace("Transaction commit! Updating LDAP attributes for object " + ldapUser.getDn().toString() + ", attributes: " + ldapUser.getAttributes());
}
ldapProvider.getLdapIdentityStore().update(ldapUser);
state = TransactionState.FINISHED;
}
@Override
public void rollback() {
if (state != TransactionState.STARTED && state != TransactionState.ROLLBACK_ONLY) {
throw new IllegalStateException("Transaction in illegal state for rollback: " + state);
}
logger.warn("Transaction rollback! Ignoring LDAP updates for object " + ldapUser.getDn().toString());
state = TransactionState.FINISHED;
}
@Override
public void setRollbackOnly() {
state = TransactionState.ROLLBACK_ONLY;
}
@Override
public boolean getRollbackOnly() {
return state == TransactionState.ROLLBACK_ONLY;
}
@Override
public boolean isActive() {
return state == TransactionState.STARTED || state == TransactionState.ROLLBACK_ONLY;
}
protected enum TransactionState {
NOT_STARTED, STARTED, ROLLBACK_ONLY, FINISHED
}
}

View file

@ -18,7 +18,6 @@
package org.keycloak.storage.ldap.mappers;
import org.jboss.logging.Logger;
import org.keycloak.models.KeycloakTransaction;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.storage.ldap.LDAPStorageProvider;
@ -33,39 +32,16 @@ public abstract class TxAwareLDAPUserModelDelegate extends UserModelDelegate {
protected LDAPStorageProvider provider;
protected LDAPObject ldapUser;
private final LDAPTransaction transaction;
public TxAwareLDAPUserModelDelegate(UserModel delegate, LDAPStorageProvider provider, LDAPObject ldapUser) {
super(delegate);
this.provider = provider;
this.ldapUser = ldapUser;
this.transaction = findOrCreateTransaction();
}
public LDAPTransaction getTransaction() {
return transaction;
}
// Try to find transaction in any delegate. We want to enlist just single transaction per all delegates
private LDAPTransaction findOrCreateTransaction() {
UserModelDelegate delegate = this;
while (true) {
UserModel deleg = delegate.getDelegate();
if (!(deleg instanceof UserModelDelegate)) {
return new LDAPTransaction();
} else {
delegate = (UserModelDelegate) deleg;
}
if (delegate instanceof TxAwareLDAPUserModelDelegate) {
TxAwareLDAPUserModelDelegate txDelegate = (TxAwareLDAPUserModelDelegate) delegate;
return txDelegate.getTransaction();
}
}
}
protected void ensureTransactionStarted() {
if (transaction.state == TransactionState.NOT_STARTED) {
LDAPTransaction transaction = provider.getUserManager().getTransaction(getId());
if (transaction.state == LDAPTransaction.TransactionState.NOT_STARTED) {
if (logger.isTraceEnabled()) {
logger.trace("Starting and enlisting transaction for object " + ldapUser.getDn().toString());
}
@ -74,63 +50,4 @@ public abstract class TxAwareLDAPUserModelDelegate extends UserModelDelegate {
}
}
protected class LDAPTransaction implements KeycloakTransaction {
protected TransactionState state = TransactionState.NOT_STARTED;
@Override
public void begin() {
if (state != TransactionState.NOT_STARTED) {
throw new IllegalStateException("Transaction already started");
}
state = TransactionState.STARTED;
}
@Override
public void commit() {
if (state != TransactionState.STARTED) {
throw new IllegalStateException("Transaction in illegal state for commit: " + state);
}
if (logger.isTraceEnabled()) {
logger.trace("Transaction commit! Updating LDAP attributes for object " + ldapUser.getDn().toString() + ", attributes: " + ldapUser.getAttributes());
}
provider.getLdapIdentityStore().update(ldapUser);
state = TransactionState.FINISHED;
}
@Override
public void rollback() {
if (state != TransactionState.STARTED && state != TransactionState.ROLLBACK_ONLY) {
throw new IllegalStateException("Transaction in illegal state for rollback: " + state);
}
logger.warn("Transaction rollback! Ignoring LDAP updates for object " + ldapUser.getDn().toString());
state = TransactionState.FINISHED;
}
@Override
public void setRollbackOnly() {
state = TransactionState.ROLLBACK_ONLY;
}
@Override
public boolean getRollbackOnly() {
return state == TransactionState.ROLLBACK_ONLY;
}
@Override
public boolean isActive() {
return state == TransactionState.STARTED || state == TransactionState.ROLLBACK_ONLY;
}
}
protected enum TransactionState {
NOT_STARTED, STARTED, ROLLBACK_ONLY, FINISHED
}
}

View file

@ -33,6 +33,7 @@ import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery;
import org.keycloak.storage.ldap.mappers.AbstractLDAPStorageMapper;
import org.keycloak.storage.ldap.mappers.LDAPOperationDecorator;
import org.keycloak.storage.ldap.mappers.PasswordUpdateCallback;
import org.keycloak.storage.ldap.mappers.TxAwareLDAPUserModelDelegate;
import javax.naming.AuthenticationException;
import java.util.HashSet;
@ -101,7 +102,7 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
control.remove(UserAccountControl.ACCOUNTDISABLE);
}
updateUserAccountControl(ldapUser, control);
updateUserAccountControl(true, ldapUser, control);
}
@Override
@ -187,23 +188,26 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
return new UserAccountControl(longValue);
}
// Update user in LDAP
protected void updateUserAccountControl(LDAPObject ldapUser, UserAccountControl accountControl) {
// Update user in LDAP if "updateInLDAP" is true. Otherwise it is assumed that LDAP update will be called at the end of transaction
protected void updateUserAccountControl(boolean updateInLDAP, LDAPObject ldapUser, UserAccountControl accountControl) {
String userAccountControlValue = String.valueOf(accountControl.getValue());
logger.debugf("Updating userAccountControl of user '%s' to value '%s'", ldapUser.getDn().toString(), userAccountControlValue);
ldapUser.setSingleAttribute(LDAPConstants.USER_ACCOUNT_CONTROL, userAccountControlValue);
ldapProvider.getLdapIdentityStore().update(ldapUser);
if (updateInLDAP) {
ldapProvider.getLdapIdentityStore().update(ldapUser);
}
}
public class MSADUserModelDelegate extends UserModelDelegate {
public class MSADUserModelDelegate extends TxAwareLDAPUserModelDelegate {
private final LDAPObject ldapUser;
public MSADUserModelDelegate(UserModel delegate, LDAPObject ldapUser) {
super(delegate);
super(delegate, ldapProvider, ldapUser);
this.ldapUser = ldapUser;
}
@ -235,7 +239,9 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
control.add(UserAccountControl.ACCOUNTDISABLE);
}
updateUserAccountControl(ldapUser, control);
ensureTransactionStarted();
updateUserAccountControl(false, ldapUser, control);
}
}
@ -257,7 +263,8 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
ldapUser.removeReadOnlyAttributeName(LDAPConstants.PWD_LAST_SET);
ldapUser.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "0");
ldapProvider.getLdapIdentityStore().update(ldapUser);
ensureTransactionStarted();
}
}
@ -283,7 +290,8 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
ldapUser.removeReadOnlyAttributeName(LDAPConstants.PWD_LAST_SET);
ldapUser.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "-1");
ldapProvider.getLdapIdentityStore().update(ldapUser);
ensureTransactionStarted();
}
}
}

View file

@ -48,7 +48,7 @@ public class MSADUserAccountControlStorageMapperFactory extends AbstractLDAPStor
return ProviderConfigurationBuilder.create()
.property().name(MSADUserAccountControlStorageMapper.LDAP_PASSWORD_POLICY_HINTS_ENABLED)
.label("Password Policy Hints Enabled")
.helpText("Applicable just for writable MSAD. If on, then updating password in MSAD will use LDAP_SERVER_POLICY_HINTS_OID " +
.helpText("Applicable just for writable MSAD. If on, then updating password of MSAD user will use LDAP_SERVER_POLICY_HINTS_OID " +
"extension, which means that advanced MSAD password policies like 'password history' or 'minimal password age' will be applied. This extension works just for MSAD 2008 R2 or newer.")
.type(ProviderConfigProperty.BOOLEAN_TYPE)
.defaultValue("false")

View file

@ -17,6 +17,7 @@
package org.keycloak.testsuite.federation.storage.ldap;
import org.jboss.logging.Logger;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.FixMethodOrder;
@ -74,6 +75,8 @@ import static org.junit.Assert.assertEquals;
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class LDAPProvidersIntegrationTest {
private static final Logger log = Logger.getLogger(LDAPProvidersIntegrationTest.class);
private static LDAPRule ldapRule = new LDAPRule();
private static ComponentModel ldapModel = null;
@ -388,6 +391,10 @@ public class LDAPProvidersIntegrationTest {
Assert.assertNotNull(user);
Assert.assertNotNull(user.getFederationLink());
Assert.assertEquals(user.getFederationLink(), ldapModel.getId());
Assert.assertEquals("registerusersuccess2", user.getUsername());
Assert.assertEquals("firstName", user.getFirstName());
Assert.assertEquals("lastName", user.getLastName());
Assert.assertTrue(user.isEnabled());
} finally {
keycloakRule.stopSession(session, false);
}

View file

@ -0,0 +1,172 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.testsuite.federation.storage.ldap;
import java.util.Map;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.FixMethodOrder;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
import org.junit.runners.MethodSorters;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.storage.UserStorageProvider;
import org.keycloak.storage.UserStorageProviderModel;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.LDAPStorageProviderFactory;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
import org.keycloak.testsuite.OAuthClient;
import org.keycloak.testsuite.pages.AccountPasswordPage;
import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.pages.RegisterPage;
import org.keycloak.testsuite.rule.KeycloakRule;
import org.keycloak.testsuite.rule.LDAPRule;
import org.keycloak.testsuite.rule.WebResource;
import org.keycloak.testsuite.rule.WebRule;
import org.openqa.selenium.WebDriver;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class MSADMapperTest {
// Run this test just on MSAD
private static LDAPRule ldapRule = new LDAPRule((Map<String, String> ldapConfig) -> {
String vendor = ldapConfig.get(LDAPConstants.VENDOR);
return !(vendor.equals(LDAPConstants.VENDOR_ACTIVE_DIRECTORY));
});
private static ComponentModel ldapModel = null;
private static KeycloakRule keycloakRule = new KeycloakRule(new KeycloakRule.KeycloakSetup() {
@Override
public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
LDAPTestUtils.addLocalUser(manager.getSession(), appRealm, "marykeycloak", "mary@test.com", "password-app");
MultivaluedHashMap<String,String> ldapConfig = LDAPTestUtils.getLdapRuleConfig(ldapRule);
ldapConfig.putSingle(LDAPConstants.SYNC_REGISTRATIONS, "true");
ldapConfig.putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.WRITABLE.toString());
UserStorageProviderModel model = new UserStorageProviderModel();
model.setLastSync(0);
model.setChangedSyncPeriod(-1);
model.setFullSyncPeriod(-1);
model.setName("test-ldap");
model.setPriority(0);
model.setProviderId(LDAPStorageProviderFactory.PROVIDER_NAME);
model.setConfig(ldapConfig);
ldapModel = appRealm.addComponentModel(model);
LDAPTestUtils.addZipCodeLDAPMapper(appRealm, ldapModel);
// Delete all LDAP users and add some new for testing
LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
LDAPTestUtils.removeAllLDAPUsers(ldapFedProvider, appRealm);
LDAPObject john = LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnkeycloak", "John", "Doe", "john@email.org", null, "1234");
LDAPTestUtils.updateLDAPPassword(ldapFedProvider, john, "Password1");
appRealm.getClientByClientId("test-app").setDirectAccessGrantsEnabled(true);
}
});
@ClassRule
public static TestRule chain = RuleChain
.outerRule(ldapRule)
.around(keycloakRule);
@Rule
public WebRule webRule = new WebRule(this);
@WebResource
protected OAuthClient oauth;
@WebResource
protected WebDriver driver;
@WebResource
protected AppPage appPage;
@WebResource
protected RegisterPage registerPage;
@WebResource
protected LoginPage loginPage;
@WebResource
protected AccountUpdateProfilePage profilePage;
@WebResource
protected AccountPasswordPage changePasswordPage;
@WebResource
protected LoginPasswordUpdatePage passwordUpdatePage;
@Test
public void test01RegisterUserWithWeakPasswordFirst() {
loginPage.open();
loginPage.clickRegister();
registerPage.assertCurrent();
// Weak password. This will fail to update password to MSAD due to password policy.
registerPage.register("firstName", "lastName", "email2@check.cz", "registerUserSuccess2", "password", "password");
// Another weak password
passwordUpdatePage.assertCurrent();
passwordUpdatePage.changePassword("pass", "pass");
Assert.assertEquals("Invalid password: new password doesn't match password policies.", passwordUpdatePage.getError());
// Strong password. Successfully update password and being redirected to the app
passwordUpdatePage.changePassword("Password1", "Password1");
Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
KeycloakSession session = keycloakRule.startSession();
try {
RealmModel appRealm = session.realms().getRealmByName("test");
UserModel user = session.users().getUserByUsername("registerUserSuccess2", appRealm);
Assert.assertNotNull(user);
Assert.assertNotNull(user.getFederationLink());
Assert.assertEquals(user.getFederationLink(), ldapModel.getId());
Assert.assertEquals("registerusersuccess2", user.getUsername());
Assert.assertEquals("firstName", user.getFirstName());
Assert.assertEquals("lastName", user.getLastName());
Assert.assertTrue(user.isEnabled());
Assert.assertEquals(0, user.getRequiredActions().size());
} finally {
keycloakRule.stopSession(session, false);
}
}
}