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>
111 lines
3.1 KiB
Ruby
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')
|