feat(documents): path-style download URLs for rep-facing readability
Storage paths stay UUID-flat per the established CRM pattern (every other content type — brochures, berth PDFs, invoices, reports, templates, expense receipts — uses the same shape). The new catch-all /api/v1/documents/[id]/download/[...slug] route serves files keyed on doc id but rebuilds the slug from current state and 404s on mismatch — a hand-edited or stale link can't render the wrong filename or fold a wrong-folder path into a forwarded URL. URLs in shared links / browser tabs read like 'Deals 2026/Q1/contract.pdf' even though storage keys remain UUIDs. listDocuments + getDocumentById now hydrate a `downloadUrl` field per row (null when no file is attached yet) so UI consumers don't reconstruct paths. Filename is batch-fetched via files-table join to keep the query builder shape unchanged. Tests: 5 integration cases — happy-path stream, wrong-folder slug, wrong-filename slug, orphaned doc (no fileId), cross-port (tenancy isolation). Storage backend swapped to a real FilesystemBackend in a tempdir so the byte-streaming path is exercised end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
98
src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts
Normal file
98
src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts
Normal file
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* Route handler for `GET /api/v1/documents/[id]/download/[...slug]`.
|
||||
*
|
||||
* Lives in handlers.ts (not route.ts) so integration tests can call it
|
||||
* directly, bypassing auth/permission middleware (per CLAUDE.md "Route
|
||||
* handler exports" convention).
|
||||
*
|
||||
* Lookup is keyed off the doc id; the slug embeds the current folder path +
|
||||
* filename so a forwarded link reads like `Deals 2026/Q1/contract.pdf` even
|
||||
* though the underlying storage key is a UUID. The slug is rebuilt from
|
||||
* current state and compared with the supplied path — a stale or
|
||||
* hand-edited URL 404s rather than silently serving the wrong file.
|
||||
*/
|
||||
|
||||
import { Readable } from 'node:stream';
|
||||
|
||||
import { NextResponse } from 'next/server';
|
||||
import { and, eq } from 'drizzle-orm';
|
||||
|
||||
import { type RouteHandler } from '@/lib/api/helpers';
|
||||
import { db } from '@/lib/db';
|
||||
import { documents, files } from '@/lib/db/schema/documents';
|
||||
import { errorResponse, NotFoundError } from '@/lib/errors';
|
||||
import { getStorageBackend } from '@/lib/storage';
|
||||
import { listTree, type FolderNode } from '@/lib/services/document-folders.service';
|
||||
import { findFolderPath } from '@/lib/services/documents.service';
|
||||
|
||||
type DownloadParams = { id: string; slug: string[] };
|
||||
|
||||
export const downloadHandler: RouteHandler<DownloadParams> = async (_req, ctx, params) => {
|
||||
try {
|
||||
const docId = params.id;
|
||||
if (!docId) throw new NotFoundError('Document');
|
||||
|
||||
const doc = await db
|
||||
.select({
|
||||
id: documents.id,
|
||||
folderId: documents.folderId,
|
||||
fileId: documents.fileId,
|
||||
fileStoragePath: files.storagePath,
|
||||
fileMimeType: files.mimeType,
|
||||
fileFilename: files.filename,
|
||||
})
|
||||
.from(documents)
|
||||
.leftJoin(files, eq(files.id, documents.fileId))
|
||||
.where(and(eq(documents.id, docId), eq(documents.portId, ctx.portId)))
|
||||
.limit(1)
|
||||
.then((r) => r[0]);
|
||||
|
||||
if (!doc) throw new NotFoundError('Document');
|
||||
if (!doc.fileStoragePath || !doc.fileFilename) {
|
||||
throw new NotFoundError('Document file');
|
||||
}
|
||||
|
||||
const tree = await listTree(ctx.portId);
|
||||
const expected = buildExpectedSlug(doc.folderId, doc.fileFilename, tree);
|
||||
const supplied = (params.slug ?? []).map(decodeURIComponent).join('/');
|
||||
if (supplied !== expected) throw new NotFoundError('Document at this path');
|
||||
|
||||
const backend = await getStorageBackend();
|
||||
const nodeStream = await backend.get(doc.fileStoragePath);
|
||||
const webStream = Readable.toWeb(nodeStream as Readable) as ReadableStream;
|
||||
|
||||
return new NextResponse(webStream, {
|
||||
status: 200,
|
||||
headers: {
|
||||
'content-type': doc.fileMimeType ?? 'application/octet-stream',
|
||||
'content-disposition': `inline; filename="${sanitizeFilenameForHeader(doc.fileFilename)}"`,
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
return errorResponse(error);
|
||||
}
|
||||
};
|
||||
|
||||
function buildExpectedSlug(
|
||||
folderId: string | null,
|
||||
filename: string,
|
||||
tree: FolderNode[],
|
||||
): string {
|
||||
const segments: string[] = [];
|
||||
if (folderId) {
|
||||
const path = findFolderPath(tree, folderId);
|
||||
for (const node of path) segments.push(node.name);
|
||||
}
|
||||
segments.push(filename);
|
||||
return segments.join('/');
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip control + quote characters from a filename before placing it in the
|
||||
* Content-Disposition header value. The legacy `inline; filename="..."` form
|
||||
* is what the rest of the CRM emits today; this is the same minimal
|
||||
* sanitization the other download endpoints rely on.
|
||||
*/
|
||||
function sanitizeFilenameForHeader(filename: string): string {
|
||||
return filename.replace(/["\\\r\n]/g, '_');
|
||||
}
|
||||
@@ -0,0 +1,5 @@
|
||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
||||
|
||||
import { downloadHandler } from './handlers';
|
||||
|
||||
export const GET = withAuth(withPermission('documents', 'view', downloadHandler));
|
||||
@@ -34,7 +34,11 @@ import {
|
||||
voidDocument as documensoVoid,
|
||||
} from '@/lib/services/documenso-client';
|
||||
import { getPortEoiSigners } from '@/lib/services/documenso-payload';
|
||||
import { listTree, collectDescendantIds } from '@/lib/services/document-folders.service';
|
||||
import {
|
||||
listTree,
|
||||
collectDescendantIds,
|
||||
type FolderNode,
|
||||
} from '@/lib/services/document-folders.service';
|
||||
import type {
|
||||
CreateDocumentInput,
|
||||
UpdateDocumentInput,
|
||||
@@ -217,7 +221,7 @@ export async function listDocuments(
|
||||
? documents.documentType
|
||||
: documents.createdAt;
|
||||
|
||||
return buildListQuery({
|
||||
const result = await buildListQuery<typeof documents.$inferSelect>({
|
||||
table: documents,
|
||||
portIdColumn: documents.portId,
|
||||
portId,
|
||||
@@ -230,6 +234,85 @@ export async function listDocuments(
|
||||
page,
|
||||
pageSize: limit,
|
||||
});
|
||||
|
||||
const hydrated = await hydrateDocumentsWithDownloadUrl(portId, result.data);
|
||||
return { ...result, data: hydrated };
|
||||
}
|
||||
|
||||
// ─── Download URL helpers ─────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Resolve the rep-facing download URL for a document. The URL embeds the
|
||||
* folder path + filename for browser-tab / shared-link readability, but the
|
||||
* route handler keys lookup off the doc id and validates the slug for truth
|
||||
* — a hand-edited URL with the wrong path 404s instead of silently serving
|
||||
* the wrong file.
|
||||
*
|
||||
* Pass the resolved folder tree once per request and call this for each doc
|
||||
* so we don't refetch the tree per row. Returns `null` when the document has
|
||||
* no attached file (signature-only docs pre-completion); UI consumers branch
|
||||
* on that to decide whether to render the download affordance.
|
||||
*/
|
||||
export function buildDocumentDownloadUrl(
|
||||
doc: { id: string; folderId: string | null; filename: string | null },
|
||||
folderTree: readonly FolderNode[],
|
||||
): string | null {
|
||||
if (!doc.filename) return null;
|
||||
const segments: string[] = [];
|
||||
if (doc.folderId) {
|
||||
const path = findFolderPath(folderTree, doc.folderId);
|
||||
for (const node of path) segments.push(encodeURIComponent(node.name));
|
||||
}
|
||||
segments.push(encodeURIComponent(doc.filename));
|
||||
return `/api/v1/documents/${doc.id}/download/${segments.join('/')}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Walk the folder tree to materialize the ancestor chain that ends at
|
||||
* `id`. Returns roots-first; empty when the id is missing (orphan
|
||||
* `folder_id` pointer — see listTree's intentional silent drop).
|
||||
*/
|
||||
export function findFolderPath(tree: readonly FolderNode[], id: string): FolderNode[] {
|
||||
for (const node of tree) {
|
||||
if (node.id === id) return [node];
|
||||
const inChild = findFolderPath(node.children, id);
|
||||
if (inChild.length > 0) return [node, ...inChild];
|
||||
}
|
||||
return [];
|
||||
}
|
||||
|
||||
type DocumentRow = typeof documents.$inferSelect;
|
||||
type DocumentRowWithDownload = DocumentRow & { downloadUrl: string | null };
|
||||
|
||||
async function hydrateDocumentsWithDownloadUrl(
|
||||
portId: string,
|
||||
rows: DocumentRow[],
|
||||
): Promise<DocumentRowWithDownload[]> {
|
||||
if (rows.length === 0) return [];
|
||||
|
||||
const fileIds = Array.from(
|
||||
new Set(rows.map((r) => r.fileId).filter((v): v is string => v !== null)),
|
||||
);
|
||||
const filenameById = new Map<string, string | null>();
|
||||
if (fileIds.length > 0) {
|
||||
const fileRows = await db
|
||||
.select({ id: files.id, filename: files.filename })
|
||||
.from(files)
|
||||
.where(and(inArray(files.id, fileIds), eq(files.portId, portId)));
|
||||
for (const f of fileRows) filenameById.set(f.id, f.filename);
|
||||
}
|
||||
|
||||
const tree = await listTree(portId);
|
||||
return rows.map((row) => {
|
||||
const filename = row.fileId ? (filenameById.get(row.fileId) ?? null) : null;
|
||||
return {
|
||||
...row,
|
||||
downloadUrl: buildDocumentDownloadUrl(
|
||||
{ id: row.id, folderId: row.folderId, filename },
|
||||
tree,
|
||||
),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
// ─── Deal docs for a berth ────────────────────────────────────────────────────
|
||||
@@ -344,7 +427,23 @@ export async function getDocumentById(id: string, portId: string) {
|
||||
});
|
||||
|
||||
if (!doc) throw new NotFoundError('Document');
|
||||
return doc;
|
||||
|
||||
let filename: string | null = null;
|
||||
if (doc.fileId) {
|
||||
const fileRow = await db
|
||||
.select({ filename: files.filename })
|
||||
.from(files)
|
||||
.where(and(eq(files.id, doc.fileId), eq(files.portId, portId)))
|
||||
.limit(1)
|
||||
.then((r) => r[0]);
|
||||
filename = fileRow?.filename ?? null;
|
||||
}
|
||||
const tree = await listTree(portId);
|
||||
const downloadUrl = buildDocumentDownloadUrl(
|
||||
{ id: doc.id, folderId: doc.folderId, filename },
|
||||
tree,
|
||||
);
|
||||
return { ...doc, downloadUrl };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
213
tests/integration/document-path-style-download.test.ts
Normal file
213
tests/integration/document-path-style-download.test.ts
Normal file
@@ -0,0 +1,213 @@
|
||||
/**
|
||||
* Integration test: GET /api/v1/documents/[id]/download/[...slug]
|
||||
*
|
||||
* Verifies the slug truth-check and tenancy isolation on the path-style
|
||||
* download route. The storage backend is swapped for a real FilesystemBackend
|
||||
* rooted in a tempdir so the byte-streaming path is exercised end-to-end.
|
||||
*/
|
||||
|
||||
import { mkdtemp, rm } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { db } from '@/lib/db';
|
||||
import { documents, files } from '@/lib/db/schema/documents';
|
||||
import { user } from '@/lib/db/schema/users';
|
||||
import { createFolder } from '@/lib/services/document-folders.service';
|
||||
import { makeMockCtx } from '../helpers/route-tester';
|
||||
import { makeFullPermissions, makePort } from '../helpers/factories';
|
||||
|
||||
let testUserId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
const [u] = await db.select({ id: user.id }).from(user).limit(1);
|
||||
testUserId = u!.id;
|
||||
});
|
||||
|
||||
describe('GET /api/v1/documents/[id]/download/[...slug]', () => {
|
||||
let storageRoot: string;
|
||||
let backend: import('@/lib/storage/filesystem').FilesystemBackend;
|
||||
|
||||
beforeEach(async () => {
|
||||
storageRoot = await mkdtemp(path.join(tmpdir(), 'pn-doc-dl-'));
|
||||
const { FilesystemBackend } = await import('@/lib/storage/filesystem');
|
||||
backend = await FilesystemBackend.create({
|
||||
root: storageRoot,
|
||||
proxyHmacSecretEncrypted: null,
|
||||
});
|
||||
vi.doMock('@/lib/storage', async () => {
|
||||
const real = await vi.importActual<typeof import('@/lib/storage')>('@/lib/storage');
|
||||
return { ...real, getStorageBackend: vi.fn(async () => backend) };
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.doUnmock('@/lib/storage');
|
||||
await rm(storageRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function setupDoc(opts: {
|
||||
portId: string;
|
||||
folderId: string | null;
|
||||
filename: string;
|
||||
body: Buffer;
|
||||
storagePath: string;
|
||||
}) {
|
||||
await backend.put(opts.storagePath, opts.body, {
|
||||
contentType: 'application/pdf',
|
||||
sizeBytes: opts.body.length,
|
||||
});
|
||||
const [fileRow] = await db
|
||||
.insert(files)
|
||||
.values({
|
||||
portId: opts.portId,
|
||||
filename: opts.filename,
|
||||
originalName: opts.filename,
|
||||
mimeType: 'application/pdf',
|
||||
sizeBytes: String(opts.body.length),
|
||||
storagePath: opts.storagePath,
|
||||
uploadedBy: testUserId,
|
||||
})
|
||||
.returning();
|
||||
const [docRow] = await db
|
||||
.insert(documents)
|
||||
.values({
|
||||
portId: opts.portId,
|
||||
documentType: 'other',
|
||||
title: opts.filename,
|
||||
createdBy: testUserId,
|
||||
folderId: opts.folderId,
|
||||
fileId: fileRow!.id,
|
||||
})
|
||||
.returning();
|
||||
return { fileRow: fileRow!, docRow: docRow! };
|
||||
}
|
||||
|
||||
it('streams the file when the slug matches the current folder path + filename', async () => {
|
||||
const port = await makePort();
|
||||
const folder = await createFolder(port.id, testUserId, { name: 'Deals 2026', parentId: null });
|
||||
const sub = await createFolder(port.id, testUserId, { name: 'Q1', parentId: folder.id });
|
||||
const body = Buffer.from('hello world');
|
||||
const { docRow } = await setupDoc({
|
||||
portId: port.id,
|
||||
folderId: sub.id,
|
||||
filename: 'contract.pdf',
|
||||
body,
|
||||
storagePath: 'test/contract.pdf',
|
||||
});
|
||||
|
||||
const { downloadHandler } = await import(
|
||||
'@/app/api/v1/documents/[id]/download/[...slug]/handlers'
|
||||
);
|
||||
const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() });
|
||||
const req = new Request('http://localhost/api/v1/documents/x/download/whatever') as never;
|
||||
const res = await downloadHandler(req, ctx, {
|
||||
id: docRow.id,
|
||||
slug: ['Deals 2026', 'Q1', 'contract.pdf'],
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
const buf = Buffer.from(await res.arrayBuffer());
|
||||
expect(buf.toString()).toBe('hello world');
|
||||
expect(res.headers.get('content-type')).toBe('application/pdf');
|
||||
});
|
||||
|
||||
it('404s when the folder-path segments do not match current state', async () => {
|
||||
const port = await makePort();
|
||||
const folder = await createFolder(port.id, testUserId, { name: 'Real Folder', parentId: null });
|
||||
const { docRow } = await setupDoc({
|
||||
portId: port.id,
|
||||
folderId: folder.id,
|
||||
filename: 'spec.pdf',
|
||||
body: Buffer.from('x'),
|
||||
storagePath: 'test/spec.pdf',
|
||||
});
|
||||
|
||||
const { downloadHandler } = await import(
|
||||
'@/app/api/v1/documents/[id]/download/[...slug]/handlers'
|
||||
);
|
||||
const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() });
|
||||
const req = new Request('http://localhost/x') as never;
|
||||
const res = await downloadHandler(req, ctx, {
|
||||
id: docRow.id,
|
||||
slug: ['Wrong Folder', 'spec.pdf'],
|
||||
});
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it('404s when the filename segment does not match current state', async () => {
|
||||
const port = await makePort();
|
||||
const folder = await createFolder(port.id, testUserId, { name: 'F', parentId: null });
|
||||
const { docRow } = await setupDoc({
|
||||
portId: port.id,
|
||||
folderId: folder.id,
|
||||
filename: 'real.pdf',
|
||||
body: Buffer.from('x'),
|
||||
storagePath: 'test/real.pdf',
|
||||
});
|
||||
|
||||
const { downloadHandler } = await import(
|
||||
'@/app/api/v1/documents/[id]/download/[...slug]/handlers'
|
||||
);
|
||||
const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() });
|
||||
const req = new Request('http://localhost/x') as never;
|
||||
const res = await downloadHandler(req, ctx, {
|
||||
id: docRow.id,
|
||||
slug: ['F', 'wrong.pdf'],
|
||||
});
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it('404s when the document has no attached file (orphaned doc)', async () => {
|
||||
const port = await makePort();
|
||||
const [docRow] = await db
|
||||
.insert(documents)
|
||||
.values({
|
||||
portId: port.id,
|
||||
documentType: 'other',
|
||||
title: 'No file',
|
||||
createdBy: testUserId,
|
||||
folderId: null,
|
||||
})
|
||||
.returning();
|
||||
|
||||
const { downloadHandler } = await import(
|
||||
'@/app/api/v1/documents/[id]/download/[...slug]/handlers'
|
||||
);
|
||||
const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() });
|
||||
const req = new Request('http://localhost/x') as never;
|
||||
const res = await downloadHandler(req, ctx, {
|
||||
id: docRow!.id,
|
||||
slug: ['anything.pdf'],
|
||||
});
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it('404s when the document belongs to another port (tenant isolation)', async () => {
|
||||
const portA = await makePort();
|
||||
const portB = await makePort();
|
||||
const folder = await createFolder(portA.id, testUserId, {
|
||||
name: 'Cross',
|
||||
parentId: null,
|
||||
});
|
||||
const { docRow } = await setupDoc({
|
||||
portId: portA.id,
|
||||
folderId: folder.id,
|
||||
filename: 'a.pdf',
|
||||
body: Buffer.from('x'),
|
||||
storagePath: 'test/a.pdf',
|
||||
});
|
||||
|
||||
const { downloadHandler } = await import(
|
||||
'@/app/api/v1/documents/[id]/download/[...slug]/handlers'
|
||||
);
|
||||
const ctx = makeMockCtx({ portId: portB.id, permissions: makeFullPermissions() });
|
||||
const req = new Request('http://localhost/x') as never;
|
||||
const res = await downloadHandler(req, ctx, {
|
||||
id: docRow.id,
|
||||
slug: ['Cross', 'a.pdf'],
|
||||
});
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user