search

canonical / identity-platform-login-ui

Login UI for the Canonical identity broker and identity provider solution
Apache License 2.0
11 stars 6 forks source link

Sign in with security key not working if email is cached #290

Closed lukasSerelis closed 1 week ago

lukasSerelis commented 3 weeks ago

What is happening If you try to sign in but don't complete MFA and visit the initial sign in page again, it caches the email and only shows password and security key sign up options, even if security key is not set up. If the security key is not set up it's not putting an error message, it just keep redirecting back to the same page.

What should happen First of all it shouldn't obscure the email address that is cached, it's bad UX practice as the user might be trying to sign in to a different account, and you're making them guess which email has been cached. Display the email even thought it's been cached. Secondly, either display all methods of sign in or only supported ones, cause now you're doing neither.

Preferred flow image

Flow in screens 1) User visits resource that is accessible via Login UI. UI displays all of possible sign in methods image

2) User signs in with email and password. Login UI asks for MFA. User closes the tab image

3) User visits resource that is accessible via Login UI. UI displays cached email with ability to edit email, and all sign in methods that this email is able to use (in this example, email has security key and google, but not Okta set up for this identity.) image

Compromise option Same flow, but display all of the possible sign in option, regardless of what options are possible. image

syncronize-issues-to-jira[bot] commented 3 weeks ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/IAM-1114.

This message was autogenerated

nsklikas commented 3 weeks ago

I don't really like the approach we take here:

We should have a look at what major providers do and copy them. For example on github (not the best example, I know) if you press back on the authorizer screen, you will be asked to enter your username/password again and if you do browser forward, you will again will be asked for totp.

I think for us the best flow would one of the following:

What do you think? cc @lukasSerelis @edlerd

Originally posted by @nsklikas in https://github.com/canonical/identity-platform-login-ui/pull/292#pullrequestreview-2337635863

lukasSerelis commented 3 weeks ago

I don't really like the approach we take here:

  • the button that says Change e-mail, seems weird/counter-intuitive to me as a user.
  • The logic for resetting aal1 feels very hacky and I think will be difficult for someone who does not have our context to extend.
  • Even though we ask the user to provide their password, this is not needed. The session is cached in the cookies (we can just redirect them to the 2fa page.

We should have a look at what major providers do and copy them. For example on github (not the best example, I know) if you press back on the authorizer screen, you will be asked to enter your username/password again and if you do browser forward, you will again will be asked for totp.

I think for us the best flow would one of the following:

  • show the user a message that says something along the lines Please validate that this is you: {email} and underneath 2 buttons (one for accepting and one for declining). If they decline, then we delete the user session and we show the login screen as well. If they accept, then we set the aal param to aal=aal2 in the query param. This will show the user the totp page again.
  • Do something similar to github and automatically reset the session and clear both fields, this would need some more work as we would want to keep the forward button working as well.

What do you think? cc @lukasSerelis @edlerd

Originally posted by @nsklikas in #292 (review)

The github option is probably less fussy. Even if we don't have the browser forward action working it is a less convoluted flow. The change email and caching email is a thing a lot of SSOs do as well, so this isn't necessarily a unique flow. But I agree that it'd be simpler to just clear all fields. More frustrating in some cases, but less complex, and I'll always lean to less complexity. I'll update the flows to match this and then we can go forward with the github style of not saving either field imho.

lukasSerelis commented 3 weeks ago

image @nsklikas @edlerd

Flow that I agree with Nikos is much more simplified. Nothing is cached. The only thing is a question of implementation if we can make it work but would be nice is the going forward in browser after going back to see working MFA again...

edlerd commented 3 weeks ago

Flow that I agree with Nikos is much more simplified. Nothing is cached. The only thing is a question of implementation if we can make it work but would be nice is the going forward in browser after going back to see working MFA again...

Kratos is controlling which flow will be rendered. For a user coming with a kratos session, kratos initiates the "Enter password" with immutable email flow. I have no idea how to change this behaviour other than removing the cookie. Doing that automatically in the frontend -- without user interaction, as the flows suggest -- seems very hacky to me. Happy for other suggestions, maybe you can configure kratos accordingly?

nsklikas commented 3 weeks ago

I don't know about the possible kratos configurations off the top of my head (probably others are more suitable in answering that). But worst case possible we could handle this in the backend. There are 2 options:

Both of these should be possible, but I will have to validate it.

edlerd commented 1 week ago

@nsklikas I think this has been fixed with #301 Can this be closed?

nsklikas commented 1 week ago

I think so, but I would leave it up to @lukasSerelis to decide

edlerd commented 1 week ago

Closing after talking to Lukas and he agreed.