8000 Do the re-hash of password in a spearate transaction to continue login in case of model exception by rmartinc · Pull Request #39140 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 25, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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;

Expand Down Expand Up @@ -279,13 +280,31 @@ private void rehashPasswordIfRequired(KeycloakSession session, RealmModel realm,
}

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);
}
}

Expand Down
8 changes: 7 additions & 1 deletion ...tion-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/HttpClientUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
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 {

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();
}
Expand Down
14 changes: 10 additions & 4 deletions ...ation-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/MutualTLSUtils.java
View file 8000
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -61,6 +63,10 @@ public static CloseableHttpClient newCloseableHttpClientWithoutKeyStoreAndTrustS
}

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
Expand All @@ -83,13 +89,13 @@ public static CloseableHttpClient newCloseableHttpClient(String keyStorePath, St
}

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());
Expand All @@ -99,7 +105,7 @@ public static CloseableHttpClient newCloseableHttpClientSSL(KeyStore keystore, S
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);
}
Expand Down
38 changes: 30 additions & 8 deletions ...ests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
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;
Expand All @@ -56,6 +55,8 @@
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;
Expand All @@ -76,7 +77,6 @@

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>
*/
Expand Down Expand Up @@ -139,13 +139,35 @@ public void concurrentLoginSingleUser() throws Throwable {
}
}

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 {
Expand Down
Loading
0