Skip to content

Commit

Permalink
Improve 'none' algorithm handling
Browse files Browse the repository at this point in the history
when decoding a valid token with `algorithm: none`, unless explicitly
requesting not to verify the token, it could not be validated and raised
DecodeError with a message about insufficient segments.

This error message is misleading, because there are the correct number
of segments provided for that algorithm.

With this change, when a token with `algorithm: none` is provided:
* if the caller requests verification && does not specify `none` as an
  algorithm (default behaviour) it now raises `IncorrectAlgorithm`,
  which is a subclass of `DecodeError` to make the issue more clear.
  This is technically a minor change, but should not be breaking -
  it is returning a subclass, so the same rescues will still work. The
  message provided will be different.

* if the caller requests verification && specifies `none` as an allowed
  algorithm, it verifies the claims and decodes the token as it would
  for a valid, signed token.
  This is new behaviour supporting claims verification for 'none' which
  was not previously available and is only "accessed" through explicit
  settings

* if the caller explicitly requests no verification, the token is
  decoded without checking anything (no change in behaviour)
  • Loading branch information
danleyden authored and excpt committed Oct 5, 2020
1 parent 82680f0 commit 98ae462
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
3 changes: 2 additions & 1 deletion lib/jwt/decode.rb
Expand Up @@ -74,6 +74,7 @@ def verify_claims
def validate_segment_count!
return if segment_length == 3
return if !@verify && segment_length == 2 # If no verifying required, the signature is not needed
return if segment_length == 2 && header['alg'] == 'none'

raise(JWT::DecodeError, 'Not enough or too many segments')
end
Expand All @@ -83,7 +84,7 @@ def segment_length
end

def decode_crypto
@signature = JWT::Base64.url_decode(@segments[2])
@signature = JWT::Base64.url_decode(@segments[2] || '')
end

def header
Expand Down
2 changes: 2 additions & 0 deletions lib/jwt/signature.rb
Expand Up @@ -38,6 +38,8 @@ def sign(algorithm, msg, key)
end

def verify(algorithm, key, signing_input, signature)
return true if algorithm == 'none'

raise JWT::DecodeError, 'No verification key available' unless key

algo = ALGOS.find do |alg|
Expand Down
47 changes: 40 additions & 7 deletions spec/jwt_spec.rb
Expand Up @@ -44,18 +44,51 @@

context 'alg: NONE' do
let(:alg) { 'none' }
let(:encoded_token) { data['NONE'] }

it 'should generate a valid token' do
token = JWT.encode payload, nil, alg

expect(token).to eq data['NONE']
expect(token).to eq encoded_token
end

it 'should decode a valid token' do
jwt_payload, header = JWT.decode data['NONE'], nil, false
context 'decoding without verification' do
it 'should decode a valid token' do
jwt_payload, header = JWT.decode encoded_token, nil, false

expect(header['alg']).to eq alg
expect(jwt_payload).to eq payload
expect(header['alg']).to eq alg
expect(jwt_payload).to eq payload
end
end

context 'decoding with verification' do
context 'without specifying the none algorithm' do
it 'should fail to decode the token' do
expect do
JWT.decode encoded_token, nil, true
end.to raise_error JWT::IncorrectAlgorithm
end
end

context 'specifying the none algorithm' do
context 'when the claims are valid' do
it 'should decode the token' do
jwt_payload, header = JWT.decode encoded_token, nil, true, { algorithms: 'none' }

expect(header['alg']).to eq 'none'
expect(jwt_payload).to eq payload
end
end

context 'when the claims are invalid' do
let(:encoded_token) { JWT.encode({ exp: 0 }, nil, 'none') }
it 'should fail to decode the token' do
expect do
JWT.decode encoded_token, nil, true
end.to raise_error JWT::DecodeError
end
end
end
end
end

Expand Down Expand Up @@ -367,7 +400,6 @@
iss_payload = payload.merge(iss: iss)
JWT.encode iss_payload, data[:secret]
end

it 'if verify_iss is set to false (default option) should not raise JWT::InvalidIssuerError' do
expect do
JWT.decode token, data[:secret], true, iss: iss, algorithm: 'HS256'
Expand All @@ -384,7 +416,8 @@

context 'a token with not enough segments' do
it 'raises JWT::DecodeError' do
expect { JWT.decode('ThisIsNotAValidJWTToken.second', nil, true) }.to raise_error(JWT::DecodeError, 'Not enough or too many segments')
token = JWT.encode('ThisIsNotAValidJWTToken', 'secret').split('.').slice(1,2).join
expect { JWT.decode(token, nil, true) }.to raise_error(JWT::DecodeError, 'Not enough or too many segments')
end
end

Expand Down

0 comments on commit 98ae462

Please sign in to comment.