Skip to content

Commit

Permalink
fix(api): remove cookie domain in development (#54518)
Browse files Browse the repository at this point in the history
  • Loading branch information
ojeytonwilliams committed Apr 26, 2024
1 parent 3f9f7e7 commit 84a81c8
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 129 deletions.
2 changes: 1 addition & 1 deletion api-server/src/server/middlewares/csurf.js
Original file line number Diff line number Diff line change
@@ -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'
};
Expand Down
2 changes: 1 addition & 1 deletion api-server/src/server/utils/getSetAccessToken.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}

Expand Down
15 changes: 14 additions & 1 deletion api-server/src/server/utils/getSetAccessToken.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,27 @@ 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',
ttl: 60000,
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: {} });
Expand Down
8 changes: 8 additions & 0 deletions api/src/server.test.ts
Original file line number Diff line number Diff line change
@@ -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();

Expand Down
2 changes: 1 addition & 1 deletion api/src/utils/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 1 addition & 7 deletions e2e/challenge-title.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
17 changes: 3 additions & 14 deletions e2e/email-settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']
});
Expand All @@ -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']
Expand Down
23 changes: 3 additions & 20 deletions e2e/email-sign-up.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
14 changes: 3 additions & 11 deletions e2e/exam-results.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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 })
Expand All @@ -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
Expand Down
12 changes: 2 additions & 10 deletions e2e/flash.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 })
Expand Down
10 changes: 1 addition & 9 deletions e2e/hotkeys.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
4 changes: 1 addition & 3 deletions e2e/internet-presence-settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 3 additions & 8 deletions e2e/reset-modal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 84a81c8

Please sign in to comment.