From 8aee5181492b4fb83071b283a9927d8a5f7e69ec Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:53:25 +0530 Subject: [PATCH] fix(integrations): restrict Linear/Notion/Shopify hook deletion to admins (#15126) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-admin agents could delete an account's Linear, Notion, or Shopify integration through the dedicated integration endpoints, which — unlike the generic hooks endpoint — never checked the caller's role. This restores the intended admin-only boundary for removing an integration. ## Closes - https://linear.app/chatwoot/issue/CW-7383 - https://linear.app/chatwoot/issue/CW-7384 - https://linear.app/chatwoot/issue/CW-7189 ## How to reproduce As a non-admin **agent**, `DELETE /api/v1/accounts/:id/integrations/{linear,notion,shopify}` returned `200` and removed the account-wide integration. After this change it returns `401` and the integration is preserved; administrators can still remove it. ## What changed - Route integration-hook deletion through `HookPolicy` (admin-only) via a shared `Integrations::BaseController`, matching the generic hooks controller. Co-authored-by: Vishnu Narayanan --- .../accounts/integrations/base_controller.rb | 9 +++++++++ .../accounts/integrations/hooks_controller.rb | 6 +----- .../accounts/integrations/linear_controller.rb | 3 ++- .../accounts/integrations/notion_controller.rb | 3 ++- .../integrations/shopify_controller.rb | 3 ++- .../integrations/linear_controller_spec.rb | 14 ++++++++++++-- .../integrations/shopify_controller_spec.rb | 18 ++++++++++++++++-- 7 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 app/controllers/api/v1/accounts/integrations/base_controller.rb 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",