diff --git a/apps/api/src/jobs/gdpr-export.ts b/apps/api/src/jobs/gdpr-export.ts index b5e99e90..683af0b9 100644 --- a/apps/api/src/jobs/gdpr-export.ts +++ b/apps/api/src/jobs/gdpr-export.ts @@ -14,11 +14,34 @@ import { readStoredFile } from "../lib/file-storage.js"; import { putObject } from "../lib/object-storage.js"; export async function gdprExportJob(userId: string, jobId: string): Promise<{ outputRef: string }> { - // 1. Fetch user profile (exclude passwordHash) - const [user] = await db.select().from(schema.users).where(eq(schema.users.id, userId)); - if (!user) throw new Error(`User ${userId} not found`); - - const { passwordHash: _, ...profile } = user; + // 1. Fetch user profile. + // + // The columns are named rather than selected with `*` so that authentication + // material never leaves Postgres. passwordHash, totpSecret and recoveryCodesHash + // are credentials, not personal data a subject-access request is owed, and this + // archive is meant to be handed to the data subject or a regulator. Listing the + // allowlist here also means a future column on `users` is excluded by default + // instead of silently joining the export. + const [profile] = await db + .select({ + id: schema.users.id, + username: schema.users.username, + role: schema.users.role, + team: schema.users.team, + email: schema.users.email, + authProvider: schema.users.authProvider, + externalId: schema.users.externalId, + mustChangePassword: schema.users.mustChangePassword, + legalHold: schema.users.legalHold, + storageUsed: schema.users.storageUsed, + storageQuota: schema.users.storageQuota, + totpEnabled: schema.users.totpEnabled, + createdAt: schema.users.createdAt, + updatedAt: schema.users.updatedAt, + }) + .from(schema.users) + .where(eq(schema.users.id, userId)); + if (!profile) throw new Error(`User ${userId} not found`); // 2. Fetch all user's files metadata const files = await db.select().from(schema.userFiles).where(eq(schema.userFiles.userId, userId)); diff --git a/apps/api/src/routes/enterprise/gdpr.ts b/apps/api/src/routes/enterprise/gdpr.ts index 98493cd2..2430b93f 100644 --- a/apps/api/src/routes/enterprise/gdpr.ts +++ b/apps/api/src/routes/enterprise/gdpr.ts @@ -121,13 +121,23 @@ export async function registerGdprRoutes(app: FastifyInstance): Promise { // Validate the target user exists const [targetUser] = await db - .select({ id: schema.users.id }) + .select({ id: schema.users.id, role: schema.users.role }) .from(schema.users) .where(eq(schema.users.id, targetUserId)); if (!targetUser) { return reply.status(404).send({ error: "User not found" }); } + // Same authority gate the purge routes apply. An export archive carries the + // target's whole library and profile, so exporting an account above your own + // role is a disclosure path, not a read-only status call. + if (!(await canManageTargetRole(user, targetUser.role))) { + return reply.status(403).send({ + error: "Cannot manage a user beyond your role authority", + code: "ESCALATION_DENIED", + }); + } + // Create a durable job row const jobId = randomUUID(); await db.insert(schema.jobs).values({ @@ -178,9 +188,20 @@ export async function registerGdprRoutes(app: FastifyInstance): Promise { }); } - const { jobId } = request.params; + const { id: targetUserId, jobId } = request.params; - const [job] = await db.select().from(schema.jobs).where(eq(schema.jobs.id, jobId)); + // Scoped to the user named in the path and to the export job type, so this + // cannot be used to resolve an arbitrary job id into a download URL. + const [job] = await db + .select() + .from(schema.jobs) + .where( + and( + eq(schema.jobs.id, jobId), + eq(schema.jobs.userId, targetUserId), + eq(schema.jobs.toolId, "gdpr-export"), + ), + ); if (!job) { return reply.status(404).send({ error: "Export job not found" }); diff --git a/tests/unit/api/gdpr-route-authority.test.ts b/tests/unit/api/gdpr-route-authority.test.ts new file mode 100644 index 00000000..6ec3fa9a --- /dev/null +++ b/tests/unit/api/gdpr-route-authority.test.ts @@ -0,0 +1,189 @@ +/** + * Authority checks on the GDPR export routes. + * + * The purge routes have always gated on `canManageTargetRole`, so a compliance + * operator cannot act on an account above their own authority. Export needs the + * same gate: the archive carries the target's entire library plus their profile, + * so exporting an administrator is a data-disclosure path, not a read-only status + * call. These run against a bare Fastify instance because the real routes sit + * behind an enterprise licence that CI has no signing key to mint. + */ +import Fastify, { type FastifyInstance } from "fastify"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const requirePermissionMock = vi.hoisted(() => vi.fn()); +const canManageTargetRoleMock = vi.hoisted(() => vi.fn()); +const selectMock = vi.hoisted(() => vi.fn()); +const insertMock = vi.hoisted(() => vi.fn()); +const queueAddMock = vi.hoisted(() => vi.fn()); +const auditMock = vi.hoisted(() => vi.fn()); + +/** Rows handed back by the next `select(...).from(...).where(...)` chain. */ +let selectResults: unknown[][] = []; +/** Predicates passed to each `.where(...)`, in call order, for assertions. */ +let whereArgs: unknown[] = []; + +function nextChain() { + const rows = selectResults.shift() ?? []; + const chain = { + from: vi.fn(() => chain), + where: vi.fn((predicate: unknown) => { + whereArgs.push(predicate); + return Promise.resolve(rows); + }), + }; + return chain; +} + +/** Flatten the nested `and(eq(...), eq(...))` shape into its leaf comparisons. */ +function eqLeaves(predicate: unknown): Array<{ col: unknown; val: unknown }> { + const node = predicate as { op?: string; conds?: unknown[]; col?: unknown; val?: unknown }; + if (node?.op === "and") return (node.conds ?? []).flatMap(eqLeaves); + if (node?.op === "eq") return [{ col: node.col, val: node.val }]; + return []; +} + +async function buildApp(): Promise { + vi.resetModules(); + selectMock.mockReset(); + insertMock.mockReset(); + queueAddMock.mockReset(); + auditMock.mockReset(); + requirePermissionMock.mockReset(); + canManageTargetRoleMock.mockReset(); + + selectMock.mockImplementation(() => nextChain()); + insertMock.mockImplementation(() => ({ values: vi.fn(() => Promise.resolve()) })); + auditMock.mockImplementation(() => vi.fn(() => Promise.resolve())); + // Set after the resets above, otherwise buildApp() would wipe it and the route + // would bail at `if (!user) return`, answering 200 with an empty body. + requirePermissionMock.mockResolvedValue(COMPLIANCE_OPERATOR); + + vi.doMock("@snapotter/enterprise", () => ({ + isFeatureEnabled: () => true, + })); + + // Plain objects instead of SQL nodes so a test can read back which columns the + // query actually constrained. + vi.doMock("drizzle-orm", () => ({ + eq: (col: unknown, val: unknown) => ({ op: "eq", col, val }), + and: (...conds: unknown[]) => ({ op: "and", conds }), + inArray: (col: unknown, vals: unknown) => ({ op: "inArray", col, vals }), + })); + + vi.doMock("../../../apps/api/src/db/index.js", () => ({ + db: { select: selectMock, insert: insertMock, delete: vi.fn(), update: vi.fn() }, + schema: { + users: { id: "users.id", role: "users.role" }, + jobs: { id: "jobs.id", userId: "jobs.user_id", toolId: "jobs.tool_id" }, + }, + })); + + vi.doMock("../../../apps/api/src/permissions.js", () => ({ + requirePermission: () => requirePermissionMock, + canManageTargetRole: canManageTargetRoleMock, + })); + + vi.doMock("../../../apps/api/src/jobs/queues.js", () => ({ + getQueue: () => ({ add: queueAddMock }), + })); + vi.doMock("../../../apps/api/src/jobs/system-jobs.js", () => ({ + SYSTEM_JOBS: { gdprExport: "gdpr-export" }, + })); + vi.doMock("../../../apps/api/src/jobs/cancel.js", () => ({ requestCancel: vi.fn() })); + vi.doMock("../../../apps/api/src/lib/audit.js", () => ({ auditFromRequest: auditMock })); + vi.doMock("../../../apps/api/src/lib/file-storage.js", () => ({ + deleteStoredFile: vi.fn(), + deleteThumbnail: vi.fn(), + })); + vi.doMock("../../../apps/api/src/lib/object-storage.js", () => ({ deletePrefix: vi.fn() })); + + const { registerGdprRoutes } = await import("../../../apps/api/src/routes/enterprise/gdpr.js"); + const app = Fastify(); + await registerGdprRoutes(app); + await app.ready(); + return app; +} + +const COMPLIANCE_OPERATOR = { id: "operator-1", username: "compliance", role: "compliance" }; + +describe("GDPR export route authority", () => { + let app: FastifyInstance; + + beforeEach(() => { + selectResults = []; + whereArgs = []; + }); + + afterEach(async () => { + await app?.close(); + vi.restoreAllMocks(); + }); + + it("refuses to export a user whose role outranks the caller", async () => { + app = await buildApp(); + selectResults = [[{ id: "admin-1", role: "admin" }]]; + canManageTargetRoleMock.mockResolvedValue(false); + + const res = await app.inject({ + method: "POST", + url: "/api/v1/enterprise/users/admin-1/export", + }); + + expect(res.statusCode).toBe(403); + expect(res.json()).toMatchObject({ code: "ESCALATION_DENIED" }); + // The job must never be enqueued: the archive is the disclosure. + expect(queueAddMock).not.toHaveBeenCalled(); + expect(insertMock).not.toHaveBeenCalled(); + }); + + it("exports a user within the caller's authority", async () => { + app = await buildApp(); + selectResults = [[{ id: "user-9", role: "user" }]]; + canManageTargetRoleMock.mockResolvedValue(true); + + const res = await app.inject({ + method: "POST", + url: "/api/v1/enterprise/users/user-9/export", + }); + + expect(res.statusCode).toBe(202); + expect(queueAddMock).toHaveBeenCalledTimes(1); + }); + + it("still 404s an unknown target before consulting role authority", async () => { + app = await buildApp(); + selectResults = [[]]; + + const res = await app.inject({ + method: "POST", + url: "/api/v1/enterprise/users/ghost/export", + }); + + expect(res.statusCode).toBe(404); + expect(canManageTargetRoleMock).not.toHaveBeenCalled(); + }); + + it("scopes the export status lookup to the named user and the export job type", async () => { + app = await buildApp(); + // A completed job row exists. Without a scoped predicate the route would hand + // back its download URL for any job id in the system, whoever owns it. + selectResults = [ + [{ id: "job-1", status: "completed", outputRefs: ["outputs/job-1/gdpr-export.zip"] }], + ]; + + await app.inject({ + method: "GET", + url: "/api/v1/enterprise/users/victim-1/export/job-1", + }); + + const leaves = eqLeaves(whereArgs.at(-1)); + expect(leaves).toEqual( + expect.arrayContaining([ + { col: "jobs.id", val: "job-1" }, + { col: "jobs.user_id", val: "victim-1" }, + { col: "jobs.tool_id", val: "gdpr-export" }, + ]), + ); + }); +}); diff --git a/tests/unit/api/jobs/gdpr-export.behavior.test.ts b/tests/unit/api/jobs/gdpr-export.behavior.test.ts index 3b3535b1..32dadc90 100644 --- a/tests/unit/api/jobs/gdpr-export.behavior.test.ts +++ b/tests/unit/api/jobs/gdpr-export.behavior.test.ts @@ -5,10 +5,27 @@ const selectMock = vi.hoisted(() => vi.fn()); const readStoredFileMock = vi.hoisted(() => vi.fn()); const putObjectMock = vi.hoisted(() => vi.fn()); +/** + * Stands in for a Drizzle query builder, including its projection behavior: when + * `select()` is given a column map, only those keys come back. Without that, a + * mock would hand every column to the caller regardless of what was selected, and + * assertions like "the export omits passwordHash" would pass even if the query + * asked for it. + */ function queryChain(result: T) { const chain = { from: vi.fn(() => chain), - where: vi.fn(() => Promise.resolve(result)), + where: vi.fn(() => { + const callIndex = selectMock.mock.results.findIndex((r) => r.value === chain); + const columns = callIndex >= 0 ? selectMock.mock.calls[callIndex]?.[0] : undefined; + if (!columns || !Array.isArray(result)) return Promise.resolve(result); + const keys = Object.keys(columns as Record); + return Promise.resolve( + (result as Record[]).map((row) => + Object.fromEntries(keys.map((k) => [k, row[k]])), + ), + ); + }), }; return chain; } @@ -28,7 +45,26 @@ async function loadGdprExport() { select: selectMock, }, schema: { - users: { id: "users.id" }, + users: { + id: "users.id", + username: "users.username", + role: "users.role", + team: "users.team", + email: "users.email", + authProvider: "users.auth_provider", + externalId: "users.external_id", + mustChangePassword: "users.must_change_password", + legalHold: "users.legal_hold", + storageUsed: "users.storage_used", + storageQuota: "users.storage_quota", + totpEnabled: "users.totp_enabled", + createdAt: "users.created_at", + updatedAt: "users.updated_at", + // Credential material -- present on the table, never exportable. + passwordHash: "users.password_hash", + totpSecret: "users.totp_secret", + recoveryCodesHash: "users.recovery_codes_hash", + }, userFiles: { userId: "userFiles.userId" }, jobs: { userId: "jobs.userId" }, auditLog: { actorId: "auditLog.actorId" }, @@ -51,6 +87,33 @@ describe("GDPR export job behavior", () => { vi.restoreAllMocks(); }); + // A subject-access export is handed to the data subject, a regulator, or outside + // counsel, so it must never carry authentication material. Naming the columns keeps + // totpSecret and recoveryCodesHash inside Postgres instead of relying on the caller + // to subtract them, which also means a future column addition cannot silently leak. + it("selects an explicit profile column list that omits every credential column", async () => { + const { gdprExportJob } = await loadGdprExport(); + selectMock + .mockReturnValueOnce(queryChain([{ id: "user-1", email: "ada@example.test" }])) + .mockReturnValueOnce(queryChain([])) + .mockReturnValueOnce(queryChain([])) + .mockReturnValueOnce(queryChain([])); + + await gdprExportJob("user-1", "export-columns"); + + const profileColumns = selectMock.mock.calls[0][0]; + expect(profileColumns, "profile select must name its columns rather than select *").toBeTypeOf( + "object", + ); + + const selected = Object.keys(profileColumns as Record); + expect(selected).not.toContain("passwordHash"); + expect(selected).not.toContain("totpSecret"); + expect(selected).not.toContain("recoveryCodesHash"); + // Personal data the subject is genuinely owed still ships. + expect(selected).toEqual(expect.arrayContaining(["id", "username", "email", "createdAt"])); + }); + it("throws before writing output when the user does not exist", async () => { const { gdprExportJob } = await loadGdprExport(); selectMock.mockReturnValueOnce(queryChain([]));