feat: Use roles in API (#2198)

theopensystemslab · Sep 19, 2023 · 03e24e2 · 03e24e2
1 parent e122eef
commit 03e24e2
Show file tree

Hide file tree

Showing 28 changed files with 275 additions and 110 deletions.
diff --git a/api.planx.uk/admin/feedback/downloadFeedbackCSV.test.ts b/api.planx.uk/admin/feedback/downloadFeedbackCSV.test.ts
@@ -48,6 +48,7 @@ const mockFeedback: Feedback = {
 
 describe("Download feedback CSV endpoint", () => {
   afterEach(() => jest.clearAllMocks());
+  const auth = authHeader({ role: "platformAdmin" });
 
   it("requires a user to be logged in", async () => {
     await supertest(app)
@@ -60,10 +61,17 @@ describe("Download feedback CSV endpoint", () => {
       );
   });
 
+  it("requires a user to have the platform admin role", async () => {
+    await supertest(app)
+      .get(ENDPOINT)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("requires the 'cookie' query parameter", async () => {
     await supertest(app)
       .get(ENDPOINT)
-      .set(authHeader())
+      .set(auth)
       .expect(401)
       .then((res) => expect(res.body).toEqual({ error: "Missing cookie" }));
   });
@@ -72,7 +80,7 @@ describe("Download feedback CSV endpoint", () => {
     mockAxios.post.mockRejectedValue(new Error("FeedbackFish query failed!"));
     await supertest(app)
       .get(ENDPOINT)
-      .set(authHeader())
+      .set(auth)
       .query({ cookie: "test cookie" })
       .expect(500)
       .then((res) =>
@@ -87,7 +95,7 @@ describe("Download feedback CSV endpoint", () => {
     const cookie = "test cookie";
     await supertest(app)
       .get(ENDPOINT)
-      .set(authHeader())
+      .set(auth)
       .query({ cookie })
       .expect(200)
       .then(() => {
@@ -106,7 +114,7 @@ describe("Download feedback CSV endpoint", () => {
     const cookie = "test cookie";
     await supertest(app)
       .get(ENDPOINT)
-      .set(authHeader())
+      .set(auth)
       .query({ cookie })
       .expect(200)
       .expect("content-type", "text/csv; charset=utf-8");

diff --git a/api.planx.uk/admin/session/bops.test.ts b/api.planx.uk/admin/session/bops.test.ts
@@ -32,10 +32,17 @@ describe("BOPS payload admin endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", async () => {
+    await supertest(app)
+      .get(endpoint`123`)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("returns a JSON payload", async () => {
     await supertest(app)
       .get(endpoint`123`)
-      .set(authHeader())
+      .set(authHeader({ role: "platformAdmin" }))
       .expect(200)
       .expect("content-type", "application/json; charset=utf-8")
       .then((res) => expect(res.body).toEqual(expectedPayload));

diff --git a/api.planx.uk/admin/session/csv.test.ts b/api.planx.uk/admin/session/csv.test.ts
@@ -27,6 +27,7 @@ jest.mock("@opensystemslab/planx-core", () => {
 
 describe("CSV data admin endpoint", () => {
   afterEach(() => jest.clearAllMocks());
+  const auth = authHeader({ role: "platformAdmin" });
 
   it("requires a user to be logged in", async () => {
     await supertest(app)
@@ -39,10 +40,17 @@ describe("CSV data admin endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", async () => {
+    await supertest(app)
+      .get(endpoint`123`)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("returns a CSV-formatted payload", async () => {
     await supertest(app)
       .get(endpoint`123`)
-      .set(authHeader())
+      .set(auth)
       .expect(200)
       .expect("content-type", "application/json; charset=utf-8")
       .then((res) =>
@@ -59,7 +67,7 @@ describe("CSV data admin endpoint", () => {
   it("downloads a CSV file if a query parameter is passed", async () => {
     await supertest(app)
       .get(endpoint`123` + `?download=true`)
-      .set(authHeader())
+      .set(auth)
       .expect(200)
       .expect("content-type", "text/csv; charset=utf-8");
   });

diff --git a/api.planx.uk/admin/session/html.test.ts b/api.planx.uk/admin/session/html.test.ts
@@ -39,10 +39,17 @@ describe("HTML data admin endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", () => {
+    return supertest(app)
+      .get(endpoint`123`)
+      .set(authHeader({ role: "teamViewer" }))
+      .expect(403);
+  });
+
   it.skip("returns a HTML-formatted payload", () => {
     return supertest(app)
       .get(endpoint`123`)
-      .set(authHeader())
+      .set(authHeader({ role: "platformAdmin" }))
       .expect(200)
       .expect("content-type", "text/html; charset=utf-8")
       .then((res) =>

diff --git a/api.planx.uk/admin/session/oneAppXML.test.ts b/api.planx.uk/admin/session/oneAppXML.test.ts
@@ -30,6 +30,7 @@ describe("OneApp XML endpoint", () => {
   });
 
   afterEach(() => jest.clearAllMocks());
+  const auth = authHeader({ role: "platformAdmin" });
 
   it("requires a user to be logged in", async () => {
     await supertest(app)
@@ -42,18 +43,25 @@ describe("OneApp XML endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", async () => {
+    await supertest(app)
+      .get(endpoint`abc123`)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("returns an error if sessionID is invalid", async () => {
     await supertest(app)
       .get(endpoint`xyz789`)
-      .set(authHeader())
+      .set(auth)
       .expect(500)
       .then((res) => expect(res.body.error).toMatch(/Invalid sessionID/));
   });
 
   it("returns XML", async () => {
     await supertest(app)
       .get(endpoint`abc123`)
-      .set(authHeader())
+      .set(auth)
       .expect(200)
       .expect("content-type", "text/xml; charset=utf-8")
       .then((res) => {

diff --git a/api.planx.uk/admin/session/summary.test.ts b/api.planx.uk/admin/session/summary.test.ts
@@ -32,10 +32,17 @@ describe("Session summary admin endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", async () => {
+    await supertest(app)
+      .get(endpoint`abc123`)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("returns JSON", async () => {
     await supertest(app)
       .get(endpoint`abc123`)
-      .set(authHeader())
+      .set(authHeader({ role: "platformAdmin" }))
       .expect(200)
       .then((res) => {
         expect(res.text).toBe("{}");

diff --git a/api.planx.uk/admin/session/zip.test.ts b/api.planx.uk/admin/session/zip.test.ts
@@ -26,10 +26,17 @@ describe("zip data admin endpoint", () => {
       );
   });
 
+  it("requires a user to have the 'platformAdmin' role", async () => {
+    await supertest(app)
+      .get(endpoint`123`)
+      .set(authHeader({ role: "teamEditor" }))
+      .expect(403);
+  });
+
   it("downloads a zip file", async () => {
     await supertest(app)
       .get(endpoint`123`)
-      .set(authHeader())
+      .set(authHeader({ role: "platformAdmin" }))
       .expect(200)
       .expect("content-type", "application/zip");
   });

diff --git a/api.planx.uk/client/index.ts b/api.planx.uk/client/index.ts
@@ -4,7 +4,7 @@ import { CoreDomainClient } from "@opensystemslab/planx-core";
  * instead, they encapsulates query and business logic to only expose declarative interfaces
  */
 export const $admin = new CoreDomainClient({
-  hasuraSecret: process.env.HASURA_GRAPHQL_ADMIN_SECRET!,
+  auth: { adminSecret: process.env.HASURA_GRAPHQL_ADMIN_SECRET! },
   targetURL: process.env.HASURA_GRAPHQL_URL!,
 });
 

diff --git a/api.planx.uk/docs/index.ts b/api.planx.uk/docs/index.ts
@@ -66,7 +66,7 @@ const responses = {
           properties: {
             error: {
               type: "string",
-              enum: ["No authorization token was found"],
+              enum: ["Access denied"],
             },
           },
         },

diff --git a/api.planx.uk/editor/copyFlow.test.ts b/api.planx.uk/editor/copyFlow.test.ts
@@ -37,6 +37,8 @@ beforeEach(() => {
   });
 });
 
+const auth = authHeader({ role: "teamEditor" });
+
 it("returns an error if authorization headers are not set", async () => {
   const validBody = {
     insert: false,
@@ -54,6 +56,19 @@ it("returns an error if authorization headers are not set", async () => {
     });
 });
 
+it("returns an error if the user does not have the correct role", async () => {
+  const validBody = {
+    insert: false,
+    replaceValue: "T3ST",
+  };
+
+  await supertest(app)
+    .post("/flows/1/copy")
+    .send(validBody)
+    .set(authHeader({ role: "teamViewer" }))
+    .expect(403);
+});
+
 it("returns an error if required replacement characters are not provided in the request body", async () => {
   const invalidBody = {
     insert: false,
@@ -62,7 +77,7 @@ it("returns an error if required replacement characters are not provided in the
   await supertest(app)
     .post("/flows/1/copy")
     .send(invalidBody)
-    .set(authHeader())
+    .set(auth)
     .expect(400)
     .then((res) => {
       expect(res.body).toEqual({
@@ -80,7 +95,7 @@ it("returns copied unique flow data without inserting a new record", async () =>
   await supertest(app)
     .post("/flows/1/copy")
     .send(body)
-    .set(authHeader())
+    .set(auth)
     .expect(200)
     .then((res) => {
       expect(res.body).toEqual(mockCopyFlowResponse);
@@ -96,7 +111,7 @@ it("inserts copied unique flow data", async () => {
   await supertest(app)
     .post("/flows/1/copy")
     .send(body)
-    .set(authHeader())
+    .set(auth)
     .expect(200)
     .then((res) => {
       expect(res.body).toEqual(mockCopyFlowResponseInserted);

diff --git a/api.planx.uk/editor/copyFlow.ts b/api.planx.uk/editor/copyFlow.ts
@@ -8,10 +8,6 @@ const copyFlow = async (
   next: NextFunction,
 ): Promise<Response | NextFunction | void> => {
   try {
-    if (!req.user?.sub) {
-      return next({ status: 401, message: "User ID missing from JWT" });
-    }
-
     if (!req.params?.flowId || !req.body?.replaceValue) {
       return next({
         status: 400,
@@ -29,7 +25,7 @@ const copyFlow = async (
     const shouldInsert = (req.body?.insert as boolean) || false;
     if (shouldInsert) {
       const newSlug = flow.slug + "-copy";
-      const creatorId = parseInt(req.user.sub, 10);
+      const creatorId = parseInt(req.user!.sub!, 10);
       // Insert the flow and an associated operation
       await insertFlow(
         flow.team_id,

diff --git a/api.planx.uk/editor/copyPortalAsFlow.test.ts b/api.planx.uk/editor/copyPortalAsFlow.test.ts
@@ -17,10 +17,21 @@ beforeEach(() => {
   });
 });
 
+it("requires a user to be logged in", async () => {
+  await supertest(app).get("/flows/1/copy-portal/eyOm0NyDSl").expect(401);
+});
+
+it("requires a user to have the 'platformAdmin' role", async () => {
+  await supertest(app)
+    .get("/flows/1/copy-portal/eyOm0NyDSl")
+    .set(authHeader({ role: "teamEditor" }))
+    .expect(403);
+});
+
 it("throws an error if the portalNodeId parameter is not a portal (type = 300)", async () => {
   await supertest(app)
     .get("/flows/1/copy-portal/eyOm0NyDSl")
-    .set(authHeader())
+    .set(authHeader({ role: "platformAdmin" }))
     .expect(404)
     .then((res) => {
       expect(res.body).toEqual({
@@ -32,7 +43,7 @@ it("throws an error if the portalNodeId parameter is not a portal (type = 300)",
 it("returns transformed, unique flow data for a valid internal portal", async () => {
   await supertest(app)
     .get("/flows/1/copy-portal/MgCe3pSTrt")
-    .set(authHeader())
+    .set(authHeader({ role: "platformAdmin" }))
     .expect(200)
     .then((res) => {
       // the portalNodeId param should have been overwritten as _root

diff --git a/api.planx.uk/editor/copyPortalAsFlow.ts b/api.planx.uk/editor/copyPortalAsFlow.ts
@@ -11,10 +11,6 @@ const copyPortalAsFlow = async (
   next: NextFunction,
 ) => {
   try {
-    if (!req.user?.sub) {
-      return next({ status: 401, message: "User ID missing from JWT" });
-    }
-
     // fetch the parent flow data
     const flow = await getFlowData(req.params.flowId);
     if (!flow) {

diff --git a/api.planx.uk/editor/findReplace.test.ts b/api.planx.uk/editor/findReplace.test.ts
@@ -29,10 +29,23 @@ beforeEach(() => {
   });
 });
 
+const auth = authHeader({ role: "platformAdmin" });
+
+it("requires a user to be logged in", async () => {
+  await supertest(app).post("/flows/1/search").expect(401);
+});
+
+it("requires a user to have the 'platformAdmin' role", async () => {
+  await supertest(app)
+    .post("/flows/1/search")
+    .set(authHeader({ role: "teamEditor" }))
+    .expect(403);
+});
+
 it("throws an error if missing query parameter `find`", async () => {
   await supertest(app)
     .post("/flows/1/search")
-    .set(authHeader())
+    .set(auth)
     .expect(401)
     .then((res) => {
       expect(res.body).toEqual({
@@ -44,7 +57,7 @@ it("throws an error if missing query parameter `find`", async () => {
 it("finds matches", async () => {
   await supertest(app)
     .post("/flows/1/search?find=designated.monument")
-    .set(authHeader())
+    .set(auth)
     .expect(200)
     .then((res) => {
       expect(res.body).toEqual({
@@ -68,7 +81,7 @@ it("finds matches", async () => {
 it("does not replace if no matches are found", async () => {
   await supertest(app)
     .post("/flows/1/search?find=bananas&replace=monument")
-    .set(authHeader())
+    .set(auth)
     .expect(200)
     .then((res) => {
       expect(res.body).toEqual({
@@ -80,7 +93,7 @@ it("does not replace if no matches are found", async () => {
 it("updates flow data and returns matches if there are matches", async () => {
   await supertest(app)
     .post("/flows/1/search?find=designated.monument&replace=monument")
-    .set(authHeader())
+    .set(auth)
     .expect(200)
     .then((res) => {
       expect(res.body).toEqual({