diff --git a/bin/api.mjs b/bin/api.mjs index f3fa7fea3..b36a2d6de 100644 --- a/bin/api.mjs +++ b/bin/api.mjs @@ -39,7 +39,6 @@ const resolveMmtHost = () => { } const localEnvDefaults = { - COOKIE_DOMAIN: '.localhost', JWT_SECRET: 'local-secret', JWT_VALID_TIME: '900', MMT_HOST: resolveMmtHost() diff --git a/bin/deploy-bamboo.sh b/bin/deploy-bamboo.sh index b41c29f26..4a5852833 100755 --- a/bin/deploy-bamboo.sh +++ b/bin/deploy-bamboo.sh @@ -23,7 +23,6 @@ config="`jq '.application.edscHost = $newValue' --arg newValue $bamboo_EDSC_HOST config="`jq '.application.gkrHost = $newValue' --arg newValue $bamboo_GKR_HOST <<< $config`" config="`jq '.application.kmsHost = $newValue' --arg newValue $bamboo_KMS_HOST <<< $config`" config="`jq '.application.mmtKeywordManagerClientId = $newValue' --arg newValue "mmt-keyword-manager-$bamboo_STAGE_NAME" <<< $config`" -config="`jq '.application.cookieDomain = $newValue' --arg newValue $bamboo_COOKIE_DOMAIN <<< $config`" config="`jq '.application.displayProdWarning = $newValue' --arg newValue $bamboo_DISPLAY_PROD_WARNING <<< $config`" config="`jq '.application.tokenValidTime = $newValue' --arg newValue $bamboo_JWT_VALID_TIME <<< $config`" config="`jq '.application.analytics.gtmPropertyId = $newValue' --arg newValue $bamboo_GTM_PROPERTY_ID <<< $config`" @@ -76,7 +75,6 @@ dockerRun() { -e "AWS_SECRET_ACCESS_KEY=$bamboo_AWS_SECRET_ACCESS_KEY" \ -e "AWS_SESSION_TOKEN=$bamboo_AWS_SESSION_TOKEN" \ -e "COLLECTION_TEMPLATES_BUCKET_NAME=${bamboo_COLLECTION_TEMPLATES_BUCKET_NAME}" \ - -e "COOKIE_DOMAIN=$bamboo_COOKIE_DOMAIN" \ -e "DISPLAY_PROD_WARNING=$bamboo_DISPLAY_PROD_WARNING" \ -e "EDL_CLIENT_ID=$bamboo_EDL_CLIENT_ID" \ -e "EDL_PASSWORD=$bamboo_EDL_PASSWORD" \ diff --git a/cdk/mmt/lib/mmt-stack.ts b/cdk/mmt/lib/mmt-stack.ts index 680a3aa2c..cf1881d2c 100644 --- a/cdk/mmt/lib/mmt-stack.ts +++ b/cdk/mmt/lib/mmt-stack.ts @@ -14,7 +14,6 @@ export interface MmtStackProps extends cdk.StackProps {} const { STAGE_NAME = 'dev', COLLECTION_TEMPLATES_BUCKET_NAME = `mmt-${STAGE_NAME}-collection-templates`, - COOKIE_DOMAIN = '.localhost', EDL_CLIENT_ID = '', EDL_PASSWORD = '', GTM_PROPERTY_ID = '', @@ -80,7 +79,6 @@ export class MmtStack extends cdk.Stack { const environment = { COLLECTION_TEMPLATES_BUCKET_NAME, - COOKIE_DOMAIN, EDL_CLIENT_ID, EDL_PASSWORD, GTM_PROPERTY_ID, diff --git a/index.html b/index.html index b4da6a51e..f62982455 100644 --- a/index.html +++ b/index.html @@ -2,6 +2,22 @@ + + Metadata Management Tool diff --git a/serverless/src/edlCallback/__tests__/handler.test.js b/serverless/src/edlCallback/__tests__/handler.test.js index d77ac30f0..a3d256450 100644 --- a/serverless/src/edlCallback/__tests__/handler.test.js +++ b/serverless/src/edlCallback/__tests__/handler.test.js @@ -10,7 +10,6 @@ import edlCallback from '../handler' import * as getConfig from '../../../../sharedUtils/getConfig' import fetchEdlProfile from '../../utils/fetchEdlProfile' import createJwt from '../../utils/createJwt' -import * as createCookieModule from '../../utils/createCookie' beforeAll(() => { vi.spyOn(console, 'error').mockImplementation(() => {}) @@ -20,7 +19,6 @@ afterAll(() => { vi.restoreAllMocks() }) -const realCreateCookie = createCookieModule.default vi.mock('../../utils/AuthorizationCode', () => ({ default: vi.fn() })) @@ -46,15 +44,27 @@ vi.mock('@sharedUtils/getConfig', () => { vi.mock('../../utils/fetchEdlProfile') vi.mock('../../utils/createJwt') -describe('edlCallback', () => { - let createCookieSpy +/** + * The complete set of headers the handler returns when redirecting back to MMT + * + * COMMENT TO BE REMVOED AFTER PR: Two things missing here: Set-Cookie because the token + * now travels in the URL fragment, and Access Control Allow Credentials because + * nothing makes a credentialed request against this endpoint and pairing that + * header with a wildcard origin is invalid per the CORS spec. + */ +const redirectHeaders = (location) => ({ + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Headers': '*', + 'Access-Control-Allow-Methods': 'GET, POST', + Location: location +}) +describe('edlCallback', () => { beforeEach(() => { vi.resetAllMocks() vi.spyOn(Date, 'now').mockReturnValue(new Date('2022-12-31T23:45:00Z').getTime()) process.env.EDL_CLIENT_ID = 'test-client-id' process.env.EDL_PASSWORD = 'test-client-secret' - process.env.COOKIE_DOMAIN = '.example.com' delete process.env.IS_OFFLINE delete process.env.JWT_VALID_TIME @@ -70,9 +80,6 @@ describe('edlCallback', () => { }) createJwt.mockReturnValue('test-jwt') - createCookieSpy = vi - .spyOn(createCookieModule, 'default') - .mockImplementation((...args) => realCreateCookie(...args)) AuthorizationCode.mockImplementation(() => ({ getToken: vi.fn().mockResolvedValue(undefined) @@ -81,7 +88,7 @@ describe('edlCallback', () => { describe('when handling EDL callback', () => { describe('when the callback is successful', () => { - test('should return a redirect with a JWT cookie', async () => { + test('should return a redirect with a JWT in the URL fragment', async () => { const mockEvent = { queryStringParameters: { code: 'test-code', @@ -107,8 +114,7 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) expect(response.statusCode).toBe(303) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2Fdashboard') - expect(response.headers['Set-Cookie']).toBe('_mmt_jwt_test=test-jwt; SameSite=Strict; Path=/; Domain=.example.com; Max-Age=900; Secure;') + expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2Fdashboard#token=test-jwt') }) }) @@ -188,7 +194,6 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) const offlineExpiration = '2023-01-01T00:15:00.000Z' - const expectedExpirationSeconds = Math.floor(new Date(offlineExpiration).getTime() / 1000) expect(AuthorizationCode).not.toHaveBeenCalled() expect(fetchEdlProfile).toHaveBeenCalledWith('ABC-1') @@ -199,9 +204,8 @@ describe('edlCallback', () => { { uid: 'offline-user' } ) - expect(createCookieSpy).toHaveBeenCalledWith('test-jwt', expectedExpirationSeconds) expect(response.statusCode).toBe(303) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2Fdashboard') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2Fdashboard#token=test-jwt')) delete process.env.IS_OFFLINE }) @@ -278,7 +282,7 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2F') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2F#token=test-jwt')) }) test('should handle custom target in state', async () => { @@ -306,12 +310,12 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2Fcustom-page') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2Fcustom-page#token=test-jwt')) }) }) - describe('when creating JWT and cookie', () => { - test('should call createJwt and createCookie with correct parameters', async () => { + describe('when creating JWT', () => { + test('should call createJwt with correct parameters', async () => { const mockEvent = { queryStringParameters: { code: 'test-code', @@ -335,10 +339,7 @@ describe('edlCallback', () => { } fetchEdlProfile.mockResolvedValue(mockEdlProfile) - await edlCallback(mockEvent) - const expectedExpirationSeconds = Math.floor( - new Date(mockToken.expires_at).getTime() / 1000 - ) + const response = await edlCallback(mockEvent) expect(createJwt).toHaveBeenCalledWith( mockToken.access_token, @@ -347,7 +348,7 @@ describe('edlCallback', () => { mockEdlProfile ) - expect(createCookieSpy).toHaveBeenCalledWith('test-jwt', expectedExpirationSeconds) + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2F#token=test-jwt')) }) }) @@ -376,8 +377,8 @@ describe('edlCallback', () => { }) }) - describe('when handling CORS headers', () => { - test('should include correct CORS headers in the response', async () => { + describe('when handing the token back to MMT', () => { + test('should url encode the token in the fragment', async () => { const mockEvent = { queryStringParameters: { code: 'test-code', @@ -400,42 +401,11 @@ describe('edlCallback', () => { assuranceLevel: 5 }) - const response = await edlCallback(mockEvent) - - expect(response.headers['Access-Control-Allow-Origin']).toBe('*') - expect(response.headers['Access-Control-Allow-Headers']).toBe('*') - expect(response.headers['Access-Control-Allow-Methods']).toBe('GET, POST') - expect(response.headers['Access-Control-Allow-Credentials']).toBe(true) - }) - }) - - describe('when handling cookie naming', () => { - test('should include the environment name in the cookie prefix', async () => { - const mockEvent = { - queryStringParameters: { - code: 'test-code', - state: encodeURIComponent(JSON.stringify({ target: '/' })) - } - } - - AuthorizationCode.mockImplementation(() => ({ - getToken: vi.fn().mockResolvedValue({ - token: { - access_token: 'test-access-token', - refresh_token: 'test-refresh-token', - expires_at: '2023-01-01T00:00:00Z' - } - }) - })) - - fetchEdlProfile.mockResolvedValue({ - uid: 'test-user', - assuranceLevel: 5 - }) + createJwt.mockReturnValue('jwt/with+reserved=characters') const response = await edlCallback(mockEvent) - expect(response.headers['Set-Cookie']).toContain('_mmt_jwt_test=') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2F#token=jwt%2Fwith%2Breserved%3Dcharacters')) }) }) @@ -538,8 +508,7 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) expect(response.statusCode).toBe(303) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2F') - expect(response.headers['Set-Cookie']).toBe('_mmt_jwt_test=test-jwt; SameSite=Strict; Path=/; Domain=.example.com; Max-Age=900; Secure;') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2F#token=test-jwt')) }) }) @@ -621,11 +590,11 @@ describe('edlCallback', () => { }) })) - expect(response.headers.Location).toBe('https://custom-mmt.example.com/auth-callback?target=%2F') + expect(response.headers).toEqual(redirectHeaders('https://custom-mmt.example.com/auth-callback?target=%2F#token=test-jwt')) }) }) - describe('when handling errors in createJwt or createCookie', () => { + describe('when handling errors in createJwt', () => { test('should throw an error if createJwt fails', async () => { const mockEvent = { queryStringParameters: { @@ -655,37 +624,6 @@ describe('edlCallback', () => { await expect(edlCallback(mockEvent)).rejects.toThrow('JWT creation failed') }) - - test('should throw an error if createCookie fails', async () => { - const mockEvent = { - queryStringParameters: { - code: 'test-code', - state: encodeURIComponent(JSON.stringify({ target: '/' })) - } - } - - AuthorizationCode.mockImplementation(() => ({ - getToken: vi.fn().mockResolvedValue({ - token: { - access_token: 'test-access-token', - refresh_token: 'test-refresh-token', - expires_at: '2023-01-01T00:00:00Z' - } - }) - })) - - fetchEdlProfile.mockResolvedValue({ - uid: 'test-user', - assuranceLevel: 5 - }) - - createJwt.mockReturnValue('test-jwt') - createCookieSpy.mockImplementation(() => { - throw new Error('Cookie creation failed') - }) - - await expect(edlCallback(mockEvent)).rejects.toThrow('Cookie creation failed') - }) }) describe('when handling unusual target URLs', () => { @@ -714,7 +652,7 @@ describe('edlCallback', () => { const response = await edlCallback(mockEvent) - expect(response.headers.Location).toBe('https://mmt.example.com/auth-callback?target=%2Funusual%20path%3Fparam%3Dvalue%26other%3D123') + expect(response.headers).toEqual(redirectHeaders('https://mmt.example.com/auth-callback?target=%2Funusual%20path%3Fparam%3Dvalue%26other%3D123#token=test-jwt')) }) }) diff --git a/serverless/src/edlCallback/handler.js b/serverless/src/edlCallback/handler.js index 5a47eb38f..443bb1c57 100644 --- a/serverless/src/edlCallback/handler.js +++ b/serverless/src/edlCallback/handler.js @@ -1,7 +1,6 @@ import { getApplicationConfig, getEdlConfig } from '../../../sharedUtils/getConfig' import fetchEdlProfile from '../utils/fetchEdlProfile' import createJwt from '../utils/createJwt' -import createCookie from '../utils/createCookie' import AuthorizationCode from '../utils/AuthorizationCode' /** @@ -108,18 +107,19 @@ const edlCallback = async (event) => { // Create JWT with EDL token and edl profile const jwt = createJwt(accessToken, refreshToken, expiresAt, edlProfile) - const location = `${mmtHost}/auth-callback?target=${encodeURIComponent(target)}` - - const expiresAtInSeconds = Math.floor(new Date(expiresAt).getTime() / 1000) + // The token is handed back in the URL fragment rather than a 'Set-Cookie' + // header. This handler runs on the API host, which sits on a different domain + // than MMT, so any cookie set would have to be scoped to a domain shared by + // every environment and would then be sent on requests to all of them. MMT + // stores the token itself, keeping the cookie scoped to its own host. + const location = `${mmtHost}/auth-callback?target=${encodeURIComponent(target)}#token=${encodeURIComponent(jwt)}` const response = { statusCode: 303, headers: { - 'Set-Cookie': createCookie(jwt, expiresAtInSeconds), 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Headers': '*', 'Access-Control-Allow-Methods': 'GET, POST', - 'Access-Control-Allow-Credentials': true, Location: location } } diff --git a/serverless/src/edlRefreshToken/__tests__/handler.test.js b/serverless/src/edlRefreshToken/__tests__/handler.test.js index 7a0f03c63..8c74b7809 100644 --- a/serverless/src/edlRefreshToken/__tests__/handler.test.js +++ b/serverless/src/edlRefreshToken/__tests__/handler.test.js @@ -10,15 +10,20 @@ import jwt from 'jsonwebtoken' import edlRefreshToken from '../handler' import * as getConfig from '../../../../sharedUtils/getConfig' import * as createJwtModule from '../../utils/createJwt' -import * as createCookieModule from '../../utils/createCookie' const originalFetch = global.fetch +const corsHeaders = { + 'Access-Control-Allow-Origin': 'https://mmt.example.com', + 'Access-Control-Allow-Headers': '*', + 'Access-Control-Allow-Methods': 'POST', + 'Access-Control-Allow-Credentials': true +} + describe('edlRefreshToken', () => { let fetchMock let jwtVerifySpy let createJwtSpy - let createCookieSpy beforeEach(() => { vi.resetAllMocks() @@ -28,7 +33,6 @@ describe('edlRefreshToken', () => { process.env.EDL_CLIENT_ID = 'test-client-id' process.env.EDL_PASSWORD = 'test-client-secret' process.env.JWT_SECRET = 'jwt-secret' - process.env.COOKIE_DOMAIN = '.example.com' delete process.env.IS_OFFLINE delete process.env.JWT_VALID_TIME @@ -51,7 +55,7 @@ describe('edlRefreshToken', () => { }) describe('when refreshing the token succeeds', () => { - test('should request a new token and return a cookie response', async () => { + test('should request a new token and return it in the response body', async () => { vi.useFakeTimers() vi.setSystemTime(new Date('2024-01-01T00:00:00Z')) @@ -71,7 +75,6 @@ describe('edlRefreshToken', () => { }) createJwtSpy = vi.spyOn(createJwtModule, 'default').mockReturnValue('new-jwt') - createCookieSpy = vi.spyOn(createCookieModule, 'default').mockReturnValue('cookie-string') const event = { headers: { @@ -101,12 +104,8 @@ describe('edlRefreshToken', () => { ) expect(response.statusCode).toBe(200) - expect(createCookieSpy).toHaveBeenCalledWith('new-jwt', 1704070800) - expect(response.headers['Set-Cookie']).toBe('cookie-string') - expect(response.headers['Access-Control-Allow-Origin']).toBe('https://mmt.example.com') - expect(response.headers['Access-Control-Allow-Headers']).toBe('*') - expect(response.headers['Access-Control-Allow-Methods']).toBe('POST') - expect(response.headers['Access-Control-Allow-Credentials']).toBe(true) + expect(JSON.parse(response.body)).toEqual({ token: 'new-jwt' }) + expect(response.headers).toEqual(corsHeaders) }) }) @@ -122,7 +121,6 @@ describe('edlRefreshToken', () => { }) createJwtSpy = vi.spyOn(createJwtModule, 'default').mockReturnValue('offline-jwt') - createCookieSpy = vi.spyOn(createCookieModule, 'default').mockReturnValue('offline-cookie') const event = { headers: { @@ -132,7 +130,6 @@ describe('edlRefreshToken', () => { const response = await edlRefreshToken(event) const offlineExpiration = '2024-02-02T00:30:00.000Z' - const expirationSeconds = Math.floor(new Date(offlineExpiration).getTime() / 1000) expect(fetchMock).not.toHaveBeenCalled() expect(createJwtSpy).toHaveBeenCalledWith( @@ -142,9 +139,8 @@ describe('edlRefreshToken', () => { { uid: 'test-user' } ) - expect(createCookieSpy).toHaveBeenCalledWith('offline-jwt', expirationSeconds) expect(response.statusCode).toBe(200) - expect(response.headers['Set-Cookie']).toBe('offline-cookie') + expect(JSON.parse(response.body)).toEqual({ token: 'offline-jwt' }) delete process.env.IS_OFFLINE }) @@ -176,10 +172,7 @@ describe('edlRefreshToken', () => { error: 'Failed to refresh token' }) - expect(response.headers['Access-Control-Allow-Origin']).toBe('https://mmt.example.com') - expect(response.headers['Access-Control-Allow-Headers']).toBe('*') - expect(response.headers['Access-Control-Allow-Methods']).toBe('POST') - expect(response.headers['Access-Control-Allow-Credentials']).toBe(true) + expect(response.headers).toEqual(corsHeaders) }) }) @@ -208,7 +201,6 @@ describe('edlRefreshToken', () => { expect(fetchMock).not.toHaveBeenCalled() expect(createJwtSpy).not.toHaveBeenCalled() - expect(createCookieSpy).not.toHaveBeenCalled() }) }) @@ -234,7 +226,6 @@ describe('edlRefreshToken', () => { }) createJwtSpy = vi.spyOn(createJwtModule, 'default').mockReturnValue('new-jwt') - createCookieSpy = vi.spyOn(createCookieModule, 'default').mockReturnValue('cookie-string') const event = { headers: { @@ -276,7 +267,6 @@ describe('edlRefreshToken', () => { }) createJwtSpy = vi.spyOn(createJwtModule, 'default').mockReturnValue('new-jwt') - createCookieSpy = vi.spyOn(createCookieModule, 'default').mockReturnValue('cookie-string') const event = { headers: { diff --git a/serverless/src/edlRefreshToken/handler.js b/serverless/src/edlRefreshToken/handler.js index 3a566f4c8..32bbccce7 100644 --- a/serverless/src/edlRefreshToken/handler.js +++ b/serverless/src/edlRefreshToken/handler.js @@ -1,7 +1,6 @@ import jwt from 'jsonwebtoken' import { getEdlConfig, getApplicationConfig } from '../../../sharedUtils/getConfig' import createJwt from '../utils/createJwt' -import createCookie from '../utils/createCookie' import { downcaseKeys } from '../utils/downcaseKeys' /** @@ -91,17 +90,17 @@ const edlRefreshToken = async (event) => { // Create a new JWT with the new access token, refresh token, and existing EDL profile const newJwt = createJwt(newAccessToken, newRefreshToken, expiresAt, edlProfile) - const expiresAtInSeconds = Math.floor(new Date(expiresAt).getTime() / 1000) - + // The refreshed token is returned in the body rather than a 'Set-Cookie' + // header so MMT can store it against its own host. return { statusCode: 200, headers: { - 'Set-Cookie': createCookie(newJwt, expiresAtInSeconds), 'Access-Control-Allow-Origin': mmtHost, 'Access-Control-Allow-Headers': '*', 'Access-Control-Allow-Methods': 'POST', 'Access-Control-Allow-Credentials': true - } + }, + body: JSON.stringify({ token: newJwt }) } } catch (error) { console.error('Token refresh error:', error.message) diff --git a/serverless/src/utils/__tests__/createCookie.test.js b/serverless/src/utils/__tests__/createCookie.test.js deleted file mode 100644 index d2726690f..000000000 --- a/serverless/src/utils/__tests__/createCookie.test.js +++ /dev/null @@ -1,67 +0,0 @@ -import { - describe, - test, - expect, - beforeEach, - afterEach, - vi -} from 'vitest' -import createCookie from '../createCookie' - -vi.mock('@sharedUtils/getConfig', () => { - const getApplicationConfig = vi.fn(() => ({ - env: 'development' - })) - - return { - getApplicationConfig - } -}) - -describe('createCookie', () => { - const OLD_ENV = process.env - - beforeEach(() => { - vi.resetModules() - process.env = { - ...OLD_ENV, - COOKIE_DOMAIN: '.example.com' - } - - vi.spyOn(Date, 'now').mockImplementation(() => 1625097600000) // 2021-07-01T00:00:00.000Z - }) - - afterEach(() => { - process.env = OLD_ENV - vi.restoreAllMocks() - }) - - describe('when not running locally', () => { - test('returns the cookie string with correct Max-Age', () => { - const jwt = 'mock-jwt' - const expiresAt = Math.floor(Date.now() / 1000) + 900 // 15 minutes from now - - const result = createCookie(jwt, expiresAt) - expect(result).toEqual('_mmt_jwt_development=mock-jwt; SameSite=Strict; Path=/; Domain=.example.com; Max-Age=900; Secure;') - }) - }) - - describe('when running locally', () => { - test('returns the cookie string without Secure flag', () => { - process.env.IS_OFFLINE = 'true' - const jwt = 'mock-jwt' - const expiresAt = Math.floor(Date.now() / 1000) + 900 // 15 minutes from now - - const result = createCookie(jwt, expiresAt) - expect(result).toEqual('_mmt_jwt_development=mock-jwt; SameSite=Strict; Path=/; Domain=.example.com; Max-Age=900;') - }) - }) - - test('handles expiration time in the past', () => { - const jwt = 'mock-jwt' - const expiresAt = Math.floor(Date.now() / 1000) - 3600 // 1 hour ago - - const result = createCookie(jwt, expiresAt) - expect(result).toEqual('_mmt_jwt_development=mock-jwt; SameSite=Strict; Path=/; Domain=.example.com; Max-Age=0; Secure;') - }) -}) diff --git a/serverless/src/utils/createCookie.js b/serverless/src/utils/createCookie.js deleted file mode 100644 index f7cef8bdc..000000000 --- a/serverless/src/utils/createCookie.js +++ /dev/null @@ -1,26 +0,0 @@ -import MMT_COOKIE from 'sharedConstants/mmtCookie' - -/** - * Returns the cookie string with the provided JWT - * @param {String} jwt JWT to use for the cookie value - * @param {Number} tokenExpirationTime Expiration time of the token in seconds since Unix epoch - */ -const createCookie = (jwt, tokenExpirationTime) => { - const { - COOKIE_DOMAIN, - IS_OFFLINE - } = process.env - - // Calculate Max-Age in seconds - const now = Math.floor(Date.now() / 1000) // Current time in seconds - const maxAge = Math.max(tokenExpirationTime - now, 0) // Ensure it's not negative - - let cookie = `${MMT_COOKIE}=${jwt}; SameSite=Strict; Path=/; Domain=${COOKIE_DOMAIN}; Max-Age=${maxAge};` - if (!IS_OFFLINE) { - cookie += ' Secure;' - } - - return cookie -} - -export default createCookie diff --git a/sharedConstants/mmtCookie.js b/sharedConstants/mmtCookie.js index b99bad9ae..ac59d4aad 100644 --- a/sharedConstants/mmtCookie.js +++ b/sharedConstants/mmtCookie.js @@ -1,9 +1,4 @@ -import { getApplicationConfig } from '../sharedUtils/getConfig' - -/** - * This is the name of the cookie that MMT uses. - */ -const { env } = getApplicationConfig() -const MMT_COOKIE = `_mmt_jwt_${env}` +// Cookie name for this app. Stored host-only, so it doesn't need an environment suffix. +const MMT_COOKIE = '_mmt_jwt' export default MMT_COOKIE diff --git a/static.config.json b/static.config.json index e2f45d004..a1bbaef98 100644 --- a/static.config.json +++ b/static.config.json @@ -15,7 +15,6 @@ "Access-Control-Allow-Headers": "*", "Access-Control-Allow-Credentials": true }, - "cookieDomain": ".localhost", "tokenValidTime": "900", "displayProdWarning": "true", "analytics": { diff --git a/static/src/js/components/AuthCallback/AuthCallback.jsx b/static/src/js/components/AuthCallback/AuthCallback.jsx index 6574a355b..82f21982d 100644 --- a/static/src/js/components/AuthCallback/AuthCallback.jsx +++ b/static/src/js/components/AuthCallback/AuthCallback.jsx @@ -7,6 +7,7 @@ import isTokenExpired from '@/js/utils/isTokenExpired' /** * This class handles the authenticated redirect from our EDL callback lambda function. + * The token itself is stored before the app renders, see 'consumeAuthToken' * We get the EDL token and redirect to the specified `target` path */ export const AuthCallback = () => { diff --git a/static/src/js/components/ErrorUnauthorizedAccess/ErrorUnauthorizedAccess.jsx b/static/src/js/components/ErrorUnauthorizedAccess/ErrorUnauthorizedAccess.jsx index 4cc79ef88..539e79fa3 100644 --- a/static/src/js/components/ErrorUnauthorizedAccess/ErrorUnauthorizedAccess.jsx +++ b/static/src/js/components/ErrorUnauthorizedAccess/ErrorUnauthorizedAccess.jsx @@ -4,7 +4,6 @@ import { useLocation } from 'react-router-dom' import useMMTCookie from '@/js/hooks/useMMTCookie' import MMT_COOKIE from 'sharedConstants/mmtCookie' import Header from '../Header/Header' -import { getApplicationConfig } from '../../../../../sharedUtils/getConfig' import './ErrorUnauthorizedAccess.scss' @@ -12,17 +11,16 @@ const ErrorUnauthorizedAccess = () => { const location = useLocation() const queryParams = new URLSearchParams(location.search) const errorType = queryParams.get('errorType') || 'default' - const { cookieDomain } = getApplicationConfig() const { removeCookie } = useMMTCookie() useEffect(() => { // Always clear the authentication cookie when showing an unauthorized error // This ensures users get a fresh authentication flow when they try again + // No 'domain' here, matching how the cookie was written removeCookie(MMT_COOKIE, { - domain: cookieDomain, path: '/' }) - }, [cookieDomain, removeCookie]) + }, [removeCookie]) const errorMessages = { deniedAccessMMT: 'It appears you are not provisioned with the proper permissions to access MMT.', diff --git a/static/src/js/components/ErrorUnauthorizedAccess/__tests__/ErrorUnauthorizedAccess.test.jsx b/static/src/js/components/ErrorUnauthorizedAccess/__tests__/ErrorUnauthorizedAccess.test.jsx index 525920599..a965842bc 100644 --- a/static/src/js/components/ErrorUnauthorizedAccess/__tests__/ErrorUnauthorizedAccess.test.jsx +++ b/static/src/js/components/ErrorUnauthorizedAccess/__tests__/ErrorUnauthorizedAccess.test.jsx @@ -10,7 +10,6 @@ import AuthContext from '@/js/context/AuthContext' import MMT_COOKIE from 'sharedConstants/mmtCookie' import ErrorUnauthorizedAccess from '../ErrorUnauthorizedAccess' -import * as getConfig from '../../../../../../sharedUtils/getConfig' const mockRemoveCookie = vi.fn() vi.mock('@/js/hooks/useMMTCookie', () => ({ @@ -20,10 +19,6 @@ vi.mock('@/js/hooks/useMMTCookie', () => ({ }) })) -vi.spyOn(getConfig, 'getApplicationConfig').mockImplementation(() => ({ - cookieDomain: '.example.com' -})) - const setup = (errorType) => { const context = { login: vi.fn() @@ -58,7 +53,6 @@ describe('ErrorUnauthorizedAccess component', () => { setup('deniedAccessMMT') expect(mockRemoveCookie).toHaveBeenCalledWith(MMT_COOKIE, { - domain: '.example.com', path: '/' }) }) @@ -74,7 +68,6 @@ describe('ErrorUnauthorizedAccess component', () => { setup('deniedNonNasaAccessMMT') expect(mockRemoveCookie).toHaveBeenCalledWith(MMT_COOKIE, { - domain: '.example.com', path: '/' }) }) @@ -90,7 +83,6 @@ describe('ErrorUnauthorizedAccess component', () => { setup('') expect(mockRemoveCookie).toHaveBeenCalledWith(MMT_COOKIE, { - domain: '.example.com', path: '/' }) }) @@ -118,7 +110,6 @@ describe('ErrorUnauthorizedAccess component', () => { setup('foo') expect(mockRemoveCookie).toHaveBeenCalledWith(MMT_COOKIE, { - domain: '.example.com', path: '/' }) }) diff --git a/static/src/js/providers/AuthContextProvider/AuthContextProvider.jsx b/static/src/js/providers/AuthContextProvider/AuthContextProvider.jsx index bc2ab8d0f..049737416 100644 --- a/static/src/js/providers/AuthContextProvider/AuthContextProvider.jsx +++ b/static/src/js/providers/AuthContextProvider/AuthContextProvider.jsx @@ -14,16 +14,14 @@ import AuthContext from '@/js/context/AuthContext' import useMMTCookie from '@/js/hooks/useMMTCookie' import errorLogger from '@/js/utils/errorLogger' +import getMMTCookieOptions from '@/js/utils/getMMTCookieOptions' import refreshToken from '@/js/utils/refreshToken' import MMT_COOKIE from 'sharedConstants/mmtCookie' import { getApplicationConfig } from '../../../../../sharedUtils/getConfig' -const { - apiHost, - cookieDomain -} = getApplicationConfig() +const { apiHost } = getApplicationConfig() const MAX_IDLE_TIMEOUT = 900000 const REFRESH_THRESHOLD_MS = 60000 @@ -64,8 +62,9 @@ const resetTokenState = ({ setTokenValue, setUser }) => { + // No 'domain' here matching how the cookie was written. Passing one here would + // target a different cookie than the host-only one MMT actually stores setCookie(MMT_COOKIE, null, { - domain: cookieDomain, path: '/', maxAge: 0, expires: new Date(0) @@ -237,10 +236,8 @@ const AuthContextProvider = ({ children }) => { setToken: (result) => { refreshInProgress.current = false - // When the token refresh succeeds, the server sets a new cookie - // The next time mmtJwt changes, our effect will process the new token // If the refresh fails, redirects happen in the refreshToken function - if (result === null) { + if (!result) { // Handle token reset, but don't redirect (already happening in refreshToken) resetTokenState({ setCookie, @@ -248,8 +245,14 @@ const AuthContextProvider = ({ children }) => { setTokenValue, setUser }) + + return } - // Result === 'refresh_success' is handled by the cookie change + + // Sets the cookie. 'useMMTCookie' picks the new value up as 'mmtJWT', which + // re-runs the effect that calls 'saveToken', so a refreshed token reaches + // state by the same path as one from a fresh login + setCookie(MMT_COOKIE, result, getMMTCookieOptions(result)) } }) } diff --git a/static/src/js/providers/AuthContextProvider/__tests__/AuthContextProvider.test.jsx b/static/src/js/providers/AuthContextProvider/__tests__/AuthContextProvider.test.jsx index f978e64f0..0fc969c42 100644 --- a/static/src/js/providers/AuthContextProvider/__tests__/AuthContextProvider.test.jsx +++ b/static/src/js/providers/AuthContextProvider/__tests__/AuthContextProvider.test.jsx @@ -24,7 +24,6 @@ vi.mock('../../../../../../sharedUtils/getConfig', async () => ({ ...await vi.importActual('../../../../../../sharedUtils/getConfig'), getApplicationConfig: vi.fn(() => ({ apiHost: 'http://test.com/dev', - cookieDomain: 'example.com', tokenValidTime: '900' })) })) @@ -110,7 +109,6 @@ describe('AuthContextProvider component', () => { expect(setCookie).toHaveBeenCalledTimes(1) expect(setCookie).toHaveBeenCalledWith(MMT_COOKIE, null, { - domain: 'example.com', path: '/', maxAge: 0, expires: new Date(0) diff --git a/static/src/js/utils/__tests__/consumeAuthToken.test.js b/static/src/js/utils/__tests__/consumeAuthToken.test.js new file mode 100644 index 000000000..1008658fd --- /dev/null +++ b/static/src/js/utils/__tests__/consumeAuthToken.test.js @@ -0,0 +1,86 @@ +import jwt from 'jsonwebtoken' + +import MMT_COOKIE from 'sharedConstants/mmtCookie' + +import consumeAuthToken from '../consumeAuthToken' + +const clearCookies = () => { + document.cookie.split(';').forEach((cookie) => { + const [name] = cookie.split('=') + + document.cookie = `${name.trim()}=; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Path=/` + }) +} + +const buildToken = () => jwt.sign( + // Always keep the expiration sometime in the future + { exp: Math.floor(Date.now() / 1000) + 900 }, + 'mock-secret' +) + +describe('consumeAuthToken', () => { + beforeEach(() => { + clearCookies() + delete window.mmtAuthToken + + delete window.location + window.location = { protocol: 'http:' } + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe('when the inline script captured a token', () => { + test('stores the token in a cookie', () => { + const token = buildToken() + window.mmtAuthToken = token + + consumeAuthToken() + + expect(document.cookie).toContain(`${MMT_COOKIE}=${token}`) + }) + + test('takes the token back off the window once it is stored', () => { + const token = buildToken() + window.mmtAuthToken = token + + consumeAuthToken() + + expect(window.mmtAuthToken).toBeUndefined() + }) + + test('writes the cookie with the options from getMMTCookieOptions', () => { + const cookieSpy = vi.spyOn(document, 'cookie', 'set') + window.mmtAuthToken = buildToken() + + consumeAuthToken() + + const [written] = cookieSpy.mock.calls.at(-1) + + expect(written).toContain('Path=/') + expect(written).toContain('Expires=') + expect(written).toContain('SameSite=strict') + + expect(written).not.toContain('Domain=') + + expect(written).not.toContain('Secure') + }) + }) + + describe('when no token was captured', () => { + test('leaves the url alone', () => { + consumeAuthToken() + + expect(document.cookie).not.toContain(MMT_COOKIE) + }) + + test('does not write a cookie', () => { + window.mmtAuthToken = '' + + consumeAuthToken() + + expect(document.cookie).not.toContain(MMT_COOKIE) + }) + }) +}) diff --git a/static/src/js/utils/__tests__/getMMTCookieOptions.test.js b/static/src/js/utils/__tests__/getMMTCookieOptions.test.js new file mode 100644 index 000000000..e410ba724 --- /dev/null +++ b/static/src/js/utils/__tests__/getMMTCookieOptions.test.js @@ -0,0 +1,71 @@ +import jwt from 'jsonwebtoken' + +import getMMTCookieOptions from '../getMMTCookieOptions' + +const setProtocol = (protocol) => { + delete window.location + window.location = { protocol } +} + +describe('getMMTCookieOptions', () => { + describe('when the token has an expiration', () => { + test('expires the cookie alongside the token', () => { + const expiresAt = new Date('2025-01-01T00:00:00.000Z') + const token = jwt.sign({ exp: expiresAt.getTime() / 1000 }, 'mock-secret') + + const options = getMMTCookieOptions(token) + + expect(options.expires).toEqual(expiresAt) + }) + }) + + describe('when the token has no expiration', () => { + test('leaves the cookie without an expiration', () => { + const token = jwt.sign({ edlToken: 'mock-token' }, 'mock-secret', { noTimestamp: true }) + + const options = getMMTCookieOptions(token) + + expect(options.expires).toBeUndefined() + }) + }) + + describe('when the token can not be decoded', () => { + test('still returns usable options', () => { + const options = getMMTCookieOptions('not-a-jwt') + + expect(options.expires).toBeUndefined() + expect(options.path).toBe('/') + }) + }) + + describe('the returned options', () => { + test('never include a domain, this is by design and keeps the cookie host-only', () => { + const token = jwt.sign({ edlToken: 'mock-token' }, 'mock-secret') + + // With a domain, the cookie would widen to a parent that + // every environment shares and each environment's cookie would be sent + // on requests to all the others + expect(getMMTCookieOptions(token).domain).toBeUndefined() + }) + }) + + describe('when served over https', () => { + test('marks the cookie secure', () => { + setProtocol('https:') + + const token = jwt.sign({ edlToken: 'mock-token' }, 'mock-secret') + + expect(getMMTCookieOptions(token).secure).toBe(true) + }) + }) + + describe('when served over http', () => { + test('does not mark the cookie secure, so local development works', () => { + setProtocol('http:') + + const token = jwt.sign({ edlToken: 'mock-token' }, 'mock-secret') + + expect(getMMTCookieOptions(token).secure).toBe(false) + }) + }) +}) diff --git a/static/src/js/utils/__tests__/refreshToken.test.js b/static/src/js/utils/__tests__/refreshToken.test.js index 263faf73e..aa77e2dab 100644 --- a/static/src/js/utils/__tests__/refreshToken.test.js +++ b/static/src/js/utils/__tests__/refreshToken.test.js @@ -10,10 +10,11 @@ vi.mock('../overrideStatic.config.json', () => ({})) describe('refreshToken in production mode', () => { describe('when the request is successful', () => { - test('calls setToken with success signal', async () => { + test('calls setToken with refreshed token', async () => { global.fetch.mockResolvedValue(Promise.resolve({ ok: true, - status: 200 + status: 200, + json: async () => ({ token: 'refreshed_token' }) })) const setToken = vi.fn() @@ -24,7 +25,7 @@ describe('refreshToken in production mode', () => { }) expect(setToken).toHaveBeenCalledTimes(1) - expect(setToken).toHaveBeenCalledWith('refresh_success') + expect(setToken).toHaveBeenCalledWith('refreshed_token') expect(fetch).toHaveBeenCalledTimes(1) expect(fetch).toHaveBeenCalledWith( @@ -40,6 +41,28 @@ describe('refreshToken in production mode', () => { }) }) + describe('when the response is missing a token', () => { + test('treats it as a failed refresh and logs the user out', async () => { + global.fetch.mockResolvedValue(Promise.resolve({ + ok: true, + status: 200, + json: async () => ({}) + })) + + const setToken = vi.fn() + + await refreshToken({ + jwt: 'mock_token', + setToken + }) + + expect(setToken).toHaveBeenCalledTimes(1) + expect(setToken).toHaveBeenCalledWith(null) + + expect(window.location.href).toEqual('/') + }) + }) + describe('when the request errors', () => { test('calls setToken and navigate to log out the user', async () => { global.fetch.mockResolvedValue(Promise.resolve({ diff --git a/static/src/js/utils/consumeAuthToken.js b/static/src/js/utils/consumeAuthToken.js new file mode 100644 index 000000000..6b05a4afa --- /dev/null +++ b/static/src/js/utils/consumeAuthToken.js @@ -0,0 +1,51 @@ +import MMT_COOKIE from 'sharedConstants/mmtCookie' + +import getMMTCookieOptions from './getMMTCookieOptions' + +/** + * Serializes cookie options into the attributes `document.cookie` expects. + * + * 'react-cookie' does this elsewhere, but this function runs before React. + * @param {Object} options Options from `getMMTCookieOptions` + */ +const serializeCookieOptions = ({ + domain, + expires, + path, + sameSite, + secure +}) => { + const attributes = [] + + if (path) attributes.push(`Path=${path}`) + if (domain) attributes.push(`Domain=${domain}`) + if (expires) attributes.push(`Expires=${expires.toUTCString()}`) + if (sameSite) attributes.push(`SameSite=${sameSite}`) + if (secure) attributes.push('Secure') + + return attributes +} + +/** + * Stores the token from the login redirect in a host-only cookie. + * + * The token arrives in the URL fragment rather than a 'Set-Cookie' + * header and an inline script in 'index.html' moves it to + * 'window.mmtAuthHeader' before any other script runs. See + * 'edlCallback' for why. + */ + +const consumeAuthToken = () => { + const token = window.mmtAuthToken + + if (!token) return + + delete window.mmtAuthToken + + document.cookie = [ + `${MMT_COOKIE}=${token}`, + ...serializeCookieOptions(getMMTCookieOptions(token)) + ].join('; ') +} + +export default consumeAuthToken diff --git a/static/src/js/utils/getMMTCookieOptions.js b/static/src/js/utils/getMMTCookieOptions.js new file mode 100644 index 000000000..0a25597af --- /dev/null +++ b/static/src/js/utils/getMMTCookieOptions.js @@ -0,0 +1,26 @@ +import jwt from 'jsonwebtoken' + +/** + * Returns the options MMT uses whenever it writes the auth cookie. + * + * `domain` is deliberately omitted so the browser stores a host-only cookie, + * scopes to the exact host serving MMT. + * @param {String} token The MMT JWT being stored, used to expire the cookie alongside the token + */ +const getMMTCookieOptions = (token) => { + const options = { + path: '/', + sameSite: 'strict', + secure: window.location.protocol === 'https:' + } + + const decodedToken = jwt.decode(token) + + if (decodedToken?.exp) { + options.expires = new Date(decodedToken.exp * 1000) + } + + return options +} + +export default getMMTCookieOptions diff --git a/static/src/js/utils/refreshToken.js b/static/src/js/utils/refreshToken.js index 7ea722003..3321a2664 100644 --- a/static/src/js/utils/refreshToken.js +++ b/static/src/js/utils/refreshToken.js @@ -1,10 +1,10 @@ import { getApplicationConfig } from '../../../../sharedUtils/getConfig' /** - * Calls refreshToken lambda to request a new token since the current one is about to expire. The new token is set as the MMT cookie + * Calls refreshToken lambda to request a new token since the current one is about to expire. * @param {Object} params * @param {String} params.jwt The user's MMT JWT - * @param {Function} params.setToken Function to update the token + * @param {Function} params.setToken Called with the refreshed JWT, or `null` when the refresh failed */ const refreshToken = async ({ jwt, @@ -32,8 +32,20 @@ const refreshToken = async ({ return } - // Success - the new token is set as a cookie, signal success - setToken('refresh_success') + const { token } = await response.json() + + // A 200 without a token is still a failed refresh. Passing it on would store + // an empty cookie and log the user out without sending them anywhere. + if (!token) { + console.error('[Auth] Token refresh returned no token') + setToken(null) + window.location.href = '/' + + return + } + + // Success - hand the refreshed token back so the caller can store it + setToken(token) } catch (error) { console.error('[Auth] Token refresh request error:', error) setToken(null) diff --git a/static/src/main.jsx b/static/src/main.jsx index f2652ef66..8c9b683d4 100644 --- a/static/src/main.jsx +++ b/static/src/main.jsx @@ -1,5 +1,9 @@ import React from 'react' import ReactDOM from 'react-dom/client' + +// Must stay above App. See storeAuthToken.js +import './storeAuthToken' + import App from './js/App' ReactDOM.createRoot(document.getElementById('root')).render() diff --git a/static/src/storeAuthToken.js b/static/src/storeAuthToken.js new file mode 100644 index 000000000..759648fa5 --- /dev/null +++ b/static/src/storeAuthToken.js @@ -0,0 +1,9 @@ +import consumeAuthToken from './js/utils/consumeAuthToken' + +/** + * Imported for its side effect and imported ahead of the application on + * purpose. 'App' pulls in 'react-cookie', which snapshots 'document.cookie' + * as it loads, so a cookie written after that is missing from the first + * render and 'AuthContextProvider' clears the session before it is ever used. +*/ +consumeAuthToken()