Allow admin to change member's name (#5233)

* feat: allow admins to change user names

* fix: review

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

@@ -30,7 +30,7 @@ exports[`#users.activate should activate a suspended user 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },
@@ -87,7 +87,7 @@ exports[`#users.demote should demote an admin 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },
@@ -126,7 +126,7 @@ exports[`#users.demote should demote an admin to member 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },
@@ -165,7 +165,7 @@ exports[`#users.demote should demote an admin to viewer 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },
@@ -222,7 +222,7 @@ exports[`#users.promote should promote a new admin 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },
@@ -288,7 +288,7 @@ exports[`#users.suspend should suspend an user 1`] = `
         "readDetails": true,
         "resendInvite": true,
         "suspend": true,
-        "update": false,
+        "update": true,
       },
       "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     },

server/routes/api/users/schema.ts

@@ -1,5 +1,6 @@
 import { z } from "zod";
-import { NotificationEventType } from "@shared/types";
+import { NotificationEventType, UserPreference } from "@shared/types";
+import BaseSchema from "../BaseSchema";
 
 export const UsersNotificationsSubscribeSchema = z.object({
   body: z.object({
@@ -20,3 +21,15 @@ export const UsersNotificationsUnsubscribeSchema = z.object({
 export type UsersNotificationsUnsubscribeReq = z.infer<
   typeof UsersNotificationsUnsubscribeSchema
 >;
+
+export const UsersUpdateSchema = BaseSchema.extend({
+  body: z.object({
+    id: z.string().uuid().optional(),
+    name: z.string().optional(),
+    avatarUrl: z.string().optional(),
+    language: z.string().optional(),
+    preferences: z.record(z.nativeEnum(UserPreference), z.boolean()).optional(),
+  }),
+});
+
+export type UsersUpdateReq = z.infer<typeof UsersUpdateSchema>;

server/routes/api/users/users.test.ts

@@ -414,6 +414,39 @@ describe("#users.update", () => {
     expect(body.data.name).toEqual("New name");
   });
 
+  it("should allow admin to update other user's profile info", async () => {
+    const admin = await buildAdmin();
+    const user = await buildUser({
+      teamId: admin.teamId,
+    });
+    const res = await server.post("/api/users.update", {
+      body: {
+        id: user.id,
+        token: admin.getJwtToken(),
+        name: "New name",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.name).toEqual("New name");
+    expect(body.data.avatarUrl).toBe(user.avatarUrl);
+  });
+
+  it("should disallow non-admin to update other user's profile info", async () => {
+    const actor = await buildUser();
+    const user = await buildUser({
+      teamId: actor.teamId,
+    });
+    const res = await server.post("/api/users.update", {
+      body: {
+        id: user.id,
+        token: actor.getJwtToken(),
+        name: "New name",
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it("should fail upon sending invalid user preference", async () => {
     const { user } = await seed();
     const res = await server.post("/api/users.update", {

server/routes/api/users/users.ts

@@ -1,6 +1,5 @@
 import crypto from "crypto";
 import Router from "koa-router";
-import { has } from "lodash";
 import { Op, WhereOptions } from "sequelize";
 import { UserPreference } from "@shared/types";
 import { UserValidation } from "@shared/validations";
@@ -30,8 +29,6 @@ import {
   assertPresent,
   assertArray,
   assertUuid,
-  assertKeysIn,
-  assertBoolean,
 } from "@server/validation";
 import pagination from "../middlewares/pagination";
 import * as T from "./schema";
@@ -197,47 +194,56 @@ router.post("users.info", auth(), async (ctx: APIContext) => {
   };
 });
 
-router.post("users.update", auth(), transaction(), async (ctx: APIContext) => {
-  const { auth, transaction } = ctx.state;
-  const { user } = auth;
-  const { name, avatarUrl, language, preferences } = ctx.request.body;
-  if (name) {
-    user.name = name;
-  }
-  if (avatarUrl) {
-    user.avatarUrl = avatarUrl;
-  }
-  if (language) {
-    user.language = language;
-  }
-  if (preferences) {
-    assertKeysIn(preferences, UserPreference);
+router.post(
+  "users.update",
+  auth(),
+  transaction(),
+  validate(T.UsersUpdateSchema),
+  async (ctx: APIContext<T.UsersUpdateReq>) => {
+    const { auth, transaction } = ctx.state;
+    const actor = auth.user;
+    const { id, name, avatarUrl, language, preferences } = ctx.input.body;
 
-    for (const value of Object.values(UserPreference)) {
-      if (has(preferences, value)) {
-        assertBoolean(preferences[value]);
-        user.setPreference(value, preferences[value]);
+    let user: User | null = actor;
+    if (id) {
+      user = await User.findByPk(id);
+    }
+    authorize(actor, "update", user);
+    const includeDetails = can(actor, "readDetails", user);
+
+    if (name) {
+      user.name = name;
+    }
+    if (avatarUrl) {
+      user.avatarUrl = avatarUrl;
+    }
+    if (language) {
+      user.language = language;
+    }
+    if (preferences) {
+      for (const key of Object.keys(preferences) as Array<UserPreference>) {
+        user.setPreference(key, preferences[key] as boolean);
       }
     }
-  }
-  await user.save({ transaction });
-  await Event.create(
-    {
-      name: "users.update",
-      actorId: user.id,
-      userId: user.id,
-      teamId: user.teamId,
-      ip: ctx.request.ip,
-    },
-    { transaction }
-  );
+    await user.save({ transaction });
+    await Event.create(
+      {
+        name: "users.update",
+        actorId: user.id,
+        userId: user.id,
+        teamId: user.teamId,
+        ip: ctx.request.ip,
+      },
+      { transaction }
+    );
 
-  ctx.body = {
-    data: presentUser(user, {
-      includeDetails: true,
-    }),
-  };
-});
+    ctx.body = {
+      data: presentUser(user, {
+        includeDetails,
+      }),
+    };
+  }
+);
 
 // Admin specific
 router.post("users.promote", auth(), async (ctx: APIContext) => {