Display "Wrong credentials" on SignInLive
Hi,
I have tried to tweak the AuthController to redirect to the SignIn with an error message in case of failure.
The flash isn't available on the single liveview component, is there a built-in "Wrong credentials" message on SignInLive?
This made me consider that maybe the normal use isn't to open directly the "/sign-in" route to liveview, but to embed it in a page with a layout?
Cheers
27 Replies
The way that I'm currently doing this is to redirect to the sign in page with flash on the root layout.
Although I think this interaction needs a bit of work. We need to submit the action over a form post, but we could technically check the password in the liveview, and then submit the action over the form post. Would be checking their password twice, but would give the incorrect credentials message right away. Thoughts?
@jart thoughts on this one?
Actually its the weekend for you
You're a day ahead š
I forget
Thanks for the reply, here are some thoughts gathered over the weekend.
I think the flash message is not "local enough" and makes for less clear feedback and not great UX. This is why I would favor integrating the feedback in the
SignInLive
component.
This way, we can get clear messages mapping to the _reason
of failure, right next to the forms.
Hopefully, the messages will be easy to override and internationalize. This is important to me at least.
This brings the following considerations for the AuthController
: if LiveView intercepts failure, the role of the failure
function is unclear. I think it will be confusing if it is called only in "some" cases of failure. Will it ever be called, in fact?
You know more about the usage of the failure
function in the wild, but in my experience, 90+% of auth failure, render to the sign in form again, with some error message. No redirect is performed until validation passes and auth succeeds.
So I think validation in LiveView and dropping failure
function could be both convenient and lead to least surprise for devs (assuming things like rate and retry limiting, last connection, and such are handled at resource level by AshAuthentication š ).
What you think of these points?A few things: i18n will be itās own discussion. Ash errors provide what you need to do it, but weāll need to add things to ash authentication phoenix to support it.
I agree on the UX pice as well.
However, Iām not sure if we will be able to entirely drop the failure handler
Because it could fail on the redirect with a token that we will have to do after successful sign in.
But that kind of failure would be exceedingly rare, and so a flash message is probably acceptable for it
Yeah, I agree w/ that assessment as well.
Yeah, I've mostly played with password auth so far, so I'm speaking from this incomplete perspective.
Agree that a flash is fine for those cases that don't involve forms š
Yeah I donāt think itās a good idea. I am worried that we will leave ourselves exposed to timing attacks. I think perhaps weād be better off to provide helpers so that you can put your own error message in the failure handler and we can pull it out of the conn and render it inside the form.
So something like:
Thatās a dumb name but you get the idea.
Also the failure handler is called any time thereās a failure in any of the plugs so also on registration failures too. Thatās why we pass the strategy and phase back.
What kind of timing attacks do you have in mind?
The flow I'm imagining that makes this UI better is that the
/sign-in
route can also be called as /sign-in?identity=user&token="<token>"&<identity_field>=<identity_value>
And then it would validate the token as if it had been passed in an authentication header and sign you in
So then we could submit the action in the liveview, and then redirect to /sign-in?...
The token contains enough information on its own to do that.
But also thatās a new strategy.
ah, so then just
?token="<token>"
I think we can just make a strategy that signs in that way.
Does it have to be a new strategy?
I think it makes the most sense that way otherwise weāre overloading the live view with a bunch of special case crud whereas at the moment theyāre nice and clean.
There is nothing wrong with making http calls. We did it for years.
I mean itās such a trivial example you would write it in a plug in a few lines.
Yeah, no one is saying there is a problem with HTTP calls, but the UX of the http call in the context of a LV isn't as good.
I'm not sure I see the issue with it TBH. All you'd add is
And by "all you'd add" I mean "all that would have to be added"
and then the existing
sign-in
route can handle the token
parameter being present, and then redirect you to where you actually want to goOkay maybe I am having fifiās about people wanting to change my baby. Let me have a think about this.
One thing you're right about though is that this technically allows password enumeration in the UI without the ability to rate limit using a plug.
And thats not so good.
Well... Perhaps another option is to simply render the flash inside the form, right?
Phoenix.Flash.get(@flash, @kind)
?Yeah, I actually made a branch that did that
but it gets weird š
Normal forms will clear the flash when you have modified the form
Well...actually I guess we can just track if the form has been altered at all and hide the message
Can give it a special type of
:kind
as well so it doesn't get pick up by the user's normal flash UI.Yeah, I had it as
authentication_error
and then a helper function of set_authentication_error(...)
in the failure handlerlike
:ash_auth_username_and_password
or something.The other sort of annoying thing is that you have to pass the flash down through all of the components
but that would technically be the "best of both worlds"
Yeah, I really do think that would probably be the more flexible and secure way to go about it... But I'm certainly no expert on the matter.
Did you just come around to basically what I was thinking? Jimsy win!
š well, I still don't like the UX of it
but unless we add rate limiting at the action level as a default feature, then its probably not okay to submit it in the LV
Although FWIW it would probably be so trivial to have
ash_rate_limiter
that comes with changes you can drop into an action that will rate limit based on data/context/actor/arguments
It would use some existing hex dependency to do the rate limiting, and then we could set context on the actions when invoked from web interfaces, like ip/ip region and stuff like that.
but like...not right now š
So yes, Jimsy win š
I think we should perhaps put some docs on this though, i.e "why don't we log in with the liveview: "
People will understand that we make concessions when it comes to authentication for the sake of security/securability (they still need to write these plugs if they want theM)
but we do need a way to show the errors better, and the special flash message will accomplish that šYup I agree with that
Can someone open a GH issue about this on the repo?
I think this needs to happen for UX, but we don't need this forum support page open anymore
Done