From 76ab8bd21db669cd3b71144080fc7d21c0e22d78 Mon Sep 17 00:00:00 2001 From: Pavuluri Sai Krishna Date: Thu, 12 Jun 2025 15:38:05 +0530 Subject: [PATCH] Implemented validation to ensure each OTP device has a unique label Closes #38465 Signed-off-by: Saikrishna Signed-off-by: Alexander Schwartz Co-authored-by: Saikrishna Co-authored-by: Alexander Schwartz --- .../models/jpa/JpaUserCredentialStore.java | 16 +++++ .../jpa/JpaUserFederatedStorageProvider.java | 21 +++++- .../keycloak/credential/CredentialModel.java | 1 + .../requiredactions/UpdateTotp.java | 18 ++++- .../services/error/KeycloakErrorHandler.java | 6 ++ .../resources/admin/UserResource.java | 6 ++ .../AppInitiatedActionTotpSetupTest.java | 29 ++++++++ .../keycloak/testsuite/admin/UserTest.java | 46 ++++++++++++ .../FederatedStorageExportImportTest.java | 72 +++++++++++++++++++ .../federation/storage/UserStorageTest.java | 6 +- .../testsuite/model/CredentialModelTest.java | 4 +- .../account/AbstractWebAuthnAccountTest.java | 4 ++ .../account/WebAuthnSigningInTest.java | 33 ++++----- .../keycloak.v2/login/login-config-totp.ftl | 2 +- 14 files changed, 234 insertions(+), 30 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserCredentialStore.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserCredentialStore.java index f470b171631..21e234233be 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserCredentialStore.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserCredentialStore.java @@ -21,6 +21,7 @@ import org.keycloak.common.util.Base64; import org.keycloak.credential.CredentialModel; import org.keycloak.credential.UserCredentialStore; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.jpa.entities.CredentialEntity; @@ -62,6 +63,7 @@ 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()); entity.setCreatedDate(cred.getCreatedDate()); entity.setUserLabel(cred.getUserLabel()); entity.setType(cred.getType()); @@ -138,7 +140,21 @@ public class JpaUserCredentialStore implements UserCredentialStore { } + private void validateDuplicateCredential(RealmModel realm, UserModel user, String userLabel, String credentialId) { + if (userLabel != null) { + boolean exists = getStoredCredentialEntities(realm, user) + .anyMatch(existing -> existing.getUserLabel() != null + && existing.getUserLabel().equalsIgnoreCase(userLabel.trim()) + && (credentialId == null || !existing.getId().equals(credentialId))); // Exclude self in update + + if (exists) { + throw new ModelDuplicateException("Device already exists with the same name", CredentialModel.USER_LABEL); + } + } + } + CredentialEntity createCredentialEntity(RealmModel realm, UserModel user, CredentialModel cred) { + validateDuplicateCredential(realm, user, cred.getUserLabel(), null); CredentialEntity entity = new CredentialEntity(); String id = cred.getId() == null ? KeycloakModelUtils.generateId() : cred.getId(); entity.setId(id); diff --git a/model/jpa/src/main/java/org/keycloak/storage/jpa/JpaUserFederatedStorageProvider.java b/model/jpa/src/main/java/org/keycloak/storage/jpa/JpaUserFederatedStorageProvider.java index a45b615c2c3..db5f7f71c3d 100644 --- a/model/jpa/src/main/java/org/keycloak/storage/jpa/JpaUserFederatedStorageProvider.java +++ b/model/jpa/src/main/java/org/keycloak/storage/jpa/JpaUserFederatedStorageProvider.java @@ -483,7 +483,7 @@ public class JpaUserFederatedStorageProvider implements return closing(paginateQuery(query, firstResult, max).getResultStream()); } - + @Override public Stream getRoleMembersStream(RealmModel realm, RoleModel role, Integer firstResult, Integer max) { TypedQuery query = em.createNamedQuery("fedRoleMembership", String.class); @@ -564,6 +564,7 @@ public class JpaUserFederatedStorageProvider implements public void updateCredential(RealmModel realm, String userId, CredentialModel cred) { FederatedUserCredentialEntity entity = em.find(FederatedUserCredentialEntity.class, cred.getId()); if (!checkCredentialEntity(entity, userId)) return; + validateDuplicateUserCredential(userId, cred.getUserLabel(), cred.getId()); createIndex(realm, userId); entity.setCreatedDate(cred.getCreatedDate()); entity.setType(cred.getType()); @@ -572,8 +573,26 @@ public class JpaUserFederatedStorageProvider implements entity.setUserLabel(cred.getUserLabel()); } + /** + * Validates if a credential with the same user label already exists for the given user. + * Excludes the credential itself if updating an existing one. + */ + private void validateDuplicateUserCredential(String userId, String userLabel, String credentialId) { + if (userLabel != null) { + boolean exists = getStoredCredentialEntitiesStream(userId) + .anyMatch(existing -> existing.getUserLabel() != null + && existing.getUserLabel().trim().equalsIgnoreCase(userLabel.trim()) + && (credentialId == null || !existing.getId().equals(credentialId))); + + if (exists) { + throw new ModelDuplicateException("Device already exists with the same name", CredentialModel.USER_LABEL); + } + } + } + @Override public CredentialModel createCredential(RealmModel realm, String userId, CredentialModel cred) { + validateDuplicateUserCredential(userId, cred.getUserLabel(), null); createIndex(realm, userId); FederatedUserCredentialEntity entity = new FederatedUserCredentialEntity(); String id = cred.getId() == null ? KeycloakModelUtils.generateId() : cred.getId(); diff --git a/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java b/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java index 25a11bff677..2884e658b6b 100755 --- a/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java +++ b/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java @@ -58,6 +58,7 @@ public class CredentialModel implements Serializable { public static final String CLIENT_CERT = "cert"; public static final String KERBEROS = "kerberos"; + public static final String USER_LABEL = "userLabel"; private String id; private String type; diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java index 34f3cb076fe..f06bab529aa 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java @@ -32,6 +32,7 @@ import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.OTPPolicy; import org.keycloak.models.UserModel; import org.keycloak.models.Constants; @@ -116,10 +117,23 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory AuthenticatorUtil.logoutOtherSessions(context); } - if (!CredentialHelper.createOTPCredential(context.getSession(), context.getRealm(), context.getUser(), challengeResponse, credentialModel)) { + try { + if (!CredentialHelper.createOTPCredential(context.getSession(), context.getRealm(), context.getUser(), challengeResponse, credentialModel)) { + Response challenge = context.form() + .setAttribute("mode", mode) + .addError(new FormMessage(Validation.FIELD_OTP_CODE, Messages.INVALID_TOTP)) + .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); + context.challenge(challenge); + return; + } + } catch (ModelDuplicateException e) { + String field = switch (e.getDuplicateFieldName()) { + case CredentialModel.USER_LABEL -> Validation.FIELD_OTP_LABEL; + default -> null; + }; Response challenge = context.form() .setAttribute("mode", mode) - .addError(new FormMessage(Validation.FIELD_OTP_CODE, Messages.INVALID_TOTP)) + .addError(new FormMessage(field, e.getMessage())) .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); context.challenge(challenge); return; diff --git a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java index 289451a5786..4f5decdb18f 100644 --- a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java +++ b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java @@ -94,6 +94,8 @@ public class KeycloakErrorHandler implements ExceptionMapper { error.setError(getErrorCode(throwable)); if (throwable.getCause() instanceof ModelException) { error.setErrorDescription(throwable.getMessage()); + } if (throwable instanceof ModelDuplicateException) { + error.setErrorDescription(throwable.getMessage()); } else if (throwable instanceof JsonProcessingException || throwable.getCause() instanceof JsonProcessingException) { error.setErrorDescription("Cannot parse the JSON"); } else if (isServerError) { @@ -153,6 +155,10 @@ public class KeycloakErrorHandler implements ExceptionMapper { return OAuthErrorException.INVALID_REQUEST; } + if (cause instanceof ModelDuplicateException || throwable instanceof ModelDuplicateException) { + return "conflict"; + } + if (throwable instanceof WebApplicationException && throwable.getMessage() != null) { return throwable.getMessage(); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 8d9082d528b..844849c36f8 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -883,6 +883,12 @@ public class UserResource { if (auth.users().canQuery()) throw new NotFoundException("Credential not found"); else throw new ForbiddenException(); } + + if (userLabel == null || userLabel.trim().isEmpty()) { + throw new ErrorResponseException("missingCredentialLabel", "Credential label must not be empty", Status.BAD_REQUEST); + + } + user.credentialManager().updateCredentialLabel(credentialId, userLabel); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java index 698bb744e45..0c147dadedb 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java @@ -95,6 +95,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT registerPage.register("firstName", "lastName", "email@mail.com", "setupTotp", "password", "password"); String userId = events.expectRegister("setupTotp", "email@mail.com").assertEvent().getUserId(); + getCleanup().addUserId(userId); doAIA(); @@ -120,6 +121,33 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT events.expectLogin().user(userId).session(authSessionId2).detail(Details.USERNAME, "setuptotp").assertEvent(); } + @Test + public void setupTotpRegisterDuplicateUserLabel() { + loginPage.open(); + loginPage.clickRegister(); + registerPage.register("firstName", "lastName", "email@mail.com", "setupTotp", "password", "password"); + + String userId = events.expectRegister("setupTotp", "email@mail.com").assertEvent().getUserId(); + getCleanup().addUserId(userId); + + doAIA(); + + totpPage.assertCurrent(); + + totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret()), "otp"); + + assertKcActionStatus(SUCCESS); + + doAIA(); + + totpPage.assertCurrent(); + + totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret()), "otp"); + + assertEquals("Device already exists with the same name", totpPage.getInputLabelError()); + + } + @Test public void cancelSetupTotp() throws Exception { try { @@ -382,6 +410,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT registerPage.register("firstName2", "lastName2", "email2@mail.com", "setupTotp2", "password2", "password2"); String userId = events.expectRegister("setupTotp2", "email2@mail.com").assertEvent().getUserId(); + getCleanup().addUserId(userId); doAIA(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 75bb86ae42f..aad1e89800d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -45,9 +45,11 @@ import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.Constants; import org.keycloak.models.LDAPConstants; +import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.models.credential.PasswordCredentialModel; +import org.keycloak.models.jpa.entities.CredentialEntity; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.StripSecretsUtils; @@ -71,6 +73,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.userprofile.config.UPAttribute; import org.keycloak.representations.userprofile.config.UPAttributePermissions; import org.keycloak.representations.userprofile.config.UPConfig; +import org.keycloak.services.ErrorResponseException; import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; import org.keycloak.testsuite.federation.DummyUserFederationProviderFactory; @@ -3403,6 +3406,49 @@ public class UserTest extends AbstractAdminTest { .findFirst().orElseThrow().getUserLabel()); } + @Test + public void testShouldFailToSetCredentialUserLabelWhenLabelIsEmpty() { + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "user-with-one-configured-otp"); + CredentialRepresentation otpCred = user.credentials().get(0); + BadRequestException ex = assertThrows(BadRequestException.class, () -> { + user.setCredentialUserLabel(otpCred.getId(), " "); + }); + Response response = ex.getResponse(); + String body = response.readEntity(String.class); + + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertTrue(body.contains("missingCredentialLabel")); + assertTrue(body.contains("Credential label must not be empty")); + } + + @Test + public void testShouldFailToSetCredentialUserLabelWhenLabelAlreadyExists() { + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "user-with-two-configured-otp"); + + List credentials = user.credentials().stream() + .filter(c -> c.getType().equals(OTPCredentialModel.TYPE)) + .toList(); + assertEquals(2, credentials.size()); + + String firstId = credentials.get(0).getId(); + String secondId = credentials.get(1).getId(); + + user.setCredentialUserLabel(firstId, "Device"); + user.setCredentialUserLabel(secondId, "Second Device"); + + // Attempt to update second credential to use the same label as the first + ClientErrorException ex = assertThrows(ClientErrorException.class, () -> { + user.setCredentialUserLabel(secondId, "Device"); + }); + + Response response = ex.getResponse(); + assertEquals(Response.Status.CONFLICT.getStatusCode(), response.getStatus()); + + String body = response.readEntity(String.class); + assertNotNull(body); + assertTrue(body.contains("Device already exists with the same name")); + } + @Test public void testUpdateCredentialLabelForFederatedUser() { // Create user federation diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/FederatedStorageExportImportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/FederatedStorageExportImportTest.java index b64bed0699d..28902d2e7d6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/FederatedStorageExportImportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/FederatedStorageExportImportTest.java @@ -31,6 +31,7 @@ import org.keycloak.exportimport.dir.DirExportProviderFactory; import org.keycloak.exportimport.singlefile.SingleFileExportProviderFactory; import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; @@ -46,6 +47,10 @@ import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -161,6 +166,73 @@ public class FederatedStorageExportImportTest extends AbstractAuthTest { }); } + @Test + public void testCreateCredentialWithDuplicateUserLabelShouldFail() { + final String userId = "f:1:testUser"; + + testingClient.server().run(session -> { + RealmModel realm = new RealmManager(session).createRealm(REALM_NAME); + UserStorageUtil.userFederatedStorage(session).setSingleAttribute(realm, userId, "dummy", "value"); + + PasswordCredentialModel cred1 = FederatedStorageExportImportTest + .getHashProvider(session, realm.getPasswordPolicy()) + .encodedCredential("secret1", realm.getPasswordPolicy().getHashIterations()); + cred1.setUserLabel("MyDevice"); + UserStorageUtil.userFederatedStorage(session).createCredential(realm, userId, cred1); + + PasswordCredentialModel cred2 = FederatedStorageExportImportTest + .getHashProvider(session, realm.getPasswordPolicy()) + .encodedCredential("secret2", realm.getPasswordPolicy().getHashIterations()); + cred2.setUserLabel("MyDevice"); + + try { + UserStorageUtil.userFederatedStorage(session).createCredential(realm, userId, cred2); + Assert.fail("Expected ModelDuplicateException was not thrown"); + } catch (ModelDuplicateException ex) { + assertEquals("Device already exists with the same name", ex.getMessage()); + } + }); + } + + @Test + public void testUpdateCredentialWithDuplicateUserLabelShouldFail() { + final String userId = "f:1:testUser"; + + testingClient.server().run(session -> { + RealmModel realm = new RealmManager(session).createRealm(REALM_NAME); + UserStorageUtil.userFederatedStorage(session).setSingleAttribute(realm, userId, "dummy", "value"); + + // Create first credential with label "DeviceOne" + PasswordCredentialModel cred1 = FederatedStorageExportImportTest + .getHashProvider(session, realm.getPasswordPolicy()) + .encodedCredential("secret1", realm.getPasswordPolicy().getHashIterations()); + cred1.setUserLabel("DeviceOne"); + UserStorageUtil.userFederatedStorage(session).createCredential(realm, userId, cred1); + + // Create second credential with label "DeviceTwo" + PasswordCredentialModel cred2 = FederatedStorageExportImportTest + .getHashProvider(session, realm.getPasswordPolicy()) + .encodedCredential("secret2", realm.getPasswordPolicy().getHashIterations()); + cred2.setUserLabel("DeviceTwo"); + UserStorageUtil.userFederatedStorage(session).createCredential(realm, userId, cred2); + + CredentialModel credentialModelUpdate = UserStorageUtil.userFederatedStorage(session) + .getStoredCredentialsStream(realm, userId) + .filter(c -> "DeviceTwo".equals(c.getUserLabel())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Credential not found")); + + credentialModelUpdate.setUserLabel("DeviceOne"); + + try { + UserStorageUtil.userFederatedStorage(session).updateCredential(realm, userId, credentialModelUpdate); + Assert.fail("Expected ModelDuplicateException was not thrown"); + } catch (ModelDuplicateException ex) { + assertEquals("Device already exists with the same name", ex.getMessage()); + } + }); + } + @Test public void testDir() { ComponentExportImportTest.clearExportImportProperties(testingClient); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java index 90f70be8139..6e398cdf160 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java @@ -979,7 +979,7 @@ public class UserStorageTest extends AbstractAuthTest { @Test @ModelTest - public void testCredentialCRUD(KeycloakSession session) throws Exception { + public void testCredentialCRUD(KeycloakSession session) { AtomicReference passwordId = new AtomicReference<>(); AtomicReference otp1Id = new AtomicReference<>(); AtomicReference otp2Id = new AtomicReference<>(); @@ -1000,8 +1000,8 @@ public class UserStorageTest extends AbstractAuthTest { passwordId.set(passwordCred.getId()); // Create Password and 2 OTP credentials (password was already created) - CredentialModel otp1 = OTPCredentialModel.createFromPolicy(realm, "secret1"); - CredentialModel otp2 = OTPCredentialModel.createFromPolicy(realm, "secret2"); + 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()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/CredentialModelTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/CredentialModelTest.java index 398f45859e1..82d46a6d886 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/CredentialModelTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/CredentialModelTest.java @@ -45,8 +45,8 @@ public class CredentialModelTest extends AbstractTestRealmKeycloakTest { passwordId.set(list.get(0).getId()); // Create 2 OTP credentials (password was already created) - CredentialModel otp1 = OTPCredentialModel.createFromPolicy(realm, "secret1"); - CredentialModel otp2 = OTPCredentialModel.createFromPolicy(realm, "secret2"); + 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()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/AbstractWebAuthnAccountTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/AbstractWebAuthnAccountTest.java index 52e1364ccf6..da8ddf767d9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/AbstractWebAuthnAccountTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/AbstractWebAuthnAccountTest.java @@ -34,6 +34,7 @@ import org.keycloak.representations.idm.RequiredActionProviderSimpleRepresentati import org.keycloak.testsuite.AbstractAuthTest; import org.keycloak.testsuite.page.AbstractPatternFlyAlert; import org.keycloak.testsuite.webauthn.pages.SigningInPage; +import org.keycloak.testsuite.webauthn.pages.WebAuthnErrorPage; import org.keycloak.testsuite.webauthn.utils.SigningInPageUtils; import org.keycloak.testsuite.pages.DeleteCredentialPage; import org.keycloak.testsuite.updaters.RealmAttributeUpdater; @@ -75,6 +76,9 @@ public abstract class AbstractWebAuthnAccountTest extends AbstractAuthTest imple @Page private DeleteCredentialPage deleteCredentialPage; + @Page + protected WebAuthnErrorPage webAuthnErrorPage; + private VirtualAuthenticatorManager webAuthnManager; protected SigningInPage.CredentialType webAuthnCredentialType; protected SigningInPage.CredentialType webAuthnPwdlessCredentialType; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/WebAuthnSigningInTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/WebAuthnSigningInTest.java index aee22201338..c2694c60faa 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/WebAuthnSigningInTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/account/WebAuthnSigningInTest.java @@ -26,6 +26,7 @@ import org.keycloak.models.credential.WebAuthnCredentialModel; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.RequiredActionProviderRepresentation; import org.keycloak.testsuite.arquillian.annotation.IgnoreBrowserDriver; +import org.keycloak.testsuite.page.AbstractPatternFlyAlert; import org.keycloak.testsuite.webauthn.pages.SigningInPage; import org.keycloak.testsuite.webauthn.pages.WebAuthnAuthenticatorsList; import org.keycloak.testsuite.webauthn.updaters.AbstractWebAuthnRealmUpdater; @@ -94,34 +95,24 @@ public class WebAuthnSigningInTest extends AbstractWebAuthnAccountTest { public void createWebAuthnSameUserLabel() { final String SAME_LABEL = "key123"; - // Do we really allow to have several authenticators with the same user label?? - SigningInPage.UserCredential webAuthn = addWebAuthnCredential(SAME_LABEL, false); assertThat(webAuthn, notNullValue()); - SigningInPage.UserCredential passwordless = addWebAuthnCredential(SAME_LABEL, true); - assertThat(passwordless, notNullValue()); - assertThat(webAuthnCredentialType.getUserCredentialsCount(), is(1)); - webAuthn = webAuthnCredentialType.getUserCredential(webAuthn.getId()); - assertThat(webAuthn, notNullValue()); - assertThat(webAuthn.getUserLabel(), is(SAME_LABEL)); + SigningInPage.CredentialType credentialType = webAuthnPwdlessCredentialType; - assertThat(webAuthnPwdlessCredentialType.getUserCredentialsCount(), is(1)); - passwordless = webAuthnPwdlessCredentialType.getUserCredential(passwordless.getId()); - assertThat(passwordless, notNullValue()); - assertThat(passwordless.getUserLabel(), is(SAME_LABEL)); + AbstractPatternFlyAlert.waitUntilHidden(); - SigningInPage.UserCredential webAuthn2 = addWebAuthnCredential(SAME_LABEL, false); - assertThat(webAuthn2, notNullValue()); - assertThat(webAuthn2.getUserLabel(), is(SAME_LABEL)); + credentialType.clickSetUpLink(); + webAuthnRegisterPage.assertCurrent(); + webAuthnRegisterPage.clickRegister(); + webAuthnRegisterPage.registerWebAuthnCredential(SAME_LABEL); + waitForPageToLoad(); - assertThat(webAuthnCredentialType.getUserCredentialsCount(), is(2)); + webAuthnErrorPage.assertCurrent(); + assertThat(webAuthnErrorPage.getError(), is("Failed to register your Passkey.\nDevice already exists with the same name")); + webAuthnErrorPage.clickTryAgain(); - SigningInPage.UserCredential passwordless2 = addWebAuthnCredential(SAME_LABEL, true); - assertThat(passwordless2, notNullValue()); - assertThat(passwordless2.getUserLabel(), is(SAME_LABEL)); - - assertThat(webAuthnPwdlessCredentialType.getUserCredentialsCount(), is(2)); + webAuthnRegisterPage.assertCurrent(); } @Test diff --git a/themes/src/main/resources/theme/keycloak.v2/login/login-config-totp.ftl b/themes/src/main/resources/theme/keycloak.v2/login/login-config-totp.ftl index 47a57fba36a..81f2329854f 100755 --- a/themes/src/main/resources/theme/keycloak.v2/login/login-config-totp.ftl +++ b/themes/src/main/resources/theme/keycloak.v2/login/login-config-totp.ftl @@ -81,7 +81,7 @@ -
+