Refresh / Logout not working

[speeddragon]

I've been using past verions of Guardian \ Guardian DB and has been working fine. I've decided to update all packages, and I've update all sucessfully except this one.

I've check multiple times Guardian and Guardian Db documentation and can't find the solution for my problem.

I've 2 tests in my project, one to test refresh token and another to test logout. For refresh token I use the following code:

case MyApp.Guardian.refresh(existing_jwt, ttl: {1, :week}) do
      {:ok, {_old_token, _old_claims}, {new_jwt, _new_claims}} ->

For logout I use the following code:

MyApp.Guardian.revoke(jwt)

I've checked the documentation, it says to use Guardian.revoke! but that doesn't exists anymore.

The thing I notice is, using token_types: ["refresh_token"], refresh works, but logout doesn't. If I remove token_types: ["refresh_token"] logout works, but refresh doesn't. In documentation it says that if token_types isn't present, should keep everything, right ?

The error that I get when trying to pass the refresh test is:

Guardian.DB.on_verify(claims, token)
{:error, :token_not_found}

If I add token_types: ["refresh_token"], refresh tokens is valid, but the revoke token doesn't revoke the token (keep it valid until expires).

Am I missing something here ?

[yordis]

@speeddragon just for double check, is your token refresh_token type

Look at this https://github.com/ueberauth/guardian_db/blob/5e1a3df4c28f56ed71465583edf81b912bd6ac85/lib/guardian/db.ex#L123

When you do

def after_encode_and_sign(resource, claims, token, _options) do
    with {:ok, _} <- Guardian.DB.after_encode_and_sign(resource, claims["typ"], claims, token) do
      {:ok, token}
    end
  end

make sure that claims["typ"] is setup correctly or the checking will fail.

[speeddragon]

@yordis, that case always return :ok. The claims["typ"] is access.

Shouldn't we have something similar for on_refresh like after_encode_and_sign, on_verify and on_revoke ?

Like:

def on_refresh(old_stuff, new_stuff, _options) do
  with {:ok, _, _} <- Guardian.DB.on_refresh(old_stuff, new_stuff) # Save the token in DB?
    {:ok, old_stuff, new_stuff}
  end
end

[yordis]

@speeddragon if you have access type then you need to add it to the whitelist if I am not mistaking token_types: ["refresh_token", "access"] or just remove it all so you save all the tokens.

Shouldn't we have something similar for on_refresh like after_encode_and_sign, on_verify and on_revoke ?

I think, no sure, the answer is no because when you refresh you suppose to use exchange, so maybe on on_exchange would make more sense here I think.

But, this is tricky around JWT tokens at least.

@hassox the idea with the refresh is to invalidate the old one or just generate a new one? In JWT wouldn't make sense to deal with any database changes because the whole point is to be stateless so you would just start using the new token and forget about the old one.

But, we could add some helper on_exchange that would either

• invalidate the old one and create a new token
• create a new token

because if I am not mistaking, I could swap JWT token with some random token so at that moment the strategy need to support the database stuff.

[speeddragon]

@yordis, just removed is what I've talked about in my first post, refresh token is invalid and doesn't doesn't work.

In terms of pure JWT (without Database), refresh token should just only exchange tokens, because there isn't any method to invalidate the old token. Using Guardian DB you can leverage this to invalidate the old token and just use the refreshed (extended) token.

I know that in the previous implementation ({:guardian, "~> 0.14"}, {:guardian_db, "~> 0.8"},) it worked like I described above, on refresh it would invalidate the old token. Maybe that wasn't correct and it was fixed in this version, but at the moment I'm confuse.

[yordis]

@speeddragon yeah, I think we should add on_exchange callback for Guardian.DB so it doesn't the swap or remove the old one and create the new one, I need confirmation on that.

[speeddragon]

So, after get inside the code I have come up with a solution to mimic the old behaviour.

def on_refresh({old_token, old_claims}, {new_token, new_claims}, _options) do
    # Revoke old token
    Guardian.DB.on_revoke(old_claims, old_token)

    # Store new token
    Guardian.DB.after_encode_and_sign(%{}, new_claims["typ"], new_claims, new_token) 

    {:ok, {old_token, old_claims}, {new_token, new_claims}}
end

Don't know if it makes sense to create a PR with this.

[yordis]

@ueberauth/core what do you think? This seems to be what we should be doing to be honest.

[doomspork]

@speeddragon / @yordis I think a PR would be a good place to start the conversation around implementation 👍

[speeddragon]

@doomspork , can you take a look into PR #84 ?

[doomspork]

Merged @speeddragon 👍

[sgessa]

I've come across this issue recently. I have a similar problem. I would like to exchange a refresh token for an access token (probably the old access token is expired already) and I was expecting the new access token to be stored in the database but nothing happens. I realized there is not an on_exchange callback. What I'm missing here?

[sgessa]

Ok I found a solution, in MyApp.Guardian I put:

  def on_exchange(old_token_with_claims, new_token_with_claims, _options) do
    Guardian.DB.on_refresh(old_token_with_claims, new_token_with_claims)
  end