From 6a4d62f00e86637060851448b43b0620f6280ae2 Mon Sep 17 00:00:00 2001 From: James Stonehill Date: Tue, 22 Jan 2019 16:08:48 +0000 Subject: [PATCH 1/3] Remove Manifest file --- Manifest | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 Manifest diff --git a/Manifest b/Manifest deleted file mode 100644 index c2e11dcb..00000000 --- a/Manifest +++ /dev/null @@ -1,8 +0,0 @@ -Rakefile -README.md -LICENSE -lib/jwt.rb -lib/jwt/json.rb -spec/spec_helper.rb -spec/jwt_spec.rb -Manifest From b2315c854274c1e00e2cb86fcee9dec2212b892e Mon Sep 17 00:00:00 2001 From: James Stonehill Date: Tue, 22 Jan 2019 17:35:43 +0000 Subject: [PATCH 2/3] Extract claims validation into class --- lib/jwt/claims_validator.rb | 31 +++++++++++++++++++++++++++++ lib/jwt/encode.rb | 21 ++++++-------------- spec/jwt/claims_validator_spec.rb | 33 +++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 lib/jwt/claims_validator.rb create mode 100644 spec/jwt/claims_validator_spec.rb diff --git a/lib/jwt/claims_validator.rb b/lib/jwt/claims_validator.rb new file mode 100644 index 00000000..5e2ad573 --- /dev/null +++ b/lib/jwt/claims_validator.rb @@ -0,0 +1,31 @@ +require_relative './error' + +module JWT + class ClaimsValidator + INTEGER_CLAIMS = %i[ + exp + ] + + def initialize(payload) + @payload = payload.each_with_object({}) { |(k, v), h| h[k.to_sym] = v } + end + + def validate! + validate_int_claims + + true + end + + private + + def validate_int_claims + INTEGER_CLAIMS.each do |claim| + validate_is_int(claim) if @payload.key?(claim) + end + end + + def validate_is_int(claim) + raise InvalidPayload, "#{claim} claim must be an integer" unless @payload[:exp].is_a?(Integer) + end + end +end diff --git a/lib/jwt/encode.rb b/lib/jwt/encode.rb index 8769e7f4..bda51bbd 100644 --- a/lib/jwt/encode.rb +++ b/lib/jwt/encode.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true +require_relative './claims_validator' + # JWT::Encode module module JWT # Encoding logic for JWT class Encode ALG_NONE = 'none'.freeze ALG_KEY = 'alg'.freeze - EXP_KEY = 'exp'.freeze - EXP_KEYS = [EXP_KEY, EXP_KEY.to_sym].freeze def initialize(options) @payload = options[:payload] @@ -22,18 +22,6 @@ def segments private - def validate_payload! - return unless @payload && @payload.is_a?(Hash) - - validate_exp! - end - - def validate_exp! - return if EXP_KEYS.all? { |key| !@payload.key?(key) || @payload[key].is_a?(Integer) } - - raise InvalidPayload, 'exp claim must be an integer' - end - def encoded_header @encoded_header ||= encode_header end @@ -55,7 +43,10 @@ def encode_header end def encode_payload - validate_payload! + if @payload && @payload.is_a?(Hash) + ClaimsValidator.new(@payload).validate! + end + encode(@payload) end diff --git a/spec/jwt/claims_validator_spec.rb b/spec/jwt/claims_validator_spec.rb new file mode 100644 index 00000000..4436ee42 --- /dev/null +++ b/spec/jwt/claims_validator_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require 'jwt/claims_validator' + +RSpec.describe JWT::ClaimsValidator do + describe '#validate!' do + it 'returns true if the payload is valid' do + valid_payload = { 'exp' => 12345 } + subject = described_class.new(valid_payload) + + expect(subject.validate!).to eq(true) + end + + context "exp validation" do + it 'raises an error when the value of the exp claim is a string' do + subject = described_class.new({ exp: '1' }) + expect { subject.validate! }.to raise_error JWT::InvalidPayload + end + + it 'raises an error when the value of the exp claim is a Time object' do + subject = described_class.new({ exp: Time.now }) + expect { subject.validate! }.to raise_error JWT::InvalidPayload + end + + it 'validates the exp when the exp key is either a string or a symbol' do + symbol = described_class.new({ exp: true }) + expect { symbol.validate! }.to raise_error JWT::InvalidPayload + + string = described_class.new({ 'exp' => true }) + expect { string.validate! }.to raise_error JWT::InvalidPayload + end + end + end +end From 9f9913eed07e6dd5f2b938de331a3bb4054f624b Mon Sep 17 00:00:00 2001 From: James Stonehill Date: Sat, 26 Jan 2019 14:00:57 +0000 Subject: [PATCH 3/3] Validate iat and nbf claims like we are doing for exp --- lib/jwt/claims_validator.rb | 6 ++++-- spec/jwt/claims_validator_spec.rb | 28 ++++++++++++++++++++-------- spec/jwt_spec.rb | 31 +++++++++++-------------------- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/lib/jwt/claims_validator.rb b/lib/jwt/claims_validator.rb index 5e2ad573..8891adfb 100644 --- a/lib/jwt/claims_validator.rb +++ b/lib/jwt/claims_validator.rb @@ -4,7 +4,9 @@ module JWT class ClaimsValidator INTEGER_CLAIMS = %i[ exp - ] + iat + nbf + ].freeze def initialize(payload) @payload = payload.each_with_object({}) { |(k, v), h| h[k.to_sym] = v } @@ -25,7 +27,7 @@ def validate_int_claims end def validate_is_int(claim) - raise InvalidPayload, "#{claim} claim must be an integer" unless @payload[:exp].is_a?(Integer) + raise InvalidPayload, "#{claim} claim must be an Integer but it is a #{@payload[claim].class}" unless @payload[claim].is_a?(Integer) end end end diff --git a/spec/jwt/claims_validator_spec.rb b/spec/jwt/claims_validator_spec.rb index 4436ee42..b5a04379 100644 --- a/spec/jwt/claims_validator_spec.rb +++ b/spec/jwt/claims_validator_spec.rb @@ -10,24 +10,36 @@ expect(subject.validate!).to eq(true) end - context "exp validation" do - it 'raises an error when the value of the exp claim is a string' do - subject = described_class.new({ exp: '1' }) + shared_examples_for 'an integer claim' do |claim| + it "raises an error when the value of the #{claim} claim is a string" do + subject = described_class.new({ claim => '1' }) expect { subject.validate! }.to raise_error JWT::InvalidPayload end - it 'raises an error when the value of the exp claim is a Time object' do - subject = described_class.new({ exp: Time.now }) + it "raises an error when the value of the #{claim} claim is a Time object" do + subject = described_class.new({ claim => Time.now }) expect { subject.validate! }.to raise_error JWT::InvalidPayload end - it 'validates the exp when the exp key is either a string or a symbol' do - symbol = described_class.new({ exp: true }) + it "validates the #{claim} claim when the key is either a string or a symbol" do + symbol = described_class.new({ claim.to_sym => true }) expect { symbol.validate! }.to raise_error JWT::InvalidPayload - string = described_class.new({ 'exp' => true }) + string = described_class.new({ claim.to_s => true }) expect { string.validate! }.to raise_error JWT::InvalidPayload end end + + context 'exp claim' do + it_should_behave_like 'an integer claim', :exp + end + + context 'iat claim' do + it_should_behave_like 'an integer claim', :iat + end + + context 'nbf claim' do + it_should_behave_like 'an integer claim', :nbf + end end end diff --git a/spec/jwt_spec.rb b/spec/jwt_spec.rb index 17c9ae36..1592c731 100644 --- a/spec/jwt_spec.rb +++ b/spec/jwt_spec.rb @@ -60,30 +60,21 @@ end context 'payload validation' do - subject { JWT.encode(payload, nil, 'none') } - let(:payload) { { 'exp' => exp } } + it 'validates the payload with the ClaimsValidator if the payload is a hash' do + validator = double() + expect(JWT::ClaimsValidator).to receive(:new) { validator } + expect(validator).to receive(:validate!) { true } - context 'when exp is given as a non Integer' do - let(:exp) { Time.now.to_i.to_s } - it 'raises an JWT::InvalidPayload error' do - expect { subject }.to raise_error(JWT::InvalidPayload, 'exp claim must be an integer') - end - end - - context 'when exp is given as an Integer' do - let(:exp) { 1234 } - - it 'encodes the payload' do - expect(subject).to be_a(String) - end + payload = {} + JWT.encode payload, "secret", JWT::Algos::Hmac::SUPPORTED.sample end - context 'when the key for exp is a symbol' do - let(:payload) { { :exp => 'NotAInteger' } } + it 'does not validate the payload if it is not present' do + validator = double() + expect(JWT::ClaimsValidator).not_to receive(:new) { validator } - it 'raises an JWT::InvalidPayload error' do - expect { subject }.to raise_error(JWT::InvalidPayload, 'exp claim must be an integer') - end + payload = nil + JWT.encode payload, "secret", JWT::Algos::Hmac::SUPPORTED.sample end end