Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make library compatible with Remix v3_singleFetch #2007

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {testConfig as testConfigImport} from '../test-helpers/test-config';
const TEST_FUTURE_FLAGS: Required<{[key in keyof FutureFlags]: true}> = {
unstable_newEmbeddedAuthStrategy: true,
removeRest: true,
remixSingleFetch: true,
} as const;

// Override the helper's future flags and logger settings for our purposes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('authorize.admin doc request path', () => {
);
});

it('throws a 401 if app is embedded and the id_token search param is invalid for XHR requests', async () => {
it('throws a 302 if app is embedded and the id_token search param is invalid for XHR requests', async () => {
// GIVEN
const shopify = shopifyApp(testConfig());
await setUpValidSession(shopify.sessionStorage);
Expand All @@ -145,7 +145,7 @@ describe('authorize.admin doc request path', () => {
);

// THEN
expect(response.status).toBe(401);
expect(response.status).toBe(302);
expect(
response.headers.get('X-Shopify-Retry-Invalid-Session-Request'),
).toBe('1');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {

describe('authorize.session token header path', () => {
describe('errors', () => {
it('throws a 401 if the session token is invalid', async () => {
it('throws a 302 if the session token is invalid', async () => {
// GIVEN
const shopify = shopifyApp(testConfig());

Expand All @@ -27,7 +27,7 @@ describe('authorize.session token header path', () => {
);

// THEN
expect(response.status).toBe(401);
expect(response.status).toBe(302);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('Cancel billing', () => {
response.headers.get(REAUTH_URL_HEADER)!,
);

expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(origin).toEqual(APP_URL);
expect(pathname).toEqual('/auth');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe('Billing check', () => {
// THEN
const reauthUrl = new URL(response.headers.get(REAUTH_URL_HEADER)!);

expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(reauthUrl.origin).toEqual(APP_URL);
expect(reauthUrl.pathname).toEqual('/auth');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ describe('Create usage record', () => {
response.headers.get(REAUTH_URL_HEADER)!,
);

expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(origin).toEqual(APP_URL);
expect(pathname).toEqual('/auth');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('Billing request', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(response.headers.get(REAUTH_URL_HEADER)).toEqual(
responses.CONFIRMATION_URL,
);
Expand Down Expand Up @@ -267,7 +267,7 @@ describe('Billing request', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);

const reauthUrl = new URL(response.headers.get(REAUTH_URL_HEADER)!);
expect(reauthUrl.origin).toEqual(APP_URL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ describe('Billing require', () => {
// THEN
const reauthUrl = new URL(response.headers.get(REAUTH_URL_HEADER)!);

expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(reauthUrl.origin).toEqual(APP_URL);
expect(reauthUrl.pathname).toEqual('/auth');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('Update usage billing plan capped amount', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
expect(response.headers.get(REAUTH_URL_HEADER)).toEqual(
responses.CONFIRMATION_URL,
);
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('Update usage billing plan capped amount', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);

const reauthUrl = new URL(response.headers.get(REAUTH_URL_HEADER)!);
expect(reauthUrl.origin).toEqual(APP_URL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,17 @@ export function redirectOutOfApp(
const isXhrRequest = request.headers.get('authorization');

if (isXhrRequest) {
// eslint-disable-next-line no-warning-comments
// TODO Check this with the beta flag disabled (with the bounce page)
// Remix is not including the X-Shopify-API-Request-Failure-Reauthorize-Url when throwing a Response
// https://github.com/remix-run/remix/issues/5356
throw new Response(undefined, {
status: 401,
statusText: 'Unauthorized',
headers: getAppBridgeHeaders(url),
});
if (config.future.remixSingleFetch) {
throw redirect(url, {
headers: getAppBridgeHeaders(url),
});
} else {
throw new Response(undefined, {
status: 401,
statusText: 'Unauthorized',
headers: getAppBridgeHeaders(url),
});
}
} else if (isEmbeddedRequest) {
const params = new URLSearchParams({
shop,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ describe('Redirect helper', () => {
}

async function assertAppBridgeHeaders(response: Response, url: string) {
expect(response.status).toBe(401);
expect(response.status).toBe(302);
expect(response.headers.get(REAUTH_URL_HEADER)).toBe(url);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import type {BasicParams} from '../../../types';

import {beginAuth} from './begin-auth';
import {redirectWithExitIframe} from './redirect-with-exitiframe';
import {redirectWithAppBridgeHeaders} from './redirect-with-app-bridge-headers';
import {
redirectWithAppBridgeHeaders,
redirectWithResponseWithAppBridgeHeaders,
} from './redirect-with-app-bridge-headers';

export async function redirectToAuthPage(
params: BasicParams,
Expand All @@ -19,7 +22,11 @@ export async function redirectToAuthPage(
if (isXhrRequest) {
const redirectUri = new URL(config.auth.path, config.appUrl);
redirectUri.searchParams.set('shop', shop);
redirectWithAppBridgeHeaders(redirectUri.toString());
if (config.future.remixSingleFetch) {
redirectWithAppBridgeHeaders(redirectUri.toString());
} else {
redirectWithResponseWithAppBridgeHeaders(redirectUri.toString());
}
} else if (isEmbeddedRequest) {
redirectWithExitIframe(params, request, shop);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import {redirect as remixRedirect} from '@remix-run/server-runtime';

import type {BasicParams} from '../../../types';

import {redirectWithAppBridgeHeaders} from './redirect-with-app-bridge-headers';
import {
redirectWithAppBridgeHeaders,
redirectWithResponseWithAppBridgeHeaders,
} from './redirect-with-app-bridge-headers';

export async function redirectToInstallPage(
params: BasicParams,
Expand All @@ -11,7 +14,11 @@ export async function redirectToInstallPage(
): Promise<never> {
const installUrl = buildInstallUrl(params, shop, optionalScopes);
if (params.config.isEmbeddedApp) {
throw redirectWithAppBridgeHeaders(installUrl);
if (params.config.future.remixSingleFetch) {
throw redirectWithAppBridgeHeaders(installUrl);
} else {
throw redirectWithResponseWithAppBridgeHeaders(installUrl);
}
} else {
throw remixRedirect(installUrl);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import {redirect} from '@remix-run/server-runtime';

import {REAUTH_URL_HEADER} from '../../const';

export function redirectWithAppBridgeHeaders(redirectUri: string): never {
throw redirect(redirectUri, {
headers: getAppBridgeHeaders(redirectUri),
});
}

export function redirectWithResponseWithAppBridgeHeaders(
redirectUri: string,
): never {
throw new Response(undefined, {
status: 401,
statusText: 'Unauthorized',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import {BasicParams} from '../../../types';
import {getSessionTokenHeader} from '../../helpers/get-session-token-header';

import {renderAppBridge} from './render-app-bridge';
import {redirectWithAppBridgeHeaders} from './redirect-with-app-bridge-headers';
import {
redirectWithAppBridgeHeaders,
redirectWithResponseWithAppBridgeHeaders,
} from './redirect-with-app-bridge-headers';

export type RedirectTarget = '_self' | '_parent' | '_top' | '_blank';
export type RedirectInit = number | (ResponseInit & {target?: RedirectTarget});
Expand Down Expand Up @@ -67,7 +70,11 @@ export function redirectFactory(
return remixRedirect(parsedUrl.toString(), init);
}
} else if (isDataRequest(request)) {
throw redirectWithAppBridgeHeaders(parsedUrl.toString());
if (config.future.remixSingleFetch) {
throw redirectWithAppBridgeHeaders(parsedUrl.toString());
} else {
throw redirectWithResponseWithAppBridgeHeaders(parsedUrl.toString());
}
} else if (isEmbeddedRequest(request)) {
throw renderAppBridge(params, request, {
url: parsedUrl.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ it('returns app bridge redirection during request headers when Shopify invalidat
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);

const {origin, pathname, searchParams} = new URL(
response.headers.get(REAUTH_URL_HEADER)!,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('request from an embedded app', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);
const reuthorizeHeader = response.headers.get(
'x-shopify-api-request-failure-reauthorize-url',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ it('returns app bridge redirection during request headers when Shopify invalidat
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);

const {origin, pathname, searchParams} = new URL(
response.headers.get(REAUTH_URL_HEADER)!,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('admin.authenticate context', () => {
);

// THEN
expect(response.status).toEqual(401);
expect(response.status).toEqual(302);

const {origin, pathname, searchParams} = new URL(
response.headers.get(REAUTH_URL_HEADER)!,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('authorize.session token header path', () => {
response.headers.get(REAUTH_URL_HEADER)!,
);

expect(response.status).toBe(401);
expect(response.status).toBe(302);
expect(origin).toBe(APP_URL);
expect(pathname).toBe('/auth');
expect(searchParams.get('shop')).toBe(TEST_SHOP);
Expand Down Expand Up @@ -67,7 +67,7 @@ describe('authorize.session token header path', () => {
const {origin, pathname, searchParams} = new URL(
response.headers.get(REAUTH_URL_HEADER)!,
);
expect(response.status).toBe(401);
expect(response.status).toBe(302);
expect(origin).toBe(APP_URL);
expect(pathname).toBe('/auth');
expect(searchParams.get('shop')).toBe(TEST_SHOP);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ describe('authenticate', () => {
);
});

test('throws 401 unauthorized on XHR request when receiving an invalid subject token response from token exchange API', async () => {
test('throws 302 unauthorized on XHR request when receiving an invalid subject token response from token exchange API', async () => {
// GIVEN
const config = testConfig();
const shopify = shopifyApp(config);
Expand All @@ -207,7 +207,7 @@ describe('authenticate', () => {
);

// THEN
expect(response.status).toBe(401);
expect(response.status).toBe(302);
expect(
response.headers.get('X-Shopify-Retry-Invalid-Session-Request'),
).toEqual('1');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {redirect} from '@remix-run/server-runtime';

import {redirectToBouncePage} from '../admin/helpers/redirect-to-bounce-page';
import {RETRY_INVALID_SESSION_HEADER} from '../const';
import {BasicParams} from '../../types';
Expand All @@ -20,9 +22,18 @@ export function respondToInvalidSessionToken({
return redirectToBouncePage({api, logger, config}, new URL(request.url));
}

if (retryRequest) {
logger.debug('Retrying request after invalid session token', {
requestUrl: request.url,
});
throw redirect(request.url, {
headers: RETRY_INVALID_SESSION_HEADER,
});
}

throw new Response(undefined, {
status: 401,
statusText: 'Unauthorized',
headers: retryRequest ? RETRY_INVALID_SESSION_HEADER : {},
headers: {},
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('JWT validation', () => {
expect(response.status).toBe(401);
});

it('throws a 401 on invalid Authorization bearer token', async () => {
it('throws a 302 on invalid Authorization bearer token', async () => {
// GIVEN
const shopify = shopifyApp(testConfig());

Expand All @@ -122,7 +122,7 @@ describe('JWT validation', () => {
);

// THEN
expect(response.status).toBe(401);
expect(response.status).toBe(302);
});

it('rejects bot requests', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('JWT validation', () => {
expect(response.status).toBe(401);
});

it('throws a 401 on invalid Authorization bearer token', async () => {
it('throws a 302 on invalid Authorization bearer token', async () => {
// GIVEN
const shopify = shopifyApp(testConfig());

Expand All @@ -121,7 +121,7 @@ describe('JWT validation', () => {
);

// THEN
expect(response.status).toBe(401);
expect(response.status).toBe(302);
});

it('rejects bot requests', async () => {
Expand Down
Loading
Loading