8000 Sequential transactions instead of nested transactions by ahus1 · Pull Request #40224 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Sequential transactions instead of nested transactions #40224

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
Jun 5, 2025
Merged

Sequential transactions instead of nested transactions #40224

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
29 changes: 21 additions & 8 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 @@ -22,6 +22,7 @@
import org.jboss.logging.Logger;
import org.keycloak.common.util.Time;
import org.keycloak.credential.hash.PasswordHashProvider;
import org.keycloak.models.AbstractKeycloakTransaction;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelException;
import org.keycloak.models.PasswordPolicy;
Expand Down Expand Up @@ -282,14 +283,26 @@ private void rehashPasswordIfRequired(KeycloakSession session, RealmModel realm,
if (!provider.policyCheck(passwordPolicy, password)) {
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);
}
// Refresh the password in a different transaction, do not fail if there is a model exception on current modifications due to concurrent logins.
// Also do not start it as a nested transaction, as the current transaction might have auto-migrated the credential.
// see: JpaUserCredentialStore#toModel for the on-the-fly migration of the salt column
session.getTransactionManager().enlistAfterCompletion(new AbstractKeycloakTransaction() {
@Override
protected void commitImpl() {
try {
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);
}
}

@Override
protected void rollbackImpl() {

}
});
}
}

Expand Down
45 changes: 45 additions & 0 deletions ...arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.keycloak.testsuite.forms;

import jakarta.persistence.EntityManager;
import jakarta.ws.rs.BadRequestException;
import org.bouncycastle.crypto.generators.Argon2BytesGenerator;
import org.jboss.arquillian.graphene.page.Page;
Expand All @@ -24,6 +25,7 @@
import org.junit.Test;
import org.keycloak.common.crypto.FipsMode;
import org.keycloak.common.util.Base64;
import org.keycloak.connections.jpa.JpaConnectionProvider;
import org.keycloak.credential.CredentialModel;
import org.keycloak.credential.hash.PasswordHashProvider;
import org.keycloak.credential.hash.PasswordHashProviderFactory;
Expand All @@ -35,8 +37,10 @@
import org.keycloak.crypto.hash.Argon2PasswordHashProviderFactory;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.cache.UserCache;
import org.keycloak.models.credential.PasswordCredentialModel;
import org.keycloak.models.credential.dto.PasswordCredentialData;
import org.keycloak.models.jpa.entities.CredentialEntity;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.ErrorRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
Expand All @@ -52,6 +56,7 @@

import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import java.nio.charset.StandardCharsets;
import java.security.spec.KeySpec;
import java.time.Duration;
import java.util.List;
Expand Down Expand Up @@ -109,6 +114,42 @@ public void testPasswordRehashedOnAlgorithmChanged() throws Exception {

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

assertEquals(Pbkdf2PasswordHashProviderFactory.ID, credential.getPasswordCredentialData().getAlgorithm());
assertEncoded(credential, password, credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA1", 1);
}

@Test
public void testPasswordRehashedOnAlgorithmChangedWithMigratedSalt() throws Exception {
setPasswordPolicy("hashAlgorithm(" + Pbkdf2Sha256PasswordHashProviderFactory.ID + ") and hashIterations(1)");

String username = "testPasswordRehashedOnAlgorithmChangedWithMigratedSalt";
final String password = createUser(username);

PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

assertEquals(Pbkdf2Sha256PasswordHashProviderFactory.ID, credential.getPasswordCredentialData().getAlgorithm());

assertEncoded(credential, password, credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 1);

setPasswordPolicy("hashAlgorithm(" + Pbkdf2PasswordHashProviderFactory.ID + ") and hashIterations(1)");

String credentialId = credential.getId();
testingClient.server().run(session -> {
EntityManager em = session.getProvider(JpaConnectionProvider.class).getEntityManager();
CredentialEntity credentialEntity = em.find(CredentialEntity.class, credentialId);
// adding a dummy value to the salt column to trigger migration in JpaUserCredentialStore#toModel on next fetch of the credential
credentialEntity.setSalt("dummy".getBytes(StandardCharsets.UTF_8));
// Clearing the user cache as we updated the database directly
session.getProvider(UserCache.class).clear();
});

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

Expand All @@ -131,6 +172,7 @@ public void testPasswordRehashedToDefaultProviderIfHashAlgorithmRemoved() {

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

Expand All @@ -152,6 +194,7 @@ public void testPasswordRehashedOnIterationsChanged() throws Exception {

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

Expand Down Expand Up @@ -180,6 +223,7 @@ public void testPasswordNotRehasedUnchangedIterations() {

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

Expand All @@ -192,6 +236,7 @@ public void testPasswordNotRehasedUnchangedIterations() {

loginPage.open();
loginPage.login(username, password);
appPage.assertCurrent();

credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));

Expand Down
Loading
0