From f95933211799c358c6b045a90220f0e5e7cb9786 Mon Sep 17 00:00:00 2001 From: girirajsharma Date: Thu, 16 Apr 2015 23:41:45 +0530 Subject: [PATCH] [KEYCLOAK-402] - Force password changes at regular intervals --- .../modules/security-vulnerabilities.xml | 5 +- .../theme/base/admin/resources/js/services.js | 6 +- .../org/keycloak/models/PasswordPolicy.java | 37 ++++++++ .../models/UserCredentialValueModel.java | 6 +- .../models/entities/CredentialEntity.java | 6 +- .../models/file/adapter/UserAdapter.java | 5 +- .../org/keycloak/models/jpa/UserAdapter.java | 3 +- .../models/jpa/entities/CredentialEntity.java | 6 +- .../mongo/keycloak/adapters/UserAdapter.java | 3 +- .../managers/AuthenticationManager.java | 28 ++++++ .../keycloak/testsuite/forms/LoginTest.java | 87 +++++++++++++++++++ .../testsuite/forms/ResetPasswordTest.java | 38 +++++--- 12 files changed, 202 insertions(+), 28 deletions(-) diff --git a/docbook/reference/en/en-US/modules/security-vulnerabilities.xml b/docbook/reference/en/en-US/modules/security-vulnerabilities.xml index 0756ebdfc4..0c4bee34d5 100755 --- a/docbook/reference/en/en-US/modules/security-vulnerabilities.xml +++ b/docbook/reference/en/en-US/modules/security-vulnerabilities.xml @@ -128,9 +128,10 @@ In the admin console, per realm, you can set up a password policy to enforce that users pick hard to guess passwords. A password has to match all policies. The password policies that can be configured are hash iterations, length, digits, - lowercase, uppercase, special characters, not username, regex patterns and expired passwords. Expired Passwords policy + lowercase, uppercase, special characters, not username, regex patterns, password history and force expired password update. + Force expired password update policy forces or requires password updates after specified span of time. Password history policy restricts a user from resetting his password to N old expired passwords. Multiple regex patterns, separated by comma, - can be specified. If there's more than one regex added, password has to match all fully. + can be specified in regex pattern policy. If there's more than one regex added, password has to match all fully. Increasing number of Hash Iterations (n) does not worsen anything (and certainly not the cipher) and it greatly increases the resistance to dictionary attacks. However the drawback to increasing n is that it has some cost (CPU usage, energy, delay) for the legitimate parties. Increasing n also slightly increases the odds that a random password gives the same result as the right diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/services.js b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/services.js index df05a13ffc..3d1cd8d660 100755 --- a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/services.js +++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/services.js @@ -908,7 +908,8 @@ module.factory('PasswordPolicy', function() { specialChars: "Minimal number (integer type) of special characters in password. Default value is 1.", notUsername: "Block passwords that are equal to the username", regexPatterns: "Block passwords that do not match all of the regex patterns (string type).", - passwordHistory: "Block passwords that are equal to previous passwords. Default value is 3." + passwordHistory: "Block passwords that are equal to previous passwords. Default value is 3.", + forceExpiredPasswordChange: "Force password change when password credential is expired. Default value is 365 days." } p.allPolicies = [ @@ -920,7 +921,8 @@ module.factory('PasswordPolicy', function() { { name: 'specialChars', value: 1 }, { name: 'notUsername', value: 1 }, { name: 'regexPatterns', value: ''}, - { name: 'passwordHistory', value: 3 } + { name: 'passwordHistory', value: 3 }, + { name: 'forceExpiredPasswordChange', value: 365 } ]; p.parse = function(policyString) { diff --git a/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java b/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java index 59d287754c..ed2282c108 100755 --- a/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java +++ b/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java @@ -78,6 +78,8 @@ public class PasswordPolicy { list.add(new RegexPatterns(args)); } else if (name.equals(PasswordHistory.NAME)) { list.add(new PasswordHistory(args)); + } else if (name.equals(ForceExpiredPasswordChange.NAME)) { + list.add(new ForceExpiredPasswordChange(args)); } } return list; @@ -114,6 +116,22 @@ public class PasswordPolicy { } return -1; } + + /** + * + * @return -1 if no force expired password change setting + */ + public int getDaysToExpirePassword() { + if (policies == null) + return -1; + for (Policy p : policies) { + if (p instanceof ForceExpiredPasswordChange) { + return ((ForceExpiredPasswordChange) p).daysToExpirePassword; + } + + } + return -1; + } public Error validate(UserModel user, String password) { for (Policy p : policies) { @@ -418,6 +436,25 @@ public class PasswordPolicy { } } + private static class ForceExpiredPasswordChange implements Policy { + private static final String NAME = "forceExpiredPasswordChange"; + private int daysToExpirePassword; + + public ForceExpiredPasswordChange(String[] args) { + daysToExpirePassword = intArg(NAME, 365, args); + } + + @Override + public Error validate(String username, String password) { + return null; + } + + @Override + public Error validate(UserModel user, String password) { + return null; + } + } + private static int intArg(String policy, int defaultValue, String... args) { if (args == null || args.length == 0) { return defaultValue; diff --git a/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java b/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java index 663eaeca19..4654ba5b60 100755 --- a/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java +++ b/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java @@ -12,7 +12,7 @@ public class UserCredentialValueModel { private String device; private byte[] salt; private int hashIterations; - private long createdDate; + private Long createdDate; public String getType() { return type; @@ -54,11 +54,11 @@ public class UserCredentialValueModel { this.hashIterations = iterations; } - public long getCreatedDate() { + public Long getCreatedDate() { return createdDate; } - public void setCreatedDate(long createdDate) { + public void setCreatedDate(Long createdDate) { this.createdDate = createdDate; } diff --git a/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java b/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java index 5493fe24d5..08f8f90277 100755 --- a/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java +++ b/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java @@ -11,7 +11,7 @@ public class CredentialEntity { private String device; private byte[] salt; private int hashIterations; - private long createdDate; + private Long createdDate; private UserEntity user; @@ -63,11 +63,11 @@ public class CredentialEntity { this.hashIterations = hashIterations; } - public long getCreatedDate() { + public Long getCreatedDate() { return createdDate; } - public void setCreatedDate(long createdDate) { + public void setCreatedDate(Long createdDate) { this.createdDate = createdDate; } diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java index 236f95c2a2..1ae5f12237 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java @@ -17,6 +17,7 @@ package org.keycloak.models.file.adapter; import org.keycloak.models.ClientModel; + import static org.keycloak.models.utils.Pbkdf2PasswordEncoder.getSalt; import org.keycloak.models.PasswordPolicy; @@ -31,7 +32,6 @@ import org.keycloak.models.utils.Pbkdf2PasswordEncoder; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -43,6 +43,7 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.entities.FederatedIdentityEntity; import org.keycloak.models.entities.RoleEntity; import org.keycloak.models.entities.UserEntity; +import org.keycloak.util.Time; /** * UserModel for JSON persistence. @@ -271,7 +272,6 @@ public class UserAdapter implements UserModel, Comparable { private CredentialEntity setCredentials(UserEntity user, UserCredentialModel cred) { CredentialEntity credentialEntity = new CredentialEntity(); credentialEntity.setType(cred.getType()); - credentialEntity.setCreatedDate(new Date().getTime()); credentialEntity.setDevice(cred.getDevice()); return credentialEntity; } @@ -285,6 +285,7 @@ public class UserAdapter implements UserModel, Comparable { if (hashIterations == -1) hashIterations = 1; } + credentialEntity.setCreatedDate(Time.toMillis(Time.currentTime())); credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue(), hashIterations)); credentialEntity.setSalt(salt); credentialEntity.setHashIterations(hashIterations); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index 2216d224b7..22796a454e 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -15,6 +15,7 @@ import org.keycloak.models.jpa.entities.UserRequiredActionEntity; import org.keycloak.models.jpa.entities.UserRoleMappingEntity; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.Pbkdf2PasswordEncoder; +import org.keycloak.util.Time; import javax.persistence.EntityManager; import javax.persistence.TypedQuery; @@ -273,7 +274,6 @@ public class UserAdapter implements UserModel { CredentialEntity credentialEntity = new CredentialEntity(); credentialEntity.setId(KeycloakModelUtils.generateId()); credentialEntity.setType(cred.getType()); - credentialEntity.setCreatedDate(new Date().getTime()); credentialEntity.setDevice(cred.getDevice()); credentialEntity.setUser(user); return credentialEntity; @@ -288,6 +288,7 @@ public class UserAdapter implements UserModel { if (hashIterations == -1) hashIterations = 1; } + credentialEntity.setCreatedDate(Time.toMillis(Time.currentTime())); credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue(), hashIterations)); credentialEntity.setSalt(salt); credentialEntity.setHashIterations(hashIterations); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java index bdbfe84f42..998c4690b2 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java @@ -38,7 +38,7 @@ public class CredentialEntity { @Column(name="HASH_ITERATIONS") protected int hashIterations; @Column(name="CREATED_DATE") - protected long createdDate; + protected Long createdDate; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name="USER_ID") @@ -100,11 +100,11 @@ public class CredentialEntity { this.hashIterations = hashIterations; } - public long getCreatedDate() { + public Long getCreatedDate() { return createdDate; } - public void setCreatedDate(long createdDate) { + public void setCreatedDate(Long createdDate) { this.createdDate = createdDate; } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java index 1214323ed2..4d3c772bf2 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java @@ -16,6 +16,7 @@ import org.keycloak.models.mongo.keycloak.entities.MongoRoleEntity; import org.keycloak.models.mongo.keycloak.entities.MongoUserEntity; import org.keycloak.models.mongo.utils.MongoModelUtils; import org.keycloak.models.utils.Pbkdf2PasswordEncoder; +import org.keycloak.util.Time; import java.util.ArrayList; import java.util.Collections; @@ -239,7 +240,6 @@ public class UserAdapter extends AbstractMongoAdapter implement private CredentialEntity setCredentials(MongoUserEntity user, UserCredentialModel cred) { CredentialEntity credentialEntity = new CredentialEntity(); credentialEntity.setType(cred.getType()); - credentialEntity.setCreatedDate(new Date().getTime()); credentialEntity.setDevice(cred.getDevice()); return credentialEntity; } @@ -253,6 +253,7 @@ public class UserAdapter extends AbstractMongoAdapter implement if (hashIterations == -1) hashIterations = 1; } + credentialEntity.setCreatedDate(Time.toMillis(Time.currentTime())); credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue(), hashIterations)); credentialEntity.setSalt(salt); credentialEntity.setHashIterations(hashIterations); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index bd512df819..b146e8a654 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -19,6 +19,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RequiredCredentialModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserCredentialModel; +import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.UserSessionModel; @@ -41,12 +42,14 @@ import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.NewCookie; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; + import java.net.URI; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; /** * Stateless object that manages authentication @@ -375,6 +378,7 @@ public class AuthenticationManager { HttpRequest request, UriInfo uriInfo, EventBuilder event) { RealmModel realm = clientSession.getRealm(); UserModel user = userSession.getUser(); + isForcePasswordUpdateRequired(realm, user); isTotpConfigurationRequired(realm, user); isEmailVerificationRequired(realm, user); ClientModel client = clientSession.getClient(); @@ -434,6 +438,30 @@ public class AuthenticationManager { return redirectAfterSuccessfulFlow(session, realm , userSession, clientSession, request, uriInfo, clientConnection); } + + private static void isForcePasswordUpdateRequired(RealmModel realm, UserModel user) { + int daysToExpirePassword = realm.getPasswordPolicy().getDaysToExpirePassword(); + if(daysToExpirePassword != -1) { + for (UserCredentialValueModel entity : user.getCredentialsDirectly()) { + if (entity.getType().equals(UserCredentialModel.PASSWORD)) { + + if(entity.getCreatedDate() == null) { + user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); + logger.debug("User is required to update password"); + } else { + long timeElapsed = Time.toMillis(Time.currentTime()) - entity.getCreatedDate(); + long timeToExpire = TimeUnit.DAYS.toMillis(daysToExpirePassword); + + if(timeElapsed > timeToExpire) { + user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); + logger.debug("User is required to update password"); + } + } + break; + } + } + } + } protected static void isTotpConfigurationRequired(RealmModel realm, UserModel user) { for (RequiredCredentialModel c : realm.getRequiredCredentials()) { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java index e645ca32f7..644fff31bf 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java @@ -28,7 +28,9 @@ import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.events.Details; import org.keycloak.events.Event; +import org.keycloak.events.EventType; import org.keycloak.models.BrowserSecurityHeaders; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; @@ -39,6 +41,7 @@ import org.keycloak.testsuite.OAuthClient; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.rule.KeycloakRule; import org.keycloak.testsuite.rule.WebResource; import org.keycloak.testsuite.rule.WebRule; @@ -48,8 +51,10 @@ import org.openqa.selenium.WebDriver; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.core.Response; + import java.util.Map; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -93,6 +98,9 @@ public class LoginTest { @WebResource protected LoginPage loginPage; + + @WebResource + protected LoginPasswordUpdatePage updatePasswordPage; private static String userId; @@ -219,7 +227,86 @@ public class LoginTest { events.expectLogin().user(userId).removeDetail(Details.USERNAME).detail(Details.AUTH_METHOD, "sso").assertEvent(); } + + @Test + public void loginWithForcePasswordChangePolicy() { + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setPasswordPolicy(new PasswordPolicy("forceExpiredPasswordChange(1)")); + } + }); + + try { + // Setting offset to more than one day to force password update + // elapsedTime > timeToExpire + Time.setOffset(86405); + + loginPage.open(); + loginPage.login("login-test", "password"); + + updatePasswordPage.assertCurrent(); + + updatePasswordPage.changePassword("updatedPassword", "updatedPassword"); + + events.expectRequiredAction(EventType.UPDATE_PASSWORD).user(userId).detail(Details.USERNAME, "login-test").assertEvent(); + + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); + + } finally { + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setPasswordPolicy(new PasswordPolicy(null)); + + UserModel user = manager.getSession().users().getUserByUsername("login-test", appRealm); + UserCredentialModel cred = new UserCredentialModel(); + cred.setType(CredentialRepresentation.PASSWORD); + cred.setValue("password"); + user.updateCredential(cred); + } + }); + Time.setOffset(0); + } + } + + @Test + public void loginWithoutForcePasswordChangePolicy() { + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setPasswordPolicy(new PasswordPolicy("forceExpiredPasswordChange(1)")); + } + }); + + try { + // Setting offset to less than one day to avoid forced password update + // elapsedTime < timeToExpire + Time.setOffset(86205); + + loginPage.open(); + + loginPage.login("login-test", "password"); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + + events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); + + } finally { + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setPasswordPolicy(new PasswordPolicy(null)); + } + }); + Time.setOffset(0); + } + } + @Test public void loginNoTimeoutWithLongWait() { try { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 5bd1450fed..68abb4aba3 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -538,19 +538,35 @@ public class ResetPasswordTest { } }); - resetPassword("login-test", "password1"); - resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); + try { + Time.setOffset(2000000); + resetPassword("login-test", "password1"); + + resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); - resetPassword("login-test", "password2"); - resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); - resetPasswordInvalidPassword("login-test", "password2", "Invalid password: must not be equal to any of last 3 passwords."); + Time.setOffset(4000000); + resetPassword("login-test", "password2"); + + resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); + resetPasswordInvalidPassword("login-test", "password2", "Invalid password: must not be equal to any of last 3 passwords."); + + Time.setOffset(8000000); + resetPassword("login-test", "password3"); + + resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); + resetPasswordInvalidPassword("login-test", "password2", "Invalid password: must not be equal to any of last 3 passwords."); + resetPasswordInvalidPassword("login-test", "password3", "Invalid password: must not be equal to any of last 3 passwords."); - resetPassword("login-test", "password3"); - resetPasswordInvalidPassword("login-test", "password1", "Invalid password: must not be equal to any of last 3 passwords."); - resetPasswordInvalidPassword("login-test", "password2", "Invalid password: must not be equal to any of last 3 passwords."); - resetPasswordInvalidPassword("login-test", "password3", "Invalid password: must not be equal to any of last 3 passwords."); - - resetPassword("login-test", "password"); + resetPassword("login-test", "password"); + } finally { + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setPasswordPolicy(new PasswordPolicy(null)); + } + }); + Time.setOffset(0); + } } @Test