ueberauth / guardian

Elixir Authentication
MIT License
3.44k stars 381 forks source link

sign_out does not clear the remember_me cookie #507

Closed obrok closed 5 years ago

obrok commented 6 years ago

Thanks for including this functionality in the first place - it's nice to have it done by someone more clever than me.

To the point: it seems initially there was some question if MyApp.Guardian.Plug.sign_out should or shouldn't clear the remember_me cookie (here https://github.com/ueberauth/guardian/pull/419). It seems that was skipped for this release, however, it seems the most intuitive for me for sign_out to clean that cookie.

Is there a clean way to do that for now? I can't seem make out what that would be, as there is quite some indirection going on in VerifyCookie. How is this going to work going forward?

Hanspagh commented 6 years ago

I would think it would be possible to use https://hexdocs.pm/plug/Plug.Conn.html#delete_resp_cookie/3 to remove it when you sign out.

yordis commented 6 years ago

I am not sure if cleaning up the cookie is what we should do since remember me is for memorizing the who the user was.

Logging off doesn't mean the computer can't memorize who you were.

For example,

Google shows you a list of accounts that you signed up or logged in in your computer.

So I don't believe that we should remove the cookie because this is based on the use cases.

That being said, maybe add some flag that cleans up the cookie for you, maybe?!

obrok commented 6 years ago

That being said, maybe add some flag that cleans up the cookie for you, maybe?!

From my perspective the easiest thing would be to have a function that reverses the effect of remember_me. Currently the interface allows me to remember_me easily, but there is no obvious, same-level way to reverse that. For example, if I just wanted to stop "remembering" the user, but not destroy the session entirely it seems like I have to dig around the implementation to figure out where the remember_me information is stored.

yordis commented 6 years ago

@obrok fair enough, we could add some function for clean up remember_me and you decide to clean it up or not in the callbacks.

Hanspagh commented 6 years ago

@yordis Are you working on this?

yordis commented 6 years ago

@Hanspagh until the next month and I am off the computer.

If you want to take the task, by all means, go ahead and do it.