From e7a5b88b2d6fa3b25b115eec20900b8a3525bd0e Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 26 Feb 2016 18:14:24 +0100 Subject: [PATCH] KEYCLOAK-2561 Fix issues with blank password --- .../hash/Pbkdf2PasswordHashProvider.java | 4 ++-- .../models/utils/CredentialValidation.java | 2 +- .../services/resources/AccountService.java | 2 +- .../services/resources/admin/UsersResource.java | 4 ++++ .../testsuite/account/ChangePasswordTest.java | 3 +++ .../org/keycloak/testsuite/admin/UserTest.java | 17 +++++++++++++++++ 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/server-spi/src/main/java/org/keycloak/hash/Pbkdf2PasswordHashProvider.java b/server-spi/src/main/java/org/keycloak/hash/Pbkdf2PasswordHashProvider.java index 7c1a3dbd65..3a25c5c87d 100644 --- a/server-spi/src/main/java/org/keycloak/hash/Pbkdf2PasswordHashProvider.java +++ b/server-spi/src/main/java/org/keycloak/hash/Pbkdf2PasswordHashProvider.java @@ -86,7 +86,7 @@ public class Pbkdf2PasswordHashProvider implements PasswordHashProviderFactory, byte[] key = getSecretKeyFactory().generateSecret(spec).getEncoded(); return Base64.encodeBytes(key); } catch (InvalidKeySpecException e) { - throw new RuntimeException("Credential could not be encoded"); + throw new RuntimeException("Credential could not be encoded", e); } } @@ -101,7 +101,7 @@ public class Pbkdf2PasswordHashProvider implements PasswordHashProviderFactory, try { return SecretKeyFactory.getInstance(PBKDF2_ALGORITHM); } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("PBKDF2 algorithm not found"); + throw new RuntimeException("PBKDF2 algorithm not found", e); } } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/CredentialValidation.java b/server-spi/src/main/java/org/keycloak/models/utils/CredentialValidation.java index 654c7644d3..074949f4ad 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/CredentialValidation.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/CredentialValidation.java @@ -71,7 +71,7 @@ public class CredentialValidation { public static boolean validateHashedCredential(KeycloakSession session, RealmModel realm, UserModel user, String unhashedCredValue, UserCredentialValueModel credential) { - if(unhashedCredValue == null){ + if (unhashedCredValue == null || unhashedCredValue.isEmpty()) { return false; } diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java index 653e266614..324136361f 100755 --- a/services/src/main/java/org/keycloak/services/resources/AccountService.java +++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java @@ -621,7 +621,7 @@ public class AccountService extends AbstractSecuredLocalService { } } - if (Validation.isEmpty(passwordNew)) { + if (Validation.isBlank(passwordNew)) { setReferrerOnPage(); return account.setError(Messages.MISSING_PASSWORD).createResponse(AccountPages.PASSWORD); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index e8e38ba491..dea9daf87c 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -93,6 +93,7 @@ import org.keycloak.services.managers.BruteForceProtector; import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.resources.AccountService; import org.keycloak.common.util.Time; +import org.keycloak.services.validation.Validation; /** * Base resource for managing users @@ -707,6 +708,9 @@ public class UsersResource { if (pass == null || pass.getValue() == null || !CredentialRepresentation.PASSWORD.equals(pass.getType())) { throw new BadRequestException("No password provided"); } + if (Validation.isBlank(pass.getValue())) { + throw new BadRequestException("Empty password not allowed"); + } UserCredentialModel cred = RepresentationToModel.convertCredential(pass); try { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ChangePasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ChangePasswordTest.java index 8e85307f0a..ce1900d01f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ChangePasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ChangePasswordTest.java @@ -62,6 +62,9 @@ public class ChangePasswordTest extends AbstractAccountManagementTest { testRealmChangePasswordPage.changePasswords(correctPassword, NEW_PASSWORD, NEW_PASSWORD + "-mismatch"); assertAlertError(); + + testRealmChangePasswordPage.changePasswords(correctPassword, " ", " "); + assertAlertError(); } @Test diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 07ed3897e6..b38b362a14 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -646,6 +646,23 @@ public class UserTest extends AbstractClientTest { assertEquals("Keycloak Account Management", driver.getTitle()); } + @Test + public void resetUserInvalidPassword() { + String userId = createUser("user1", "user1@localhost"); + + try { + CredentialRepresentation cred = new CredentialRepresentation(); + cred.setType(CredentialRepresentation.PASSWORD); + cred.setValue(" "); + cred.setTemporary(false); + realm.users().get(userId).resetPassword(cred); + fail("Expected failure"); + } catch (ClientErrorException e) { + assertEquals(400, e.getResponse().getStatus()); + e.getResponse().close(); + } + } + private void switchEditUsernameAllowedOn() { RealmRepresentation rep = realm.toRepresentation(); rep.setEditUsernameAllowed(true);