mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-21 06:11:45 +00:00
fix(backups,files): auto-backups rejected by validator; trip file download broken after cookie migration
Fixes #773: isValidBackupFilename regex anchored to ^backup- rejected all auto-backup-* filenames, causing 400 on download/restore/delete. Broadened to ^(?:auto-)?backup-. Fixes #774: three regressions in the trip Files tab — - openFile import shadowed by a local function of the same name inside FileManager; PDF preview modal was calling the local with a URL string, corrupting state and crashing on the second click (mime_type read on undefined). Fixed by aliasing the import as openFileUrl. - GET /:id/download used a bespoke authenticateDownload that checked only Bearer header and ?token= query param, ignoring the trek_session cookie. After the JWT-to-cookie migration the client sends cookies only, so every download silently 401-ed. Extended authenticateDownload to accept req and check cookie → Bearer → query token in priority order. - files.download and files.openError translation keys were missing from all 15 locale files; t() was returning the raw key as a truthy string, defeating the || 'Download' fallback.
This commit is contained in:
@@ -77,15 +77,11 @@ const upload = multer({
|
||||
// Routes
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Authenticated file download (supports Bearer header or ?token= query param)
|
||||
// Authenticated file download (supports cookie, Bearer header, or ?token= query param)
|
||||
router.get('/:id/download', (req: Request, res: Response) => {
|
||||
const { tripId, id } = req.params;
|
||||
|
||||
const authHeader = req.headers['authorization'];
|
||||
const bearerToken = authHeader && authHeader.split(' ')[1];
|
||||
const queryToken = req.query.token as string | undefined;
|
||||
|
||||
const auth = authenticateDownload(bearerToken, queryToken);
|
||||
const auth = authenticateDownload(req);
|
||||
if ('error' in auth) return res.status(auth.status).json({ error: auth.error });
|
||||
|
||||
const trip = verifyTripAccess(tripId, auth.userId);
|
||||
|
||||
@@ -62,7 +62,7 @@ export function parseAutoBackupBody(body: Record<string, unknown>): {
|
||||
}
|
||||
|
||||
export function isValidBackupFilename(filename: string): boolean {
|
||||
return /^backup-[\w\-]+\.zip$/.test(filename);
|
||||
return /^(?:auto-)?backup-[\w-]+\.zip$/.test(filename);
|
||||
}
|
||||
|
||||
export function backupFilePath(filename: string): string {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import path from 'path';
|
||||
import fs from 'fs';
|
||||
import type { Request } from 'express';
|
||||
import { db, canAccessTrip } from '../db/database';
|
||||
import { consumeEphemeralToken } from './ephemeralTokens';
|
||||
import { verifyJwtAndLoadUser } from '../middleware/auth';
|
||||
@@ -72,23 +73,30 @@ export function resolveFilePath(filename: string): { resolved: string; safe: boo
|
||||
// Token-based download auth
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export function authenticateDownload(bearerToken: string | undefined, queryToken: string | undefined): { userId: number } | { error: string; status: number } {
|
||||
if (!bearerToken && !queryToken) {
|
||||
return { error: 'Authentication required', status: 401 };
|
||||
}
|
||||
export function authenticateDownload(req: Request): { userId: number } | { error: string; status: number } {
|
||||
const cookieToken = (req as any).cookies?.trek_session as string | undefined;
|
||||
const authHeader = req.headers['authorization'];
|
||||
const bearerToken = authHeader ? (authHeader.split(' ')[1] || undefined) : undefined;
|
||||
const queryToken = req.query.token as string | undefined;
|
||||
|
||||
if (bearerToken) {
|
||||
// Cookie and Bearer both carry a full JWT — try them first (cookie wins).
|
||||
const jwtToken = cookieToken || bearerToken;
|
||||
if (jwtToken) {
|
||||
// Use the shared helper so the password_version gate applies here too;
|
||||
// previously this bypassed the check and stolen download tokens stayed
|
||||
// valid across a password reset.
|
||||
const user = verifyJwtAndLoadUser(bearerToken);
|
||||
const user = verifyJwtAndLoadUser(jwtToken);
|
||||
if (!user) return { error: 'Invalid or expired token', status: 401 };
|
||||
return { userId: user.id };
|
||||
}
|
||||
|
||||
const uid = consumeEphemeralToken(queryToken!, 'download');
|
||||
if (!uid) return { error: 'Invalid or expired token', status: 401 };
|
||||
return { userId: uid };
|
||||
if (queryToken) {
|
||||
const uid = consumeEphemeralToken(queryToken, 'download');
|
||||
if (!uid) return { error: 'Invalid or expired token', status: 401 };
|
||||
return { userId: uid };
|
||||
}
|
||||
|
||||
return { error: 'Authentication required', status: 401 };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -365,13 +365,12 @@ describe('File download', () => {
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it('FILE-008 — GET /:id/download with Bearer JWT downloads or 404s (no physical file in tests)', async () => {
|
||||
it('FILE-008 — GET /:id/download with Bearer JWT downloads file', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const upload = await uploadFile(trip.id, user.id, FIXTURE_PDF);
|
||||
const fileId = upload.body.file.id;
|
||||
|
||||
// authenticateDownload accepts a signed JWT as Bearer token
|
||||
const token = generateToken(user.id);
|
||||
|
||||
const dl = await request(app)
|
||||
@@ -380,4 +379,18 @@ describe('File download', () => {
|
||||
// multer stores the file to disk during uploadFile — physical file exists
|
||||
expect(dl.status).toBe(200);
|
||||
});
|
||||
|
||||
it('FILE-011 — GET /:id/download with trek_session cookie downloads file', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const upload = await uploadFile(trip.id, user.id, FIXTURE_PDF);
|
||||
const fileId = upload.body.file.id;
|
||||
|
||||
const token = generateToken(user.id);
|
||||
|
||||
const dl = await request(app)
|
||||
.get(`/api/trips/${trip.id}/files/${fileId}/download`)
|
||||
.set('Cookie', `trek_session=${token}`);
|
||||
expect(dl.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -234,6 +234,22 @@ describe('BACKUP-034 isValidBackupFilename', () => {
|
||||
it('accepts filename with hyphens and underscores', () => {
|
||||
expect(isValidBackupFilename('backup-my_trek-2026.zip')).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts auto-backup filename', () => {
|
||||
expect(isValidBackupFilename('auto-backup-2026-04-21T00-00-00.zip')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects auto-backup with empty body', () => {
|
||||
expect(isValidBackupFilename('auto-backup-.zip')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects backup with empty body', () => {
|
||||
expect(isValidBackupFilename('backup-.zip')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects arbitrary auto- prefix that is not auto-backup', () => {
|
||||
expect(isValidBackupFilename('auto-notbackup-2026.zip')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user