add functionality to exchange one token to another

[Hanspagh]

Makes it possible to exchange a token with type 'refresh' to a token with type 'token', like we discussed in #111, it will play nicely with guardianDB once ueberauth/guardian_db#15

[hassox]

Is this the normal proceedure to revoke the refresh token? I thought these stayed valid and could be used multiple times.

[hassox]

This looks pretty good, just the q about revoking a refresh token. I could be wrong but my understanding is that they should stick around.

Also can you please add some words to the readme and changelog?

Thanks :D

[Hanspagh]

I guess we could make it up to the user to revoke it, or create two different methods exchangeAndRevoke and exchange. What do you think?
I am updating the readme as we speak.

[Hanspagh]

I just realized that you can use the refresh token as a normal token. Is this the case or did I miss something?
Edit
I guess it can be solved by setting the typ in EnsureAutenticated, but this should properly be mention in the Readme

[doomspork]

Do we want to support :refresh? We use atoms elsewhere.

[Hanspagh]

yes, but this ensures that we can set a default living time for the refresh tokens
Edit:
I read your comment wrong, the reason I used "refresh" is because guardian currently use "token" as default

[hassox]

@Hanspagh correct you could use it. Once you start using different types of tokens EnsureAuthenticated should check the typ field. I use access tokens for normal calls and other types as appropriate.

[hassox]

As an example, here's my plug for one of my apps

  pipeline :api_auth do
    plug Guardian.Plug.VerifyHeader, realm: "bearer"
    plug Guardian.Plug.EnsureAuthenticated, typ: "access", handler: MyAwesomeApp.Api.V1.ApiAuthErrorHandler
    plug Guardian.Plug.LoadResource
  end

[Hanspagh]

Makes sense, my only concern is that if someone isn't aware of this, you could authenticate your self with a refresh token, which defeats the whole purpose of this. So we might consider having EnsureAuthenticated check for the typ "token" per default. What do you think?

[hassox]

I'm not sure if that would be good or not. I'm thinking about pipelines and setting the token type there. It might be ok. Let me percolate on it a bit. Also I think we should change the default type to 'access' rather than 'token'. Token doesn't really mean anything.

[Hanspagh]

I think you are right 'access' makes much more sense then token. If we don't make it a default in EnsureAuthenticated as well, I think we should at least inform the user about the implications.

[Hanspagh]

Any progress on this?

[hassox]

Hey @Hanspagh thanks for pinging on this one. I think we definitely need an exchange function but I don't think we should limit it to only refresh tokens. You could exchange any kind. I do like the refresh TTL of the token too and would love to see it come in.

I think what would be great is:

1. we add an exchange function that can handle whatever type of token and exchange it for another. This should take the existing token, the new type, the old type and any options to modify
2. We should change the existing default from token to access.
3. Update the README to define the implications of using different token types
4. add a configuration entry for token_ttl where you can specify the defaults of each type of token.

Thoughts?

[Hanspagh]

I think the overall idea sounds really solid.
This allows to define your own token type and still benefit from the exchange functionality. The only downside I see is that someone could potentially exchange a "access" token for a "refresh" token.
We could predefine these two types and enforce some behavior to prevent that. What do you think?

[hassox]

Hmm true facts. I guess there's a couple of ways I can think of that we could deal with that.

1. Add a config option that provides a mapping from -> to
2. Add into the exchange function signature the expected from -> to. If the from doesn't match it doesn't work

I think 2 seems more natural to me. That way the calling code would be responsible:

Guardian.exchange(jwt, "refresh", "access")

Or in the case where it's many:

Guardian.exchange(jwt, ["refresh", "rememberMe"], "access")

Thoughts?

[Hanspagh]

Seems good, I will start working on it

[Hanspagh]

Here is first shot at the proposed solutions. Still missing docs and being able to modify the token created
Some questions:

• Should we keep the ttl config, so we wont break existing projects, and just use it as a fallback if the token_ttl does not exists.
• We might consider lowering the fallback ttl from 1000.000.000s to something more sense able or throw and error if there is no config

[hassox]

I think we want to drop some of the claims here and use the remaining claims. If we just use an empty map we'll lose any custom claims that were embedded in the 'refresh' token.

[Hanspagh]

I will have a look. I have a very busy work week coming up, but I will be back in action the 27.

[Hanspagh]

I am sorry for the huge delay on this, but I have fixed the two issues you mentioned namely; to handle atom types and not drop all claims when doing the exchange

[doomspork]

Thank you @Hanspagh! @hassox any additional feedback?

[Hanspagh]

If you don't have anymore feedback, I will update the docs and we can finish this.

[Moussenger]

When this pull request will be merged?

[doomspork]

@hassox any final feedback? @Hanspagh do you mind rebasing?

[Hanspagh]

I will update the docs and rebase it this weekend

[doomspork]

Thank you @Hanspagh

[Hanspagh]

Okay, I did the rebase, and added some docs.
One thing we didn't talk about is the ability to have more than one token defined the the the config file.
Where should document this?

[Hanspagh]

I am sorry for reviving this again, but I am pretty sure that we could just merge this?

[doomspork]

Sorry for the delays @Hanspagh, I've not been following this thread. Let me review the changes and see about merging.