Files
chatwoot/app/controllers/api/v1/accounts/portals_controller.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

111 lines
3.1 KiB
Ruby

class Api::V1::Accounts::PortalsController < Api::V1::Accounts::BaseController
include ::FileTypeHelper
before_action :fetch_portal, except: [:index, :create]
before_action :check_authorization
before_action :set_current_page, only: [:index]
def index
@portals = Current.account.portals
end
def show
@all_articles = @portal.articles
@articles = @all_articles.search(locale: params[:locale])
end
def create
@portal = Current.account.portals.build(portal_params.merge(live_chat_widget_params))
@portal.custom_domain = parsed_custom_domain
@portal.save!
process_attached_logo
end
def update
ActiveRecord::Base.transaction do
@portal.update!(portal_params.merge(live_chat_widget_params)) if params[:portal].present?
# @portal.custom_domain = parsed_custom_domain
process_attached_logo if params[:blob_id].present?
rescue ActiveRecord::RecordInvalid => e
render_record_invalid(e)
end
end
def destroy
@portal.destroy!
head :ok
end
def archive
@portal.update(archive: true)
head :ok
end
def logo
@portal.logo.purge if @portal.logo.attached?
head :ok
end
def send_instructions
email = permitted_params[:email]
return render_could_not_create_error(I18n.t('portals.send_instructions.email_required')) if email.blank?
return render_could_not_create_error(I18n.t('portals.send_instructions.invalid_email_format')) unless valid_email?(email)
return render_could_not_create_error(I18n.t('portals.send_instructions.custom_domain_not_configured')) if @portal.custom_domain.blank?
PortalInstructionsMailer.send_cname_instructions(
portal: @portal,
recipient_email: email
).deliver_later
render json: { message: I18n.t('portals.send_instructions.instructions_sent_successfully') }, status: :ok
end
def process_attached_logo
blob = ActiveStorage::Blob.find_signed(params[:blob_id].to_s)
@portal.logo.attach(blob) if blob
end
private
def fetch_portal
@portal = Current.account.portals.find_by(slug: permitted_params[:id])
end
def permitted_params
params.permit(:id, :email)
end
def portal_params
params.require(:portal).permit(
:id, :color, :custom_domain, :header_text, :homepage_link,
:name, :page_title, :slug, :archived, { config: [:default_locale, { allowed_locales: [] }, { draft_locales: [] }] }
)
end
def live_chat_widget_params
permitted_params = params.permit(:inbox_id)
return {} unless permitted_params.key?(:inbox_id)
return { channel_web_widget_id: nil } if permitted_params[:inbox_id].blank?
inbox = Inbox.find(permitted_params[:inbox_id])
return {} unless inbox.web_widget?
{ channel_web_widget_id: inbox.channel.id }
end
def set_current_page
@current_page = params[:page] || 1
end
def parsed_custom_domain
domain = URI.parse(@portal.custom_domain)
domain.is_a?(URI::HTTP) ? domain.host : @portal.custom_domain
end
def valid_email?(email)
ValidEmail2::Address.new(email).valid?
end
end
Api::V1::Accounts::PortalsController.prepend_mod_with('Api::V1::Accounts::PortalsController')