From 242ecdf2c52f85204a60a6c9d82abdd6ba76c081 Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Wed, 29 Jul 2026 23:02:03 +0300 Subject: [PATCH] fix(whatsapp): require a write role on the WhatsApp write routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five write handlers under /api/whatsapp resolved the caller's account_id straight off their profile row, which only proves account membership — never a role. Every other write surface in the app (account/, ai/, automations/, flows/, quick-replies/) already goes through requireRole(); this family was missed. RLS did not backstop it, because each of these routes calls Meta *before* it persists anything: - broadcast: makes no database write at all, so there was no policy involved on any path - send / react: the Meta call precedes the insert, so the outbound message or reaction reached the customer and only the local mirror was refused - templates/submit, templates/sync: push to Meta before upserting the local catalog Gate each handler at the role its data class already implies, matching lib/auth/roles and the migration 017 policies: - send, react, broadcast -> 'agent' (canSendMessages) - templates/submit, sync -> 'admin' (canEditSettings, and the message_templates_insert/update policies) Also switches these handlers to toErrorResponse so an authorization failure returns 401/403 instead of being folded into a generic 500. The two template routes keep their existing message-surfacing 500 for non-auth errors, so a Meta failure still reports its own reason. Adds role-enforcement coverage to the send route's test: a viewer is refused with 403 and sendTemplateMessage is never called, while an agent passes through. Co-Authored-By: Claude Opus 5 --- src/app/api/whatsapp/broadcast/route.ts | 51 ++++++----------- src/app/api/whatsapp/react/route.ts | 45 ++++----------- src/app/api/whatsapp/send/route.test.ts | 57 ++++++++++++++++++- src/app/api/whatsapp/send/route.ts | 52 ++++++----------- .../api/whatsapp/templates/submit/route.ts | 51 ++++++++--------- src/app/api/whatsapp/templates/sync/route.ts | 47 +++++++-------- 6 files changed, 147 insertions(+), 156 deletions(-) diff --git a/src/app/api/whatsapp/broadcast/route.ts b/src/app/api/whatsapp/broadcast/route.ts index e4205a3d5a..0033d6a1ed 100644 --- a/src/app/api/whatsapp/broadcast/route.ts +++ b/src/app/api/whatsapp/broadcast/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server' -import { createClient } from '@/lib/supabase/server' +import { requireRole, toErrorResponse } from '@/lib/auth/account' import { sendTemplateMessage } from '@/lib/whatsapp/meta-api' import { decrypt } from '@/lib/whatsapp/encryption' import type { SendTimeParams } from '@/lib/whatsapp/template-send-builder' @@ -60,42 +60,28 @@ interface NewRecipient { export async function POST(request: Request) { try { - const supabase = await createClient() - - const { - data: { user }, - error: authError, - } = await supabase.auth.getUser() - - if (authError || !user) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) - } + // Requires the 'agent' role — `canSendMessages` in lib/auth/roles is + // explicit that running broadcasts is a write operation and that + // viewers are read-only. + // + // This endpoint writes NOTHING to the database: it reads the config + // and template, then calls Meta directly. So unlike the rest of the + // app there was no RLS policy backstopping a missing role check — + // resolving `account_id` straight off the profile (which only needs + // 'viewer') was the ONLY gate, and it let a viewer blast a template + // to arbitrary phone numbers from the account's WhatsApp number. + // Nothing about that is recoverable after the fact, so the check has + // to happen here. + const { supabase, accountId, userId } = await requireRole('agent') // Per-user broadcast budget. Note: this limits how often a user // can *start* a campaign, not how many messages go out inside // one — the fan-out loop below runs without additional gating. - const limit = checkRateLimit(`broadcast:${user.id}`, RATE_LIMITS.broadcast) + const limit = checkRateLimit(`broadcast:${userId}`, RATE_LIMITS.broadcast) if (!limit.success) { return rateLimitResponse(limit) } - // Resolve the caller's account_id. whatsapp_config + templates - // + broadcasts are all account-scoped post-multi-user, so the - // old `.eq('user_id', user.id)` filters miss every row created - // by a teammate. - const { data: profile } = await supabase - .from('profiles') - .select('account_id') - .eq('user_id', user.id) - .maybeSingle() - const accountId = profile?.account_id as string | undefined - if (!accountId) { - return NextResponse.json( - { error: 'Your profile is not linked to an account.' }, - { status: 403 }, - ) - } - const body = await request.json() const { recipients: newRecipients, @@ -254,10 +240,9 @@ export async function POST(request: Request) { results, }) } catch (error) { + // requireRole throws Unauthorized/Forbidden; toErrorResponse maps + // those to 401/403 and collapses anything else to a generic 500. console.error('Error in WhatsApp broadcast POST:', error) - return NextResponse.json( - { error: 'Failed to process broadcast' }, - { status: 500 } - ) + return toErrorResponse(error) } } diff --git a/src/app/api/whatsapp/react/route.ts b/src/app/api/whatsapp/react/route.ts index a912c3f8bd..0ba5b4b559 100644 --- a/src/app/api/whatsapp/react/route.ts +++ b/src/app/api/whatsapp/react/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { createClient } from '@/lib/supabase/server'; +import { requireRole, toErrorResponse } from '@/lib/auth/account'; import { sendReactionMessage } from '@/lib/whatsapp/meta-api'; import { decrypt } from '@/lib/whatsapp/encryption'; import { sanitizePhoneForMeta } from '@/lib/whatsapp/phone-utils'; @@ -20,37 +20,17 @@ import { */ export async function POST(request: Request) { try { - const supabase = await createClient(); + // Reacting is a write operation (`canSendMessages`), and it pushes the + // reaction to Meta before mirroring it locally — so, as on /send, a + // missing role check let a read-only viewer put a visible reaction on + // the customer's message even though RLS blocked the local mirror. + const { supabase, accountId, userId } = await requireRole('agent'); - const { - data: { user }, - error: authError, - } = await supabase.auth.getUser(); - - if (authError || !user) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - - const limit = checkRateLimit(`react:${user.id}`, RATE_LIMITS.react); + const limit = checkRateLimit(`react:${userId}`, RATE_LIMITS.react); if (!limit.success) { return rateLimitResponse(limit); } - // Resolve the caller's account_id so conversation + whatsapp_config - // lookups work for teammates who didn't author the rows directly. - const { data: profile } = await supabase - .from('profiles') - .select('account_id') - .eq('user_id', user.id) - .maybeSingle(); - const accountId = profile?.account_id as string | undefined; - if (!accountId) { - return NextResponse.json( - { error: 'Your profile is not linked to an account.' }, - { status: 403 }, - ); - } - const body = await request.json(); const { message_id, emoji } = body as { message_id?: string; @@ -150,7 +130,7 @@ export async function POST(request: Request) { .delete() .eq('message_id', targetMessage.id) .eq('actor_type', 'agent') - .eq('actor_id', user.id); + .eq('actor_id', userId); if (delError) { console.error('[whatsapp/react] DB delete failed:', delError.message); @@ -167,7 +147,7 @@ export async function POST(request: Request) { message_id: targetMessage.id, conversation_id: targetMessage.conversation_id, actor_type: 'agent', - actor_id: user.id, + actor_id: userId, emoji, }, { onConflict: 'message_id,actor_type,actor_id' }, @@ -184,10 +164,9 @@ export async function POST(request: Request) { return NextResponse.json({ success: true }); } catch (error) { + // requireRole throws Unauthorized/Forbidden; toErrorResponse maps + // those to 401/403 and collapses anything else to a generic 500. console.error('Error in WhatsApp react POST:', error); - return NextResponse.json( - { error: 'Failed to react to message' }, - { status: 500 }, - ); + return toErrorResponse(error); } } diff --git a/src/app/api/whatsapp/send/route.test.ts b/src/app/api/whatsapp/send/route.test.ts index aa547175a8..4f729c881e 100644 --- a/src/app/api/whatsapp/send/route.test.ts +++ b/src/app/api/whatsapp/send/route.test.ts @@ -14,6 +14,9 @@ const messageInserts: Array> = [] // Toggles for the per-test scenario. let existingConversation: Record | null = null let contactRow: Record | null = null +// The caller's role, as `requireRole` reads it off the profile. Sending +// requires 'agent'; 'viewer' must be refused before anything reaches Meta. +let callerRole: string = 'admin' // A conversation created during the request becomes retrievable by id — // the shared send core re-loads the conversation (with its contact) from // just the id, so the mock must model insert-then-select-by-id. @@ -35,7 +38,12 @@ function makeSupabaseMock() { const selectResult = () => { switch (table) { case 'profiles': - return { data: { account_id: 'acct-1' }, error: null } + return { + data: { account_id: 'acct-1', account_role: callerRole }, + error: null, + } + case 'accounts': + return { data: { id: 'acct-1', name: 'Acme' }, error: null } case 'contacts': return { data: contactRow, error: null } case 'conversations': @@ -179,6 +187,7 @@ describe('POST /api/whatsapp/send — contact_id template path', () => { existingConversation = null createdConversation = null contactRow = CONTACT + callerRole = 'admin' supabaseMock = makeSupabaseMock() sendTemplateMessage.mockClear() }) @@ -259,3 +268,49 @@ describe('POST /api/whatsapp/send — contact_id template path', () => { expect(res.status).toBe(400) }) }) + +describe('POST /api/whatsapp/send — role enforcement', () => { + beforeEach(() => { + conversationInserts.length = 0 + messageInserts.length = 0 + existingConversation = { + id: 'conv-existing', + account_id: 'acct-1', + contact_id: 'contact-1', + contact: CONTACT, + } + createdConversation = null + contactRow = CONTACT + callerRole = 'admin' + supabaseMock = makeSupabaseMock() + sendTemplateMessage.mockClear() + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('refuses a viewer with 403 and never reaches Meta', async () => { + // A viewer is read-only (`canSendMessages`). The route used to resolve + // account_id straight off the profile with no role check: RLS blocked + // the message INSERT, but the send core calls Meta first, so the + // customer still received a real WhatsApp message that RLS could not + // un-send. The gate has to come before any outbound call. + callerRole = 'viewer' + + const res = await postContactTemplate() + + expect(res.status).toBe(403) + expect(sendTemplateMessage).not.toHaveBeenCalled() + expect(messageInserts).toHaveLength(0) + }) + + it('allows an agent through', async () => { + callerRole = 'agent' + + const res = await postContactTemplate() + + expect(res.status).toBe(200) + expect(sendTemplateMessage).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/app/api/whatsapp/send/route.ts b/src/app/api/whatsapp/send/route.ts index 085add6427..4c3928dc29 100644 --- a/src/app/api/whatsapp/send/route.ts +++ b/src/app/api/whatsapp/send/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from 'next/server' import { createClient } from '@/lib/supabase/server' +import { requireRole, toErrorResponse } from '@/lib/auth/account' import { checkRateLimit, rateLimitResponse, @@ -22,44 +23,24 @@ import { // dashboard's internal `{ error }` shape. export async function POST(request: Request) { try { - const supabase = await createClient() - - const { - data: { user }, - error: authError, - } = await supabase.auth.getUser() - - if (authError || !user) { - return NextResponse.json( - { error: 'Unauthorized' }, - { status: 401 } - ) - } + // Requires the 'agent' role, matching both `canSendMessages` and the + // `messages_modify` RLS policy (migration 017). + // + // Resolving `account_id` off the profile — which any 'viewer' has — + // was previously the only gate. RLS did block the message INSERT, but + // the send core calls Meta BEFORE it persists, so a viewer's request + // still delivered a real WhatsApp message to the customer and merely + // failed to record it (surfacing as "sent to Meta but failed to save + // to DB"). RLS can't un-send that, so the role check belongs here. + const { supabase, accountId, userId } = await requireRole('agent') // Per-user rate limit. Bucket key is scoped to this route so // `/broadcast` has an independent budget. - const limit = checkRateLimit(`send:${user.id}`, RATE_LIMITS.send) + const limit = checkRateLimit(`send:${userId}`, RATE_LIMITS.send) if (!limit.success) { return rateLimitResponse(limit) } - // Resolve the caller's account_id. Every downstream lookup - // (conversation, whatsapp_config, message_templates) is account- - // scoped post-multi-user, so the previous `user_id` filters - // returned nothing for teammates who didn't author the row. - const { data: profile } = await supabase - .from('profiles') - .select('account_id') - .eq('user_id', user.id) - .maybeSingle() - const accountId = profile?.account_id as string | undefined - if (!accountId) { - return NextResponse.json( - { error: 'Your profile is not linked to an account.' }, - { status: 403 }, - ) - } - const body = await request.json() const { // `conversation_id` targets an existing thread (inbox). `contact_id` @@ -148,7 +129,7 @@ export async function POST(request: Request) { const resolved = await findOrCreateConversation( supabase, accountId, - user.id, + userId, contact_id ) if (!resolved) { @@ -201,11 +182,10 @@ export async function POST(request: Request) { throw err } } catch (error) { + // requireRole throws Unauthorized/Forbidden; toErrorResponse maps + // those to 401/403 and collapses anything else to a generic 500. console.error('Error in WhatsApp send POST:', error) - return NextResponse.json( - { error: 'Failed to send message' }, - { status: 500 } - ) + return toErrorResponse(error) } } diff --git a/src/app/api/whatsapp/templates/submit/route.ts b/src/app/api/whatsapp/templates/submit/route.ts index 2569f5965e..9b496f8def 100644 --- a/src/app/api/whatsapp/templates/submit/route.ts +++ b/src/app/api/whatsapp/templates/submit/route.ts @@ -1,6 +1,11 @@ import { NextResponse } from 'next/server' import type { SupabaseClient } from '@supabase/supabase-js' -import { createClient } from '@/lib/supabase/server' +import { + ForbiddenError, + UnauthorizedError, + requireRole, + toErrorResponse, +} from '@/lib/auth/account' import { decrypt } from '@/lib/whatsapp/encryption' import { submitMessageTemplate } from '@/lib/whatsapp/meta-api' import { @@ -88,29 +93,13 @@ async function upsertTemplateRow( */ export async function POST(request: Request) { try { - const supabase = await createClient() - const { - data: { user }, - error: authError, - } = await supabase.auth.getUser() - if (authError || !user) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) - } - - // Resolve the caller's account_id — whatsapp_config + the - // message_templates row are account-scoped post-multi-user. - const { data: profile } = await supabase - .from('profiles') - .select('account_id') - .eq('user_id', user.id) - .maybeSingle() - const accountId = profile?.account_id as string | undefined - if (!accountId) { - return NextResponse.json( - { error: 'Your profile is not linked to an account.' }, - { status: 403 }, - ) - } + // Message templates are settings-class data: `canEditSettings` and the + // message_templates_insert/update RLS policies (migration 017) both + // require 'admin'. Resolving account_id off the profile only proved + // membership, so a viewer or agent could push a template to Meta for + // approval — an external side effect RLS can't roll back — before the + // local upsert was refused. + const { supabase, accountId, userId } = await requireRole('admin') let payload: TemplatePayload try { @@ -203,7 +192,7 @@ export async function POST(request: Request) { // until they fix and re-submit. await upsertTemplateRow( supabase, - buildUpsertRow(accountId, user.id, payload, { + buildUpsertRow(accountId, userId, payload, { status: 'DRAFT', metaTemplateId: null, submissionError: message, @@ -223,7 +212,7 @@ export async function POST(request: Request) { const { data: row, error: upsertErr } = await upsertTemplateRow( supabase, - buildUpsertRow(accountId, user.id, payload, { + buildUpsertRow(accountId, userId, payload, { status: normalizeStatus(metaStatus), metaTemplateId, submissionError: null, @@ -249,6 +238,16 @@ export async function POST(request: Request) { dry_run: dryRun, }) } catch (error) { + // Auth failures map to 401/403. Handled before the generic branch + // below, which surfaces `error.message` as a 500 — reporting "you + // aren't an admin" as a template submission failure would send the + // user chasing the wrong problem. + if ( + error instanceof UnauthorizedError || + error instanceof ForbiddenError + ) { + return toErrorResponse(error) + } console.error('Error submitting template:', error) return NextResponse.json( { diff --git a/src/app/api/whatsapp/templates/sync/route.ts b/src/app/api/whatsapp/templates/sync/route.ts index dfe676f2f4..9df0d4e93d 100644 --- a/src/app/api/whatsapp/templates/sync/route.ts +++ b/src/app/api/whatsapp/templates/sync/route.ts @@ -1,5 +1,10 @@ import { NextResponse } from 'next/server' -import { createClient } from '@/lib/supabase/server' +import { + ForbiddenError, + UnauthorizedError, + requireRole, + toErrorResponse, +} from '@/lib/auth/account' import { decrypt } from '@/lib/whatsapp/encryption' import { normalizeStatus } from '@/lib/whatsapp/template-status-normalize' import type { TemplateButton, TemplateSampleValues } from '@/types' @@ -124,31 +129,11 @@ function extractSampleValues( export async function POST() { try { - const supabase = await createClient() - - const { - data: { user }, - error: authError, - } = await supabase.auth.getUser() - - if (authError || !user) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) - } - - // Resolve the caller's account_id — both whatsapp_config and - // the message_templates we sync into are account-scoped. - const { data: profile } = await supabase - .from('profiles') - .select('account_id') - .eq('user_id', user.id) - .maybeSingle() - const accountId = profile?.account_id as string | undefined - if (!accountId) { - return NextResponse.json( - { error: 'Your profile is not linked to an account.' }, - { status: 403 }, - ) - } + // Syncing rewrites the account-wide template catalog, which is + // settings-class data: `canEditSettings` and the message_templates + // insert/update RLS policies (migration 017) both require 'admin'. + // Resolving account_id off the profile only proved membership. + const { supabase, accountId, userId } = await requireRole('admin') const { data: config, error: configError } = await supabase .from('whatsapp_config') @@ -237,7 +222,7 @@ export async function POST() { // route. account_id is NOT NULL on message_templates // post-017, so an INSERT without it errors. account_id: accountId, - user_id: user.id, + user_id: userId, name: t.name, category: normalizeCategory(t.category), language: t.language, @@ -310,6 +295,14 @@ export async function POST() { truncated: pageCount >= PAGE_CAP && nextUrl !== null, }) } catch (error) { + // Auth failures map to 401/403 rather than being folded into the + // generic 500 below, which surfaces `error.message` as a sync failure. + if ( + error instanceof UnauthorizedError || + error instanceof ForbiddenError + ) { + return toErrorResponse(error) + } console.error('Error syncing WhatsApp templates:', error) return NextResponse.json( {