KEYCLOAK-19039 Sync UPDATE_PASSWORD required action to only to MSAD with WRITABLE edit mode. Add tests for MSAD mapper

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msad/MSADUserAccountControlStorageMapper.java

@@ -276,9 +276,6 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
 
         @Override
         public void addRequiredAction(String action) {
-            // Always update DB
-            super.addRequiredAction(action);
-
             if (ldapProvider.getEditMode() == UserStorageProvider.EditMode.WRITABLE && RequiredAction.UPDATE_PASSWORD.toString().equals(action)) {
                 MSADUserAccountControlStorageMapper.logger.debugf("Going to propagate required action UPDATE_PASSWORD to MSAD for ldap user '%s'. Keycloak user '%s' in realm '%s'",
                         ldapUser.getDn().toString(), getUsername(), getRealmName());
@@ -289,6 +286,10 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
                 ldapUser.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "0");
 
                 markUpdatedRequiredActionInTransaction(action);
+            } else {
+                // Update DB
+                MSADUserAccountControlStorageMapper.logger.debugf("Going to add required action '%s' of user '%s' in realm '%s' to the DB", action, getUsername(), getRealmName());
+                super.addRequiredAction(action);
             }
         }
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPMSADMapperTest.java

@@ -17,22 +17,34 @@
 
 package org.keycloak.testsuite.federation.ldap;
 
+import java.util.Collections;
+
+import org.hamcrest.Matchers;
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Assert;
 import org.junit.ClassRule;
 import org.junit.FixMethodOrder;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runners.MethodSorters;
+import org.keycloak.admin.client.resource.UserResource;
+import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
+import org.keycloak.representations.idm.UserRepresentation;
+import org.keycloak.storage.UserStorageProvider;
 import org.keycloak.storage.ldap.LDAPStorageProvider;
 import org.keycloak.storage.ldap.idm.model.LDAPObject;
+import org.keycloak.storage.ldap.mappers.msad.UserAccountControl;
+import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
 import org.keycloak.testsuite.util.LDAPRule;
 import org.keycloak.testsuite.util.LDAPTestConfiguration;
 import org.keycloak.testsuite.util.LDAPTestUtils;
 
+import static org.junit.Assert.assertFalse;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
@@ -42,13 +54,10 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
     // Run this test just on MSAD
     @ClassRule
     public static LDAPRule ldapRule = new LDAPRule()
+
             .assumeTrue((LDAPTestConfiguration ldapConfig) -> {
-
+                String vendor = ldapConfig.getLDAPConfig().get(LDAPConstants.VENDOR);
-                // TODO: This is skipped as it requires that MSAD server is set to not allow weak passwords (There needs to be pwdProperties=1 set on MSAD side).
+                return LDAPConstants.VENDOR_ACTIVE_DIRECTORY.equals(vendor);
-                // TODO: Currently we can't rely on it. See KEYCLOAK-4276
-                return false;
-                // return LDAPConstants.VENDOR_ACTIVE_DIRECTORY.equals(vendor);
-
             });
 
     @Override
@@ -79,36 +88,13 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
     }
 
 
-
-//    @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;
-//
-
     @Page
     protected LoginPasswordUpdatePage passwordUpdatePage;
 
 
+    // TODO: This is skipped as it requires that MSAD server is set to not allow weak passwords (There needs to be pwdProperties=1 set on MSAD side).
+    // TODO: Currently we can't rely on it. See KEYCLOAK-4276
+    @Ignore
     @Test
     public void test01RegisterUserWithWeakPasswordFirst() {
         loginPage.open();
@@ -142,4 +128,335 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
             Assert.assertEquals(0, user.getRequiredActionsStream().count());
         });
     }
+
+
+    @Test
+    public void test02UpdatePasswordTest() {
+        // Add required action to user johnkeycloak through Keycloak admin API
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        johnRep.setRequiredActions(Collections.singletonList(UserModel.RequiredAction.UPDATE_PASSWORD.name()));
+        john.update(johnRep);
+
+        // Check in LDAP, that johnkeycloak has pwdLastSet set to 0 in LDAP
+        Assert.assertEquals(0, getPwdLastSetOfJohn());
+
+        // Login as johnkeycloak and update password after login
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+
+        passwordUpdatePage.assertCurrent();
+        passwordUpdatePage.changePassword("Password1", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Check in LDAP, that johnkeycloak does not have pwdLastSet set to 0
+        Assert.assertThat(getPwdLastSetOfJohn(), Matchers.greaterThan(0L));
+
+        // Check in admin REST API, that johnkeycloak does not have required action on him
+        johnRep = john.toRepresentation();
+        Assert.assertTrue(johnRep.getRequiredActions().isEmpty());
+
+        // Logout and login again. There should not be a need to update required action anymore
+        john.logout();
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+    }
+
+
+    // KEYCLOAK-19039
+    @Test
+    public void test03UpdatePasswordWithLDAPDirectly() {
+        // Add required action to user johnkeycloak through Keycloak admin API
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        johnRep.setRequiredActions(Collections.singletonList(UserModel.RequiredAction.UPDATE_PASSWORD.name()));
+        john.update(johnRep);
+
+        // Check in LDAP, that johnkeycloak has pwdLastSet set to 0 in LDAP
+        Assert.assertEquals(0, getPwdLastSetOfJohn());
+
+        // Update password directly in MSAD
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+            LDAPTestUtils.updateLDAPPassword(ctx.getLdapProvider(), ldapJohn, "Password1");
+        });
+
+        // Check in LDAP, that johnkeycloak does not have pwdLastSet set to 0
+        Assert.assertThat(getPwdLastSetOfJohn(), Matchers.greaterThan(0L));
+
+        // Check in admin REST API, that johnkeycloak does not have required action on him
+        johnRep = john.toRepresentation();
+        Assert.assertTrue(johnRep.getRequiredActions().isEmpty());
+
+        // Logout and login again. There should not be a need to update required action anymore
+        john.logout();
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+    }
+
+
+    @Test
+    public void test04UpdateLDAPDirectlyToSetUpdatePassword() {
+        // Add required action to user johnkeycloak through Keycloak admin API
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+
+            ldapJohn.removeReadOnlyAttributeName(LDAPConstants.PWD_LAST_SET);
+            ldapJohn.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "0");
+            ctx.getLdapProvider().getLdapIdentityStore().update(ldapJohn);
+        });
+
+        // Check in LDAP, that johnkeycloak has pwdLastSet set to 0 in LDAP
+        Assert.assertEquals(0, getPwdLastSetOfJohn());
+
+        // Check Admin REST API contains UPDATE_PASSWORD required action
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        Assert.assertEquals(UserModel.RequiredAction.UPDATE_PASSWORD.name(), johnRep.getRequiredActions().get(0));
+
+        // Login as johnkeycloak and update password after login
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+
+        passwordUpdatePage.assertCurrent();
+        passwordUpdatePage.changePassword("Password1", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Check in LDAP, that johnkeycloak does not have pwdLastSet set to 0
+        Assert.assertThat(getPwdLastSetOfJohn(), Matchers.greaterThan(0L));
+
+        // Check in admin REST API, that johnkeycloak does not have required action on him
+        johnRep = john.toRepresentation();
+        Assert.assertTrue(johnRep.getRequiredActions().isEmpty());
+
+        // Logout and login again. There should not be a need to update required action anymore
+        john.logout();
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+    }
+
+
+    @Test
+    public void test05UpdatePasswordUnsyncedMode() {
+        // Switch edit mode to UNSYNCED
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.UNSYNCED.toString());
+            appRealm.updateComponent(ctx.getLdapModel());
+        });
+
+        // Add required action to user johnkeycloak through Keycloak admin API. Due UNSYNCED mode, this should update Keycloak DB, but not MSAD
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        johnRep.setRequiredActions(Collections.singletonList(UserModel.RequiredAction.UPDATE_PASSWORD.name()));
+        john.update(johnRep);
+
+        // Check in LDAP, that johnkeycloak has pwdLastSet set attribute set in MSAD to bigger value than 0. Previous update of requiredAction did not updated LDAP
+        long pwdLastSetFromLDAP = getPwdLastSetOfJohn();
+        Assert.assertThat(pwdLastSetFromLDAP, Matchers.greaterThan(0L));
+
+        // Login as johnkeycloak and update password after login
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+
+        passwordUpdatePage.assertCurrent();
+        passwordUpdatePage.changePassword("Password1", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Check in LDAP, that pwdLastSet attribute of MSAD user johnkeycloak did not change in MSAD
+        Assert.assertEquals(pwdLastSetFromLDAP, getPwdLastSetOfJohn());
+
+        // Check in admin REST API, that johnkeycloak does not have required action on him
+        johnRep = john.toRepresentation();
+        Assert.assertTrue(johnRep.getRequiredActions().isEmpty());
+
+        // Logout and login again. There should not be a need to update required action anymore
+        john.logout();
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Switch edit mode back to WRITABLE
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.WRITABLE.toString());
+            appRealm.updateComponent(ctx.getLdapModel());
+        });
+    }
+
+
+    @Test
+    public void test06RegisterNewUser() {
+        loginPage.open();
+        loginPage.clickRegister();
+        registerPage.assertCurrent();
+
+        // Register user
+        registerPage.register("firstName", "lastName", "email3@check.cz", "registeruser3", "Password1", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Check user enabled in MSAD
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+
+            String pwdLastSet = ldapJohn.getAttributeAsString(LDAPConstants.PWD_LAST_SET);
+            Assert.assertTrue(Long.parseLong(pwdLastSet) > 0);
+
+            String userAccountControl = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
+            long longValue = userAccountControl == null ? 0 : Long.parseLong(userAccountControl);
+
+            Assert.assertFalse(new UserAccountControl(longValue).has(UserAccountControl.ACCOUNTDISABLE));
+        });
+
+        // Logout and login again. Success
+        ApiUtil.findUserByUsernameId(adminClient.realm("test"), "registeruser3").logout();
+        loginPage.open();
+        loginPage.login("registeruser3", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+    }
+
+
+    @Test
+    public void test07DisabledUserInMSADSwitchedToEnabledInKeycloak() {
+        // Disable user in MSAD
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+
+            String userAccountControlStr = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
+            UserAccountControl control = new UserAccountControl(Long.parseLong(userAccountControlStr));
+            control.add(UserAccountControl.ACCOUNTDISABLE);
+
+            ldapJohn.setSingleAttribute(LDAPConstants.USER_ACCOUNT_CONTROL, String.valueOf(control.getValue()));
+            ctx.getLdapProvider().getLdapIdentityStore().update(ldapJohn);
+        });
+
+        // Check user disabled in both admin REST API and MSAD
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        Assert.assertFalse(johnRep.isEnabled());
+
+        Assert.assertFalse(isJohnEnabledInMSAD());
+
+        // Login as johnkeycloak, but user disabled
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals("Account is disabled, contact your administrator.", loginPage.getError());
+
+        // Enable user in admin REST API
+        johnRep.setEnabled(true);
+        john.update(johnRep);
+
+        // Assert user enabled also in MSAD
+        Assert.assertTrue(isJohnEnabledInMSAD());
+
+        // Logout and login again. There should not be a need to update required action anymore
+        john.logout();
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+    }
+
+
+    @Test
+    public void test08DisabledUserUnsyncedMode() {
+        // Switch edit mode to UNSYNCED
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.UNSYNCED.toString());
+            appRealm.updateComponent(ctx.getLdapModel());
+        });
+
+        // Disable user johnkeycloak through Keycloak admin API. Due UNSYNCED mode, this should update Keycloak DB, but not MSAD
+        UserResource john = ApiUtil.findUserByUsernameId(adminClient.realm("test"), "johnkeycloak");
+        UserRepresentation johnRep = john.toRepresentation();
+        johnRep.setEnabled(false);
+        john.update(johnRep);
+
+        // Check in LDAP, that johnkeycloak is still enabled in MSAD
+        Assert.assertTrue(isJohnEnabledInMSAD());
+
+        // Login as johnkeycloak and see the user is disabled
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals("Account is disabled, contact your administrator.", loginPage.getError());
+
+        // Enable johnkeycloak in admin REST API
+        johnRep.setEnabled(true);
+        john.update(johnRep);
+
+        // Check in LDAP, that johnkeycloak is still enabled in MSAD
+        Assert.assertTrue(isJohnEnabledInMSAD());
+
+        // Login again. User should be enabled
+        loginPage.open();
+        loginPage.login("johnkeycloak", "Password1");
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Switch edit mode back to WRITABLE
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.WRITABLE.toString());
+            appRealm.updateComponent(ctx.getLdapModel());
+        });
+    }
+
+    private long getPwdLastSetOfJohn() {
+        String pwdLastSett = testingClient.server().fetchString(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+
+            String pwdLastSet = ldapJohn.getAttributeAsString(LDAPConstants.PWD_LAST_SET);
+            return pwdLastSet;
+        });
+
+        if (pwdLastSett == null) {
+            Assert.fail("LDAP user johnkeycloak does not have pwdLastSet on him");
+        }
+
+        // Need to remove double quotes TODO: Ideally fix fetchString method and all the tests, which uses it as it is dummy to need to remove quotes in each test individually...
+        return Long.parseLong(pwdLastSett.replace("\"",""));
+    }
+
+
+    private boolean isJohnEnabledInMSAD() {
+        String userAccountControls = testingClient.server().fetchString(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+            LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
+
+            String userAccountControl = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
+            return userAccountControl;
+        });
+
+        if (userAccountControls == null) {
+            Assert.fail("LDAP user johnkeycloak does not have userAccountControl attribute on him");
+        }
+
+        // Need to remove double quotes TODO: Ideally fix fetchString method and all the tests, which uses it as it is dummy to need to remove quotes in each test individually...
+        UserAccountControl acControl = new UserAccountControl(Long.parseLong(userAccountControls.replace("\"","")));
+        return !acControl.has(UserAccountControl.ACCOUNTDISABLE);
+    }
 }