fix: Restrict write access to flows.data column via GraphQL API (#2488

)

api.planx.uk/helpers.ts

@@ -2,7 +2,7 @@ import { gql } from "graphql-request";
 import { capitalize } from "lodash";
 import { Flow, Node } from "./types";
 import { ComponentType, FlowGraph } from "@opensystemslab/planx-core/types";
-import { $public, getClient } from "./client";
+import { $api, $public, getClient } from "./client";
 
 // Get a flow's data (unflattened, without external portal nodes)
 const getFlowData = async (id: string): Promise<Flow> => {
@@ -23,6 +23,11 @@ const getFlowData = async (id: string): Promise<Flow> => {
   return flow;
 };
 
+interface InsertFlow {
+  flow: {
+    id: string;
+  };
+}
 // Insert a new flow into the `flows` table
 const insertFlow = async (
   teamId: number,
@@ -32,40 +37,64 @@ const insertFlow = async (
   copiedFrom?: Flow["id"],
 ) => {
   const { client: $client } = getClient();
-  const data = await $client.request<{ flow: { id: string } }>(
-    gql`
-      mutation InsertFlow(
-        $team_id: Int!
-        $slug: String!
-        $data: jsonb = {}
-        $creator_id: Int
-        $copied_from: uuid
-      ) {
-        flow: insert_flows_one(
-          object: {
-            team_id: $team_id
-            slug: $slug
-            data: $data
-            version: 1
-            creator_id: $creator_id
-            copied_from: $copied_from
-          }
+  try {
+    const {
+      flow: { id },
+    } = await $client.request<InsertFlow>(
+      gql`
+        mutation InsertFlow(
+          $team_id: Int!
+          $slug: String!
+          $creator_id: Int
+          $copied_from: uuid
         ) {
-          id
+          flow: insert_flows_one(
+            object: {
+              team_id: $team_id
+              slug: $slug
+              version: 1
+              creator_id: $creator_id
+              copied_from: $copied_from
+            }
+          ) {
+            id
+          }
         }
-      }
-    `,
-    {
-      team_id: teamId,
-      slug: slug,
-      data: flowData,
-      creator_id: creatorId,
-      copied_from: copiedFrom,
-    },
-  );
+      `,
+      {
+        team_id: teamId,
+        slug: slug,
+        creator_id: creatorId,
+        copied_from: copiedFrom,
+      },
+    );
+
+    // Populate flow data using API role now that we know user has permission to insert flow
+    // Access to flow.data column is restricted to limit unsafe content that could be inserted
+    await $api.client.request<InsertFlow>(
+      gql`
+        mutation UpdateFlowData($data: jsonb = {}, $id: uuid!) {
+          flow: update_flows_by_pk(
+            pk_columns: { id: $id }
+            _set: { data: $data }
+          ) {
+            id
+          }
+        }
+      `,
+      {
+        id: id,
+        data: flowData,
+      },
+    );
 
-  if (data) await createAssociatedOperation(data?.flow?.id);
-  return data?.flow;
+    await createAssociatedOperation(id);
+    return { id };
+  } catch (error) {
+    throw Error(
+      `User ${creatorId} failed to insert flow to teamId ${teamId}. Please check permissions.`,
+    );
+  }
 };
 
 // Add a row to `operations` for an inserted flow, otherwise ShareDB throws a silent error when opening the flow in the UI

api.planx.uk/modules/flows/copyFlow/controller.ts

@@ -27,7 +27,7 @@ export type CopyFlowController = ValidatedRequestHandler<
 >;
 
 export const copyFlowController: CopyFlowController = async (
-  req,
+  _req,
   res,
   next,
 ) => {
@@ -48,7 +48,7 @@ export const copyFlowController: CopyFlowController = async (
     });
   } catch (error) {
     return next(
-      new ServerError({ message: "Failed to copy flow", cause: error }),
+      new ServerError({ message: `Failed to copy flow. Error: ${error}` }),
     );
   }
 };

api.planx.uk/modules/flows/copyFlow/copyFlow.test.ts

@@ -26,6 +26,16 @@ beforeEach(() => {
     },
   });
 
+  queryMock.mockQuery({
+    name: "UpdateFlowData",
+    matchOnVariables: false,
+    data: {
+      flow: {
+        id: 2,
+      },
+    },
+  });
+
   queryMock.mockQuery({
     name: "InsertOperation",
     matchOnVariables: false,
@@ -85,6 +95,53 @@ it("returns an error if required replacement characters are not provided in the
     });
 });
 
+it("returns an error if the operation to insert a new flow fails", async () => {
+  queryMock.reset();
+
+  const body = {
+    insert: true,
+    replaceValue: "T3ST1",
+  };
+
+  queryMock.mockQuery({
+    name: "GetFlowData",
+    matchOnVariables: false,
+    data: {
+      flow: {
+        data: mockFlowData,
+      },
+    },
+  });
+
+  queryMock.mockQuery({
+    name: "InsertFlow",
+    matchOnVariables: false,
+    data: {
+      flow: {
+        id: 2,
+      },
+    },
+    graphqlErrors: [
+      {
+        message: "Something went wrong",
+      },
+    ],
+    variables: {
+      id: "3",
+    },
+  });
+
+  await supertest(app)
+    .post("/flows/3/copy")
+    .send(body)
+    .set(auth)
+    .expect(500)
+    .then((res) => {
+      expect(res.body.error).toMatch(/failed to insert flow/);
+      expect(res.body.error).toMatch(/Please check permissions/);
+    });
+});
+
 it("returns copied unique flow data without inserting a new record", async () => {
   const body = {
     insert: false,

api.planx.uk/modules/flows/findReplace/controller.ts

@@ -4,6 +4,12 @@ import { z } from "zod";
 import { ServerError } from "../../../errors";
 import { findAndReplaceInFlow } from "./service";
 import { FlowGraph } from "@opensystemslab/planx-core/types";
+import { JSDOM } from "jsdom";
+import createDOMPurify from "dompurify";
+
+// Setup JSDOM and DOMPurify
+const window = new JSDOM("").window;
+const DOMPurify = createDOMPurify(window);
 
 interface FindAndReplaceResponse {
   message: string;
@@ -17,7 +23,10 @@ export const findAndReplaceSchema = z.object({
   }),
   query: z.object({
     find: z.string(),
-    replace: z.string().optional(),
+    replace: z
+      .string()
+      .optional()
+      .transform((val) => val && DOMPurify.sanitize(val)),
   }),
 });