Files
chatwoot/spec/controllers/api/v1/accounts/portals_controller_spec.rb
b0aa844a32 fix(portals): handle integer blob_id in process_attached_logo without 500 (#14274)
Updating portal settings (name, header text, page title, homepage link)
on a portal that already has a logo attached returns 500. The error is
\`NoMethodError: undefined method 'valid_encoding?' for an instance of
Integer\`. The fix is a two-character change in
\`process_attached_logo\`.

Closes #13300

## Root cause

\`ActiveStorage::Blob.find_signed\` expects a signed ID string (e.g.
\`"eyJfcmFpbH..."\`). Internally it calls \`valid_encoding?\` on the
argument to validate the signature payload — a method that exists on
\`String\` but not \`Integer\`.

When a portal already has a logo, the frontend includes the blob's raw
database integer ID (e.g. \`blob_id: 170\`) in the update request
payload. The controller passes this integer directly to \`find_signed\`,
which immediately raises \`NoMethodError\` before any database query is
made.

\`\`\`ruby
# before, crashes when blob_id is an Integer
blob_id = params[:blob_id]
blob = ActiveStorage::Blob.find_signed(blob_id)  # NoMethodError here
@portal.logo.attach(blob)
\`\`\`

## What changed

\`\`\`ruby
# after, safe for any input type
blob = ActiveStorage::Blob.find_signed(params[:blob_id].to_s)
@portal.logo.attach(blob) if blob
\`\`\`

\`.to_s\` on an Integer produces a plain decimal string (\`"170"\`),
which is not a valid signed ID. \`find_signed\` returns \`nil\` for any
invalid signature rather than raising, so the nil guard prevents a
broken \`attach\` call. The existing logo remains attached and the
settings update succeeds.

## Trade-offs considered

| Option | Decision |
|---|---|
| \`find(blob_id)\` when input is an Integer | Bypasses signature
verification — any authenticated user knowing a blob ID could attach
arbitrary files to a portal. Security risk. Rejected. |
| Raise a 422 for non-string blob_id | Overly strict — the frontend
sending an integer is pre-existing behaviour this PR shouldn't break. |
| Silently no-op for invalid blob_id (chosen) | Correct product
behaviour: if no valid signed upload is provided, leave the logo
unchanged. The settings update still succeeds. |

## Known limitation

The correct long-term fix is also on the frontend: it should only send
\`blob_id\` when attaching a **new** upload (using the signed ID from
the direct-upload flow), not when re-submitting the existing logo's raw
database integer ID. This PR makes the server robust against the current
frontend behaviour without requiring a coordinated frontend change.

## How to reproduce

1. Create a Help Center portal and upload a logo
2. Update any text field via \`PUT /api/v1/accounts/:id/portals/:slug\`
while including \`blob_id: <integer>\` in the payload
3. Observe 500 with \`NoMethodError: undefined method 'valid_encoding?'
for an instance of Integer\`

After this fix, the request returns 200, settings are updated, and the
existing logo is preserved.

Co-authored-by: Ramalau Debeila <rdebeila@datacentrix.co.za>
2026-04-28 01:14:51 +05:30

342 lines
13 KiB
Ruby

require 'rails_helper'
RSpec.describe 'Api::V1::Accounts::Portals', type: :request do
let(:account) { create(:account) }
let(:agent) { create(:user, account: account, role: :agent) }
let(:admin) { create(:user, account: account, role: :administrator) }
let(:agent_1) { create(:user, account: account, role: :agent) }
let(:agent_2) { create(:user, account: account, role: :agent) }
let!(:portal) { create(:portal, slug: 'portal-1', name: 'test_portal', account_id: account.id) }
describe 'GET /api/v1/accounts/{account.id}/portals' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
get "/api/v1/accounts/#{account.id}/portals"
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
it 'get all portals' do
portal2 = create(:portal, name: 'test_portal_2', account_id: account.id, slug: 'portal-2')
expect(portal2.id).not_to be_nil
get "/api/v1/accounts/#{account.id}/portals",
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['payload'].length).to be 2
expect(json_response['payload'][0]['id']).to be portal.id
end
end
end
describe 'GET /api/v1/accounts/{account.id}/portals/{portal.slug}' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
get "/api/v1/accounts/#{account.id}/portals"
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
it 'get one portals' do
get "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['name']).to eq portal.name
expect(json_response['meta']['all_articles_count']).to eq 0
end
it 'returns portal articles metadata' do
portal.update(config: { allowed_locales: %w[en es], default_locale: 'en' })
en_cat = create(:category, locale: :en, portal_id: portal.id, slug: 'en-cat')
es_cat = create(:category, locale: :es, portal_id: portal.id, slug: 'es-cat')
create(:article, category_id: en_cat.id, portal_id: portal.id, author_id: agent.id)
create(:article, category_id: en_cat.id, portal_id: portal.id, author_id: admin.id)
create(:article, category_id: es_cat.id, portal_id: portal.id, author_id: agent.id)
get "/api/v1/accounts/#{account.id}/portals/#{portal.slug}?locale=en",
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['name']).to eq portal.name
expect(json_response['meta']['all_articles_count']).to eq 2
expect(json_response['meta']['mine_articles_count']).to eq 1
end
end
end
describe 'POST /api/v1/accounts/{account.id}/portals' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
post "/api/v1/accounts/#{account.id}/portals",
params: {},
headers: agent.create_new_auth_token
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
it 'creates portal' do
portal_params = {
portal: {
name: 'test_portal',
slug: 'test_kbase',
custom_domain: 'https://support.chatwoot.dev'
}
}
post "/api/v1/accounts/#{account.id}/portals",
params: portal_params,
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['name']).to eql('test_portal')
expect(json_response['custom_domain']).to eql('support.chatwoot.dev')
end
end
end
describe 'PUT /api/v1/accounts/{account.id}/portals/{portal.slug}' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}", params: {}
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
it 'updates portal' do
portal_params = {
portal: {
name: 'updated_test_portal',
config: { 'allowed_locales' => %w[en es], 'draft_locales' => ['es'], 'default_locale' => 'en' }
}
}
expect(portal.name).to eql('test_portal')
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
params: portal_params,
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['name']).to eql(portal_params[:portal][:name])
expect(json_response['config']).to eql(
{
'allowed_locales' => [
{ 'articles_count' => 0, 'categories_count' => 0, 'code' => 'en', 'draft' => false },
{ 'articles_count' => 0, 'categories_count' => 0, 'code' => 'es', 'draft' => true }
]
}
)
end
it 'preserves drafted locales when draft_locales is omitted' do
portal.update!(config: { allowed_locales: %w[en es fr], draft_locales: ['es'], default_locale: 'en' })
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
params: {
portal: {
config: { allowed_locales: %w[en es fr], default_locale: 'en' }
}
},
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
portal.reload
expect(portal.draft_locale_codes).to eq(['es'])
expect(response.parsed_body.dig('config', 'allowed_locales')).to include(
a_hash_including('code' => 'es', 'draft' => true)
)
end
it 'archive portal' do
portal_params = {
portal: {
archived: true
}
}
expect(portal.archived).to be_falsy
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
params: portal_params,
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
json_response = response.parsed_body
expect(json_response['archived']).to eql(portal_params[:portal][:archived])
portal.reload
expect(portal.archived).to be_truthy
end
it 'does not raise when blob_id is an integer (existing logo re-sent by frontend)' do
portal.logo.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
params: { portal: { name: 'updated_name' }, blob_id: portal.logo.blob.id },
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
expect(response.parsed_body['name']).to eq('updated_name')
expect(portal.reload.logo).to be_attached
end
it 'clears associated web widget when inbox selection is blank' do
web_widget_inbox = create(:inbox, account: account)
portal.update!(channel_web_widget: web_widget_inbox.channel)
expect(portal.channel_web_widget_id).to eq(web_widget_inbox.channel.id)
put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
params: {
portal: { name: portal.name },
inbox_id: ''
},
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
portal.reload
expect(portal.channel_web_widget_id).to be_nil
expect(response.parsed_body['inbox']).to be_nil
end
end
end
describe 'DELETE /api/v1/accounts/{account.id}/portals/{portal.slug}' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
delete "/api/v1/accounts/#{account.id}/portals/#{portal.slug}", params: {}
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
it 'deletes portal' do
delete "/api/v1/accounts/#{account.id}/portals/#{portal.slug}",
headers: admin.create_new_auth_token
expect(response).to have_http_status(:success)
deleted_portal = Portal.find_by(id: portal.slug)
expect(deleted_portal).to be_nil
end
end
end
# Portal members endpoint removed
describe 'DELETE /api/v1/accounts/{account.id}/portals/{portal.slug}/logo' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
delete "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/logo"
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated user' do
before do
portal.logo.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
end
it 'throw error if agent' do
delete "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/logo",
headers: agent.create_new_auth_token,
as: :json
expect(response).to have_http_status(:unauthorized)
end
it 'delete portal logo if admin' do
delete "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/logo",
headers: admin.create_new_auth_token,
as: :json
expect { portal.logo.attachment.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect(response).to have_http_status(:success)
end
end
end
describe 'POST /api/v1/accounts/{account.id}/portals/{portal.slug}/send_instructions' do
let(:portal_with_domain) { create(:portal, slug: 'portal-with-domain', account_id: account.id, custom_domain: 'docs.example.com') }
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
post "/api/v1/accounts/#{account.id}/portals/#{portal_with_domain.slug}/send_instructions",
params: { email: 'dev@example.com' }
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated agent' do
it 'returns unauthorized' do
post "/api/v1/accounts/#{account.id}/portals/#{portal_with_domain.slug}/send_instructions",
headers: agent.create_new_auth_token,
params: { email: 'dev@example.com' },
as: :json
expect(response).to have_http_status(:unauthorized)
end
end
context 'when it is an authenticated admin' do
it 'returns error when email is missing' do
post "/api/v1/accounts/#{account.id}/portals/#{portal_with_domain.slug}/send_instructions",
headers: admin.create_new_auth_token,
params: {},
as: :json
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Email is required')
end
it 'returns error when email is invalid' do
post "/api/v1/accounts/#{account.id}/portals/#{portal_with_domain.slug}/send_instructions",
headers: admin.create_new_auth_token,
params: { email: 'invalid-email' },
as: :json
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid email format')
end
it 'returns error when custom domain is not configured' do
post "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/send_instructions",
headers: admin.create_new_auth_token,
params: { email: 'dev@example.com' },
as: :json
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Custom domain is not configured')
end
it 'sends instructions successfully' do
mailer_double = instance_double(ActionMailer::MessageDelivery)
allow(PortalInstructionsMailer).to receive(:send_cname_instructions).and_return(mailer_double)
allow(mailer_double).to receive(:deliver_later)
post "/api/v1/accounts/#{account.id}/portals/#{portal_with_domain.slug}/send_instructions",
headers: admin.create_new_auth_token,
params: { email: 'dev@example.com' },
as: :json
expect(response).to have_http_status(:success)
expect(response.parsed_body['message']).to eq('Instructions sent successfully')
expect(PortalInstructionsMailer).to have_received(:send_cname_instructions)
.with(portal: portal_with_domain, recipient_email: 'dev@example.com')
end
end
end
end