From 84a81c842b0fe9cf51981dd5ab64eb175a4b2053 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Fri, 26 Apr 2024 17:32:46 +0200 Subject: [PATCH] fix(api): remove cookie domain in development (#54518) --- api-server/src/server/middlewares/csurf.js | 2 +- .../src/server/utils/getSetAccessToken.js | 2 +- .../server/utils/getSetAccessToken.test.js | 15 +++++- api/src/server.test.ts | 8 +++ api/src/utils/env.ts | 2 +- e2e/challenge-title.spec.ts | 8 +-- e2e/email-settings.spec.ts | 17 ++----- e2e/email-sign-up.spec.ts | 23 ++------- e2e/exam-results.spec.ts | 14 ++--- e2e/flash.spec.ts | 12 +---- e2e/hotkeys.spec.ts | 10 +--- e2e/internet-presence-settings.spec.ts | 4 +- e2e/reset-modal.spec.ts | 11 ++-- e2e/settings.spec.ts | 51 ++++--------------- e2e/shortcuts-modal.spec.ts | 6 +-- 15 files changed, 56 insertions(+), 129 deletions(-) diff --git a/api-server/src/server/middlewares/csurf.js b/api-server/src/server/middlewares/csurf.js index 4a7b12056ad..7831537b50d 100644 --- a/api-server/src/server/middlewares/csurf.js +++ b/api-server/src/server/middlewares/csurf.js @@ -1,7 +1,7 @@ import csurf from 'csurf'; export const csrfOptions = { - domain: process.env.COOKIE_DOMAIN || 'localhost', + domain: process.env.COOKIE_DOMAIN, sameSite: 'strict', secure: process.env.FREECODECAMP_NODE_ENV === 'production' }; diff --git a/api-server/src/server/utils/getSetAccessToken.js b/api-server/src/server/utils/getSetAccessToken.js index 0a67ab6719a..552122a988d 100644 --- a/api-server/src/server/utils/getSetAccessToken.js +++ b/api-server/src/server/utils/getSetAccessToken.js @@ -8,7 +8,7 @@ export const jwtCookieNS = 'jwt_access_token'; export function createCookieConfig(req) { return { signed: !!req.signedCookies, - domain: process.env.COOKIE_DOMAIN || 'localhost' + domain: process.env.COOKIE_DOMAIN }; } diff --git a/api-server/src/server/utils/getSetAccessToken.test.js b/api-server/src/server/utils/getSetAccessToken.test.js index 2e1aa39980c..be5e63e8b7b 100644 --- a/api-server/src/server/utils/getSetAccessToken.test.js +++ b/api-server/src/server/utils/getSetAccessToken.test.js @@ -12,7 +12,7 @@ describe('getSetAccessToken', () => { const invalidJWTSecret = 'This is not correct secret'; const now = new Date(Date.now()); const theBeginningOfTime = new Date(0); - const domain = process.env.COOKIE_DOMAIN || 'localhost'; + const domain = 'www.example.com'; const accessToken = { id: '123abc', userId: '456def', @@ -20,6 +20,19 @@ describe('getSetAccessToken', () => { created: now }; + // https://stackoverflow.com/questions/48033841/test-process-env-with-jest + const OLD_ENV = process.env; + + beforeEach(() => { + jest.resetModules(); // process is implicitly cached by Jest, so hence the reset + process.env = { ...OLD_ENV }; // Shallow clone that we can modify + process.env.COOKIE_DOMAIN = domain; + }); + + afterAll(() => { + process.env = OLD_ENV; + }); + describe('getAccessTokenFromRequest', () => { it('return `no token` error if no token is found', () => { const req = mockReq({ headers: {}, cookie: {} }); diff --git a/api/src/server.test.ts b/api/src/server.test.ts index 4f977a9cd4c..97e22c3d41a 100644 --- a/api/src/server.test.ts +++ b/api/src/server.test.ts @@ -1,6 +1,14 @@ import { setupServer, superRequest } from '../jest.utils'; import { HOME_LOCATION, COOKIE_DOMAIN } from './utils/env'; +jest.mock('./utils/env', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return { + ...jest.requireActual('./utils/env'), + COOKIE_DOMAIN: 'freecodecamp.org' + }; +}); + describe('server', () => { setupServer(); diff --git a/api/src/utils/env.ts b/api/src/utils/env.ts index 131cb0c8aab..3b5e0f3d5e9 100644 --- a/api/src/utils/env.ts +++ b/api/src/utils/env.ts @@ -126,7 +126,7 @@ export const SENTRY_DSN = process.env.SENTRY_DSN === 'dsn_from_sentry_dashboard' ? '' : process.env.SENTRY_DSN; -export const COOKIE_DOMAIN = process.env.COOKIE_DOMAIN || 'localhost'; +export const COOKIE_DOMAIN = process.env.COOKIE_DOMAIN; export const COOKIE_SECRET = process.env.COOKIE_SECRET; export const JWT_SECRET = process.env.JWT_SECRET; export const SES_ID = process.env.SES_ID; diff --git a/e2e/challenge-title.spec.ts b/e2e/challenge-title.spec.ts index 604c5094196..a348a73003c 100644 --- a/e2e/challenge-title.spec.ts +++ b/e2e/challenge-title.spec.ts @@ -77,14 +77,8 @@ test.describe('Challenge Title Component (signed in)', () => { test.use({ storageState: 'playwright/.auth/certified-user.json' }); test('should display GreenPass after challenge completion', async ({ - page, - browserName + page }) => { - test.skip( - browserName === 'webkit', - 'user does not seem to be authenticated on Safari' - ); - await expect( page.getByRole('heading', { name: 'Developing a Port Scanner' }) ).toBeVisible(); diff --git a/e2e/email-settings.spec.ts b/e2e/email-settings.spec.ts index 6be2ba76795..4c3d0155f7f 100644 --- a/e2e/email-settings.spec.ts +++ b/e2e/email-settings.spec.ts @@ -47,11 +47,8 @@ test.describe('Email Settings', () => { }); test('should display email verification alert after email update', async ({ - page, - browserName + page }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); - const newEmailAddress = 'foo-update@bar.com'; // Need exact match as there are "New email" and "Confirm new email" labels @@ -87,12 +84,7 @@ test.describe('Email Settings', () => { ); }); - test('should toggle email subscription correctly', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); - + test('should toggle email subscription correctly', async ({ page }) => { const yesPleaseButton = page.getByRole('button', { name: translations.buttons['yes-please'] }); @@ -110,11 +102,8 @@ test.describe('Email Settings', () => { }); test('should display flash message when email subscription is toggled', async ({ - page, - browserName + page }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); - await page .getByRole('button', { name: translations.buttons['yes-please'] diff --git a/e2e/email-sign-up.spec.ts b/e2e/email-sign-up.spec.ts index 83bbe5071a6..e5afedafc13 100644 --- a/e2e/email-sign-up.spec.ts +++ b/e2e/email-sign-up.spec.ts @@ -54,14 +54,8 @@ test.describe('Email sign-up page when user is not signed in', () => { }); test("should not enable Quincy's weekly newsletter when the user clicks the sign up button", async ({ - page, - browserName + page }) => { - test.skip( - browserName === 'webkit', - 'user appears to not signed in on Webkit' - ); - await expect(page).toHaveTitle('Email Sign Up | freeCodeCamp.org'); await expect( page.getByText( @@ -100,12 +94,7 @@ test.describe('Email sign-up page when user is not signed in', () => { test.describe('Email sign-up page when user is signed in', () => { test.use({ storageState: 'playwright/.auth/certified-user.json' }); - test.beforeEach(async ({ page, browserName }) => { - test.skip( - browserName === 'webkit', - 'user appears to not signed in on Webkit' - ); - + test.beforeEach(async ({ page }) => { // It's necessary to seed with a user that has not accepted the privacy // terms, otherwise the user will be redirected away from the email sign-up // page. @@ -139,14 +128,8 @@ test.describe('Email sign-up page when user is signed in', () => { }); test("should disable Quincy's weekly newsletter if the user clicks No", async ({ - page, - browserName + page }) => { - test.skip( - browserName === 'webkit', - 'user appears to not signed in on Webkit' - ); - await expect(page).toHaveTitle('Email Sign Up | freeCodeCamp.org'); await expect( page.getByText( diff --git a/e2e/exam-results.spec.ts b/e2e/exam-results.spec.ts index 8a4bfd5ee32..c6aa9d2e4bc 100644 --- a/e2e/exam-results.spec.ts +++ b/e2e/exam-results.spec.ts @@ -40,10 +40,8 @@ test.describe('Exam Results E2E Test Suite', () => { }); test('Verifies the Correct Rendering of the Exam results', async ({ - page, - browserName + page }) => { - test.skip(browserName === 'webkit', 'It is failing on webkit'); await expect( page .locator('div.exam-results-wrapper') @@ -81,11 +79,7 @@ test.describe('Exam Results E2E Test Suite', () => { ).toBeVisible(); }); - test('Exam Results when the User clicks on Exit button', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'It is failing on webkit'); + test('Exam Results when the User clicks on Exit button', async ({ page }) => { await page .locator('div.exam-results-wrapper') .getByRole('button', { name: translations.buttons.exit }) @@ -104,10 +98,8 @@ test.describe('Exam Results E2E Test Suite', () => { test.describe('Exam Results E2E Test Suite', () => { test('Exam Results When the User clicks on Download button', async ({ - page, - browserName + page }) => { - test.skip(browserName === 'webkit', 'It is failing on webkit'); const [download] = await Promise.all([ page.waitForEvent('download'), page diff --git a/e2e/flash.spec.ts b/e2e/flash.spec.ts index 93a85d82bc4..d13e2fc960d 100644 --- a/e2e/flash.spec.ts +++ b/e2e/flash.spec.ts @@ -15,11 +15,7 @@ const checkFlashMessageVisibility = async (page: Page, translation: string) => { }; test.describe('Flash Message component E2E test', () => { - test('Flash Message Visibility for Night Mode Toggle', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit'); + test('Flash Message Visibility for Night Mode Toggle', async ({ page }) => { await page .getByRole('button', { name: translations.buttons.menu, exact: true }) .click(); @@ -35,11 +31,7 @@ test.describe('Flash Message component E2E test', () => { ); }); - test('Flash Message Visibility for Sound Mode Toggle', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit'); + test('Flash Message Visibility for Sound Mode Toggle', async ({ page }) => { await page .getByLabel(translations.settings.labels['sound-mode']) .getByRole('button', { name: translations.buttons.on }) diff --git a/e2e/hotkeys.spec.ts b/e2e/hotkeys.spec.ts index 69ae5e0d019..a78aff9f41a 100644 --- a/e2e/hotkeys.spec.ts +++ b/e2e/hotkeys.spec.ts @@ -9,15 +9,7 @@ const editorPaneLabel = test.use({ storageState: 'playwright/.auth/certified-user.json' }); -test('User can interact with the app using the keyboard', async ({ - page, - browserName -}) => { - test.skip( - browserName === 'webkit', - 'Failing on webkit for no apparent reason. Can not reproduce locally.' - ); - +test('User can interact with the app using the keyboard', async ({ page }) => { // Enable keyboard shortcuts await page.goto('/settings'); const keyboardShortcutGroup = page.getByRole('group', { diff --git a/e2e/internet-presence-settings.spec.ts b/e2e/internet-presence-settings.spec.ts index 1a2ebce3c7d..68a94ef9e2d 100644 --- a/e2e/internet-presence-settings.spec.ts +++ b/e2e/internet-presence-settings.spec.ts @@ -69,9 +69,7 @@ test.describe('Your Internet Presence', () => { await expect(page.getByTestId(social.checkTestId)).toBeHidden(); }); - test(`should update ${social.name} URL`, async ({ browserName, page }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); - + test(`should update ${social.name} URL`, async ({ page }) => { const socialInput = page.getByLabel(social.label); await socialInput.fill(social.url); const socialCheckmark = page.getByTestId(social.checkTestId); diff --git a/e2e/reset-modal.spec.ts b/e2e/reset-modal.spec.ts index ca62cd8a675..bc93f33bfae 100644 --- a/e2e/reset-modal.spec.ts +++ b/e2e/reset-modal.spec.ts @@ -2,13 +2,6 @@ import { test, expect } from '@playwright/test'; import translations from '../client/i18n/locales/english/translations.json'; -test.beforeEach(({ browserName }) => { - test.skip( - browserName === 'webkit', - 'Failing on webkit for no apparent reason. Can not reproduce locally.' - ); -}); - test('should render the modal content correctly', async ({ page }) => { await page.goto( '/learn/2022/responsive-web-design/learn-html-by-building-a-cat-photo-app/step-2' @@ -74,7 +67,9 @@ test('User can reset challenge', async ({ page }) => { // are reset) await page .getByRole('button', { - name: translations.buttons['check-code'] + // check-code-2 works on all browsers because it does not include Command + // or Ctrl + name: translations.buttons['check-code-2'] }) .click(); diff --git a/e2e/settings.spec.ts b/e2e/settings.spec.ts index 68bf4dceb8b..468245f989e 100644 --- a/e2e/settings.spec.ts +++ b/e2e/settings.spec.ts @@ -55,13 +55,11 @@ test.describe('Settings', () => { await page.goto('/settings'); }); - test('Should have the correct page title', async ({ page, browserName }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should have the correct page title', async ({ page }) => { await expect(page).toHaveTitle(settingsObject.pageTitle); }); - test('Should display the correct header', async ({ page, browserName }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should display the correct header', async ({ page }) => { const header = page.getByTestId(settingsTestIds.settingsHeading); await expect(header).toBeVisible(); await expect(header).toContainText( @@ -72,8 +70,7 @@ test.describe('Settings', () => { ); }); - test('Should validate Username Settings', async ({ page, browserName }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Username Settings', async ({ page }) => { const inputLabel = page.getByLabel(translations.settings.labels.username); await expect(inputLabel).toBeVisible(); await inputLabel.fill(settingsObject.testUser); @@ -114,8 +111,7 @@ test.describe('Settings', () => { await expect(saveButton).toBeVisible(); }); - test('Should validate Privacy Settings', async ({ page, browserName }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Privacy Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings.privacy @@ -220,11 +216,7 @@ test.describe('Settings', () => { await expect(downloadButton).toBeVisible(); }); - test('Should validate Internet Presence Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Internet Presence Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings.internet @@ -239,8 +231,7 @@ test.describe('Settings', () => { await expect(saveButton).toBeVisible(); }); - test('Should validate Portfolio Settings', async ({ page, browserName }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Portfolio Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings.portfolio @@ -289,11 +280,7 @@ test.describe('Settings', () => { await expect(removeButton).toBeHidden(); }); - test('Should validate Personal Information Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Personal Information Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings['personal-info'] @@ -344,11 +331,7 @@ test.describe('Settings', () => { ).toBeVisible(); }); - test('Should validate Academy Honesty Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Academy Honesty Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings.honesty @@ -390,11 +373,7 @@ test.describe('Settings', () => { ).toBeVisible(); }); - test('Should validate Certification Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Certification Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings.certs, @@ -416,11 +395,7 @@ test.describe('Settings', () => { } }); - test('Should validate Legacy Certification Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Legacy Certification Settings', async ({ page }) => { await expect( page.getByRole('heading', { name: translations.settings.headings['legacy-certs'], @@ -443,11 +418,7 @@ test.describe('Settings', () => { } }); - test('Should validate Danger Section Settings', async ({ - page, - browserName - }) => { - test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted'); + test('Should validate Danger Section Settings', async ({ page }) => { await expect( page.getByText(translations.settings.danger.heading, { exact: true diff --git a/e2e/shortcuts-modal.spec.ts b/e2e/shortcuts-modal.spec.ts index 6466f8c3c66..a9271800a49 100644 --- a/e2e/shortcuts-modal.spec.ts +++ b/e2e/shortcuts-modal.spec.ts @@ -9,10 +9,10 @@ const editorPaneLabel = test.use({ storageState: 'playwright/.auth/certified-user.json' }); -test.beforeEach(async ({ page, browserName, isMobile }) => { +test.beforeEach(async ({ page, isMobile }) => { test.skip( - browserName === 'webkit' || isMobile, - 'Failing on webkit for no apparent reason. Can not reproduce locally. Also, skipping on mobile as it does not have a physical keyboard' + isMobile, + 'Skipping on mobile as it does not have a physical keyboard' ); // Enable keyboard shortcuts