From 01bcb7623adc548ae9497912eb052df1c4b0b429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 21 Dec 2023 14:11:10 +0000 Subject: [PATCH] feat: Set `httpOnly` flag in cookie (#2591) * feat: Refactor cookie generation, set httpOnly flag * feat: Update frontend code to handle httpOnly cookie * test(e2e): Update getUser URL * feat: Handle origin CORS header * fix: Update tiptap/link to resolve HTMLAttributes issue --- api.planx.uk/modules/auth/controller.ts | 102 ++++++++---------- api.planx.uk/server.ts | 11 +- .../ui-driven/src/create-flow/helpers.ts | 7 +- editor.planx.uk/package.json | 3 +- editor.planx.uk/pnpm-lock.yaml | 16 +-- editor.planx.uk/src/index.tsx | 37 +++---- .../src/pages/FlowEditor/lib/store/user.ts | 21 ++-- .../src/routes/views/authenticated.tsx | 4 +- 8 files changed, 84 insertions(+), 117 deletions(-) diff --git a/api.planx.uk/modules/auth/controller.ts b/api.planx.uk/modules/auth/controller.ts index 69ac070316..58a8472ef4 100644 --- a/api.planx.uk/modules/auth/controller.ts +++ b/api.planx.uk/modules/auth/controller.ts @@ -1,8 +1,6 @@ import { CookieOptions, RequestHandler, Response } from "express"; import { Request } from "express-jwt"; -import { isLiveEnv } from "../../helpers"; - export const failedLogin: RequestHandler = (_req, _res, next) => next({ status: 401, @@ -17,61 +15,55 @@ export const logout: RequestHandler = (req, res) => { }; export const handleSuccess = (req: Request, res: Response) => { - if (req.user) { - const { returnTo = process.env.EDITOR_URL_EXT } = req.session!; - - const domain = (() => { - if (isLiveEnv()) { - if (returnTo?.includes("editor.planx.")) { - // user is logging in to staging from editor.planx.dev - // or production from editor.planx.uk - return `.${new URL(returnTo).host}`; - } else { - // user is logging in from either a netlify preview build, - // or from localhost, to staging (or production... temporarily) - return undefined; - } - } else { - // user is logging in from localhost, to development - return "localhost"; - } - })(); - - if (domain) { - // As domain is set, we know that we're either redirecting back to - // editor.planx.dev/login, editor.planx.uk, or localhost:PORT - // (if this code is running in development). With the respective - // domain set in the cookie. - const cookie: CookieOptions = { - domain, - maxAge: new Date( - new Date().setFullYear(new Date().getFullYear() + 1), - ).getTime(), - httpOnly: false, - }; - - if (isLiveEnv()) { - cookie.secure = true; - cookie.sameSite = "none"; - } - - res.cookie("jwt", req.user.jwt, cookie); - - res.redirect(returnTo); - } else { - // Redirect back to localhost:PORT/login (if this API is in staging or - // production), or a netlify preview build url. As the login page is on a - // different domain to whatever this API is running on, we can't set a - // cookie. To solve this issue we inject the JWT into the return url as - // a parameter that can be extracted by the frontend code instead. - const url = new URL(returnTo); - url.searchParams.set("jwt", req.user.jwt); - res.redirect(url.href); - } - } else { - res.json({ + if (!req.user) { + return res.json({ message: "no user", success: true, }); } + + // Check referrer of original request + // This means requests from Pizzas to the staging API will not get flagged as `isStagingOrProd` + const { returnTo = process.env.EDITOR_URL_EXT } = req.session!; + if (!returnTo) throw Error("Can't generate returnTo URL from session"); + + const isStagingOrProd = returnTo.includes("editor.planx."); + + isStagingOrProd + ? setJWTCookie(returnTo, res, req) + : setJWTSearchParams(returnTo, res, req); }; + +/** + * Handle auth for staging and production + * + * Use a httpOnly cookie to pass the JWT securely back to the client. + * The client will then use the JWT to make authenticated requests to the API. + */ +function setJWTCookie(returnTo: string, res: Response, req: Request) { + const cookie: CookieOptions = { + domain: `.${new URL(returnTo).host}`, + maxAge: new Date( + new Date().setFullYear(new Date().getFullYear() + 1), + ).getTime(), + httpOnly: true, + secure: true, + sameSite: "none", + }; + + res.cookie("jwt", req.user!.jwt, cookie); + + res.redirect(returnTo); +} + +/** + * Handle auth for local development and Pizzas + * + * We can't use cookies cross-domain. + * Inject the JWT into the return URL, which can then be set as a cookie by the frontend + */ +function setJWTSearchParams(returnTo: string, res: Response, req: Request) { + const url = new URL(returnTo); + url.searchParams.set("jwt", req.user!.jwt); + res.redirect(url.href); +} diff --git a/api.planx.uk/server.ts b/api.planx.uk/server.ts index 5d0d2c18f7..2d2275cc11 100644 --- a/api.planx.uk/server.ts +++ b/api.planx.uk/server.ts @@ -38,19 +38,12 @@ useSwaggerDocs(app); app.set("trust proxy", 1); -app.use((req, res, next) => { - res.header("Access-Control-Allow-Origin", req.headers.origin); - res.header( - "Access-Control-Allow-Headers", - "Origin, X-Requested-With, Content-Type, Accept", - ); - next(); -}); - app.use( cors({ credentials: true, methods: "*", + origin: process.env.EDITOR_URL_EXT, + allowedHeaders: "Origin, X-Requested-With, Content-Type, Accept", }), ); diff --git a/e2e/tests/ui-driven/src/create-flow/helpers.ts b/e2e/tests/ui-driven/src/create-flow/helpers.ts index 2bc95b7eaf..edaea7b683 100644 --- a/e2e/tests/ui-driven/src/create-flow/helpers.ts +++ b/e2e/tests/ui-driven/src/create-flow/helpers.ts @@ -1,11 +1,8 @@ import { Browser, Page, Request } from "@playwright/test"; import { createAuthenticatedSession } from "../globalHelpers"; -export const isGetUserRequest = (req: Request) => { - const isHasuraRequest = req.url().includes("/graphql"); - const isGetUserOperation = req.postData()?.toString().includes("GetUserById"); - return Boolean(isHasuraRequest && isGetUserOperation); -}; +export const isGetUserRequest = (req: Request) => + req.url().includes("/user/me"); export async function getAdminPage({ browser, diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json index 5f950c3c8c..2b47434d65 100644 --- a/editor.planx.uk/package.json +++ b/editor.planx.uk/package.json @@ -25,7 +25,7 @@ "@tiptap/extension-history": "^2.0.3", "@tiptap/extension-image": "^2.0.3", "@tiptap/extension-italic": "^2.0.3", - "@tiptap/extension-link": "^2.0.3", + "@tiptap/extension-link": "^2.1.13", "@tiptap/extension-list-item": "^2.0.3", "@tiptap/extension-mention": "^2.1.8", "@tiptap/extension-ordered-list": "^2.1.8", @@ -52,7 +52,6 @@ "graphql-tag": "^2.12.6", "immer": "^9.0.21", "js-cookie": "^3.0.5", - "jwt-decode": "^4.0.0", "lodash": "^4.17.21", "marked": "^4.3.0", "mathjs": "^11.8.2", diff --git a/editor.planx.uk/pnpm-lock.yaml b/editor.planx.uk/pnpm-lock.yaml index 292ddbdefb..786d4015ed 100644 --- a/editor.planx.uk/pnpm-lock.yaml +++ b/editor.planx.uk/pnpm-lock.yaml @@ -78,8 +78,8 @@ dependencies: specifier: ^2.0.3 version: 2.0.3(@tiptap/core@2.0.3) '@tiptap/extension-link': - specifier: ^2.0.3 - version: 2.0.3(@tiptap/core@2.0.3)(@tiptap/pm@2.0.3) + specifier: ^2.1.13 + version: 2.1.13(@tiptap/core@2.0.3)(@tiptap/pm@2.0.3) '@tiptap/extension-list-item': specifier: ^2.0.3 version: 2.0.3(@tiptap/core@2.0.3) @@ -158,9 +158,6 @@ dependencies: js-cookie: specifier: ^3.0.5 version: 3.0.5 - jwt-decode: - specifier: ^4.0.0 - version: 4.0.0 lodash: specifier: ^4.17.21 version: 4.17.21 @@ -7126,8 +7123,8 @@ packages: '@tiptap/core': 2.0.3(@tiptap/pm@2.0.3) dev: false - /@tiptap/extension-link@2.0.3(@tiptap/core@2.0.3)(@tiptap/pm@2.0.3): - resolution: {integrity: sha512-H72tXQ5rkVCkAhFaf08fbEU7EBUCK0uocsqOF+4th9sOlrhfgyJtc8Jv5EXPDpxNgG5jixSqWBo0zKXQm9s9eg==} + /@tiptap/extension-link@2.1.13(@tiptap/core@2.0.3)(@tiptap/pm@2.0.3): + resolution: {integrity: sha512-wuGMf3zRtMHhMrKm9l6Tft5M2N21Z0UP1dZ5t1IlOAvOeYV2QZ5UynwFryxGKLO0NslCBLF/4b/HAdNXbfXWUA==} peerDependencies: '@tiptap/core': ^2.0.0 '@tiptap/pm': ^2.0.0 @@ -14422,11 +14419,6 @@ packages: setimmediate: 1.0.5 dev: false - /jwt-decode@4.0.0: - resolution: {integrity: sha512-+KJGIyHgkGuIq3IEBNftfhW/LfWhXUIY6OmyVWjliu5KH1y0fw7VQ8YndE2O4qZdMSd9SqbnC8GOcZEy0Om7sA==} - engines: {node: '>=18'} - dev: false - /keyv@4.5.4: resolution: {integrity: sha512-oxVHkHR/EJf2CNXnWxRLW6mg7JyCCUcG0DtEGmL2ctUo1PNTin1PUil+r/+4r5MpVgC/fn1kjsx7mjSujKqIpw==} dependencies: diff --git a/editor.planx.uk/src/index.tsx b/editor.planx.uk/src/index.tsx index 2f218a7b18..e8631ae66e 100644 --- a/editor.planx.uk/src/index.tsx +++ b/editor.planx.uk/src/index.tsx @@ -9,7 +9,6 @@ import { ApolloProvider } from "@apollo/client"; import CssBaseline from "@mui/material/CssBaseline"; import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles"; import { MyMap } from "@opensystemslab/map"; -import { jwtDecode } from "jwt-decode"; import { getCookie, setCookie } from "lib/cookie"; import ErrorPage from "pages/ErrorPage"; import { AnalyticsProvider } from "pages/FlowEditor/lib/analyticsProvider"; @@ -36,30 +35,18 @@ if (!window.customElements.get("my-map")) { } const hasJWT = (): boolean | void => { - let jwt = getCookie("jwt"); - if (jwt) { - try { - if ( - Number( - (jwtDecode(jwt) as any)["https://hasura.io/jwt/claims"][ - "x-hasura-user-id" - ], - ) > 0 - ) { - return true; - } - } catch (e) {} - window.location.href = "/logout"; - } else { - jwt = new URLSearchParams(window.location.search).get("jwt"); - if (jwt) { - setCookie("jwt", jwt); - // set the jwt, and remove it from the url, then re-run this function - window.location.href = "/"; - } else { - return false; - } - } + const jwtCookie = getCookie("jwt"); + if (jwtCookie) return true; + + // If JWT not set via cookie, check search params + const jwtSearchParams = new URLSearchParams(window.location.search).get( + "jwt", + ); + if (!jwtSearchParams) return false; + + // Remove JWT from URL, and re-run this function + setCookie("jwt", jwtSearchParams); + window.location.href = "/"; }; const Layout: React.FC<{ diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts index da46a652d8..0d4caa550e 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts @@ -1,6 +1,6 @@ import { User, UserTeams } from "@opensystemslab/planx-core/types"; +import axios from "axios"; import { _client } from "client"; -import { jwtDecode } from "jwt-decode"; import { Team } from "types"; import type { StateCreator } from "zustand"; @@ -10,7 +10,7 @@ export interface UserStore { setUser: (user: User) => void; getUser: () => User | undefined; canUserEditTeam: (teamSlug: Team["slug"]) => boolean; - initUserStore: (jwt: string) => Promise; + initUserStore: () => Promise; } export const userStore: StateCreator = ( @@ -31,15 +31,22 @@ export const userStore: StateCreator = ( return user.isPlatformAdmin || user.teams.some(hasTeamEditorRole); }, - async initUserStore(jwt: string) { + async initUserStore() { const { getUser, setUser } = get(); if (getUser()) return; - const id = (jwtDecode(jwt) as any)["sub"]; - const user = await _client.user.getById(id); - if (!user) throw new Error(`Failed to get user with ID ${id}`); - + const user = await getLoggedInUser(); setUser(user); }, }); + +const getLoggedInUser = async () => { + const url = `${process.env.REACT_APP_API_URL}/user/me`; + try { + const response = await axios.get(url, { withCredentials: true }); + return response.data; + } catch (error) { + throw Error("Failed to fetch user matching JWT cookie"); + } +}; diff --git a/editor.planx.uk/src/routes/views/authenticated.tsx b/editor.planx.uk/src/routes/views/authenticated.tsx index e621d788aa..c2760c3bf0 100644 --- a/editor.planx.uk/src/routes/views/authenticated.tsx +++ b/editor.planx.uk/src/routes/views/authenticated.tsx @@ -8,13 +8,13 @@ import AuthenticatedLayout from "../../pages/layout/AuthenticatedLayout"; /** * View wrapper for all authenticated routes - * Parses JWT and inits user store + * Initialises user store */ export const authenticatedView = async () => { const jwt = getCookie("jwt"); if (!jwt) return redirect("/login"); - await useStore.getState().initUserStore(jwt); + await useStore.getState().initUserStore(); useStore.getState().setPreviewEnvironment("editor");