Skip to content

Commit 8edaf0f

Browse files
authored
Revert "Allow for CORS preflight cacheability by passing passport through custom header" (#168)
1 parent 285a6a1 commit 8edaf0f

File tree

3 files changed

+31
-65
lines changed

3 files changed

+31
-65
lines changed

lib/core/network.ts

+16-14
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import type { ResolvedConfig } from "../config";
22
import { default as buildInfo } from "../build.json";
33
import { LocalStorage } from "./storage";
44

5-
const identityHeaderName = "X-Optable-Visitor";
6-
75
function buildRequest(path: string, config: ResolvedConfig, init?: RequestInit): Request {
86
const { site, host, cookies } = config;
97

@@ -26,24 +24,21 @@ function buildRequest(path: string, config: ResolvedConfig, init?: RequestInit):
2624
url.searchParams.set("tcf", config.consent.tcf);
2725
}
2826

29-
const requestInit: RequestInit = { ...init };
30-
3127
if (cookies) {
32-
requestInit.credentials = "include";
3328
url.searchParams.set("cookies", "yes");
3429
} else {
3530
const ls = new LocalStorage(config);
3631
const pass = ls.getPassport();
3732
url.searchParams.set("cookies", "no");
38-
39-
if (pass) {
40-
const headers = new Headers(requestInit.headers);
41-
headers.set(identityHeaderName, pass);
42-
requestInit.headers = headers;
43-
}
33+
url.searchParams.set("passport", pass ? pass : "");
4434
}
4535

46-
return new Request(url.toString(), requestInit);
36+
const requestInit: RequestInit = { ...init };
37+
requestInit.credentials = "include";
38+
39+
const request = new Request(url.toString(), requestInit);
40+
41+
return request;
4742
}
4843

4944
async function fetch<T>(path: string, config: ResolvedConfig, init?: RequestInit): Promise<T> {
@@ -56,9 +51,16 @@ async function fetch<T>(path: string, config: ResolvedConfig, init?: RequestInit
5651
throw new Error(data.error);
5752
}
5853

59-
if (response.headers.has(identityHeaderName)) {
54+
if (data.passport) {
6055
const ls = new LocalStorage(config);
61-
ls.setPassport(response.headers.get(identityHeaderName) || "");
56+
ls.setPassport(data.passport);
57+
58+
// We delete the passport attribute from the returned payload. This is because
59+
// the targeting edge handler was initially made to return targeting data directly
60+
// in the form of 'key values' on the returned JSON payload -- if we don't delete
61+
// the `passport` attribute here, it may end up sent as targeting data to ad servers.
62+
// Not the end of the world, but something we want to avoid due to passport size.
63+
delete data.passport;
6264
}
6365

6466
return data;

lib/sdk.test.ts

+2-38
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ const defaultConfig = {
6464

6565
describe("Breaking change detection: if typescript complains or a test fails it's likely a breaking change has occurred.", () => {
6666
beforeEach(() => localStorage.clear());
67-
afterEach(() => jest.clearAllMocks());
6867

6968
test("TEST SHOULD NEVER NEED TO BE UPDATED, UNLESS MAJOR VERSION UPDATE: constructor with cookies and initPassport set", async () => {
7069
new OptableSDK({
@@ -233,7 +232,7 @@ describe("behavior testing of", () => {
233232
expect.objectContaining({
234233
method: "POST",
235234
_bodyText: '["c:a1a335b8216658319f96a4b0c718557ba41dd1f5"]',
236-
url: `${TEST_BASE_URL}/identify?osdk=web-0.0.0-experimental&cookies=no`,
235+
url: `${TEST_BASE_URL}/identify?osdk=web-0.0.0-experimental&cookies=no&passport=`,
237236
})
238237
);
239238

@@ -242,9 +241,8 @@ describe("behavior testing of", () => {
242241
expect(fetchSpy).toHaveBeenLastCalledWith(
243242
expect.objectContaining({
244243
method: "POST",
245-
headers: { map: expect.objectContaining({ "x-optable-visitor": "PASSPORT" }) },
246244
_bodyText: '["c:a1a335b8216658319f96a4b0c718557ba41dd1f6"]',
247-
url: `${TEST_BASE_URL}/identify?osdk=web-0.0.0-experimental&cookies=no`,
245+
url: `${TEST_BASE_URL}/identify?osdk=web-0.0.0-experimental&cookies=no&passport=PASSPORT`,
248246
})
249247
);
250248
});
@@ -467,38 +465,4 @@ describe("behavior testing of", () => {
467465
})
468466
);
469467
});
470-
471-
test("passport is passed along", async () => {
472-
const fetchSpy = jest.spyOn(window, "fetch");
473-
474-
const buildRes = (passport) => ({
475-
headers: new Headers({ "Content-Type": "application/json", "X-Optable-Visitor": passport }),
476-
ok: true,
477-
json: () => Promise.resolve({}),
478-
});
479-
480-
fetchSpy.mockImplementationOnce(() => Promise.resolve(buildRes("one")));
481-
fetchSpy.mockImplementationOnce(() => Promise.resolve(buildRes("two")));
482-
fetchSpy.mockImplementationOnce(() => Promise.resolve(buildRes("three")));
483-
484-
const sdk = new OptableSDK({ ...defaultConfig, initPassport: true, cookies: false });
485-
486-
let request;
487-
488-
await sdk.init;
489-
request = fetchSpy.mock.lastCall[0];
490-
expect(request.headers.get("X-Optable-Visitor")).toBe(null);
491-
492-
await sdk.identify("c:id").catch(() => {});
493-
request = fetchSpy.mock.lastCall[0];
494-
expect(request.headers.get("X-Optable-Visitor")).toBe("one");
495-
496-
await sdk.identify("c:id").catch(() => {});
497-
request = fetchSpy.mock.lastCall[0];
498-
expect(request.headers.get("X-Optable-Visitor")).toBe("two");
499-
500-
await sdk.identify("c:id").catch(() => {});
501-
request = fetchSpy.mock.lastCall[0];
502-
expect(request.headers.get("X-Optable-Visitor")).toBe("three");
503-
});
504468
});

lib/test/handlers.ts

+13-13
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import { ResolveResponse } from "edge/resolve";
99

1010
const ok200 = {
1111
status: 200,
12-
headers: new Headers({
13-
"X-Optable-Visitor": "PASSPORT",
14-
"Content-Type": "application/json",
15-
}),
12+
};
13+
14+
const passport: EdgePassport = {
15+
passport: "PASSPORT",
1616
};
1717

1818
const handlers = [
@@ -23,11 +23,11 @@ const handlers = [
2323
auctionConfig: null,
2424
getTopicsURL: "https://ads.optable.co/ca/topics/v1/get?origin=70cc15ee-484c-4d26-8868-c949a5c084b8",
2525
};
26-
return HttpResponse.json({ ...data }, ok200);
26+
return HttpResponse.json({ ...data, ...passport }, ok200);
2727
}),
2828

2929
http.post(`${TEST_BASE_URL}/identify`, async ({}) => {
30-
return HttpResponse.json({}, ok200);
30+
return HttpResponse.json({ ...passport }, ok200);
3131
}),
3232

3333
http.post(`${TEST_BASE_URL}/uid2/token`, async ({}) => {
@@ -39,22 +39,22 @@ const handlers = [
3939
RefreshExpires: 2734462312780,
4040
RefreshResponseKey: "gdsfgfsd",
4141
};
42-
return HttpResponse.json({ ...data }, ok200);
42+
return HttpResponse.json({ ...data, ...passport }, ok200);
4343
}),
4444

4545
http.post(`${TEST_BASE_URL}/witness`, async ({}) => {
46-
return HttpResponse.json({}, ok200);
46+
return HttpResponse.json({ ...passport }, ok200);
4747
}),
4848

4949
http.post(`${TEST_BASE_URL}/profile`, async ({}) => {
50-
return HttpResponse.json({}, ok200);
50+
return HttpResponse.json({ ...passport }, ok200);
5151
}),
5252

5353
http.get(`${TEST_BASE_URL}/v1/resolve`, async ({}) => {
5454
const data: ResolveResponse = {
5555
clusters: [],
5656
};
57-
return HttpResponse.json({ ...data }, ok200);
57+
return HttpResponse.json({ ...data, ...passport }, ok200);
5858
}),
5959

6060
http.post(`${TEST_BASE_URL}/v1/tokenize`, async ({}) => {
@@ -64,22 +64,22 @@ const handlers = [
6464
ext: undefined,
6565
},
6666
};
67-
return HttpResponse.json({ ...data }, ok200);
67+
return HttpResponse.json({ ...data, ...passport }, ok200);
6868
}),
6969

7070
http.get(`${TEST_BASE_URL}/v2/targeting`, async ({}) => {
7171
const data: TargetingResponse = {
7272
audience: [],
7373
user: [],
7474
};
75-
return HttpResponse.json({ ...data }, ok200);
75+
return HttpResponse.json({ ...data, ...passport }, ok200);
7676
}),
7777

7878
http.get(`${TEST_BASE_URL}/v1/resolve`, async ({}) => {
7979
const data: ResolveResponse = {
8080
clusters: [],
8181
};
82-
return HttpResponse.json({ ...data }, ok200);
82+
return HttpResponse.json({ ...data, ...passport }, ok200);
8383
}),
8484
];
8585

0 commit comments

Comments
 (0)