From c66ff60c58e27c39c6a602ac5a772a17adf93d6c Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 17 Nov 2017 11:34:32 -0500 Subject: [PATCH 1/2] KEYCLOAK-5715 --- .../cache/infinispan/UserCacheSession.java | 6 ++++ .../hash/Pbkdf2PasswordHashProvider.java | 10 ++++++ .../credential/hash/PasswordHashProvider.java | 5 +++ .../AbstractUsernameFormAuthenticator.java | 33 +++++++++++++++++-- 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java index c209979a29..6522c879be 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java @@ -611,6 +611,8 @@ public class UserCacheSession implements UserCache { @Override public List getUsers(RealmModel realm, int firstResult, int maxResults) { + // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers + // method must be reimplemented when "localOnly" query parameter is true. return getUsers(realm, firstResult, maxResults, false); } @@ -621,6 +623,8 @@ public class UserCacheSession implements UserCache { @Override public List searchForUser(String search, RealmModel realm, int firstResult, int maxResults) { + // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers + // method must be reimplemented when "localOnly" query parameter is true. return getDelegate().searchForUser(search, realm, firstResult, maxResults); } @@ -631,6 +635,8 @@ public class UserCacheSession implements UserCache { @Override public List searchForUser(Map attributes, RealmModel realm, int firstResult, int maxResults) { + // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers + // method must be reimplemented when "localOnly" query parameter is true. return getDelegate().searchForUser(attributes, realm, firstResult, maxResults); } diff --git a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java index f010dd3729..e71ff6d842 100644 --- a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java @@ -73,6 +73,16 @@ public class Pbkdf2PasswordHashProvider implements PasswordHashProvider { credential.setValue(encodedPassword); } + @Override + public String encode(String rawPassword, int iterations) { + if (iterations == -1) { + iterations = defaultIterations; + } + + byte[] salt = getSalt(); + return encode(rawPassword, iterations, salt); + } + @Override public boolean verify(String rawPassword, CredentialModel credential) { return encode(rawPassword, credential.getHashIterations(), credential.getSalt()).equals(credential.getValue()); diff --git a/server-spi/src/main/java/org/keycloak/credential/hash/PasswordHashProvider.java b/server-spi/src/main/java/org/keycloak/credential/hash/PasswordHashProvider.java index ee555c2530..1745c4a9c2 100644 --- a/server-spi/src/main/java/org/keycloak/credential/hash/PasswordHashProvider.java +++ b/server-spi/src/main/java/org/keycloak/credential/hash/PasswordHashProvider.java @@ -29,5 +29,10 @@ public interface PasswordHashProvider extends Provider { void encode(String rawPassword, int iterations, CredentialModel credential); + default + String encode(String rawPassword, int iterations) { + return rawPassword; + } + boolean verify(String rawPassword, CredentialModel credential); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index a0f13bce47..72c10f5e22 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -22,9 +22,12 @@ import org.keycloak.authentication.AbstractFormAuthenticator; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.credential.CredentialInput; +import org.keycloak.credential.CredentialModel; +import org.keycloak.credential.hash.PasswordHashProvider; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.models.ModelDuplicateException; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; @@ -83,8 +86,32 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth return challengeResponse; } + protected void runDefaultDummyHash(AuthenticationFlowContext context) { + PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT); + hash.encode("dummypassword", PasswordPolicy.HASH_ITERATIONS_DEFAULT); + } + + protected void dummyHash(AuthenticationFlowContext context) { + PasswordPolicy policy = context.getRealm().getPasswordPolicy(); + if (policy == null) { + runDefaultDummyHash(context); + return; + } else { + PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, policy.getHashAlgorithm()); + if (hash == null) { + runDefaultDummyHash(context); + return; + + } else { + hash.encode("dummypassword", policy.getHashIterations()); + } + } + + } + public boolean invalidUser(AuthenticationFlowContext context, UserModel user) { if (user == null) { + dummyHash(context); context.getEvent().error(Errors.USER_NOT_FOUND); Response challengeResponse = invalidUser(context); context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse); @@ -144,15 +171,15 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth return false; } - if (invalidUser(context, user)){ + if (invalidUser(context, user)) { return false; } - if (!validatePassword(context, user, inputData)){ + if (!validatePassword(context, user, inputData)) { return false; } - if(!enabledUser(context, user)){ + if (!enabledUser(context, user)) { return false; } From 83ff0eab10d6b4d8e2dfe2933e16ff39aa9a9021 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 17 Nov 2017 11:36:49 -0500 Subject: [PATCH 2/2] remove irrelevant comments --- .../models/cache/infinispan/UserCacheSession.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java index 6522c879be..a773d9dfbf 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java @@ -611,9 +611,7 @@ public class UserCacheSession implements UserCache { @Override public List getUsers(RealmModel realm, int firstResult, int maxResults) { - // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers - // method must be reimplemented when "localOnly" query parameter is true. - return getUsers(realm, firstResult, maxResults, false); + return getUsers(realm, firstResult, maxResults, false); } @Override @@ -623,8 +621,6 @@ public class UserCacheSession implements UserCache { @Override public List searchForUser(String search, RealmModel realm, int firstResult, int maxResults) { - // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers - // method must be reimplemented when "localOnly" query parameter is true. return getDelegate().searchForUser(search, realm, firstResult, maxResults); } @@ -635,8 +631,6 @@ public class UserCacheSession implements UserCache { @Override public List searchForUser(Map attributes, RealmModel realm, int firstResult, int maxResults) { - // NOTE: If we ever end up caching this query, or the users returned by this query. The UsersResource.getUsers - // method must be reimplemented when "localOnly" query parameter is true. return getDelegate().searchForUser(attributes, realm, firstResult, maxResults); }