diff --git a/app/controllers/api/v1/accounts/integrations/linear_controller.rb b/app/controllers/api/v1/accounts/integrations/linear_controller.rb index eb6525bb1..9ca0c72fd 100644 --- a/app/controllers/api/v1/accounts/integrations/linear_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/linear_controller.rb @@ -126,7 +126,7 @@ class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::Bas return unless @hook&.access_token begin - linear_client = Linear.new(@hook.access_token) + linear_client = Linear.new(@hook.access_token, refresh_token: @hook.settings&.[]('refresh_token')) linear_client.revoke_token rescue StandardError => e Rails.logger.error "Failed to revoke Linear token: #{e.message}" diff --git a/app/controllers/linear/callbacks_controller.rb b/app/controllers/linear/callbacks_controller.rb index 2eea49333..cdf6f630e 100644 --- a/app/controllers/linear/callbacks_controller.rb +++ b/app/controllers/linear/callbacks_controller.rb @@ -2,6 +2,8 @@ class Linear::CallbacksController < ApplicationController include Linear::IntegrationHelper def show + return redirect_to(safe_linear_redirect_uri) if params[:code].blank? || account_id.blank? + @response = oauth_client.auth_code.get_token( params[:code], redirect_uri: "#{base_url}/linear/callback" @@ -10,7 +12,7 @@ class Linear::CallbacksController < ApplicationController handle_response rescue StandardError => e Rails.logger.error("Linear callback error: #{e.message}") - redirect_to linear_redirect_uri + redirect_to safe_linear_redirect_uri end private @@ -31,22 +33,19 @@ class Linear::CallbacksController < ApplicationController end def handle_response - hook = account.hooks.new( + raise ArgumentError, 'Missing access token in Linear OAuth response' if parsed_body['access_token'].blank? + + hook = account.hooks.find_or_initialize_by(app_id: 'linear') + hook.assign_attributes( access_token: parsed_body['access_token'], status: 'enabled', - app_id: 'linear', - settings: { - token_type: parsed_body['token_type'], - expires_in: parsed_body['expires_in'], - scope: parsed_body['scope'] - } + settings: merged_integration_settings(hook.settings) ) - # You may wonder why we're not handling the refresh token update, since the token will expire only after 10 years, https://github.com/linear/linear/issues/251 hook.save! redirect_to linear_redirect_uri rescue StandardError => e Rails.logger.error("Linear callback error: #{e.message}") - redirect_to linear_redirect_uri + redirect_to safe_linear_redirect_uri end def account @@ -54,19 +53,47 @@ class Linear::CallbacksController < ApplicationController end def account_id - return unless params[:state] + return @account_id if instance_variable_defined?(:@account_id) - verify_linear_token(params[:state]) + @account_id = params[:state].present? ? verify_linear_token(params[:state]) : nil end def linear_redirect_uri "#{ENV.fetch('FRONTEND_URL', nil)}/app/accounts/#{account.id}/settings/integrations/linear" end + def safe_linear_redirect_uri + return base_url if account_id.blank? + + linear_redirect_uri + rescue StandardError + base_url + end + def parsed_body @parsed_body ||= @response.response.parsed end + def integration_settings + { + token_type: parsed_body['token_type'], + expires_in: parsed_body['expires_in'], + expires_on: expires_on, + scope: parsed_body['scope'], + refresh_token: parsed_body['refresh_token'] + }.compact + end + + def merged_integration_settings(existing_settings) + existing_settings.to_h.with_indifferent_access.merge(integration_settings) + end + + def expires_on + return if parsed_body['expires_in'].blank? + + (Time.current.utc + parsed_body['expires_in'].to_i.seconds).to_s + end + def base_url ENV.fetch('FRONTEND_URL', 'http://localhost:3000') end diff --git a/lib/integrations/linear/access_token_service.rb b/lib/integrations/linear/access_token_service.rb new file mode 100644 index 000000000..be6fa481e --- /dev/null +++ b/lib/integrations/linear/access_token_service.rb @@ -0,0 +1,121 @@ +class Integrations::Linear::AccessTokenService + TOKEN_URL = 'https://api.linear.app/oauth/token'.freeze + MIGRATE_OLD_TOKEN_URL = 'https://api.linear.app/oauth/migrate_old_token'.freeze + TOKEN_EXPIRY_BUFFER = 1.minute + + pattr_initialize [:hook!] + + def access_token + return hook.access_token if token_valid? + return refresh_access_token if refresh_token.present? + return migrate_legacy_token if migration_applicable? + + hook.access_token + end + + private + + def refresh_access_token + response = HTTParty.post( + TOKEN_URL, + headers: url_encoded_headers, + body: { + grant_type: 'refresh_token', + refresh_token: refresh_token, + client_id: client_id, + client_secret: client_secret + } + ) + + return fallback_access_token unless response.success? + + persist_tokens(response.parsed_response) + hook.access_token + rescue StandardError => e + Rails.logger.error("Linear token refresh failed for hook #{hook.id}: #{e.message}") + fallback_access_token + end + + def migrate_legacy_token + response = HTTParty.post( + MIGRATE_OLD_TOKEN_URL, + headers: url_encoded_headers, + body: { + access_token: hook.access_token, + client_id: client_id, + client_secret: client_secret + } + ) + + return fallback_access_token unless response.success? + + persist_tokens(response.parsed_response) + hook.access_token + rescue StandardError => e + Rails.logger.error("Linear legacy token migration failed for hook #{hook.id}: #{e.message}") + fallback_access_token + end + + def persist_tokens(token_data) + raise ArgumentError, 'Missing access token in Linear token response' if token_data['access_token'].blank? + + current_settings = hook_settings + updated_settings = current_settings.merge( + token_type: token_data['token_type'] || current_settings[:token_type], + expires_in: token_data['expires_in'] || current_settings[:expires_in], + expires_on: expires_on(token_data['expires_in']), + scope: token_data['scope'] || current_settings[:scope], + refresh_token: token_data['refresh_token'] || current_settings[:refresh_token] + ).compact + + hook.update!( + access_token: token_data['access_token'], + settings: updated_settings + ) + end + + def token_valid? + expiry = hook_settings[:expires_on] + return false if expiry.blank? + + Time.zone.parse(expiry).utc > (Time.current.utc + TOKEN_EXPIRY_BUFFER) + rescue StandardError + false + end + + def migration_applicable? + hook_settings[:token_type].present? + end + + def refresh_token + hook_settings[:refresh_token] + end + + def hook_settings + hook.settings.to_h.with_indifferent_access + end + + def expires_on(expires_in) + return hook_settings[:expires_on] if expires_in.blank? + + (Time.current.utc + expires_in.to_i.seconds).to_s + end + + def url_encoded_headers + { 'Content-Type' => 'application/x-www-form-urlencoded' } + end + + def client_id + GlobalConfigService.load('LINEAR_CLIENT_ID', nil) + end + + def client_secret + GlobalConfigService.load('LINEAR_CLIENT_SECRET', nil) + end + + def fallback_access_token + hook.reload.access_token + rescue StandardError + hook.access_token + end +end diff --git a/lib/integrations/linear/processor_service.rb b/lib/integrations/linear/processor_service.rb index a3447b79e..bb978ea66 100644 --- a/lib/integrations/linear/processor_service.rb +++ b/lib/integrations/linear/processor_service.rb @@ -77,6 +77,10 @@ class Integrations::Linear::ProcessorService end def linear_client - @linear_client ||= Linear.new(linear_hook.access_token) + @linear_client ||= Linear.new(linear_access_token) + end + + def linear_access_token + @linear_access_token ||= Integrations::Linear::AccessTokenService.new(hook: linear_hook).access_token end end diff --git a/lib/linear.rb b/lib/linear.rb index 2ffcf8cca..0f8011cf2 100644 --- a/lib/linear.rb +++ b/lib/linear.rb @@ -3,8 +3,9 @@ class Linear REVOKE_URL = 'https://api.linear.app/oauth/revoke'.freeze PRIORITY_LEVELS = (0..4).to_a - def initialize(access_token) + def initialize(access_token, refresh_token: nil) @access_token = access_token + @refresh_token = refresh_token raise ArgumentError, 'Missing Credentials' if access_token.blank? end @@ -79,9 +80,13 @@ class Linear end def revoke_token + token = @refresh_token.presence || @access_token + token_type_hint = @refresh_token.present? ? 'refresh_token' : 'access_token' + response = HTTParty.post( REVOKE_URL, - headers: { 'Authorization' => "Bearer #{@access_token}", 'Content-Type' => 'application/json' } + headers: { 'Content-Type' => 'application/x-www-form-urlencoded' }, + body: { token: token, token_type_hint: token_type_hint } ) response.success? end diff --git a/spec/controllers/linear/callbacks_controller_spec.rb b/spec/controllers/linear/callbacks_controller_spec.rb index 66018776c..aa8479479 100644 --- a/spec/controllers/linear/callbacks_controller_spec.rb +++ b/spec/controllers/linear/callbacks_controller_spec.rb @@ -9,9 +9,11 @@ RSpec.describe Linear::CallbacksController, type: :request do describe 'GET /linear/callback' do let(:access_token) { SecureRandom.hex(10) } + let(:refresh_token) { SecureRandom.hex(10) } let(:response_body) do { 'access_token' => access_token, + 'refresh_token' => refresh_token, 'token_type' => 'Bearer', 'expires_in' => 7200, 'scope' => 'read,write' @@ -35,7 +37,7 @@ RSpec.describe Linear::CallbacksController, type: :request do ) end - it 'creates a new integration hook' do + it 'creates a new integration hook', :aggregate_failures do expect do get linear_callback_path, params: { code: code, state: state } end.to change(Integrations::Hook, :count).by(1) @@ -44,11 +46,11 @@ RSpec.describe Linear::CallbacksController, type: :request do expect(hook.access_token).to eq(access_token) expect(hook.app_id).to eq('linear') expect(hook.status).to eq('enabled') - expect(hook.settings).to eq( - 'token_type' => 'Bearer', - 'expires_in' => 7200, - 'scope' => 'read,write' - ) + expect(hook.settings['token_type']).to eq('Bearer') + expect(hook.settings['expires_in']).to eq(7200) + expect(hook.settings['scope']).to eq('read,write') + expect(hook.settings['refresh_token']).to eq(refresh_token) + expect(hook.settings['expires_on']).to be_present expect(response).to redirect_to(linear_redirect_uri) end end @@ -69,6 +71,106 @@ RSpec.describe Linear::CallbacksController, type: :request do end end + context 'when state is missing' do + it 'redirects to frontend root' do + get linear_callback_path, params: { code: code } + expect(response).to redirect_to('http://www.example.com') + end + end + + context 'when state is invalid' do + it 'redirects to frontend root' do + get linear_callback_path, params: { code: code, state: 'invalid-state' } + expect(response).to redirect_to('http://www.example.com') + end + end + + context 'when hook exists and response omits refresh_token' do + let!(:existing_hook) do + create( + :integrations_hook, + :linear, + account: account, + settings: { + 'refresh_token' => 'existing_refresh_token', + 'token_type' => 'Bearer', + 'scope' => 'read,write', + 'expires_on' => 1.day.from_now.utc.to_s + } + ) + end + let(:response_body) do + { + 'access_token' => access_token, + 'token_type' => 'Bearer', + 'expires_in' => 7200, + 'scope' => 'read,write' + } + end + + before do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return( + status: 200, + body: response_body.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + end + + it 'preserves existing refresh token', :aggregate_failures do + get linear_callback_path, params: { code: code, state: state } + + existing_hook.reload + expect(existing_hook.access_token).to eq(access_token) + expect(existing_hook.settings['refresh_token']).to eq('existing_refresh_token') + end + end + + context 'when hook exists and response omits access_token' do + let!(:existing_hook) do + create( + :integrations_hook, + :linear, + account: account, + access_token: 'existing_access_token', + settings: { + 'refresh_token' => 'existing_refresh_token', + 'token_type' => 'Bearer', + 'scope' => 'read,write', + 'expires_on' => 1.day.from_now.utc.to_s + } + ) + end + let(:response_body) do + { + 'refresh_token' => refresh_token, + 'token_type' => 'Bearer', + 'expires_in' => 7200, + 'scope' => 'read,write' + } + end + + before do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return( + status: 200, + body: response_body.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + end + + it 'does not overwrite the existing hook', :aggregate_failures do + expect do + get linear_callback_path, params: { code: code, state: state } + end.not_to change(Integrations::Hook, :count) + + existing_hook.reload + expect(existing_hook.access_token).to eq('existing_access_token') + expect(existing_hook.settings['refresh_token']).to eq('existing_refresh_token') + expect(response).to redirect_to(linear_redirect_uri) + end + end + context 'when the token is invalid' do before do stub_request(:post, 'https://api.linear.app/oauth/token') diff --git a/spec/lib/integrations/linear/access_token_service_spec.rb b/spec/lib/integrations/linear/access_token_service_spec.rb new file mode 100644 index 000000000..023af048f --- /dev/null +++ b/spec/lib/integrations/linear/access_token_service_spec.rb @@ -0,0 +1,178 @@ +require 'rails_helper' + +describe Integrations::Linear::AccessTokenService do + let(:account) { create(:account) } + let(:client_id) { 'linear_client_id' } + let(:client_secret) { 'linear_client_secret' } + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('LINEAR_CLIENT_ID', nil).and_return(client_id) + allow(GlobalConfigService).to receive(:load).with('LINEAR_CLIENT_SECRET', nil).and_return(client_secret) + end + + describe '#access_token' do + context 'when access token is still valid' do + let(:hook) do + create( + :integrations_hook, + :linear, + account: account, + access_token: 'valid_access_token', + settings: { + refresh_token: 'refresh_token', + token_type: 'Bearer', + scope: 'read,write', + expires_on: 30.minutes.from_now.utc.to_s + } + ) + end + + it 'returns the current access token' do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return(status: 200, body: {}.to_json, headers: { 'Content-Type' => 'application/json' }) + stub_request(:post, 'https://api.linear.app/oauth/migrate_old_token') + .to_return(status: 200, body: {}.to_json, headers: { 'Content-Type' => 'application/json' }) + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('valid_access_token') + expect(WebMock).not_to have_requested(:post, 'https://api.linear.app/oauth/token') + expect(WebMock).not_to have_requested(:post, 'https://api.linear.app/oauth/migrate_old_token') + end + end + + context 'when access token is expired and refresh token is present' do + let(:hook) do + create( + :integrations_hook, + :linear, + account: account, + access_token: 'expired_access_token', + settings: { + refresh_token: 'old_refresh_token', + token_type: 'Bearer', + scope: 'read,write', + expires_on: 1.hour.ago.utc.to_s + } + ) + end + + it 'refreshes the token and persists new values' do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return( + status: 200, + body: { + access_token: 'new_access_token', + refresh_token: 'new_refresh_token', + token_type: 'Bearer', + expires_in: 7200, + scope: 'read,write' + }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('new_access_token') + hook.reload + expect(hook.access_token).to eq('new_access_token') + expect(hook.settings['refresh_token']).to eq('new_refresh_token') + expect(hook.settings['expires_in']).to eq(7200) + expect(hook.settings['expires_on']).to be_present + end + + it 'falls back to latest persisted token on refresh failure' do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return(status: 401, body: { error: 'invalid_grant' }.to_json, headers: { 'Content-Type' => 'application/json' }) + + Integrations::Hook.find(hook.id).update!(access_token: 'rotated_access_token') + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('rotated_access_token') + end + + it 'does not overwrite the existing token on malformed success response' do + stub_request(:post, 'https://api.linear.app/oauth/token') + .to_return( + status: 200, + body: { + refresh_token: 'new_refresh_token', + token_type: 'Bearer', + expires_in: 7200, + scope: 'read,write' + }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('expired_access_token') + hook.reload + expect(hook.access_token).to eq('expired_access_token') + expect(hook.settings['refresh_token']).to eq('old_refresh_token') + end + end + + context 'when refresh token is missing and legacy migration is applicable' do + let(:hook) do + create( + :integrations_hook, + :linear, + account: account, + access_token: 'legacy_access_token', + settings: { + token_type: 'Bearer', + scope: 'read,write' + } + ) + end + + it 'migrates the legacy token and persists refresh token data' do + stub_request(:post, 'https://api.linear.app/oauth/migrate_old_token') + .to_return( + status: 200, + body: { + access_token: 'migrated_access_token', + refresh_token: 'migrated_refresh_token', + token_type: 'Bearer', + expires_in: 7200, + scope: 'read,write' + }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('migrated_access_token') + hook.reload + expect(hook.access_token).to eq('migrated_access_token') + expect(hook.settings['refresh_token']).to eq('migrated_refresh_token') + expect(hook.settings['expires_in']).to eq(7200) + expect(hook.settings['expires_on']).to be_present + end + + it 'does not overwrite the existing token on malformed migration success response' do + stub_request(:post, 'https://api.linear.app/oauth/migrate_old_token') + .to_return( + status: 200, + body: { + refresh_token: 'migrated_refresh_token', + token_type: 'Bearer', + expires_in: 7200, + scope: 'read,write' + }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + + service = described_class.new(hook: hook) + + expect(service.access_token).to eq('legacy_access_token') + hook.reload + expect(hook.access_token).to eq('legacy_access_token') + expect(hook.settings['token_type']).to eq('Bearer') + end + end + end +end diff --git a/spec/lib/linear_spec.rb b/spec/lib/linear_spec.rb index 65c7cb664..d6e9bc0b1 100644 --- a/spec/lib/linear_spec.rb +++ b/spec/lib/linear_spec.rb @@ -2,7 +2,9 @@ require 'rails_helper' describe Linear do let(:access_token) { 'valid_access_token' } + let(:refresh_token) { 'valid_refresh_token' } let(:url) { 'https://api.linear.app/graphql' } + let(:revoke_url) { 'https://api.linear.app/oauth/revoke' } let(:linear_client) { described_class.new(access_token) } let(:headers) { { 'Content-Type' => 'application/json', 'Authorization' => "Bearer #{access_token}" } } @@ -433,4 +435,30 @@ describe Linear do end end end + + context 'when revoking a token' do + it 'uses the refresh token when present' do + client = described_class.new(access_token, refresh_token: refresh_token) + + stub_request(:post, revoke_url) + .with( + headers: { 'Content-Type' => 'application/x-www-form-urlencoded' }, + body: { token: refresh_token, token_type_hint: 'refresh_token' } + ) + .to_return(status: 200, body: '', headers: {}) + + expect(client.revoke_token).to be(true) + end + + it 'falls back to the access token when refresh token is absent' do + stub_request(:post, revoke_url) + .with( + headers: { 'Content-Type' => 'application/x-www-form-urlencoded' }, + body: { token: access_token, token_type_hint: 'access_token' } + ) + .to_return(status: 200, body: '', headers: {}) + + expect(linear_client.revoke_token).to be(true) + end + end end