Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infinite login_url loop when hints are not met #604

Open
panva opened this issue May 24, 2024 · 7 comments
Open

infinite login_url loop when hints are not met #604

panva opened this issue May 24, 2024 · 7 comments

Comments

@panva
Copy link

panva commented May 24, 2024

In the implementation that i'm testing with (127.0.6496.0 canary with FedCmButtonMode and FedCmAuthz flags) if the RP provides either a loginHint or a domainHint that doesn't match the account's endpoint response it invokes the IdP's login_url.

When the result of the IdP sign-in flow finishes and the loginHint or domainHint are still not met, the login_url is invoked again, and again, and again.

The only way to get out of it is to close the popup. I imagine this would not be a good experience in a mobile browser.

I believe if the hints cannot be satisfied after the first invocation it should not be repeated and the whole promise rejected.


Furthermore, in the build i'm testing with the "show an IDP login dialog helper algorithm" which appends the two hints to the login_url query does not work, there are no additional query string parameters at the invoked url. I think this affordance may have been removed from the implementation because it allows the RP to pass unbound context to the login url at the IdP before consent was given? It's still in the spec though and without the hints present, how's the IdP going to know which way to steer the login experience in?

@npm1
Copy link
Collaborator

npm1 commented May 24, 2024

There are some cases where the login url includes the hints and some cases where it does not, so I'd need more information about how you arrived to the url to know if it is an issue or not. The 'infinite loop' part is something we are aware of and working to improve in Chrome UX.

@panva
Copy link
Author

panva commented May 24, 2024

There are some cases where the login url includes the hints and some cases where it does not

Can you elaborate?

The 'infinite loop' part is something we are aware of and working to improve in Chrome UX.

Good to know.

@panva
Copy link
Author

panva commented May 27, 2024

so I'd need more information about how you arrived to the url to know if it is an issue or not

When button mode is used the query strings are not populated.

Screen.Recording.2024-05-27.at.10.37.58.mov

@npm1
Copy link
Collaborator

npm1 commented Jun 11, 2024

Sorry, forgot to reply. Based on looking at the code right now, I think the parameters are passed when the login URL is shown as a result of the mismatch UI. The parameters are not being passed when the login URL is shown as a result of button mode or use another account. Does that match your testing? Note that these APIs are experimental so subject to change. But I believe the idea was that these APIs in theory could have a lower bar in terms of how the login URL is opened (for instance, only user interaction is sufficient for button mode), so we wanted to avoid leaking information in that scenario.

@panva
Copy link
Author

panva commented Jun 12, 2024

This should be reliable and consistent across login URL invocations if it's to serve its purpose.

@npm1
Copy link
Collaborator

npm1 commented Jun 12, 2024

I hear you that this is not ideal, but in button mode and in particular when the user is logged out, there is no browser UI before we open the url to login the user. Since the login_hint could contain arbitary information, we cannot let the RP pass this to the top-level IdP frame. Any thoughts on how to make this better are welcome!

@samuelgoto
Copy link
Collaborator

One option that I think may be worth exploring is to have the browser UI show the accounts that are logged-in but don't match the login_hint as "disabled", so that the user knows that they need to login to another account other than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants