diff --git a/backend/__tests__/unit/routes/pods.external-links.test.js b/backend/__tests__/unit/routes/pods.external-links.test.js index d705b5c5..a06fa91e 100644 --- a/backend/__tests__/unit/routes/pods.external-links.test.js +++ b/backend/__tests__/unit/routes/pods.external-links.test.js @@ -241,12 +241,30 @@ describe('pods routes - external links', () => { await request(app).delete('/api/pods/external-link/l1').expect(404); }); - it('returns 403 when deleting link as non-owner', async () => { - ExternalLink.findById.mockResolvedValue({ podId: 'p1' }); - Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'other' } }); + it('returns 403 when deleting link as neither owner nor creator', async () => { + ExternalLink.findById.mockResolvedValue({ + podId: 'p1', + createdBy: { toString: () => 'someone-else' }, + }); + Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'other-owner' } }); await request(app).delete('/api/pods/external-link/l1').expect(403); }); + it('allows the link creator to delete (even if not pod owner)', async () => { + const podSave = jest.fn(); + ExternalLink.findById.mockResolvedValue({ + podId: 'p1', + createdBy: { toString: () => 'user1' }, // current authed user + }); + Pod.findById.mockResolvedValue({ + createdBy: { toString: () => 'other-owner' }, + externalLinks: ['l1'], + save: podSave, + }); + await request(app).delete('/api/pods/external-link/l1').expect(200); + expect(ExternalLink.findByIdAndDelete).toHaveBeenCalledWith('l1'); + }); + it('fetches qrcode when user is member', async () => { ExternalLink.findById.mockResolvedValue({ podId: 'p1', diff --git a/backend/__tests__/unit/routes/pods.files-delete.test.js b/backend/__tests__/unit/routes/pods.files-delete.test.js new file mode 100644 index 00000000..65cfd328 --- /dev/null +++ b/backend/__tests__/unit/routes/pods.files-delete.test.js @@ -0,0 +1,109 @@ +const request = require('supertest'); +const express = require('express'); + +jest.mock('../../../middleware/auth', () => (req, res, next) => { + req.user = { id: 'user1' }; + req.userId = 'user1'; + next(); +}); + +const Pod = require('../../../models/Pod'); +const File = require('../../../models/File'); + +jest.mock('../../../models/Pod'); +jest.mock('../../../models/File'); + +const mockObjectStoreDelete = jest.fn().mockResolvedValue(undefined); +jest.mock('../../../services/objectStore', () => ({ + getObjectStore: () => ({ delete: mockObjectStoreDelete }), + __resetObjectStoreForTests: jest.fn(), +})); + +const routes = require('../../../routes/pods'); + +const VALID_FILE_ID = '67a9ceb240f8f53015944a05'; + +describe('pods routes - DELETE /:podId/files/:fileId', () => { + let app; + beforeEach(() => { + app = express(); + app.use(express.json()); + app.use('/api/pods', routes); + jest.clearAllMocks(); + mockObjectStoreDelete.mockClear(); + }); + + it('deletes a file when caller is the pod owner', async () => { + File.findById.mockResolvedValue({ + _id: VALID_FILE_ID, + fileName: '1234-5678.pdf', + podId: { toString: () => 'p1' }, + uploadedBy: { toString: () => 'someone-else' }, + }); + Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'user1' } }); + File.deleteOne = jest.fn().mockResolvedValue(undefined); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(200); + expect(File.deleteOne).toHaveBeenCalledWith({ _id: VALID_FILE_ID }); + expect(mockObjectStoreDelete).toHaveBeenCalledWith('1234-5678.pdf'); + }); + + it('deletes a file when caller is the original uploader (not owner)', async () => { + File.findById.mockResolvedValue({ + _id: VALID_FILE_ID, + fileName: '1234-5678.pdf', + podId: { toString: () => 'p1' }, + uploadedBy: { toString: () => 'user1' }, + }); + Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'other-owner' } }); + File.deleteOne = jest.fn().mockResolvedValue(undefined); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(200); + expect(File.deleteOne).toHaveBeenCalled(); + }); + + it('returns 403 when caller is neither owner nor uploader', async () => { + File.findById.mockResolvedValue({ + _id: VALID_FILE_ID, + fileName: '1234-5678.pdf', + podId: { toString: () => 'p1' }, + uploadedBy: { toString: () => 'someone-else' }, + }); + Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'other-owner' } }); + File.deleteOne = jest.fn(); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(403); + expect(File.deleteOne).not.toHaveBeenCalled(); + expect(mockObjectStoreDelete).not.toHaveBeenCalled(); + }); + + it('returns 400 for an invalid fileId shape (ReDoS-style abuse)', async () => { + await request(app).delete('/api/pods/p1/files/not-an-objectid').expect(400); + }); + + it('returns 404 when the file is not found', async () => { + File.findById.mockResolvedValue(null); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(404); + }); + + it('returns 404 when the file is in a different pod', async () => { + File.findById.mockResolvedValue({ + _id: VALID_FILE_ID, + fileName: '1234-5678.pdf', + podId: { toString: () => 'OTHER_POD' }, + uploadedBy: { toString: () => 'user1' }, + }); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(404); + }); + + it('completes the metadata delete even when ObjectStore.delete throws', async () => { + File.findById.mockResolvedValue({ + _id: VALID_FILE_ID, + fileName: '1234-5678.pdf', + podId: { toString: () => 'p1' }, + uploadedBy: { toString: () => 'user1' }, + }); + Pod.findById.mockResolvedValue({ createdBy: { toString: () => 'other-owner' } }); + File.deleteOne = jest.fn().mockResolvedValue(undefined); + mockObjectStoreDelete.mockRejectedValueOnce(new Error('GCS down')); + await request(app).delete(`/api/pods/p1/files/${VALID_FILE_ID}`).expect(200); + expect(File.deleteOne).toHaveBeenCalled(); + }); +}); diff --git a/backend/routes/pods.ts b/backend/routes/pods.ts index fec17676..9d7d6204 100644 --- a/backend/routes/pods.ts +++ b/backend/routes/pods.ts @@ -198,11 +198,16 @@ router.post('/external-link', auth, upload.single('qrCode'), async (req: AuthReq router.delete('/external-link/:id', auth, async (req: AuthReq, res: Res) => { try { const linkId = req.params?.id; - const externalLink = await ExternalLink.findById(linkId) as { podId?: unknown; qrCodePath?: string } | null; + const externalLink = await ExternalLink.findById(linkId) as { podId?: unknown; qrCodePath?: string; createdBy?: { toString: () => string } } | null; if (!externalLink) return res.status(404).json({ message: 'External link not found' }); const pod = await Pod.findById(externalLink.podId) as { createdBy?: { toString: () => string }; externalLinks?: Array<{ toString: () => string }>; save: () => Promise } | null; if (!pod) return res.status(404).json({ message: 'Pod not found' }); - if (pod.createdBy?.toString() !== req.user?.id) return res.status(403).json({ message: 'Only pod owner can delete external links' }); + // Pod owner OR the user who added the link can delete it. Members at + // large can't — keeps a malicious pod-mate from nuking shared artifacts. + const userId = req.user?.id; + const isOwner = pod.createdBy?.toString() === userId; + const isCreator = externalLink.createdBy?.toString() === userId; + if (!isOwner && !isCreator) return res.status(403).json({ message: 'Only pod owner or link creator can delete' }); pod.externalLinks = pod.externalLinks?.filter((id) => id.toString() !== linkId); await pod.save(); if (externalLink.qrCodePath && fs.existsSync(externalLink.qrCodePath)) fs.unlinkSync(externalLink.qrCodePath); @@ -320,6 +325,42 @@ router.get('/:podId/files', auth, async (req: AuthReq, res: Res) => { } }); +// Delete a pod-scoped file. Permitted: the pod owner OR the original +// uploader. The route also deletes the bytes from the configured ObjectStore +// driver, mirroring the lifecycle the legacy /external-link/:id delete uses +// for QR-code blobs. Failures to delete from the driver are logged but don't +// block the metadata delete — orphaned bytes can be GC'd later (ADR-002 +// Phase 4 nightly sweep). +router.delete('/:podId/files/:fileId', auth, async (req: AuthReq, res: Res) => { + try { + const { podId, fileId } = req.params || {}; + if (!fileId || !/^[A-Fa-f0-9]{24}$/.test(fileId)) return res.status(400).json({ message: 'Invalid fileId' }); + const fileDoc = await File.findById(fileId) as { _id: unknown; fileName?: string; podId?: { toString: () => string }; uploadedBy?: { toString: () => string }; deleteOne?: () => Promise } | null; + if (!fileDoc) return res.status(404).json({ message: 'File not found' }); + if (fileDoc.podId?.toString() !== podId) return res.status(404).json({ message: 'File not in this pod' }); + const pod = await Pod.findById(podId) as { createdBy?: { toString: () => string } } | null; + if (!pod) return res.status(404).json({ message: 'Pod not found' }); + const userId = req.user?.id; + const isOwner = pod.createdBy?.toString() === userId; + const isUploader = fileDoc.uploadedBy?.toString() === userId; + if (!isOwner && !isUploader) return res.status(403).json({ message: 'Only pod owner or file uploader can delete' }); + if (fileDoc.fileName) { + try { + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { getObjectStore } = require('../services/objectStore') as { getObjectStore: () => { delete: (key: string) => Promise } }; + await getObjectStore().delete(fileDoc.fileName); + } catch (driverErr) { + console.warn('ObjectStore delete failed (will be GC\'d):', (driverErr as Error).message); + } + } + await File.deleteOne({ _id: fileDoc._id }); + return res.status(200).json({ message: 'File deleted' }); + } catch (error) { + console.error('Error deleting pod file:', error); + return res.status(500).json({ message: 'Server error' }); + } +}); + router.get('/:podId/external-links', auth, async (req: AuthReq, res: Res) => { try { const { podId } = req.params || {}; diff --git a/frontend/src/v2/components/V2PodInspector.tsx b/frontend/src/v2/components/V2PodInspector.tsx index ffc15215..f28e901f 100644 --- a/frontend/src/v2/components/V2PodInspector.tsx +++ b/frontend/src/v2/components/V2PodInspector.tsx @@ -51,6 +51,7 @@ interface ExternalLinkItem { name?: string; type?: string; url?: string; + createdBy?: { _id?: string; username?: string } | string; } interface PodFileItem { @@ -60,8 +61,15 @@ interface PodFileItem { contentType?: string; size?: number; createdAt?: string; + uploadedBy?: { _id?: string; username?: string } | string; } +const itemOwnerId = (raw: ExternalLinkItem['createdBy'] | PodFileItem['uploadedBy']): string | undefined => { + if (!raw) return undefined; + if (typeof raw === 'string') return raw; + return raw._id; +}; + // Per-type label for the Artifacts row icon (1-2 char glyph) and the // human-readable kind shown under the title. Keep aligned with the enum in // `backend/models/ExternalLink.ts`. Unknown kinds fall back to "L" / "Link". @@ -670,11 +678,28 @@ const V2PodInspector: React.FC = ({ // sits at the top — matches the user's expectation that the row they just // pasted is visible without scrolling. Within each source, server returns // newest-first. - const artifactItems: Array<{ id: string; kind: string; title: string; subtitle?: string; url?: string; fileName?: string }> = [ + const artifactItems: Array<{ + id: string; + kind: string; + title: string; + subtitle?: string; + url?: string; + fileName?: string; + // Identifiers for the underlying record (link _id or file _id) and the + // user who created it. Lets renderArtifactDetail decide whether the + // current viewer can delete (pod owner OR creator) and which DELETE + // endpoint to hit. Announcements aren't deletable from this surface yet + // — they live under a separate admin flow. + sourceKind?: 'announcement' | 'link' | 'file'; + sourceId?: string; + createdById?: string; + }> = [ ...announcements.map((a) => ({ id: `ann-${a._id}`, kind: 'Announcement', title: a.title || a.content || 'Untitled announcement', + sourceKind: 'announcement' as const, + sourceId: a._id, })), ...externalLinks.map((l) => ({ id: `link-${l._id}`, @@ -682,6 +707,9 @@ const V2PodInspector: React.FC = ({ title: l.name || l.url || 'External link', subtitle: l.url, url: l.url, + sourceKind: 'link' as const, + sourceId: l._id, + createdById: itemOwnerId(l.createdBy), })), ...podFiles.map((f) => ({ id: `file-${f._id}`, @@ -691,6 +719,9 @@ const V2PodInspector: React.FC = ({ // No `url` here — files require a signed-URL mint. The detail view's // Open button calls getSignedAttachmentUrl(`/api/uploads/${fileName}`). fileName: f.fileName, + sourceKind: 'file' as const, + sourceId: f._id, + createdById: itemOwnerId(f.uploadedBy), })), ]; @@ -1004,6 +1035,43 @@ const V2PodInspector: React.FC = ({ if (href) window.open(href, '_blank', 'noopener,noreferrer'); }; + // Pod owner OR original creator/uploader can delete an artifact. Backend + // enforces this independently — frontend gating is just to avoid showing + // a button that would 403. Announcements not yet deletable from here. + const [deleting, setDeleting] = useState(false); + const [deleteError, setDeleteError] = useState(null); + const canDeleteItem = (item: typeof artifactItems[0]): boolean => { + if (!item.sourceKind || item.sourceKind === 'announcement') return false; + const me = currentUser?._id; + if (!me) return false; + if (ownerId && String(ownerId) === String(me)) return true; + if (item.createdById && String(item.createdById) === String(me)) return true; + return false; + }; + const handleDeleteArtifact = async (item: typeof artifactItems[0]) => { + if (!item.sourceId || !item.sourceKind || !pod) return; + const label = item.title || 'this item'; + if (!window.confirm(`Delete ${label}? This can't be undone.`)) return; + setDeleting(true); + setDeleteError(null); + try { + if (item.sourceKind === 'link') { + await api.del(`/api/pods/external-link/${item.sourceId}`); + setExternalLinks((prev) => prev.filter((l) => l._id !== item.sourceId)); + } else if (item.sourceKind === 'file') { + await api.del(`/api/pods/${pod._id}/files/${item.sourceId}`); + setPodFiles((prev) => prev.filter((f) => f._id !== item.sourceId)); + } + // Pop back to overview now that this artifact no longer exists. + onBack(); + } catch (err) { + const e = err as { response?: { data?: { message?: string } }; message?: string }; + setDeleteError(e.response?.data?.message || e.message || 'Could not delete.'); + } finally { + setDeleting(false); + } + }; + const renderArtifactDetail = (artifactId: string) => { const found = artifactItems.find((a) => a.id === artifactId); if (!found) { @@ -1028,17 +1096,32 @@ const V2PodInspector: React.FC = ({ title: found.title, }} /> - {openable && ( -
- + {(openable || canDeleteItem(found)) && ( +
+ {openable && ( + + )} + {canDeleteItem(found) && ( + + )}
)} + {deleteError && ( +
{deleteError}
+ )} {found.subtitle && (
{found.fileName ? 'Type' : 'Source'}