From d369d4c841e15d5d55b1aafc3f2f768959d844e7 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 7 Aug 2026 01:14:24 +1000 Subject: [PATCH 1/2] Validate OAuth state before token exchange --- lib/xero-ruby/api_client.rb | 3 +- spec/api_client_spec.rb | 83 +++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/lib/xero-ruby/api_client.rb b/lib/xero-ruby/api_client.rb index dc9d679a..4f79e673 100644 --- a/lib/xero-ruby/api_client.rb +++ b/lib/xero-ruby/api_client.rb @@ -164,6 +164,8 @@ def get_client_credentials_token end def get_token_set_from_callback(params) + validate_state(params) + data = { grant_type: @grant_type, code: params['code'], @@ -172,7 +174,6 @@ def get_token_set_from_callback(params) token_set = token_request(data, '/token') validate_tokens(token_set) - validate_state(params) return token_set end diff --git a/spec/api_client_spec.rb b/spec/api_client_spec.rb index 617433b7..9253d56b 100644 --- a/spec/api_client_spec.rb +++ b/spec/api_client_spec.rb @@ -100,6 +100,89 @@ end end + describe '#get_token_set_from_callback' do + let(:credentials) do + { + client_id: 'abc', + client_secret: '123', + redirect_uri: 'https://mydomain.com/callback', + scopes: 'openid profile email', + state: 'expected-state' + } + end + let(:api_client) { XeroRuby::ApiClient.new(credentials: credentials) } + let(:existing_token_set) do + { + 'access_token' => 'existing-access-token', + 'id_token' => 'existing-id-token' + } + end + let(:new_token_set) do + { + 'access_token' => 'new-access-token', + 'id_token' => 'new-id-token' + } + end + + it 'rejects a mismatched state before requesting or mutating tokens' do + api_client.set_token_set(existing_token_set) + + expect(api_client).not_to receive(:token_request) + expect(api_client).not_to receive(:set_token_set) + + expect { + api_client.get_token_set_from_callback( + 'code' => 'callback-code', + 'state' => 'attacker-state' + ) + }.to raise_error( + StandardError, + 'WARNING: @config.state: expected-state and OAuth callback state: attacker-state do not match!' + ) + + expect(api_client.token_set).to eq(existing_token_set.with_indifferent_access) + expect(api_client.access_token).to eq('existing-access-token') + expect(api_client.id_token).to eq('existing-id-token') + end + + it 'exchanges and stores tokens when a supplied state matches' do + callback_params = { + 'code' => 'callback-code', + 'state' => 'expected-state' + } + + expect(api_client).to receive(:token_request).with( + { + grant_type: 'authorization_code', + code: 'callback-code', + redirect_uri: 'https://mydomain.com/callback' + }, + '/token' + ) do + api_client.set_token_set(new_token_set) + new_token_set + end + expect(api_client).to receive(:validate_tokens).with(new_token_set).and_return(true) + + expect(api_client.get_token_set_from_callback(callback_params)).to eq(new_token_set) + expect(api_client.token_set).to eq(new_token_set.with_indifferent_access) + end + + it 'continues to allow callbacks without state when no state was configured' do + client_without_state = XeroRuby::ApiClient.new(credentials: { + client_id: 'abc', + client_secret: '123', + redirect_uri: 'https://mydomain.com/callback', + scopes: 'openid profile email' + }) + + expect(client_without_state).to receive(:token_request).and_return(new_token_set) + expect(client_without_state).to receive(:validate_tokens).with(new_token_set).and_return(true) + + expect(client_without_state.get_token_set_from_callback('code' => 'callback-code')).to eq(new_token_set) + end + end + describe 'api_client helper functions' do let(:api_client) { XeroRuby::ApiClient.new } let(:token_set) { { 'access_token': 'eyx.authorization.data', 'id_token': 'eyx.authentication.data', 'refresh_token': 'REFRESHMENTS' } } From b43b77bc5ee79ed1747b1e0f36ecb45531fa3519 Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:05:49 +1000 Subject: [PATCH 2/2] fix: keep OAuth state values out of errors --- lib/xero-ruby/api_client.rb | 2 +- spec/api_client_spec.rb | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/xero-ruby/api_client.rb b/lib/xero-ruby/api_client.rb index 4f79e673..df80928a 100644 --- a/lib/xero-ruby/api_client.rb +++ b/lib/xero-ruby/api_client.rb @@ -190,7 +190,7 @@ def validate_tokens(token_set) def validate_state(params) if params['state'] != @state - raise StandardError.new "WARNING: @config.state: #{@state} and OAuth callback state: #{params['state']} do not match!" + raise StandardError.new 'WARNING: OAuth callback state does not match!' end return true end diff --git a/spec/api_client_spec.rb b/spec/api_client_spec.rb index 9253d56b..3387923d 100644 --- a/spec/api_client_spec.rb +++ b/spec/api_client_spec.rb @@ -71,7 +71,7 @@ } api_client = XeroRuby::ApiClient.new(credentials: creds) altered_state = { 'state': 'not-original-state' } - expect { api_client.validate_state(altered_state) }.to raise_error(StandardError, 'WARNING: @config.state: custom-state and OAuth callback state: do not match!') + expect { api_client.validate_state(altered_state) }.to raise_error(StandardError, 'WARNING: OAuth callback state does not match!') end end @@ -137,8 +137,10 @@ ) }.to raise_error( StandardError, - 'WARNING: @config.state: expected-state and OAuth callback state: attacker-state do not match!' - ) + 'WARNING: OAuth callback state does not match!' + ) { |error| + expect(error.message).not_to include('expected-state', 'attacker-state') + } expect(api_client.token_set).to eq(existing_token_set.with_indifferent_access) expect(api_client.access_token).to eq('existing-access-token')