Skip to content

Commit 0a2e057

Browse files
RieranthonyRieranthony
committed
Fix contact identify auth order and visitor ID fallback
1 parent a6c973e commit 0a2e057

5 files changed

Lines changed: 269 additions & 13 deletions

File tree

‎apps/api/src/rest/openapi-contract.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from "bun:test";
22
import { readdirSync, readFileSync } from "node:fs";
33
import path from "node:path";
4+
import { identifyContactRequestSchema } from "@cossistant/types/api/contact";
45
import {
56
createConversationRequestSchema,
67
getConversationResponseSchema,
@@ -34,6 +35,7 @@ type OpenAPISchemaWithProperties = {
3435
string,
3536
OpenAPIMetadataSchema | OpenAPISchemaWithProperties
3637
>;
38+
required?: string[];
3739
};
3840

3941
type OpenAPIJsonContent = {
@@ -186,4 +188,52 @@ describe("REST OpenAPI contract guards", () => {
186188
expect(requestValueTypes).toEqual(["boolean", "null", "number", "string"]);
187189
expect(responseValueTypes).toEqual(["boolean", "null", "number", "string"]);
188190
});
191+
192+
it("documents contact identify visitorId precedence between body and X-Visitor-Id", () => {
193+
const app = new OpenAPIHono();
194+
195+
app.openapi(
196+
{
197+
method: "post",
198+
path: "/contacts/identify",
199+
request: {
200+
body: {
201+
required: true,
202+
content: {
203+
"application/json": {
204+
schema: identifyContactRequestSchema,
205+
},
206+
},
207+
},
208+
},
209+
responses: {
210+
200: {
211+
description: "Contact identified",
212+
},
213+
},
214+
},
215+
(() => new Response(null)) as never
216+
);
217+
218+
const doc = app.getOpenAPI31Document({
219+
openapi: "3.1.0",
220+
info: {
221+
title: "OpenAPI metadata contract test",
222+
version: "1.0.0",
223+
},
224+
});
225+
226+
const postPath = doc.paths?.["/contacts/identify"]?.post;
227+
const requestBody = postPath?.requestBody as OpenAPIJsonContent | undefined;
228+
const requestSchema = requestBody?.content?.["application/json"]?.schema as
229+
| OpenAPISchemaWithProperties
230+
| undefined;
231+
const visitorIdSchema = requestSchema?.properties?.visitorId as
232+
| OpenAPIMetadataSchema
233+
| undefined;
234+
235+
expect(requestSchema?.required ?? []).not.toContain("visitorId");
236+
expect(visitorIdSchema?.description).toContain("X-Visitor-Id");
237+
expect(visitorIdSchema?.description).toContain("body value wins");
238+
});
189239
});

‎apps/api/src/rest/routers/contact-auth-order.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { readFileSync } from "node:fs";
3+
import { OpenAPIHono } from "@hono/zod-openapi";
4+
5+
const contactRouterPath = new URL("./contact.ts", import.meta.url);
6+
7+
describe("contact router auth ordering", () => {
8+
it("mounts runtime routes before control routes and preserves public identify access", async () => {
9+
const source = readFileSync(contactRouterPath, "utf8");
10+
const runtimeRouteIndex = source.indexOf(
11+
'.route("/", contactRuntimeRouter)'
12+
);
13+
const controlRouteIndex = source.indexOf(
14+
'.route("/", contactControlRouter)'
15+
);
16+
17+
expect(runtimeRouteIndex).toBeGreaterThan(-1);
18+
expect(controlRouteIndex).toBeGreaterThan(-1);
19+
expect(runtimeRouteIndex).toBeLessThan(controlRouteIndex);
20+
21+
const runtimeRouter = new OpenAPIHono();
22+
runtimeRouter.use("/*", async (c, next) => {
23+
if (!c.req.header("X-Public-Key")) {
24+
return c.json(
25+
{ error: "UNAUTHORIZED", message: "API key is required" },
26+
401
27+
);
28+
}
29+
30+
await next();
31+
});
32+
runtimeRouter.post("/identify", (c) => c.json({ ok: true }, 200));
33+
34+
const controlRouter = new OpenAPIHono();
35+
controlRouter.use("/*", async (c, next) => {
36+
if (!c.req.header("Authorization")?.startsWith("Bearer ")) {
37+
return c.json(
38+
{ error: "UNAUTHORIZED", message: "API key is required" },
39+
401
40+
);
41+
}
42+
43+
await next();
44+
});
45+
46+
const router = new OpenAPIHono()
47+
.route("/", runtimeRouter)
48+
.route("/", controlRouter);
49+
50+
const response = await router.request(
51+
new Request("http://localhost/identify", {
52+
method: "POST",
53+
headers: {
54+
"X-Public-Key": "pk_test_123",
55+
},
56+
})
57+
);
58+
59+
expect(response.status).toBe(200);
60+
});
61+
});

‎apps/api/src/rest/routers/contact-identify.test.ts‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { beforeEach, describe, expect, it, mock } from "bun:test";
2+
import { APIKeyType } from "@cossistant/types";
23

34
const safelyExtractRequestDataMock = mock((async () => ({})) as (
45
...args: unknown[]
@@ -101,6 +102,7 @@ mock.module("@api/realtime/emitter", () => ({
101102

102103
mock.module("../middleware", () => ({
103104
protectedPublicApiKeyMiddleware: [],
105+
protectedPrivateApiKeyMiddleware: [],
104106
}));
105107

106108
const contactRouterModulePromise = import("./contact");
@@ -226,9 +228,135 @@ describe("contact identify route", () => {
226228
expect(identifyArg.email).toBeUndefined();
227229
});
228230

231+
it("returns 200 when visitorId is provided via X-Visitor-Id header", async () => {
232+
const db = {};
233+
safelyExtractRequestDataMock.mockResolvedValue({
234+
db,
235+
website: {
236+
id: "site-1",
237+
organizationId: "org-1",
238+
},
239+
visitorIdHeader: "visitor-header-1",
240+
body: {
241+
externalId: "user-123",
242+
email: undefined,
243+
name: "User",
244+
image: undefined,
245+
metadata: undefined,
246+
contactOrganizationId: undefined,
247+
},
248+
});
249+
250+
const { contactRouter } = await contactRouterModulePromise;
251+
const response = await contactRouter.request(
252+
new Request("http://localhost/identify", {
253+
method: "POST",
254+
})
255+
);
256+
const payload = (await response.json()) as {
257+
visitorId: string;
258+
};
259+
260+
expect(response.status).toBe(200);
261+
expect(findVisitorForWebsiteMock).toHaveBeenCalledWith(db, {
262+
visitorId: "visitor-header-1",
263+
websiteId: "site-1",
264+
});
265+
expect(linkVisitorToContactMock).toHaveBeenCalledWith(db, {
266+
visitorId: "visitor-header-1",
267+
contactId: "contact-1",
268+
websiteId: "site-1",
269+
});
270+
expect(payload.visitorId).toBe("visitor-header-1");
271+
});
272+
273+
it("prefers body visitorId over X-Visitor-Id header when both are provided", async () => {
274+
const db = {};
275+
safelyExtractRequestDataMock.mockResolvedValue({
276+
db,
277+
website: {
278+
id: "site-1",
279+
organizationId: "org-1",
280+
},
281+
visitorIdHeader: "visitor-header-1",
282+
body: {
283+
visitorId: "visitor-body-1",
284+
externalId: "user-123",
285+
email: undefined,
286+
name: "User",
287+
image: undefined,
288+
metadata: undefined,
289+
contactOrganizationId: undefined,
290+
},
291+
});
292+
293+
const { contactRouter } = await contactRouterModulePromise;
294+
const response = await contactRouter.request(
295+
new Request("http://localhost/identify", {
296+
method: "POST",
297+
})
298+
);
299+
const payload = (await response.json()) as {
300+
visitorId: string;
301+
};
302+
303+
expect(response.status).toBe(200);
304+
expect(findVisitorForWebsiteMock).toHaveBeenCalledWith(db, {
305+
visitorId: "visitor-body-1",
306+
websiteId: "site-1",
307+
});
308+
expect(linkVisitorToContactMock).toHaveBeenCalledWith(db, {
309+
visitorId: "visitor-body-1",
310+
contactId: "contact-1",
311+
websiteId: "site-1",
312+
});
313+
expect(payload.visitorId).toBe("visitor-body-1");
314+
});
315+
316+
it("returns 400 when neither body nor header provides a visitorId", async () => {
317+
safelyExtractRequestDataMock.mockResolvedValue({
318+
db: {},
319+
website: {
320+
id: "site-1",
321+
organizationId: "org-1",
322+
},
323+
visitorIdHeader: null,
324+
body: {
325+
externalId: "user-123",
326+
email: undefined,
327+
name: "User",
328+
image: undefined,
329+
metadata: undefined,
330+
contactOrganizationId: undefined,
331+
},
332+
});
333+
334+
const { contactRouter } = await contactRouterModulePromise;
335+
const response = await contactRouter.request(
336+
new Request("http://localhost/identify", {
337+
method: "POST",
338+
})
339+
);
340+
341+
const payload = (await response.json()) as {
342+
error: string;
343+
message: string;
344+
};
345+
346+
expect(response.status).toBe(400);
347+
expect(payload).toEqual({
348+
error: "BAD_REQUEST",
349+
message: "Visitor not found, please pass a valid visitorId",
350+
});
351+
expect(findVisitorForWebsiteMock).toHaveBeenCalledTimes(0);
352+
expect(identifyContactMock).toHaveBeenCalledTimes(0);
353+
});
354+
229355
it("POST /contacts returns 201 when externalId upsert creates a new contact", async () => {
230356
safelyExtractRequestDataMock.mockResolvedValue({
231357
db: {},
358+
apiKey: { keyType: APIKeyType.PRIVATE },
359+
organization: { id: "org-1" },
232360
website: {
233361
id: "site-1",
234362
organizationId: "org-1",
@@ -272,6 +400,8 @@ describe("contact identify route", () => {
272400
it("POST /contacts returns 200 when externalId upsert updates an existing contact", async () => {
273401
safelyExtractRequestDataMock.mockResolvedValue({
274402
db: {},
403+
apiKey: { keyType: APIKeyType.PRIVATE },
404+
organization: { id: "org-1" },
275405
website: {
276406
id: "site-1",
277407
organizationId: "org-1",
@@ -307,6 +437,8 @@ describe("contact identify route", () => {
307437
it("POST /contacts keeps create-only behavior when externalId is not provided", async () => {
308438
safelyExtractRequestDataMock.mockResolvedValue({
309439
db: {},
440+
apiKey: { keyType: APIKeyType.PRIVATE },
441+
organization: { id: "org-1" },
310442
website: {
311443
id: "site-1",
312444
organizationId: "org-1",

‎apps/api/src/rest/routers/contact.ts‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ contactRuntimeRouter.openapi(
122122
path: "/identify",
123123
summary: "Identify a visitor",
124124
description:
125-
"Creates or updates a contact for a visitor. If a contact with the same externalId or email exists, it will be updated. The visitor will be linked to the contact.",
125+
"Creates or updates a contact for a visitor. If a contact with the same externalId or email exists, it will be updated. The visitor will be linked to the contact. Public callers may pass the visitor ID in the request body or via X-Visitor-Id, and the body value takes precedence when both are provided.",
126126
request: {
127127
body: {
128128
content: {
@@ -150,10 +150,8 @@ contactRuntimeRouter.openapi(
150150
},
151151
async (c) => {
152152
try {
153-
const { db, website, body } = await safelyExtractRequestData(
154-
c,
155-
identifyContactRequestSchema
156-
);
153+
const { db, website, body, visitorIdHeader } =
154+
await safelyExtractRequestData(c, identifyContactRequestSchema);
157155

158156
if (!(website?.id && website.organizationId)) {
159157
return c.json(
@@ -162,6 +160,18 @@ contactRuntimeRouter.openapi(
162160
);
163161
}
164162

163+
const resolvedVisitorId = body.visitorId ?? visitorIdHeader;
164+
165+
if (!resolvedVisitorId) {
166+
return c.json(
167+
{
168+
error: "BAD_REQUEST",
169+
message: "Visitor not found, please pass a valid visitorId",
170+
},
171+
400
172+
);
173+
}
174+
165175
const { externalId, email } = normalizeIdentifyContactIdentifiers({
166176
externalId: body.externalId,
167177
email: body.email,
@@ -178,7 +188,7 @@ contactRuntimeRouter.openapi(
178188
}
179189

180190
const visitor = await findVisitorForWebsite(db, {
181-
visitorId: body.visitorId,
191+
visitorId: resolvedVisitorId,
182192
websiteId: website.id,
183193
});
184194

@@ -201,13 +211,13 @@ contactRuntimeRouter.openapi(
201211
});
202212

203213
await linkVisitorToContact(db, {
204-
visitorId: body.visitorId,
214+
visitorId: resolvedVisitorId,
205215
contactId: contact.id,
206216
websiteId: website.id,
207217
});
208218

209219
const visitorRecord = await getCompleteVisitorWithContact(db, {
210-
visitorId: body.visitorId,
220+
visitorId: resolvedVisitorId,
211221
});
212222

213223
if (visitorRecord) {
@@ -228,7 +238,7 @@ contactRuntimeRouter.openapi(
228238

229239
const response: IdentifyContactResponse = {
230240
contact: formatContactResponse(contact),
231-
visitorId: body.visitorId,
241+
visitorId: resolvedVisitorId,
232242
};
233243

234244
return c.json(
@@ -1054,6 +1064,8 @@ contactControlRouter.openapi(
10541064
}
10551065
);
10561066

1067+
// Mount shared runtime routes before private control routes so public requests
1068+
// like POST /contacts/identify are not intercepted by private-only middleware.
10571069
export const contactRouter = new OpenAPIHono<RestContext>()
1058-
.route("/", contactControlRouter)
1059-
.route("/", contactRuntimeRouter);
1070+
.route("/", contactRuntimeRouter)
1071+
.route("/", contactControlRouter);

‎packages/types/src/api/contact.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,9 @@ export const identifyContactRequestSchema = z.object({
141141
"Optional contact ID to update when linking the visitor to an existing contact.",
142142
example: "01JG000000000000000000000",
143143
}),
144-
visitorId: z.ulid().openapi({
145-
description: "The visitor ID to link to the contact.",
144+
visitorId: z.ulid().optional().openapi({
145+
description:
146+
"The visitor ID to link to the contact. Optional when passed via the X-Visitor-Id header. If both are provided, the body value wins.",
146147
example: "01JG000000000000000000000",
147148
}),
148149
externalId: z

0 commit comments

Comments
 (0)