From 826dfe87d03cc5a67d80f7239ff0f79c1b07680d Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Sun, 29 Dec 2024 20:29:40 +0200 Subject: [PATCH] Improve specs and blind spots --- lib/jwt/decode.rb | 7 ++--- lib/jwt/jwa/ecdsa.rb | 4 --- lib/jwt/jwa/hmac.rb | 4 --- lib/jwt/jwk/ec.rb | 4 --- lib/jwt/jwk/rsa.rb | 3 ++ spec/jwt/claims/verifier_spec.rb | 27 +++++++++++++++++ spec/jwt/jwa/none_spec.rb | 17 +++++++++++ spec/jwt/jwa/ps_spec.rb | 4 +-- spec/jwt/jwa/rsa_spec.rb | 4 +-- spec/jwt/jwa/unsupported_spec.rb | 15 ++++++++++ spec/jwt/jwk/decode_with_jwk_spec.rb | 8 ++--- spec/jwt/jwk/ec_spec.rb | 26 ++++++++++++++-- spec/jwt/jwk/hmac_spec.rb | 7 +++++ spec/jwt/jwk/rsa_spec.rb | 2 +- spec/jwt/jwk/set_spec.rb | 20 ++++++------- spec/jwt/jwk_spec.rb | 11 +++++-- spec/jwt/jwt_spec.rb | 10 +++++++ spec/jwt/token_spec.rb | 44 ++++++++++++++++++++++++++++ 18 files changed, 177 insertions(+), 40 deletions(-) create mode 100644 spec/jwt/claims/verifier_spec.rb create mode 100644 spec/jwt/jwa/none_spec.rb create mode 100644 spec/jwt/jwa/unsupported_spec.rb diff --git a/lib/jwt/decode.rb b/lib/jwt/decode.rb index 67437c03..5928f401 100644 --- a/lib/jwt/decode.rb +++ b/lib/jwt/decode.rb @@ -77,11 +77,8 @@ def allowed_and_valid_algorithms :algorithms].freeze def given_algorithms - ALGORITHM_KEYS.each do |alg_key| - alg = @options[alg_key] - return Array(alg) if alg - end - [] + alg_key = ALGORITHM_KEYS.find { |key| @options[key] } + Array(@options[alg_key]) end def allowed_algorithms diff --git a/lib/jwt/jwa/ecdsa.rb b/lib/jwt/jwa/ecdsa.rb index 9876ecca..e011ff44 100644 --- a/lib/jwt/jwa/ecdsa.rb +++ b/lib/jwt/jwa/ecdsa.rb @@ -56,10 +56,6 @@ def verify(data:, signature:, verification_key:) register_algorithm(new(v[:algorithm], v[:digest])) end - def self.from_algorithm(algorithm) - new(algorithm, algorithm.downcase.gsub('es', 'sha')) - end - def self.curve_by_name(name) NAMED_CURVES.fetch(name) do raise UnsupportedEcdsaCurve, "The ECDSA curve '#{name}' is not supported" diff --git a/lib/jwt/jwa/hmac.rb b/lib/jwt/jwa/hmac.rb index e66d8ffd..86b3278c 100644 --- a/lib/jwt/jwa/hmac.rb +++ b/lib/jwt/jwa/hmac.rb @@ -6,10 +6,6 @@ module JWA class Hmac include JWT::JWA::SigningAlgorithm - def self.from_algorithm(algorithm) - new(algorithm, OpenSSL::Digest.new(algorithm.downcase.gsub('hs', 'sha'))) - end - def initialize(alg, digest) @alg = alg @digest = digest diff --git a/lib/jwt/jwk/ec.rb b/lib/jwt/jwk/ec.rb index 567be48f..aedc2ff4 100644 --- a/lib/jwt/jwk/ec.rb +++ b/lib/jwt/jwk/ec.rb @@ -124,10 +124,6 @@ def encode_octets(octets) ::JWT::Base64.url_encode(octets) end - def encode_open_ssl_bn(key_part) - ::JWT::Base64.url_encode(key_part.to_s(BINARY)) - end - def parse_ec_key(key) crv, x_octets, y_octets = keypair_components(key) octets = key.private_key&.to_bn&.to_s(BINARY) diff --git a/lib/jwt/jwk/rsa.rb b/lib/jwt/jwk/rsa.rb index 44044075..df0eeab2 100644 --- a/lib/jwt/jwk/rsa.rb +++ b/lib/jwt/jwk/rsa.rb @@ -165,6 +165,8 @@ def create_rsa_key_using_sets(rsa_parameters) end end + # :nocov: + # Before openssl 2.0, we need to use the accessors to set the key def create_rsa_key_using_accessors(rsa_parameters) # rubocop:disable Metrics/AbcSize validate_rsa_parameters!(rsa_parameters) @@ -179,6 +181,7 @@ def create_rsa_key_using_accessors(rsa_parameters) # rubocop:disable Metrics/Abc rsa_key.iqmp = rsa_parameters[:qi] if rsa_parameters[:qi] end end + # :nocov: def validate_rsa_parameters!(rsa_parameters) return unless rsa_parameters.key?(:d) diff --git a/spec/jwt/claims/verifier_spec.rb b/spec/jwt/claims/verifier_spec.rb new file mode 100644 index 00000000..079eb48a --- /dev/null +++ b/spec/jwt/claims/verifier_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.describe JWT::Claims::Verifier do + describe '.verify!' do + context 'when all claims are given' do + let(:options) do + [ + :exp, + :nbf, + { iss: 'issuer' }, + :iat, + :jti, + { aud: 'aud' }, + :sub, + :crit, + { required: [] }, + :numeric + ] + end + + it 'verifies all claims' do + token = SpecSupport::Token.new(payload: { 'iss' => 'issuer', 'jti' => 1, 'aud' => 'aud' }, header: { 'crit' => [] }) + expect(described_class.verify!(token, *options)).to eq(nil) + end + end + end +end diff --git a/spec/jwt/jwa/none_spec.rb b/spec/jwt/jwa/none_spec.rb new file mode 100644 index 00000000..ca5a1094 --- /dev/null +++ b/spec/jwt/jwa/none_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.describe JWT::JWA::None do + subject { described_class.new } + + describe '#sign' do + it 'returns an empty string' do + expect(subject.sign('data', 'key')).to eq('') + end + end + + describe '#verify' do + it 'returns true' do + expect(subject.verify('data', 'signature', 'key')).to be true + end + end +end diff --git a/spec/jwt/jwa/ps_spec.rb b/spec/jwt/jwa/ps_spec.rb index 13d4d087..8c472f42 100644 --- a/spec/jwt/jwa/ps_spec.rb +++ b/spec/jwt/jwa/ps_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe JWT::JWA::Ps do - let(:rsa_key) { OpenSSL::PKey::RSA.generate(2048) } + let(:rsa_key) { test_pkey('rsa-2048-private.pem') } let(:data) { 'test data' } let(:ps256_instance) { described_class.new('PS256') } let(:ps384_instance) { described_class.new('PS384') } @@ -44,7 +44,7 @@ end context 'with a key length less than 2048 bits' do - let(:rsa_key) { OpenSSL::PKey::RSA.generate(1024) } + let(:rsa_key) { OpenSSL::PKey::RSA.generate(2047) } it 'raises an error' do expect do diff --git a/spec/jwt/jwa/rsa_spec.rb b/spec/jwt/jwa/rsa_spec.rb index edd7f1cf..f1cbb836 100644 --- a/spec/jwt/jwa/rsa_spec.rb +++ b/spec/jwt/jwa/rsa_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe JWT::JWA::Rsa do - let(:rsa_key) { OpenSSL::PKey::RSA.generate(2048) } + let(:rsa_key) { test_pkey('rsa-2048-private.pem') } let(:data) { 'test data' } let(:rsa_instance) { described_class.new('RS256') } @@ -21,7 +21,7 @@ end context 'with a key length less than 2048 bits' do - let(:rsa_key) { OpenSSL::PKey::RSA.generate(1024) } + let(:rsa_key) { OpenSSL::PKey::RSA.generate(2047) } it 'raises an error' do expect do diff --git a/spec/jwt/jwa/unsupported_spec.rb b/spec/jwt/jwa/unsupported_spec.rb new file mode 100644 index 00000000..ba904096 --- /dev/null +++ b/spec/jwt/jwa/unsupported_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +RSpec.describe JWT::JWA::Unsupported do + describe '.sign' do + it 'raises an error for unsupported signing method' do + expect { described_class.sign('data', 'key') }.to raise_error(JWT::EncodeError, 'Unsupported signing method') + end + end + + describe '.verify' do + it 'raises an error for unsupported algorithm' do + expect { described_class.verify('data', 'signature', 'key') }.to raise_error(JWT::VerificationError, 'Algorithm not supported') + end + end +end diff --git a/spec/jwt/jwk/decode_with_jwk_spec.rb b/spec/jwt/jwk/decode_with_jwk_spec.rb index 62d82638..dfbde197 100644 --- a/spec/jwt/jwk/decode_with_jwk_spec.rb +++ b/spec/jwt/jwk/decode_with_jwk_spec.rb @@ -2,7 +2,7 @@ RSpec.describe JWT do describe '.decode for JWK usecase' do - let(:keypair) { OpenSSL::PKey::RSA.new(2048) } + let(:keypair) { test_pkey('rsa-2048-private.pem') } let(:jwk) { JWT::JWK.new(keypair) } let(:public_jwks) { { keys: [jwk.export, { kid: 'not_the_correct_one', kty: 'oct', k: 'secret' }] } } let(:token_payload) { { 'data' => 'something' } } @@ -102,9 +102,9 @@ context 'mixing algorithms using kid header' do let(:hmac_jwk) { JWT::JWK.new('secret') } - let(:rsa_jwk) { JWT::JWK.new(OpenSSL::PKey::RSA.new(2048)) } - let(:ec_jwk_secp384r1) { JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1')) } - let(:ec_jwk_secp521r1) { JWT::JWK.new(OpenSSL::PKey::EC.generate('secp521r1')) } + let(:rsa_jwk) { JWT::JWK.new(test_pkey('rsa-2048-private.pem')) } + let(:ec_jwk_secp384r1) { JWT::JWK.new(test_pkey('ec384-private.pem')) } + let(:ec_jwk_secp521r1) { JWT::JWK.new(test_pkey('ec384-private.pem')) } let(:jwks) { { keys: [hmac_jwk.export(include_private: true), rsa_jwk.export, ec_jwk_secp384r1.export, ec_jwk_secp521r1.export] } } context 'when RSA key is pointed to as HMAC secret' do diff --git a/spec/jwt/jwk/ec_spec.rb b/spec/jwt/jwk/ec_spec.rb index 27acda24..a469dae9 100644 --- a/spec/jwt/jwk/ec_spec.rb +++ b/spec/jwt/jwk/ec_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe JWT::JWK::EC do - let(:ec_key) { OpenSSL::PKey::EC.generate('secp384r1') } + let(:ec_key) { test_pkey('ec384-private.pem') } describe '.new' do subject { described_class.new(keypair) } @@ -21,16 +21,38 @@ expect(subject.private?).to eq false end end + + context 'when a number is given' do + let(:keypair) { 1234 } + it 'raises an argument error' do + expect { subject }.to raise_error(ArgumentError, 'key must be of type OpenSSL::PKey::EC or Hash with key parameters') + end + end + + context 'when EC with unsupported curve is given' do + let(:keypair) { OpenSSL::PKey::EC.generate('prime239v2') } + it 'raises an error' do + expect { subject }.to raise_error(JWT::JWKError, "Unsupported curve 'prime239v2'") + end + end end describe '#keypair' do subject(:jwk) { described_class.new(ec_key) } - it 'warns to stderr' do + it 'returns the key' do expect(jwk.keypair).to eq(ec_key) end end + describe '#public_key' do + subject(:jwk) { described_class.new(ec_key) } + + it 'returns the key' do + expect(jwk.public_key).to eq(ec_key) + end + end + describe '#export' do let(:kid) { nil } subject { described_class.new(keypair, kid).export } diff --git a/spec/jwt/jwk/hmac_spec.rb b/spec/jwt/jwk/hmac_spec.rb index 12f4832a..18c6efab 100644 --- a/spec/jwt/jwk/hmac_spec.rb +++ b/spec/jwt/jwk/hmac_spec.rb @@ -12,6 +12,13 @@ expect(jwk.private?).to eq true end end + + context 'when key is a number' do + let(:key) { 123 } + it 'raises an ArgumentError' do + expect { jwk }.to raise_error(ArgumentError, 'key must be of type String or Hash with key parameters') + end + end end describe '#keypair' do diff --git a/spec/jwt/jwk/rsa_spec.rb b/spec/jwt/jwk/rsa_spec.rb index 9f5d8323..7c574e00 100644 --- a/spec/jwt/jwk/rsa_spec.rb +++ b/spec/jwt/jwk/rsa_spec.rb @@ -228,7 +228,7 @@ describe '.create_rsa_key_using_accessors' do before do - skip 'OpenSSL if RSA#set_key is available there is no accessors anymore' if OpenSSL::PKey::RSA.new.respond_to?(:set_key) + skip 'OpenSSL if RSA#d= is not available there is no accessors anymore' unless OpenSSL::PKey::RSA.new.respond_to?(:d=) end subject(:rsa) { described_class.create_rsa_key_using_accessors(rsa_parameters) } diff --git a/spec/jwt/jwk/set_spec.rb b/spec/jwt/jwk/set_spec.rb index 999622ac..972f35ff 100644 --- a/spec/jwt/jwk/set_spec.rb +++ b/spec/jwt/jwk/set_spec.rb @@ -83,8 +83,8 @@ describe '.select!' do it 'filters the keyset' do jwks = described_class.new([]) - jwks << JWT::JWK.new(OpenSSL::PKey::RSA.new(2048)) - jwks << JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1')) + jwks << JWT::JWK.new(test_pkey('rsa-2048-private.pem')) + jwks << JWT::JWK.new(test_pkey('ec384-private.pem')) jwks.select! { |k| k[:kty] == 'RSA' } expect(jwks.size).to eql(1) expect(jwks.keys[0][:kty]).to eql('RSA') @@ -94,8 +94,8 @@ describe '.reject!' do it 'filters the keyset' do jwks = described_class.new([]) - jwks << JWT::JWK.new(OpenSSL::PKey::RSA.new(2048)) - jwks << JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1')) + jwks << JWT::JWK.new(test_pkey('rsa-2048-private.pem')) + jwks << JWT::JWK.new(test_pkey('ec384-private.pem')) jwks.reject! { |k| k[:kty] == 'RSA' } expect(jwks.size).to eql(1) expect(jwks.keys[0][:kty]).to eql('EC') @@ -105,8 +105,8 @@ describe '.merge' do context 'merges two JWKSs' do it 'when called via .union' do - jwks1 = described_class.new(JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))) - jwks2 = described_class.new(JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1'))) + jwks1 = described_class.new(JWT::JWK.new(test_pkey('rsa-2048-private.pem'))) + jwks2 = described_class.new(JWT::JWK.new(test_pkey('ec384-private.pem'))) jwks3 = jwks1.union(jwks2) expect(jwks1.size).to eql(1) expect(jwks2.size).to eql(1) @@ -116,8 +116,8 @@ end it 'when called via "|" operator' do - jwks1 = described_class.new(JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))) - jwks2 = described_class.new(JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1'))) + jwks1 = described_class.new(JWT::JWK.new(test_pkey('rsa-2048-private.pem'))) + jwks2 = described_class.new(JWT::JWK.new(test_pkey('ec384-private.pem'))) jwks3 = jwks1 | jwks2 expect(jwks1.size).to eql(1) expect(jwks2.size).to eql(1) @@ -127,8 +127,8 @@ end it 'when called directly' do - jwks1 = described_class.new(JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))) - jwks2 = described_class.new(JWT::JWK.new(OpenSSL::PKey::EC.generate('secp384r1'))) + jwks1 = described_class.new(JWT::JWK.new(test_pkey('rsa-2048-private.pem'))) + jwks2 = described_class.new(JWT::JWK.new(test_pkey('ec384-private.pem'))) jwks3 = jwks1.merge(jwks2) expect(jwks1.size).to eql(2) expect(jwks2.size).to eql(1) diff --git a/spec/jwt/jwk_spec.rb b/spec/jwt/jwk_spec.rb index 8945e685..135910f2 100644 --- a/spec/jwt/jwk_spec.rb +++ b/spec/jwt/jwk_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true RSpec.describe JWT::JWK do - let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) } - let(:ec_key) { OpenSSL::PKey::EC.generate('secp384r1') } + let(:rsa_key) { test_pkey('rsa-2048-private.pem') } + let(:ec_key) { test_pkey('ec256k-private.pem') } describe '.import' do let(:keypair) { rsa_key.public_key } @@ -16,6 +16,13 @@ expect(subject.export).to eq(exported_key) end + context 'when number is given' do + let(:params) { 1234 } + it 'raises an error' do + expect { subject }.to raise_error(JWT::JWKError, 'Cannot create JWK from a Integer') + end + end + context 'parsed from JSON' do let(:params) { exported_key } it 'creates a ::JWT::JWK::RSA instance from JSON parsed JWK' do diff --git a/spec/jwt/jwt_spec.rb b/spec/jwt/jwt_spec.rb index e8e2be71..d6a8fe7b 100644 --- a/spec/jwt/jwt_spec.rb +++ b/spec/jwt/jwt_spec.rb @@ -429,6 +429,16 @@ end.to raise_error JWT::IncorrectAlgorithm end + it 'raises error when keyfinder does not find anything' do + token = JWT.encode(payload, 'secret', 'HS256') + + expect do + JWT.decode(token, nil, true, algorithm: 'HS256') do + nil + end + end.to raise_error JWT::DecodeError, 'No verification key available' + end + it 'should raise JWT::IncorrectAlgorithm when algorithms array does not contain algorithm' do token = JWT.encode payload, data[:secret], 'HS512' diff --git a/spec/jwt/token_spec.rb b/spec/jwt/token_spec.rb index 00e47eb4..63d8efe3 100644 --- a/spec/jwt/token_spec.rb +++ b/spec/jwt/token_spec.rb @@ -64,4 +64,48 @@ end end end + + describe '#verify_claims!' do + context 'when required_claims is passed' do + it 'raises error' do + expect { token.verify_claims!(required: ['exp']) }.to raise_error(JWT::MissingRequiredClaim, 'Missing required claim exp') + end + end + end + + describe '#valid_claims?' do + context 'exp claim' do + let(:payload) { { 'exp' => Time.now.to_i - 10, 'pay' => 'load' } } + + context 'when claim is valid' do + it 'returns true' do + expect(token.valid_claims?(exp: { leeway: 1000 })).to be(true) + end + end + + context 'when claim is invalid' do + it 'returns true' do + expect(token.valid_claims?(:exp)).to be(false) + end + end + end + end + + describe '#claim_errors' do + context 'exp claim' do + let(:payload) { { 'exp' => Time.now.to_i - 10, 'pay' => 'load' } } + + context 'when claim is valid' do + it 'returns empty array' do + expect(token.claim_errors(exp: { leeway: 1000 })).to be_empty + end + end + + context 'when claim is invalid' do + it 'returns array with error objects' do + expect(token.claim_errors(:exp).map(&:message)).to eq(['Signature has expired']) + end + end + end + end end