fix: Actor on users.delete

* 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

package.json

@@ -348,5 +348,5 @@
     "js-yaml": "^3.14.1",
     "jpeg-js": "0.4.4"
   },
-  "version": "0.65.2"
+  "version": "0.66.1"
 }

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

@@ -28,6 +28,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";
@@ -419,7 +420,7 @@ class User extends ParanoidModel {
 
     if (res.count >= 1) {
       if (to === "member") {
-        return this.update(
+        await this.update(
           {
             isAdmin: false,
             isViewer: false,
@@ -427,13 +428,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 {
@@ -362,6 +362,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);
   }

server/routes/api/fileOperations.ts

@@ -7,6 +7,7 @@ import { FileOperation, Team } from "@server/models";
 import { FileOperationType } from "@server/models/FileOperation";
 import { authorize } from "@server/policies";
 import { presentFileOperation } from "@server/presenters";
+import { ContextWithState } from "@server/types";
 import { getSignedUrl } from "@server/utils/s3";
 import { assertIn, assertSort, assertUuid } from "@server/validation";
 import pagination from "./middlewares/pagination";
@@ -68,8 +69,8 @@ router.post(
   }
 );
 
-router.post("fileOperations.redirect", auth({ admin: true }), async (ctx) => {
-  const { id } = ctx.body;
+const handleFileOperationsRedirect = async (ctx: ContextWithState) => {
+  const { id } = ctx.body as { id?: string };
   assertUuid(id, "id is required");
 
   const { user } = ctx.state;
@@ -84,7 +85,18 @@ router.post("fileOperations.redirect", auth({ admin: true }), async (ctx) => {
 
   const accessUrl = await getSignedUrl(fileOperation.key);
   ctx.redirect(accessUrl);
-});
+};
+
+router.get(
+  "fileOperations.redirect",
+  auth({ admin: true }),
+  handleFileOperationsRedirect
+);
+router.post(
+  "fileOperations.redirect",
+  auth({ admin: true }),
+  handleFileOperationsRedirect
+);
 
 router.post("fileOperations.delete", auth({ admin: true }), async (ctx) => {
   const { id } = ctx.body;

server/routes/api/users.ts

@@ -402,6 +402,7 @@ router.post(
   rateLimiter(RateLimiterStrategy.TenPerHour),
   async (ctx) => {
     const { id, code = "" } = ctx.body;
+    const actor = ctx.state.user;
     let user: User;
 
     if (id) {
@@ -410,13 +411,13 @@ router.post(
         rejectOnEmpty: true,
       });
     } else {
-      user = ctx.state.user;
+      user = actor;
     }
-    authorize(user, "delete", user);
+    authorize(actor, "delete", user);
 
     // If we're attempting to delete our own account then a confirmation code
     // is required. This acts as CSRF protection.
-    if (!id || id === ctx.state.user.id) {
+    if (!id || id === actor.id) {
       const deleteConfirmationCode = user.deleteConfirmationCode;
 
       if (
@@ -433,7 +434,7 @@ router.post(
 
     await userDestroyer({
       user,
-      actor: user,
+      actor,
       ip: ctx.request.ip,
     });