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

verify takes 2 params, second being payload closes: #207 #238

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

ab320012
Copy link
Contributor

@ab320012 ab320012 commented Sep 19, 2017

Added optional payload to verify_jti + passing tests for payload and arguements #207

@@ -63,7 +63,11 @@ def verify_jti
jti = @payload['jti']

if options_verify_jti.respond_to?(:call)
raise(JWT::InvalidJtiError, 'Invalid jti') unless options_verify_jti.call(jti)
raise(JWT::InvalidJtiError, 'Invalid jti') unless (

Choose a reason for hiding this comment

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

Don't use parentheses around the condition of an unless.

@@ -63,7 +63,11 @@ def verify_jti
jti = @payload['jti']

if options_verify_jti.respond_to?(:call)
raise(JWT::InvalidJtiError, 'Invalid jti') unless options_verify_jti.call(jti)
raise(JWT::InvalidJtiError, 'Invalid jti') unless (
options_verify_jti.arity == 2 ?

Choose a reason for hiding this comment

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

Avoid multi-line ternary operators, use if or unless instead.

@ab320012 ab320012 changed the title verify takes 2 params, second being payload closes: https://github.com/jwt/ruby-jwt/issues/207 verify takes 2 params, second being payload closes: #207 Sep 19, 2017
@excpt excpt self-requested a review October 5, 2017 15:11
@excpt excpt added this to the Version 2.1.0 milestone Oct 5, 2017
@excpt excpt self-assigned this Oct 5, 2017
@excpt
Copy link
Member

excpt commented Oct 5, 2017

@ab320012 Thanks for the PR!

@excpt excpt merged commit fd1f86c into jwt:master Oct 5, 2017
@ab320012
Copy link
Contributor Author

ab320012 commented Oct 5, 2017

@excpt Thanks Tim!

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.

2 participants