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

added token date checks & signature is now forced to lowercase hex before validation. #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mhassman
Copy link

For some reason, i couldn't get token verification to work as-is..
signature = basexx.from_url64(str:sub(dotSecond+1)) was returning binary, but
['HS256'] = function(data, signature, key) return signature == tohex(hmac.new (key, 'sha256'):final (data)) end, was returning lowercase hex.
So, forced extracted message signature to lowercase hex.

Also, added token date (nbf, and exp) checks if they are defined within the token.

Hope this helps..

- signature is now forced to lowercase hex before validation.
@DorianGray
Copy link
Member

Would you mind adding some tests that describe the behavior and fail with the old code and then pass with the new?

- added check - if json decode fails, do not continue and propagate error to caller.
@mhassman
Copy link
Author

Hi Robert,

Apologies on delayed response - crazy week.

I'm happy to help, but not quite understanding what you're asking. Are you
asking to enhance the code for robustness? or something else?

fyi.. i committed a minor change that permits errors in json decode to flow
upward so they can be handled by the calling lua code. otherwise, nginx
fails with 500 error. I don't know if i should do another pull request? or
if you will see the second commit? Please advise..

Thnx!
:-)

-Mark


From: Robert Andrew Ditthardt [mailto:[email protected]]
Sent: Monday, June 13, 2016 8:06 PM
To: Olivine-Labs/lua-jwt
Cc: mhassman; Author
Subject: Re: [Olivine-Labs/lua-jwt] added token date checks & signature is
now forced to lowercase hex before validation. (#9)

Would you mind adding some tests that describe the behavior and fail with
the old code and then pass with the new?

You are receiving this because you authored the thread.
Reply to this email directly, view
#9 (comment) it
on GitHub, or mute
<https://github.com/notifications/unsubscribe/AA37PCEf8UXue0FbDzUxKGyKj2q0Za
qyks5qLfCAgaJpZM4I0FPn> the thread.
<https://github.com/notifications/beacon/AA37PKfPPdqjvjmpTOkdxZOttpkvLK8Iks5
qLfCAgaJpZM4I0FPn.gif>

@DorianGray
Copy link
Member

Specifically I'm asking for tests to be written in the spec folder so that running busted confirms that this code works. There are existing tests over all of the functionality in this library.

@moteus
Copy link
Contributor

moteus commented Mar 16, 2017

This PR add nginx as dependency.
I think better use os module for that.

@moteus
Copy link
Contributor

moteus commented Apr 11, 2017

I have idea to add options.checks table which will allows provide some arbitrary values (and may be functions). E.g. it can provide current timestamp to checks nbf and exp. Also e.g. server name to check iss.

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.

3 participants