fix remember_me plug

[Hanspagh]

Fix for #405 Whats need to be done:

• [x] Save the token under the key: guardian_"key"_token, so the verify cookie plug finds it
• [x] Set the secure property on the cookie, so it will only be sent over https
• [x] Set the max_age property on the cookie, should properly be the same as the ttl of the token, is there a convenient way to get the ttl of a token?
• [x] More tests
• [x] Update docs, see #386
• [x] Decide if sign_out should also clear the remeber_me cookie
• [x] Decide if we should ignore the token expired error and let Plug.EnsureAuthenticated handle it

[hassox]

I think I'd like to hold this cookie / remember me functionality back fro 1.0 so we can get it out the door then bring it back shortly after once we've had a good chance to kick the tires. What do you think?

[Hanspagh]

I am totally fine with delaying it or even closing this PR. Just wanted to notify that I actually started working on this, and get a possible solutions to the max age

[Hanspagh]

Okay, I fixed all the things on my to do, expect for

Decide if we should ignore the token expired error and let Plug.EnsureAuthenticated handle it #405

Do you have any opinions on this?

[Hanspagh]

I would love to finish this, but I would love a clear decisions on the token expired error . Should we just ignore it and assumePlug.EnsureAuthenticated will handle it later in the pipeline or does it actually make sense to have Guardian.Plug.VerifyCookie throw the error

[doomspork]

@Hanspagh I'm open to letting Plug.EnsureAuthenticated handle it, I think that keeps everything in one place. @ueberauth/core any objections to this?

[Hanspagh]

I changed the verify cookie plug to not handle the expired token and for now I will skip this sign_out should also clear the remeber_me cookie

[doomspork]

@Hanspagh is this good to review now? It appears all the items have been addressed 😀

[yordis]

Don't forget to run mix format (just remembering you about it 😅)

[doomspork]

@yordis I think elixir-lang enforces the formatter has been run by CI? We should do that if possible 😁

[Hanspagh]

@doomspork all done. @yordis isnt mix format only part of the v1.6.0-dev elixir version?

[yordis]

@Hanspagh yes (this is the first programming language that I have no fair for using dev version 😄)

[doomspork]

@hassox can you give this another look over before we merge and release?

[Hanspagh]

Requested changes. English isn't my first language so any feedback on the comments and docs is very welcome.

[doomspork]

Thank you @Hanspagh! @ueberauth/developers any final thoughts before we merge?

[robmadole]

@Hanspagh do we need a remember_me added to the __using__ macro so this will get mixed in when defining the Guardian module?

[Hanspagh]

@robmadole To be honest, I dont know.

[robmadole]

@Hanspagh to enable this from the README:

# Set a "refresh" token directly on a cookie.
# Can be used in conjunction with `Guardian.Plug.VerifyCookie`
conn = MyApp.Guardian.Plug.remember_me(conn, resource)

I'm 97% sure that it needs to be added to the Guardian.Plug __using__ macro.

[robmadole]

Also line 45 where the default cookie stuff is defined:

@default_cookie_options [secure: true, max_age: 60 * 60 * 24 * 7 * 4]

Probably should remove the secure: true because this will break local developement that isn't running on https. Plug.Conn.put_resp_cookie will handle setting this correctly according to the documentation.

[Hanspagh]

Sure, I can add it to the plug macro and remove the secure default. I guess its on the users if they save token in a cookie on a http connection.

[Hanspagh]

Hi, any status on when is could be accepted?

[yordis]

@ueberauth/core are good to go?

[doomspork]

@hassox / @scrogson any final feedback? You both spent more time reviewing this PR than I did.

[robmadole]

We've basically got this running in production for fontawesome.com. So it's had a bit of testing. I'd love to switch over to it once it's released (to get off my local Plugs)

[robmadole]

Oh one more thing, if you want to release a pre-release version I'll upgrade our app and give it a test.

[doomspork]

That's awesome to hear @robmadole. I'm merging this so we can cut a release this week 👍

[Hanspagh]

Awesome, thank you.