diff --git a/app/controllers/api/v1/accounts/integrations/base_controller.rb b/app/controllers/api/v1/accounts/integrations/base_controller.rb new file mode 100644 index 000000000..ef1ebb713 --- /dev/null +++ b/app/controllers/api/v1/accounts/integrations/base_controller.rb @@ -0,0 +1,9 @@ +class Api::V1::Accounts::Integrations::BaseController < Api::V1::Accounts::BaseController + private + + # Managing an integration hook (create/update/destroy) is admin-only, enforced via HookPolicy. + # Subclasses opt in per action with `before_action :check_authorization, only: [...]`. + def check_authorization + authorize(:hook) + end +end diff --git a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb index 087a9b78d..aec105930 100644 --- a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_hook, except: [:create] before_action :check_authorization @@ -35,10 +35,6 @@ class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Base @hook = Current.account.hooks.find(params[:id]) end - def check_authorization - authorize(:hook) - end - def permitted_params params.require(:hook).permit(:app_id, :inbox_id, :status, settings: {}) end diff --git a/app/controllers/api/v1/accounts/integrations/linear_controller.rb b/app/controllers/api/v1/accounts/integrations/linear_controller.rb index 9ca0c72fd..8ae3109b9 100644 --- a/app/controllers/api/v1/accounts/integrations/linear_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/linear_controller.rb @@ -1,6 +1,7 @@ -class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_conversation, only: [:create_issue, :link_issue, :unlink_issue, :linked_issues] before_action :fetch_hook, only: [:destroy] + before_action :check_authorization, only: [:destroy] def destroy revoke_linear_token diff --git a/app/controllers/api/v1/accounts/integrations/notion_controller.rb b/app/controllers/api/v1/accounts/integrations/notion_controller.rb index ecf6bae6e..29343e4f5 100644 --- a/app/controllers/api/v1/accounts/integrations/notion_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/notion_controller.rb @@ -1,5 +1,6 @@ -class Api::V1::Accounts::Integrations::NotionController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::NotionController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_hook, only: [:destroy] + before_action :check_authorization, only: [:destroy] def destroy @hook.destroy! diff --git a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb index 7fe31889b..c847a85df 100644 --- a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb @@ -1,7 +1,8 @@ -class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::Integrations::BaseController include Shopify::IntegrationHelper before_action :setup_shopify_context, only: [:orders] before_action :fetch_hook, except: [:auth] + before_action :check_authorization, only: [:destroy] before_action :validate_contact, only: [:orders] def auth diff --git a/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb index 5f512b2bd..8d28a2f2a 100644 --- a/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb @@ -13,7 +13,9 @@ RSpec.describe 'Linear Integration API', type: :request do end describe 'DELETE /api/v1/accounts/:account_id/integrations/linear' do - it 'deletes the linear integration' do + let(:admin) { create(:user, account: account, role: :administrator) } + + it 'deletes the linear integration when the user is an administrator' do # Stub the HTTP call to Linear's revoke endpoint allow(HTTParty).to receive(:post).with( 'https://api.linear.app/oauth/revoke', @@ -21,11 +23,19 @@ RSpec.describe 'Linear Integration API', type: :request do ).and_return(instance_double(HTTParty::Response, success?: true)) delete "/api/v1/accounts/#{account.id}/integrations/linear", - headers: agent.create_new_auth_token, + headers: admin.create_new_auth_token, as: :json expect(response).to have_http_status(:ok) expect(account.hooks.count).to eq(0) end + + it 'returns unauthorized for an agent and keeps the integration' do + delete "/api/v1/accounts/#{account.id}/integrations/linear", + headers: agent.create_new_auth_token, + as: :json + expect(response).to have_http_status(:unauthorized) + expect(account.hooks.count).to eq(1) + end end describe 'GET /api/v1/accounts/:account_id/integrations/linear/teams' do diff --git a/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb index ef6c4d367..7f9a032ce 100644 --- a/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb @@ -159,15 +159,17 @@ RSpec.describe 'Shopify Integration API', type: :request do end describe 'DELETE /api/v1/accounts/:account_id/integrations/shopify' do + let(:admin) { create(:user, account: account, role: :administrator) } + before do create(:integrations_hook, :shopify, account: account) end - context 'when it is an authenticated user' do + context 'when it is an administrator' do it 'deletes the shopify integration' do expect do delete "/api/v1/accounts/#{account.id}/integrations/shopify", - headers: agent.create_new_auth_token, + headers: admin.create_new_auth_token, as: :json end.to change { account.hooks.count }.by(-1) @@ -175,6 +177,18 @@ RSpec.describe 'Shopify Integration API', type: :request do end end + context 'when it is an agent' do + it 'returns unauthorized and keeps the integration' do + expect do + delete "/api/v1/accounts/#{account.id}/integrations/shopify", + headers: agent.create_new_auth_token, + as: :json + end.not_to(change { account.hooks.count }) + + expect(response).to have_http_status(:unauthorized) + end + end + context 'when it is an unauthenticated user' do it 'returns unauthorized' do delete "/api/v1/accounts/#{account.id}/integrations/shopify",