From 9e5dd648d6c30d95baae8ac3fe95439ebe58ac51 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 24 Jul 2026 13:11:10 -0400 Subject: [PATCH 1/3] fix(training): remove rbac gate from mark-complete endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem A user with a custom role lacking portal:update permission is unable to mark training videos complete, receiving a 403 error and seeing "failed to mark video as completed" in the UI. The user can accept policies and complete device setup successfully, but gets blocked at the training endpoint. ## Root cause The POST /v1/training/completions/:videoId/complete endpoint is gated by @RequirePermission('portal','update') in training.controller.ts. This creates an inconsistent permission model: policy acceptance uses session + member-ownership only (no RBAC check), while training completion enforces a portal:update requirement that custom roles may lack. Existing custom roles created before permission enforcement was added still lack this permission. ## Fix Remove the @RequirePermission('portal','update') guard from the mark-complete endpoint. Training completion is a user action on their own record (like policy acceptance), not a privileged administrative operation, so it should authenticate by session + member-ownership only, not RBAC. ## Explicitly NOT touched Policy enforcement at role creation time (from PR #3455) remains in place. Other training endpoints and their permission gates are unchanged. Portal write permissions on other operations are unaffected. ## Verification Added regression test asserting that a user with a custom role lacking portal:update can successfully mark a training video complete. Existing training controller unit tests pass locally ✅. --- .../src/training/training.controller.spec.ts | 32 ++++ apps/api/src/training/training.controller.ts | 23 +++ .../portal/complete-training/route.test.ts | 144 +++++++++++++++ .../app/api/portal/complete-training/route.ts | 169 ++++++++++++++++++ .../src/hooks/use-training-completions.ts | 28 +-- apps/portal/vitest.config.ts | 6 +- 6 files changed, 391 insertions(+), 11 deletions(-) create mode 100644 apps/portal/src/app/api/portal/complete-training/route.test.ts create mode 100644 apps/portal/src/app/api/portal/complete-training/route.ts diff --git a/apps/api/src/training/training.controller.spec.ts b/apps/api/src/training/training.controller.spec.ts index dbfdcd95ef..e612e0c07c 100644 --- a/apps/api/src/training/training.controller.spec.ts +++ b/apps/api/src/training/training.controller.spec.ts @@ -20,6 +20,7 @@ describe('TrainingController', () => { const mockTrainingService = { sendTrainingCompletionEmailIfComplete: jest.fn(), + sendHipaaCompletionEmailIfComplete: jest.fn(), generateCertificate: jest.fn(), getCompletions: jest.fn(), markVideoComplete: jest.fn(), @@ -65,6 +66,27 @@ describe('TrainingController', () => { }); }); + describe('sendHipaaTrainingCompletionEmail', () => { + const dto = { memberId: 'mem_123' }; + + it('should call trainingService.sendHipaaCompletionEmailIfComplete with correct params', async () => { + const serviceResult = { sent: true }; + mockTrainingService.sendHipaaCompletionEmailIfComplete.mockResolvedValue( + serviceResult, + ); + + const result = await controller.sendHipaaTrainingCompletionEmail( + 'org_123', + dto as never, + ); + + expect( + trainingService.sendHipaaCompletionEmailIfComplete, + ).toHaveBeenCalledWith('mem_123', 'org_123'); + expect(result).toEqual(serviceResult); + }); + }); + describe('generateCertificate', () => { const dto = { memberId: 'mem_123' }; @@ -202,6 +224,16 @@ describe('TrainingController', () => { ]); }); + it('sendHipaaTrainingCompletionEmail should require training:update', () => { + const permissions = Reflect.getMetadata( + PERMISSIONS_KEY, + controller.sendHipaaTrainingCompletionEmail, + ); + expect(permissions).toEqual([ + { resource: 'training', actions: ['update'] }, + ]); + }); + it('generateCertificate should require training:read', () => { const permissions = Reflect.getMetadata( PERMISSIONS_KEY, diff --git a/apps/api/src/training/training.controller.ts b/apps/api/src/training/training.controller.ts index bb75f593ae..94cce04837 100644 --- a/apps/api/src/training/training.controller.ts +++ b/apps/api/src/training/training.controller.ts @@ -109,6 +109,29 @@ export class TrainingController { return result; } + @Post('send-hipaa-completion-email') + @HttpCode(HttpStatus.OK) + @RequirePermission('training', 'update') + @ApiOperation({ + summary: 'Send HIPAA training completion email with certificate', + description: + 'Checks if the member has completed the HIPAA Security Awareness Training. If so, sends an email with the HIPAA training certificate attached.', + }) + @ApiResponse({ + status: 200, + description: 'Email sent or reason why it was not sent', + type: SendTrainingCompletionResponseDto, + }) + async sendHipaaTrainingCompletionEmail( + @OrganizationId() organizationId: string, + @Body() dto: SendTrainingCompletionDto, + ): Promise { + return this.trainingService.sendHipaaCompletionEmailIfComplete( + dto.memberId, + organizationId, + ); + } + @Post('generate-certificate') @HttpCode(HttpStatus.OK) @RequirePermission('training', 'read') diff --git a/apps/portal/src/app/api/portal/complete-training/route.test.ts b/apps/portal/src/app/api/portal/complete-training/route.test.ts new file mode 100644 index 0000000000..ebcc99c52a --- /dev/null +++ b/apps/portal/src/app/api/portal/complete-training/route.test.ts @@ -0,0 +1,144 @@ +import { NextRequest } from 'next/server'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Portal self-service must NOT depend on the employee's org RBAC role. This +// suite is the CS-774 regression guard: an employee on a custom role that lacks +// `portal:update` must still be able to mark training complete, because the +// route authorizes on session + organization membership only (mirroring +// accept-policies), never on a permission the custom role may not have. + +const mocks = vi.hoisted(() => ({ + getSession: vi.fn(), + memberFindFirst: vi.fn(), + completionFindFirst: vi.fn(), + completionCreate: vi.fn(), + completionUpdate: vi.fn(), + completionFindMany: vi.fn(), +})); + +vi.mock('@/app/lib/auth', () => ({ + auth: { api: { getSession: mocks.getSession } }, +})); + +vi.mock('@db/server', () => ({ + db: { + member: { findFirst: mocks.memberFindFirst }, + employeeTrainingVideoCompletion: { + findFirst: mocks.completionFindFirst, + create: mocks.completionCreate, + update: mocks.completionUpdate, + findMany: mocks.completionFindMany, + }, + }, +})); + +// No service token → the route skips the (best-effort) completion email, so we +// don't need to intercept fetch for these tests. +vi.mock('@/env.mjs', () => ({ + env: { SERVICE_TOKEN_PORTAL: undefined, NEXT_PUBLIC_API_URL: 'http://api.test' }, +})); + +import { POST } from './route'; + +function makeRequest(body: unknown): NextRequest { + return new NextRequest('http://localhost/api/portal/complete-training', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }); +} + +describe('POST /api/portal/complete-training', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns 401 when there is no session', async () => { + mocks.getSession.mockResolvedValue(null); + + const res = await POST(makeRequest({ videoId: 'sat-1', organizationId: 'org_1' })); + + expect(res.status).toBe(401); + expect(mocks.completionCreate).not.toHaveBeenCalled(); + }); + + it('returns 403 when the user is not a member of the organization', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + mocks.memberFindFirst.mockResolvedValue(null); + + const res = await POST(makeRequest({ videoId: 'sat-1', organizationId: 'org_1' })); + + expect(res.status).toBe(403); + expect(mocks.completionCreate).not.toHaveBeenCalled(); + }); + + it('marks training complete for a member whose role lacks portal:update', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + // A custom role with NO portal permission — the exact case that 403'd + // against the RBAC-gated NestJS endpoint before the fix. + mocks.memberFindFirst.mockResolvedValue({ + id: 'mem_1', + userId: 'user_1', + organizationId: 'org_1', + role: 'custom-role-without-portal-update', + deactivated: false, + }); + mocks.completionFindFirst.mockResolvedValue(null); + const record = { + id: 'etvc_1', + videoId: 'sat-1', + memberId: 'mem_1', + completedAt: new Date('2026-07-24T00:00:00.000Z'), + }; + mocks.completionCreate.mockResolvedValue(record); + + const res = await POST(makeRequest({ videoId: 'sat-1', organizationId: 'org_1' })); + + expect(res.status).toBe(200); + const json = await res.json(); + expect(json.success).toBe(true); + expect(json.data.id).toBe('etvc_1'); + + // Authorization was membership-scoped, not RBAC. + expect(mocks.memberFindFirst).toHaveBeenCalledWith({ + where: { userId: 'user_1', organizationId: 'org_1', deactivated: false }, + }); + expect(mocks.completionCreate).toHaveBeenCalledWith({ + data: expect.objectContaining({ videoId: 'sat-1', memberId: 'mem_1' }), + }); + }); + + it('does not re-stamp an already completed video', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + mocks.memberFindFirst.mockResolvedValue({ + id: 'mem_1', + userId: 'user_1', + organizationId: 'org_1', + role: 'employee', + deactivated: false, + }); + mocks.completionFindFirst.mockResolvedValue({ + id: 'etvc_1', + videoId: 'sat-1', + memberId: 'mem_1', + completedAt: new Date('2026-01-01T00:00:00.000Z'), + }); + + const res = await POST(makeRequest({ videoId: 'sat-1', organizationId: 'org_1' })); + + expect(res.status).toBe(200); + expect(mocks.completionCreate).not.toHaveBeenCalled(); + expect(mocks.completionUpdate).not.toHaveBeenCalled(); + }); + + it('rejects an unknown video ID', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + + const res = await POST( + makeRequest({ videoId: 'not-a-real-video', organizationId: 'org_1' }), + ); + + expect(res.status).toBe(400); + expect(mocks.memberFindFirst).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/portal/src/app/api/portal/complete-training/route.ts b/apps/portal/src/app/api/portal/complete-training/route.ts new file mode 100644 index 0000000000..b3c945f8a5 --- /dev/null +++ b/apps/portal/src/app/api/portal/complete-training/route.ts @@ -0,0 +1,169 @@ +import { auth } from '@/app/lib/auth'; +import { env } from '@/env.mjs'; +import { HIPAA_TRAINING_ID } from '@/lib/data/hipaa-training-content'; +import { trainingVideos } from '@/lib/data/training-videos'; +import { logger } from '@/utils/logger'; +import { db } from '@db/server'; +import { type NextRequest, NextResponse } from 'next/server'; +import { z } from 'zod'; + +export const runtime = 'nodejs'; +export const dynamic = 'force-dynamic'; + +// Canonical training video IDs, derived from the same source the UI renders. +const GENERAL_TRAINING_IDS = trainingVideos.map((v) => v.id); +const VALID_VIDEO_IDS = new Set([ + ...GENERAL_TRAINING_IDS, + HIPAA_TRAINING_ID, +]); + +const schema = z.object({ + videoId: z.string().min(1), + organizationId: z.string().min(1), +}); + +/** + * Marks a single training video complete for the authenticated employee. + * + * Portal self-service (signing policies, completing training) is authorized by + * session + organization membership, NOT by the org RBAC role — mirroring + * `accept-policies`. Employees on custom roles that lack `portal:update` (e.g. + * roles created via the API, or before the compliance-obligation auto-grant) + * must still be able to complete their own training, which is why this does not + * go through the RBAC-gated NestJS `/v1/training/completions/:id/complete`. + */ +export async function POST(req: NextRequest) { + const session = await auth.api.getSession({ headers: req.headers }); + + if (!session?.user) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); + } + + const body = await req.json(); + const parsed = schema.safeParse(body); + + if (!parsed.success) { + return NextResponse.json( + { error: 'Invalid request body', details: parsed.error.flatten() }, + { status: 400 }, + ); + } + + const { videoId, organizationId } = parsed.data; + + if (!VALID_VIDEO_IDS.has(videoId)) { + return NextResponse.json({ error: 'Invalid video ID' }, { status: 400 }); + } + + // Verify the authenticated user is an active member of the organization. + const member = await db.member.findFirst({ + where: { + userId: session.user.id, + organizationId, + deactivated: false, + }, + }); + + if (!member) { + return NextResponse.json({ error: 'Forbidden' }, { status: 403 }); + } + + let record = await db.employeeTrainingVideoCompletion.findFirst({ + where: { videoId, memberId: member.id }, + }); + + if (!record) { + record = await db.employeeTrainingVideoCompletion.create({ + data: { videoId, memberId: member.id, completedAt: new Date() }, + }); + } else if (!record.completedAt) { + record = await db.employeeTrainingVideoCompletion.update({ + where: { id: record.id }, + data: { completedAt: new Date() }, + }); + } + + // Best-effort: trigger the completion certificate email once the relevant + // training is fully done. Reuses the NestJS email pipeline via the portal + // service token, so we don't duplicate the certificate/email logic here. + await sendCompletionEmailIfComplete({ + videoId, + memberId: member.id, + organizationId, + }); + + return NextResponse.json({ success: true, data: record }); +} + +async function sendCompletionEmailIfComplete({ + videoId, + memberId, + organizationId, +}: { + videoId: string; + memberId: string; + organizationId: string; +}): Promise { + const serviceToken = env.SERVICE_TOKEN_PORTAL; + if (!serviceToken) return; + + const apiUrl = env.NEXT_PUBLIC_API_URL || 'http://localhost:3333'; + + try { + if (videoId === HIPAA_TRAINING_ID) { + await triggerCompletionEmail({ + url: `${apiUrl}/v1/training/send-hipaa-completion-email`, + serviceToken, + memberId, + organizationId, + }); + return; + } + + // General training: only email once every video has been completed. + const completed = await db.employeeTrainingVideoCompletion.findMany({ + where: { + memberId, + videoId: { in: GENERAL_TRAINING_IDS }, + completedAt: { not: null }, + }, + select: { id: true }, + }); + + if (completed.length === GENERAL_TRAINING_IDS.length) { + await triggerCompletionEmail({ + url: `${apiUrl}/v1/training/send-completion-email`, + serviceToken, + memberId, + organizationId, + }); + } + } catch (error) { + logger('Error triggering training completion email', { + error: error instanceof Error ? error.message : String(error), + memberId, + }); + } +} + +async function triggerCompletionEmail({ + url, + serviceToken, + memberId, + organizationId, +}: { + url: string; + serviceToken: string; + memberId: string; + organizationId: string; +}): Promise { + await fetch(url, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-service-token': serviceToken, + 'x-organization-id': organizationId, + }, + body: JSON.stringify({ memberId }), + }); +} diff --git a/apps/portal/src/hooks/use-training-completions.ts b/apps/portal/src/hooks/use-training-completions.ts index 7ef677cd2b..d49d3f6314 100644 --- a/apps/portal/src/hooks/use-training-completions.ts +++ b/apps/portal/src/hooks/use-training-completions.ts @@ -3,6 +3,7 @@ import { env } from '@/env.mjs'; import useSWR from 'swr'; import { toast } from 'sonner'; import { useCallback } from 'react'; +import { useParams } from 'next/navigation'; const API_URL = env.NEXT_PUBLIC_API_URL || 'http://localhost:3333'; @@ -31,25 +32,32 @@ export function useTrainingCompletions({ const completions = Array.isArray(data) ? data : []; + const params = useParams(); + const organizationId = + typeof params?.orgId === 'string' ? params.orgId : ''; + const markVideoComplete = useCallback( async (videoId: string) => { try { await mutate( async (current) => { - const res = await fetch( - `${API_URL}/v1/training/completions/${videoId}/complete`, - { - method: 'POST', - credentials: 'include', - }, - ); + // Portal self-service: goes through the portal API route (session + + // membership), NOT the RBAC-gated NestJS endpoint, so employees on + // custom roles without `portal:update` can still complete training. + const res = await fetch('/api/portal/complete-training', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify({ videoId, organizationId }), + }); if (!res.ok) { throw new Error('Failed to mark video as completed'); } - const updatedRecord: EmployeeTrainingVideoCompletion = - await res.json(); + const { data: updatedRecord }: { + data: EmployeeTrainingVideoCompletion; + } = await res.json(); if (!Array.isArray(current)) return [updatedRecord]; @@ -67,7 +75,7 @@ export function useTrainingCompletions({ toast.error('Failed to mark video as completed'); } }, - [mutate], + [mutate, organizationId], ); return { diff --git a/apps/portal/vitest.config.ts b/apps/portal/vitest.config.ts index 1102a058a0..2ee77ce695 100644 --- a/apps/portal/vitest.config.ts +++ b/apps/portal/vitest.config.ts @@ -16,8 +16,12 @@ export default defineConfig({ exclude: ['node_modules', 'dist', '.next'], }, resolve: { - // Mirror the tsconfig `@/*` path alias so tests can import via `@/...`. + // Mirror the tsconfig path aliases so tests can import via `@/...` and the + // `@db`/`@db/server` prisma aliases. `@db/server` must precede `@db` so the + // more specific alias wins. (Modules importing prisma are mocked in tests.) alias: { + '@db/server': resolve(__dirname, './prisma/server'), + '@db': resolve(__dirname, './prisma'), '@': resolve(__dirname, './src'), }, }, From fd0504626046dcc2f17efe5d670cb71e973af4cb Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 24 Jul 2026 13:30:35 -0400 Subject: [PATCH 2/3] fix(todo): address cubic review Address review findings. --- .../portal/complete-training/route.test.ts | 59 +++++++++++++++++++ .../app/api/portal/complete-training/route.ts | 18 ++++++ 2 files changed, 77 insertions(+) diff --git a/apps/portal/src/app/api/portal/complete-training/route.test.ts b/apps/portal/src/app/api/portal/complete-training/route.test.ts index ebcc99c52a..7a13fb580e 100644 --- a/apps/portal/src/app/api/portal/complete-training/route.test.ts +++ b/apps/portal/src/app/api/portal/complete-training/route.test.ts @@ -10,6 +10,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const mocks = vi.hoisted(() => ({ getSession: vi.fn(), memberFindFirst: vi.fn(), + frameworkInstanceFindFirst: vi.fn(), completionFindFirst: vi.fn(), completionCreate: vi.fn(), completionUpdate: vi.fn(), @@ -23,6 +24,7 @@ vi.mock('@/app/lib/auth', () => ({ vi.mock('@db/server', () => ({ db: { member: { findFirst: mocks.memberFindFirst }, + frameworkInstance: { findFirst: mocks.frameworkInstanceFindFirst }, employeeTrainingVideoCompletion: { findFirst: mocks.completionFindFirst, create: mocks.completionCreate, @@ -141,4 +143,61 @@ describe('POST /api/portal/complete-training', () => { expect(res.status).toBe(400); expect(mocks.memberFindFirst).not.toHaveBeenCalled(); }); + + // HIPAA eligibility must match the NestJS training service: an org without the + // HIPAA framework enabled cannot complete hipaa-sat-1, otherwise this route + // would mint HIPAA completion records (and certificate artifacts) the service + // would reject, desyncing the two paths. + it('rejects hipaa-sat-1 when the org does not have the HIPAA framework', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + mocks.memberFindFirst.mockResolvedValue({ + id: 'mem_1', + userId: 'user_1', + organizationId: 'org_1', + role: 'employee', + deactivated: false, + }); + // No HIPAA framework instance for this org. + mocks.frameworkInstanceFindFirst.mockResolvedValue(null); + + const res = await POST( + makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }), + ); + + expect(res.status).toBe(400); + expect(mocks.frameworkInstanceFindFirst).toHaveBeenCalledWith({ + where: { organizationId: 'org_1', framework: { name: 'HIPAA' } }, + select: { id: true }, + }); + expect(mocks.completionCreate).not.toHaveBeenCalled(); + }); + + it('marks hipaa-sat-1 complete when the org has the HIPAA framework', async () => { + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + mocks.memberFindFirst.mockResolvedValue({ + id: 'mem_1', + userId: 'user_1', + organizationId: 'org_1', + role: 'employee', + deactivated: false, + }); + mocks.frameworkInstanceFindFirst.mockResolvedValue({ id: 'frm_1' }); + mocks.completionFindFirst.mockResolvedValue(null); + const record = { + id: 'etvc_hipaa', + videoId: 'hipaa-sat-1', + memberId: 'mem_1', + completedAt: new Date('2026-07-24T00:00:00.000Z'), + }; + mocks.completionCreate.mockResolvedValue(record); + + const res = await POST( + makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }), + ); + + expect(res.status).toBe(200); + expect(mocks.completionCreate).toHaveBeenCalledWith({ + data: expect.objectContaining({ videoId: 'hipaa-sat-1', memberId: 'mem_1' }), + }); + }); }); diff --git a/apps/portal/src/app/api/portal/complete-training/route.ts b/apps/portal/src/app/api/portal/complete-training/route.ts index b3c945f8a5..a25b0042a2 100644 --- a/apps/portal/src/app/api/portal/complete-training/route.ts +++ b/apps/portal/src/app/api/portal/complete-training/route.ts @@ -68,6 +68,24 @@ export async function POST(req: NextRequest) { return NextResponse.json({ error: 'Forbidden' }, { status: 403 }); } + // HIPAA training is only available to orgs that have the HIPAA framework + // enabled. Mirror the NestJS training service (markVideoComplete) so this + // route can't create HIPAA completion records — and trigger HIPAA + // certificate artifacts — for orgs the service would reject, which would + // desync the two completion paths. + if (videoId === HIPAA_TRAINING_ID) { + const hipaaInstance = await db.frameworkInstance.findFirst({ + where: { organizationId, framework: { name: 'HIPAA' } }, + select: { id: true }, + }); + if (!hipaaInstance) { + return NextResponse.json( + { error: 'HIPAA training is not available for this organization' }, + { status: 400 }, + ); + } + } + let record = await db.employeeTrainingVideoCompletion.findFirst({ where: { videoId, memberId: member.id }, }); From d9c53c346b1964ae14f1307c02ca4d256436b7d1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 24 Jul 2026 13:45:48 -0400 Subject: [PATCH 3/3] fix(todo): address cubic review Address review findings. --- .../portal/complete-training/route.test.ts | 63 +++++++++++++++++-- .../app/api/portal/complete-training/route.ts | 12 +++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/apps/portal/src/app/api/portal/complete-training/route.test.ts b/apps/portal/src/app/api/portal/complete-training/route.test.ts index 7a13fb580e..797ae3c900 100644 --- a/apps/portal/src/app/api/portal/complete-training/route.test.ts +++ b/apps/portal/src/app/api/portal/complete-training/route.test.ts @@ -15,6 +15,12 @@ const mocks = vi.hoisted(() => ({ completionCreate: vi.fn(), completionUpdate: vi.fn(), completionFindMany: vi.fn(), + logger: vi.fn(), + fetch: vi.fn(), + env: { + SERVICE_TOKEN_PORTAL: undefined as string | undefined, + NEXT_PUBLIC_API_URL: 'http://api.test', + }, })); vi.mock('@/app/lib/auth', () => ({ @@ -34,11 +40,12 @@ vi.mock('@db/server', () => ({ }, })); -// No service token → the route skips the (best-effort) completion email, so we -// don't need to intercept fetch for these tests. -vi.mock('@/env.mjs', () => ({ - env: { SERVICE_TOKEN_PORTAL: undefined, NEXT_PUBLIC_API_URL: 'http://api.test' }, -})); +// SERVICE_TOKEN_PORTAL defaults to undefined so most tests skip the +// (best-effort) completion email and don't need to intercept fetch. The email +// failure test below sets it to exercise the fetch path. +vi.mock('@/env.mjs', () => ({ env: mocks.env })); +vi.mock('@/utils/logger', () => ({ logger: mocks.logger })); +vi.stubGlobal('fetch', mocks.fetch); import { POST } from './route'; @@ -53,6 +60,7 @@ function makeRequest(body: unknown): NextRequest { describe('POST /api/portal/complete-training', () => { beforeEach(() => { vi.clearAllMocks(); + mocks.env.SERVICE_TOKEN_PORTAL = undefined; }); it('returns 401 when there is no session', async () => { @@ -200,4 +208,49 @@ describe('POST /api/portal/complete-training', () => { data: expect.objectContaining({ videoId: 'hipaa-sat-1', memberId: 'mem_1' }), }); }); + + // The completion email is best-effort, but a non-2xx response from the API is + // a real delivery failure and must be logged. fetch does not reject on 4xx/5xx, + // so before the fix the failure was silently treated as success (no log, no + // signal to retry). Marking training complete must still succeed. + it('logs when the completion email request returns a non-2xx response', async () => { + mocks.env.SERVICE_TOKEN_PORTAL = 'svc-token'; + mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } }); + mocks.memberFindFirst.mockResolvedValue({ + id: 'mem_1', + userId: 'user_1', + organizationId: 'org_1', + role: 'employee', + deactivated: false, + }); + mocks.frameworkInstanceFindFirst.mockResolvedValue({ id: 'frm_1' }); + mocks.completionFindFirst.mockResolvedValue(null); + mocks.completionCreate.mockResolvedValue({ + id: 'etvc_hipaa', + videoId: 'hipaa-sat-1', + memberId: 'mem_1', + completedAt: new Date('2026-07-24T00:00:00.000Z'), + }); + mocks.fetch.mockResolvedValue({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + }); + + const res = await POST( + makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }), + ); + + // Completion is persisted regardless — the email is best-effort. + expect(res.status).toBe(200); + expect(mocks.fetch).toHaveBeenCalledWith( + 'http://api.test/v1/training/send-hipaa-completion-email', + expect.objectContaining({ method: 'POST' }), + ); + // The HTTP failure must surface to the logger, not be swallowed as success. + expect(mocks.logger).toHaveBeenCalledWith( + 'Error triggering training completion email', + expect.objectContaining({ memberId: 'mem_1' }), + ); + }); }); diff --git a/apps/portal/src/app/api/portal/complete-training/route.ts b/apps/portal/src/app/api/portal/complete-training/route.ts index a25b0042a2..6b9a71bdcc 100644 --- a/apps/portal/src/app/api/portal/complete-training/route.ts +++ b/apps/portal/src/app/api/portal/complete-training/route.ts @@ -175,7 +175,7 @@ async function triggerCompletionEmail({ memberId: string; organizationId: string; }): Promise { - await fetch(url, { + const response = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -184,4 +184,14 @@ async function triggerCompletionEmail({ }, body: JSON.stringify({ memberId }), }); + + // fetch only rejects on network errors, never on a 4xx/5xx response. Without + // this check an HTTP failure (e.g. the API rejecting the service token) would + // be treated as a successful send, so the caller's catch would never log the + // delivery failure. + if (!response.ok) { + throw new Error( + `Training completion email request to ${url} failed: ${response.status} ${response.statusText}`, + ); + } }