Do the re-hash of password in a separate transaction to continue login in case of model exception

Closes #38970

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
Ricardo Martin 2025-04-25 09:20:16 +02:00 committed by GitHub
parent 75557bc312
commit 6e66a7e255
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 72 additions and 19 deletions

View File

@ -29,6 +29,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.credential.PasswordCredentialModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.policy.PasswordPolicyManagerProvider;
import org.keycloak.policy.PolicyError;
@ -279,13 +280,31 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
}
if (!provider.policyCheck(passwordPolicy, password)) {
int iterations = passwordPolicy != null ? passwordPolicy.getHashIterations() : -1;
final int iterations = passwordPolicy != null ? passwordPolicy.getHashIterations() : -1;
final String hashAlgorithm = passwordPolicy != null ? passwordPolicy.getHashAlgorithm() : null;
try {
// refresh the password in a different transaction, do not fail if there is a model exception
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), session.getContext(),
(KeycloakSession s) -> refreshPassword(s, hashAlgorithm, iterations, input.getChallengeResponse(),
password.getId(), password.getCreatedDate(), password.getUserLabel(), user.getId()));
} catch (ModelException e) {
logger.info("Error re-hashing the password in a different transaction", e);
}
}
}
PasswordCredentialModel newPassword = provider.encodedCredential(input.getChallengeResponse(), iterations);
newPassword.setId(password.getId());
newPassword.setCreatedDate(password.getCreatedDate());
newPassword.setUserLabel(password.getUserLabel());
user.credentialManager().updateStoredCredential(newPassword);
private static void refreshPassword(KeycloakSession s, String hashAlgorithm, int iterations, String challenge,
String passwordId, Long passwordDate, String passwordLabel, String userId) {
PasswordCredentialModel newPassword = ((hashAlgorithm != null)
? s.getProvider(PasswordHashProvider.class, hashAlgorithm)
: s.getProvider(PasswordHashProvider.class))
.encodedCredential(challenge, iterations);
newPassword.setId(passwordId);
newPassword.setCreatedDate(passwordDate);
newPassword.setUserLabel(passwordLabel);
UserModel userModel = s.users().getUserById(s.getContext().getRealm(), userId);
if (userModel != null) {
userModel.credentialManager().updateStoredCredential(newPassword);
}
}

View File

@ -1,6 +1,8 @@
package org.keycloak.testsuite.util;
import org.apache.http.client.RedirectStrategy;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.DefaultRedirectStrategy;
import org.apache.http.impl.client.HttpClientBuilder;
public class HttpClientUtils {
@ -8,12 +10,16 @@ public class HttpClientUtils {
private static final boolean SSL_REQUIRED = Boolean.parseBoolean(System.getProperty("auth.server.ssl.required"));
public static CloseableHttpClient createDefault() {
return createDefault(DefaultRedirectStrategy.INSTANCE);
}
public static CloseableHttpClient createDefault(RedirectStrategy redirectStrategy) {
if (SSL_REQUIRED) {
String keyStorePath = System.getProperty("client.certificate.keystore");
String keyStorePassword = System.getProperty("client.certificate.keystore.passphrase");
String trustStorePath = System.getProperty("client.truststore");
String trustStorePassword = System.getProperty("client.truststore.passphrase");
return MutualTLSUtils.newCloseableHttpClient(keyStorePath, keyStorePassword, trustStorePath, trustStorePassword);
return MutualTLSUtils.newCloseableHttpClient(keyStorePath, keyStorePassword, trustStorePath, trustStorePassword, redirectStrategy);
}
return HttpClientBuilder.create().build();
}

View File

@ -1,8 +1,10 @@
package org.keycloak.testsuite.util;
import org.apache.http.client.RedirectStrategy;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.DefaultRedirectStrategy;
import org.apache.http.impl.client.HttpClientBuilder;
import org.keycloak.common.util.Base64Url;
import org.keycloak.common.util.KeystoreUtil;
@ -61,6 +63,10 @@ public class MutualTLSUtils {
}
public static CloseableHttpClient newCloseableHttpClient(String keyStorePath, String keyStorePassword, String trustStorePath, String trustStorePassword) {
return newCloseableHttpClient(keyStorePath, keyStorePassword, trustStorePath, trustStorePassword, DefaultRedirectStrategy.INSTANCE);
}
public static CloseableHttpClient newCloseableHttpClient(String keyStorePath, String keyStorePassword, String trustStorePath, String trustStorePassword, RedirectStrategy redirectStrategy) {
KeyStore keystore = null;
// Load the keystore file
@ -83,13 +89,13 @@ public class MutualTLSUtils {
}
if (keystore != null || truststore != null) {
return newCloseableHttpClientSSL(keystore, keyStorePassword, truststore);
return newCloseableHttpClientSSL(keystore, keyStorePassword, truststore, redirectStrategy);
}
return HttpClientBuilder.create().build();
return HttpClientBuilder.create().setRedirectStrategy(redirectStrategy).build();
}
public static CloseableHttpClient newCloseableHttpClientSSL(KeyStore keystore, String keyStorePassword, KeyStore truststore) {
public static CloseableHttpClient newCloseableHttpClientSSL(KeyStore keystore, String keyStorePassword, KeyStore truststore, RedirectStrategy redirectStrategy) {
try {
SSLContext sslContext = SSLContext.getInstance("TLS");
KeyManagerFactory kmfactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
@ -99,7 +105,7 @@ public class MutualTLSUtils {
sslContext.init(kmfactory.getKeyManagers(), tmf.getTrustManagers(), null);
SSLConnectionSocketFactory sf = new SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE);
return HttpClientBuilder.create().setSSLSocketFactory(sf).build();
return HttpClientBuilder.create().setSSLSocketFactory(sf).setRedirectStrategy(redirectStrategy).build();
} catch (NoSuchAlgorithmException|KeyStoreException|KeyManagementException|UnrecoverableKeyException e) {
throw new RuntimeException(e);
}

View File

@ -36,7 +36,6 @@ import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.client.utils.URLEncodedUtils;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.client.LaxRedirectStrategy;
import org.apache.http.message.BasicNameValuePair;
import org.apache.http.util.EntityUtils;
@ -56,6 +55,8 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.common.util.Retry;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.util.ClientBuilder;
@ -76,7 +77,6 @@ import org.keycloak.util.JsonSerialization;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.keycloak.testsuite.util.ServerURLs.AUTH_SERVER_SSL_REQUIRED;
/**
* @author <a href="mailto:vramik@redhat.com">Vlastislav Ramik</a>
*/
@ -139,13 +139,35 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
}
}
protected CloseableHttpClient getHttpsAwareClient() {
HttpClientBuilder builder = HttpClientBuilder.create()
.setRedirectStrategy(new LaxRedirectStrategy());
if (AUTH_SERVER_SSL_REQUIRED) {
builder.setSSLHostnameVerifier((s, sslSession) -> true);
@Test
public void concurrentLoginSingleUserSingleClientRehash() throws Throwable {
log.info("*********************************************");
final RealmRepresentation realmRep = testRealm().toRepresentation();
try {
realmRep.setPasswordPolicy("hashAlgorithm(pbkdf2-sha256)");
testRealm().update(realmRep);
// change the password of the test user to the same to force re-hashing
CredentialRepresentation rep = new CredentialRepresentation();
rep.setTemporary(Boolean.FALSE);
rep.setValue("password");
rep.setType(CredentialRepresentation.PASSWORD);
ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost").resetPassword(rep);
} finally {
realmRep.setPasswordPolicy("");
testRealm().update(realmRep);
}
return builder.build();
// execute the login to re-hash in parallel
run(2, 10, (KeycloakRunnable) (int threadIndex, Keycloak keycloak, RealmResource realm) -> {
try (CloseableHttpClient httpClient = getHttpsAwareClient()) {
createHttpClientContextForUser(httpClient, "test-user@localhost", "password");
}
});
}
protected CloseableHttpClient getHttpsAwareClient() {
return HttpClientUtils.createDefault(LaxRedirectStrategy.INSTANCE);
}
protected HttpClientContext createHttpClientContextForUser(final CloseableHttpClient httpClient, String userName, String password) throws IOException {