check for cross site redirect, if that occur warn and redirect to def…

…ault
Shopify · Mar 20, 2024 · 665a7ec · 665a7ec
1 parent 4f395dc
commit 665a7ec
Show file tree

Hide file tree

Showing 3 changed files with 214 additions and 86 deletions.
diff --git a/packages/hydrogen/src/customer/customer.test.ts b/packages/hydrogen/src/customer/customer.test.ts
@@ -113,121 +113,210 @@ 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,
         customerAccountId: 'customerAccountId',
         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,
         customerAccountId: 'customerAccountId',
         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
@@ -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
@@ -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) {