libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

KEYCLOAK-12295 After password reset, the new password has low priority (#6653)

Browse source
This commit is contained in:
Tomas Kyjovsky 2020-01-16 09:11:25 +01:00 committed by Marek Posolda
parent 562dc3ff8c
commit 05c428f6e7
4 changed files with 267 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
common/src/main/java/org/keycloak/common/util/Time.java
View file

@ -39,7 +39,7 @@ public class Time {
* @return see description
*/
public static long currentTimeMillis() {
return System.currentTimeMillis() + (offset * 1000);
return System.currentTimeMillis() + (offset * 1000L);
}
/**
@ -48,7 +48,7 @@ public class Time {
* @return see description
*/
public static Date toDate(int time) {
return new Date(((long) time ) * 1000);
return new Date(time * 1000L);
}
/**
@ -66,7 +66,7 @@ public class Time {
* @return Time in milliseconds
*/
public static long toMillis(int time) {
return ((long) time) * 1000;
return time * 1000L;
}
/**

63
services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
View file

@ -32,7 +32,6 @@ import org.keycloak.models.cache.UserCache;
import org.keycloak.policy.PasswordPolicyManagerProvider;
import org.keycloak.policy.PolicyError;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Set;
@ -90,21 +89,46 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
@Override
public CredentialModel createCredential(RealmModel realm, UserModel user, PasswordCredentialModel credentialModel) {
PasswordPolicy policy = realm.getPasswordPolicy();
try {
expirePassword(realm, user, policy);
int expiredPasswordsPolicyValue = policy.getExpiredPasswords();
// 1) create new or reset existing password
CredentialModel createdCredential;
CredentialModel oldPassword = getPassword(realm, user);
if (credentialModel.getCreatedDate() == null) {
credentialModel.setCreatedDate(Time.currentTimeMillis());
}
CredentialModel createdCredential = getCredentialStore().createCredential(realm, user, credentialModel);
if (oldPassword == null) { // no password exists --> create new
createdCredential = getCredentialStore().createCredential(realm, user, credentialModel);
} else { // password exists --> update existing
credentialModel.setId(oldPassword.getId());
getCredentialStore().updateCredential(realm, user, credentialModel);
createdCredential = credentialModel;
// 2) add a password history item based on the old password
if (expiredPasswordsPolicyValue > 1) {
oldPassword.setId(null);
oldPassword.setType(PasswordCredentialModel.PASSWORD_HISTORY);
getCredentialStore().createCredential(realm, user, oldPassword);
}
}
// 3) remove old password history items
List<CredentialModel> passwordHistoryList = getCredentialStore().getStoredCredentialsByType(realm, user, PasswordCredentialModel.PASSWORD_HISTORY);
final int passwordHistoryListMaxSize = Math.max(0, expiredPasswordsPolicyValue - 1);
if (passwordHistoryList.size() > passwordHistoryListMaxSize) {
passwordHistoryList.stream()
.sorted(CredentialModel.comparingByStartDateDesc())
.skip(passwordHistoryListMaxSize)
.forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
}
UserCache userCache = session.userCache();
if (userCache != null) {
userCache.evict(realm, user);
}
return createdCredential;
} catch (IOException e) {
throw new RuntimeException(e);
}
}
@Override
@ -118,31 +142,6 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
}
protected void expirePassword(RealmModel realm, UserModel user, PasswordPolicy policy) throws IOException {
CredentialModel oldPassword = getPassword(realm, user);
if (oldPassword == null) return;
int expiredPasswordsPolicyValue = policy.getExpiredPasswords();
List<CredentialModel> list = getCredentialStore().getStoredCredentialsByType(realm, user, PasswordCredentialModel.PASSWORD_HISTORY);
if (expiredPasswordsPolicyValue > 1) {
// oldPassword will expire few lines below, and there is one active password,
// hence (expiredPasswordsPolicyValue - 2) passwords should be left in history
final int passwordsToLeave = expiredPasswordsPolicyValue - 2;
if (list.size() > passwordsToLeave) {
list.stream()
.sorted(CredentialModel.comparingByStartDateDesc())
.skip(passwordsToLeave)
.forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
}
oldPassword.setType(PasswordCredentialModel.PASSWORD_HISTORY);
getCredentialStore().updateCredential(realm, user, oldPassword);
} else {
list.stream().forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
getCredentialStore().removeStoredCredential(realm, user, oldPassword.getId());
}
}
protected PasswordHashProvider getHashProvider(PasswordPolicy policy) {
PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
if (hash == null) {

226
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordHistoryPolicyTest.java Normal file
View file

@ -0,0 +1,226 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.testsuite.policy;
import java.util.function.Consumer;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.AbstractAuthTest;
import static org.keycloak.representations.idm.CredentialRepresentation.PASSWORD;
import static org.keycloak.testsuite.admin.ApiUtil.getCreatedId;
/**
* @author <a href="mailto:tkyjovsk@redhat.com">Tomas Kyjovsky</a>
*/
public class PasswordHistoryPolicyTest extends AbstractAuthTest {
UserResource user;
private void setPasswordHistory(String passwordHistory) {
log.info(String.format("Setting %s", passwordHistory));
RealmRepresentation testRealmRepresentation = testRealmResource().toRepresentation();
testRealmRepresentation.setPasswordPolicy(passwordHistory);
testRealmResource().update(testRealmRepresentation);
}
private void setPasswordHistoryValue(String value) {
setPasswordHistory(String.format("passwordHistory(%s)", value));
}
private void setPasswordHistoryValue(int value) {
setPasswordHistoryValue(String.valueOf(value));
}
public UserRepresentation createUserRepresentation(String username) {
UserRepresentation userRepresentation = new UserRepresentation();
userRepresentation.setUsername(username);
userRepresentation.setEmail(String.format("%s@email.test", userRepresentation.getUsername()));
userRepresentation.setEmailVerified(true);
return userRepresentation;
}
public UserResource createUser(UserRepresentation user) {
String createdUserId;
try (Response response = testRealmResource().users().create(user)) {
createdUserId = getCreatedId(response);
}
return testRealmResource().users().get(createdUserId);
}
public void resetUserPassword(UserResource userResource, String newPassword) {
CredentialRepresentation newCredential = new CredentialRepresentation();
newCredential.setType(PASSWORD);
newCredential.setValue(newPassword);
newCredential.setTemporary(false);
userResource.resetPassword(newCredential);
}
private void expectBadRequestException(Consumer<Void> f) {
try {
f.accept(null);
throw new AssertionError("An expected BadRequestException was not thrown.");
} catch (BadRequestException bre) {
log.info("An expected BadRequestException was caught.");
}
}
@Before
public void before() {
user = createUser(createUserRepresentation("test_user"));
}
@After
public void after() {
user.remove();
}
@Test
public void testPasswordHistory_noHistory() {
setPasswordHistory("");
resetUserPassword(user, "secret"); // history:
resetUserPassword(user, "secret"); // history:
}
@Test
public void testPasswordHistory_length1() {
setPasswordHistoryValue(1);
resetUserPassword(user, "secret"); // history: secret
expectBadRequestException(f -> {
resetUserPassword(user, "secret");
});
resetUserPassword(user, "secret_2"); // history: secret_2
}
@Test
public void testPasswordHistory_length2() {
setPasswordHistoryValue(2);
resetUserPassword(user, "secret"); // history: secret
resetUserPassword(user, "secret_2"); // history: secret_2, secret
expectBadRequestException(f -> {
resetUserPassword(user, "secret_2");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret");
});
resetUserPassword(user, "secret_3"); // history: secret_3, secret_2
expectBadRequestException(f -> {
resetUserPassword(user, "secret_3");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret_2");
});
resetUserPassword(user, "secret"); // history: secret, secret_3
}
@Test
public void testPasswordHistory_length3() {
setPasswordHistoryValue(3);
resetUserPassword(user, "secret"); // history: secret
resetUserPassword(user, "secret_2"); // history: secret_2, secret
resetUserPassword(user, "secret_3"); // history: secret_3, secret_2, secret
expectBadRequestException(f -> {
resetUserPassword(user, "secret_3");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret_2");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret");
});
resetUserPassword(user, "secret_4"); // history: secret_4, secret_3, secret_2
expectBadRequestException(f -> {
resetUserPassword(user, "secret_4");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret_3");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret_2");
});
resetUserPassword(user, "secret"); // history: secret, secret_4, secret_3
}
@Test
public void testPasswordHistory_lengthChange() {
testPasswordHistory_length3(); // history: secret, secret_4, secret_3
setPasswordHistoryValue(2); // history: secret, secret_4
expectBadRequestException(f -> {
resetUserPassword(user, "secret");
});
expectBadRequestException(f -> {
resetUserPassword(user, "secret_4");
});
resetUserPassword(user, "secret_2"); // history: secret_2, secret
setPasswordHistoryValue(1); // history: secret_2
expectBadRequestException(f -> {
resetUserPassword(user, "secret_2");
});
resetUserPassword(user, "secret"); // history: secret
setPasswordHistory(""); // history:
resetUserPassword(user, "secret"); // history:
}
@Test
public void testInvalidPasswordHistoryPolicyValue_notANumber() {
expectBadRequestException(f -> {
setPasswordHistoryValue("abc");
});
expectBadRequestException(f -> {
setPasswordHistoryValue("2-");
});
expectBadRequestException(f -> {
setPasswordHistoryValue("-2-");
});
}
@Test
@Ignore("KEYCLOAK-12673")
public void testInvalidPasswordHistoryPolicyValue_zero() {
expectBadRequestException(f -> {
setPasswordHistoryValue(0);
});
}
@Test
@Ignore("KEYCLOAK-12673")
public void testInvalidPasswordHistoryPolicyValue_negative() {
expectBadRequestException(f -> {
setPasswordHistoryValue(-1);
});
expectBadRequestException(f -> {
setPasswordHistoryValue(-2);
});
expectBadRequestException(f -> {
setPasswordHistoryValue(-10);
});
}
}

1
testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/authentication/PasswordPolicyTest.java
View file

@ -166,7 +166,6 @@ public class PasswordPolicyTest extends AbstractConsoleTest {
}
@Test
@Ignore("Disabled until KEYCLOAK-11922 is resolved.")
public void testPasswordHistoryPolicy() {
RealmRepresentation realm = testRealmResource().toRepresentation();
realm.setPasswordPolicy("passwordHistory(2)");