From 651db39765eed0b7a8ec38af01f0db2539e18984 Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Wed, 29 Jul 2026 17:28:45 +0530 Subject: [PATCH] fix: harden article author updates (#15229) ## Description Article edits now ignore an `author_id` that does not belong to the current account, retain the existing author, and still apply other valid article changes. Article creation continues to reject cross-account authors with a generic validation error and without creating a record. The authenticated article serializers still omit authors without a current-account membership so existing forged or stale records cannot expose agent profile fields. The guard now checks `current_account_user` directly to make that intent explicit. ## Closes Follow-up to [CW-7665](https://linear.app/chatwoot/issue/CW-7665) and [#15191](https://github.com/chatwoot/chatwoot/pull/15191). ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Creating an article with a cross-account author returns `422` and does not create an article. - Updating an article with a cross-account author retains the previous author while applying other valid attributes. - Articles whose previous author is no longer an account member can still be edited without exposing that author's agent profile. - Existing OSS and Enterprise article request coverage passes locally. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules Co-authored-by: Vishnu Narayanan --- .../api/v1/accounts/articles_controller.rb | 17 ++++++++++++++++- .../v1/accounts/articles/_article.json.jbuilder | 2 +- .../articles/_associated_article.json.jbuilder | 2 +- .../api/v1/accounts/articles_controller_spec.rb | 17 ++++++++++------- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/v1/accounts/articles_controller.rb b/app/controllers/api/v1/accounts/articles_controller.rb index f81775bb7..e05871a49 100644 --- a/app/controllers/api/v1/accounts/articles_controller.rb +++ b/app/controllers/api/v1/accounts/articles_controller.rb @@ -4,6 +4,7 @@ class Api::V1::Accounts::ArticlesController < Api::V1::Accounts::BaseController before_action :fetch_article, except: [:index, :create, :reorder] before_action :set_current_page, only: [:index] before_action :validate_author, only: [:create] + before_action :discard_invalid_author, only: [:update] def index @portal_articles = @portal.articles @@ -67,11 +68,25 @@ class Api::V1::Accounts::ArticlesController < Api::V1::Accounts::BaseController def validate_author author_id = params.dig(:article, :author_id) return if author_id.blank? - return if author_id.to_s.match?(/\A\d+\z/) && Current.account.users.exists?(id: author_id.to_i) + return if author_belongs_to_account?(author_id) render json: { error: 'Invalid author ID' }, status: :unprocessable_entity end + def discard_invalid_author + submitted_article_params = params[:article] + return unless submitted_article_params&.key?(:author_id) + + author_id = submitted_article_params[:author_id] + return if author_belongs_to_account?(author_id) + + submitted_article_params.delete(:author_id) + end + + def author_belongs_to_account?(author_id) + author_id.to_s.match?(/\A\d+\z/) && Current.account.users.exists?(id: author_id.to_i) + end + def portal @portal ||= Current.account.portals.find_by!(slug: params[:portal_id]) end diff --git a/app/views/api/v1/accounts/articles/_article.json.jbuilder b/app/views/api/v1/accounts/articles/_article.json.jbuilder index f6874adc0..c5f1245cd 100644 --- a/app/views/api/v1/accounts/articles/_article.json.jbuilder +++ b/app/views/api/v1/accounts/articles/_article.json.jbuilder @@ -22,7 +22,7 @@ end json.views article.views -if article.author&.account +if article.author&.current_account_user json.author do json.partial! 'api/v1/models/agent', formats: [:json], resource: article.author end diff --git a/app/views/api/v1/accounts/articles/_associated_article.json.jbuilder b/app/views/api/v1/accounts/articles/_associated_article.json.jbuilder index a0ea47ba5..9a215d587 100644 --- a/app/views/api/v1/accounts/articles/_associated_article.json.jbuilder +++ b/app/views/api/v1/accounts/articles/_associated_article.json.jbuilder @@ -14,7 +14,7 @@ end json.views article.views -if article.author&.account +if article.author&.current_account_user json.author do json.partial! 'api/v1/models/agent', formats: [:json], resource: article.author end diff --git a/spec/controllers/api/v1/accounts/articles_controller_spec.rb b/spec/controllers/api/v1/accounts/articles_controller_spec.rb index d65c5a9e2..f4e03f776 100644 --- a/spec/controllers/api/v1/accounts/articles_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/articles_controller_spec.rb @@ -52,14 +52,15 @@ RSpec.describe 'Api::V1::Accounts::Articles', type: :request do author_id: foreign_user.id } } - post "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles", - params: article_params, - headers: admin.create_new_auth_token + expect do + post "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles", + params: article_params, + headers: admin.create_new_auth_token + end.not_to(change { portal.articles.count }) expect(response).to have_http_status(:unprocessable_entity) expect(response.parsed_body).to eq('error' => 'Invalid author ID') expect(response.body).not_to include(foreign_user.email) - expect(portal.articles.where(author_id: foreign_user.id)).to be_empty end it 'rejects a malformed author_id without raising' do @@ -205,16 +206,18 @@ RSpec.describe 'Api::V1::Accounts::Articles', type: :request do expect(json_response['payload']['position']).to eql(article_params[:article][:position]) end - it 'does not expose a cross-account author set through update' do + it 'ignores a cross-account author while updating other attributes' do foreign_user = create(:user, account: create(:account), role: :agent) put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/#{article.id}", - params: { article: { author_id: foreign_user.id } }, + params: { article: { title: 'Updated title', author_id: foreign_user.id } }, headers: admin.create_new_auth_token expect(response).to have_http_status(:success) + expect(response.parsed_body.dig('payload', 'title')).to eq('Updated title') + expect(response.parsed_body.dig('payload', 'author', 'id')).to eq(agent.id) expect(response.body).not_to include(foreign_user.email) - expect(response.parsed_body['payload']).not_to have_key('author') + expect(article.reload.author_id).to eq(agent.id) end it 'allows editing an article whose author is no longer an account member' do