github/TREK
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #349 from mauriceboe/343-bug-attachments-in-collab-notes-seem-to-be-broken

Browse Source 
fix: collab note attachments broken (#343)
This commit is contained in:
Julien G.
2026-04-03 14:14:42 +02:00
committed by GitHub
parent 36f2292f2d bb54fda6dc
commit 816696d0fe
3 changed files with 132 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
client/src/components/Collab/CollabNotes.tsx
View File

@@ -5,6 +5,7 @@ import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'
import { Plus, Trash2, Pin, PinOff, Pencil, X, Check, StickyNote, Settings, ExternalLink, Maximize2 } from 'lucide-react'
import { collabApi } from '../../api/client'
import { getAuthUrl } from '../../api/authUrl'
import { useCanDo } from '../../store/permissionsStore'
import { useTripStore } from '../../store/tripStore'
import { addListener, removeListener } from '../../api/websocket'
@@ -96,22 +97,33 @@ interface FilePreviewPortalProps {
}
function FilePreviewPortal({ file, onClose }: FilePreviewPortalProps) {
const [authUrl, setAuthUrl] = useState('')
const rawUrl = file?.url || ''
useEffect(() => {
if (!rawUrl) return
getAuthUrl(rawUrl, 'download').then(setAuthUrl)
}, [rawUrl])
if (!file) return null
const url = file.url || `/uploads/${file.filename}`
const isImage = file.mime_type?.startsWith('image/')
const isPdf = file.mime_type === 'application/pdf'
const isTxt = file.mime_type?.startsWith('text/')
const openInNewTab = async () => {
const u = await getAuthUrl(rawUrl, 'download')
window.open(u, '_blank', 'noreferrer')
}
return ReactDOM.createPortal(
<div style={{ position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.88)', zIndex: 10000, display: 'flex', alignItems: 'center', justifyContent: 'center', padding: 16 }} onClick={onClose}>
{isImage ? (
/* Image lightbox — floating controls */
<div style={{ position: 'relative', maxWidth: '90vw', maxHeight: '90vh' }} onClick={e => e.stopPropagation()}>
<img src={url} alt={file.original_name} style={{ maxWidth: '90vw', maxHeight: '90vh', objectFit: 'contain', borderRadius: 8, display: 'block' }} />
<img src={authUrl} alt={file.original_name} style={{ maxWidth: '90vw', maxHeight: '90vh', objectFit: 'contain', borderRadius: 8, display: 'block' }} />
<div style={{ position: 'absolute', top: -36, left: 0, right: 0, display: 'flex', alignItems: 'center', justifyContent: 'space-between', padding: '0 4px' }}>
<span style={{ fontSize: 11, color: 'rgba(255,255,255,0.7)', overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap', maxWidth: '70%' }}>{file.original_name}</span>
<div style={{ display: 'flex', gap: 8 }}>
<a href={url} target="_blank" rel="noreferrer" style={{ color: 'rgba(255,255,255,0.7)', display: 'flex' }}><ExternalLink size={15} /></a>
<button onClick={openInNewTab} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'rgba(255,255,255,0.7)', display: 'flex', padding: 0 }}><ExternalLink size={15} /></button>
<button onClick={onClose} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'rgba(255,255,255,0.7)', display: 'flex', padding: 0 }}><X size={17} /></button>
</div>
</div>
@@ -122,19 +134,19 @@ function FilePreviewPortal({ file, onClose }: FilePreviewPortalProps) {
<div style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between', padding: '10px 16px', borderBottom: '1px solid var(--border-primary)', flexShrink: 0 }}>
<span style={{ fontSize: 13, fontWeight: 600, color: 'var(--text-primary)', overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap', flex: 1 }}>{file.original_name}</span>
<div style={{ display: 'flex', gap: 8, flexShrink: 0 }}>
<a href={url} target="_blank" rel="noreferrer" style={{ display: 'flex', alignItems: 'center', gap: 3, fontSize: 11, color: 'var(--text-muted)', textDecoration: 'none' }}><ExternalLink size={13} /></a>
<button onClick={openInNewTab} style={{ background: 'none', border: 'none', cursor: 'pointer', display: 'flex', alignItems: 'center', gap: 3, fontSize: 11, color: 'var(--text-muted)', padding: 0 }}><ExternalLink size={13} /></button>
<button onClick={onClose} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'var(--text-faint)', display: 'flex', padding: 2 }}><X size={18} /></button>
</div>
</div>
{(isPdf || isTxt) ? (
<object data={`${url}#view=FitH`} type={file.mime_type} style={{ flex: 1, width: '100%', border: 'none', background: '#fff' }} title={file.original_name}>
<object data={authUrl ? `${authUrl}#view=FitH` : ''} type={file.mime_type} style={{ flex: 1, width: '100%', border: 'none', background: '#fff' }} title={file.original_name}>
<p style={{ padding: 24, textAlign: 'center', color: 'var(--text-muted)' }}>
<a href={url} target="_blank" rel="noopener noreferrer" style={{ color: 'var(--text-primary)', textDecoration: 'underline' }}>Download</a>
<button onClick={openInNewTab} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'var(--text-primary)', textDecoration: 'underline', fontSize: 14, padding: 0 }}>Download</button>
</p>
</object>
) : (
<div style={{ flex: 1, display: 'flex', alignItems: 'center', justifyContent: 'center', padding: 40 }}>
<a href={url} target="_blank" rel="noopener noreferrer" style={{ color: 'var(--text-primary)', textDecoration: 'underline', fontSize: 14 }}>Download {file.original_name}</a>
<button onClick={openInNewTab} style={{ background: 'none', border: 'none', cursor: 'pointer', color: 'var(--text-primary)', textDecoration: 'underline', fontSize: 14, padding: 0 }}>Download {file.original_name}</button>
</div>
)}
</div>
@@ -144,6 +156,14 @@ function FilePreviewPortal({ file, onClose }: FilePreviewPortalProps) {
)
}
function AuthedImg({ src, style, onClick, onMouseEnter, onMouseLeave, alt }: { src: string; style?: React.CSSProperties; onClick?: () => void; onMouseEnter?: React.MouseEventHandler<HTMLImageElement>; onMouseLeave?: React.MouseEventHandler<HTMLImageElement>; alt?: string }) {
const [authSrc, setAuthSrc] = useState('')
useEffect(() => {
getAuthUrl(src, 'download').then(setAuthSrc)
}, [src])
return authSrc ? <img src={authSrc} alt={alt} style={style} onClick={onClick} onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave} /> : null
}
const NOTE_COLORS = [
{ value: '#6366f1', label: 'Indigo' },
{ value: '#ef4444', label: 'Red' },
@@ -460,7 +480,7 @@ function NoteFormModal({ onClose, onSubmit, onDeleteFile, existingCategories, ca
<div style={{ fontSize: 9, fontWeight: 600, color: 'var(--text-faint)', textTransform: 'uppercase', letterSpacing: '0.05em', marginBottom: 4, fontFamily: FONT }}>
{t('collab.notes.attachFiles')}
</div>
<input id="note-file-input" ref={fileRef} type="file" multiple style={{ display: 'none' }} onChange={e => { setPendingFiles(prev => [...prev, ...Array.from((e.target as HTMLInputElement).files)]); e.target.value = '' }} />
<input ref={fileRef} type="file" multiple style={{ display: 'none' }} onChange={e => { const files = e.target.files; if (files?.length) setPendingFiles(prev => [...prev, ...Array.from(files)]); e.target.value = '' }} />
<div style={{ display: 'flex', gap: 6, flexWrap: 'wrap', alignItems: 'center' }}>
{/* Existing attachments (edit mode) */}
{existingAttachments.map(a => {
@@ -484,10 +504,10 @@ function NoteFormModal({ onClose, onSubmit, onDeleteFile, existingCategories, ca
</button>
</div>
))}
<label htmlFor="note-file-input"
<button type="button" onClick={() => fileRef.current?.click()}
style={{ padding: '4px 10px', borderRadius: 8, border: '1px dashed var(--border-faint)', background: 'transparent', cursor: 'pointer', color: 'var(--text-faint)', fontSize: 11, fontFamily: FONT, display: 'inline-flex', alignItems: 'center', gap: 4 }}>
<Plus size={11} /> {t('files.attach') || 'Add'}
</label>
</button>
</div>
</div>}
@@ -845,7 +865,7 @@ function NoteCard({ note, currentUser, canEdit, onUpdate, onDelete, onEdit, onVi
const isImage = a.mime_type?.startsWith('image/')
const ext = (a.original_name || '').split('.').pop()?.toUpperCase() || '?'
return isImage ? (
<img key={a.id} src={a.url} alt={a.original_name}
<AuthedImg key={a.id} src={a.url} alt={a.original_name}
style={{ width: 48, height: 48, objectFit: 'cover', borderRadius: 8, cursor: 'pointer', transition: 'transform 0.12s, box-shadow 0.12s' }}
onClick={() => onPreviewFile?.(a)}
onMouseEnter={e => { e.currentTarget.style.transform = 'scale(1.08)'; e.currentTarget.style.boxShadow = '0 2px 8px rgba(0,0,0,0.15)' }}
@@ -974,7 +994,7 @@ export default function CollabNotes({ tripId, currentUser }: CollabNotesProps) {
for (const file of pendingFiles) {
const fd = new FormData()
fd.append('file', file)
try { await collabApi.uploadNoteFile(tripId, note.id, fd) } catch {}
try { await collabApi.uploadNoteFile(tripId, note.id, fd) } catch (err) { console.error('Failed to upload note attachment:', err) }
}
// Reload note with attachments
const fresh = await collabApi.getNotes(tripId)

4
server/src/services/collabService.ts
View File

@@ -101,7 +101,7 @@ export function formatNote(note: CollabNote) {
return {
...note,
avatar_url: avatarUrl(note),
attachments: attachments.map(a => ({ ...a, url: `/uploads/${a.filename}` })),
attachments: attachments.map(a => ({ ...a, url: `/api/trips/${note.trip_id}/files/${a.id}/download` })),
};
}
@@ -190,7 +190,7 @@ export function addNoteFile(tripId: string | number, noteId: string | number, fi
).run(tripId, noteId, `files/${file.filename}`, file.originalname, file.size, file.mimetype);
const saved = db.prepare('SELECT * FROM trip_files WHERE id = ?').get(result.lastInsertRowid) as TripFile;
return { file: { ...saved, url: `/uploads/${saved.filename}` } };
return { file: { ...saved, url: `/api/trips/${tripId}/files/${saved.id}/download` } };
}
export function getFormattedNoteById(noteId: string | number) {

99
server/tests/integration/collab.test.ts
View File

@@ -47,7 +47,7 @@ import { createTables } from '../../src/db/schema';
import { runMigrations } from '../../src/db/migrations';
import { resetTestDb } from '../helpers/test-db';
import { createUser, createTrip, addTripMember } from '../helpers/factories';
import { authCookie } from '../helpers/auth';
import { authCookie, generateToken } from '../helpers/auth';
import { loginAttempts, mfaAttempts } from '../../src/routes/auth';
const app: Application = createApp();
@@ -249,6 +249,103 @@ describe('Collab notes', () => {
expect(del.status).toBe(200);
expect(del.body.success).toBe(true);
});
it('COLLAB-028 — uploaded note file URL uses authenticated download path, not /uploads/', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const create = await request(app)
.post(`/api/trips/${trip.id}/collab/notes`)
.set('Cookie', authCookie(user.id))
.send({ title: 'URL check' });
const noteId = create.body.note.id;
const upload = await request(app)
.post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`)
.set('Cookie', authCookie(user.id))
.attach('file', FIXTURE_PDF);
expect(upload.status).toBe(201);
const fileUrl = upload.body.file.url;
expect(fileUrl).toMatch(/^\/api\/trips\/\d+\/files\/\d+\/download$/);
expect(fileUrl).not.toContain('/uploads/');
});
it('COLLAB-029 — note attachments in listing use authenticated download URLs', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const create = await request(app)
.post(`/api/trips/${trip.id}/collab/notes`)
.set('Cookie', authCookie(user.id))
.send({ title: 'List URL check' });
const noteId = create.body.note.id;
await request(app)
.post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`)
.set('Cookie', authCookie(user.id))
.attach('file', FIXTURE_PDF);
const list = await request(app)
.get(`/api/trips/${trip.id}/collab/notes`)
.set('Cookie', authCookie(user.id));
expect(list.status).toBe(200);
const note = list.body.notes.find((n: any) => n.id === noteId);
expect(note.attachments.length).toBe(1);
expect(note.attachments[0].url).toMatch(/^\/api\/trips\/\d+\/files\/\d+\/download$/);
expect(note.attachments[0].url).not.toContain('/uploads/');
});
it('COLLAB-030 — note file is downloadable via files endpoint with ephemeral token', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const create = await request(app)
.post(`/api/trips/${trip.id}/collab/notes`)
.set('Cookie', authCookie(user.id))
.send({ title: 'Downloadable note' });
const noteId = create.body.note.id;
const upload = await request(app)
.post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`)
.set('Cookie', authCookie(user.id))
.attach('file', FIXTURE_PDF);
const fileUrl = upload.body.file.url;
// Obtain an ephemeral resource token (same flow as getAuthUrl on the client)
const tokenRes = await request(app)
.post('/api/auth/resource-token')
.set('Cookie', authCookie(user.id))
.send({ purpose: 'download' });
expect(tokenRes.status).toBe(200);
const { token } = tokenRes.body;
// Download with ?token= should succeed
const dl = await request(app).get(`${fileUrl}?token=${token}`);
expect(dl.status).toBe(200);
});
it('COLLAB-031 — note file download without auth returns 401', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const create = await request(app)
.post(`/api/trips/${trip.id}/collab/notes`)
.set('Cookie', authCookie(user.id))
.send({ title: 'Auth required note' });
const noteId = create.body.note.id;
const upload = await request(app)
.post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`)
.set('Cookie', authCookie(user.id))
.attach('file', FIXTURE_PDF);
const fileUrl = upload.body.file.url;
// Download without any auth should fail
const dl = await request(app).get(fileUrl);
expect(dl.status).toBe(401);
});
});
// ─────────────────────────────────────────────────────────────────────────────