fix: Disallow adding self to collection (#4299)

* api

* ui

* update collection permissions

app/scenes/CollectionPermissions/AddPeopleToCollection.tsx

@@ -109,7 +109,9 @@ class AddPeopleToCollection extends React.Component<Props> {
               <Empty>{t("No people left to add")}</Empty>
             )
           }
-          items={users.notInCollection(collection.id, this.query)}
+          items={users
+            .notInCollection(collection.id, this.query)
+            .filter((member) => member.id !== user.id)}
           fetch={this.query ? undefined : users.fetchPage}
           renderItem={(item: User) => (
             <MemberListItem

server/commands/userDemoter.test.ts

@@ -0,0 +1,41 @@
+import { CollectionPermission } from "@shared/types";
+import { CollectionUser } from "@server/models";
+import { UserRole } from "@server/models/User";
+import { buildUser, buildAdmin, buildCollection } from "@server/test/factories";
+import { getTestDatabase } from "@server/test/support";
+import userDemoter from "./userDemoter";
+
+const db = getTestDatabase();
+
+afterAll(db.disconnect);
+
+beforeEach(db.flush);
+
+describe("userDemoter", () => {
+  const ip = "127.0.0.1";
+
+  it("should change role and associated collection permissions", async () => {
+    const admin = await buildAdmin();
+    const user = await buildUser({ teamId: admin.teamId });
+    const collection = await buildCollection({ teamId: admin.teamId });
+
+    const membership = await CollectionUser.create({
+      createdById: admin.id,
+      userId: user.id,
+      collectionId: collection.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+
+    await userDemoter({
+      user,
+      actorId: admin.id,
+      to: UserRole.Viewer,
+      ip,
+    });
+
+    expect(user.isViewer).toEqual(true);
+
+    await membership.reload();
+    expect(membership.permission).toEqual(CollectionPermission.Read);
+  });
+});

server/models/User.ts

@@ -33,6 +33,7 @@ import env from "@server/env";
 import { ValidationError } from "../errors";
 import ApiKey from "./ApiKey";
 import Collection from "./Collection";
+import CollectionUser from "./CollectionUser";
 import NotificationSetting from "./NotificationSetting";
 import Star from "./Star";
 import Team from "./Team";
@@ -487,7 +488,7 @@ class User extends ParanoidModel {
 
     if (res.count >= 1) {
       if (to === "member") {
-        return this.update(
+        await this.update(
           {
             isAdmin: false,
             isViewer: false,
@@ -495,13 +496,24 @@ class User extends ParanoidModel {
           options
         );
       } else if (to === "viewer") {
-        return this.update(
+        await this.update(
           {
             isAdmin: false,
             isViewer: true,
           },
           options
         );
+        await CollectionUser.update(
+          {
+            permission: CollectionPermission.Read,
+          },
+          {
+            ...options,
+            where: {
+              userId: this.id,
+            },
+          }
+        );
       }
 
       return undefined;

server/routes/api/__snapshots__/collections.test.ts.snap

@@ -8,6 +8,14 @@ Object {
 }
 `;
 
+exports[`#collections.add_user should not allow add self 1`] = `
+Object {
+  "error": "authorization_error",
+  "message": "Authorization error",
+  "ok": false,
+}
+`;
+
 exports[`#collections.add_user should require user in team 1`] = `
 Object {
   "error": "authorization_error",

server/routes/api/collections.test.ts

@@ -422,6 +422,24 @@ describe("#collections.add_user", () => {
     expect(users.length).toEqual(2);
   });
 
+  it("should not allow add self", async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      permission: null,
+    });
+    const res = await server.post("/api/collections.add_user", {
+      body: {
+        token: user.getJwtToken(),
+        id: collection.id,
+        userId: user.id,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(403);
+    expect(body).toMatchSnapshot();
+  });
+
   it("should require user in team", async () => {
     const user = await buildUser();
     const collection = await buildCollection({

server/routes/api/collections.ts

@@ -9,7 +9,7 @@ import { RateLimiterStrategy } from "@server/RateLimiter";
 import collectionExporter from "@server/commands/collectionExporter";
 import teamUpdater from "@server/commands/teamUpdater";
 import { sequelize } from "@server/database/sequelize";
-import { ValidationError } from "@server/errors";
+import { AuthorizationError, ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import { rateLimiter } from "@server/middlewares/rateLimiter";
 import {
@@ -370,6 +370,10 @@ router.post("collections.add_user", auth(), async (ctx) => {
     },
   });
 
+  if (userId === ctx.state.user.id) {
+    throw AuthorizationError("You cannot add yourself to a collection");
+  }
+
   if (permission) {
     assertCollectionPermission(permission);
   }