diff --git a/app/controllers/api/v1/accounts/articles_controller.rb b/app/controllers/api/v1/accounts/articles_controller.rb index 439ba3e31..f81775bb7 100644 --- a/app/controllers/api/v1/accounts/articles_controller.rb +++ b/app/controllers/api/v1/accounts/articles_controller.rb @@ -3,6 +3,7 @@ class Api::V1::Accounts::ArticlesController < Api::V1::Accounts::BaseController before_action :check_authorization before_action :fetch_article, except: [:index, :create, :reorder] before_action :set_current_page, only: [:index] + before_action :validate_author, only: [:create] def index @portal_articles = @portal.articles @@ -63,6 +64,14 @@ class Api::V1::Accounts::ArticlesController < Api::V1::Accounts::BaseController @article = @portal.articles.find(params[:id]) end + 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) + + render json: { error: 'Invalid author ID' }, status: :unprocessable_entity + 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 426b24afe..f6874adc0 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.present? +if article.author&.account 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 342018857..a0ea47ba5 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.present? +if article.author&.account 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 6651a5b20..d65c5a9e2 100644 --- a/spec/controllers/api/v1/accounts/articles_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/articles_controller_spec.rb @@ -40,6 +40,40 @@ RSpec.describe 'Api::V1::Accounts::Articles', type: :request do expect(json_response['payload']['position']).to be(3) end + it 'rejects a cross-account author without exposing their details' do + foreign_user = create(:user, account: create(:account), role: :agent) + article_params = { + article: { + category_id: category.id, + title: 'MyTitle', + slug: 'my-title', + content: 'This is my content.', + status: :published, + 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(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 + [%w[3 4], ' 3 ', '3abc'].each do |bad_author_id| + post "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles", + params: { article: { title: 'MyTitle', slug: 'my-title', content: 'This is my content.', author_id: bad_author_id } }, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body).to eq('error' => 'Invalid author ID') + end + end + it 'creates article even if category is not provided' do article_params = { article: { @@ -171,6 +205,30 @@ 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 + 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 } }, + headers: admin.create_new_auth_token + + expect(response).to have_http_status(:success) + expect(response.body).not_to include(foreign_user.email) + expect(response.parsed_body['payload']).not_to have_key('author') + end + + it 'allows editing an article whose author is no longer an account member' do + foreign_user = create(:user, account: create(:account), role: :agent) + article.update!(author_id: foreign_user.id) + + put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/#{article.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(article.reload.title).to eq('Updated title') + end + it 'stages draft-only fields without bumping updated_at' do expect do put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/#{article.id}", @@ -329,6 +387,18 @@ RSpec.describe 'Api::V1::Accounts::Articles', type: :request do expect(json_response['payload']['id']).to eq(article2.id) end + it 'does not expose an author who is not a member of the account' do + foreign_user = create(:user, account: create(:account), role: :agent) + article.update!(author_id: foreign_user.id) + + get "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/#{article.id}", + headers: admin.create_new_auth_token + + expect(response).to have_http_status(:success) + expect(response.parsed_body['payload']).not_to have_key('author') + expect(response.body).not_to include(foreign_user.email) + end + it 'get associated articles' do root_article = create(:article, category: category, portal: portal, account_id: account.id, author_id: agent.id, associated_article_id: nil) child_article_1 = create(:article, slug: 'child-1', category: category, portal: portal, account_id: account.id, author_id: agent.id,