From 612d01d2573b15385ef20da063cc11b771ef10c4 Mon Sep 17 00:00:00 2001 From: Theo <328805+theodesp@users.noreply.github.com> Date: Tue, 2 Apr 2024 15:18:41 +0100 Subject: [PATCH] Revert "Only request token endpoint initially, then use a cookie to determine if there is an authenticated user (#1740)" This reverts commit 0759959fa4949abe9835afa28e9ff34f795cc632. --- .changeset/brave-cougars-lie.md | 5 - package-lock.json | 16 -- packages/faustwp-core/package.json | 2 - .../src/components/Toolbar/Toolbar.tsx | 15 +- .../faustwp-core/src/server/auth/cookie.ts | 49 ++---- .../src/server/auth/middleware.ts | 1 - .../faustwp-core/src/server/auth/token.ts | 34 +--- .../tests/server/auth/cookie.test.ts | 28 ---- .../tests/server/auth/middleware.test.ts | 150 +----------------- 9 files changed, 20 insertions(+), 280 deletions(-) delete mode 100644 .changeset/brave-cougars-lie.md delete mode 100644 packages/faustwp-core/tests/server/auth/cookie.test.ts diff --git a/.changeset/brave-cougars-lie.md b/.changeset/brave-cougars-lie.md deleted file mode 100644 index e1359eec0..000000000 --- a/.changeset/brave-cougars-lie.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@faustwp/core': patch ---- - -Fixed the behavior of a request to the `api/faust/auth/token` endpoint on every page load when the toolbar is enabled. We now set a `WP_URL-has-rt` token with a `0` or `1` value that can be read client side (aka, not an `httpOnly` cookie) for determining if there is a logged in user or not. diff --git a/package-lock.json b/package-lock.json index 6801eef67..8695d0b45 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8415,12 +8415,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/js-cookie": { - "version": "3.0.6", - "resolved": "https://registry.npmjs.org/@types/js-cookie/-/js-cookie-3.0.6.tgz", - "integrity": "sha512-wkw9yd1kEXOPnvEeEV1Go1MmxtBJL0RR79aOTAApecWFVu7w0NNXNqhcWgvw2YgZDYadliXkl14pa3WXw5jlCQ==", - "dev": true - }, "node_modules/@types/jsdom": { "version": "20.0.1", "dev": true, @@ -19893,14 +19887,6 @@ "integrity": "sha512-WZzeDOEtTOBK4Mdsar0IqEU5sMr3vSV2RqkAIzUEV2BHnUfKGyswWFPFwK5EeDo93K3FohSHbLAjj0s1Wzd+dg==", "dev": true }, - "node_modules/js-cookie": { - "version": "3.0.5", - "resolved": "https://registry.npmjs.org/js-cookie/-/js-cookie-3.0.5.tgz", - "integrity": "sha512-cEiJEAEoIbWfCZYKWhVwFuvPX1gETRYPw6LlaTKoxD3s2AkXzkCjnp6h0V77ozyqj0jakteJ4YqDJT830+lVGw==", - "engines": { - "node": ">=14" - } - }, "node_modules/js-library-detector": { "version": "6.7.0", "resolved": "https://registry.npmjs.org/js-library-detector/-/js-library-detector-6.7.0.tgz", @@ -31806,7 +31792,6 @@ "deepmerge": "^4.2.2", "fast-xml-parser": "^4.2.5", "isomorphic-fetch": "^3.0.0", - "js-cookie": "^3.0.5", "js-sha256": "^0.9.0", "lodash": "^4.17.21", "zen-observable-ts": "^1.1.0" @@ -31818,7 +31803,6 @@ "@types/is-number": "^7.0.1", "@types/isomorphic-fetch": "^0.0.35", "@types/jest": "^27.0.2", - "@types/js-cookie": "^3.0.6", "@types/lodash": "^4.14.176", "@types/node": "^17.0.17", "@types/testing-library__react": "10.2.0", diff --git a/packages/faustwp-core/package.json b/packages/faustwp-core/package.json index 7f7af10c4..66532c62e 100644 --- a/packages/faustwp-core/package.json +++ b/packages/faustwp-core/package.json @@ -18,7 +18,6 @@ "@types/is-number": "^7.0.1", "@types/isomorphic-fetch": "^0.0.35", "@types/jest": "^27.0.2", - "@types/js-cookie": "^3.0.6", "@types/lodash": "^4.14.176", "@types/node": "^17.0.17", "@types/testing-library__react": "10.2.0", @@ -39,7 +38,6 @@ "deepmerge": "^4.2.2", "fast-xml-parser": "^4.2.5", "isomorphic-fetch": "^3.0.0", - "js-cookie": "^3.0.5", "js-sha256": "^0.9.0", "lodash": "^4.17.21", "zen-observable-ts": "^1.1.0" diff --git a/packages/faustwp-core/src/components/Toolbar/Toolbar.tsx b/packages/faustwp-core/src/components/Toolbar/Toolbar.tsx index c00853018..bceeaa665 100644 --- a/packages/faustwp-core/src/components/Toolbar/Toolbar.tsx +++ b/packages/faustwp-core/src/components/Toolbar/Toolbar.tsx @@ -1,16 +1,14 @@ import { gql, useQuery } from '@apollo/client'; -import cookies from 'js-cookie'; import React, { useEffect, useMemo, useState } from 'react'; import { getApolloAuthClient } from '../../client.js'; import { useAuth } from '../../hooks/useAuth.js'; -import { getWpUrl } from '../../lib/getWpUrl.js'; import { SeedNode } from '../../queries/seedQuery.js'; import { hooks } from '../../wpHooks/index.js'; -import { ToolbarNode } from './ToolbarNode.js'; import { Edit } from './nodes/Edit.js'; import { GraphiQL } from './nodes/GraphiQL.js'; import { MyAccount } from './nodes/MyAccount.js'; import { SiteName } from './nodes/SiteName.js'; +import { ToolbarNode } from './ToolbarNode.js'; /** * The available menu locations that nodes can be added to. @@ -217,16 +215,7 @@ export function ToolbarAwaitUser({ seedNode }: ToolbarProps) { * Renders a Toolbar that is based on WordPress' own toolbar. */ export function Toolbar({ seedNode }: ToolbarProps) { - const hasAuthenticatedUser = cookies.get(`${getWpUrl()}-has-rt`); - - const { isAuthenticated } = useAuth({ - strategy: 'redirect', - /** - * If the hasAuthenticatedUser cookie exists and it's "0", skip - * running the useAuth hook. - */ - skip: hasAuthenticatedUser === '0', - }); + const { isAuthenticated } = useAuth(); if (isAuthenticated !== true) { return null; diff --git a/packages/faustwp-core/src/server/auth/cookie.ts b/packages/faustwp-core/src/server/auth/cookie.ts index dfab790c4..fe7ba53f9 100644 --- a/packages/faustwp-core/src/server/auth/cookie.ts +++ b/packages/faustwp-core/src/server/auth/cookie.ts @@ -9,38 +9,6 @@ export interface CookieOptions { isJson?: boolean; } -/** - * Merge cookies from current Set-Cookie header with a new cookie string. - * - * @param setCookieHeader Current Set-Cookie header if exists. - * @param newCookie The new cookie string to be applied. - * @returns A cookie string or array of cookie strings. - */ -export function mergeCookies( - setCookieHeader: string | string[] | number | undefined, - newCookie: string, -) { - // If there is no setCookieHeader, return the newCookie early. - if (!setCookieHeader) { - return newCookie; - } - - /** - * If there is already a Set-Cookie header, create an array and merge - * the existing ones with the new cookie. - */ - let newCookies: string[] = []; - if (Array.isArray(setCookieHeader)) { - newCookies = [...setCookieHeader]; - } else { - newCookies = [setCookieHeader as string]; - } - - newCookies = [...newCookies, newCookie]; - - return newCookies; -} - export class Cookies { private request: IncomingMessage; @@ -90,13 +58,20 @@ export class Cookies { this.cookies[key] = cookieValue; - const existingCookieHeader = this.response?.getHeader('Set-Cookie'); - - const newCookies = mergeCookies( - existingCookieHeader, + this.response?.setHeader( + 'Set-Cookie', cookie.serialize(key, cookieValue, serializeOptions), ); + } - this.response?.setHeader('Set-Cookie', newCookies); + public removeCookie(key: string): void { + delete this.cookies[key]; + + this.response?.setHeader( + 'Set-Cookie', + cookie.serialize(key, '', { + expires: new Date(0), + }), + ); } } diff --git a/packages/faustwp-core/src/server/auth/middleware.ts b/packages/faustwp-core/src/server/auth/middleware.ts index 77c86b3f5..0f8edb6a1 100644 --- a/packages/faustwp-core/src/server/auth/middleware.ts +++ b/packages/faustwp-core/src/server/auth/middleware.ts @@ -32,7 +32,6 @@ export async function authorizeHandler( if (!refreshToken && !code) { res.statusCode = 401; - oauth.setRefreshToken(undefined); res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify({ error: 'Unauthorized' })); diff --git a/packages/faustwp-core/src/server/auth/token.ts b/packages/faustwp-core/src/server/auth/token.ts index 192f1f24b..f11f26cb5 100644 --- a/packages/faustwp-core/src/server/auth/token.ts +++ b/packages/faustwp-core/src/server/auth/token.ts @@ -23,12 +23,9 @@ export class OAuth { private tokenKey: string; - private hasTokenKey: string; - constructor(cookies: Cookies) { this.cookies = cookies; this.tokenKey = `${getWpUrl()}-rt`; - this.hasTokenKey = `${getWpUrl()}-has-rt`; } public getRefreshToken(): string | undefined { @@ -36,40 +33,19 @@ export class OAuth { } public setRefreshToken(token?: string, expires?: number): void { - let maxAge: number | undefined = 2592000; - let expiresIn: Date | undefined; - if (!isString(token) || token.length === 0) { - this.cookies.setCookie(this.tokenKey, '', { - path: '/', - expires: new Date(0), - secure: true, - httpOnly: true, - }); - - this.cookies.setCookie(this.hasTokenKey, '0', { - path: '/', - encoded: false, - maxAge, - expires: expiresIn, - }); - - return; + this.cookies.removeCookie(this.tokenKey); } + let maxAge: number | undefined = 2592000; + let expiresIn: Date | undefined; + if (isNumber(expires)) { expiresIn = new Date(expires * 1000); maxAge = undefined; } - this.cookies.setCookie(this.hasTokenKey, '1', { - path: '/', - encoded: false, - maxAge, - expires: expiresIn, - }); - - this.cookies.setCookie(this.tokenKey, token, { + this.cookies.setCookie(this.tokenKey, token as string, { expires: expiresIn, maxAge, path: '/', diff --git a/packages/faustwp-core/tests/server/auth/cookie.test.ts b/packages/faustwp-core/tests/server/auth/cookie.test.ts deleted file mode 100644 index b350d4d09..000000000 --- a/packages/faustwp-core/tests/server/auth/cookie.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { mergeCookies } from '../../../src/server/auth/cookie'; - -describe('mergeCookies', () => { - it('merges cookies from an existing setCookie header and a new cookie', () => { - const existingSetCookieHeader = `http://headless.local-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure`; - const newCookie = `http://headless.local-has-rt=0; Max-Age=2592000; Path=/`; - const result = mergeCookies(existingSetCookieHeader, newCookie); - - expect(result).toStrictEqual([existingSetCookieHeader, newCookie]); - }); - - it('returns the cookie if existing set cookie header does not exist', () => { - const newCookie = `http://headless.local-has-rt=0; Max-Age=2592000; Path=/`; - - expect(mergeCookies(undefined, newCookie)).toStrictEqual(newCookie); - }); - - it('merges cookies from an existing array of setCookies', () => { - const existingSetCookieHeader = [ - `http://headless.local-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure`, - `http://testing.local-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure`, - ]; - const newCookie = `http://headless.local-has-rt=0; Max-Age=2592000; Path=/`; - const result = mergeCookies(existingSetCookieHeader, newCookie); - - expect(result).toStrictEqual([...existingSetCookieHeader, newCookie]); - }); -}); diff --git a/packages/faustwp-core/tests/server/auth/middleware.test.ts b/packages/faustwp-core/tests/server/auth/middleware.test.ts index 4d2977deb..6cd5177c5 100644 --- a/packages/faustwp-core/tests/server/auth/middleware.test.ts +++ b/packages/faustwp-core/tests/server/auth/middleware.test.ts @@ -1,26 +1,11 @@ import 'isomorphic-fetch'; import fetchMock from 'fetch-mock'; import { IncomingMessage, ServerResponse } from 'http'; -import { - authorizeHandler, - logoutHandler, -} from '../../../src/server/auth/middleware'; +import { authorizeHandler } from '../../../src/server/auth/middleware'; import * as getWpUrl from '../../../src/lib/getWpUrl'; import * as getWpSecret from '../../../src/lib/getWpSecret'; -import { base64Encode } from '../../../src/utils'; describe('auth/middleware', () => { - const envBackup = process.env; - - beforeEach(() => { - process.env = { ...envBackup }; - process.env.NEXT_PUBLIC_WORDPRESS_URL = 'http://headless.local'; - }); - - afterAll(() => { - process.env = envBackup; - }); - test('authorizeHandler will send a 401 when there is no code or refresh token', async () => { const req: IncomingMessage = { headers: {}, @@ -29,12 +14,10 @@ describe('auth/middleware', () => { const res: ServerResponse = { setHeader() {}, writeHead() {}, - getHeader() {}, end() {}, } as any; const endSpy = jest.spyOn(res, 'end'); - const setHeaderSpy = jest.spyOn(res, 'setHeader'); await authorizeHandler(req, res); @@ -42,20 +25,7 @@ describe('auth/middleware', () => { expect(res.statusCode).toBe(401); expect(endSpy).toBeCalledWith(JSON.stringify({ error: 'Unauthorized' })); - // Expect the refresh token cookie to be set with an empty string past expiration - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://headless.local-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure', - ); - - // Expect the reference rt cookie to be set with 0 as there is no logged in user. - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://headless.local-has-rt=0; Max-Age=2592000; Path=/', - ); - endSpy.mockRestore(); - setHeaderSpy.mockRestore(); }); test('authorizeHandler will throw an error if the client secret is not defined', async () => { @@ -66,7 +36,6 @@ describe('auth/middleware', () => { const res: ServerResponse = { setHeader() {}, writeHead() {}, - getHeader() {}, end() {}, } as any; @@ -89,12 +58,10 @@ describe('auth/middleware', () => { const res: ServerResponse = { setHeader() {}, writeHead() {}, - getHeader() {}, end() {}, } as any; const endSpy = jest.spyOn(res, 'end'); - const setHeaderSpy = jest.spyOn(res, 'setHeader'); const wpUrl = 'http://my-wp-site.com'; const getWpSecretSpy = jest @@ -113,19 +80,6 @@ describe('auth/middleware', () => { expect(res.statusCode).toBe(401); expect(endSpy).toBeCalledWith(JSON.stringify({ error: 'some error' })); - // Expect the refresh token cookie to be set with an empty string past expiration - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://my-wp-site.com-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure', - ); - - // Expect the reference rt cookie to be set with 0 as there is no logged in user. - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://my-wp-site.com-has-rt=0; Max-Age=2592000; Path=/', - ); - - setHeaderSpy.mockRestore(); endSpy.mockRestore(); fetchMock.restore(); }); @@ -145,12 +99,10 @@ describe('auth/middleware', () => { const res: ServerResponse = { setHeader() {}, writeHead() {}, - getHeader() {}, end() {}, } as any; const endSpy = jest.spyOn(res, 'end'); - const setHeaderSpy = jest.spyOn(res, 'setHeader'); const warningSpy = jest.spyOn(console, 'log').mockImplementation(jest.fn()); const successResponse = { message: 'Successfully called deprecated endpoint.', @@ -177,108 +129,8 @@ describe('auth/middleware', () => { expect(res.statusCode).toBe(200); expect(endSpy).toBeCalledWith(JSON.stringify(successResponse)); - // Expect the reference rt cookie to be set with a 1 since there is a logged in user. - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://my-wp-site.com-has-rt=1; Max-Age=2592000; Path=/', - ); - - // Expect the refresh token cookie to be set with the refresh token. - // Note: The refresh token is base64 encoded then uri encoded before its saved - // as the cookie value. - const rtCookie = `http://my-wp-site.com-rt=${encodeURIComponent( - base64Encode('valid-rt'), - )}; Max-Age=2592000; Path=/; HttpOnly; Secure; SameSite=Strict`; - - expect(setHeaderSpy).toBeCalledWith('Set-Cookie', rtCookie); - endSpy.mockRestore(); - setHeaderSpy.mockRestore(); warningSpy.mockRestore(); fetchMock.restore(); }); }); - -describe('logout handler', () => { - const envBackup = process.env; - - beforeEach(() => { - process.env = { ...envBackup }; - }); - - afterEach(() => { - jest.clearAllMocks(); - fetchMock.restore(); - }); - - afterAll(() => { - process.env = envBackup; - }); - - it('logs out the user be setting the rt cookie and reference cookies to empty', async () => { - const wpUrl = 'http://my-wp-site.com'; - const getWpSecretSpy = jest - .spyOn(getWpSecret, 'getWpSecret') - .mockReturnValue('secret'); - const getWpUrlSpy = jest.spyOn(getWpUrl, 'getWpUrl').mockReturnValue(wpUrl); - - const req: IncomingMessage = { - url: 'https://my-headless-site.com/api/faust/auth/logout', - headers: {}, - method: 'POST', - } as any; - - const res: ServerResponse = { - setHeader() {}, - writeHead() {}, - getHeader() {}, - end() {}, - } as any; - - const endSpy = jest.spyOn(res, 'end'); - const setHeaderSpy = jest.spyOn(res, 'setHeader'); - - await logoutHandler(req, res); - - expect(endSpy).toBeCalled(); - expect(res.statusCode).toBe(205); - - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://my-wp-site.com-rt=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure', - ); - - expect(setHeaderSpy).toBeCalledWith( - 'Set-Cookie', - 'http://my-wp-site.com-has-rt=0; Max-Age=2592000; Path=/', - ); - }); - - it('only allows post requests', async () => { - const wpUrl = 'http://my-wp-site.com'; - const getWpSecretSpy = jest - .spyOn(getWpSecret, 'getWpSecret') - .mockReturnValue('secret'); - const getWpUrlSpy = jest.spyOn(getWpUrl, 'getWpUrl').mockReturnValue(wpUrl); - - const req: IncomingMessage = { - url: 'https://my-headless-site.com/api/faust/auth/logout', - headers: {}, - method: 'GET', - } as any; - - const res: ServerResponse = { - setHeader() {}, - writeHead() {}, - getHeader() {}, - end() {}, - } as any; - - const endSpy = jest.spyOn(res, 'end'); - - await logoutHandler(req, res); - - expect(endSpy).toBeCalled(); - expect(res.statusCode).toBe(405); - }); -});