Allow querying by user emails in order to @mention them (outline#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
FigPedro · Oct 22, 2024 · 0d7ce76 · 0d7ce76
1 parent c8d307c
commit 0d7ce76
Show file tree

Hide file tree

Showing 11 changed files with 213 additions and 23 deletions.
diff --git 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 ? (
               <HoverPreviewDocument

diff --git a/app/components/HoverPreview/HoverPreviewMention.tsx b/app/components/HoverPreview/HoverPreviewMention.tsx
@@ -7,7 +7,7 @@ import { Preview, Title, Info, Card, CardContent } from "./Components";
 type Props = Omit<UnfurlResponse[UnfurlResourceType.Mention], "type">;
 
 const HoverPreviewMention = React.forwardRef(function _HoverPreviewMention(
-  { avatarUrl, name, lastActive, color }: Props,
+  { avatarUrl, name, lastActive, color, email }: Props,
   ref: React.Ref<HTMLDivElement>
 ) {
   return (
@@ -25,6 +25,7 @@ const HoverPreviewMention = React.forwardRef(function _HoverPreviewMention(
             />
             <Flex column gap={2} justify="center">
               <Title>{name}</Title>
+              {email && <Info>{email}</Info>}
               <Info>{lastActive}</Info>
             </Flex>
           </Flex>

diff --git 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
@@ -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
@@ -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
@@ -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<string, any>) {
+async function presentUnfurl(
+  data: Record<string, any>,
+  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<string, any>
+  data: Record<string, any>,
+  options?: { includeEmail: boolean }
 ): Promise<UnfurlResponse[UnfurlResourceType.Mention]> => {
   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
@@ -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
@@ -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
@@ -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: "[email protected]",
+    });
+
+    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
@@ -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
@@ -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 */