From c8a41dea99135dd21d1662fffb265041c61c8e57 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 15 Jan 2026 09:10:25 -0300 Subject: [PATCH] Reverting format changes, updating docs, and only exposing the method to fetch first-factor credentials Signed-off-by: Pedro Igor --- .../organizations/authenticating-members.adoc | 5 +- .../credential/UserCredentialManager.java | 89 +++---- .../models/light/EmptyCredentialManager.java | 5 - .../models/SubjectCredentialManager.java | 7 - .../OrganizationPostBrokerLoginTest.java | 233 ++++++------------ 5 files changed, 117 insertions(+), 222 deletions(-) diff --git a/docs/documentation/server_admin/topics/organizations/authenticating-members.adoc b/docs/documentation/server_admin/topics/organizations/authenticating-members.adoc index 80caa3edaaf..2a683ee130e 100644 --- a/docs/documentation/server_admin/topics/organizations/authenticating-members.adoc +++ b/docs/documentation/server_admin/topics/organizations/authenticating-members.adoc @@ -35,6 +35,7 @@ In addition to identifying the user once the username is provided, the identity- * Matching an email domain to an organization. * Deciding if the authentication flow should continue or not if an account already exists for the username provided * Deciding how the user should be authenticated depending on how the domains and the identity providers are configured to an organization +and the set of credentials configured to the user account. * Seamlessly authenticating users through an identity provider associated with an organization if the email domain matches the domain set to the identity provider The identity-first login provides the same capabilities that are provided by the usual login page with the username and @@ -65,7 +66,9 @@ For more details, see <<_managing_identity_provider_,Managing identity providers In a similar situation to the previous section, the organization may have an identity provider set with one of the organization domains. In this situation, the user is redirected to the identity provider if that user's email matches a specific domain from the organization. -Once the flow completes, an account is created and the user joins the organization. +Once the flow completes, an account is created and the user joins the organization. In case the user has any first-factor credentials +configured (e.g.: password, passwordless, kerberos) to the account, the user is not automatically redirected to the identity provider but asked to authenticate using their +credentials. == Configuring existing authentication flows diff --git a/model/storage/src/main/java/org/keycloak/credential/UserCredentialManager.java b/model/storage/src/main/java/org/keycloak/credential/UserCredentialManager.java index f0a64fd0cd2..901ed1b0479 100644 --- a/model/storage/src/main/java/org/keycloak/credential/UserCredentialManager.java +++ b/model/storage/src/main/java/org/keycloak/credential/UserCredentialManager.java @@ -44,22 +44,24 @@ import io.opentelemetry.api.trace.StatusCode; * * @author Alexander Schwartz */ -public class UserCredentialManager extends AbstractStorageManager - implements org.keycloak.models.UserCredentialManager { +public class UserCredentialManager extends AbstractStorageManager implements org.keycloak.models.UserCredentialManager { + + private static final List FIRST_FACTOR_CREDENTIAL_TYPES = List.of( + PasswordCredentialModel.TYPE, + CredentialModel.CLIENT_CERT, + CredentialModel.KERBEROS, + WebAuthnCredentialModel.TYPE_PASSWORDLESS + ); private final UserModel user; private final KeycloakSession session; private final RealmModel realm; /** - * It is not recommended to use this method directly from your user-storage - * providers! Please use - * {@link org.keycloak.models.UserProvider#getUserCredentialManager(UserModel) - * session.users().getUserCredentialManager(user)} instead. + * It is not recommended to use this method directly from your user-storage providers! Please use {@link org.keycloak.models.UserProvider#getUserCredentialManager(UserModel) session.users().getUserCredentialManager(user)} instead. */ public UserCredentialManager(KeycloakSession session, RealmModel realm, UserModel user) { - super(session, UserStorageProviderFactory.class, UserStorageProvider.class, UserStorageProviderModel::new, - "user"); + super(session, UserStorageProviderFactory.class, UserStorageProvider.class, UserStorageProviderModel::new, "user"); this.user = user; this.session = session; this.realm = realm; @@ -75,8 +77,7 @@ public class UserCredentialManager extends AbstractStorageManager types = Stream.empty(); if (user.isFederated()) { UserStorageProviderModel model = getStorageProviderModel(realm, user.getFederationLink()); - if (model == null || !model.isEnabled()) - return types; + if (model == null || !model.isEnabled()) return types; CredentialInputUpdater updater = getStorageProviderInstance(model, CredentialInputUpdater.class); - if (updater != null) - types = updater.getDisableableCredentialTypesStream(realm, user); + if (updater != null) types = updater.getDisableableCredentialTypesStream(realm, user); } return Stream.concat(types, getCredentialProviders(session, CredentialInputUpdater.class) - .flatMap(updater -> updater.getDisableableCredentialTypesStream(realm, user))) + .flatMap(updater -> updater.getDisableableCredentialTypesStream(realm, user))) .distinct(); } @@ -225,13 +219,10 @@ public class UserCredentialManager extends AbstractStorageManager validator.supportsCredentialType(type) - && validator.isConfiguredFor(realm, user, type)); + .anyMatch(validator -> validator.supportsCredentialType(type) && validator.isConfiguredFor(realm, user, type)); } @Override public Stream getConfiguredUserStorageCredentialTypesStream() { return getCredentialProviders(session, CredentialProvider.class).map(CredentialProvider::getType) - .filter(credentialType -> UserStorageCredentialConfigured.CONFIGURED == isConfiguredThroughUserStorage( - realm, user, credentialType)); + .filter(credentialType -> UserStorageCredentialConfigured.CONFIGURED == isConfiguredThroughUserStorage(realm, user, credentialType)); } @Override @@ -270,16 +259,13 @@ public class UserCredentialManager extends AbstractStorageManager toValidate, - CredentialInputValidator validator) { + private void validate(RealmModel realm, UserModel user, List toValidate, CredentialInputValidator validator) { toValidate.removeIf(input -> { - if (validator.supportsCredentialType(input.getType())) { + if(validator.supportsCredentialType(input.getType())) { return session.getProvider(TracingProvider.class).trace(validator.getClass(), "isValid", span -> { boolean valid = validator.isValid(realm, user, input); if (!valid) { @@ -310,7 +295,7 @@ public class UserCredentialManager extends AbstractStorageManager Stream getCredentialProviders(KeycloakSession session, Class type) { - // noinspection unchecked + //noinspection unchecked return session.getKeycloakSessionFactory().getProviderFactoriesStream(CredentialProvider.class) .filter(f -> Types.supports(type, f, CredentialProviderFactory.class)) .map(f -> (T) session.getProvider(CredentialProvider.class, f.getId())); @@ -333,17 +318,7 @@ public class UserCredentialManager extends AbstractStorageManager getFirstFactorCredentialsStream() { - List firstFactorTypes = getFirstFactorCredentialTypes(); - return getStoredCredentialsStream().filter(c -> firstFactorTypes.contains(c.getType())); - } - - @Override - public List getFirstFactorCredentialTypes() { - return List.of( - PasswordCredentialModel.TYPE, - CredentialModel.CLIENT_CERT, - CredentialModel.KERBEROS, - WebAuthnCredentialModel.TYPE_PASSWORDLESS - ); + return getStoredCredentialsStream() + .filter(c -> FIRST_FACTOR_CREDENTIAL_TYPES.contains(c.getType())); } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/light/EmptyCredentialManager.java b/server-spi-private/src/main/java/org/keycloak/models/light/EmptyCredentialManager.java index a68fdcf9129..e5dda6b4b18 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/light/EmptyCredentialManager.java +++ b/server-spi-private/src/main/java/org/keycloak/models/light/EmptyCredentialManager.java @@ -119,9 +119,4 @@ class EmptyCredentialManager implements SubjectCredentialManager { return null; } - @Override - public List getFirstFactorCredentialTypes() { - return List.of(); - } - } diff --git a/server-spi/src/main/java/org/keycloak/models/SubjectCredentialManager.java b/server-spi/src/main/java/org/keycloak/models/SubjectCredentialManager.java index 558a9dced42..eb29149aa7c 100644 --- a/server-spi/src/main/java/org/keycloak/models/SubjectCredentialManager.java +++ b/server-spi/src/main/java/org/keycloak/models/SubjectCredentialManager.java @@ -138,13 +138,6 @@ public interface SubjectCredentialManager { return getStoredCredentialsStream(); } - /** - * Returns a list of types for first-factor credentials. - * - * @return a list of first-factor credential types - */ - List getFirstFactorCredentialTypes(); - /** * Check if the credential type is configured for this entity. * @param type credential type to check diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationPostBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationPostBrokerLoginTest.java index df89192cdb2..be12ddab64a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationPostBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationPostBrokerLoginTest.java @@ -17,28 +17,23 @@ package org.keycloak.testsuite.organization.broker; import java.util.List; -import java.util.stream.Collectors; -import org.keycloak.admin.client.resource.AuthenticationManagementResource; import org.keycloak.admin.client.resource.RealmResource; -import org.keycloak.authentication.authenticators.broker.IdpCreateUserIfUniqueAuthenticatorFactory; -import org.keycloak.credential.CredentialModel; +import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; -import org.keycloak.models.UserModel; -import org.keycloak.models.utils.DefaultAuthenticationFlows; -import org.keycloak.organization.OrganizationProvider; +import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.organization.utils.Organizations; -import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; -import org.keycloak.representations.idm.AuthenticatorConfigRepresentation; +import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.OrganizationDomainRepresentation; import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.broker.AbstractBrokerTest; import org.keycloak.testsuite.broker.AbstractInitializedBaseBrokerTest; @@ -47,12 +42,16 @@ import org.keycloak.testsuite.pages.LoginTotpPage; import org.keycloak.testsuite.runonserver.RunOnServer; import org.keycloak.testsuite.util.AccountHelper; +import org.hamcrest.Matchers; import org.jboss.arquillian.graphene.page.Page; import org.junit.Rule; import org.junit.Test; import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + /** * Tests interaction between organization linked identity provider (redirect on email domain match + hide on login) * and post-broker login flow which requires OTP setup. Ensures that after OTP is configured the user WILL be @@ -66,7 +65,7 @@ public class OrganizationPostBrokerLoginTest extends AbstractInitializedBaseBrok private final String ORG_ALIAS = "org-post-broker"; @Page - private LoginConfigTotpPage totpPage; + private LoginConfigTotpPage totpConfigPage; @Page private LoginTotpPage loginTotpPage; @@ -108,159 +107,89 @@ public class OrganizationPostBrokerLoginTest extends AbstractInitializedBaseBrok consumerRealm.identityProviders().get(bc.getIDPAlias()).update(idp); // 3) Configure post-broker login flow for this idp to require OTP setup - final String idpAlias = bc.getIDPAlias(); - + String idpAlias = bc.getIDPAlias(); testingClient.server(bc.consumerRealmName()).run(new ConfigureOtpPostBrokerFlow(idpAlias)); // Disable update profile prompts so we can reach the post-broker OTP flow directly updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); - try { - // 4) First login via broker, run post-broker OTP setup - oauth.clientId("broker-app"); - loginPage.open(bc.consumerRealmName()); + // 4) First login via broker, run post-broker OTP setup + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + logInWithBroker(bc); - logInWithBroker(bc); - // Post broker flow should require TOTPs - totpPage.assertCurrent(); - final String totpSecret = totpPage.getTotpSecret(); - totpPage.configure(totp.generateTOTP(totpSecret)); + // Post broker flow should require TOTPs + totpConfigPage.assertCurrent(); + String totpSecret = totpConfigPage.getTotpSecret(); + totpConfigPage.configure(totp.generateTOTP(totpSecret)); + UsersResource usersApi = realmsResouce().realm(bc.consumerRealmName()).users(); + List users = usersApi.search(bc.getUserLogin(), true); + assertThat(1, equalTo(users.size())); + UserRepresentation user = users.get(0); + List credentials = usersApi.get(user.getId()).credentials(); + assertThat("Expected exactly one credential after TOTP setup", credentials.size(), equalTo(1)); + assertThat("Expected TOTP credential type after setup", credentials.get(0).getType(), equalTo(OTPCredentialModel.TYPE)); - testingClient.server(bc.consumerRealmName()).run(new AssertNoFirstFactorCredentials(bc.consumerRealmName(), bc.getUserLogin())); + IdentityProviderRepresentation postTotpIdp = consumerRealm.identityProviders().get(idp.getAlias()).toRepresentation(); + postTotpIdp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, orgDomain); + postTotpIdp.getConfig().put(OrganizationModel.IdentityProviderRedirectMode.EMAIL_MATCH.getKey(), Boolean.TRUE.toString()); + postTotpIdp.setHideOnLogin(true); + consumerRealm.identityProviders().get(postTotpIdp.getAlias()).update(postTotpIdp); + consumerRealm.organizations().get(orgId).identityProviders().addIdentityProvider(postTotpIdp.getAlias()).close(); - IdentityProviderRepresentation postTotpIdp = consumerRealm.identityProviders().get(idp.getAlias()).toRepresentation(); - postTotpIdp.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, orgDomain); - postTotpIdp.getConfig().put(OrganizationModel.IdentityProviderRedirectMode.EMAIL_MATCH.getKey(), Boolean.TRUE.toString()); - postTotpIdp.setHideOnLogin(true); - consumerRealm.identityProviders().get(postTotpIdp.getAlias()).update(postTotpIdp); - consumerRealm.organizations().get(orgId).identityProviders().addIdentityProvider(postTotpIdp.getAlias()).close(); + AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin()); + // 5) Try re-login: user SHOULD be automatically redirected to the identity provider + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + // submit username/email to trigger organization resolution which should redirect to the provider + loginPage.loginUsername(bc.getUserEmail()); - testingClient.server(bc.consumerRealmName()).run(new LogOrganizationState(bc.consumerRealmName(), orgId, bc.getUserLogin(), orgDomain)); + // wait for provider login page (title contains "sign in to ") + waitForPage(driver, "sign in to", true); + assertThat("Driver should be on the provider realm page right now", + driver.getCurrentUrl(), Matchers.containsString("/auth/realms/" + bc.providerRealmName() + "/")); + String landingUrl = driver.getCurrentUrl(); + String landingTitle = driver.getTitle(); + log.infof("Organization broker re-login landed on page title='%s' url='%s' providerRealName='%s'", landingTitle, landingUrl, bc.providerRealmName()); + loginPage.login(bc.getUserPassword()); - AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin()); - // AccountHelper.logout(adminClient.realm(bc.providerRealmName()), bc.getUserLogin()); - - // 5) Try re-login: user SHOULD be automatically redirected to the identity provider - oauth.clientId("broker-app"); - loginPage.open(bc.consumerRealmName()); - - // submit username/email to trigger organization resolution which should redirect to the provider - loginPage.loginUsername(bc.getUserEmail()); - - // provide OTP code required on re-login for the configured credential - if (loginTotpPage.isCurrent()) { - log.infof("Submitting TOTP on re-login"); - loginTotpPage.login(totp.generateTOTP(totpSecret)); - } - - // wait for provider login page (title contains "sign in to ") - waitForPage(driver, "sign in to", true); - - String landingUrl = driver.getCurrentUrl(); - String landingTitle = driver.getTitle(); - log.infof("Organization broker re-login landed on page title='%s' url='%s' providerRealName='%s'", landingTitle, landingUrl, bc.providerRealmName()); - - // check we are on provider realm login page (redirected to IDP) - org.junit.Assert.assertTrue(landingUrl.contains("/auth/realms/" + bc.providerRealmName())); - } finally { - updateExecutions(OrganizationPostBrokerLoginTest::restoreDefaultFirstBrokerLoginConfig); - } + // provide OTP code required on re-login for the configured credential + assertThat("Driver should be on the consumer realm page right now", + driver.getCurrentUrl(), Matchers.containsString("/auth/realms/" + bc.consumerRealmName() + "/")); + loginTotpPage.assertCurrent(); } - private static void restoreDefaultFirstBrokerLoginConfig(AuthenticationExecutionInfoRepresentation execution, - AuthenticationManagementResource flows) { - if (execution.getProviderId() != null && execution.getProviderId().equals(IdpCreateUserIfUniqueAuthenticatorFactory.PROVIDER_ID)) { - execution.setRequirement(AuthenticationExecutionModel.Requirement.ALTERNATIVE.name()); - flows.updateExecutions(DefaultAuthenticationFlows.FIRST_BROKER_LOGIN_FLOW, execution); - } else if (execution.getAlias() != null && execution.getAlias().equals(DefaultAuthenticationFlows.IDP_REVIEW_PROFILE_CONFIG_ALIAS)) { - AuthenticatorConfigRepresentation config = flows.getAuthenticatorConfig(execution.getAuthenticationConfig()); - config.getConfig().put("update.profile.on.first.login", IdentityProviderRepresentation.UPFLM_MISSING); - flows.updateAuthenticatorConfig(config.getId(), config); + private static class ConfigureOtpPostBrokerFlow implements RunOnServer { + private final String idpAlias; + + private ConfigureOtpPostBrokerFlow(String idpAlias) { + this.idpAlias = idpAlias; + } + + @Override + public void run(KeycloakSession session) { + RealmModel realm = session.getContext().getRealm(); + + // Build dedicated post-broker flow that enforces OTP setup before redirect + AuthenticationFlowModel postBrokerFlow = new AuthenticationFlowModel(); + postBrokerFlow.setAlias("post-broker"); + postBrokerFlow.setDescription("post-broker flow with OTP"); + postBrokerFlow.setProviderId("basic-flow"); + postBrokerFlow.setTopLevel(true); + postBrokerFlow.setBuiltIn(false); + postBrokerFlow = realm.addAuthenticationFlow(postBrokerFlow); + + AuthenticationExecutionModel execution = new AuthenticationExecutionModel(); + execution.setParentFlow(postBrokerFlow.getId()); + execution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED); + execution.setAuthenticator("auth-otp-form"); + execution.setPriority(20); + execution.setAuthenticatorFlow(false); + realm.addAuthenticatorExecution(execution); + + IdentityProviderModel idpModel = session.identityProviders().getByAlias(idpAlias); + idpModel.setPostBrokerLoginFlowId(postBrokerFlow.getId()); + session.identityProviders().update(idpModel); } } - - private static class ConfigureOtpPostBrokerFlow implements RunOnServer { - private final String idpAlias; - - private ConfigureOtpPostBrokerFlow(String idpAlias) { - this.idpAlias = idpAlias; - } - - @Override - public void run(KeycloakSession session) { - RealmModel realm = session.getContext().getRealm(); - - // Build dedicated post-broker flow that enforces OTP setup before redirect - AuthenticationFlowModel postBrokerFlow = new AuthenticationFlowModel(); - postBrokerFlow.setAlias("post-broker"); - postBrokerFlow.setDescription("post-broker flow with OTP"); - postBrokerFlow.setProviderId("basic-flow"); - postBrokerFlow.setTopLevel(true); - postBrokerFlow.setBuiltIn(false); - postBrokerFlow = realm.addAuthenticationFlow(postBrokerFlow); - - AuthenticationExecutionModel execution = new AuthenticationExecutionModel(); - execution.setParentFlow(postBrokerFlow.getId()); - execution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED); - execution.setAuthenticator("auth-otp-form"); - execution.setPriority(20); - execution.setAuthenticatorFlow(false); - realm.addAuthenticatorExecution(execution); - - IdentityProviderModel idpModel = session.identityProviders().getByAlias(idpAlias); - idpModel.setPostBrokerLoginFlowId(postBrokerFlow.getId()); - session.identityProviders().update(idpModel); - } - } - - private static class AssertNoFirstFactorCredentials implements RunOnServer { - private final String realmName; - private final String username; - - private AssertNoFirstFactorCredentials(String realmName, String username) { - this.realmName = realmName; - this.username = username; - } - - @Override - public void run(KeycloakSession session) { - RealmModel realm = session.realms().getRealmByName(realmName); - UserModel user = session.users().getUserByUsername(realm, username); - boolean hasFirstFactor = user.credentialManager().getFirstFactorCredentialsStream().findAny().isPresent(); - org.junit.Assert.assertFalse("Unexpected first-factor credentials present after OTP setup", hasFirstFactor); - } - } - - private static class LogOrganizationState implements RunOnServer { - private final String realmName; - private final String organizationId; - private final String username; - private final String expectedDomain; - - private LogOrganizationState(String realmName, String organizationId, String username, String expectedDomain) { - this.realmName = realmName; - this.organizationId = organizationId; - this.username = username; - this.expectedDomain = expectedDomain; - } - - @Override - public void run(KeycloakSession session) { - RealmModel realm = session.realms().getRealmByName(realmName); - OrganizationProvider provider = session.getProvider(OrganizationProvider.class); - OrganizationModel organization = provider.getById(organizationId); - UserModel user = session.users().getUserByUsername(realm, username); - - boolean managedMember = organization != null && user != null && organization.isManaged(user); - List idpInfo = organization == null ? List.of() : organization.getIdentityProviders() - .map(broker -> broker.getAlias() + "|domain=" + broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE) + - "|emailMatch=" + broker.getConfig().get(OrganizationModel.IdentityProviderRedirectMode.EMAIL_MATCH.getKey())) - .collect(Collectors.toList()); - List credentialTypes = user == null ? List.of() : user.credentialManager().getStoredCredentialsStream() - .map(CredentialModel::getType) - .collect(Collectors.toList()); - - System.out.printf("[OrgState] managed=%s expectedDomain=%s idps=%s credentials=%s%n", managedMember, expectedDomain, idpInfo, credentialTypes); - } - } -} \ No newline at end of file +}