From 20d85e3d3a4292a19c29ecdae98e36b592e10f5b Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Sat, 22 Apr 2023 20:48:51 +0530 Subject: [PATCH] Allow admin to change member's name (#5233) * feat: allow admins to change user names * fix: review --- app/components/ConfirmationDialog.tsx | 10 ++- app/components/UserRoleDialogs.tsx | 35 ++++++++ app/menus/UserMenu.tsx | 21 +++++ server/policies/user.ts | 4 + .../users/__snapshots__/users.test.ts.snap | 12 +-- server/routes/api/users/schema.ts | 15 +++- server/routes/api/users/users.test.ts | 33 +++++++ server/routes/api/users/users.ts | 86 ++++++++++--------- shared/i18n/locales/en_US/translation.json | 5 +- 9 files changed, 172 insertions(+), 49 deletions(-) diff --git a/app/components/ConfirmationDialog.tsx b/app/components/ConfirmationDialog.tsx index 317648985..74077fc29 100644 --- a/app/components/ConfirmationDialog.tsx +++ b/app/components/ConfirmationDialog.tsx @@ -15,6 +15,8 @@ type Props = { savingText?: string; /** If true, the submit button will be a dangerous red */ danger?: boolean; + /** Keep the submit button disabled */ + disabled?: boolean; }; const ConfirmationDialog: React.FC = ({ @@ -23,6 +25,7 @@ const ConfirmationDialog: React.FC = ({ submitText, savingText, danger, + disabled = false, }) => { const [isSaving, setIsSaving] = React.useState(false); const { dialogs } = useStores(); @@ -50,7 +53,12 @@ const ConfirmationDialog: React.FC = ({
{children} -
diff --git a/app/components/UserRoleDialogs.tsx b/app/components/UserRoleDialogs.tsx index 6ead42f0e..ceaf76205 100644 --- a/app/components/UserRoleDialogs.tsx +++ b/app/components/UserRoleDialogs.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import { useTranslation } from "react-i18next"; import User from "~/models/User"; import ConfirmationDialog from "~/components/ConfirmationDialog"; +import Input from "~/components/Input"; import useStores from "~/hooks/useStores"; type Props = { @@ -106,3 +107,37 @@ export function UserSuspendDialog({ user, onSubmit }: Props) { ); } + +export function UserChangeNameDialog({ user, onSubmit }: Props) { + const { t } = useTranslation(); + const [name, setName] = React.useState(user.name); + + const handleSubmit = async () => { + await user.save({ name }); + onSubmit(); + }; + + const handleChange = (ev: React.ChangeEvent) => { + setName(ev.target.value); + }; + + return ( + + + + ); +} diff --git a/app/menus/UserMenu.tsx b/app/menus/UserMenu.tsx index 3d18457f0..1d8192f7f 100644 --- a/app/menus/UserMenu.tsx +++ b/app/menus/UserMenu.tsx @@ -11,6 +11,7 @@ import { UserChangeToMemberDialog, UserChangeToViewerDialog, UserSuspendDialog, + UserChangeNameDialog, } from "~/components/UserRoleDialogs"; import usePolicy from "~/hooks/usePolicy"; import useStores from "~/hooks/useStores"; @@ -80,6 +81,20 @@ function UserMenu({ user }: Props) { [dialogs, t, user] ); + const handleChangeName = React.useCallback( + (ev: React.SyntheticEvent) => { + ev.preventDefault(); + dialogs.openModal({ + title: t("Change name"), + isCentered: true, + content: ( + + ), + }); + }, + [dialogs, t, user] + ); + const handleSuspend = React.useCallback( (ev: React.SyntheticEvent) => { ev.preventDefault(); @@ -154,6 +169,12 @@ function UserMenu({ user }: Props) { onClick: handlePromote, visible: can.promote && user.role !== "admin", }, + { + type: "button", + title: `${t("Change name")}…`, + onClick: handleChangeName, + visible: can.update && user.role !== "admin", + }, { type: "button", title: t("Resend invite"), diff --git a/server/policies/user.ts b/server/policies/user.ts index de7533fff..308db6ae0 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -28,6 +28,10 @@ allow(User, "update", User, (actor, user) => { return true; } + if (actor.isAdmin) { + return true; + } + return false; }); diff --git a/server/routes/api/users/__snapshots__/users.test.ts.snap b/server/routes/api/users/__snapshots__/users.test.ts.snap index fa5470807..3efbd54d6 100644 --- a/server/routes/api/users/__snapshots__/users.test.ts.snap +++ b/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", }, diff --git a/server/routes/api/users/schema.ts b/server/routes/api/users/schema.ts index 20f71ca42..28eb0d6db 100644 --- a/server/routes/api/users/schema.ts +++ b/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; diff --git a/server/routes/api/users/users.test.ts b/server/routes/api/users/users.test.ts index 8800e33cd..221012743 100644 --- a/server/routes/api/users/users.test.ts +++ b/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", { diff --git a/server/routes/api/users/users.ts b/server/routes/api/users/users.ts index 0884034d5..f312da7eb 100644 --- a/server/routes/api/users/users.ts +++ b/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) => { + 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) { + 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) => { diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index be1e0eddf..3f3c58e8a 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -229,6 +229,9 @@ "Are you sure you want to make {{ userName }} a member?": "Are you sure you want to make {{ userName }} a member?", "Are you sure you want to make {{ userName }} an admin? Admins can modify team and billing information.": "Are you sure you want to make {{ userName }} an admin? Admins can modify team and billing information.", "Are you sure you want to suspend {{ userName }}? Suspended users will be prevented from logging in.": "Are you sure you want to suspend {{ userName }}? Suspended users will be prevented from logging in.", + "Save": "Save", + "New name": "New name", + "Name can't be empty": "Name can't be empty", "Profile picture": "Profile picture", "Insert column after": "Insert column after", "Insert column before": "Insert column before", @@ -354,6 +357,7 @@ "Change role to admin": "Change role to admin", "Change role to member": "Change role to member", "Change role to viewer": "Change role to viewer", + "Change name": "Change name", "Suspend account": "Suspend account", "An error occurred while sending the invite": "An error occurred while sending the invite", "User options": "User options", @@ -388,7 +392,6 @@ "You can edit the name and other details at any time, however doing so often might confuse your team mates.": "You can edit the name and other details at any time, however doing so often might confuse your team mates.", "Name": "Name", "Sort": "Sort", - "Save": "Save", "Collections are for grouping your documents. They work best when organized around a topic or internal team — Product or Engineering for example.": "Collections are for grouping your documents. They work best when organized around a topic or internal team — Product or Engineering for example.", "This is the default level of access, you can give individual users or groups more access once the collection is created.": "This is the default level of access, you can give individual users or groups more access once the collection is created.", "Public document sharing": "Public document sharing",