From c2762ce087736e9f21970f0c85b9f12951d8ca42 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 13 Nov 2025 19:24:06 -0500 Subject: [PATCH] fix: Update to require manage users policy for group management (#10637) --- server/routes/api/documents/documents.test.ts | 332 ++++++++++++++++++ server/routes/api/documents/documents.ts | 6 +- 2 files changed, 335 insertions(+), 3 deletions(-) diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index fb44cd83d8..0dbeb24c45 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -5249,6 +5249,338 @@ describe("#documents.remove_user", () => { }); }); +describe("#documents.add_group", () => { + it("should require id", async () => { + const user = await buildUser(); + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.message).toEqual("id: Must be a valid UUID or slug"); + }); + + it("should require authentication", async () => { + const document = await buildDocument(); + const group = await buildGroup(); + const res = await server.post("/api/documents.add_group", { + body: { + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(401); + }); + + it("should require authorization", async () => { + const document = await buildDocument(); + const user = await buildUser(); + const group = await buildGroup({ + teamId: user.teamId, + }); + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(403); + }); + + it("should require group in team", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup(); + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(403); + }); + + it("should add group to document", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup({ + teamId: user.teamId, + }); + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + const body = await res.json(); + const groupMemberships = await GroupMembership.findAll({ + where: { documentId: document.id }, + }); + expect(res.status).toEqual(200); + expect(groupMemberships.length).toEqual(1); + expect(body.data).not.toBeFalsy(); + expect(body.data.groupMemberships).not.toBeFalsy(); + expect(body.data.groupMemberships).toHaveLength(1); + expect(body.data.groupMemberships[0].groupId).toEqual(group.id); + expect(body.data.groupMemberships[0].documentId).toEqual(document.id); + expect(body.data.groupMemberships[0].permission).toEqual( + DocumentPermission.ReadWrite + ); + }); + + it("should add group to document with custom permission", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup({ + teamId: user.teamId, + }); + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + permission: DocumentPermission.Read, + }, + }); + const body = await res.json(); + const groupMemberships = await GroupMembership.findAll({ + where: { documentId: document.id }, + }); + expect(res.status).toEqual(200); + expect(groupMemberships.length).toEqual(1); + expect(body.data.groupMemberships[0].permission).toEqual( + DocumentPermission.Read + ); + }); + + it("should update existing group membership permission", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup({ + teamId: user.teamId, + }); + + // First add with Read permission + await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + permission: DocumentPermission.Read, + }, + }); + + // Then update to ReadWrite permission + const res = await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + permission: DocumentPermission.ReadWrite, + }, + }); + + const body = await res.json(); + const groupMemberships = await GroupMembership.findAll({ + where: { documentId: document.id }, + }); + expect(res.status).toEqual(200); + expect(groupMemberships.length).toEqual(1); + expect(body.data.groupMemberships[0].permission).toEqual( + DocumentPermission.ReadWrite + ); + + // Verify only one membership exists + const memberships = await GroupMembership.findAll({ + where: { + documentId: document.id, + groupId: group.id, + }, + }); + expect(memberships.length).toEqual(1); + }); +}); + +describe("#documents.remove_group", () => { + it("should require id", async () => { + const user = await buildUser(); + const res = await server.post("/api/documents.remove_group", { + body: { + token: user.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.message).toEqual("id: Must be a valid UUID or slug"); + }); + + it("should require authentication", async () => { + const document = await buildDocument(); + const group = await buildGroup(); + const res = await server.post("/api/documents.remove_group", { + body: { + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(401); + }); + + it("should require authorization", async () => { + const document = await buildDocument(); + const user = await buildUser(); + const group = await buildGroup({ + teamId: user.teamId, + }); + const res = await server.post("/api/documents.remove_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(403); + }); + + it("should require group in team", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup(); + const res = await server.post("/api/documents.remove_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + expect(res.status).toEqual(403); + }); + + it("should remove group from document", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup({ + teamId: user.teamId, + }); + + // First add the group + await server.post("/api/documents.add_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + + let groupMemberships = await GroupMembership.findAll({ + where: { documentId: document.id }, + }); + expect(groupMemberships.length).toEqual(1); + + // Then remove the group + const res = await server.post("/api/documents.remove_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + + groupMemberships = await GroupMembership.findAll({ + where: { documentId: document.id }, + }); + expect(res.status).toEqual(200); + expect(groupMemberships.length).toEqual(0); + }); + + it("should fail when group membership does not exist", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + createdById: user.id, + permission: null, + }); + const document = await buildDocument({ + collectionId: collection.id, + createdById: user.id, + teamId: user.teamId, + }); + const group = await buildGroup({ + teamId: user.teamId, + }); + + // Try to remove group that was never added + const res = await server.post("/api/documents.remove_group", { + body: { + token: user.getJwtToken(), + id: document.id, + groupId: group.id, + }, + }); + + expect(res.status).toEqual(404); + }); +}); + describe("#documents.memberships", () => { let actor: User, document: Document; beforeEach(async () => { diff --git a/server/routes/api/documents/documents.ts b/server/routes/api/documents/documents.ts index bf3f7af374..a4cff61fad 100644 --- a/server/routes/api/documents/documents.ts +++ b/server/routes/api/documents/documents.ts @@ -1771,8 +1771,8 @@ router.post( }), ]); - authorize(actor, "read", user); authorize(actor, "manageUsers", document); + authorize(actor, "read", user); const UserMemberships = await UserMembership.findAll({ where: { @@ -1893,7 +1893,7 @@ router.post( transaction, }), ]); - authorize(user, "update", document); + authorize(user, "manageUsers", document); authorize(user, "read", group); const [membership, created] = await GroupMembership.findOrCreate({ @@ -1947,7 +1947,7 @@ router.post( transaction, }), ]); - authorize(user, "update", document); + authorize(user, "manageUsers", document); authorize(user, "read", group); const membership = await GroupMembership.findOne({