Continue client browser flow after User login from Identity Provider by cgeorgilakis · Pull Request #12891 · keycloak/keycloak

cgeorgilakis · 2022-07-04T10:36:26Z

Our implementation saves in AuthNotes of AuthenticationSessionModel all needed information for Client Browser Flow ( AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION of Client Browser Flow, flowid) before redirecting to Identity Provider login. After Identity Provider login has successfully finished, Keycloak can retrieve all needed information for Client Browser Flow from AuthNotes of AuthenticationSessionModel and proceed with Client Browser Flow.

Browser flows execution are now continuing after Identity Provider Login. That's why some tests need to be changed. Some changes indicate that Browser flow with Identity Provider login, is working well. I do not know if we need extra tests.
More tests failures are related with SamlClientBuilder and its related code. I do not know well test implementation of SamlClientBuilder in order to make appropriate changes in it or found possible errors in my code. Finally, KcOidcBrokerTest/KcSamlBrokerTest test method testPostBrokerLoginFlowWithOTP failure is strange.

cgeorgilakis · 2022-07-04T13:26:23Z

I have added a check for CLIENT_AUTHENTICATION_EXECUTION and CLIENT_FLOW_ID not being null in method finishBrokerAuthentication of IdentityBrokerService for overcome errors and test failures. A warn is logged if at least one of CLIENT_AUTHENTICATION_EXECUTION and CLIENT_FLOW_ID is null. However, I am not sure if this check is needed and I do not understand why these variables are null in some cases. Is it related with test authedication flow implementation?

cgeorgilakis · 2022-10-04T08:52:11Z

After rebasing some test methods( AbstractAdvancedBrokerTest.testPostBrokerLoginFlowWithOTP_bruteForceEnabled, KcOidcFirstBrokerLoginNewAuthTest.testReAuthenticateWithPasswordAndConditionalOTP_otpRequested, KcOidcFirstBrokerLoginNewAuthTest.testReAuthenticateWithPasswordOrOTP_otpConfigured_otpUsed) are failing. The problem is that otp is failed - added by loginTotpPage.login(totp.generateTOTP(totpSecret));. This happens when otp check is passed during IdP flow and Client flow requires otp again after successful IdP login. Why this happen? How can I fix it?

Moreover, SAML tests ( methods BrokerTest.testInResponseToSetCorrectly, BrokerTest.testLogoutPropagatesToSamlIdentityProvider) are not failing only for quarkus! Why? How can I fix it?

cgeorgilakis · 2024-03-12T08:05:20Z

I have rebased it. Now, only failure tests is related to AbstractAdvancedBrokerTest.testPostBrokerLoginFlowWithOTP_bruteForceEnabled.

mposolda · 2024-03-13T13:45:47Z

Thanks a lot for the PR and sorry for it not yet being reviewed. We will try to review this once we have some time...

trixpan · 2024-04-01T23:37:16Z

@mposolda any luck with this review?

JSmith-Aura · 2024-04-22T22:54:59Z

Any news on this? Just discovered this bug myself and am very keen to get it fixed....

thereiam · 2024-04-26T12:48:18Z

Same here. It would be amazing if this bug could be fixed! Thank you for all your great work.

NicolasLiampotis · 2024-04-30T07:29:54Z

Looking forward to the review whenever the team has a chance.

tulequ · 2024-05-02T17:25:09Z

Looking for this feature!!!

piaoyunbo · 2024-06-05T11:38:38Z

Any updates on this?Looking for this feature!!!

JeffResc · 2024-06-06T14:33:09Z

Any updates on this? We are looking forward to this feature

mixman68 · 2024-06-18T10:37:37Z

I need this feature too during my transition phase

jirutka

Fix code-style

Sign in to view

+        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);


Sign in to view

+        Response response = processResult(result, true);
+        if (response == null) {
+            return continueAuthenticationAfterSuccessfulAction(model);
+        } else return response;


Sign in to view


    private void redirect(AuthenticationFlowContext context, String providerId) {
-        Optional<IdentityProviderModel> idp = context.getRealm().getIdentityProvidersStream()
+       Optional<IdentityProviderModel> idp = context.getRealm().getIdentityProvidersStream()


Sign in to view

+        //BROKER_USER_ID not null user have been login with Idp -> return success
+        if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null)
+            context.success();


Sign in to view

-        }
-        if (!validateForm(context, formData)) {
-            return;
+        //BROKER_USER_ID not null user have been login with Idp -> return success


Sign in to view

+        String execution = authSession.getAuthNote(AuthenticationProcessor.CLIENT_AUTHENTICATION_EXECUTION);
+        String flowId =  authSession.getAuthNote(AuthenticationProcessor.CLIENT_FLOW_ID);
+
+        if (execution != null && flowId!=null) {


Sign in to view

+            AuthenticationFlow authenticationFlow = processor.createFlowExecution(flowId, model);
+            //maybe find next flow
+            Response challenge = authenticationFlow.continueClientAuthAfterIdPLogin(model);
+            if (challenge != null) return challenge;


Sign in to view

+            //return to client flow -> otp has been configured -> ask again
+            loginTotpPage.login(totp.generateTOTP(totpSecret));
+            waitForAccountManagementTitle();
+             AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin());


cgeorgilakis · 2024-07-04T12:35:17Z

Fix code-style

@jirutka I have make code-style changes suggested and I have rebased it. Thanks.
@jirutka @mposolda could we proceed with reviewing this important bug fix for supporting client authentication flows with broker users?

jirutka · 2024-07-04T12:53:12Z

+        if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null)
+            context.success();


Suggested change

if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null)

context.success();

if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null) {

context.success();

}

jirutka · 2024-07-04T12:58:39Z

Unfortunately, I’m not familiar with the Keycloak codebase so I can’t tell whether it’s conceptually and logically correct. However, I’m also affected by this bug/flaw and look forward to the review.

igor-nikiforov · 2024-10-22T04:00:23Z

@mposolda just a friendly ping here.

mposolda · 2024-12-18T10:20:38Z

@cgeorgilakis Sorry for the late review. Thanks for the PR. Unfortunately I think it makes sense even more complex and fragile (authentication flows are already quite fragile in general). And the test failures are also problematic...

I believe we can address your use-case (and hopefully most other use-cases as well) by some easier way. I've commented on the issue #10250 (comment) ).

Closes keycloak#10250 Signed-off-by: cgeorgilakis-grnet <[email protected]>

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch from 5243afe to da9ff6a Compare July 4, 2022 13:22

cgeorgilakis mentioned this pull request Aug 23, 2022

Ability to continue with "browser" authentication flow after Identity provider login #10250

Open

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch from da9ff6a to 0d511d8 Compare September 26, 2022 10:41

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch from c39ca9d to 0d511d8 Compare October 4, 2022 08:39

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch from 0d511d8 to 1114d78 Compare March 11, 2024 14:31

cgeorgilakis requested review from a team as code owners March 11, 2024 14:31

keycloak-github-bot Bot added team/cloud-native team/core-clients team/core-iam flaky-test labels Mar 11, 2024

keycloak-github-bot Bot mentioned this pull request Mar 11, 2024

Flaky test: org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#multipleTransports #23884

Closed

mposolda self-assigned this Mar 13, 2024

jirutka suggested changes Jul 3, 2024

View reviewed changes

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch 3 times, most recently from aec0feb to 447fff5 Compare July 4, 2024 12:29

jirutka suggested changes Jul 4, 2024

View reviewed changes

Continue client browser flow after User login from Identity Provider

553ab09

Closes keycloak#10250 Signed-off-by: cgeorgilakis-grnet <[email protected]>

cgeorgilakis force-pushed the RCIAM-1038-idpflow branch from 447fff5 to 553ab09 Compare November 4, 2025 10:26

keycloak-github-bot Bot mentioned this pull request Nov 4, 2025

Flaky test: org.keycloak.testsuite.oauth.ClientAuthSignedJWTTest#testClientWithGeneratedKeysJKS #43713

Closed

fix

11c7333

keycloak-github-bot Bot mentioned this pull request Nov 4, 2025

Flaky test: org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest#updateLDAPUsernameTest #43754

Closed

		if (context.getAuthenticationSession().getUserSessionNotes().get(Details.IDENTITY_PROVIDER_USERNAME) != null)
		context.success();

Conversation

cgeorgilakis commented Jul 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgeorgilakis commented Jul 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgeorgilakis commented Oct 4, 2022

Uh oh!

cgeorgilakis commented Mar 12, 2024

Uh oh!

mposolda commented Mar 13, 2024

Uh oh!

trixpan commented Apr 1, 2024

Uh oh!

JSmith-Aura commented Apr 22, 2024

Uh oh!

thereiam commented Apr 26, 2024

Uh oh!

NicolasLiampotis commented Apr 30, 2024

Uh oh!

tulequ commented May 2, 2024

Uh oh!

piaoyunbo commented Jun 5, 2024

Uh oh!

JeffResc commented Jun 6, 2024

Uh oh!

mixman68 commented Jun 18, 2024

Uh oh!

jirutka left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

cgeorgilakis commented Jul 4, 2024

Uh oh!

jirutka Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

jirutka commented Jul 4, 2024

Uh oh!

igor-nikiforov commented Oct 22, 2024

Uh oh!

mposolda commented Dec 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

cgeorgilakis commented Jul 4, 2022 •

edited

Loading

cgeorgilakis commented Jul 4, 2022 •

edited

Loading