Skip to content

Commit 1212038

Browse files
authored
bug: allow all https origins in handleMessage (#141)
1 parent 8a3690c commit 1212038

2 files changed

Lines changed: 54 additions & 42 deletions

File tree

platforms/web/src/checkout.test.ts

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,19 +1111,20 @@ describe("<shopify-checkout>", () => {
11111111
});
11121112

11131113
describe("message routing", () => {
1114-
it("drops protocol messages from an unexpected origin", async () => {
1114+
it("handles protocol messages from any HTTPS origin when the source matches", async () => {
11151115
const { checkout, mockCheckoutWindow } = openPopupCheckout();
11161116
const onStartSpy = vi.fn();
1117+
const payload = makeCheckoutPayload();
11171118
checkout.addEventListener("checkout:start", onStartSpy);
11181119

1119-
simulateProtocolMessageEvent(checkout, "ec.start", makeCheckoutPayload(), {
1120+
simulateProtocolMessageEvent(checkout, "ec.start", payload, {
11201121
source: mockCheckoutWindow,
11211122
origin: "https://other.example.com",
11221123
});
11231124
await Promise.resolve();
11241125

1125-
expect(onStartSpy).not.toHaveBeenCalled();
1126-
expect(checkout.checkout).toBeUndefined();
1126+
expect(onStartSpy).toHaveBeenCalledOnce();
1127+
expect(checkout.checkout).toBe(payload.checkout);
11271128
});
11281129

11291130
it("drops protocol messages when the source is not the checkout window", async () => {
@@ -1145,19 +1146,13 @@ describe("<shopify-checkout>", () => {
11451146
expect(checkout.checkout).toBeUndefined();
11461147
});
11471148

1148-
it("drops protocol messages when src is unset (no expected origin)", async () => {
1149-
// Set up a popup-style checkout WITHOUT a src so #expectedOrigin
1150-
// returns undefined and every inbound message is dropped.
1149+
it("drops protocol messages when src is unset even if the event origin is HTTPS", async () => {
11511150
const checkout = document.createElement("shopify-checkout");
11521151
document.body.appendChild(checkout);
11531152
const mockCheckoutWindow = createMockWindow();
11541153
vi.spyOn(window, "open").mockReturnValue(mockCheckoutWindow);
11551154
vi.spyOn(HTMLDialogElement.prototype, "showModal").mockImplementation(() => {});
11561155
vi.spyOn(HTMLDialogElement.prototype, "close").mockImplementation(() => {});
1157-
// open() with no src is a no-op (logs a warning), so we can't use
1158-
// it to set #checkoutWindow. Instead, set src first, open, then
1159-
// clear src so #expectedOrigin becomes undefined while
1160-
// #checkoutWindow remains the popup mock.
11611156
checkout.src = "https://shop.example.com/checkout";
11621157
checkout.open();
11631158
checkout.removeAttribute("src");
@@ -1178,16 +1173,11 @@ describe("<shopify-checkout>", () => {
11781173
await Promise.resolve();
11791174

11801175
expect(onStartSpy).not.toHaveBeenCalled();
1176+
expect(checkout.checkout).toBeUndefined();
11811177
});
11821178

1183-
it("drops protocol messages when src uses a non-https scheme", async () => {
1184-
// Open with valid https src so #checkoutWindow is set, then swap
1185-
// src to a non-https value. #expectedOrigin now returns
1186-
// undefined, and inbound messages should be dropped even though
1187-
// the source still matches #checkoutWindow.
1179+
it("drops protocol messages when the event origin is not HTTPS", async () => {
11881180
const { checkout, mockCheckoutWindow } = openPopupCheckout();
1189-
checkout.src = "http://shop.example.com/checkout";
1190-
11911181
const onStartSpy = vi.fn();
11921182
checkout.addEventListener("checkout:start", onStartSpy);
11931183

@@ -1198,6 +1188,22 @@ describe("<shopify-checkout>", () => {
11981188
await Promise.resolve();
11991189

12001190
expect(onStartSpy).not.toHaveBeenCalled();
1191+
expect(checkout.checkout).toBeUndefined();
1192+
});
1193+
1194+
it("drops protocol messages when the event origin is opaque", async () => {
1195+
const { checkout, mockCheckoutWindow } = openPopupCheckout();
1196+
const onStartSpy = vi.fn();
1197+
checkout.addEventListener("checkout:start", onStartSpy);
1198+
1199+
simulateProtocolMessageEvent(checkout, "ec.start", makeCheckoutPayload(), {
1200+
source: mockCheckoutWindow,
1201+
origin: "null",
1202+
});
1203+
await Promise.resolve();
1204+
1205+
expect(onStartSpy).not.toHaveBeenCalled();
1206+
expect(checkout.checkout).toBeUndefined();
12011207
});
12021208

12031209
it("ignores window 'message' events that aren't JSON-RPC checkout protocol messages", async () => {
@@ -1288,12 +1294,12 @@ describe("<shopify-checkout>", () => {
12881294

12891295
simulateProtocolMessageEvent(checkout, "ec.start", makeCheckoutPayload(), {
12901296
source: mockCheckoutWindow,
1291-
origin: "https://other.example.com",
1297+
origin: "http://shop.example.com",
12921298
});
12931299
await Promise.resolve();
12941300

12951301
expect(consoleWarnSpy).toHaveBeenCalledWith(
1296-
expect.stringContaining("Dropped message from unexpected origin"),
1302+
expect.stringContaining("Dropped message from non-HTTPS origin"),
12971303
);
12981304
});
12991305

@@ -1303,7 +1309,7 @@ describe("<shopify-checkout>", () => {
13031309

13041310
simulateProtocolMessageEvent(checkout, "ec.start", makeCheckoutPayload(), {
13051311
source: mockCheckoutWindow,
1306-
origin: "https://other.example.com",
1312+
origin: "http://shop.example.com",
13071313
});
13081314
await Promise.resolve();
13091315

@@ -1429,14 +1435,14 @@ describe("<shopify-checkout>", () => {
14291435
/**
14301436
* Dispatches a synthetic checkout-protocol MessageEvent at `window` so
14311437
* the component's listener processes it. By default both `source` and
1432-
* `origin` are derived from `checkout` so that the component's strict
1433-
* source-and-origin validation passes:
1438+
* `origin` are derived from `checkout` so that the component's source and
1439+
* HTTPS-origin validation passes:
14341440
*
14351441
* - `source`: pass the checkout browsing context (the mock window returned
14361442
* from `window.open` after `open()`, or another `MessageEventSource` to
14371443
* test drops). When omitted, defaults to `null` (messages are dropped).
14381444
* - `origin`: the origin of `checkout.src`. Override `origin` to test
1439-
* that messages from foreign origins are dropped.
1445+
* that messages from non-HTTPS origins are dropped.
14401446
*/
14411447
function simulateProtocolMessageEvent<Message extends keyof CheckoutProtocolMessageMap>(
14421448
checkout: ShopifyCheckout,
@@ -1529,9 +1535,9 @@ function createMockWindow() {
15291535

15301536
/**
15311537
* Sets up a popup-target checkout whose `#checkoutWindow` is a controllable
1532-
* mock window. Tests that exercise the strict source-and-origin validation
1533-
* in `#handleMessage` should use this helper and pass `mockCheckoutWindow`
1534-
* as `source` in `simulateProtocolMessageEvent`.
1538+
* mock window. Tests that exercise `#handleMessage` source validation should
1539+
* use this helper and pass `mockCheckoutWindow` as `source` in
1540+
* `simulateProtocolMessageEvent`.
15351541
*
15361542
* Callers receive the checkout, the mock window (use as both `source` for
15371543
* `simulateProtocolMessageEvent` and the spy target for response

platforms/web/src/checkout.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,16 +167,6 @@ export class ShopifyCheckout
167167
return url;
168168
}
169169

170-
/**
171-
* The origin we expect to receive postMessage events from. This is the
172-
* origin of `src` after URL parsing and scheme validation. Returns
173-
* `undefined` if `src` is unset or invalid, in which case all inbound
174-
* messages are dropped.
175-
*/
176-
#expectedOrigin(): string | undefined {
177-
return this.#srcAsURL()?.origin;
178-
}
179-
180170
/**
181171
* Whether the component should log diagnostic messages to the console.
182172
*/
@@ -510,6 +500,23 @@ export class ShopifyCheckout
510500
return message.id != null;
511501
}
512502

503+
#validateMessageOrigin(event: MessageEvent) {
504+
if (!this.#srcAsURL()) {
505+
throw new Error("Dropped message because src is invalid or unset");
506+
}
507+
508+
let origin: URL;
509+
try {
510+
origin = new URL(event.origin);
511+
} catch {
512+
throw new Error(`Dropped message from non-HTTPS origin "${event.origin}"`);
513+
}
514+
515+
if (origin.protocol !== "https:") {
516+
throw new Error(`Dropped message from non-HTTPS origin "${event.origin}"`);
517+
}
518+
}
519+
513520
#initCheckoutProtocol() {
514521
// Clean up any existing checkout protocol controller to prevent memory leaks
515522
// Necessary because connectedCallback() can be called multiple times
@@ -529,11 +536,10 @@ export class ShopifyCheckout
529536
// SDKs, browser extensions, etc.) is dropped silently.
530537
if (event.source !== this.#checkoutWindow) return;
531538

532-
const expected = this.#expectedOrigin();
533-
if (!expected || event.origin !== expected) {
534-
this.#debugWarn(
535-
`Dropped message from unexpected origin "${event.origin}" (expected "${expected ?? "none — src is invalid or unset"}")`,
536-
);
539+
try {
540+
this.#validateMessageOrigin(event);
541+
} catch (error) {
542+
this.#debugWarn(error instanceof Error ? error.message : String(error));
537543
return;
538544
}
539545

0 commit comments

Comments
 (0)