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

Feature: Add configuration to only store one token type #15

Closed
wants to merge 1 commit into from

Conversation

hannahhoward
Copy link

Hey! This is an additional feature that would allow you to only store tokens that match a certain type. My thinking is if you want to add refresh tokens to Guardian, then the ideal scenario is to set short expirations on the access tokens and not worry about storing then (therefore avoiding a DB check) but to store refresh tokens in a DB so you can revoke them if they are compromised (or a user logs out).

So you'd use GuardianDB only for tokens of type "refresh" for example.

@@ -101,6 +111,11 @@ defmodule GuardianDb do
end
end

def configured_type_matches(type) do
env_token_type = Keyword.get(Application.get_env(:guardian_db, GuardianDb), :token_type)
Copy link
Member

Choose a reason for hiding this comment

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

I think if we made it so that you could configure the token type to use a list of tokens that would be sweet

@hassox
Copy link
Member

hassox commented May 11, 2016

This is a great addition @hannahhoward. The only thing I think would make it more awesome is if we were able to handle a list of token types. Thoughts?

@doomspork
Copy link
Member

Ping @hannahhoward / @hassox — do we still want to move forward with this? If @hannahhoward isn't available I can take over the changes.

@doomspork
Copy link
Member

If there's no objections by either @hannahhoward or @hassox, myself or @ybur-yug or I will clean this up and get it merged in.

@imranismail
Copy link
Contributor

I can work on the list of types

@doomspork
Copy link
Member

That works @imranismail 👍

@doomspork
Copy link
Member

Closing this in favor of #47

@doomspork doomspork closed this Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants