Skip to content

Commit

Permalink
fix: remove sessionId from URLs (#2485)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Mike-Heneghan authored Nov 27, 2023
1 parent ef16e2a commit 255204a
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 13 deletions.
10 changes: 9 additions & 1 deletion editor.planx.uk/src/pages/Preview/ResumePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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>();

Expand Down
4 changes: 2 additions & 2 deletions editor.planx.uk/src/pages/Preview/SaveAndReturn.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>);

Expand All @@ -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}`);
});
});

Expand Down
10 changes: 0 additions & 10 deletions editor.planx.uk/src/pages/Preview/SaveAndReturn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
6 changes: 6 additions & 0 deletions editor.planx.uk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

0 comments on commit 255204a

Please sign in to comment.