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

Add more information to the tokens #50

Closed
faisalabujabal opened this issue Jun 4, 2017 · 7 comments
Closed

Add more information to the tokens #50

faisalabujabal opened this issue Jun 4, 2017 · 7 comments

Comments

@faisalabujabal
Copy link
Contributor

Hello

Big fan of the package!

I've been trying for few hours now to find an efficient way to store more information to the tokens when JWT_BLACKLIST_ENABLED is True. As far as I can tell the only way to access the token is using get_stored_token(jti), but I don't have access to the jti.

I am trying to avoid using get_stored_tokens(identity) and going over the list.

Any suggestions?

Thanks

@faisalabujabal
Copy link
Contributor Author

faisalabujabal commented Jun 4, 2017

So I did find a way to solve this by doing the following:

data = decode_jwt(encoded_token, config.secret_key, config.algorithm, csrf=False)
jti = data['jti']

but that involves using the decode function, which I don't think was intended to be used outside the package. Maybe something along the lines of:

def get_jti(encoded_token, csrf=False):
     return decode_jwt(encoded_token, config.secret_key, config.algorithm, csrf)['jti']

would be nice.

Even better (if possible):

def get_stored_token(jti=None, encoded_token=None):
     if jti is None and bool(encoded_token):
           jti = get_jti(encoded_token)
     elif jti is None and encoded_token is None:
          raise exception
     return _get_token_from_store(jti)

Thanks!

@vimalloc
Copy link
Owner

vimalloc commented Jun 4, 2017

Just to verify, you want to get the jti right after creating a new token? That solution seems like it would be great, only thing I would want to change is csrf flag there. We should be able to just use the csrf value in the config instead of having to pass it in as a kwarg: csrf=config.csrf_protect.

I can look at adding this later this week, or if you wanted to submit a pull request I could look it over ASAP.

Cheers :)

@faisalabujabal
Copy link
Contributor Author

Great! I'll start working on the PR. What do you think of the get_stored_token? Ofcourse anyone that's using the function in their code will have to go and change it.

@faisalabujabal
Copy link
Contributor Author

and what do you think the method should be called? In #22 you suggested that we create get_jwt_jti.

get_jti or get_jwt_jti?

@vimalloc
Copy link
Owner

vimalloc commented Jun 4, 2017

As long as you put the encoded_token kwarg after jti (and make that a kwarg as well, like you have it in your example), it shouldn't cause any breaking changes for current users. It can currently be called it via get_stored_token('abc123') or get_stored_token(jti=`abc123`), and both of those will still work with your proposed changes. This seems fine to me.

As per the name, I think get_jti is fine. In this case, we are passing in the encoded token directly ex: get_jti(encoded_token) instead of grabbing it from the flask context stack, so I think that fits the overall naming convention better.

@faisalabujabal
Copy link
Contributor Author

Done! #51

I was also curious why wasn't simplekv included in requirements.txt?

@vimalloc
Copy link
Owner

vimalloc commented Jun 4, 2017

Thanks! 👍

And good catch with the reqiurements.txt, it absolutely should be in there. Looks like it was accidently removed in adff7b2. I'll get it re-added

@vimalloc vimalloc closed this as completed Jun 4, 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

No branches or pull requests

2 participants