feat: Adds route-level role filtering. (#3734)

* feat: Adds route-level role filtering. Another layer in the onion of security and performance

* fix: Regression in authentication middleware

app/components/Sidebar/App.tsx

@@ -10,6 +10,7 @@ import Scrollable from "~/components/Scrollable";
 import Text from "~/components/Text";
 import { inviteUser } from "~/actions/definitions/users";
 import useCurrentTeam from "~/hooks/useCurrentTeam";
+import useCurrentUser from "~/hooks/useCurrentUser";
 import usePolicy from "~/hooks/usePolicy";
 import useStores from "~/hooks/useStores";
 import OrganizationMenu from "~/menus/OrganizationMenu";
@@ -34,12 +35,15 @@ function AppSidebar() {
   const { t } = useTranslation();
   const { documents } = useStores();
   const team = useCurrentTeam();
+  const user = useCurrentUser();
   const can = usePolicy(team.id);
 
   React.useEffect(() => {
-    documents.fetchDrafts();
-    documents.fetchTemplates();
-  }, [documents]);
+    if (!user.isViewer) {
+      documents.fetchDrafts();
+      documents.fetchTemplates();
+    }
+  }, [documents, user.isViewer]);
 
   const [dndArea, setDndArea] = React.useState();
   const handleSidebarRef = React.useCallback((node) => setDndArea(node), []);

app/routes/authenticated.tsx

@@ -1,3 +1,4 @@
+import { observer } from "mobx-react";
 import * as React from "react";
 import { Switch, Redirect, RouteComponentProps } from "react-router-dom";
 import Archive from "~/scenes/Archive";
@@ -11,6 +12,8 @@ import CenteredContent from "~/components/CenteredContent";
 import PlaceholderDocument from "~/components/PlaceholderDocument";
 import Route from "~/components/ProfiledRoute";
 import SocketProvider from "~/components/SocketProvider";
+import useCurrentTeam from "~/hooks/useCurrentTeam";
+import usePolicy from "~/hooks/usePolicy";
 import { matchDocumentSlug as slug } from "~/utils/routeHelpers";
 
 const SettingsRoutes = React.lazy(
@@ -59,7 +62,10 @@ const RedirectDocument = ({
   />
 );
 
-export default function AuthenticatedRoutes() {
+function AuthenticatedRoutes() {
+  const team = useCurrentTeam();
+  const can = usePolicy(team.id);
+
   return (
     <SocketProvider>
       <Layout>
@@ -71,14 +77,24 @@ export default function AuthenticatedRoutes() {
           }
         >
           <Switch>
+            {can.createDocument && (
+              <Route exact path="/templates" component={Templates} />
+            )}
+            {can.createDocument && (
+              <Route exact path="/templates/:sort" component={Templates} />
+            )}
+            {can.createDocument && (
+              <Route exact path="/drafts" component={Drafts} />
+            )}
+            {can.createDocument && (
+              <Route exact path="/archive" component={Archive} />
+            )}
+            {can.createDocument && (
+              <Route exact path="/trash" component={Trash} />
+            )}
             <Redirect from="/dashboard" to="/home" />
             <Route path="/home/:tab" component={Home} />
             <Route path="/home" component={Home} />
-            <Route exact path="/templates" component={Templates} />
-            <Route exact path="/templates/:sort" component={Templates} />
-            <Route exact path="/drafts" component={Drafts} />
-            <Route exact path="/archive" component={Archive} />
-            <Route exact path="/trash" component={Trash} />
             <Redirect exact from="/starred" to="/home" />
             <Redirect exact from="/collections/*" to="/collection/*" />
             <Route exact path="/collection/:id/new" component={DocumentNew} />
@@ -103,3 +119,5 @@ export default function AuthenticatedRoutes() {
     </SocketProvider>
   );
 }
+
+export default observer(AuthenticatedRoutes);

server/middlewares/authentication.ts

@@ -1,15 +1,28 @@
 import { Next } from "koa";
+import Logger from "@server/logging/Logger";
 import tracer, { APM } from "@server/logging/tracing";
 import { User, Team, ApiKey } from "@server/models";
 import { getUserForJWT } from "@server/utils/jwt";
-import { AuthenticationError, UserSuspendedError } from "../errors";
-import { ContextWithState } from "../types";
-
-export default function auth(
-  options: {
-    required?: boolean;
-  } = {}
-) {
+import {
+  AuthenticationError,
+  AuthorizationError,
+  UserSuspendedError,
+} from "../errors";
+import { ContextWithState, AuthenticationTypes } from "../types";
+
+type AuthenticationOptions = {
+  /* An admin user role is required to access the route */
+  admin?: boolean;
+  /* A member or admin user role is required to access the route */
+  member?: boolean;
+  /**
+   * Authentication is parsed, but optional. Note that if a token is provided
+   * in the request it must be valid or the requst will be rejected.
+   */
+  optional?: boolean;
+};
+
+export default function auth(options: AuthenticationOptions = {}) {
   return async function authMiddleware(ctx: ContextWithState, next: Next) {
     let token;
     const authorizationHeader = ctx.request.get("authorization");
@@ -29,8 +42,11 @@ export default function auth(
           `Bad Authorization header format. Format is "Authorization: Bearer <token>"`
         );
       }
-      // @ts-expect-error ts-migrate(2571) FIXME: Object is of type 'unknown'.
-    } else if (ctx.body && ctx.body.token) {
+    } else if (
+      ctx.body &&
+      typeof ctx.body === "object" &&
+      "token" in ctx.body
+    ) {
       // @ts-expect-error ts-migrate(2571) FIXME: Object is of type 'unknown'.
       token = ctx.body.token;
     } else if (ctx.request.query.token) {
@@ -39,15 +55,15 @@ export default function auth(
       token = ctx.cookies.get("accessToken");
     }
 
-    if (!token && options.required !== false) {
+    if (!token && options.optional !== true) {
       throw AuthenticationError("Authentication required");
     }
 
-    let user;
+    let user: User | null | undefined;
 
     if (token) {
       if (String(token).match(/^[\w]{38}$/)) {
-        ctx.state.authType = "api";
+        ctx.state.authType = AuthenticationTypes.API;
         let apiKey;
 
         try {
@@ -78,7 +94,7 @@ export default function auth(
           throw AuthenticationError("Invalid API key");
         }
       } else {
-        ctx.state.authType = "app";
+        ctx.state.authType = AuthenticationTypes.APP;
         user = await getUserForJWT(String(token));
       }
 
@@ -94,8 +110,23 @@ export default function auth(
         });
       }
 
+      if (options.admin) {
+        if (!user.isAdmin) {
+          throw AuthorizationError("Admin role required");
+        }
+      }
+
+      if (options.member) {
+        if (user.isViewer) {
+          throw AuthorizationError("Member role required");
+        }
+      }
+
       // not awaiting the promise here so that the request is not blocked
-      user.updateActiveAt(ctx.request.ip);
+      user.updateActiveAt(ctx.request.ip).catch((err) => {
+        Logger.error("Failed to update user activeAt", err);
+      });
+
       ctx.state.token = String(token);
       ctx.state.user = user;
 

server/models/User.ts

@@ -276,7 +276,7 @@ class User extends ParanoidModel {
       .map((c) => c.id);
   };
 
-  updateActiveAt = (ip: string, force = false) => {
+  updateActiveAt = async (ip: string, force = false) => {
     const fiveMinutesAgo = subMinutes(new Date(), 5);
 
     // ensure this is updated only every few minutes otherwise

server/routes/api/apiKeys.ts

@@ -8,7 +8,7 @@ import pagination from "./middlewares/pagination";
 
 const router = new Router();
 
-router.post("apiKeys.create", auth(), async (ctx) => {
+router.post("apiKeys.create", auth({ member: true }), async (ctx) => {
   const { name } = ctx.body;
   assertPresent(name, "name is required");
   const { user } = ctx.state;
@@ -35,24 +35,29 @@ router.post("apiKeys.create", auth(), async (ctx) => {
   };
 });
 
-router.post("apiKeys.list", auth(), pagination(), async (ctx) => {
-  const { user } = ctx.state;
-  const keys = await ApiKey.findAll({
-    where: {
-      userId: user.id,
-    },
-    order: [["createdAt", "DESC"]],
-    offset: ctx.state.pagination.offset,
-    limit: ctx.state.pagination.limit,
-  });
+router.post(
+  "apiKeys.list",
+  auth({ member: true }),
+  pagination(),
+  async (ctx) => {
+    const { user } = ctx.state;
+    const keys = await ApiKey.findAll({
+      where: {
+        userId: user.id,
+      },
+      order: [["createdAt", "DESC"]],
+      offset: ctx.state.pagination.offset,
+      limit: ctx.state.pagination.limit,
+    });
 
-  ctx.body = {
-    pagination: ctx.state.pagination,
-    data: keys.map(presentApiKey),
-  };
-});
+    ctx.body = {
+      pagination: ctx.state.pagination,
+      data: keys.map(presentApiKey),
+    };
+  }
+);
 
-router.post("apiKeys.delete", auth(), async (ctx) => {
+router.post("apiKeys.delete", auth({ member: true }), async (ctx) => {
   const { id } = ctx.body;
   assertUuid(id, "id is required");
   const { user } = ctx.state;

server/routes/api/documents.test.ts

@@ -15,6 +15,7 @@ import {
   buildCollection,
   buildUser,
   buildDocument,
+  buildViewer,
 } from "@server/test/factories";
 import { flushdb, seed } from "@server/test/support";
 
@@ -1432,12 +1433,76 @@ describe("#documents.archived", () => {
     expect(body.data.length).toEqual(0);
   });
 
+  it("should require member", async () => {
+    const viewer = await buildViewer();
+    const res = await server.post("/api/documents.archived", {
+      body: {
+        token: viewer.getJwtToken(),
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it("should require authentication", async () => {
     const res = await server.post("/api/documents.archived");
     expect(res.status).toEqual(401);
   });
 });
 
+describe("#documents.deleted", () => {
+  it("should return deleted documents", async () => {
+    const { user } = await seed();
+    const document = await buildDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    await document.delete(user.id);
+    const res = await server.post("/api/documents.deleted", {
+      body: {
+        token: user.getJwtToken(),
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(1);
+  });
+
+  it("should not return documents in private collections not a member of", async () => {
+    const { user } = await seed();
+    const collection = await buildCollection({
+      permission: null,
+    });
+    const document = await buildDocument({
+      teamId: user.teamId,
+      collectionId: collection.id,
+    });
+    await document.delete(user.id);
+    const res = await server.post("/api/documents.deleted", {
+      body: {
+        token: user.getJwtToken(),
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
+  it("should require member", async () => {
+    const viewer = await buildViewer();
+    const res = await server.post("/api/documents.deleted", {
+      body: {
+        token: viewer.getJwtToken(),
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+
+  it("should require authentication", async () => {
+    const res = await server.post("/api/documents.deleted");
+    expect(res.status).toEqual(401);
+  });
+});
+
 describe("#documents.viewed", () => {
   it("should return empty result if no views", async () => {
     const { user } = await seed();