External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-10 15:32:05 -03:30
Code Issues Packages Projects Releases Wiki Activity

Don't validate duplicate credential label on update if label is unchanged

Browse Source 
Closes #41945

Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
This commit is contained in:
Alexander Schwartz 2025-08-20 08:06:06 +02:00 committed by GitHub
parent c7fedb77e3
commit 09f863bf9d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 144 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserCredentialStore.java
View File

@ -63,7 +63,11 @@ public class JpaUserCredentialStore implements UserCredentialStore {
public void updateCredential(RealmModel realm, UserModel user, CredentialModel cred) {
CredentialEntity entity = em.find(CredentialEntity.class, cred.getId());
if (!checkCredentialEntity(entity, user)) return;
validateDuplicateCredential(realm, user, cred.getUserLabel(), cred.getId());
if (!Objects.equals(cred.getUserLabel(), entity.getUserLabel())) {
// For legacy entries in the credentials, there might be a duplicate for historical reasons.
// Ignore them when the credential is updated, which might happen when credentials are verified.
validateDuplicateCredential(realm, user, cred.getUserLabel(), cred.getId());
}
entity.setCreatedDate(cred.getCreatedDate());
entity.setUserLabel(cred.getUserLabel());
entity.setType(cred.getType());

213
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/CredentialModelTest.java
View File

@ -1,11 +1,14 @@
package org.keycloak.testsuite.model;
import jakarta.persistence.EntityManager;
import org.junit.Test;
import org.keycloak.connections.jpa.JpaConnectionProvider;
import org.keycloak.credential.CredentialModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.credential.OTPCredentialModel;
import org.keycloak.models.jpa.entities.CredentialEntity;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
@ -53,93 +56,155 @@ public class CredentialModelTest extends AbstractTestRealmKeycloakTest {
otp2Id.set(otp2.getId());
});
try {
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: password, otp1, otp2
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, passwordId.get(), otp1Id.get(), otp2Id.get());
// Assert can't move password when newPreviousCredential not found
Assert.assertFalse(user.credentialManager().moveStoredCredentialTo(passwordId.get(), "not-known"));
// Assert can't move credential when not found
Assert.assertFalse(user.credentialManager().moveStoredCredentialTo("not-known", otp2Id.get()));
// Move otp2 up 1 position
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), passwordId.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: password, otp2, otp1
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, passwordId.get(), otp2Id.get(), otp1Id.get());
// Move otp2 to the top
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), null));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, password, otp1
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp2Id.get(), passwordId.get(), otp1Id.get());
// Move password down
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(passwordId.get(), otp1Id.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, otp1, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp2Id.get(), otp1Id.get(), passwordId.get());
// Remove otp2 down two positions
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), passwordId.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, otp1, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp1Id.get(), passwordId.get(), otp2Id.get());
// Remove password
Assert.assertTrue(user.credentialManager().removeStoredCredentialById(passwordId.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp1Id.get(), otp2Id.get());
});
} finally {
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
user.credentialManager().removeStoredCredentialById(otp1Id.get());
user.credentialManager().removeStoredCredentialById(otp2Id.get());
});
}
}
@Test
@ModelTest
public void testCredentialUpdateWithDuplicateLabel(KeycloakSession session) {
AtomicReference<String> otp1Id = new AtomicReference<>();
AtomicReference<String> otp2Id = new AtomicReference<>();
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: password, otp1, otp2
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, passwordId.get(), otp1Id.get(), otp2Id.get());
// Create 2 OTP credentials with the same label (as it was in pre-26.3)
CredentialModel otp1 = OTPCredentialModel.createFromPolicy(realm, "secret1", "label1");
CredentialModel otp2 = OTPCredentialModel.createFromPolicy(realm, "secret2", "label2");
otp1 = user.credentialManager().createStoredCredential(otp1);
otp2 = user.credentialManager().createStoredCredential(otp2);
otp1Id.set(otp1.getId());
otp2Id.set(otp2.getId());
// Assert can't move password when newPreviousCredential not found
Assert.assertFalse(user.credentialManager().moveStoredCredentialTo(passwordId.get(), "not-known"));
// Assert can't move credential when not found
Assert.assertFalse(user.credentialManager().moveStoredCredentialTo("not-known", otp2Id.get()));
// Move otp2 up 1 position
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), passwordId.get()));
// Fake a duplicate label by setting the label directly on the JPA level
EntityManager em = currentSession.getProvider(JpaConnectionProvider.class).getEntityManager();
CredentialEntity credentialEntity = em.find(CredentialEntity.class, otp2.getId());
credentialEntity.setUserLabel("label1");
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
try {
// Assert priorities: password, otp2, otp1
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, passwordId.get(), otp2Id.get(), otp1Id.get());
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
CredentialModel credential = user.credentialManager().getStoredCredentialById(otp1Id.get());
// Move otp2 to the top
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), null));
});
// must not throw an exception even when the two labels are the same
credential.setSecretData("newsecret");
user.credentialManager().updateStoredCredential(credential);
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
} finally {
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
user.credentialManager().removeStoredCredentialById(otp1Id.get());
user.credentialManager().removeStoredCredentialById(otp2Id.get());
});
}
// Assert priorities: otp2, password, otp1
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp2Id.get(), passwordId.get(), otp1Id.get());
// Move password down
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(passwordId.get(), otp1Id.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, otp1, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp2Id.get(), otp1Id.get(), passwordId.get());
// Remove otp2 down two positions
Assert.assertTrue(user.credentialManager().moveStoredCredentialTo(otp2Id.get(), passwordId.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, otp1, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp1Id.get(), passwordId.get(), otp2Id.get());
// Remove password
Assert.assertTrue(user.credentialManager().removeStoredCredentialById(passwordId.get()));
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession currentSession) -> {
RealmModel realm = currentSession.realms().getRealmByName("test");
currentSession.getContext().setRealm(realm);
UserModel user = currentSession.users().getUserByUsername(realm, "test-user@localhost");
// Assert priorities: otp2, password
List<CredentialModel> list = user.credentialManager().getStoredCredentialsStream()
.collect(Collectors.toList());
assertOrder(list, otp1Id.get(), otp2Id.get());
});
}