From 47f0e180e3fb2c283baeeceb9dbdc00c70c5e3f4 Mon Sep 17 00:00:00 2001 From: kmck133 Date: Tue, 26 May 2026 18:23:08 +1200 Subject: [PATCH 1/6] Allowed renaming of resources in edit view --- backend/src/routes/api/files.js | 23 ++++ .../resources/ManageResourcesPage.jsx | 126 +++++++++++++++--- 2 files changed, 128 insertions(+), 21 deletions(-) diff --git a/backend/src/routes/api/files.js b/backend/src/routes/api/files.js index b64816740..8dc095f02 100644 --- a/backend/src/routes/api/files.js +++ b/backend/src/routes/api/files.js @@ -145,6 +145,29 @@ router.post("/upload", upload.array("files"), async (req, res) => { } }); +/** + * @route PATCH /api/files/:fileId + * @desc Update the name of a stored file + */ +router.patch("/:fileId", async (req, res) => { + try { + const { fileId } = req.params; + const { name } = req.body; + if (!name || !name.trim()) { + return res.status(400).json({ error: "name is required" }); + } + const meta = await StoredFile.findById(fileId); + if (!meta) return res.status(404).json({ error: "File not found" }); + meta.name = name.trim(); + await meta.save(); + const file = meta.toObject(); + delete file.gridFsId; + return res.json(file); + } catch (err) { + return res.status(500).json({ error: err.message }); + } +}); + /** * @route DELETE /api/files/:fileId * @desc Delete a stored file and its GridFS data diff --git a/frontend/src/features/resources/ManageResourcesPage.jsx b/frontend/src/features/resources/ManageResourcesPage.jsx index 892c43ff6..da899161a 100644 --- a/frontend/src/features/resources/ManageResourcesPage.jsx +++ b/frontend/src/features/resources/ManageResourcesPage.jsx @@ -11,6 +11,8 @@ import { UsersIcon, PlusIcon, XIcon, + PencilIcon, + CheckIcon, } from "lucide-react"; import AddGroup from "./components/AddGroup"; import StateConditionalMenu from "../../components/StateVariables/StateConditionalMenu"; @@ -47,6 +49,8 @@ export default function ManageResourcesPage() { // Groups (each with files) const [groups, setGroups] = useState([]); const [selectedFile, setSelectedFile] = useState(null); + const [renamingFileId, setRenamingFileId] = useState(null); + const [renameInput, setRenameInput] = useState(""); // Load groups and files useEffect(() => { @@ -133,6 +137,50 @@ export default function ManageResourcesPage() { } } + async function renameFile(fileId, newName) { + try { + const user = getAuth().currentUser; + if (!user) { + toast.error("You must be logged in."); + return; + } + const idToken = await user.getIdToken(); + const { data } = await axios.patch( + `/api/files/${fileId}`, + { name: newName }, + { headers: { Authorization: `Bearer ${idToken}` } } + ); + + setGroups((prev) => + prev.map((g) => ({ + ...g, + files: (g.files || []).map((f) => + f.id === fileId ? { ...f, name: data.name } : f + ), + })) + ); + + if (selectedFile?.id === fileId) { + setSelectedFile((prev) => ({ ...prev, name: data.name })); + } + + toast.success("Renamed"); + } catch (err) { + console.error(err); + toast.error(err?.response?.data?.error || "Rename failed"); + } + } + + async function handleRenameSubmit() { + const trimmed = renameInput.trim(); + if (!trimmed) { setRenamingFileId(null); return; } + const current = groups.flatMap((g) => g.files).find((f) => f.id === renamingFileId); + if (current && trimmed !== current.name) { + await renameFile(renamingFileId, trimmed); + } + setRenamingFileId(null); + } + async function removeFile(fileId) { try { const user = getAuth().currentUser; @@ -301,27 +349,63 @@ export default function ManageResourcesPage() { {group.files.map((f) => (
  • -
    - - setSelectedFile({ - ...f, - groupId: group.id, - groupName: group.name, - }) - } - > - {f.name} - - -
    + {renamingFileId === f.id ? ( +
    + setRenameInput(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") handleRenameSubmit(); + if (e.key === "Escape") setRenamingFileId(null); + }} + /> + + +
    + ) : ( +
    + + setSelectedFile({ + ...f, + groupId: group.id, + groupName: group.name, + }) + } + > + {f.name} + + + +
    + )}
    • ))} From 10b3fbde491463790597f8e0c528eb1d2a7cad5b Mon Sep 17 00:00:00 2001 From: kmck133 Date: Tue, 26 May 2026 18:36:48 +1200 Subject: [PATCH 2/6] Fixed resourse page code format --- .../resources/ManageResourcesPage.jsx | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/frontend/src/features/resources/ManageResourcesPage.jsx b/frontend/src/features/resources/ManageResourcesPage.jsx index da899161a..0b6e70e87 100644 --- a/frontend/src/features/resources/ManageResourcesPage.jsx +++ b/frontend/src/features/resources/ManageResourcesPage.jsx @@ -173,8 +173,13 @@ export default function ManageResourcesPage() { async function handleRenameSubmit() { const trimmed = renameInput.trim(); - if (!trimmed) { setRenamingFileId(null); return; } - const current = groups.flatMap((g) => g.files).find((f) => f.id === renamingFileId); + if (!trimmed) { + setRenamingFileId(null); + return; + } + const current = groups + .flatMap((g) => g.files) + .find((f) => f.id === renamingFileId); if (current && trimmed !== current.name) { await renameFile(renamingFileId, trimmed); } @@ -355,10 +360,14 @@ export default function ManageResourcesPage() { autoFocus className="input input-bordered input-xs flex-1 min-w-0" value={renameInput} - onChange={(e) => setRenameInput(e.target.value)} + onChange={(e) => + setRenameInput(e.target.value) + } onKeyDown={(e) => { - if (e.key === "Enter") handleRenameSubmit(); - if (e.key === "Escape") setRenamingFileId(null); + if (e.key === "Enter") + handleRenameSubmit(); + if (e.key === "Escape") + setRenamingFileId(null); }} /> ) : ( -
    +
    setSelectedFile({ ...f, From bab4def5b27328216f29a90d33336339e2e088a2 Mon Sep 17 00:00:00 2001 From: kmck133 Date: Wed, 3 Jun 2026 12:35:32 +1200 Subject: [PATCH 4/6] Prettier + lint --- backend/src/routes/api/files.js | 4 ++- .../resources/ManageResourcesPage.jsx | 27 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/backend/src/routes/api/files.js b/backend/src/routes/api/files.js index 928c6931b..defd3f5f1 100644 --- a/backend/src/routes/api/files.js +++ b/backend/src/routes/api/files.js @@ -157,7 +157,9 @@ router.patch("/:fileId", async (req, res) => { return res.status(400).json({ error: "Name is required" }); } if (name.trim().length > 255) { - return res.status(400).json({ error: "Name must be 255 characters or fewer" }); + return res + .status(400) + .json({ error: "Name must be 255 characters or fewer" }); } const meta = await StoredFile.findById(fileId); if (!meta) return res.status(404).json({ error: "File not found" }); diff --git a/frontend/src/features/resources/ManageResourcesPage.jsx b/frontend/src/features/resources/ManageResourcesPage.jsx index 97176f8e4..08b0b9321 100644 --- a/frontend/src/features/resources/ManageResourcesPage.jsx +++ b/frontend/src/features/resources/ManageResourcesPage.jsx @@ -387,11 +387,24 @@ export default function ManageResourcesPage() {
    ) : ( -
    +
    setSelectedFile({ ...f, @@ -561,10 +574,14 @@ function Preview({ file }) { return (
    -
    -

    {file.name}

    +
    +

    {file.name}

    {downloadUrl && ( -     + Download )} From cf35f744995774d1739d6ab585a5f58fc8b3fadd Mon Sep 17 00:00:00 2001 From: kmck133 Date: Mon, 20 Jul 2026 16:56:36 +1200 Subject: [PATCH 5/6] bug fix: file name didn't wrap --- frontend/src/features/resources/ManageResourcesPage.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/features/resources/ManageResourcesPage.jsx b/frontend/src/features/resources/ManageResourcesPage.jsx index 68e7b8747..4e80a05ef 100644 --- a/frontend/src/features/resources/ManageResourcesPage.jsx +++ b/frontend/src/features/resources/ManageResourcesPage.jsx @@ -505,7 +505,7 @@ export default function ManageResourcesPage() {
    {selectedTargetType}
    -

    {selectedTarget.name}

    +

    {selectedTarget.name}

    ) : null} Date: Mon, 20 Jul 2026 17:28:50 +1200 Subject: [PATCH 6/6] fix: enforce file ownership checks and harden resource rename flow --- .../src/routes/api/__tests__/filesApi.test.js | 66 ++++++++++++++++++- backend/src/routes/api/files.js | 51 ++++++++------ .../resources/ManageResourcesPage.jsx | 51 +++++++------- 3 files changed, 119 insertions(+), 49 deletions(-) diff --git a/backend/src/routes/api/__tests__/filesApi.test.js b/backend/src/routes/api/__tests__/filesApi.test.js index 03147db6c..8157242e4 100644 --- a/backend/src/routes/api/__tests__/filesApi.test.js +++ b/backend/src/routes/api/__tests__/filesApi.test.js @@ -15,6 +15,7 @@ import FormData from "form-data"; import filesRouter from "../files.js"; import StoredFile from "../../../db/models/StoredFile.js"; import CollectionGroup from "../../../db/models/CollectionGroup.js"; +import Scenario from "../../../db/models/scenario.js"; import auth from "../../../middleware/firebaseAuth.js"; import errorHandler from "../../../middleware/errorHandler.js"; @@ -58,11 +59,17 @@ describe("Files API tests", () => { return app; }); - const scenarioId = new mongoose.mongo.ObjectId("ccc000000000000000000001"); + let scenarioId; let collectionGroup; let storedFile; beforeEach(async () => { + const scenario = await Scenario.create({ + name: "Test Scenario", + uid: "user1", + }); + scenarioId = scenario._id; + collectionGroup = await CollectionGroup.create({ scenarioId, name: "Test Group", @@ -200,6 +207,47 @@ describe("Files API tests", () => { ).rejects.toMatchObject({ response: { status: 404 } }); }); + it("DELETE /files/:fileId returns 403 when caller does not own the scenario", async () => { + await expect( + axios.delete( + `http://localhost:${ctx.port}/api/files/${storedFile._id}`, + authHeaders("stranger") + ) + ).rejects.toMatchObject({ response: { status: 403 } }); + }); + + // --- Rename file --- + + it("PATCH /files/:fileId renames the file", async () => { + const response = await axios.patch( + `http://localhost:${ctx.port}/api/files/${storedFile._id}`, + { name: "renamed.png" }, + authHeaders("user1") + ); + expect(response.status).toBe(200); + expect(response.data.name).toBe("renamed.png"); + }); + + it("PATCH /files/:fileId returns 400 when name is empty", async () => { + await expect( + axios.patch( + `http://localhost:${ctx.port}/api/files/${storedFile._id}`, + { name: " " }, + authHeaders("user1") + ) + ).rejects.toMatchObject({ response: { status: 400 } }); + }); + + it("PATCH /files/:fileId returns 403 when caller does not own the scenario", async () => { + await expect( + axios.patch( + `http://localhost:${ctx.port}/api/files/${storedFile._id}`, + { name: "renamed.png" }, + authHeaders("stranger") + ) + ).rejects.toMatchObject({ response: { status: 403 } }); + }); + // --- State conditionals --- it("POST /files/state-conditionals/:fileId adds a state conditional", async () => { @@ -219,6 +267,22 @@ describe("Files API tests", () => { expect(response.data.stateConditionals[0].stateVariableId).toBe("var-1"); }); + it("POST /files/state-conditionals/:fileId returns 403 when caller does not own the scenario", async () => { + await expect( + axios.post( + `http://localhost:${ctx.port}/api/files/state-conditionals/${storedFile._id}`, + { + stateConditional: { + stateVariableId: "var-1", + comparator: "=", + value: "open", + }, + }, + authHeaders("stranger") + ) + ).rejects.toMatchObject({ response: { status: 403 } }); + }); + it("POST /files/state-conditionals/:fileId returns 404 for unknown file", async () => { await expect( axios.post( diff --git a/backend/src/routes/api/files.js b/backend/src/routes/api/files.js index 61fae5185..4ec2697b1 100644 --- a/backend/src/routes/api/files.js +++ b/backend/src/routes/api/files.js @@ -2,6 +2,7 @@ import { Router } from "express"; import mongoose from "mongoose"; import multer from "multer"; import auth from "../../middleware/firebaseAuth.js"; +import { isAuthor } from "../../middleware/scenarioAuth.js"; import CollectionGroup from "../../db/models/CollectionGroup.js"; import StoredFile from "../../db/models/StoredFile.js"; import { @@ -81,6 +82,21 @@ async function assertGroupInScenario({ scenarioId, groupId }) { } } +/** + * Load a stored file and verify the caller has access to its scenario. + * Throws an Error tagged with .status (404/403) for the route's catch block. + */ +async function loadAuthorizedFile(fileId, uid) { + const meta = await StoredFile.findById(fileId); + if (!meta) { + throw Object.assign(new Error("File not found"), { status: 404 }); + } + if (!(await isAuthor(meta.scenarioId, uid))) { + throw Object.assign(new Error("Forbidden"), { status: 403 }); + } + return meta; +} + /** * @route POST /api/files/upload * @desc Upload one or more files to a group within a scenario @@ -152,8 +168,8 @@ router.post("/upload", upload.array("files"), async (req, res) => { router.patch("/:fileId", async (req, res) => { try { const { fileId } = req.params; - const { name } = req.body; - if (!name || !name.trim()) { + const { name, uid } = req.body; + if (typeof name !== "string" || !name.trim()) { return res.status(400).json({ error: "Name is required" }); } if (name.trim().length > 255) { @@ -161,15 +177,14 @@ router.patch("/:fileId", async (req, res) => { .status(400) .json({ error: "Name must be 255 characters or fewer" }); } - const meta = await StoredFile.findById(fileId); - if (!meta) return res.status(404).json({ error: "File not found" }); + const meta = await loadAuthorizedFile(fileId, uid); meta.name = name.trim(); await meta.save(); const file = meta.toObject(); delete file.gridFsId; return res.json(file); } catch (err) { - return res.status(500).json({ error: err.message }); + return res.status(err.status || 500).json({ error: err.message }); } }); @@ -180,15 +195,15 @@ router.patch("/:fileId", async (req, res) => { router.delete("/:fileId", async (req, res) => { try { const { fileId } = req.params; - const meta = await StoredFile.findById(fileId); - if (!meta) return res.status(404).json({ error: "File not found" }); + const { uid } = req.body; + const meta = await loadAuthorizedFile(fileId, uid); await deleteGridFsById(meta.gridFsId); await meta.deleteOne(); return res.json({ deleted: 1 }); } catch (err) { - return res.status(500).json({ error: err.message }); + return res.status(err.status || 500).json({ error: err.message }); } }); @@ -199,16 +214,15 @@ router.delete("/:fileId", async (req, res) => { router.post("/state-conditionals/:fileId", async (req, res) => { try { const { fileId } = req.params; - const { stateConditional } = req.body; - const meta = await StoredFile.findById(fileId); - if (!meta) return res.status(404).json({ error: "File not found" }); + const { stateConditional, uid } = req.body; + const meta = await loadAuthorizedFile(fileId, uid); meta.stateConditionals.push(stateConditional); await meta.save(); const file = meta.toObject(); delete file.gridFsId; return res.json(file); } catch (err) { - return res.status(500).json({ error: err.message }); + return res.status(err.status || 500).json({ error: err.message }); } }); @@ -219,9 +233,8 @@ router.post("/state-conditionals/:fileId", async (req, res) => { router.put("/state-conditionals/:fileId", async (req, res) => { try { const { fileId } = req.params; - const { stateConditional } = req.body; - const meta = await StoredFile.findById(fileId); - if (!meta) return res.status(404).json({ error: "File not found" }); + const { stateConditional, uid } = req.body; + const meta = await loadAuthorizedFile(fileId, uid); meta.stateConditionals = meta.stateConditionals.map((sc) => sc._id.toString() === stateConditional._id ? stateConditional : sc @@ -232,7 +245,7 @@ router.put("/state-conditionals/:fileId", async (req, res) => { delete file.gridFsId; return res.json(file); } catch (err) { - return res.status(500).json({ error: err.message }); + return res.status(err.status || 500).json({ error: err.message }); } }); @@ -245,8 +258,8 @@ router.delete( async (req, res) => { try { const { fileId, stateConditionalId } = req.params; - const meta = await StoredFile.findById(fileId); - if (!meta) return res.status(404).json({ error: "File not found" }); + const { uid } = req.body; + const meta = await loadAuthorizedFile(fileId, uid); const originalLength = meta.stateConditionals.length; meta.stateConditionals = meta.stateConditionals.filter( @@ -262,7 +275,7 @@ router.delete( delete file.gridFsId; return res.json(file); } catch (err) { - return res.status(500).json({ error: err.message }); + return res.status(err.status || 500).json({ error: err.message }); } } ); diff --git a/frontend/src/features/resources/ManageResourcesPage.jsx b/frontend/src/features/resources/ManageResourcesPage.jsx index 4e80a05ef..744b1f297 100644 --- a/frontend/src/features/resources/ManageResourcesPage.jsx +++ b/frontend/src/features/resources/ManageResourcesPage.jsx @@ -105,10 +105,6 @@ export default function ManageResourcesPage() { async function addFilesTo(groupId, files) { try { const user = getAuth().currentUser; - if (!user) { - toast.error("You must be logged in to upload."); - return; - } const idToken = await user.getIdToken(); const fd = new FormData(); @@ -154,10 +150,6 @@ export default function ManageResourcesPage() { async function renameFile(fileId, newName) { try { const user = getAuth().currentUser; - if (!user) { - toast.error("You must be logged in."); - return; - } const idToken = await user.getIdToken(); const { data } = await axios.patch( `/api/files/${fileId}`, @@ -174,28 +166,31 @@ export default function ManageResourcesPage() { })) ); - if (selectedFile?.id === fileId) { - setSelectedFile((prev) => ({ ...prev, name: data.name })); - } + setSelectedFile((prev) => + prev?.id === fileId ? { ...prev, name: data.name } : prev + ); toast.success("Renamed"); + return true; } catch (err) { console.error(err); toast.error(err?.response?.data?.error || "Rename failed"); + return false; } } async function handleRenameSubmit() { const trimmed = renameInput.trim(); if (!trimmed) { - setRenamingFileId(null); + toast.error("Name cannot be empty"); return; } const current = groups .flatMap((g) => g.files) .find((f) => f.id === renamingFileId); if (current && trimmed !== current.name) { - await renameFile(renamingFileId, trimmed); + const ok = await renameFile(renamingFileId, trimmed); + if (!ok) return; } setRenamingFileId(null); } @@ -203,10 +198,6 @@ export default function ManageResourcesPage() { async function removeFile(fileId) { try { const user = getAuth().currentUser; - if (!user) { - toast.error("You must be logged in to delete."); - return; - } const idToken = await user.getIdToken(); await axios.delete(`/api/files/${fileId}`, { headers: { Authorization: `Bearer ${idToken}` }, @@ -234,10 +225,6 @@ export default function ManageResourcesPage() { if (!ok) return; try { const user = getAuth().currentUser; - if (!user) { - toast.error("You must be logged in to delete."); - return; - } const idToken = await user.getIdToken(); await axios.delete(`/api/collections/groups/${groupId}`, { headers: { Authorization: `Bearer ${idToken}` }, @@ -340,7 +327,6 @@ export default function ManageResourcesPage() { onAdd={async (name) => { try { const user = getAuth().currentUser; - if (!user) return toast.error("You must be logged in."); const idToken = await user.getIdToken(); const { data } = await axios.post( "/api/collections/groups", @@ -451,13 +437,10 @@ export default function ManageResourcesPage() { }} > setSelectedFile({ ...f, @@ -465,6 +448,16 @@ export default function ManageResourcesPage() { groupName: group.name, }) } + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setSelectedFile({ + ...f, + groupId: group.id, + groupName: group.name, + }); + } + }} > {f.name} @@ -640,7 +633,7 @@ function Preview({ file }) {

    {file.name}

    {downloadUrl && (