From 0d7ce76c21646938b28740d9d89243befc1210ce Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Tue, 22 Oct 2024 20:24:11 +0530 Subject: [PATCH] Allow querying by user emails in order to `@mention` them (#7807) * fix: readEmail permission * fix: allow querying over user email in users.list * fix: allow searching by email in @mention * fix: include email in mentioned user's hover card * fix: put email on separate line in hover card --- app/components/HoverPreview/HoverPreview.tsx | 1 + .../HoverPreview/HoverPreviewMention.tsx | 3 +- app/editor/extensions/MentionMenu.tsx | 2 +- server/policies/user.ts | 18 ++- server/policies/utils.ts | 11 ++ server/presenters/unfurl.ts | 11 +- server/presenters/user.ts | 5 + server/routes/api/urls/urls.ts | 15 +- server/routes/api/users/users.test.ts | 152 +++++++++++++++++- server/routes/api/users/users.ts | 16 +- shared/types.ts | 2 + 11 files changed, 213 insertions(+), 23 deletions(-) diff --git a/app/components/HoverPreview/HoverPreview.tsx b/app/components/HoverPreview/HoverPreview.tsx index 4d1c6ec0bd3e..c6a79d8f6e89 100644 --- a/app/components/HoverPreview/HoverPreview.tsx +++ b/app/components/HoverPreview/HoverPreview.tsx @@ -125,6 +125,7 @@ function HoverPreviewDesktop({ element, data, dataLoading, onClose }: Props) { avatarUrl={data.avatarUrl} color={data.color} lastActive={data.lastActive} + email={data.email} /> ) : data.type === UnfurlResourceType.Document ? ( ; const HoverPreviewMention = React.forwardRef(function _HoverPreviewMention( - { avatarUrl, name, lastActive, color }: Props, + { avatarUrl, name, lastActive, color, email }: Props, ref: React.Ref ) { return ( @@ -25,6 +25,7 @@ const HoverPreviewMention = React.forwardRef(function _HoverPreviewMention( /> {name} + {email && {email}} {lastActive} diff --git a/app/editor/extensions/MentionMenu.tsx b/app/editor/extensions/MentionMenu.tsx index ffbf16cfe75e..4e9fc1c68eff 100644 --- a/app/editor/extensions/MentionMenu.tsx +++ b/app/editor/extensions/MentionMenu.tsx @@ -8,7 +8,7 @@ export default class MentionMenuExtension extends Suggestion { get defaultOptions() { return { // ported from https://github.com/tc39/proposal-regexp-unicode-property-escapes#unicode-aware-version-of-w - openRegex: /(?:^|\s|\()@([\p{L}\p{M}\d\s{1}]+)?$/u, + openRegex: /(?:^|\s|\()@([\p{L}\p{M}\d\s{1}@\.]+)?$/u, closeRegex: /(?:^|\s|\()@(([\p{L}\p{M}\d]*\s{2})|(\s+[\p{L}\p{M}\d]+))$/u, }; } diff --git a/server/policies/user.ts b/server/policies/user.ts index e254a3be5918..af8b7d897fb5 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -1,7 +1,14 @@ import { TeamPreference } from "@shared/types"; import { User, Team } from "@server/models"; import { allow } from "./cancan"; -import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils"; +import { + and, + isTeamAdmin, + isTeamMember, + isTeamModel, + isTeamMutable, + or, +} from "./utils"; allow(User, "read", User, isTeamModel); @@ -31,6 +38,15 @@ allow(User, ["update", "readDetails", "listApiKeys"], User, (actor, user) => ) ); +allow(User, "readEmail", User, (actor, user) => + or( + // + isTeamAdmin(actor, user), + isTeamMember(actor, user), + actor.id === user?.id + ) +); + allow(User, "delete", User, (actor, user) => or( isTeamAdmin(actor, user), diff --git a/server/policies/utils.ts b/server/policies/utils.ts index 176b9b9bbc7b..c7f97a0672fb 100644 --- a/server/policies/utils.ts +++ b/server/policies/utils.ts @@ -71,6 +71,17 @@ export function isTeamAdmin( return !!and(isTeamModel(actor, model), actor.isAdmin); } +/** + * Check if the actor is a member of the team. + * + * @param actor The actor to check + * @param model The model to check + * @returns True if the actor is a member of the team the model belongs to + */ +export function isTeamMember(actor: User, model: Model | null | undefined) { + return !!and(isTeamModel(actor, model), actor.isMember); +} + /** * Check the actors team is mutable, meaning the team models can be modified. * diff --git a/server/presenters/unfurl.ts b/server/presenters/unfurl.ts index c9fc1363729c..0a7826bbee80 100644 --- a/server/presenters/unfurl.ts +++ b/server/presenters/unfurl.ts @@ -6,10 +6,13 @@ import { Document, User, View } from "@server/models"; import { opts } from "@server/utils/i18n"; import { GitHubUtils } from "plugins/github/shared/GitHubUtils"; -async function presentUnfurl(data: Record) { +async function presentUnfurl( + data: Record, + options?: { includeEmail: boolean } +) { switch (data.type) { case UnfurlResourceType.Mention: - return presentMention(data); + return presentMention(data, options); case UnfurlResourceType.Document: return presentDocument(data); case UnfurlResourceType.PR: @@ -32,7 +35,8 @@ const presentOEmbed = ( }); const presentMention = async ( - data: Record + data: Record, + options?: { includeEmail: boolean } ): Promise => { const user: User = data.user; const document: Document = data.document; @@ -43,6 +47,7 @@ const presentMention = async ( return { type: UnfurlResourceType.Mention, name: user.name, + email: options && options.includeEmail ? user.email : null, avatarUrl: user.avatarUrl, color: user.color, lastActive: `${lastOnlineInfo} • ${lastViewedInfo}`, diff --git a/server/presenters/user.ts b/server/presenters/user.ts index d01d940f3cf1..c877a8603941 100644 --- a/server/presenters/user.ts +++ b/server/presenters/user.ts @@ -4,6 +4,7 @@ import { User } from "@server/models"; type Options = { includeDetails?: boolean; + includeEmail?: boolean; }; type UserPresentation = { @@ -45,5 +46,9 @@ export default function presentUser( userData.notificationSettings = user.notificationSettings; } + if (options.includeEmail) { + userData.email = user.email; + } + return userData; } diff --git a/server/routes/api/urls/urls.ts b/server/routes/api/urls/urls.ts index 55dfb220b789..e77b985128b8 100644 --- a/server/routes/api/urls/urls.ts +++ b/server/routes/api/urls/urls.ts @@ -10,7 +10,7 @@ import auth from "@server/middlewares/authentication"; import { rateLimiter } from "@server/middlewares/rateLimiter"; import validate from "@server/middlewares/validate"; import { Document, Share, Team, User } from "@server/models"; -import { authorize } from "@server/policies"; +import { authorize, can } from "@server/policies"; import presentUnfurl from "@server/presenters/unfurl"; import { APIContext, Unfurl } from "@server/types"; import { CacheHelper } from "@server/utils/CacheHelper"; @@ -53,11 +53,14 @@ router.post( authorize(actor, "read", user); authorize(actor, "read", document); - ctx.body = await presentUnfurl({ - type: UnfurlResourceType.Mention, - user, - document, - }); + ctx.body = await presentUnfurl( + { + type: UnfurlResourceType.Mention, + user, + document, + }, + { includeEmail: !!can(actor, "readEmail", user) } + ); return; } diff --git a/server/routes/api/users/users.test.ts b/server/routes/api/users/users.test.ts index 0d835987a76c..33f7bafb62fb 100644 --- a/server/routes/api/users/users.test.ts +++ b/server/routes/api/users/users.test.ts @@ -18,6 +18,24 @@ afterAll(() => { }); describe("#users.list", () => { + it("should return users whose emails match the query", async () => { + const user = await buildUser({ + name: "John Doe", + email: "john.doe@example.com", + }); + + const res = await server.post("/api/users.list", { + body: { + query: "john.doe@e", + token: user.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(1); + expect(body.data[0].id).toEqual(user.id); + }); + it("should allow filtering by user name", async () => { const user = await buildUser({ name: "Tèster", @@ -189,20 +207,142 @@ describe("#users.list", () => { expect(body.data[0].id).toEqual(user.id); }); - it("should require admin for detailed info", async () => { + it("should restrict guest from viewing other user's email", async () => { const team = await buildTeam(); - await buildAdmin({ teamId: team.id }); - const user = await buildUser({ teamId: team.id }); + await buildUser({ teamId: team.id }); + const guest = await buildUser({ teamId: team.id, role: UserRole.Guest }); const res = await server.post("/api/users.list", { body: { - token: user.getJwtToken(), + token: guest.getJwtToken(), }, }); const body = await res.json(); expect(res.status).toEqual(200); - expect(body.data.length).toEqual(2); + expect(body.data).toHaveLength(2); expect(body.data[0].email).toEqual(undefined); - expect(body.data[1].email).toEqual(user.email); + expect(body.data[1].email).toEqual(guest.email); + }); + + it("should restrict viewer from viewing other user's email", async () => { + const team = await buildTeam(); + await buildUser({ teamId: team.id }); + const viewer = await buildUser({ teamId: team.id, role: UserRole.Viewer }); + const res = await server.post("/api/users.list", { + body: { + token: viewer.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].email).toEqual(undefined); + expect(body.data[1].email).toEqual(viewer.email); + }); + + it("should allow member to view other user's email", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id }); + const member = await buildUser({ teamId: team.id, role: UserRole.Member }); + const res = await server.post("/api/users.list", { + body: { + token: member.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].email).toEqual(user.email); + expect(body.data[1].email).toEqual(member.email); + }); + + it("should restrict guest from viewing other user's details", async () => { + const team = await buildTeam(); + await buildUser({ teamId: team.id }); + const guest = await buildUser({ teamId: team.id, role: UserRole.Guest }); + const res = await server.post("/api/users.list", { + body: { + token: guest.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].language).toEqual(undefined); + expect(body.data[0].preferences).toEqual(undefined); + expect(body.data[0].notificationSettings).toEqual(undefined); + expect(body.data[1].language).toEqual(guest.language); + expect(body.data[1].preferences).toEqual(guest.preferences); + expect(body.data[1].notificationSettings).toEqual( + guest.notificationSettings + ); + }); + + it("should restrict viewer from viewing other user's details", async () => { + const team = await buildTeam(); + await buildUser({ teamId: team.id }); + const viewer = await buildUser({ teamId: team.id, role: UserRole.Viewer }); + const res = await server.post("/api/users.list", { + body: { + token: viewer.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].language).toEqual(undefined); + expect(body.data[0].preferences).toEqual(undefined); + expect(body.data[0].notificationSettings).toEqual(undefined); + expect(body.data[1].language).toEqual(viewer.language); + expect(body.data[1].preferences).toEqual(viewer.preferences); + expect(body.data[1].notificationSettings).toEqual( + viewer.notificationSettings + ); + }); + + it("should restrict member from viewing other user's details", async () => { + const team = await buildTeam(); + await buildUser({ teamId: team.id }); + const member = await buildUser({ teamId: team.id, role: UserRole.Member }); + const res = await server.post("/api/users.list", { + body: { + token: member.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].language).toEqual(undefined); + expect(body.data[0].preferences).toEqual(undefined); + expect(body.data[0].notificationSettings).toEqual(undefined); + expect(body.data[1].language).toEqual(member.language); + expect(body.data[1].preferences).toEqual(member.preferences); + expect(body.data[1].notificationSettings).toEqual( + member.notificationSettings + ); + }); + + it("should allow admin to view other user's details", async () => { + const team = await buildTeam(); + const admin = await buildAdmin({ teamId: team.id }); + const user = await buildUser({ teamId: team.id }); + const res = await server.post("/api/users.list", { + body: { + token: admin.getJwtToken(), + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data).toHaveLength(2); + expect(body.data[0].language).toEqual(user.language); + expect(body.data[0].preferences).toEqual(user.preferences); + expect(body.data[0].notificationSettings).toEqual( + user.notificationSettings + ); + expect(body.data[1].language).toEqual(admin.language); + expect(body.data[1].preferences).toEqual(admin.preferences); + expect(body.data[1].notificationSettings).toEqual( + admin.notificationSettings + ); }); }); diff --git a/server/routes/api/users/users.ts b/server/routes/api/users/users.ts index a745ff21fd75..71adf6467ad1 100644 --- a/server/routes/api/users/users.ts +++ b/server/routes/api/users/users.ts @@ -124,11 +124,16 @@ router.post( if (query) { where = { ...where, - [Op.and]: [ - Sequelize.literal( - `unaccent(LOWER(name)) like unaccent(LOWER(:query))` - ), - ], + [Op.and]: { + [Op.or]: [ + Sequelize.literal( + `unaccent(LOWER(email)) like unaccent(LOWER(:query))` + ), + Sequelize.literal( + `unaccent(LOWER(name)) like unaccent(LOWER(:query))` + ), + ], + }, }; } @@ -167,6 +172,7 @@ router.post( pagination: { ...ctx.state.pagination, total }, data: users.map((user) => presentUser(user, { + includeEmail: !!can(actor, "readEmail", user), includeDetails: !!can(actor, "readDetails", user), }) ), diff --git a/shared/types.ts b/shared/types.ts index 5a6488cb0761..627f6f0fd1a5 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -335,6 +335,8 @@ export type UnfurlResponse = { type: UnfurlResourceType.Mention; /** Mentioned user's name */ name: string; + /** Mentioned user's email */ + email: string | null; /** Mentioned user's avatar URL */ avatarUrl: string | null; /** Used to create mentioned user's avatar if no avatar URL provided */