feat: Allow viewers to be upgraded to editors on individual collections (#4023)

* Improve types

* More types, fix default permission for viewers added to collection

* fix change of default role for CollectionGroup

* Restore policy

* test

* tests
This commit is contained in:
Tom Moor
2022-08-31 08:12:27 +02:00
committed by GitHub
parent b8115ae3ce
commit 212985e18f
42 changed files with 537 additions and 435 deletions

View File

@@ -1,3 +1,4 @@
import { CollectionPermission } from "@shared/types";
import { CollectionUser, Collection } from "@server/models";
import { buildUser, buildTeam, buildCollection } from "@server/test/factories";
import { getTestDatabase } from "@server/test/support";
@@ -9,128 +10,248 @@ afterAll(db.disconnect);
beforeEach(db.flush);
describe("read_write permission", () => {
it("should allow read write permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
describe("member", () => {
describe("read_write permission", () => {
it("should allow read write permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.ReadWrite,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
const collection = await buildCollection({
teamId: team.id,
permission: "read_write",
it("should override read membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.ReadWrite,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.Read,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
it("should override read membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
describe("read permission", () => {
it("should allow read permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.Read,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
const collection = await buildCollection({
teamId: team.id,
permission: "read_write",
it("should allow override with read_write membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.Read,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.ReadWrite,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: "read",
});
describe("no permission", () => {
it("should allow no permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(false);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
it("should allow override with team member membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.ReadWrite,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
});
describe("read permission", () => {
it("should allow read permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
describe("viewer", () => {
describe("read_write permission", () => {
it("should allow read permissions for viewer", async () => {
const team = await buildTeam();
const user = await buildUser({
isViewer: true,
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.ReadWrite,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
const collection = await buildCollection({
teamId: team.id,
permission: "read",
it("should override read membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
isViewer: true,
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.ReadWrite,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.ReadWrite,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
it("should allow override with read_write membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
describe("read permission", () => {
it("should allow override with read_write membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
isViewer: true,
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.Read,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.ReadWrite,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
const collection = await buildCollection({
teamId: team.id,
permission: "read",
});
describe("no permission", () => {
it("should allow no permissions for viewer", async () => {
const team = await buildTeam();
const user = await buildUser({
isViewer: true,
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(false);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: "read_write",
it("should allow override with team member membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
isViewer: true,
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: CollectionPermission.ReadWrite,
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
});
describe("no permission", () => {
it("should allow no permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
const abilities = serialize(user, collection);
expect(abilities.read).toEqual(false);
expect(abilities.update).toEqual(false);
expect(abilities.share).toEqual(false);
});
it("should allow override with team member membership permission", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: null,
});
await CollectionUser.create({
createdById: user.id,
collectionId: collection.id,
userId: user.id,
permission: "read_write",
});
// reload to get membership
const reloaded = await Collection.scope({
method: ["withMembership", user.id],
}).findByPk(collection.id);
const abilities = serialize(user, reloaded);
expect(abilities.read).toEqual(true);
expect(abilities.update).toEqual(true);
expect(abilities.share).toEqual(true);
});
});

View File

@@ -1,5 +1,6 @@
import invariant from "invariant";
import { some } from "lodash";
import { CollectionPermission } from "@shared/types";
import { Collection, User, Team } from "@server/models";
import { AdminRequiredError } from "../errors";
import { allow } from "./cancan";
@@ -57,7 +58,7 @@ allow(User, ["read", "star", "unstar"], Collection, (user, collection) => {
...collection.collectionGroupMemberships,
];
return some(allMemberships, (m) =>
["read", "read_write", "maintainer"].includes(m.permission)
Object.values(CollectionPermission).includes(m.permission)
);
}
@@ -65,9 +66,6 @@ allow(User, ["read", "star", "unstar"], Collection, (user, collection) => {
});
allow(User, "share", Collection, (user, collection) => {
if (user.isViewer) {
return false;
}
if (!collection || user.teamId !== collection.teamId) {
return false;
}
@@ -78,7 +76,10 @@ allow(User, "share", Collection, (user, collection) => {
return true;
}
if (collection.permission !== "read_write") {
if (
collection.permission !== CollectionPermission.ReadWrite ||
user.isViewer
) {
invariant(
collection.memberships,
"membership should be preloaded, did you forget withMembership scope?"
@@ -87,8 +88,9 @@ allow(User, "share", Collection, (user, collection) => {
...collection.memberships,
...collection.collectionGroupMemberships,
];
return some(allMemberships, (m) =>
["read_write", "maintainer"].includes(m.permission)
return some(
allMemberships,
(m) => m.permission === CollectionPermission.ReadWrite
);
}
@@ -96,9 +98,6 @@ allow(User, "share", Collection, (user, collection) => {
});
allow(User, ["publish", "update"], Collection, (user, collection) => {
if (user.isViewer) {
return false;
}
if (!collection || user.teamId !== collection.teamId) {
return false;
}
@@ -106,7 +105,10 @@ allow(User, ["publish", "update"], Collection, (user, collection) => {
return true;
}
if (collection.permission !== "read_write") {
if (
collection.permission !== CollectionPermission.ReadWrite ||
user.isViewer
) {
invariant(
collection.memberships,
"membership should be preloaded, did you forget withMembership scope?"
@@ -115,8 +117,9 @@ allow(User, ["publish", "update"], Collection, (user, collection) => {
...collection.memberships,
...collection.collectionGroupMemberships,
];
return some(allMemberships, (m) =>
["read_write", "maintainer"].includes(m.permission)
return some(
allMemberships,
(m) => m.permission === CollectionPermission.ReadWrite
);
}
@@ -124,9 +127,6 @@ allow(User, ["publish", "update"], Collection, (user, collection) => {
});
allow(User, "delete", Collection, (user, collection) => {
if (user.isViewer) {
return false;
}
if (!collection || user.teamId !== collection.teamId) {
return false;
}
@@ -134,7 +134,10 @@ allow(User, "delete", Collection, (user, collection) => {
return true;
}
if (collection.permission !== "read_write") {
if (
collection.permission !== CollectionPermission.ReadWrite ||
user.isViewer
) {
invariant(
collection.memberships,
"membership should be preloaded, did you forget withMembership scope?"
@@ -143,8 +146,9 @@ allow(User, "delete", Collection, (user, collection) => {
...collection.memberships,
...collection.collectionGroupMemberships,
];
return some(allMemberships, (m) =>
["read_write", "maintainer"].includes(m.permission)
return some(
allMemberships,
(m) => m.permission === CollectionPermission.ReadWrite
);
}

View File

@@ -1,3 +1,4 @@
import { CollectionPermission } from "@shared/types";
import {
buildUser,
buildTeam,
@@ -14,14 +15,14 @@ afterAll(db.disconnect);
beforeEach(db.flush);
describe("read_write collection", () => {
it("should allow read write permissions for team member", async () => {
it("should allow read write permissions for member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: "read_write",
permission: CollectionPermission.ReadWrite,
});
const document = await buildDocument({
teamId: team.id,
@@ -37,17 +38,42 @@ describe("read_write collection", () => {
expect(abilities.share).toEqual(true);
expect(abilities.move).toEqual(true);
});
it("should allow read permissions for viewer", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
isViewer: true,
});
const collection = await buildCollection({
teamId: team.id,
permission: CollectionPermission.ReadWrite,
});
const document = await buildDocument({
teamId: team.id,
collectionId: collection.id,
});
const abilities = serialize(user, document);
expect(abilities.read).toEqual(true);
expect(abilities.download).toEqual(true);
expect(abilities.update).toEqual(false);
expect(abilities.createChildDocument).toEqual(false);
expect(abilities.archive).toEqual(false);
expect(abilities.delete).toEqual(false);
expect(abilities.share).toEqual(false);
expect(abilities.move).toEqual(false);
});
});
describe("read collection", () => {
it("should allow read only permissions permissions for team member", async () => {
it("should allow read permissions for team member", async () => {
const team = await buildTeam();
const user = await buildUser({
teamId: team.id,
});
const collection = await buildCollection({
teamId: team.id,
permission: "read",
permission: CollectionPermission.Read,
});
const document = await buildDocument({
teamId: team.id,

View File

@@ -209,9 +209,6 @@ allow(User, "delete", Document, (user, document) => {
if (document.deletedAt) {
return false;
}
if (user.isViewer) {
return false;
}
// allow deleting document without a collection
if (document.collection && cannot(user, "update", document.collection)) {
@@ -237,9 +234,6 @@ allow(User, "permanentDelete", Document, (user, document) => {
if (!document.deletedAt) {
return false;
}
if (user.isViewer) {
return false;
}
// allow deleting document without a collection
if (document.collection && cannot(user, "update", document.collection)) {
@@ -256,9 +250,6 @@ allow(User, "restore", Document, (user, document) => {
if (!document.deletedAt) {
return false;
}
if (user.isViewer) {
return false;
}
if (document.collection && cannot(user, "update", document.collection)) {
return false;

View File

@@ -22,6 +22,7 @@ it("should allow reading only", async () => {
expect(abilities.createGroup).toEqual(false);
expect(abilities.createIntegration).toEqual(false);
});
it("should allow admins to manage", async () => {
const team = await buildTeam();
const admin = await buildAdmin({