fix: Open permissions for guests that have collection manage permission (#7075)

* fix: Opens up permissions for guests that have collection manage permission

* tsc

* tests
This commit is contained in:
Tom Moor
2024-06-20 09:18:18 -04:00
committed by GitHub
parent 95b9453269
commit a19fb25bea
2 changed files with 123 additions and 111 deletions

View File

@@ -316,112 +316,121 @@ describe("archived document", () => {
}); });
describe("read document", () => { describe("read document", () => {
it("should allow read permissions for team member", async () => { for (const role of Object.values(UserRole)) {
const team = await buildTeam(); it(`should allow read permissions for ${role}`, async () => {
const user = await buildUser({ teamId: team.id }); const team = await buildTeam();
const collection = await buildCollection({ const user = await buildUser({ teamId: team.id });
teamId: team.id, const collection = await buildCollection({
permission: null, teamId: team.id,
}); permission: null,
const doc = await buildDocument({ });
teamId: team.id, const doc = await buildDocument({
collectionId: collection.id, teamId: team.id,
}); collectionId: collection.id,
await UserMembership.create({ });
userId: user.id, await UserMembership.create({
documentId: doc.id, userId: user.id,
permission: DocumentPermission.Read, documentId: doc.id,
createdById: user.id, permission: DocumentPermission.Read,
}); createdById: user.id,
});
// reload to get membership // reload to get membership
const document = await Document.findByPk(doc.id, { userId: user.id }); const document = await Document.findByPk(doc.id, { userId: user.id });
const abilities = serialize(user, document); const abilities = serialize(user, document);
expect(abilities.read).toEqual(true); expect(abilities.read).toEqual(true);
expect(abilities.download).toEqual(true); expect(abilities.download).toEqual(true);
expect(abilities.subscribe).toEqual(true); expect(abilities.subscribe).toEqual(true);
expect(abilities.unsubscribe).toEqual(true); expect(abilities.unsubscribe).toEqual(true);
expect(abilities.comment).toEqual(true); expect(abilities.comment).toEqual(true);
expect(abilities.update).toEqual(false); expect(abilities.update).toEqual(false);
expect(abilities.createChildDocument).toEqual(false); expect(abilities.createChildDocument).toEqual(false);
expect(abilities.manageUsers).toEqual(false); expect(abilities.manageUsers).toEqual(false);
expect(abilities.archive).toEqual(false); expect(abilities.archive).toEqual(false);
expect(abilities.delete).toEqual(false); expect(abilities.delete).toEqual(false);
expect(abilities.share).toEqual(false); expect(abilities.share).toEqual(false);
expect(abilities.move).toEqual(false); expect(abilities.move).toEqual(false);
}); });
}
}); });
describe("read_write document", () => { describe("read_write document", () => {
it("should allow write permissions for team member", async () => { for (const role of Object.values(UserRole)) {
const team = await buildTeam(); it(`should allow write permissions for ${role}`, async () => {
const user = await buildUser({ teamId: team.id }); const team = await buildTeam();
const collection = await buildCollection({ const user = await buildUser({ teamId: team.id, role });
teamId: team.id, const collection = await buildCollection({
permission: null, teamId: team.id,
}); permission: null,
const doc = await buildDocument({ });
teamId: team.id, const doc = await buildDocument({
collectionId: collection.id, teamId: team.id,
}); collectionId: collection.id,
await UserMembership.create({ });
userId: user.id, await UserMembership.create({
documentId: doc.id, userId: user.id,
permission: DocumentPermission.ReadWrite, documentId: doc.id,
createdById: user.id, permission: DocumentPermission.ReadWrite,
}); createdById: user.id,
});
// reload to get membership // reload to get membership
const document = await Document.findByPk(doc.id, { userId: user.id }); const document = await Document.findByPk(doc.id, { userId: user.id });
const abilities = serialize(user, document); const abilities = serialize(user, document);
expect(abilities.read).toEqual(true); expect(abilities.read).toEqual(true);
expect(abilities.download).toEqual(true); expect(abilities.download).toEqual(true);
expect(abilities.update).toEqual(true); expect(abilities.update).toEqual(true);
expect(abilities.delete).toEqual(true); expect(abilities.delete).toEqual(true);
expect(abilities.subscribe).toEqual(true); expect(abilities.subscribe).toEqual(true);
expect(abilities.unsubscribe).toEqual(true); expect(abilities.unsubscribe).toEqual(true);
expect(abilities.comment).toEqual(true); expect(abilities.comment).toEqual(true);
expect(abilities.createChildDocument).toEqual(false); expect(abilities.createChildDocument).toEqual(false);
expect(abilities.manageUsers).toEqual(false); expect(abilities.manageUsers).toEqual(false);
expect(abilities.archive).toEqual(false); expect(abilities.archive).toEqual(false);
expect(abilities.share).toEqual(false); expect(abilities.share).toEqual(false);
expect(abilities.move).toEqual(false); expect(abilities.move).toEqual(false);
}); });
}
}); });
describe("manage document", () => { describe("manage document", () => {
it("should allow write permissions, user management, and sub-document creation", async () => { for (const role of Object.values(UserRole)) {
const team = await buildTeam(); it(`should allow write permissions, user management, and sub-document creation for ${role}`, async () => {
const user = await buildUser({ teamId: team.id }); const team = await buildTeam();
const collection = await buildCollection({ const user = await buildUser({
teamId: team.id, teamId: team.id,
permission: null, role,
}); });
const doc = await buildDocument({ const collection = await buildCollection({
teamId: team.id, teamId: team.id,
collectionId: collection.id, permission: null,
}); });
await UserMembership.create({ const doc = await buildDocument({
userId: user.id, teamId: team.id,
documentId: doc.id, collectionId: collection.id,
permission: DocumentPermission.Admin, });
createdById: user.id, await UserMembership.create({
}); userId: user.id,
documentId: doc.id,
permission: DocumentPermission.Admin,
createdById: user.id,
});
// reload to get membership // reload to get membership
const document = await Document.findByPk(doc.id, { userId: user.id }); const document = await Document.findByPk(doc.id, { userId: user.id });
const abilities = serialize(user, document); const abilities = serialize(user, document);
expect(abilities.read).toEqual(true); expect(abilities.read).toEqual(true);
expect(abilities.download).toEqual(true); expect(abilities.download).toEqual(true);
expect(abilities.update).toEqual(true); expect(abilities.update).toEqual(true);
expect(abilities.delete).toEqual(true); expect(abilities.delete).toEqual(true);
expect(abilities.subscribe).toEqual(true); expect(abilities.subscribe).toEqual(true);
expect(abilities.unsubscribe).toEqual(true); expect(abilities.unsubscribe).toEqual(true);
expect(abilities.comment).toEqual(true); expect(abilities.comment).toEqual(true);
expect(abilities.createChildDocument).toEqual(true); expect(abilities.createChildDocument).toEqual(true);
expect(abilities.manageUsers).toEqual(true); expect(abilities.manageUsers).toEqual(true);
expect(abilities.archive).toEqual(true); expect(abilities.archive).toEqual(true);
expect(abilities.move).toEqual(true); expect(abilities.move).toEqual(false);
expect(abilities.share).toEqual(false); expect(abilities.share).toEqual(false);
}); });
}
}); });

View File

@@ -35,10 +35,9 @@ allow(User, "read", Document, (actor, document) =>
); );
allow(User, ["listRevisions", "listViews"], Document, (actor, document) => allow(User, ["listRevisions", "listViews"], Document, (actor, document) =>
and( or(
// and(can(actor, "read", document), !actor.isGuest),
can(actor, "read", document), and(can(actor, "update", document), actor.isGuest)
!actor.isGuest
) )
); );
@@ -83,7 +82,6 @@ allow(User, "share", Document, (actor, document) =>
isTeamMutable(actor), isTeamMutable(actor),
!!document?.isActive, !!document?.isActive,
!document?.template, !document?.template,
!actor.isGuest,
or(!document?.collection, can(actor, "share", document?.collection)) or(!document?.collection, can(actor, "share", document?.collection))
) )
); );
@@ -114,12 +112,21 @@ allow(User, "publish", Document, (actor, document) =>
) )
); );
allow(User, ["move", "duplicate", "manageUsers"], Document, (actor, document) => allow(User, ["manageUsers", "duplicate"], Document, (actor, document) =>
and( and(
!actor.isGuest,
can(actor, "update", document), can(actor, "update", document),
or( or(
includesMembership(document, [DocumentPermission.Admin]), includesMembership(document, [DocumentPermission.Admin]),
can(actor, "updateDocument", document?.collection),
!!document?.isDraft && actor.id === document?.createdById
)
)
);
allow(User, "move", Document, (actor, document) =>
and(
can(actor, "update", document),
or(
can(actor, "updateDocument", document?.collection), can(actor, "updateDocument", document?.collection),
and(!!document?.isDraft && actor.id === document?.createdById) and(!!document?.isDraft && actor.id === document?.createdById)
) )
@@ -134,8 +141,7 @@ allow(User, "createChildDocument", Document, (actor, document) =>
can(actor, "read", document?.collection) can(actor, "read", document?.collection)
), ),
!document?.isDraft, !document?.isDraft,
!document?.template, !document?.template
!actor.isGuest
) )
); );
@@ -164,7 +170,6 @@ allow(User, "delete", Document, (actor, document) =>
and( and(
isTeamModel(actor, document), isTeamModel(actor, document),
isTeamMutable(actor), isTeamMutable(actor),
!actor.isGuest,
!document?.isDeleted, !document?.isDeleted,
or( or(
can(actor, "unarchive", document), can(actor, "unarchive", document),
@@ -193,7 +198,6 @@ allow(User, ["restore", "permanentDelete"], Document, (actor, document) =>
allow(User, "archive", Document, (actor, document) => allow(User, "archive", Document, (actor, document) =>
and( and(
!actor.isGuest,
!document?.template, !document?.template,
!document?.isDraft, !document?.isDraft,
!!document?.isActive, !!document?.isActive,
@@ -207,7 +211,6 @@ allow(User, "archive", Document, (actor, document) =>
allow(User, "unarchive", Document, (actor, document) => allow(User, "unarchive", Document, (actor, document) =>
and( and(
!actor.isGuest,
!document?.template, !document?.template,
!document?.isDraft, !document?.isDraft,
!document?.isDeleted, !document?.isDeleted,