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 algos/strategy classes + structs for inputs #230

Merged
merged 10 commits into from
Sep 9, 2017
Merged

Conversation

ab320012
Copy link
Contributor

@ab320012 ab320012 commented Sep 7, 2017

made changes discussed here: #229 , changes don't affect the public api.


raise JWT::VerificationError, 'Algorithm not supported' if algo.nil?
verified = algo.verify(ToVerify.new(algorithm, key, signing_input, signature))

Choose a reason for hiding this comment

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

Trailing whitespace detected.

end
def verify(algorithm, key, signing_input, signature)
algo = ALGOS.find{|algo|
algo.const_get(:SUPPORTED).include? algorithm

Choose a reason for hiding this comment

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

Trailing whitespace detected.

algo.const_get(:SUPPORTED).include? algorithm
}
raise NotImplementedError, 'Unsupported signing method' if algo.nil?
algo.sign ToSign.new(algorithm, msg, key)

Choose a reason for hiding this comment

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

Trailing whitespace detected.

algo = ALGOS.find{|algo|
algo.const_get(:SUPPORTED).include? algorithm
}
raise NotImplementedError, 'Unsupported signing method' if algo.nil?

Choose a reason for hiding this comment

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

Trailing whitespace detected.

end
algo = ALGOS.find{|algo|
algo.const_get(:SUPPORTED).include? algorithm
}

Choose a reason for hiding this comment

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

Trailing whitespace detected.

module JWT
module Algos
module Ecdsa
extend self

Choose a reason for hiding this comment

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

Use module_function instead of extend self.

SecurityUtils.asn1_to_raw(key.dsa_sign_asn1(digest.digest(msg)), key)
end

def verify to_verify

Choose a reason for hiding this comment

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

Use def with parentheses when there are parameters.

'secp521r1' => 'ES512'
}.freeze

def sign to_sign

Choose a reason for hiding this comment

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

Use def with parentheses when there are parameters.

module JWT
module Algos
module Eddsa
extend self

Choose a reason for hiding this comment

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

Use module_function instead of extend self.

key.sign(msg)
end

def verify to_verify

Choose a reason for hiding this comment

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

Use def with parentheses when there are parameters.

module_function
SUPPORTED = %w(ED25519).freeze

def sign to_sign

Choose a reason for hiding this comment

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

Use def with parentheses when there are parameters.

module JWT
module Algos
module Rsa
module_function

Choose a reason for hiding this comment

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

Keep a blank line after module_function.

module JWT
module Algos
module Hmac
module_function

Choose a reason for hiding this comment

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

Keep a blank line after module_function.

end

raise JWT::VerificationError, 'Algorithm not supported' if algo.nil?

Choose a reason for hiding this comment

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

JWT::Signature#verify performs a nil-check

Read more about it here.

def verify(*)
raise JWT::VerificationError, 'Algorithm not supported'
end
def sign(*)

Choose a reason for hiding this comment

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

Use empty lines between method definitions.

module Unsupported
module_function

SUPPORTED = Object.new.tap{ |o| o.define_singleton_method(:include?){ |*| true }}

Choose a reason for hiding this comment

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

Trailing whitespace detected.

module Unsupported
module_function

SUPPORTED = Object.new.tap{ |o| o.define_singleton_method(:include?){ |*| true }}

Choose a reason for hiding this comment

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

Space missing inside }.

module Unsupported
module_function

SUPPORTED = Object.new.tap{ |o| o.define_singleton_method(:include?){ |*| true }}

Choose a reason for hiding this comment

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

Space missing to the left of {.

module Unsupported
module_function

SUPPORTED = Object.new.tap{ |o| o.define_singleton_method(:include?){ |*| true }}

Choose a reason for hiding this comment

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

Space missing to the left of {.

module Unsupported
module_function

SUPPORTED = Object.new.tap{ |o| o.define_singleton_method(:include?){ |*| true }}

Choose a reason for hiding this comment

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

JWT::Algos::Unsupported has the variable name 'o'

Read more about it here.

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 14 fixed issues! 🎉

You can see more details about this review at https://ebertapp.io/github/jwt/ruby-jwt/pulls/230.

Copy link
Member

@excpt excpt left a comment

Choose a reason for hiding this comment

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

Thank you very much. Great refactoring. 🥇

@excpt excpt merged commit 51a162f into jwt:master Sep 9, 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.

2 participants