From 553ab0992ef7e5415c03c7562687b4e158f49632 Mon Sep 17 00:00:00 2001 From: Konstantinos Georgilakis Date: Mon, 27 Jun 2022 09:27:35 +0300 Subject: [PATCH 1/2] Continue client browser flow after User login from Identity Provider Closes #10250 Signed-off-by: cgeorgilakis-grnet --- .../authentication/AuthenticationFlow.java | 4 ++ .../AuthenticationFlowContext.java | 5 +++ .../AuthenticationProcessor.java | 21 ++++++++++ .../DefaultAuthenticationFlow.java | 14 +++++++ .../IdentityProviderAuthenticator.java | 6 +++ .../browser/UsernamePasswordForm.java | 31 +++++++++------ .../resources/IdentityBrokerService.java | 38 +++++++++++++++++++ .../broker/AbstractAdvancedBrokerTest.java | 20 ++++++++++ .../KcOidcFirstBrokerLoginNewAuthTest.java | 11 +++++- 9 files changed, 137 insertions(+), 13 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlow.java b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlow.java index 2fee6ef14ca2..34dee65f1684 100755 --- a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlow.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlow.java @@ -21,6 +21,8 @@ import java.util.Collections; import java.util.List; +import org.keycloak.models.AuthenticationExecutionModel; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -36,4 +38,6 @@ public interface AuthenticationFlow { default List getFlowExceptions(){ return Collections.emptyList(); } + + default Response continueClientAuthAfterIdPLogin(AuthenticationExecutionModel model){ throw new IllegalStateException("Not supposed to be invoked"); } } diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java index 390be08b3615..ad5853a6ebd5 100755 --- a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java @@ -74,6 +74,11 @@ public interface AuthenticationFlowContext extends AbstractAuthenticationFlowCon */ String getFlowPath(); + /** + * @return flow id + */ + String getFlowId(); + /** * Create a Freemarker form builder that presets the user, action URI, and a generated access code * diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 4f356267b88c..7e14a57c5c94 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -89,6 +89,8 @@ public class AuthenticationProcessor { public static final String CURRENT_AUTHENTICATION_EXECUTION = "current.authentication.execution"; public static final String LAST_PROCESSED_EXECUTION = "last.processed.execution"; public static final String CURRENT_FLOW_PATH = "current.flow.path"; + public static final String CLIENT_FLOW_ID = "client.flow.id"; + public static final String CLIENT_AUTHENTICATION_EXECUTION = "client.authentication.execution"; public static final String FORKED_FROM = "forked.from"; public static final String AUTHN_CREDENTIALS = "authn.credentials"; @@ -288,6 +290,10 @@ public String getFlowPath() { return flowPath; } + public String getFlowId() { + return flowId; + } + public void setAutheticatedUser(UserModel user) { UserModel previousUser = getAuthenticationSession().getAuthenticatedUser(); if (previousUser != null && !user.getId().equals(previousUser.getId())) @@ -544,6 +550,11 @@ public String getFlowPath() { return AuthenticationProcessor.this.getFlowPath(); } + @Override + public String getFlowId() { + return AuthenticationProcessor.this.getFlowId(); + } + @Override public ClientConnection getConnection() { return AuthenticationProcessor.this.getConnection(); @@ -935,6 +946,7 @@ public Response handleClientAuthException(Exception failure) { } public AuthenticationFlow createFlowExecution(String flowId, AuthenticationExecutionModel execution) { + AuthenticationFlowModel flow = realm.getAuthenticationFlowById(flowId); if (flow == null) { logger.error("Unknown flow to execute with"); @@ -996,7 +1008,16 @@ public static void resetFlow(AuthenticationSessionModel authSession, String flow authSession.setAuthenticatedUser(null); authSession.clearExecutionStatus(); authSession.clearUserSessionNotes(); + String client_execution = authSession.getAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION); + String client_flow = authSession.getAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID); authSession.clearAuthNotes(); + //keep CLIENT_AUTHENTICATION_EXECUTION and CLIENT_FLOW_ID if they exist + if (client_execution != null) { + authSession.setAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION, client_execution); + } + if (client_flow != null) { + authSession.setAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID, client_flow); + } Set requiredActions = authSession.getRequiredActions(); for (String reqAction : requiredActions) { diff --git a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java index 8451f8d79752..615efdc97440 100755 --- a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java @@ -153,6 +153,20 @@ public Response processAction(String actionExecution) { } else return response; } + @Override + public Response continueClientAuthAfterIdPLogin(AuthenticationExecutionModel model){ + AuthenticatorFactory factory = getAuthenticatorFactory(model); + Authenticator authenticator = createAuthenticator(factory); + AuthenticationProcessor.Result result = processor.createAuthenticatorContext(model, null, null); + authenticator.action(result); + Response response = processResult(result, true); + if (response == null) { + return continueAuthenticationAfterSuccessfulAction(model); + } else { + return response; + } + } + /** * Called after "actionExecutionModel" execution is finished (Either successful or attempted). Find the next appropriate authentication diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/IdentityProviderAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/IdentityProviderAuthenticator.java index fdf7d7732347..f6ca459f39a2 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/IdentityProviderAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/IdentityProviderAuthenticator.java @@ -22,6 +22,7 @@ import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.authentication.Authenticator; import org.keycloak.constants.AdapterConstants; +import org.keycloak.events.Details; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -78,6 +79,8 @@ protected void redirect(AuthenticationFlowContext context, String providerId) { protected void redirect(AuthenticationFlowContext context, String providerId, String loginHint) { IdentityProviderModel idp = context.getSession().identityProviders().getByAlias(providerId); if (idp != null && idp.isEnabled()) { + context.getAuthenticationSession().setAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID, context.getFlowId()); + context.getAuthenticationSession().setAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION, context.getExecution().getId()); String accessCode = new ClientSessionCode<>(context.getSession(), context.getRealm(), context.getAuthenticationSession()).getOrGenerateCode(); String clientId = context.getAuthenticationSession().getClient().getClientId(); String tabId = context.getAuthenticationSession().getTabId(); @@ -101,6 +104,9 @@ protected void redirect(AuthenticationFlowContext context, String providerId, St @Override public void action(AuthenticationFlowContext context) { + //If IDENTITY_PROVIDER_USERNAME is not null, then user has logged in via IdP -> return success + if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null) + context.success(); } @Override diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java index 15021167e6c9..1673dea7e43b 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java @@ -19,8 +19,10 @@ import org.keycloak.WebAuthnConstants; import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.authentication.AuthenticatorUtil; import org.keycloak.authentication.Authenticator; +import org.keycloak.events.Details; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -52,18 +54,21 @@ public UsernamePasswordForm(KeycloakSession session) { @Override public void action(AuthenticationFlowContext context) { - MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); - if (formData.containsKey("cancel")) { - context.cancelLogin(); - return; - } else if (webauthnAuth != null && webauthnAuth.isPasskeysEnabled() - && (formData.containsKey(WebAuthnConstants.AUTHENTICATOR_DATA) || formData.containsKey(WebAuthnConstants.ERROR))) { - // webauth form submission, try to action using the webauthn authenticator - webauthnAuth.action(context); - return; - } else if (!validateForm(context, formData)) { - // normal username and form authenticator - return; + // If IDENTITY_PROVIDER_USERNAME is not null, user has logged in via IdP -> return success + if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) == null) { + MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); + if (formData.containsKey("cancel")) { + context.cancelLogin(); + return; + } else if (webauthnAuth != null && webauthnAuth.isPasskeysEnabled() + && (formData.containsKey(WebAuthnConstants.AUTHENTICATOR_DATA) || formData.containsKey(WebAuthnConstants.ERROR))) { + // webauth form submission, try to action using the webauthn authenticator + webauthnAuth.action(context); + return; + } else if (!validateForm(context, formData)) { + // normal username and form authenticator + return; + } } context.success(PasswordCredentialModel.TYPE); } @@ -86,6 +91,8 @@ protected boolean alreadyAuthenticatedUsingPasswordlessCredential(Authentication public void authenticate(AuthenticationFlowContext context) { MultivaluedMap formData = new MultivaluedHashMap<>(); String loginHint = context.getAuthenticationSession().getClientNote(OIDCLoginProtocol.LOGIN_HINT_PARAM); + context.getAuthenticationSession().setAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID, context.getFlowId()); + context.getAuthenticationSession().setAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION, context.getExecution().getId()); String rememberMeUsername = AuthenticationManager.getRememberMeUsername(context.getSession()); diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 85189ee26007..0d9bd799e505 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -25,6 +25,8 @@ import org.keycloak.broker.provider.UserAuthenticationIdentityProvider; import org.keycloak.http.HttpRequest; import org.keycloak.OAuthErrorException; +import org.keycloak.authentication.AuthenticationFlow; +import org.keycloak.authentication.AuthenticationFlowException; import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; import org.keycloak.authentication.authenticators.broker.util.PostBrokerLoginConstants; @@ -52,6 +54,7 @@ import org.keycloak.locale.LocaleSelectorProvider; import org.keycloak.models.AccountRoles; import org.keycloak.models.AuthenticatedClientSessionModel; +import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientSessionContext; @@ -913,6 +916,41 @@ private Response finishBrokerAuthentication(BrokeredIdentityContext context, Use logger.debugf("Performing local authentication for user [%s].", federatedUser); } + //code for returning to client browser flow + String execution = authSession.getAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION); + String flowId = authSession.getAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID); + + if (execution != null && flowId !=null) { + AuthenticationExecutionModel model = realmModel.getAuthenticationExecutionById(execution); + //remove the CLIENT_AUTHENTICATION_EXECUTION and CLIENT_FLOW_ID from session + authSession.removeAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION); + authSession.removeAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID); + + AuthenticationProcessor processor = new AuthenticationProcessor(); + processor.setAuthenticationSession(authSession) + .setFlowPath(LoginActionsService.AUTHENTICATE_PATH) + .setBrowserFlow(true) + .setFlowId(flowId) + .setConnection(clientConnection) + .setEventBuilder(event) + .setRealm(realmModel) + .setSession(session) + .setUriInfo(session.getContext().getUri()) + .setRequest(request); + processor.setAutheticatedUser(federatedUser); + AuthenticationFlow authenticationFlow = processor.createFlowExecution(flowId, model); + //maybe find next flow + Response challenge = authenticationFlow.continueClientAuthAfterIdPLogin(model); + if (challenge != null) { + return challenge; + } + if (!authenticationFlow.isSuccessful()) { + throw new AuthenticationFlowException(authenticationFlow.getFlowExceptions()); + } + } else { + logger.warn("CLIENT_AUTHENTICATION_EXECUTION and CLIENT_FLOW_ID are not included in the authenticationSession"); + } + AuthenticationManager.setClientScopesInSession(session, authSession); String nextRequiredAction = AuthenticationManager.nextRequiredAction(session, authSession, request, event); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java index 4743e6949095..51afde1d31c4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java @@ -6,9 +6,11 @@ import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.util.Time; +import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.models.utils.TimeBasedOTP; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; @@ -37,6 +39,7 @@ import java.net.URI; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -75,6 +78,9 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { @Rule public AssertEvents events = new AssertEvents(this); + private static final String Browser_Conditional_OTP = "Browser - Conditional OTP"; + private static final String Browser = "browser"; + protected void createRoleMappersForConsumerRealm() { createRoleMappersForConsumerRealm(IdentityProviderMapperSyncMode.FORCE); } @@ -566,6 +572,15 @@ public void testLogoutWorksWithTokenTimeout() { */ @Test public void testWithLinkedFederationProvider() { + + //disable otp for browser flow + List executionReps = adminClient.realm(bc.consumerRealmName()).flows().getExecutions(Browser); + AuthenticationExecutionInfoRepresentation exec = executionReps.stream().filter(authExec -> authExec.getDisplayName().equals(Browser_Conditional_OTP)).findFirst().orElse(null); + if (exec != null) { + exec.setRequirement(AuthenticationExecutionModel.Requirement.DISABLED.name()); + adminClient.realm(bc.consumerRealmName()).flows().updateExecutions(Browser, exec); + } + try { updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); @@ -607,6 +622,11 @@ public void testWithLinkedFederationProvider() { } finally { removeUserByUsername(adminClient.realm(bc.consumerRealmName()), "test-user"); removeUserByUsername(adminClient.realm(bc.consumerRealmName()), "test-user-noemail"); + + if (exec != null) { + exec.setRequirement(AuthenticationExecutionModel.Requirement.CONDITIONAL.name()); + adminClient.realm(bc.consumerRealmName()).flows().updateExecutions(Browser, exec); + } } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java index 5516b6738902..bac03d66a39a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java @@ -93,6 +93,9 @@ public void testReAuthenticateWithPasswordAndConditionalOTP_otpRequested() { loginTotpPage.assertCurrent(); loginTotpPage.login(totp.generateTOTP(totpSecret)); + //return to client flow -> otp has been configured -> ask again + loginTotpPage.login(totp.generateTOTP(totpSecret)); + assertUserAuthenticatedInConsumer(consumerRealmUserId); } @@ -131,7 +134,7 @@ public void testReAuthenticateWithPasswordOrOTP_otpConfigured_passwordUsed() { // Create user and link him with TOTP String consumerRealmUserId = createUser("consumer"); - addTOTPToUser("consumer"); + String totpSecret = addTOTPToUser("consumer"); loginWithBrokerAndConfirmLinkAccount(); @@ -150,6 +153,9 @@ public void testReAuthenticateWithPasswordOrOTP_otpConfigured_passwordUsed() { Assert.assertTrue(passwordPage.isCurrent("consumer")); passwordPage.login("password"); + //return to client flow -> otp has been configured -> ask again + loginTotpPage.login(totp.generateTOTP(totpSecret)); + assertUserAuthenticatedInConsumer(consumerRealmUserId); } @@ -184,6 +190,9 @@ public void testReAuthenticateWithPasswordOrOTP_otpConfigured_otpUsed() { // Login with OTP now loginTotpPage.login(totp.generateTOTP(totpSecret)); + //return to client flow -> otp has been configured -> ask again + loginTotpPage.login(totp.generateTOTP(totpSecret)); + assertUserAuthenticatedInConsumer(consumerRealmUserId); } From 11c733371e4d3ff20bb1bd4fd9b45b3591eaf070 Mon Sep 17 00:00:00 2001 From: cgeorgilakis-grnet Date: Tue, 4 Nov 2025 17:33:36 +0200 Subject: [PATCH 2/2] fix --- .../org/keycloak/testsuite/broker/KcOidcPostBrokerLoginTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcPostBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcPostBrokerLoginTest.java index 14109a84d8c0..684d8357c1e5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcPostBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcPostBrokerLoginTest.java @@ -252,6 +252,7 @@ public void testPostBrokerLoginFlowWithOTP() { loginPage.open(bc.consumerRealmName()); logInWithBroker(bc); + loginTotpPage.login(totp.generateTOTP(totpSecret)); appPage.assertCurrent(); AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin()); AccountHelper.logout(adminClient.realm(bc.providerRealmName()), bc.getUserLogin());