From bfcbbfac393687cf14f05a62969fb85ba74cbfff Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 02:32:20 +0200 Subject: [PATCH] fix(orders): forward only the checkout's fields on an order update PUT /api/orders/:uuid forwarded the browser's body to the CMS unchanged. It now forwards { data } with only the seven fields of the checkout's steps: email, acceptedTermsAndConditionsAt, invoiceAddress, deliveryAddress, invoiceAddressStructured, deliveryAddressStructured and delivery. Any other field, a field beside data, or a body of another shape is answered 400 "Invalid order update" with the CMS's error format, without calling the CMS. The values are left to the CMS, which checks them and stays the authority; this is defence in depth. pickCustomerUpdate (server/utils/customerUpdate.ts) is pure and tested with the exact payloads of steps 1 and 2 and with every server-only attribute of the order. Refs https://git.librete.ch/libretech/mp/issues/71 --- server/api/orders/[uuid].put.ts | 9 +- server/utils/customerUpdate.ts | 79 ++++++++++++++ tests/unit/customerUpdate.test.ts | 168 ++++++++++++++++++++++++++++++ 3 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 server/utils/customerUpdate.ts create mode 100644 tests/unit/customerUpdate.test.ts diff --git a/server/api/orders/[uuid].put.ts b/server/api/orders/[uuid].put.ts index de2ac64..559f705 100644 --- a/server/api/orders/[uuid].put.ts +++ b/server/api/orders/[uuid].put.ts @@ -1,4 +1,5 @@ import { forwardToCms } from "~/server/utils/cmsApi"; +import { invalidOrderUpdate, pickCustomerUpdate } from "~/server/utils/customerUpdate"; export default defineEventHandler(async (event) => { const uuid = getRouterParam(event, "uuid"); @@ -6,10 +7,14 @@ export default defineEventHandler(async (event) => { throw createError({ statusCode: 400, statusMessage: "Missing order UUID" }); } - const body = await readBody(event); + // Only the fields of the checkout's steps reach the CMS, which checks their values. Any other field is answered 400 here. + const update = pickCustomerUpdate(await readBody(event)); + if (update.ok === false) { + throw createError(invalidOrderUpdate(update.errors)); + } return await forwardToCms(`/orders/${uuid}/cart`, { method: "PUT", - body + body: { data: update.data } }); }); diff --git a/server/utils/customerUpdate.ts b/server/utils/customerUpdate.ts new file mode 100644 index 0000000..b24319d --- /dev/null +++ b/server/utils/customerUpdate.ts @@ -0,0 +1,79 @@ +// What PUT /api/orders/:uuid forwards to the CMS: the fields of the checkout's steps, and nothing else. +// Step 1 (pages/checkout/1.vue) sends { data: { email, acceptedTermsAndConditionsAt } }, step 2 (pages/checkout/2.vue) sends +// { data: { invoiceAddress, deliveryAddress, invoiceAddressStructured, deliveryAddressStructured, delivery } }. +// The CMS checks the values and stays the authority (libreshop/cms src/checkout/customer-update.ts). This check is defence in depth: +// a body with any other field is answered 400 without calling the CMS, in the format of the CMS's own answer. +// Pure: no Nuxt, Nitro or h3 imports, tested in tests/unit/customerUpdate.test.ts. +import type { ShopError } from "./cmsError"; + +/** The order fields a customer may set, in the order of the checkout's steps. */ +export const CUSTOMER_UPDATE_FIELDS = [ + "email", + "acceptedTermsAndConditionsAt", + "invoiceAddress", + "deliveryAddress", + "invoiceAddressStructured", + "deliveryAddressStructured", + "delivery" +] as const; + +export type CustomerUpdateField = (typeof CUSTOMER_UPDATE_FIELDS)[number]; + +/** The fields to forward, with the values the browser sent: the CMS checks them. */ +export type CustomerUpdate = Partial>; + +/** + * unknown: the names of the rejected fields, such as "data.paymentAuthorised", or "email" for a field sent beside data. + * errors: one message per rejected field or malformed part, in the CMS's format "name: reason". + */ +export type PickedCustomerUpdate = { ok: true; data: CustomerUpdate } | { ok: false; unknown: string[]; errors: string[] }; + +export const INVALID_ORDER_UPDATE = "Invalid order update"; + +// A field name is chosen by the client and ends up in the answer, so a long one is cut. +const MAX_NAME_LENGTH = 64; + +const isObject = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); + +const isCustomerUpdateField = (key: string): key is CustomerUpdateField => (CUSTOMER_UPDATE_FIELDS as readonly string[]).includes(key); + +const fieldName = (key: string): string => (key.length > MAX_NAME_LENGTH ? `${key.slice(0, MAX_NAME_LENGTH)}…` : key); + +/** Picks the fields a customer may set from the body { data: { … } }. Any other field, or another shape, rejects the whole body. */ +export const pickCustomerUpdate = (body: unknown): PickedCustomerUpdate => { + if (!isObject(body)) { + return { ok: false, unknown: [], errors: ["body: must be an object of the form { data: { … } }"] }; + } + + const unknown: string[] = []; + const errors: string[] = []; + for (const key of Object.keys(body)) { + if (key === "data") continue; + unknown.push(fieldName(key)); + errors.push(`${fieldName(key)}: not accepted, the fields belong in data`); + } + + const fields = body.data; + const data: CustomerUpdate = {}; + if (!isObject(fields)) { + errors.push(fields === undefined ? "data: missing" : "data: must be an object"); + } else { + for (const key of Object.keys(fields)) { + if (isCustomerUpdateField(key)) { + data[key] = fields[key]; + } else { + unknown.push(`data.${fieldName(key)}`); + errors.push(`data.${fieldName(key)}: not a field the customer may set`); + } + } + } + + return errors.length > 0 ? { ok: false, unknown, errors } : { ok: true, data }; +}; + +/** The 400 for a rejected body: the shape the shop answers for the CMS's own 400 (cmsError.ts). */ +export const invalidOrderUpdate = (errors: string[]): ShopError => ({ + statusCode: 400, + statusMessage: INVALID_ORDER_UPDATE, + data: { message: INVALID_ORDER_UPDATE, errors } +}); diff --git a/tests/unit/customerUpdate.test.ts b/tests/unit/customerUpdate.test.ts new file mode 100644 index 0000000..f281f2f --- /dev/null +++ b/tests/unit/customerUpdate.test.ts @@ -0,0 +1,168 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { CUSTOMER_UPDATE_FIELDS, invalidOrderUpdate, pickCustomerUpdate } from "../../server/utils/customerUpdate.ts"; + +// The body as readBody hands it to the route: sent by useShopApi().updateOrder as JSON. +const overTheWire = (body: unknown): unknown => JSON.parse(JSON.stringify(body)); + +const address = { + givenName: "Erika", + familyName: "Mustermann", + streetAddress: "Musterstraße 1", + postalCode: "70190", + addressLevel2: "Stuttgart", + country: "DE" +}; +const otherAddress = { + givenName: "Max", + familyName: "Muster", + streetAddress: "Hauptstraße 5", + postalCode: "10115", + addressLevel2: "Berlin", + country: "DE" +}; + +// pages/checkout/1.vue: cart.update({ email, acceptedTermsAndConditionsAt }). +const step1 = { data: { email: "erika@example.org", acceptedTermsAndConditionsAt: "2026-10-09T08:15:00.000Z" } }; + +// pages/checkout/2.vue without a separate delivery address: the invoice address is sent as the delivery address too. +const step2 = { + data: { + invoiceAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", + deliveryAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", + invoiceAddressStructured: address, + deliveryAddressStructured: address, + delivery: 1 + } +}; + +// The order's attributes in the CMS (src/api/order/content-types/order/schema.json) that only the server writes, and Strapi's own. +const SERVER_FIELDS = [ + "id", + "uuid", + "date", + "customer", + "invoice", + "deliveryNote", + "hash", + "payment", + "VAT", + "subtotal", + "total", + "cart", + "paymentAuthorised", + "paymentStatus", + "paypalOrderId", + "paypalCaptureId", + "paymentCapturedAt", + "emailSent", + "invoiceSent", + "deliveryNoteSent", + "invoiceNumber", + "deliveryNoteNumber", + "deliveryTrackingNumber", + "createdAt", + "updatedAt", + "publishedAt" +]; + +test("lists the seven fields of the checkout's steps", () => { + assert.deepEqual( + [...CUSTOMER_UPDATE_FIELDS], + [ + "email", + "acceptedTermsAndConditionsAt", + "invoiceAddress", + "deliveryAddress", + "invoiceAddressStructured", + "deliveryAddressStructured", + "delivery" + ] + ); +}); + +test("passes the payload of checkout step 1 unchanged", () => { + assert.deepEqual(pickCustomerUpdate(overTheWire(step1)), { ok: true, data: step1.data }); +}); + +test("passes the payload of checkout step 2 unchanged", () => { + assert.deepEqual(pickCustomerUpdate(overTheWire(step2)), { ok: true, data: step2.data }); +}); + +test("passes the payload of checkout step 2 with a separate delivery address unchanged", () => { + const body = { + data: { + ...step2.data, + deliveryAddress: "Max Muster\nHauptstraße 5\n10115 Berlin", + deliveryAddressStructured: otherAddress, + delivery: 2 + } + }; + + assert.deepEqual(pickCustomerUpdate(overTheWire(body)), { ok: true, data: body.data }); +}); + +test("leaves the values to the CMS, which checks them", () => { + const body = { data: { email: "no address", delivery: null, invoiceAddressStructured: { street: "x" } } }; + + assert.deepEqual(pickCustomerUpdate(body), { ok: true, data: body.data }); +}); + +test("rejects each field only the server writes", () => { + for (const field of SERVER_FIELDS) { + assert.deepEqual( + pickCustomerUpdate({ data: { [field]: 1 } }), + { ok: false, unknown: [`data.${field}`], errors: [`data.${field}: not a field the customer may set`] }, + field + ); + } +}); + +test("rejects the whole body instead of dropping the field, and names every rejected field", () => { + const body = { data: { ...step1.data, paymentAuthorised: true, total: 0.01 } }; + + assert.deepEqual(pickCustomerUpdate(body), { + ok: false, + unknown: ["data.paymentAuthorised", "data.total"], + errors: ["data.paymentAuthorised: not a field the customer may set", "data.total: not a field the customer may set"] + }); +}); + +test("rejects a field sent beside data", () => { + assert.deepEqual(pickCustomerUpdate({ email: "erika@example.org", data: {} }), { + ok: false, + unknown: ["email"], + errors: ["email: not accepted, the fields belong in data"] + }); +}); + +test("rejects a body without data, with data that is not an object, and a body that is not an object", () => { + assert.deepEqual(pickCustomerUpdate({}), { ok: false, unknown: [], errors: ["data: missing"] }); + assert.deepEqual(pickCustomerUpdate({ data: [step1.data] }), { ok: false, unknown: [], errors: ["data: must be an object"] }); + assert.deepEqual(pickCustomerUpdate({ data: null }), { ok: false, unknown: [], errors: ["data: must be an object"] }); + for (const body of [undefined, null, "data", [step1]]) { + assert.deepEqual(pickCustomerUpdate(body), { ok: false, unknown: [], errors: ["body: must be an object of the form { data: { … } }"] }); + } +}); + +test("rejects __proto__ and constructor without touching any prototype", () => { + const result = pickCustomerUpdate(JSON.parse('{"data":{"__proto__":{"paymentAuthorised":true},"constructor":{"prototype":{}}}}')); + + assert.equal(result.ok, false); + assert.deepEqual(result.ok === false && result.unknown, ["data.__proto__", "data.constructor"]); + assert.equal(({} as Record).paymentAuthorised, undefined); +}); + +test("cuts a long field name in the answer", () => { + const result = pickCustomerUpdate({ data: { ["x".repeat(100)]: 1 } }); + + assert.deepEqual(result.ok === false && result.unknown, [`data.${"x".repeat(64)}…`]); +}); + +test("answers a rejected body with a 400 in the shape of the CMS's own 400", () => { + assert.deepEqual(invalidOrderUpdate(["data.total: not a field the customer may set"]), { + statusCode: 400, + statusMessage: "Invalid order update", + data: { message: "Invalid order update", errors: ["data.total: not a field the customer may set"] } + }); +});