Merge pull request #777 from mauriceboe/fix/issues-773-774-backups-and-trip-files

fix(backups,files): auto-backups rejected by validator; trip file download broken after cookie migration
This commit is contained in:
Julien G.
2026-04-21 11:24:44 +02:00
committed by GitHub
21 changed files with 85 additions and 22 deletions
+4 -4
View File
@@ -10,7 +10,7 @@ import { useCanDo } from '../../store/permissionsStore'
import { useTripStore } from '../../store/tripStore' import { useTripStore } from '../../store/tripStore'
import { getAuthUrl } from '../../api/authUrl' import { getAuthUrl } from '../../api/authUrl'
import { downloadFile, openFile } from '../../utils/fileDownload' import { downloadFile, openFile as openFileUrl } from '../../utils/fileDownload'
function isImage(mimeType) { function isImage(mimeType) {
if (!mimeType) return false if (!mimeType) return false
@@ -113,7 +113,7 @@ function ImageLightbox({ files, initialIndex, onClose }: ImageLightboxProps) {
</span> </span>
<div style={{ display: 'flex', gap: 8, flexShrink: 0 }}> <div style={{ display: 'flex', gap: 8, flexShrink: 0 }}>
<button <button
onClick={() => openFile(file.url).catch(() => {})} onClick={() => openFileUrl(file.url, file.original_name).catch(() => {})}
style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'rgba(255,255,255,0.7)', display: 'flex', padding: 4 }} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'rgba(255,255,255,0.7)', display: 'flex', padding: 4 }}
title={t('files.openTab')}> title={t('files.openTab')}>
<ExternalLink size={16} /> <ExternalLink size={16} />
@@ -743,7 +743,7 @@ export default function FileManager({ files = [], onUpload, onDelete, onUpdate,
<span style={{ fontSize: 13, fontWeight: 600, color: 'var(--text-primary)', overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap', flex: 1 }}>{previewFile.original_name}</span> <span style={{ fontSize: 13, fontWeight: 600, color: 'var(--text-primary)', overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap', flex: 1 }}>{previewFile.original_name}</span>
<div style={{ display: 'flex', alignItems: 'center', gap: 8, flexShrink: 0 }}> <div style={{ display: 'flex', alignItems: 'center', gap: 8, flexShrink: 0 }}>
<button <button
onClick={() => openFile(previewFile.url).catch(() => {})} onClick={() => openFileUrl(previewFile.url, previewFile.original_name).catch(() => toast.error(t('files.openError')))}
style={{ display: 'flex', alignItems: 'center', gap: 4, fontSize: 12, color: 'var(--text-muted)', background: 'none', border: 'none', cursor: 'pointer', textDecoration: 'none', padding: '4px 8px', borderRadius: 6, transition: 'color 0.15s' }} style={{ display: 'flex', alignItems: 'center', gap: 4, fontSize: 12, color: 'var(--text-muted)', background: 'none', border: 'none', cursor: 'pointer', textDecoration: 'none', padding: '4px 8px', borderRadius: 6, transition: 'color 0.15s' }}
onMouseEnter={e => (e.currentTarget as HTMLElement).style.color = 'var(--text-primary)'} onMouseEnter={e => (e.currentTarget as HTMLElement).style.color = 'var(--text-primary)'}
onMouseLeave={e => (e.currentTarget as HTMLElement).style.color = 'var(--text-muted)'}> onMouseLeave={e => (e.currentTarget as HTMLElement).style.color = 'var(--text-muted)'}>
@@ -771,7 +771,7 @@ export default function FileManager({ files = [], onUpload, onDelete, onUpdate,
title={previewFile.original_name} title={previewFile.original_name}
> >
<p style={{ padding: 24, textAlign: 'center', color: 'var(--text-muted)' }}> <p style={{ padding: 24, textAlign: 'center', color: 'var(--text-muted)' }}>
<button onClick={() => openFile(previewFile.url).catch(() => {})} style={{ color: 'var(--text-primary)', textDecoration: 'underline', background: 'none', border: 'none', cursor: 'pointer', font: 'inherit' }}>{t('files.downloadPdf')}</button> <button onClick={() => openFileUrl(previewFile.url, previewFile.original_name).catch(() => toast.error(t('files.openError')))} style={{ color: 'var(--text-primary)', textDecoration: 'underline', background: 'none', border: 'none', cursor: 'pointer', font: 'inherit' }}>{t('files.downloadPdf')}</button>
</p> </p>
</object> </object>
</div> </div>
+2
View File
@@ -1222,6 +1222,8 @@ const ar: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'الملفات', 'files.title': 'الملفات',
'files.pageTitle': 'الملفات والمستندات', 'files.pageTitle': 'الملفات والمستندات',
'files.subtitle': '{count} ملف لـ {trip}', 'files.subtitle': '{count} ملف لـ {trip}',
'files.download': 'تنزيل',
'files.openError': 'تعذر فتح الملف',
'files.downloadPdf': 'تنزيل PDF', 'files.downloadPdf': 'تنزيل PDF',
'files.count': '{count} ملفات', 'files.count': '{count} ملفات',
'files.countSingular': 'ملف واحد', 'files.countSingular': 'ملف واحد',
+2
View File
@@ -1191,6 +1191,8 @@ const br: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Arquivos', 'files.title': 'Arquivos',
'files.pageTitle': 'Arquivos e documentos', 'files.pageTitle': 'Arquivos e documentos',
'files.subtitle': '{count} arquivos para {trip}', 'files.subtitle': '{count} arquivos para {trip}',
'files.download': 'Baixar',
'files.openError': 'Não foi possível abrir o arquivo',
'files.downloadPdf': 'Baixar PDF', 'files.downloadPdf': 'Baixar PDF',
'files.count': '{count} arquivos', 'files.count': '{count} arquivos',
'files.countSingular': '1 arquivo', 'files.countSingular': '1 arquivo',
+2
View File
@@ -1220,6 +1220,8 @@ const cs: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Soubory', 'files.title': 'Soubory',
'files.pageTitle': 'Soubory a dokumenty', 'files.pageTitle': 'Soubory a dokumenty',
'files.subtitle': '{count} souborů pro {trip}', 'files.subtitle': '{count} souborů pro {trip}',
'files.download': 'Stáhnout',
'files.openError': 'Soubor nelze otevřít',
'files.downloadPdf': 'Stáhnout PDF', 'files.downloadPdf': 'Stáhnout PDF',
'files.count': '{count} souborů', 'files.count': '{count} souborů',
'files.countSingular': '1 soubor', 'files.countSingular': '1 soubor',
+2
View File
@@ -1224,6 +1224,8 @@ const de: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Dateien', 'files.title': 'Dateien',
'files.pageTitle': 'Dateien & Dokumente', 'files.pageTitle': 'Dateien & Dokumente',
'files.subtitle': '{count} Dateien für {trip}', 'files.subtitle': '{count} Dateien für {trip}',
'files.download': 'Herunterladen',
'files.openError': 'Datei konnte nicht geöffnet werden',
'files.downloadPdf': 'PDF herunterladen', 'files.downloadPdf': 'PDF herunterladen',
'files.count': '{count} Dateien', 'files.count': '{count} Dateien',
'files.countSingular': '1 Datei', 'files.countSingular': '1 Datei',
+2
View File
@@ -1281,6 +1281,8 @@ const en: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Files', 'files.title': 'Files',
'files.pageTitle': 'Files & Documents', 'files.pageTitle': 'Files & Documents',
'files.subtitle': '{count} files for {trip}', 'files.subtitle': '{count} files for {trip}',
'files.download': 'Download',
'files.openError': 'Could not open file',
'files.downloadPdf': 'Download PDF', 'files.downloadPdf': 'Download PDF',
'files.count': '{count} files', 'files.count': '{count} files',
'files.countSingular': '1 file', 'files.countSingular': '1 file',
+2
View File
@@ -1168,6 +1168,8 @@ const es: Record<string, string> = {
'files.title': 'Archivos', 'files.title': 'Archivos',
'files.pageTitle': 'Archivos y documentos', 'files.pageTitle': 'Archivos y documentos',
'files.subtitle': '{count} archivos para {trip}', 'files.subtitle': '{count} archivos para {trip}',
'files.download': 'Descargar',
'files.openError': 'No se pudo abrir el archivo',
'files.downloadPdf': 'Descargar PDF', 'files.downloadPdf': 'Descargar PDF',
'files.count': '{count} archivos', 'files.count': '{count} archivos',
'files.countSingular': '1 archivo', 'files.countSingular': '1 archivo',
+2
View File
@@ -1218,6 +1218,8 @@ const fr: Record<string, string> = {
'files.title': 'Fichiers', 'files.title': 'Fichiers',
'files.pageTitle': 'Fichiers et documents', 'files.pageTitle': 'Fichiers et documents',
'files.subtitle': '{count} fichiers pour {trip}', 'files.subtitle': '{count} fichiers pour {trip}',
'files.download': 'Télécharger',
'files.openError': "Impossible d'ouvrir le fichier",
'files.downloadPdf': 'Télécharger le PDF', 'files.downloadPdf': 'Télécharger le PDF',
'files.count': '{count} fichiers', 'files.count': '{count} fichiers',
'files.countSingular': '1 fichier', 'files.countSingular': '1 fichier',
+2
View File
@@ -1219,6 +1219,8 @@ const hu: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Fájlok', 'files.title': 'Fájlok',
'files.pageTitle': 'Fájlok és dokumentumok', 'files.pageTitle': 'Fájlok és dokumentumok',
'files.subtitle': '{count} fájl a következőhöz: {trip}', 'files.subtitle': '{count} fájl a következőhöz: {trip}',
'files.download': 'Letöltés',
'files.openError': 'A fájl megnyitása sikertelen',
'files.downloadPdf': 'PDF letöltése', 'files.downloadPdf': 'PDF letöltése',
'files.count': '{count} fájl', 'files.count': '{count} fájl',
'files.countSingular': '1 fájl', 'files.countSingular': '1 fájl',
+2
View File
@@ -1279,6 +1279,8 @@ const id: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'File', 'files.title': 'File',
'files.pageTitle': 'File & Dokumen', 'files.pageTitle': 'File & Dokumen',
'files.subtitle': '{count} file untuk {trip}', 'files.subtitle': '{count} file untuk {trip}',
'files.download': 'Unduh',
'files.openError': 'Tidak dapat membuka file',
'files.downloadPdf': 'Unduh PDF', 'files.downloadPdf': 'Unduh PDF',
'files.count': '{count} file', 'files.count': '{count} file',
'files.countSingular': '1 berkas', 'files.countSingular': '1 berkas',
+2
View File
@@ -1219,6 +1219,8 @@ const it: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'File', 'files.title': 'File',
'files.pageTitle': 'File e documenti', 'files.pageTitle': 'File e documenti',
'files.subtitle': '{count} file per {trip}', 'files.subtitle': '{count} file per {trip}',
'files.download': 'Scarica',
'files.openError': 'Impossibile aprire il file',
'files.downloadPdf': 'Scarica PDF', 'files.downloadPdf': 'Scarica PDF',
'files.count': '{count} file', 'files.count': '{count} file',
'files.countSingular': '1 documento', 'files.countSingular': '1 documento',
+2
View File
@@ -1218,6 +1218,8 @@ const nl: Record<string, string> = {
'files.title': 'Bestanden', 'files.title': 'Bestanden',
'files.pageTitle': 'Bestanden en documenten', 'files.pageTitle': 'Bestanden en documenten',
'files.subtitle': '{count} bestanden voor {trip}', 'files.subtitle': '{count} bestanden voor {trip}',
'files.download': 'Downloaden',
'files.openError': 'Bestand kon niet worden geopend',
'files.downloadPdf': 'PDF downloaden', 'files.downloadPdf': 'PDF downloaden',
'files.count': '{count} bestanden', 'files.count': '{count} bestanden',
'files.countSingular': '1 bestand', 'files.countSingular': '1 bestand',
+2
View File
@@ -1170,6 +1170,8 @@ const pl: Record<string, string | { name: string; category: string }[]> = {
'files.title': 'Pliki', 'files.title': 'Pliki',
'files.pageTitle': 'Pliki i dokumenty', 'files.pageTitle': 'Pliki i dokumenty',
'files.subtitle': '{count} plików dla {trip}', 'files.subtitle': '{count} plików dla {trip}',
'files.download': 'Pobierz',
'files.openError': 'Nie można otworzyć pliku',
'files.downloadPdf': 'Pobierz PDF', 'files.downloadPdf': 'Pobierz PDF',
'files.count': '{count} plików', 'files.count': '{count} plików',
'files.countSingular': '1 plik', 'files.countSingular': '1 plik',
+2
View File
@@ -1218,6 +1218,8 @@ const ru: Record<string, string> = {
'files.title': 'Файлы', 'files.title': 'Файлы',
'files.pageTitle': 'Файлы и документы', 'files.pageTitle': 'Файлы и документы',
'files.subtitle': '{count} файлов для {trip}', 'files.subtitle': '{count} файлов для {trip}',
'files.download': 'Скачать',
'files.openError': 'Не удалось открыть файл',
'files.downloadPdf': 'Скачать PDF', 'files.downloadPdf': 'Скачать PDF',
'files.count': '{count} файлов', 'files.count': '{count} файлов',
'files.countSingular': '1 файл', 'files.countSingular': '1 файл',
+2
View File
@@ -1218,6 +1218,8 @@ const zh: Record<string, string> = {
'files.title': '文件', 'files.title': '文件',
'files.pageTitle': '文件与文档', 'files.pageTitle': '文件与文档',
'files.subtitle': '{trip} 的 {count} 个文件', 'files.subtitle': '{trip} 的 {count} 个文件',
'files.download': '下载',
'files.openError': '无法打开文件',
'files.downloadPdf': '下载 PDF', 'files.downloadPdf': '下载 PDF',
'files.count': '{count} 个文件', 'files.count': '{count} 个文件',
'files.countSingular': '1 个文件', 'files.countSingular': '1 个文件',
+2
View File
@@ -1278,6 +1278,8 @@ const zhTw: Record<string, string> = {
'files.title': '檔案', 'files.title': '檔案',
'files.pageTitle': '檔案與文件', 'files.pageTitle': '檔案與文件',
'files.subtitle': '{trip} 的 {count} 個檔案', 'files.subtitle': '{trip} 的 {count} 個檔案',
'files.download': '下載',
'files.openError': '無法開啟檔案',
'files.downloadPdf': '下載 PDF', 'files.downloadPdf': '下載 PDF',
'files.count': '{count} 個檔案', 'files.count': '{count} 個檔案',
'files.countSingular': '1 個檔案', 'files.countSingular': '1 個檔案',
+2 -6
View File
@@ -77,15 +77,11 @@ const upload = multer({
// Routes // 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) => { router.get('/:id/download', (req: Request, res: Response) => {
const { tripId, id } = req.params; const { tripId, id } = req.params;
const authHeader = req.headers['authorization']; const auth = authenticateDownload(req);
const bearerToken = authHeader && authHeader.split(' ')[1];
const queryToken = req.query.token as string | undefined;
const auth = authenticateDownload(bearerToken, queryToken);
if ('error' in auth) return res.status(auth.status).json({ error: auth.error }); if ('error' in auth) return res.status(auth.status).json({ error: auth.error });
const trip = verifyTripAccess(tripId, auth.userId); const trip = verifyTripAccess(tripId, auth.userId);
+1 -1
View File
@@ -62,7 +62,7 @@ export function parseAutoBackupBody(body: Record<string, unknown>): {
} }
export function isValidBackupFilename(filename: string): boolean { 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 { export function backupFilePath(filename: string): string {
+17 -9
View File
@@ -1,5 +1,6 @@
import path from 'path'; import path from 'path';
import fs from 'fs'; import fs from 'fs';
import type { Request } from 'express';
import { db, canAccessTrip } from '../db/database'; import { db, canAccessTrip } from '../db/database';
import { consumeEphemeralToken } from './ephemeralTokens'; import { consumeEphemeralToken } from './ephemeralTokens';
import { verifyJwtAndLoadUser } from '../middleware/auth'; import { verifyJwtAndLoadUser } from '../middleware/auth';
@@ -72,23 +73,30 @@ export function resolveFilePath(filename: string): { resolved: string; safe: boo
// Token-based download auth // Token-based download auth
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
export function authenticateDownload(bearerToken: string | undefined, queryToken: string | undefined): { userId: number } | { error: string; status: number } { export function authenticateDownload(req: Request): { userId: number } | { error: string; status: number } {
if (!bearerToken && !queryToken) { const cookieToken = (req as any).cookies?.trek_session as string | undefined;
return { error: 'Authentication required', status: 401 }; 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; // Use the shared helper so the password_version gate applies here too;
// previously this bypassed the check and stolen download tokens stayed // previously this bypassed the check and stolen download tokens stayed
// valid across a password reset. // valid across a password reset.
const user = verifyJwtAndLoadUser(bearerToken); const user = verifyJwtAndLoadUser(jwtToken);
if (!user) return { error: 'Invalid or expired token', status: 401 }; if (!user) return { error: 'Invalid or expired token', status: 401 };
return { userId: user.id }; return { userId: user.id };
} }
const uid = consumeEphemeralToken(queryToken!, 'download'); if (queryToken) {
if (!uid) return { error: 'Invalid or expired token', status: 401 }; const uid = consumeEphemeralToken(queryToken, 'download');
return { userId: uid }; if (!uid) return { error: 'Invalid or expired token', status: 401 };
return { userId: uid };
}
return { error: 'Authentication required', status: 401 };
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
+15 -2
View File
@@ -365,13 +365,12 @@ describe('File download', () => {
expect(res.status).toBe(401); 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 { user } = createUser(testDb);
const trip = createTrip(testDb, user.id); const trip = createTrip(testDb, user.id);
const upload = await uploadFile(trip.id, user.id, FIXTURE_PDF); const upload = await uploadFile(trip.id, user.id, FIXTURE_PDF);
const fileId = upload.body.file.id; const fileId = upload.body.file.id;
// authenticateDownload accepts a signed JWT as Bearer token
const token = generateToken(user.id); const token = generateToken(user.id);
const dl = await request(app) const dl = await request(app)
@@ -380,4 +379,18 @@ describe('File download', () => {
// multer stores the file to disk during uploadFile — physical file exists // multer stores the file to disk during uploadFile — physical file exists
expect(dl.status).toBe(200); 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', () => { it('accepts filename with hyphens and underscores', () => {
expect(isValidBackupFilename('backup-my_trek-2026.zip')).toBe(true); 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);
});
}); });
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------