diff --git a/CHANGELOG.md b/CHANGELOG.md index 715428bc..4d8ad30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Please file a bug if you notice a violation of semantic versioning. ### Fixed - `OAuth2::AccessToken` no longer raises `NoMethodError` when `expires_latency` is set but the token has no known expiry (no `expires_in`/`expires_at`, or `expires_in: 0`), e.g. `client.get_token(params, expires_latency: 30)` against a provider that omits `expires_in` by @iamibi +- `OAuth2::Client#get_token` raises response parse errors (e.g. `JSON::ParserError`) when using the deprecated `extract_access_token` option, rather than returning `nil` by @dpep ### Security diff --git a/lib/oauth2/client.rb b/lib/oauth2/client.rb index 37e90149..1f9f6298 100644 --- a/lib/oauth2/client.rb +++ b/lib/oauth2/client.rb @@ -600,12 +600,15 @@ def build_access_token(response, access_token_opts, access_token_class) # # @api private def build_access_token_legacy(response, access_token_opts, extract_access_token) - extract_access_token.call(self, response.parsed.merge(access_token_opts)) - rescue - # An error will be raised by the called if nil is returned and options[:raise_errors] is truthy, so this rescue is but temporary. - # Unfortunately, it does hide the real error, but this is deprecated legacy code, - # and this was effectively the long-standing pre-existing behavior, so there is little point in changing it. - nil + parsed = response.parsed + begin + extract_access_token.call(self, parsed.merge(access_token_opts)) + rescue + # An error will be raised by the called if nil is returned and options[:raise_errors] is truthy, so this rescue is but temporary. + # Unfortunately, it does hide the real error, but this is deprecated legacy code, + # and this was effectively the long-standing pre-existing behavior, so there is little point in changing it. + nil + end end def oauth_debug_logging(builder) diff --git a/spec/oauth2/client_spec.rb b/spec/oauth2/client_spec.rb index f1654779..ca71844f 100644 --- a/spec/oauth2/client_spec.rb +++ b/spec/oauth2/client_spec.rb @@ -1212,8 +1212,8 @@ let(:status_code) { 200 } let(:extract_proc) { proc { |client, hash| raise ArgumentError } } - it "returns a nil :access_token" do - expect(client.get_token({}, {}, extract_proc)).to be_nil + it "raises the parse error" do + expect { client.get_token({}, {}, extract_proc) }.to raise_error(JSON::ParserError) end end end @@ -1229,6 +1229,14 @@ expect(token.response).to be_a OAuth2::Response expect(token.response.parsed).to eq("access_token" => "the-token") end + + context "when extract_access_token raises an exception" do + let(:extract_proc) { proc { |client, hash| raise ArgumentError } } + + it "returns a nil :access_token" do + expect(client.get_token({}, {}, extract_proc)).to be_nil + end + end end context "when Content-Type is not JSON" do