Skip to content

Commit

Permalink
Only issue invalid base64 warning on successful decode
Browse files Browse the repository at this point in the history
To make the patch minimally invasive, this attempts to decode the
given jwt in strict base64 mode only after a successful full
decode and validation.

Most jwts are small, so this should have minimal performance
impact, though it does require decoding twice.  It is possible to
fix the issue and only decode once, but it would be more invasive.

A big advantage of this approach is the use of :uplevel when
calling Kernel#warn, so that the calling location that triggers
the warning is included in the warning message, greatly simplifying
debugging.

Co-authored-by: Joakim Antman <[email protected]>
  • Loading branch information
jeremyevans and anakinj committed Jun 16, 2024
1 parent 764bc34 commit 5db30c9
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

**Features:**

- Print deprecation warnings only on when token decoding succeeds [#601](https://github.com/jwt/ruby-jwt/pull/601) ([@jeremyevans](https://github.com/jeremyevans))
- Your contribution here

**Fixes and enhancements:**
Expand Down
14 changes: 13 additions & 1 deletion lib/jwt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ def encode(payload, key, algorithm = 'HS256', header_fields = {})
end

def decode(jwt, key = nil, verify = true, options = {}, &keyfinder) # rubocop:disable Style/OptionalBooleanParameter
Decode.new(jwt, key, verify, configuration.decode.to_h.merge(options), &keyfinder).decode_segments
if (res = Decode.new(jwt, key, verify, configuration.decode.to_h.merge(options), &keyfinder).decode_segments)
begin
jwt.split('.').each { |part| ::Base64.urlsafe_decode64(part) }
rescue ArgumentError
issue_warning = true
end

if issue_warning
warn('[DEPRECATION WARNING] Invalid base64 input detected, could be because of invalid padding, trailing whitespaces or newline chars. Graceful handling of invalid input will be dropped in the next major version of ruby-jwt', uplevel: 1)
end
end

res
end
end
4 changes: 1 addition & 3 deletions lib/jwt/base64.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ def url_decode(str)
raise unless e.message == 'invalid base64'
raise Base64DecodeError, 'Invalid base64 encoding' if JWT.configuration.strict_base64_decoding

loose_urlsafe_decode64(str).tap do
Deprecations.warning('Invalid base64 input detected, could be because of invalid padding, trailing whitespaces or newline chars. Graceful handling of invalid input will be dropped in the next major version of ruby-jwt')
end
loose_urlsafe_decode64(str)
end

def loose_urlsafe_decode64(str)
Expand Down
19 changes: 19 additions & 0 deletions spec/jwt/jwt_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -944,4 +944,23 @@ def valid_alg?(alg)
end
end
end

context 'when invalid token is valid loose base64' do
it 'does not output deprecations warnings' do
expect {
2.times do
JWT.decode("#{JWT.encode('a', 'b')} 9", 'b')
rescue JWT::VerificationError
nil
end
JWT.decode(JWT.encode('a', 'b'), 'b')
}.not_to output(/DEPRECATION/).to_stderr
end
end

context 'when valid token is invalid strict base64' do
it 'does outputs deprecation warning' do
expect { JWT.decode("#{JWT.encode('a', 'b')} ", 'b') }.to output(/DEPRECATION/).to_stderr
end
end
end

0 comments on commit 5db30c9

Please sign in to comment.