From 255204a6772bef3d7e993445d12d28f983cf3aa2 Mon Sep 17 00:00:00 2001
From: Mike <36415632+Mike-Heneghan@users.noreply.github.com>
Date: Mon, 27 Nov 2023 17:21:06 +0000
Subject: [PATCH] fix: remove sessionId from URLs (#2485)

* fix: remove resume on browser refresh to avoid exposing sessionId

- Temporarily removing the feature which allows users to resume on browser refresh
- The implementation exposed the sessionId which has security implications
- Removed the code for the feature and accompanying test

* fix: on loading a magic resume link and reading sessionId remove it from the url

- Exposing the sessionId has security implications
- The sessionId and the user email are required to successfully resume their session
- Read the sessionId but then immediately remove it from the url.
- This means it's barely visible and not dispalyed for the rest of the session

* refactor: reinstate test but check sessionId isn't in url

- As per: https://github.com/theopensystemslab/planx-new/pull/2485/files/fcc4df227e74eb399d496b3bb14e69a1617f4512#r1406392300
- Reinstate the test as it can add value checking that the sessionId is *not* in the url after being input by user
---
 editor.planx.uk/src/pages/Preview/ResumePage.tsx       | 10 +++++++++-
 .../src/pages/Preview/SaveAndReturn.test.tsx           |  4 ++--
 editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx    | 10 ----------
 editor.planx.uk/src/utils.ts                           |  6 ++++++
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/editor.planx.uk/src/pages/Preview/ResumePage.tsx b/editor.planx.uk/src/pages/Preview/ResumePage.tsx
index 370d0501a6..6edda24386 100644
--- a/editor.planx.uk/src/pages/Preview/ResumePage.tsx
+++ b/editor.planx.uk/src/pages/Preview/ResumePage.tsx
@@ -16,6 +16,7 @@ import { ApplicationPath, SendEmailPayload } from "types";
 import Input from "ui/Input";
 import InputLabel from "ui/InputLabel";
 import InputRow from "ui/InputRow";
+import { removeSessionIdSearchParamWithoutReloading } from "utils";
 import { object, string } from "yup";
 
 import ReconciliationPage from "./ReconciliationPage";
@@ -215,7 +216,14 @@ const ResumePage: React.FC = () => {
     getInitialEmailValue(route.url.query.email),
   );
   const [paymentRequest, setPaymentRequest] = useState<MinPaymentRequest>();
-  const sessionId = useCurrentRoute().url.query.sessionId;
+
+  // Read the sessionId from the url to validate against
+  const sessionId = route.url.query.sessionId;
+
+  // As the sessionId has been extracted it can now be removed to avoid
+  // unnecessarily exposing it
+  removeSessionIdSearchParamWithoutReloading();
+
   const [reconciliationResponse, setReconciliationResponse] =
     useState<ReconciliationResponse>();
 
diff --git a/editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx b/editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx
index 87a154a4c6..6c3e5bd5ac 100644
--- a/editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx
+++ b/editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx
@@ -67,7 +67,7 @@ describe("Save and Return component", () => {
     expect(results).toHaveNoViolations();
   });
 
-  it("stores the sessionId as part of the URL once an email has been submitted", async () => {
+  it("does not store the sessionId as part of the URL once an email has been submitted", async () => {
     const children = <Button>Testing 123</Button>;
     const { user } = setup(<SaveAndReturn children={children}></SaveAndReturn>);
 
@@ -89,7 +89,7 @@ describe("Save and Return component", () => {
       expect(screen.getByText("Testing 123")).toBeInTheDocument();
     });
 
-    expect(window.location.href).toContain(`sessionId=${sessionId}`);
+    expect(window.location.href).not.toContain(`sessionId=${sessionId}`);
   });
 });
 
diff --git a/editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx b/editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx
index 431c52d5a1..eaab65c3a4 100644
--- a/editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx
+++ b/editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx
@@ -84,20 +84,10 @@ const SaveAndReturn: React.FC<{ children: React.ReactNode }> = ({
   children,
 }) => {
   const isEmailCaptured = Boolean(useStore((state) => state.saveToEmail));
-  const sessionId = useStore((state) => state.sessionId);
   const isContentPage = useCurrentRoute()?.data?.isContentPage;
 
-  // Setting the URL search param "sessionId" will route the user to ApplicationPath.Resume
-  // Without this the user will need to click the magic link in their email after a refresh
-  const allowResumeOnBrowserRefresh = () => {
-    const url = new URL(window.location.href);
-    url.searchParams.set("sessionId", sessionId);
-    window.history.pushState({}, document.title, url);
-  };
-
   const handleSubmit = (email: string) => {
     useStore.setState({ saveToEmail: email });
-    allowResumeOnBrowserRefresh();
   };
 
   return (
diff --git a/editor.planx.uk/src/utils.ts b/editor.planx.uk/src/utils.ts
index 754cc1902f..edf1aa191c 100644
--- a/editor.planx.uk/src/utils.ts
+++ b/editor.planx.uk/src/utils.ts
@@ -62,3 +62,9 @@ export const removeSessionIdSearchParam = () => {
   window.history.pushState({}, document.title, currentURL);
   window.location.reload();
 };
+
+export const removeSessionIdSearchParamWithoutReloading = () => {
+  const currentURL = new URL(window.location.href);
+  currentURL.searchParams.delete("sessionId");
+  window.history.replaceState({}, document.title, currentURL);
+};