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

Fix RuboCop TODOs #476

Merged
merged 2 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 0 additions & 114 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,123 +14,9 @@ Gemspec/RequireMFA:
Exclude:
- 'ruby-jwt.gemspec'

# Offense count: 10
# Cop supports --auto-correct.
Layout/EmptyLineAfterGuardClause:
Exclude:
- 'lib/jwt/algos/ecdsa.rb'
- 'lib/jwt/algos/eddsa.rb'
- 'lib/jwt/algos/rsa.rb'
- 'lib/jwt/decode.rb'
- 'lib/jwt/jwk.rb'
- 'lib/jwt/jwk/rsa.rb'
- 'lib/jwt/verify.rb'

# Offense count: 3
# Cop supports --auto-correct.
# Configuration parameters: AllowMultipleStyles, EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle.
# SupportedHashRocketStyles: key, separator, table
# SupportedColonStyles: key, separator, table
# SupportedLastArgumentHashStyles: always_inspect, always_ignore, ignore_implicit, ignore_explicit
Layout/HashAlignment:
Exclude:
- 'lib/jwt/algos/ecdsa.rb'

# Offense count: 1
Lint/MissingSuper:
Exclude:
- 'lib/jwt/jwk/key_base.rb'


# Offense count: 1
# Configuration parameters: IgnoredMethods, CountRepeatedAttributes.
Metrics/AbcSize:
Max: 25

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: IgnoredMethods.
# IgnoredMethods: ==, equal?, eql?
Style/ClassEqualityComparison:
Exclude:
- 'lib/jwt/algos/rsa.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/ExpandPathArguments:
Exclude:
- 'ruby-jwt.gemspec'

# Offense count: 19
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'Appraisals'
- 'Gemfile'
- 'Rakefile'
- 'bin/console.rb'
- 'lib/jwt/algos/ecdsa.rb'
- 'lib/jwt/algos/eddsa.rb'
- 'lib/jwt/algos/hmac.rb'
- 'lib/jwt/algos/none.rb'
- 'lib/jwt/algos/ps.rb'
- 'lib/jwt/algos/rsa.rb'
- 'lib/jwt/algos/unsupported.rb'
- 'lib/jwt/claims_validator.rb'
- 'lib/jwt/default_options.rb'
- 'lib/jwt/security_utils.rb'
- 'ruby-jwt.gemspec'

# Offense count: 2
# Cop supports --auto-correct.
Style/HashTransformKeys:
Exclude:
- 'lib/jwt/claims_validator.rb'
- 'lib/jwt/encode.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, Autocorrect.
# SupportedStyles: module_function, extend_self, forbidden
Style/ModuleFunction:
Exclude:
- 'lib/jwt/signature.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: Strict.
Style/NumericLiterals:
MinDigits: 6

# Offense count: 1
# Configuration parameters: AllowedMethods.
# AllowedMethods: respond_to_missing?
Style/OptionalBooleanParameter:
Exclude:
- 'lib/jwt.rb'

# Offense count: 5
# Cop supports --auto-correct.
Style/RedundantFreeze:
Exclude:
- 'lib/jwt/encode.rb'
- 'lib/jwt/jwk/ec.rb'
- 'lib/jwt/jwk/hmac.rb'
- 'lib/jwt/jwk/rsa.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods.
# AllowedMethods: present?, blank?, presence, try, try!
Style/SafeNavigation:
Exclude:
- 'lib/jwt/encode.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/SlicingWithRange:
Exclude:
- 'lib/jwt/security_utils.rb'

2 changes: 2 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

appraise 'standalone' do
# No additions
end
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

source 'https://rubygems.org'

gemspec
Expand Down
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'bundler/setup'
require 'bundler/gem_tasks'

Expand Down
1 change: 1 addition & 0 deletions bin/console.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

require 'bundler/setup'
require 'jwt'
Expand Down
9 changes: 6 additions & 3 deletions lib/jwt/algos/ecdsa.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# frozen_string_literal: true

module JWT
module Algos
module Ecdsa
module_function

NAMED_CURVES = {
'prime256v1' => 'ES256',
'secp256r1' => 'ES256', # alias for prime256v1
'secp384r1' => 'ES384',
'secp521r1' => 'ES512'
'secp256r1' => 'ES256', # alias for prime256v1
'secp384r1' => 'ES384',
'secp521r1' => 'ES512'
}.freeze

SUPPORTED = NAMED_CURVES.values.uniq.freeze
Expand All @@ -31,6 +33,7 @@ def verify(to_verify)
if algorithm != key_algorithm
raise IncorrectAlgorithm, "payload algorithm is #{algorithm} but #{key_algorithm} verification key was provided"
end

digest = OpenSSL::Digest.new(curve_definition[:digest])
public_key.dsa_verify_asn1(digest.digest(signing_input), SecurityUtils.raw_to_asn1(signature, public_key))
end
Expand Down
3 changes: 3 additions & 0 deletions lib/jwt/algos/eddsa.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module Eddsa
Expand All @@ -23,6 +25,7 @@ def verify(to_verify)
raise IncorrectAlgorithm, "payload algorithm is #{algorithm} but #{key.primitive} signing key was provided"
end
raise DecodeError, "key given is a #{public_key.class} but has to be a RbNaCl::Signatures::Ed25519::VerifyKey" if public_key.class != RbNaCl::Signatures::Ed25519::VerifyKey

public_key.verify(signature, signing_input)
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/algos/hmac.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module Hmac
Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/algos/none.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module None
Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/algos/ps.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module Ps
Expand Down
5 changes: 4 additions & 1 deletion lib/jwt/algos/rsa.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module Rsa
Expand All @@ -7,7 +9,8 @@ module Rsa

def sign(to_sign)
algorithm, msg, key = to_sign.values
raise EncodeError, "The given key is a #{key.class}. It has to be an OpenSSL::PKey::RSA instance." if key.class == String
raise EncodeError, "The given key is a #{key.class}. It has to be an OpenSSL::PKey::RSA instance." if key.instance_of?(String)

key.sign(OpenSSL::Digest.new(algorithm.sub('RS', 'sha')), msg)
end

Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/algos/unsupported.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module Algos
module Unsupported
Expand Down
4 changes: 3 additions & 1 deletion lib/jwt/claims_validator.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require_relative './error'

module JWT
Expand All @@ -9,7 +11,7 @@ class ClaimsValidator
].freeze

def initialize(payload)
@payload = payload.each_with_object({}) { |(k, v), h| h[k.to_sym] = v }
@payload = payload.transform_keys(&:to_sym)
end

def validate!
Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/decode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module JWT
class Decode
def initialize(jwt, key, verify, options, &keyfinder)
raise(JWT::DecodeError, 'Nil JSON web token') unless jwt

@jwt = jwt
@key = key
@options = options
Expand All @@ -30,6 +31,7 @@ def decode_segments
verify_claims
end
raise(JWT::DecodeError, 'Not enough or too many segments') unless header && payload

[payload, header]
end

Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/default_options.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
module DefaultOptions
DEFAULT_OPTIONS = {
Expand Down
8 changes: 4 additions & 4 deletions lib/jwt/encode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
module JWT
# Encoding logic for JWT
class Encode
ALG_NONE = 'none'.freeze
ALG_KEY = 'alg'.freeze
ALG_NONE = 'none'
ALG_KEY = 'alg'

def initialize(options)
@payload = options[:payload]
@key = options[:key]
_, @algorithm = Algos.find(options[:algorithm])
@headers = options[:headers].each_with_object({}) { |(key, value), headers| headers[key.to_s] = value }
@headers = options[:headers].transform_keys(&:to_s)
end

def segments
Expand Down Expand Up @@ -45,7 +45,7 @@ def encode_header
end

def encode_payload
if @payload && @payload.is_a?(Hash)
if @payload.is_a?(Hash)
ClaimsValidator.new(@payload).validate!
end

Expand Down
1 change: 1 addition & 0 deletions lib/jwt/jwk.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def mappings
def generate_mappings
classes.each_with_object({}) do |klass, hash|
next unless klass.const_defined?('KTYS')

Array(klass::KTYS).each do |kty|
hash[kty] = klass
end
Expand Down
2 changes: 1 addition & 1 deletion lib/jwt/jwk/ec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class EC < KeyBase
extend Forwardable
def_delegators :@keypair, :public_key

KTY = 'EC'.freeze
KTY = 'EC'
KTYS = [KTY, OpenSSL::PKey::EC].freeze
BINARY = 2

Expand Down
2 changes: 1 addition & 1 deletion lib/jwt/jwk/hmac.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module JWT
module JWK
class HMAC < KeyBase
KTY = 'oct'.freeze
KTY = 'oct'
KTYS = [KTY, String].freeze

def initialize(keypair, kid = nil)
Expand Down
1 change: 1 addition & 0 deletions lib/jwt/jwk/key_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def initialize(keypair, kid = nil)
end

def self.inherited(klass)
super
anakinj marked this conversation as resolved.
Show resolved Hide resolved
::JWT::JWK.classes << klass
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/jwt/jwk/rsa.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ module JWT
module JWK
class RSA < KeyBase
BINARY = 2
KTY = 'RSA'.freeze
KTY = 'RSA'
KTYS = [KTY, OpenSSL::PKey::RSA].freeze
RSA_KEY_ELEMENTS = %i[n e d p q dp dq qi].freeze

def initialize(keypair, kid = nil)
raise ArgumentError, 'keypair must be of type OpenSSL::PKey::RSA' unless keypair.is_a?(OpenSSL::PKey::RSA)

super(keypair, kid || generate_kid(keypair.public_key))
end

Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/security_utils.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module JWT
# Collection of security methods
#
Expand Down
3 changes: 2 additions & 1 deletion lib/jwt/signature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
module JWT
# Signature logic for JWT
module Signature
extend self
module_function

ToSign = Struct.new(:algorithm, :msg, :key)
ToVerify = Struct.new(:algorithm, :public_key, :signing_input, :signature)

Expand Down
3 changes: 3 additions & 0 deletions lib/jwt/verify.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class << self
def verify_claims(payload, options)
options.each do |key, val|
next unless key.to_s =~ /verify/

Verify.send(key, payload, options) if val
end
end
Expand Down Expand Up @@ -82,12 +83,14 @@ def verify_not_before

def verify_sub
return unless (options_sub = @options[:sub])

sub = @payload['sub']
raise(JWT::InvalidSubError, "Invalid subject. Expected #{options_sub}, received #{sub || '<none>'}") unless sub.to_s == options_sub.to_s
end

def verify_required_claims
return unless (options_required_claims = @options[:required_claims])

options_required_claims.each do |required_claim|
raise(JWT::MissingRequiredClaim, "Missing required claim #{required_claim}") unless @payload.include?(required_claim)
end
Expand Down
4 changes: 3 additions & 1 deletion ruby-jwt.gemspec
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
lib = File.expand_path('../lib/', __FILE__)
# frozen_string_literal: true

lib = File.expand_path('lib', __dir__)
$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
require 'jwt/version'

Expand Down
2 changes: 1 addition & 1 deletion spec/jwt/claims_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

shared_examples_for 'a NumericDate claim' do |claim|
context "when #{claim} payload is an integer" do
let(:claims) { { claim => 12345 } }
let(:claims) { { claim => 12_345 } }

it 'does not raise error' do
expect { subject }.not_to raise_error
Expand Down