From 665a7ecb1c3eeb4316534383279ec05d6fc26aca Mon Sep 17 00:00:00 2001 From: Michelle Chen Date: Wed, 20 Mar 2024 17:55:19 -0400 Subject: [PATCH] check for cross site redirect, if that occur warn and redirect to default --- .../hydrogen/src/customer/customer.test.ts | 231 ++++++++++++------ packages/hydrogen/src/customer/customer.ts | 22 +- .../hydrogen/src/utils/get-redirect-url.ts | 47 +++- 3 files changed, 214 insertions(+), 86 deletions(-) diff --git a/packages/hydrogen/src/customer/customer.test.ts b/packages/hydrogen/src/customer/customer.test.ts index a6dca446e5..fffe2919df 100644 --- a/packages/hydrogen/src/customer/customer.test.ts +++ b/packages/hydrogen/src/customer/customer.test.ts @@ -113,8 +113,9 @@ describe('customer', () => { expect(params.get('code_challenge_method')).toBe('S256'); }); - it('Redirects to the customer account api logout url', async () => { - const origin = 'https://shop123.com'; + it('Redirects to the customer account api login url with authUrl as param', async () => { + const origin = 'https://localhost'; + const authUrl = '/customer-account/auth'; const customer = createCustomerAccountClient({ session, @@ -122,29 +123,24 @@ describe('customer', () => { customerAccountUrl: 'https://customer-api', request: new Request(origin), waitUntil: vi.fn(), + authUrl, }); - const response = await customer.logout(); - - expect(response.status).toBe(302); - expect(response.headers.get('Set-Cookie')).toBe('cookie'); + const response = await customer.login(); const url = new URL(response.headers.get('location')!); expect(url.origin).toBe('https://customer-api'); - expect(url.pathname).toBe('/auth/logout'); + expect(url.pathname).toBe('/auth/oauth/authorize'); const params = new URLSearchParams(url.search); - - expect(params.get('id_token_hint')).toBe('id_token'); - expect(params.get('post_logout_redirect_uri')).toBe(origin); - - // Session is cleared - expect(session.unset).toHaveBeenCalledWith(CUSTOMER_ACCOUNT_SESSION_KEY); + expect(params.get('redirect_uri')).toBe( + new URL(authUrl, origin).toString(), + ); }); - it('Redirects to the customer account api logout url with postLogoutRedirectUri in the param', async () => { - const origin = 'https://shop123.com'; - const postLogoutRedirectUri = '/post-logout-landing-page'; + it('Redirects to the customer account api login url with DEFAULT_AUTH_URL as param if authUrl is cross domain', async () => { + const origin = 'https://something-good.com'; + const authUrl = 'https://something-bad.com/customer-account/auth'; const customer = createCustomerAccountClient({ session, @@ -152,82 +148,175 @@ describe('customer', () => { customerAccountUrl: 'https://customer-api', request: new Request(origin), waitUntil: vi.fn(), + authUrl, }); - const response = await customer.logout({postLogoutRedirectUri}); - + const response = await customer.login(); const url = new URL(response.headers.get('location')!); + expect(url.origin).toBe('https://customer-api'); - expect(url.pathname).toBe('/auth/logout'); + expect(url.pathname).toBe('/auth/oauth/authorize'); const params = new URLSearchParams(url.search); - expect(params.get('id_token_hint')).toBe('id_token'); - expect(params.get('post_logout_redirect_uri')).toBe( - `${origin}${postLogoutRedirectUri}`, + expect(params.get('redirect_uri')).toBe( + new URL('/account/authorize', origin).toString(), ); - - // Session is cleared - expect(session.unset).toHaveBeenCalledWith(CUSTOMER_ACCOUNT_SESSION_KEY); }); - it('Redirects to app origin when customer is not login by default', async () => { - const origin = 'https://shop123.com'; - const mockSession: HydrogenSession = { - commit: vi.fn(() => new Promise((resolve) => resolve('cookie'))), - get: vi.fn(() => undefined) as HydrogenSession['get'], - set: vi.fn(), - unset: vi.fn(), - }; + describe('logout', () => { + it('Redirects to the customer account api logout url', async () => { + const origin = 'https://shop123.com'; - const customer = createCustomerAccountClient({ - session: mockSession, - customerAccountId: 'customerAccountId', - customerAccountUrl: 'https://customer-api', - request: new Request(origin), - waitUntil: vi.fn(), + const customer = createCustomerAccountClient({ + session, + customerAccountId: 'customerAccountId', + customerAccountUrl: 'https://customer-api', + request: new Request(origin), + waitUntil: vi.fn(), + }); + + const response = await customer.logout(); + + expect(response.status).toBe(302); + expect(response.headers.get('Set-Cookie')).toBe('cookie'); + const url = new URL(response.headers.get('location')!); + + expect(url.origin).toBe('https://customer-api'); + expect(url.pathname).toBe('/auth/logout'); + + const params = new URLSearchParams(url.search); + + expect(params.get('id_token_hint')).toBe('id_token'); + expect(params.get('post_logout_redirect_uri')).toBe( + new URL(origin).toString(), + ); + + // Session is cleared + expect(session.unset).toHaveBeenCalledWith( + CUSTOMER_ACCOUNT_SESSION_KEY, + ); }); - const response = await customer.logout(); + it('Redirects to the customer account api logout url with postLogoutRedirectUri in the param', async () => { + const origin = 'https://shop123.com'; + const postLogoutRedirectUri = '/post-logout-landing-page'; - const url = new URL(response.headers.get('location')!); - expect(url.toString()).toBe(new URL(origin).toString()); + const customer = createCustomerAccountClient({ + session, + customerAccountId: 'customerAccountId', + customerAccountUrl: 'https://customer-api', + request: new Request(origin), + waitUntil: vi.fn(), + }); - // Session is cleared - expect(mockSession.unset).toHaveBeenCalledWith( - CUSTOMER_ACCOUNT_SESSION_KEY, - ); - }); + const response = await customer.logout({postLogoutRedirectUri}); - it('Redirects to postLogoutRedirectUri when customer is not login', async () => { - const origin = 'https://shop123.com'; - const postLogoutRedirectUri = '/post-logout-landing-page'; + const url = new URL(response.headers.get('location')!); + expect(url.origin).toBe('https://customer-api'); + expect(url.pathname).toBe('/auth/logout'); - const mockSession: HydrogenSession = { - commit: vi.fn(() => new Promise((resolve) => resolve('cookie'))), - get: vi.fn(() => undefined) as HydrogenSession['get'], - set: vi.fn(), - unset: vi.fn(), - }; + const params = new URLSearchParams(url.search); + expect(params.get('id_token_hint')).toBe('id_token'); + expect(params.get('post_logout_redirect_uri')).toBe( + `${origin}${postLogoutRedirectUri}`, + ); - const customer = createCustomerAccountClient({ - session: mockSession, - customerAccountId: 'customerAccountId', - customerAccountUrl: 'https://customer-api', - request: new Request(origin), - waitUntil: vi.fn(), + // Session is cleared + expect(session.unset).toHaveBeenCalledWith( + CUSTOMER_ACCOUNT_SESSION_KEY, + ); }); - const response = await customer.logout({postLogoutRedirectUri}); + it('Redirects to app origin when customer is not login by default', async () => { + const origin = 'https://shop123.com'; + const mockSession: HydrogenSession = { + commit: vi.fn(() => new Promise((resolve) => resolve('cookie'))), + get: vi.fn(() => undefined) as HydrogenSession['get'], + set: vi.fn(), + unset: vi.fn(), + }; + + const customer = createCustomerAccountClient({ + session: mockSession, + customerAccountId: 'customerAccountId', + customerAccountUrl: 'https://customer-api', + request: new Request(origin), + waitUntil: vi.fn(), + }); + + const response = await customer.logout(); + + const url = new URL(response.headers.get('location')!); + expect(url.toString()).toBe(new URL(origin).toString()); + + // Session is cleared + expect(mockSession.unset).toHaveBeenCalledWith( + CUSTOMER_ACCOUNT_SESSION_KEY, + ); + }); - const url = new URL(response.headers.get('location')!); - expect(url.toString()).toBe( - new URL(postLogoutRedirectUri, origin).toString(), - ); + it('Redirects to postLogoutRedirectUri when customer is not login', async () => { + const origin = 'https://shop123.com'; + const postLogoutRedirectUri = '/post-logout-landing-page'; + + const mockSession: HydrogenSession = { + commit: vi.fn(() => new Promise((resolve) => resolve('cookie'))), + get: vi.fn(() => undefined) as HydrogenSession['get'], + set: vi.fn(), + unset: vi.fn(), + }; + + const customer = createCustomerAccountClient({ + session: mockSession, + customerAccountId: 'customerAccountId', + customerAccountUrl: 'https://customer-api', + request: new Request(origin), + waitUntil: vi.fn(), + }); + + const response = await customer.logout({postLogoutRedirectUri}); + + const url = new URL(response.headers.get('location')!); + expect(url.toString()).toBe( + new URL(postLogoutRedirectUri, origin).toString(), + ); - // Session is cleared - expect(mockSession.unset).toHaveBeenCalledWith( - CUSTOMER_ACCOUNT_SESSION_KEY, - ); + // Session is cleared + expect(mockSession.unset).toHaveBeenCalledWith( + CUSTOMER_ACCOUNT_SESSION_KEY, + ); + }); + + it('Redirects to app origin if postLogoutRedirectUri is cross-site when customer is not login', async () => { + const origin = 'https://shop123.com'; + const postLogoutRedirectUri = + 'https://something-bad.com/post-logout-landing-page'; + + const mockSession: HydrogenSession = { + commit: vi.fn(() => new Promise((resolve) => resolve('cookie'))), + get: vi.fn(() => undefined) as HydrogenSession['get'], + set: vi.fn(), + unset: vi.fn(), + }; + + const customer = createCustomerAccountClient({ + session: mockSession, + customerAccountId: 'customerAccountId', + customerAccountUrl: 'https://customer-api', + request: new Request(origin), + waitUntil: vi.fn(), + }); + + const response = await customer.logout({postLogoutRedirectUri}); + + const url = new URL(response.headers.get('location')!); + expect(url.toString()).toBe(new URL(origin).toString()); + + // Session is cleared + expect(mockSession.unset).toHaveBeenCalledWith( + CUSTOMER_ACCOUNT_SESSION_KEY, + ); + }); }); it('Saved redirectPath to session by default if `return_to` param was found', async () => { diff --git a/packages/hydrogen/src/customer/customer.ts b/packages/hydrogen/src/customer/customer.ts index 2f96f73863..76aad6c711 100644 --- a/packages/hydrogen/src/customer/customer.ts +++ b/packages/hydrogen/src/customer/customer.ts @@ -35,7 +35,10 @@ import { getDebugHeaders, } from '../utils/request'; import {getCallerStackLine, withSyncStack} from '../utils/callsites'; -import {getRedirectUrl, buildLocalRedirectUrl} from '../utils/get-redirect-url'; +import { + getRedirectUrl, + ensureLocalRedirectUrl, +} from '../utils/get-redirect-url'; import type { CustomerAccountOptions, CustomerAccount, @@ -67,7 +70,7 @@ export function createCustomerAccountClient({ customerApiVersion = DEFAULT_CUSTOMER_API_VERSION, request, waitUntil, - authUrl = DEFAULT_AUTH_URL, + authUrl, customAuthStatusHandler, logErrors = true, }: CustomerAccountOptions): CustomerAccount { @@ -91,7 +94,11 @@ export function createCustomerAccountClient({ requestUrl.protocol === 'http:' ? requestUrl.origin.replace('http', 'https') : requestUrl.origin; - const redirectUri = buildLocalRedirectUrl(request.url, authUrl); + const redirectUri = ensureLocalRedirectUrl({ + requestUrl: request.url, + defaultUrl: DEFAULT_AUTH_URL, + redirectUrl: authUrl, + }); const customerAccountApiUrl = `${customerAccountUrl}/account/customer/api/${customerApiVersion}/graphql`; const locks: Locks = {}; @@ -294,13 +301,16 @@ export function createCustomerAccountClient({ }, }); }, + logout: async (options?: LogoutOptions) => { ifInvalidCredentialThrowError(customerAccountUrl, customerAccountId); const idToken = session.get(CUSTOMER_ACCOUNT_SESSION_KEY)?.idToken; - const postLogoutRedirectUri = options?.postLogoutRedirectUri - ? buildLocalRedirectUrl(origin, options?.postLogoutRedirectUri) - : origin; + const postLogoutRedirectUri = ensureLocalRedirectUrl({ + requestUrl: origin, + defaultUrl: origin, + redirectUrl: options?.postLogoutRedirectUri, + }); const logoutUrl = idToken ? new URL( diff --git a/packages/hydrogen/src/utils/get-redirect-url.ts b/packages/hydrogen/src/utils/get-redirect-url.ts index a82dabb368..deebeeb803 100644 --- a/packages/hydrogen/src/utils/get-redirect-url.ts +++ b/packages/hydrogen/src/utils/get-redirect-url.ts @@ -33,15 +33,44 @@ function isLocalPath(requestUrl: string, redirectUrl: string) { } } -// build redirect url using request url origin to ensure there is no cross domain redirect -// redirectUrl can be absolute or relative url -export function buildLocalRedirectUrl(requestUrl: string, redirectUrl: string) { - return isAbsoluteUrl(redirectUrl) - ? new URL( - new URL(redirectUrl).pathname, - new URL(requestUrl).origin, - ).toString() - : new URL(redirectUrl, new URL(requestUrl).origin).toString(); +/** Ensure redirect url are always using request origin so we never redirect cross domain. Return the full url with request origin. + * + * @param requestUrl - Use to find app origin + * @param defaultUrl - The default URL to redirect to. + * @param redirectUrl - Relative or absolute URL of redirect. If the absolute URL is cross domain return undefined. + * */ +export function ensureLocalRedirectUrl({ + requestUrl, + defaultUrl, + redirectUrl, +}: { + requestUrl: string; + defaultUrl: string; + redirectUrl?: string; +}): string { + const fromUrl = requestUrl; + const defautlUrl = buildURLObject(requestUrl, defaultUrl); + const toUrl = redirectUrl + ? buildURLObject(requestUrl, redirectUrl) + : defautlUrl; + + if (isLocalPath(requestUrl, toUrl.toString())) { + return toUrl.toString(); + } else { + console.warn( + `Cross-domain redirects are not supported. Tried to redirect from ${fromUrl} to ${toUrl}. Default url ${defautlUrl} is used instead.`, + ); + return defautlUrl.toString(); + } +} + +function buildURLObject( + requestUrl: string, + relativeOrAbsoluteUrl: string, +): URL { + return isAbsoluteUrl(relativeOrAbsoluteUrl) + ? new URL(relativeOrAbsoluteUrl) + : new URL(relativeOrAbsoluteUrl, new URL(requestUrl).origin); } function isAbsoluteUrl(url: string) {