From fc5338fa8a2d01a2f21eda5741254c6a1738df7a Mon Sep 17 00:00:00 2001 From: ali Date: Fri, 12 Jun 2026 19:31:58 +0330 Subject: [PATCH] fix: require admin session to download AdminJS PII exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GET /admin/download/:filename route serves project/donor email-address CSV exports (PII) from src/server/adminJs/tabs/exports. It is registered as a plain Express route, before and outside the AdminJS authenticated router, so it never inherited any auth check — anyone on the internet could download the files. Extract the handler into createDownloadAdminJsExportHandler, which now requires a valid AdminJS admin session (via getCurrentAdminJsSession) and returns 401 otherwise. getCurrentAdminJsSession throws when the request has no cookie header, so both thrown errors and falsy results are treated as unauthenticated. The existing path-traversal protection is preserved unchanged. The session resolver is injected so the handler can be unit-tested without Redis/DB; adds tests for the no-session, throwing-session, and valid-admin cases. Co-Authored-By: Claude Opus 4.8 --- src/server/adminJsExportDownload.test.ts | 70 ++++++++++++++++++++++++ src/server/adminJsExportDownload.ts | 58 ++++++++++++++++++++ src/server/bootstrap.ts | 31 +++++------ 3 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 src/server/adminJsExportDownload.test.ts create mode 100644 src/server/adminJsExportDownload.ts diff --git a/src/server/adminJsExportDownload.test.ts b/src/server/adminJsExportDownload.test.ts new file mode 100644 index 000000000..67ef2c7ca --- /dev/null +++ b/src/server/adminJsExportDownload.test.ts @@ -0,0 +1,70 @@ +import { assert } from 'chai'; +import sinon from 'sinon'; +import { Request, Response } from 'express'; +import { createDownloadAdminJsExportHandler } from './adminJsExportDownload'; + +describe( + 'createDownloadAdminJsExportHandler() test cases', + downloadAdminJsExportTestCases, +); + +function downloadAdminJsExportTestCases() { + const buildReq = (filename: string): Request => + ({ params: { filename }, headers: {} }) as unknown as Request; + + const buildRes = () => { + const res: any = {}; + res.statusCode = undefined; + res.status = sinon.stub().callsFake((code: number) => { + res.statusCode = code; + return res; + }); + res.send = sinon.stub().returnsThis(); + res.download = sinon.stub(); + return res as Response & { + status: sinon.SinonStub; + send: sinon.SinonStub; + download: sinon.SinonStub; + }; + }; + + it('responds 401 and serves no file when there is no admin session', async () => { + const handler = createDownloadAdminJsExportHandler( + sinon.stub().resolves(false), + ); + const res = buildRes(); + + await handler(buildReq('emails.csv'), res); + + assert.isTrue(res.status.calledOnceWith(401)); + assert.isTrue(res.send.calledOnceWith('Unauthorized')); + assert.isFalse(res.download.called); + }); + + it('responds 401 when resolving the session throws (e.g. no cookie header)', async () => { + // getCurrentAdminJsSession throws TypeError when there is no cookie header. + const handler = createDownloadAdminJsExportHandler( + sinon.stub().rejects(new TypeError('argument str must be a string')), + ); + const res = buildRes(); + + await handler(buildReq('emails.csv'), res); + + assert.isTrue(res.status.calledOnceWith(401)); + assert.isFalse(res.download.called); + }); + + it('serves the requested export from the exports dir for a valid admin session', async () => { + const handler = createDownloadAdminJsExportHandler( + sinon.stub().resolves({ id: 1 }), + ); + const res = buildRes(); + + await handler(buildReq('emails.csv'), res); + + assert.isFalse(res.status.calledWith(401)); + assert.isTrue(res.download.calledOnce); + const servedPath = res.download.firstCall.args[0] as string; + assert.match(servedPath, /adminJs\/tabs\/exports\/emails\.csv$/); + }); +} diff --git a/src/server/adminJsExportDownload.ts b/src/server/adminJsExportDownload.ts new file mode 100644 index 000000000..587702501 --- /dev/null +++ b/src/server/adminJsExportDownload.ts @@ -0,0 +1,58 @@ +import path from 'path'; +import { Request, Response } from 'express'; +import { logger } from '../utils/logger'; +import type { IncomingMessage } from 'connect'; + +/** + * Resolves the current AdminJS admin session for a request (from its signed + * session cookie). Mirrors the signature of `getCurrentAdminJsSession` in + * `./adminJs/adminJs`; a truthy value means "authenticated admin", `false` + * means "no valid session". It is injected (rather than imported directly) so + * this module — and its unit test — stay decoupled from the AdminJS/Redis + * machinery. + */ +export type AdminJsSessionResolver = (req: IncomingMessage) => Promise; + +// AdminJS-generated CSV exports live here (e.g. project/donor email-address +// lists). Resolved relative to this file so it matches where projectsTab +// writes them (src/server/adminJs/tabs/exports). +const exportsDir = path.join(__dirname, '/adminJs/tabs/exports'); + +/** + * Builds the handler for `GET /admin/download/:filename`, which serves the + * AdminJS CSV exports created from the projects tab. + * + * Those exports contain PII (project/donor email addresses). The route is + * registered as a plain Express route, OUTSIDE the AdminJS authenticated + * router, so it does NOT inherit AdminJS auth. Without the explicit session + * check below, anyone could download the files. We therefore require a valid + * admin session and reject everything else with 401. + */ +export const createDownloadAdminJsExportHandler = ( + getCurrentAdminJsSession: AdminJsSessionResolver, +) => { + return async (req: Request, res: Response): Promise => { + // Require a valid admin session. `getCurrentAdminJsSession` throws when the + // request carries no cookie header, so treat any failure (thrown or falsy) + // as "not authenticated". + let isAuthenticatedAdmin = false; + try { + isAuthenticatedAdmin = Boolean(await getCurrentAdminJsSession(req)); + } catch (e) { + logger.error('admin export download auth check failed', e); + } + if (!isAuthenticatedAdmin) { + res.status(401).send('Unauthorized'); + return; + } + + // Prevent path traversal: reduce to a bare filename (strips any `../`), + // then confirm the resolved path is directly inside the exports dir. + const filePath = path.join(exportsDir, path.basename(req.params.filename)); + if (path.dirname(filePath) !== exportsDir) { + res.status(400).send('Invalid filename'); + return; + } + res.download(filePath); + }; +}; diff --git a/src/server/bootstrap.ts b/src/server/bootstrap.ts index f59e37209..431ce7c9a 100644 --- a/src/server/bootstrap.ts +++ b/src/server/bootstrap.ts @@ -1,6 +1,5 @@ // @ts-check import http from 'http'; -import path from 'path'; import { Resource } from '@adminjs/typeorm'; import { ApolloServer } from '@apollo/server'; import { ApolloServerPluginLandingPageGraphQLPlayground } from '@apollo/server-plugin-landing-page-graphql-playground'; @@ -37,7 +36,12 @@ import { import { logger } from '../utils/logger'; import { flushSentryAndExit } from '../utils/globalErrorHandlers'; import { isTrustedVercelRequest } from '../utils/ipWhitelist'; -import { adminJsRootPath, getAdminJsRouter } from './adminJs/adminJs'; +import { + adminJsRootPath, + getAdminJsRouter, + getCurrentAdminJsSession, +} from './adminJs/adminJs'; +import { createDownloadAdminJsExportHandler } from './adminJsExportDownload'; // import { apiGivRouter } from '../routers/apiGivRoutes'; import { AppDataSource, CronDataSource } from '../orm'; import { @@ -209,21 +213,14 @@ export async function bootstrap() { limit: (config.get('UPLOAD_FILE_MAX_SIZE') as number) || '5mb', }); - // To download email addresses of projects in AdminJS projects tab - app.get('/admin/download/:filename', (req, res) => { - const exportsDir = path.join(__dirname, '/adminJs/tabs/exports'); - // Prevent path traversal: reduce to a bare filename (strips any `../`), - // then confirm the resolved path is directly inside the exports dir. - const filePath = path.join( - exportsDir, - path.basename(req.params.filename), - ); - if (path.dirname(filePath) !== exportsDir) { - res.status(400).send('Invalid filename'); - return; - } - res.download(filePath); - }); + // Download email-address CSV exports generated from the AdminJS projects + // tab. These contain PII and this route lives outside the AdminJS + // authenticated router, so the handler enforces a valid admin session + // itself (see createDownloadAdminJsExportHandler). + app.get( + '/admin/download/:filename', + createDownloadAdminJsExportHandler(getCurrentAdminJsSession), + ); // Lightweight "hello world" health check for deploy verification. // Defined BEFORE global CORS middleware so it's always reachable from any origin.