From 957039077d3681a7c90cb1f246a3a5aeb6a3a8a1 Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Wed, 27 May 2026 12:54:56 -0400 Subject: [PATCH 1/2] fix(api): support API key auth on upload-submission endpoint --- .../evidence-forms.controller.spec.ts | 69 +++++++++++++++++-- .../evidence-forms.controller.ts | 26 +++++-- .../evidence-forms/evidence-forms.service.ts | 4 +- 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/apps/api/src/evidence-forms/evidence-forms.controller.spec.ts b/apps/api/src/evidence-forms/evidence-forms.controller.spec.ts index 4277a92ccc..45c683dba9 100644 --- a/apps/api/src/evidence-forms/evidence-forms.controller.spec.ts +++ b/apps/api/src/evidence-forms/evidence-forms.controller.spec.ts @@ -1,9 +1,16 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { BadRequestException } from '@nestjs/common'; import { EvidenceFormsController } from './evidence-forms.controller'; import { EvidenceFormsService } from './evidence-forms.service'; +import { ActingUserResolver } from '../auth/acting-user.service'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; -import type { AuthContext as AuthContextType } from '../auth/types'; +import type { + AuthContext as AuthContextType, + AuthenticatedRequest, +} from '../auth/types'; + +jest.mock('@db', () => ({ db: {} })); jest.mock('../auth/auth.server', () => ({ auth: { api: { getSession: jest.fn() } }, @@ -61,10 +68,17 @@ describe('EvidenceFormsController', () => { userRoles: ['admin'], }; + const mockActingUser = { + resolve: jest.fn(), + }; + beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [EvidenceFormsController], - providers: [{ provide: EvidenceFormsService, useValue: mockService }], + providers: [ + { provide: EvidenceFormsService, useValue: mockService }, + { provide: ActingUserResolver, useValue: mockActingUser }, + ], }) .overrideGuard(HybridAuthGuard) .useValue(mockGuard) @@ -298,26 +312,71 @@ describe('EvidenceFormsController', () => { }); describe('uploadSubmission', () => { - it('should call service.uploadSubmission with correct params', async () => { + it('should call service.uploadSubmission with the session userId', async () => { const body = { fileUrl: 'https://example.com/file.pdf' }; const mockResult = { id: 'sub_upload' }; mockService.uploadSubmission.mockResolvedValue(mockResult); + mockActingUser.resolve.mockResolvedValue({ + userId: 'user_1', + source: 'session', + }); + const req = {} as AuthenticatedRequest; const result = await controller.uploadSubmission( 'org_1', - mockAuthContext, 'security-awareness', body, + req, ); expect(result).toEqual(mockResult); + expect(mockActingUser.resolve).toHaveBeenCalledWith(req, 'org_1'); expect(service.uploadSubmission).toHaveBeenCalledWith({ organizationId: 'org_1', formType: 'security-awareness', - authContext: mockAuthContext, + userId: 'user_1', payload: body, }); }); + + it('should attribute to the org owner when called with API key auth', async () => { + const body = { fileUrl: 'https://example.com/file.pdf' }; + const mockResult = { id: 'sub_upload' }; + mockService.uploadSubmission.mockResolvedValue(mockResult); + mockActingUser.resolve.mockResolvedValue({ + userId: 'owner_user', + source: 'org-owner-fallback', + callerLabel: 'via API key "CI"', + }); + + const req = {} as AuthenticatedRequest; + await controller.uploadSubmission('org_1', 'meeting', body, req); + + expect(service.uploadSubmission).toHaveBeenCalledWith({ + organizationId: 'org_1', + formType: 'meeting', + userId: 'owner_user', + payload: body, + }); + }); + + it('should throw BadRequestException when no owner can be resolved', async () => { + mockActingUser.resolve.mockResolvedValue({ + userId: null, + source: 'org-owner-fallback', + callerLabel: 'via API key', + }); + + await expect( + controller.uploadSubmission( + 'org_1', + 'meeting', + { fileUrl: 'x' }, + {} as AuthenticatedRequest, + ), + ).rejects.toBeInstanceOf(BadRequestException); + expect(service.uploadSubmission).not.toHaveBeenCalled(); + }); }); describe('reviewSubmission', () => { diff --git a/apps/api/src/evidence-forms/evidence-forms.controller.ts b/apps/api/src/evidence-forms/evidence-forms.controller.ts index cec36356d0..2597065fde 100644 --- a/apps/api/src/evidence-forms/evidence-forms.controller.ts +++ b/apps/api/src/evidence-forms/evidence-forms.controller.ts @@ -1,10 +1,15 @@ +import { ActingUserResolver } from '@/auth/acting-user.service'; import { AuthContext, OrganizationId } from '@/auth/auth-context.decorator'; import { HybridAuthGuard } from '@/auth/hybrid-auth.guard'; import { PermissionGuard } from '@/auth/permission.guard'; import { RequirePermission } from '@/auth/require-permission.decorator'; -import type { AuthContext as AuthContextType } from '@/auth/types'; +import type { + AuthContext as AuthContextType, + AuthenticatedRequest, +} from '@/auth/types'; import { AuditRead } from '@/audit/skip-audit-log.decorator'; import { + BadRequestException, Body, Controller, Delete, @@ -14,6 +19,7 @@ import { Patch, Post, Query, + Req, Res, UseGuards, } from '@nestjs/common'; @@ -32,7 +38,10 @@ import { EvidenceFormsService } from './evidence-forms.service'; required: false, }) export class EvidenceFormsController { - constructor(private readonly evidenceFormsService: EvidenceFormsService) {} + constructor( + private readonly evidenceFormsService: EvidenceFormsService, + private readonly actingUser: ActingUserResolver, + ) {} @Get() @RequirePermission('evidence', 'read') @@ -213,18 +222,25 @@ export class EvidenceFormsController { @ApiOperation({ summary: 'Upload a file as an evidence submission', description: - 'Upload a PDF or image file and create a submission for the given form type, bypassing form-specific validation', + 'Upload a PDF or image file and create a submission for the given form type, bypassing form-specific validation. ' + + "Accepts session, API key, or service token auth. For API key / service token callers without an explicit user attribution, the submission is attributed to the org's oldest owner.", }) async uploadSubmission( @OrganizationId() organizationId: string, - @AuthContext() authContext: AuthContextType, @Param('formType') formType: string, @Body() body: unknown, + @Req() req: AuthenticatedRequest, ) { + const acting = await this.actingUser.resolve(req, organizationId); + if (!acting.userId) { + throw new BadRequestException( + 'Cannot attribute this submission — your organization must have at least one user with the "owner" role.', + ); + } return this.evidenceFormsService.uploadSubmission({ organizationId, formType, - authContext, + userId: acting.userId, payload: body, }); } diff --git a/apps/api/src/evidence-forms/evidence-forms.service.ts b/apps/api/src/evidence-forms/evidence-forms.service.ts index b1589ed906..b2ed8afb8c 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.ts @@ -648,7 +648,7 @@ export class EvidenceFormsService { async uploadSubmission(params: { organizationId: string; formType: string; - authContext: AuthContext; + userId: string; payload: unknown; }) { const parsedType = evidenceFormTypeSchema.safeParse(params.formType); @@ -656,7 +656,7 @@ export class EvidenceFormsService { throw new BadRequestException('Unsupported form type'); } - const userId = this.requireJwtUser(params.authContext); + const { userId } = params; const parsed = uploadSubmissionBodySchema.safeParse(params.payload); if (!parsed.success) { From ec6a1fa9beff9fe474dba37785a06bb0ae834ba8 Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 1 Jun 2026 15:10:03 -0400 Subject: [PATCH 2/2] fix(api): exclude deactivated owners from acting-user fallback --- apps/api/src/auth/acting-user.service.spec.ts | 23 +++++++++++++++++++ apps/api/src/auth/acting-user.service.ts | 11 ++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/apps/api/src/auth/acting-user.service.spec.ts b/apps/api/src/auth/acting-user.service.spec.ts index bf6762093a..3d28ba71ca 100644 --- a/apps/api/src/auth/acting-user.service.spec.ts +++ b/apps/api/src/auth/acting-user.service.spec.ts @@ -122,6 +122,29 @@ describe('ActingUserResolver', () => { ); }); + it('filters out deactivated / inactive members so uploads cannot be attributed to offboarded owners', async () => { + // Regression guard — without these filters, the oldest "owner"-role + // Member would win even if the user has been deactivated/offboarded, + // attributing API-driven mutations to someone who no longer has access. + mockDb.member.findFirst.mockResolvedValueOnce({ userId: 'usr_owner' }); + const req = makeReq({ + userId: undefined, + isApiKey: true, + apiKeyName: 'X', + }); + + await resolver.resolve(req, 'org_1'); + + expect(mockDb.member.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + deactivated: false, + isActive: true, + }), + }), + ); + }); + it('picks the OLDEST owner deterministically (orderBy createdAt asc)', async () => { // Determinism matters — re-running the same automation should always // attribute to the same user, even if newer owners are added/removed. diff --git a/apps/api/src/auth/acting-user.service.ts b/apps/api/src/auth/acting-user.service.ts index 8224bb3c1d..9091375e57 100644 --- a/apps/api/src/auth/acting-user.service.ts +++ b/apps/api/src/auth/acting-user.service.ts @@ -91,14 +91,17 @@ export class ActingUserResolver { } /** - * Find the oldest owner of an organization. Oldest is deterministic and - * stable: removing a recently-added owner doesn't change the attribution + * Find the oldest ACTIVE owner of an organization. Oldest is deterministic + * and stable: removing a recently-added owner doesn't change the attribution * target, removing the oldest one just promotes the next one. Matches the - * pattern used elsewhere (e.g. tasks/task-notifier.service.ts). + * pattern used elsewhere (e.g. tasks/tasks.service.ts:getApiKeyActorUserId). * * Member.role is a comma-separated string (e.g. "owner,admin"), so we use * Prisma's `contains` filter — same query shape as the 19+ other owner * lookups in this codebase. + * + * `deactivated: false` + `isActive: true` excludes offboarded owners so we + * don't attribute new mutations to a user who no longer has org access. */ private async findOrgOwnerUserId( organizationId: string, @@ -106,6 +109,8 @@ export class ActingUserResolver { const owner = await db.member.findFirst({ where: { organizationId, + deactivated: false, + isActive: true, role: { contains: 'owner' }, }, orderBy: { createdAt: 'asc' },