Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a new One time token type. #421

Closed
wants to merge 3 commits into from
Closed

Adds a new One time token type. #421

wants to merge 3 commits into from

Conversation

hassox
Copy link
Member

@hassox hassox commented Nov 13, 2017

This token type is not a JWT and is useful for when you need one time access.

The one time token uses Ecto to keep track of it's contents if it has been used or not.

If a token has already been used it will be as though that token never existed and will not verify.

From the tests:

{:ok, claims} = Impl.decode_and_verify(ctx.token)
assert claims["claims"] == ctx.claims["claims"]

assert Impl.resource_from_claims(claims) == {:ok, %{id: ctx.id}}

assert Impl.decode_and_verify(ctx.token) == {:error, :token_not_found_or_expired}

See Guardian.Token.OneTime for more information

This token type is _not_ a JWT and is useful for when you need one time access.

The one time token uses Ecto to keep track of it's contents if it has been used or not.

If a token has already been used it will be as though that token never existed and will not verify.

From the tests:

```elixir
{:ok, claims} = Impl.decode_and_verify(ctx.token)
assert claims["claims"] == ctx.claims["claims"]

assert Impl.resource_from_claims(claims) == {:ok, %{id: ctx.id}}

assert Impl.decode_and_verify(ctx.token) == {:error, :token_not_found_or_expired}
```

See `Guardian.Token.OneTime` for more information
Copy link
Member

@scrogson scrogson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hassox I really like this idea mate!

There are a couple of things that make me think this belongs in its own package:

  • additional dependencies (even though they are optional).
  • assumes Postgres (even though it is my go-to database, its not always going to be possible for everyone).

I think we should put this into a guardian_one_time_token package (or some better, more clever name).

Thoughts?

@hassox
Copy link
Member Author

hassox commented Nov 15, 2017

I originally had it in it's own package but put up a q on #ueberauth which came back as put it in core.

Does it assume postgres? It's my understanding of ecto that it's just going to use whatever your backing DB is, so long as it's an ecto repo. The OneTimeToken requires that you provide a repo to use with it.

@scrogson
Copy link
Member

@hassox it was my understanding that MySQL didn't support JSON columns? Or does the MySQL adapter handle that automatically as a text column and handles casting and dumping automatically?

@hassan
Copy link

hassan commented Nov 15, 2017

@scrogson MySQL as of 5.7.x supports a JSON datatype.

@scrogson
Copy link
Member

@scrogson MySQL as of 5.7.x supports a JSON datatype.

Good to know. Thanks @hassan

@scrogson
Copy link
Member

Looks like this was merged recently: xerions/mariaex#201

However, it's not yet released on hex.

I think we have decided to put this into its own package so that we don't block progress on Guardian 1.0.

@scrogson scrogson closed this Nov 15, 2017
@doomspork doomspork requested review from a team and removed request for a team November 17, 2017 20:52
@doomspork doomspork deleted the one_time_token branch December 7, 2018 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants