Merge pull request #62 from daniel-frak/password_policy_handling

Password policy handling
daniel-frak · Jun 15, 2022 · 4de28f2 · 4de28f2
2 parents f9d51b2 + 11d1da9
commit 4de28f2
Show file tree

Hide file tree

Showing 5 changed files with 215 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -163,7 +163,7 @@ The following example uses the default `master` realm but the demo will also wor
 1. Run `mvn clean package` in the repository root
 2. Navigate to `./docker`
 3. Execute `docker-compose up`
-4. Open [http://localhost:8024/auth/admin/](http://localhost:8024/auth/admin/) in a browser
+4. Open [http://localhost:8024/admin/](http://localhost:8024/admin/) in a browser
 5. Log in with the credentials:
 
 * User: `admin`
@@ -195,7 +195,7 @@ them automatically.
 
 ![Sign out from admin account](readme-images/sign-out-from-admin.png)
 
-2. Go to the [http://localhost:8024/auth/realms/master/account](http://localhost:8024/auth/realms/master/account) URI.
+2. Go to the [http://localhost:8024/realms/master/account](http://localhost:8024/realms/master/account) URI.
    Click the `Sign in` button to login as an example user:
 
 ![Welcome to Keycloak account](readme-images/welcome-to-keycloak-account.png)
@@ -216,7 +216,7 @@ Setting `requiredActions`, `groups`, `attributes` or `roles` is completely optio
 legacy system for illustration purposes only.
 
 4. The example user is successfully migrated. Log in again as admin
-   ([http://localhost:8024/auth/admin/](http://localhost:8024/auth/admin/)) and navigate to `Users` to verify the
+   ([http://localhost:8024/admin/](http://localhost:8024/admin/)) and navigate to `Users` to verify the
    results:
 
 ![Realm user list](readme-images/realm-user-list.png)

diff --git a/docker/e2e/cypress/integration/migrating_users.spec.js b/docker/e2e/cypress/integration/migrating_users.spec.js
@@ -127,7 +127,11 @@ describe('user migration plugin', () => {
 
     beforeEach(() => {
         deleteEmails();
-        deleteTestUserIfExists();
+        signInAsAdmin();
+        deleteTestUserIfExists().then(() => {
+            deletePasswordPoliciesIfExist()
+                .then(() => signOutViaUI());
+        });
     });
 
     function deleteEmails() {
@@ -137,23 +141,59 @@ describe('user migration plugin', () => {
     }
 
     function deleteTestUserIfExists() {
-        signInAsAdmin();
+        cy.log("Deleting test user...");
         cy.visit('/admin/master/console/#/realms/master/users');
+        getAllUsers();
+
+        return cy.get('td').contains(LEGACY_USER_EMAIL)
+            .should('have.length.gte', 0).then(userElement => {
+            if (!userElement.length) {
+                return;
+            }
+
+            cy.intercept('DELETE', '/admin/realms/master/users/**').as("userDelete");
+            cy.wrap(userElement).parent().contains('Delete').click();
+            cy.get('.modal-dialog button').contains('Delete').click();
+            cy.wait('@userDelete');
+            cy.get('.alert').should('contain', "Success");
+        });
+    }
+
+    function getAllUsers() {
         cy.intercept('GET', '/admin/realms/master/users*').as("userGet");
         cy.get('#viewAllUsers').click();
         cy.wait('@userGet');
+        cy.wait(1000);
+    }
 
-        cy.get('body').then($body => {
-            if ($body.find('td:contains("' + LEGACY_USER_EMAIL + '")').length > 0) {
-                cy.contains(LEGACY_USER_EMAIL).parent().contains('Delete').click();
-                cy.get('.modal-dialog button').contains('Delete').click();
-                cy.get('.alert').should('contain', "Success");
+    function deletePasswordPoliciesIfExist() {
+        goToPasswordPoliciesPage();
+        return deleteEveryPasswordPolicyAndSave();
+    }
+
+    function deleteEveryPasswordPolicyAndSave() {
+        cy.log("Deleting password policies...");
+        return cy.get('td[ng-click*="removePolicy"]')
+            .should('have.length.gte', 0).then(btn => {
+            if (!btn.length) {
+                return;
             }
-            signOutViaUI();
+            cy.wrap(btn).click({multiple: true});
+
+            cy.intercept('GET', '/admin/realms/master').as("masterPut");
+            cy.get('button').contains('Save').click();
+            cy.wait('@masterPut');
         });
     }
 
-    it('should migrate users', () => {
+    function goToPasswordPoliciesPage() {
+        cy.intercept('GET', '/admin/realms/master').as("masterGet");
+        cy.visit('/admin/master/console/#/realms/master/authentication/password-policy');
+        cy.wait('@masterGet');
+        cy.get("h1").should('contain', 'Authentication');
+    }
+
+    it('should migrate user', () => {
         signInAsLegacyUser();
         updateAccountInformation();
         assertIsLoggedInAsLegacyUser();
@@ -237,4 +277,29 @@ describe('user migration plugin', () => {
         triggerPasswordReset();
         resetPasswordViaEmail()
     });
+
+    it('should migrate user when password breaks policy', () => {
+        signInAsAdmin();
+        addSpecialCharactersPasswordPolicy();
+        signOutViaUI();
+
+        signInAsLegacyUser();
+        provideNewPassword();
+        updateAccountInformation();
+        assertIsLoggedInAsLegacyUser();
+    });
+
+    function addSpecialCharactersPasswordPolicy() {
+        cy.visit('/admin/master/console/#/realms/master/authentication/password-policy');
+        let policyDropdownSelector = 'select[ng-model="selectedPolicy"]';
+        cy.get(policyDropdownSelector).select('Special Characters');
+        cy.get('button').contains('Save').click();
+        cy.get('.alert').should('contain', "Your changes have been saved to the realm");
+    }
+
+    function provideNewPassword() {
+        cy.get('#password-new').type("pa$$word");
+        cy.get('#password-confirm').type("pa$$word");
+        cy.get("input").contains("Submit").click();
+    }
 });
diff --git a/pom.xml b/pom.xml
@@ -46,6 +46,14 @@
             <scope>provided</scope>
         </dependency>
 
+        <!-- https://mvnrepository.com/artifact/org.keycloak/keycloak-server-spi-private -->
+        <dependency>
+            <groupId>org.keycloak</groupId>
+            <artifactId>keycloak-server-spi-private</artifactId>
+            <version>${keycloak.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
         <!-- https://mvnrepository.com/artifact/org.jboss.logging/jboss-logging -->
         <dependency>
             <groupId>org.jboss.logging</groupId>

diff --git a/src/main/java/com/danielfrak/code/keycloak/providers/rest/LegacyProvider.java b/src/main/java/com/danielfrak/code/keycloak/providers/rest/LegacyProvider.java
@@ -12,6 +12,8 @@
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.credential.PasswordCredentialModel;
+import org.keycloak.policy.PasswordPolicyManagerProvider;
+import org.keycloak.policy.PolicyError;
 import org.keycloak.storage.UserStorageProvider;
 import org.keycloak.storage.user.UserLookupProvider;
 
@@ -69,12 +71,18 @@ public boolean isValid(RealmModel realmModel, UserModel userModel, CredentialInp
         }
 
         var userIdentifier = getUserIdentifier(userModel);
-        if (legacyUserService.isPasswordValid(userIdentifier, input.getChallengeResponse())) {
+
+        if (!legacyUserService.isPasswordValid(userIdentifier, input.getChallengeResponse())) {
+            return false;
+        }
+
+        if (passwordDoesNotBreakPolicy(realmModel, userModel, input.getChallengeResponse())) {
             session.userCredentialManager().updateCredential(realmModel, userModel, input);
-            return true;
+        } else {
+            addUpdatePasswordAction(userModel, userIdentifier);
         }
 
-        return false;
+        return true;
     }
 
     private String getUserIdentifier(UserModel userModel) {
@@ -83,6 +91,29 @@ private String getUserIdentifier(UserModel userModel) {
         return useUserId ? userModel.getId() : userModel.getUsername();
     }
 
+    private boolean passwordDoesNotBreakPolicy(RealmModel realmModel, UserModel userModel, String password) {
+        PasswordPolicyManagerProvider passwordPolicyManagerProvider = session.getProvider(
+                PasswordPolicyManagerProvider.class);
+        PolicyError error = passwordPolicyManagerProvider
+                .validate(realmModel, userModel, password);
+
+        return error == null;
+    }
+
+    private void addUpdatePasswordAction(UserModel userModel, String userIdentifier) {
+        if (updatePasswordActionMissing(userModel)) {
+            LOG.infof("Could not use legacy password for user %s due to password policy." +
+                            " Adding UPDATE_PASSWORD action.",
+                    userIdentifier);
+            userModel.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
+        }
+    }
+
+    private boolean updatePasswordActionMissing(UserModel userModel) {
+        return userModel.getRequiredActionsStream()
+                .noneMatch(s -> s.contains(UserModel.RequiredAction.UPDATE_PASSWORD.name()));
+    }
+
     @Override
     public boolean supportsCredentialType(String s) {
         return supportedCredentialTypes.contains(s);
@@ -105,11 +136,16 @@ public void close() {
 
     @Override
     public boolean updateCredential(RealmModel realm, UserModel user, CredentialInput input) {
+        severFederationLink(user);
+        return false;
+    }
+
+    private void severFederationLink(UserModel user) {
+        LOG.info("Severing federation link for " + user.getUsername());
         String link = user.getFederationLink();
         if (link != null && !link.isBlank()) {
             user.setFederationLink(null);
         }
-        return false;
     }
 
     @Override