add SlidingCookie plug

[mwri]

Adds a SlidingCookie plug which implements a sliding window/session behaviour, allowing the remember_me cookie to be replaced before it expires.

Configure (or use plug option) like so:

config :my_app, MyApp.Guardian,
  ttl: {10, :minutes},
  sliding_cookie: {5, :minutes}

Add to Guardian pipeline, for example:

  plug Guardian.Plug.VerifyCookie
  plug Guardian.Plug.SlidingCookie
  plug Guardian.Plug.LoadResource
  plug Holco.Web.Plug.PostAuth

Provide sliding_cookie callback implementation:

defmodule MyAppWeb.Guardian do
  use Guardian, otp_app: :my_app
  def subject_for_token(user_rec, claims), do: ......
  def resource_from_claims(claims), do ......

  def sliding_cookie(claims, resource, opts) do
    {:ok, %{"new_claim" => "whatevs"}}
  end
end

[codecov-io]

Codecov Report

Merging #616 into master will increase coverage by 0.2%.
The diff coverage is 91.3%.

@@            Coverage Diff            @@
##           master     #616     +/-   ##
=========================================
+ Coverage   86.32%   86.52%   +0.2%     
=========================================
  Files          21       22      +1     
  Lines         424      438     +14     
=========================================
+ Hits          366      379     +13     
- Misses         58       59      +1

Impacted Files	Coverage Δ
lib/guardian/plug/verify_cookie.ex	90% <ø> (-1.31%)	⬇️
lib/guardian/token/jwt.ex	85.71% <100%> (+0.34%)	⬆️
lib/guardian/plug.ex	85.18% <100%> (+0.56%)	⬆️
lib/guardian.ex	89.28% <83.33%> (-0.72%)	⬇️
lib/guardian/plug/sliding_cookie.ex	92.3% <92.3%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9286581...0236c17. Read the comment docs.

[mwri]

Ah ha. Fixed formatting :)

[mwri]

Rebased on master and added :halt plug option following merge of #617. I'm not sure it really ever makes sense not to halt when the implementation is broken, but I think it's nice to be entirely consistent with the other plugs.

[Hanspagh]

Very cool idea and makes the remeber_me and verify token much more useful. 👍
I think it is really important to note that this can cause a session to be authorized indefinitely, but you already touch upon this.

I have added a few comments apart from this, great PR

[Hanspagh]

It might make sense to move this to https://github.com/ueberauth/guardian/blob/master/lib/guardian/token/jwt.ex and do a small refactor to avoid a bit of code dublication

[mwri]

Well.. it's funny you should say this, as a version of this logic did previously exist in jwt.ex, I necessarily decoupled the Map.put stuff in assign_exp_from_ttl from the actual ttl parsing to avoid the duplication, and then moved it out of jwt.ex to guardian.ex as I viewed it as a general utility function; it didn't seem to have anything really to do with the JWT token implementation at all as such at that point... and calling code from jwt.ex to parse a config option just didn't seem quite right to me... I'll move it to jwt.ex if you like though.

[Hanspagh]

I think you are right, that it should actually live in guardian, and we should just refactor jwt to use the code from guardian and not the other way around

[Hanspagh]

I am thinking this sounds a bit backward. Would it make sense that, what we configure is the minimum remaining time of the token, before we would refresh it? For me, this is mostly the way I think about refresh time

I think the current way can cause some trouble if you change the ttl of the refresh token.
Say we have a refresh ttl of 7 days, and we want to refresh only when there is one day left, we would set the sliding ttl to 6 days. Changing the refresh ttl to 14 would cause the refresh to happen when there are still 8 days left.

@mwri @ueberauth/developers Let me hear what you think

[mwri]

I'm absolutely with you on this, I tossed up about which way to go to be honest, a time after which it's renewed vs a time relative to the expiry... I'm very happy to change it to a minimum time remaining instead.

[Hanspagh]

I think that would make sense :D

[Hanspagh]

Maybe we should share this function between verifyCookie and slidingCookie?

[mwri]

Sure. I don't like code duplication but let this pass as it was a small function, and I wasn't sure it was worth coupling the two modules to share it. I don't feel strongly either way though, happy to call the VerifyCookie version.

[mwri]

Ah, could move it to plug.ex.

[Hanspagh]

Would be great I think we have some code in plug that basically does this aswell

[mwri]

I looked at that, it has quite a lot in common with fetch_token_key/2, but it they don't really refactor together nicely IMO; I think the result would just be more abstract less clear code. The kicker is find_token_from_cookies/2 does token = conn.req_cookies[key] || conn.req_cookies[to_string(key)], using a string and atom version of the key. I'm not sure if this is necessary, the unit tests certainly pass fine with just token = conn.req_cookies[to_string(key)]` and my app is ok with it, but I'm a little reluctant to change such an obviously deliberate effort without more research, I don't know what behaviour older versions of Phoenix might have had that might necessitate it, or something else I haven't thought of...

[mwri]

Sorry, I don't know how I accidentally closed it there, must have hit a key when the button was highlighted or something.

[mwri]

Agreed changes completed, I think.

[Hanspagh]

Awesome work 👍

[Hanspagh]

Would love to have a view from one other @ueberauth/core and a rebase from you @mwri

[mwri]

Rebased.

[mwri]

Cheers. Always a pleasure to contribute a well received PR.