fix(security): Enforce admin authorization on custom attribute definitions API (#14392)
Custom attribute definitions can now only be created, edited, or deleted by administrators, matching the existing settings UI restriction. Previously, an agent could call the `custom_attribute_definitions` API directly and modify account configuration that they couldn't reach through the dashboard — a Broken Access Control vulnerability reported externally. Fixes https://linear.app/chatwoot/issue/CW-7038/broken-access-control-on-custom-attribute-definitions-api ## How to test 1. Sign in as an agent. 2. Try to create a custom attribute by calling `POST /api/v1/accounts/<id>/custom_attribute_definitions` directly (the settings page is hidden for agents — use curl with the agent's `api_access_token`). 3. Expect `401 Unauthorized` with body `{"error":"You are not authorized to do this action"}`. Repeat for `PATCH` and `DELETE`. 4. Sign in as an administrator and confirm create/edit/delete still work from Settings → Custom Attributes. 5. As either role, the listing endpoint (`GET .../custom_attribute_definitions`) should still succeed — agents need this to render attributes in conversation and contact panels. Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
class Api::V1::Accounts::CustomAttributeDefinitionsController < Api::V1::Accounts::BaseController
|
||||
before_action :fetch_custom_attributes_definitions, except: [:create]
|
||||
before_action :fetch_custom_attribute_definition, only: [:show, :update, :destroy]
|
||||
before_action :check_authorization
|
||||
DEFAULT_ATTRIBUTE_MODEL = 'conversation_attribute'.freeze
|
||||
|
||||
def index; end
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
class CustomAttributeDefinitionPolicy < ApplicationPolicy
|
||||
def index?
|
||||
@account_user.administrator? || @account_user.agent?
|
||||
end
|
||||
|
||||
def show?
|
||||
@account_user.administrator? || @account_user.agent?
|
||||
end
|
||||
|
||||
def create?
|
||||
@account_user.administrator?
|
||||
end
|
||||
|
||||
def update?
|
||||
@account_user.administrator?
|
||||
end
|
||||
|
||||
def destroy?
|
||||
@account_user.administrator?
|
||||
end
|
||||
end
|
||||
@@ -2,7 +2,8 @@ require 'rails_helper'
|
||||
|
||||
RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
let(:account) { create(:account) }
|
||||
let(:user) { create(:user, account: account) }
|
||||
let(:agent) { create(:user, account: account, role: :agent) }
|
||||
let(:admin) { create(:user, account: account, role: :administrator) }
|
||||
|
||||
describe 'GET /api/v1/accounts/{account.id}/custom_attribute_definitions' do
|
||||
context 'when it is an unauthenticated user' do
|
||||
@@ -19,7 +20,7 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
create(:custom_attribute_definition, attribute_model: 'contact_attribute', account: account)
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/custom_attribute_definitions",
|
||||
headers: user.create_new_auth_token,
|
||||
headers: admin.create_new_auth_token,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
@@ -45,7 +46,7 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
context 'when it is an authenticated user' do
|
||||
it 'shows the custom attribute definition' do
|
||||
get "/api/v1/accounts/#{account.id}/custom_attribute_definitions/#{custom_attribute_definition.id}",
|
||||
headers: user.create_new_auth_token,
|
||||
headers: admin.create_new_auth_token,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
@@ -81,7 +82,7 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
context 'when it is an authenticated user' do
|
||||
it 'creates the filter' do
|
||||
expect do
|
||||
post "/api/v1/accounts/#{account.id}/custom_attribute_definitions", headers: user.create_new_auth_token,
|
||||
post "/api/v1/accounts/#{account.id}/custom_attribute_definitions", headers: admin.create_new_auth_token,
|
||||
params: payload
|
||||
end.to change(CustomAttributeDefinition, :count).by(1)
|
||||
|
||||
@@ -90,6 +91,18 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
expect(json_response['attribute_key']).to eq 'developer_id'
|
||||
end
|
||||
|
||||
context 'when it is an agent' do
|
||||
it 'returns forbidden and does not create the custom attribute' do
|
||||
expect do
|
||||
post "/api/v1/accounts/#{account.id}/custom_attribute_definitions",
|
||||
headers: agent.create_new_auth_token,
|
||||
params: payload
|
||||
end.not_to change(CustomAttributeDefinition, :count)
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when creating with a conflicting attribute_key' do
|
||||
let(:standard_key) { CustomAttributeDefinition::STANDARD_ATTRIBUTES[:conversation].first }
|
||||
let(:conflicting_payload) do
|
||||
@@ -105,7 +118,7 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
|
||||
it 'returns error for conflicting key' do
|
||||
post "/api/v1/accounts/#{account.id}/custom_attribute_definitions",
|
||||
headers: user.create_new_auth_token,
|
||||
headers: admin.create_new_auth_token,
|
||||
params: conflicting_payload
|
||||
|
||||
expect(response).to have_http_status(:unprocessable_entity)
|
||||
@@ -132,7 +145,7 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
context 'when it is an authenticated user' do
|
||||
it 'updates the custom attribute definition' do
|
||||
patch "/api/v1/accounts/#{account.id}/custom_attribute_definitions/#{custom_attribute_definition.id}",
|
||||
headers: user.create_new_auth_token,
|
||||
headers: admin.create_new_auth_token,
|
||||
params: payload,
|
||||
as: :json
|
||||
expect(response).to have_http_status(:success)
|
||||
@@ -141,6 +154,19 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
expect(custom_attribute_definition.reload.attribute_model).to eq('conversation_attribute')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is an agent' do
|
||||
it 'returns forbidden and does not update the custom attribute' do
|
||||
original_name = custom_attribute_definition.attribute_display_name
|
||||
patch "/api/v1/accounts/#{account.id}/custom_attribute_definitions/#{custom_attribute_definition.id}",
|
||||
headers: agent.create_new_auth_token,
|
||||
params: payload,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(custom_attribute_definition.reload.attribute_display_name).to eq(original_name)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'DELETE /api/v1/accounts/{account.id}/custom_attribute_definitions/:id' do
|
||||
@@ -156,11 +182,22 @@ RSpec.describe 'Custom Attribute Definitions API', type: :request do
|
||||
context 'when it is an authenticated admin user' do
|
||||
it 'deletes custom attribute' do
|
||||
delete "/api/v1/accounts/#{account.id}/custom_attribute_definitions/#{custom_attribute_definition.id}",
|
||||
headers: user.create_new_auth_token,
|
||||
headers: admin.create_new_auth_token,
|
||||
as: :json
|
||||
expect(response).to have_http_status(:no_content)
|
||||
expect(account.custom_attribute_definitions.count).to be 0
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is an agent' do
|
||||
it 'returns forbidden and does not delete the custom attribute' do
|
||||
delete "/api/v1/accounts/#{account.id}/custom_attribute_definitions/#{custom_attribute_definition.id}",
|
||||
headers: agent.create_new_auth_token,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(account.custom_attribute_definitions.count).to be 1
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user