Repository navigation
Improve session polling to prevent accidental redirects - #35143
Conversation
|
Putting this on hold until we're able to test that this indeed fixes the issue, please do not merge until this label is removed. |
|
As far as the team can tell this fix should work for the reported issue, we intend to merge this in next week Monday if there are no objections. |
|
@jonkoops I'm sorry for my late response.
Tested Authentication Methods and Results
The issue persists only in the case of Passkey authentication. Upon inspecting the HTML, I found that the "Sign in with Passkey" button does not use a form submission during authentication. As a result, the session polling event is not properly stopped. Here is the relevant HTML structure for Passkey authentication: <div class="pf-v5-c-form__group no-bottom-margin">
<form id="authn_select" class="pf-v5-c-form">
<input type="hidden" name="authn_use_chk" value="<redacted>"/>
<input type="hidden" name="authn_use_chk" value="<redacted>"/>
<input type="hidden" name="authn_use_chk" value="<redacted>"/>
</form>
<div id="kc-form-buttons" class="col-xs-12 col-sm-12 col-md-12 col-lg-12">
<input id="authenticateWebAuthnButton" type="button" autofocus="autofocus"
value="Sign in with Passkey"
class="pf-v5-c-button pf-v5-c-button pf-m-primary pf-m-block btn-lg"/>
</div>
</div>To resolve the issue in my environment, I added the following code to the proposed implementation, which attaches a const buttonInputs = document.querySelectorAll('input[type="button"]');
buttonInputs.forEach((input) =>
input.addEventListener("click", () => stopSessionPolling())
);While I have not thoroughly tested the impact of attaching |
Closes keycloak#33071 Signed-off-by: Jon Koops <jonkoops@gmail.com>
2afbfab to
cb5d7cf
Compare
|
@t-tani thank you so much for doing this detailed testing, it really helps us out! The reason why the form submission was not working is because we're calling
I went ahead an replaced all instances where |
|
@jonkoops I tested the updated implementation, in which Thank you very much for the fix and your efforts! |
|
Great to hear! I'll make sure to get this merged and backported ASAP. Thanks again for all your assistance! |
|
Backport for v26 is up under #35408 |
|
@mposolda The change from I would like to suggest rolling this back to the Side note: We also try to open a case at Microsoft to evaluate why they ship such an old embedded browser. |
|
@FlxPeters You can always create a custom theme for those old clients changing the method. |
|
@FlxPeters If you rollback the change then the issue #33071 is again in place. Which is for iphone users. So or we have issues in one place or in another, the only real fix would be separating by browsers that we are don't want to do. Somebody should have to change themes. IMHO it should be the one not supporting the new standards. But let's see what people think. |
Improves the session polling in the login flow used to detect user sign-ins from other contexts (e.g. another tab or window). This fixes an issue where a user experiences a redirect to another page when submitting the login form, causing an unexpected re-start authentication error.
This change also cleans up the code a little by renaming some functions to be more appropriate with their purpose, as well as adding additional comments to explain various parts of the code.
Closes #33071