Skip to content

Merge in updates up to v1.5.6 of ruby-jwt#213

Closed
britton wants to merge 6 commits intojwt:masterfrom
semcat:update_to_1.5.6
Closed

Merge in updates up to v1.5.6 of ruby-jwt#213
britton wants to merge 6 commits intojwt:masterfrom
semcat:update_to_1.5.6

Conversation

@britton
Copy link
Copy Markdown

@britton britton commented Jul 12, 2017

No description provided.

@britton britton closed this Jul 12, 2017
@sourcelevel-bot
Copy link
Copy Markdown

Hello, @britton! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@britton britton deleted the update_to_1.5.6 branch July 12, 2017 20:00
Comment thread lib/jwt/verify.rb


fail(
raise(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Favor a normal unless-statement over a modifier clause in a multiline statement.

Comment thread lib/jwt/verify.rb
if @payload['aud'].is_a?(Array)
verify_aud_array(@payload['aud'], options_aud)
else
raise(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Favor a normal unless-statement over a modifier clause in a multiline statement.

Comment thread lib/jwt/verify.rb
raise(JWT::InvalidJtiError, 'Invalid jti') unless options_verify_jti.call(@payload['jti'])
else
fail(JWT::InvalidJtiError, 'Missing jti') if payload['jti'].to_s == ''
raise(JWT::InvalidJtiError, 'Missing jti') if @payload['jti'].to_s.strip.empty?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Convert if nested inside else to elsif.

Comment thread lib/jwt/verify.rb
JWT::InvalidAudError,
"Invalid audience. Expected #{options[:aud]}, received #{payload['aud'] || '<none>'}"
) unless payload['aud'].to_s == options[:aud].to_s
if @payload['nbf'].to_i > (Time.now.to_i + leeway)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use a guard clause instead of wrapping the code inside a conditional expression.

Comment thread lib/jwt/verify.rb

if !(payload['iat'].is_a?(Integer)) || payload['iat'].to_i > (Time.now.to_i + options[:leeway])
fail(JWT::InvalidIatError, 'Invalid iat')
if @payload['iss'].to_s != options_iss.to_s
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use a guard clause instead of wrapping the code inside a conditional expression.

Comment thread lib/jwt/verify.rb
JWT::InvalidIssuerError,
"Invalid issuer. Expected #{options[:iss]}, received #{payload['iss'] || '<none>'}"
)
if !@payload['iat'].is_a?(Numeric) || @payload['iat'].to_f > (Time.now.to_f + leeway)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use a guard clause instead of wrapping the code inside a conditional expression.

Comment thread lib/jwt/verify.rb

if payload['nbf'].to_i > (Time.now.to_i + options[:leeway])
fail(JWT::ImmatureSignature, 'Signature nbf has not been reached')
if @payload['exp'].to_i <= (Time.now.to_i - leeway)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use a guard clause instead of wrapping the code inside a conditional expression.

Comment thread lib/jwt.rb
OpenSSL.errors.clear
end

def verify_signature_algo(algo, key, signing_input, signature)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perceived complexity for verify_signature_algo is too high. [8/7]

Comment thread lib/jwt.rb
[payload, header]
end

def decode_verify_signature(key, header, signature, signing_input, options, &keyfinder)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid parameter lists longer than 5 parameters.

Comment thread lib/jwt.rb
OpenSSL.errors.clear
end

def verify_signature_algo(algo, key, signing_input, signature)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cyclomatic complexity for verify_signature_algo is too high. [7/6]

Comment thread lib/jwt.rb
def verify_signature_algo(algo, key, signing_input, signature)
if %w(HS256 HS384 HS512).include?(algo)
fail(JWT::VerificationError, 'Signature verification raised') unless secure_compare(signature, sign_hmac(algo, signing_input, key))
raise(JWT::VerificationError, 'Signature verification raised') unless secure_compare(signature, sign_hmac(algo, signing_input, key))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT#verify_signature_algo calls 'raise(JWT::VerificationError, 'Signature verification raised')' 3 times

Read more about it here.

Comment thread lib/jwt.rb

def decode(jwt, key = nil, verify = true, custom_options = {}, &keyfinder)
fail(JWT::DecodeError, 'Nil JSON web token') unless jwt
def decoded_segments(jwt, key = nil, verify = true, custom_options = {}, &keyfinder)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT#decoded_segments has boolean parameter 'verify'

Read more about it here.

Comment thread lib/jwt.rb
end

merged_options = options.merge(custom_options)
def decode(jwt, key = nil, verify = true, custom_options = {}, &keyfinder)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT#decode has boolean parameter 'verify'

Read more about it here.

Comment thread lib/jwt/verify.rb
fail(JWT::InvalidJtiError, 'Invalid jti') unless _options[:verify_jti].call(payload['jti'])
def verify_jti
options_verify_jti = extract_option(:verify_jti)
if options_verify_jti.respond_to?(:call)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT::Verify#verify_jti manually dispatches method call

Comment thread lib/jwt/verify.rb
def verify_aud
return unless (options_aud = extract_option(:aud))

if @payload['aud'].is_a?(Array)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT::Verify#verify_aud calls '@payload['aud']' 4 times

Read more about it here.

Comment thread lib/jwt/verify.rb

def self.verify_aud(payload, options)
return unless options[:aud]
def verify_not_before
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar code found in 1 other location (mass = 22) (lib/jwt/verify.rb#79 and lib/jwt/verify.rb#43)

Comment thread lib/jwt/verify.rb

def self.verify_not_before(payload, options)
return unless payload.include?('nbf')
def verify_expiration
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar code found in 1 other location (mass = 22) (lib/jwt/verify.rb#43 and lib/jwt/verify.rb#79)

Comment thread lib/jwt/verify.rb

def self.verify_sub(payload, options)
return unless options[:sub]
def verify_sub
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar code found in 1 other location (mass = 28) (lib/jwt/verify.rb#87 and lib/jwt/verify.rb#59)

Comment thread lib/jwt/verify.rb

def self.verify_iat(payload, options)
return unless payload.include?('iat')
def verify_iss
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar code found in 1 other location (mass = 28) (lib/jwt/verify.rb#59 and lib/jwt/verify.rb#87)

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.

2 participants