From b9670b7229e51e50ba1555e97b37e5d28cf0b667 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Thu, 8 Oct 2026 22:54:59 +0200 Subject: [PATCH 01/16] feat(order): parse the customer's order update against the checkout fields parseCustomerUpdate accepts { data } with a subset of the seven fields the shop's checkout sends: email, acceptedTermsAndConditionsAt, invoiceAddress, deliveryAddress, invoiceAddressStructured, deliveryAddressStructured and delivery. Any other key, at the top level, in data or in a structured address, rejects the whole body, and the errors name the fields without repeating their values. The terms are recorded at the server's time whenever the client sends a true value: the invoice is dated by it. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/checkout/customer-update.ts | 146 ++++++++++++ tests/unit/customer-update.test.ts | 348 +++++++++++++++++++++++++++++ 2 files changed, 494 insertions(+) create mode 100644 src/checkout/customer-update.ts create mode 100644 tests/unit/customer-update.test.ts diff --git a/src/checkout/customer-update.ts b/src/checkout/customer-update.ts new file mode 100644 index 0000000..43dd8ba --- /dev/null +++ b/src/checkout/customer-update.ts @@ -0,0 +1,146 @@ +// What a customer may write to an order: the fields of the shop's checkout steps, and nothing else. +// The shop forwards the browser's body unchanged (libreshop/shop server/api/orders/[uuid].put.ts), so the body is untrusted. +// Step 1 (pages/checkout/1.vue) sends { data: { email, acceptedTermsAndConditionsAt } }, step 2 (pages/checkout/2.vue) sends +// { data: { invoiceAddress, deliveryAddress, invoiceAddressStructured, deliveryAddressStructured, delivery } }. +// Payment state, totals, numbers, documents, the cart and the customer are written by the server alone. A body that sends any +// of them is rejected as a whole instead of stripped, so a client that relies on such a write fails visibly. + +/** The attributes of the address.structured-address component (src/components/address/structured-address.json). */ +const STRUCTURED_ADDRESS_FIELDS = ["givenName", "familyName", "streetAddress", "postalCode", "addressLevel2", "country"] as const; + +export type CustomerStructuredAddress = Partial>; + +/** The order fields a customer may set, each optional. */ +export type CustomerUpdate = { + email?: string; + acceptedTermsAndConditionsAt?: string; + invoiceAddress?: string; + deliveryAddress?: string; + invoiceAddressStructured?: CustomerStructuredAddress; + deliveryAddressStructured?: CustomerStructuredAddress; + delivery?: number; +}; + +/** + * errors: one message per rejected field, which names the field and never repeats its value. + * fields: the names of the rejected fields alone, for the log. + */ +export type CustomerUpdateResult = { ok: true; data: CustomerUpdate } | { ok: false; errors: string[]; fields: string[] }; + +// The longest address an SMTP path of 256 octets, angle brackets included, can carry (RFC 5321). +const MAX_EMAIL_LENGTH = 254; +const MAX_ADDRESS_LENGTH = 500; +const MAX_ADDRESS_COMPONENT_LENGTH = 200; +// Strapi's ids are Postgres integer columns: a larger number makes the query fail instead of finding nothing. +const MAX_ID = 2147483647; +// Rejected keys are chosen by the client and end up in the response and the log, so a long one is cut. +const MAX_KEY_LENGTH = 64; + +const isObject = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); + +const keyName = (key: string): string => (key.length > MAX_KEY_LENGTH ? `${key.slice(0, MAX_KEY_LENGTH)}…` : key); + +const isStructuredAddressField = (key: string): key is (typeof STRUCTURED_ADDRESS_FIELDS)[number] => + (STRUCTURED_ADDRESS_FIELDS as readonly string[]).includes(key); + +// An "@" with something on both sides, and no whitespace or control character: the invoice is mailed to this address. +const isEmail = (email: string): boolean => { + const at = email.lastIndexOf("@"); + return email.length <= MAX_EMAIL_LENGTH && at > 0 && at < email.length - 1 && !/[\s\u0000-\u001f\u007f]/.test(email); +}; + +const isText = (value: unknown, maxLength: number): value is string => typeof value === "string" && value.length <= maxLength; + +// The shop's form sends every component as a string (pages/checkout/2.vue, mapToStructuredAddress), but it fills the form from +// the stored address, whose unset components Strapi returns as null. Every component of the address is optional. +const isAddressComponent = (value: unknown): value is string | null => value === null || isText(value, MAX_ADDRESS_COMPONENT_LENGTH); + +/** + * Checks the body of PUT /orders/:uuid/cart. Returns the fields to write, or why the body is rejected. + * `now` is the server's time, recorded as the time the customer accepted the terms. + */ +export const parseCustomerUpdate = (body: unknown, now: Date): CustomerUpdateResult => { + const rejected: { field: string; reason: string }[] = []; + const reject = (field: string, reason: string) => { + rejected.push({ field, reason }); + }; + const data: CustomerUpdate = {}; + + const parseStructuredAddress = (field: string, value: unknown): CustomerStructuredAddress | undefined => { + if (!isObject(value)) { + reject(field, "must be an object"); + return undefined; + } + const address: CustomerStructuredAddress = {}; + let valid = true; + for (const key of Object.keys(value)) { + const component = value[key]; + if (!isStructuredAddressField(key)) { + reject(`${field}.${keyName(key)}`, "not a structured address field"); + valid = false; + } else if (isAddressComponent(component)) { + address[key] = component; + } else { + reject(`${field}.${key}`, `must be a string of at most ${MAX_ADDRESS_COMPONENT_LENGTH} characters, or null`); + valid = false; + } + } + return valid ? address : undefined; + }; + + if (!isObject(body)) { + reject("body", "must be an object of the form { data: { … } }"); + } else { + for (const key of Object.keys(body)) { + if (key !== "data") reject(keyName(key), "not accepted, the fields belong in data"); + } + + const fields = body.data; + if (fields === undefined) { + reject("data", "missing"); + } else if (!isObject(fields)) { + reject("data", "must be an object"); + } else { + for (const key of Object.keys(fields)) { + const value = fields[key]; + switch (key) { + case "email": { + const email = typeof value === "string" ? value.trim() : ""; + if (isEmail(email)) data.email = email; + else reject("data.email", `must be an email address of at most ${MAX_EMAIL_LENGTH} characters`); + break; + } + case "acceptedTermsAndConditionsAt": + // The invoice is dated by this time: it is the Ausstellungsdatum in the invoice PDF (the order service's + // defaultPdfBody) and in the invoice email (src/templates/emailTemplates.ts). The client's clock and value + // are ignored, so that no client can backdate or postdate an invoice. + if (value) data.acceptedTermsAndConditionsAt = now.toISOString(); + else reject("data.acceptedTermsAndConditionsAt", "must be sent only to accept the terms, as true or a timestamp"); + break; + case "invoiceAddress": + case "deliveryAddress": + if (isText(value, MAX_ADDRESS_LENGTH)) data[key] = value; + else reject(`data.${key}`, `must be a string of at most ${MAX_ADDRESS_LENGTH} characters`); + break; + case "invoiceAddressStructured": + case "deliveryAddressStructured": { + const address = parseStructuredAddress(`data.${key}`, value); + if (address) data[key] = address; + break; + } + case "delivery": + if (typeof value === "number" && Number.isInteger(value) && value > 0 && value <= MAX_ID) data.delivery = value; + else reject("data.delivery", "must be the id of a delivery method, a positive integer"); + break; + default: + reject(`data.${keyName(key)}`, "not a field the customer may set"); + } + } + } + } + + if (rejected.length > 0) { + return { ok: false, errors: rejected.map(({ field, reason }) => `${field}: ${reason}`), fields: rejected.map(({ field }) => field) }; + } + return { ok: true, data }; +}; diff --git a/tests/unit/customer-update.test.ts b/tests/unit/customer-update.test.ts new file mode 100644 index 0000000..9dd12af --- /dev/null +++ b/tests/unit/customer-update.test.ts @@ -0,0 +1,348 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { parseCustomerUpdate } from "../../src/checkout/customer-update.ts"; + +// The server's time when the update arrives. +const NOW = new Date("2026-10-08T19:12:05.000Z"); + +const parse = (body: unknown) => parseCustomerUpdate(body, NOW); + +// The names of the fields a body is rejected for, or [] if it is accepted. +const rejectedFields = (body: unknown) => { + const result = parse(body); + return result.ok ? [] : result.fields; +}; + +// What the controller receives: the JSON the shop sent, parsed by Strapi's body parser. +const wire = (body: unknown) => JSON.parse(JSON.stringify(body)); + +// The shop's checkout, libreshop/shop: useCart().update(data) sends { data } (composables/useShopApi.ts, updateOrder), and its +// server route forwards that body unchanged. +// Step 1, pages/checkout/1.vue: the email and the time the browser thinks the terms were accepted. +const STEP_1 = { data: { email: "erika@example.org", acceptedTermsAndConditionsAt: "2026-10-08T19:12:03.512Z" } }; + +// Step 2, pages/checkout/2.vue: addressToString and mapToStructuredAddress of the form, and the id of the chosen delivery method. +// Without a different delivery address, the delivery address is the invoice address. +const ERIKA = { + givenName: "Erika", + familyName: "Mustermann", + streetAddress: "Musterstraße 1", + postalCode: "70190", + addressLevel2: "Stuttgart", + country: "DE" +}; +const ERIKA_TEXT = "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart"; +const STEP_2 = { + data: { invoiceAddress: ERIKA_TEXT, deliveryAddress: ERIKA_TEXT, invoiceAddressStructured: ERIKA, deliveryAddressStructured: ERIKA, delivery: 1 } +}; + +const MAX = { + givenName: "Max", + familyName: "Mustermann", + streetAddress: "Hauptstraße 5", + postalCode: "10115", + addressLevel2: "Berlin", + country: "DE" +}; +const MAX_TEXT = "Max Mustermann\nHauptstraße 5\n10115 Berlin"; + +// Every attribute of the order that only the server writes (src/api/order/content-types/order/schema.json), the fields Strapi +// manages, and the paypalOrderId the capture writes; with values a client would send to get a paid order for less. +const SERVER_FIELDS = { + paymentAuthorised: true, + paymentStatus: "authorised", + paypalOrderId: "5O190127TN364715T", + total: 0.01, + subtotal: 0.01, + VAT: 0, + invoiceNumber: "R-1-1", + invoice: 1, + invoiceSent: false, + deliveryNote: 1, + deliveryNoteNumber: "L-1-1", + deliveryNoteSent: true, + deliveryTrackingNumber: "00340434161094042557", + emailSent: true, + cart: [{ product: 101, count: 100 }], + customer: 1, + payment: 1, + uuid: "11111111-2222-4333-8444-555555555555", + hash: "0", + date: "2026-01-01", + id: 1, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + publishedAt: "2026-01-01T00:00:00.000Z", + createdBy: 1, + updatedBy: 1 +}; + +// What the shop sends + +test("accepts the shop's step 1 payload: the email, and the terms accepted at the server's time", (t) => { + const result = parse(wire(STEP_1)); + t.diagnostic(`output: ${JSON.stringify(result)}`); + + assert.deepEqual(result, { ok: true, data: { email: "erika@example.org", acceptedTermsAndConditionsAt: "2026-10-08T19:12:05.000Z" } }); +}); + +test("accepts the shop's step 2 payload: both addresses as text and structured, and the delivery method", (t) => { + const result = parse(wire(STEP_2)); + t.diagnostic(`output: ${JSON.stringify(result)}`); + + assert.deepEqual(result, { + ok: true, + data: { + invoiceAddress: ERIKA_TEXT, + deliveryAddress: ERIKA_TEXT, + invoiceAddressStructured: ERIKA, + deliveryAddressStructured: ERIKA, + delivery: 1 + } + }); +}); + +test("accepts the shop's step 2 payload with a different delivery address", () => { + const body = { data: { ...STEP_2.data, deliveryAddress: MAX_TEXT, deliveryAddressStructured: MAX, delivery: 2 } }; + + assert.deepEqual(parse(wire(body)), { ok: true, data: body.data }); +}); + +// What only the server writes + +for (const [field, value] of Object.entries(SERVER_FIELDS)) { + test(`rejects ${field}, which only the server writes`, () => { + assert.deepEqual(parse({ data: { [field]: value } }), { + ok: false, + errors: [`data.${field}: not a field the customer may set`], + fields: [`data.${field}`] + }); + }); +} + +test("rejects the whole update, not just the server-only fields, when they come with the shop's fields", () => { + assert.deepEqual(parse({ data: { ...STEP_1.data, paymentAuthorised: true, paymentStatus: "authorised" } }), { + ok: false, + errors: ["data.paymentAuthorised: not a field the customer may set", "data.paymentStatus: not a field the customer may set"], + fields: ["data.paymentAuthorised", "data.paymentStatus"] + }); +}); + +test("names the rejected fields but never repeats their values", () => { + const result = parse({ + data: { + email: "SECRET erika@example.org", + invoiceAddress: { SECRET: true }, + invoiceAddressStructured: { ...ERIKA, givenName: "SECRET".repeat(40) }, + invoiceNumber: "R-SECRET", + delivery: "SECRET" + } + }); + + assert.equal(result.ok, false); + assert.deepEqual(result.ok ? [] : result.fields, [ + "data.email", + "data.invoiceAddress", + "data.invoiceAddressStructured.givenName", + "data.invoiceNumber", + "data.delivery" + ]); + assert.ok(!JSON.stringify(result).includes("SECRET"), JSON.stringify(result)); +}); + +// The terms + +test("records the server's time as the time the terms were accepted, whatever the client sends", () => { + for (const sent of ["2026-10-08T19:12:03.512Z", "1999-12-31T23:00:00.000Z", "2030-01-01T00:00:00.000Z", "yes", true, 1]) { + assert.deepEqual(parse({ data: { acceptedTermsAndConditionsAt: sent } }), { + ok: true, + data: { acceptedTermsAndConditionsAt: "2026-10-08T19:12:05.000Z" } + }); + } + + assert.deepEqual(parseCustomerUpdate({ data: { acceptedTermsAndConditionsAt: true } }, new Date("2026-10-09T06:30:00.000Z")), { + ok: true, + data: { acceptedTermsAndConditionsAt: "2026-10-09T06:30:00.000Z" } + }); +}); + +test("rejects a terms acceptance without a true value", () => { + for (const sent of [false, null, "", 0]) { + assert.deepEqual(parse({ data: { acceptedTermsAndConditionsAt: sent } }), { + ok: false, + errors: ["data.acceptedTermsAndConditionsAt: must be sent only to accept the terms, as true or a timestamp"], + fields: ["data.acceptedTermsAndConditionsAt"] + }); + } +}); + +// The email + +test("accepts an email address trimmed, up to 254 characters", () => { + const longest = `${"e".repeat(242)}@example.org`; + assert.equal(longest.length, 254); + + assert.deepEqual(parse({ data: { email: " erika@example.org\n" } }), { ok: true, data: { email: "erika@example.org" } }); + assert.deepEqual(parse({ data: { email: longest } }), { ok: true, data: { email: longest } }); +}); + +test("rejects an email that is not an address of at most 254 characters", () => { + const emails = [ + "", + " ", + "erika", + "erika.example.org", + "@example.org", + "erika@", + `${"e".repeat(243)}@example.org`, + "erika mustermann@example.org", + "erika@example.org\r\nBcc: everyone@example.org", + "erika@exa\u0000mple.org", + null, + 42, + true, + ["erika@example.org"], + { address: "erika@example.org" } + ]; + for (const email of emails) { + assert.deepEqual( + parse({ data: { email } }), + { ok: false, errors: ["data.email: must be an email address of at most 254 characters"], fields: ["data.email"] }, + JSON.stringify(email) + ); + } +}); + +// The addresses + +test("accepts an invoice or delivery address of up to 500 characters", () => { + const longest = `${ERIKA_TEXT}\n${"c/o ".repeat(113)}`; + assert.equal(longest.length, 500); + + assert.deepEqual(parse({ data: { invoiceAddress: longest, deliveryAddress: longest } }), { + ok: true, + data: { invoiceAddress: longest, deliveryAddress: longest } + }); +}); + +test("rejects an invoice or delivery address that is not a string of at most 500 characters", () => { + for (const field of ["invoiceAddress", "deliveryAddress"]) { + for (const value of ["a".repeat(501), null, 42, true, [ERIKA_TEXT], { street: "Musterstraße 1" }]) { + assert.deepEqual(parse({ data: { [field]: value } }), { + ok: false, + errors: [`data.${field}: must be a string of at most 500 characters`], + fields: [`data.${field}`] + }); + } + } +}); + +test("accepts a structured address with only some components, null for a component without a value, and 200 characters", () => { + const partial = { givenName: "Erika", familyName: null, postalCode: "70190", country: "DE" }; + const longest = { ...ERIKA, streetAddress: "s".repeat(200) }; + + assert.deepEqual(parse({ data: { invoiceAddressStructured: partial } }), { ok: true, data: { invoiceAddressStructured: partial } }); + assert.deepEqual(parse({ data: { deliveryAddressStructured: longest } }), { ok: true, data: { deliveryAddressStructured: longest } }); +}); + +test("rejects a structured address with keys that are not its components, and names them", () => { + assert.deepEqual(parse({ data: { invoiceAddressStructured: { ...ERIKA, id: 7, street: "Musterstraße 1" } } }), { + ok: false, + errors: [ + "data.invoiceAddressStructured.id: not a structured address field", + "data.invoiceAddressStructured.street: not a structured address field" + ], + fields: ["data.invoiceAddressStructured.id", "data.invoiceAddressStructured.street"] + }); +}); + +test("rejects a structured address that is not an object", () => { + for (const field of ["invoiceAddressStructured", "deliveryAddressStructured"]) { + for (const value of [null, ERIKA_TEXT, 42, true, [ERIKA]]) { + assert.deepEqual(parse({ data: { [field]: value } }), { + ok: false, + errors: [`data.${field}: must be an object`], + fields: [`data.${field}`] + }); + } + } +}); + +test("rejects a structured address component that is not a string of at most 200 characters, or null", () => { + for (const value of ["s".repeat(201), 42, true, ["Erika"], { name: "Erika" }]) { + assert.deepEqual(parse({ data: { deliveryAddressStructured: { ...ERIKA, givenName: value } } }), { + ok: false, + errors: ["data.deliveryAddressStructured.givenName: must be a string of at most 200 characters, or null"], + fields: ["data.deliveryAddressStructured.givenName"] + }); + } +}); + +// The delivery method + +test("accepts the id of a delivery method", () => { + for (const delivery of [1, 2, 2147483647]) { + assert.deepEqual(parse({ data: { delivery } }), { ok: true, data: { delivery } }); + } +}); + +test("rejects a delivery that is not a positive integer", () => { + // NaN and Infinity cannot come as JSON, but the parser does not rely on that. + for (const delivery of [0, -1, 1.5, "1", null, true, [1], { id: 1 }, 2147483648, Number.NaN, Number.POSITIVE_INFINITY]) { + assert.deepEqual(parse({ data: { delivery } }), { + ok: false, + errors: ["data.delivery: must be the id of a delivery method, a positive integer"], + fields: ["data.delivery"] + }); + } +}); + +// The body + +test("rejects a body that is not an object", () => { + for (const body of [undefined, null, "", "data", 42, true, [], [STEP_1]]) { + assert.deepEqual(parse(body), { ok: false, errors: ["body: must be an object of the form { data: { … } }"], fields: ["body"] }); + } +}); + +test("rejects a body without data, and names the fields sent outside it", () => { + assert.deepEqual(parse({}), { ok: false, errors: ["data: missing"], fields: ["data"] }); + assert.deepEqual(parse(STEP_1.data), { + ok: false, + errors: [ + "email: not accepted, the fields belong in data", + "acceptedTermsAndConditionsAt: not accepted, the fields belong in data", + "data: missing" + ], + fields: ["email", "acceptedTermsAndConditionsAt", "data"] + }); +}); + +test("rejects data that is not an object", () => { + for (const data of [null, "email=erika@example.org", 42, true, [], [STEP_1.data]]) { + assert.deepEqual(parse({ data }), { ok: false, errors: ["data: must be an object"], fields: ["data"] }); + } +}); + +test("rejects keys next to data", () => { + assert.deepEqual(rejectedFields({ ...STEP_1, meta: {}, paymentAuthorised: true }), ["meta", "paymentAuthorised"]); +}); + +test("rejects __proto__ and constructor like any other key", () => { + assert.deepEqual(rejectedFields(JSON.parse('{"__proto__": {"paymentAuthorised": true}, "data": {}}')), ["__proto__"]); + assert.deepEqual(rejectedFields(JSON.parse('{"data": {"__proto__": {"paymentAuthorised": true}, "constructor": {}}}')), [ + "data.__proto__", + "data.constructor" + ]); + assert.deepEqual(rejectedFields(JSON.parse('{"data": {"invoiceAddressStructured": {"__proto__": {"givenName": "Erika"}}}}')), [ + "data.invoiceAddressStructured.__proto__" + ]); +}); + +test("cuts a rejected key to 64 characters", () => { + assert.deepEqual(rejectedFields({ data: { ["k".repeat(10000)]: 1 } }), [`data.${"k".repeat(64)}…`]); +}); + +test("accepts an update without fields", () => { + assert.deepEqual(parse({ data: {} }), { ok: true, data: {} }); +}); -- 2.36.6 From 88f4ee75d73fd7229da4a31d0b8755c5fa00c36f Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Thu, 8 Oct 2026 22:54:59 +0200 Subject: [PATCH 02/16] feat(order): lock an order for the customer once its payment is authorised Refs https://git.librete.ch/libretech/mp/issues/71 --- src/checkout/order-state.ts | 10 ++++++++++ tests/unit/order-state.test.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 src/checkout/order-state.ts create mode 100644 tests/unit/order-state.test.ts diff --git a/src/checkout/order-state.ts b/src/checkout/order-state.ts new file mode 100644 index 0000000..7c66128 --- /dev/null +++ b/src/checkout/order-state.ts @@ -0,0 +1,10 @@ +/** The fields of an order that decide whether the customer may still change it. */ +export type OrderLockState = { + paymentAuthorised?: boolean | null; +}; + +/** + * Once its payment is authorised, the order is what the customer paid for and what the invoice is generated from, + * so the customer can no longer change its contact details, addresses, delivery method or cart. + */ +export const isLockedForCustomer = (order: OrderLockState): boolean => Boolean(order.paymentAuthorised); diff --git a/tests/unit/order-state.test.ts b/tests/unit/order-state.test.ts new file mode 100644 index 0000000..3274d24 --- /dev/null +++ b/tests/unit/order-state.test.ts @@ -0,0 +1,26 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { isLockedForCustomer } from "../../src/checkout/order-state.ts"; + +// Shaped like the row PUT /orders/:uuid/cart reads with strapi.db.query("api::order.order").findOne before it updates. +const order = (overrides = {}) => ({ + id: 4711, + uuid: "11111111-2222-4333-8444-555555555555", + email: "erika@example.org", + paymentAuthorised: false, + paymentStatus: null, + invoiceSent: false, + ...overrides +}); + +test("an order is open for the customer until its payment is authorised", () => { + assert.equal(isLockedForCustomer(order()), false); + // false is the schema's default, but a row can hold null + assert.equal(isLockedForCustomer(order({ paymentAuthorised: null })), false); + assert.equal(isLockedForCustomer({}), false); +}); + +test("an order is locked once its payment is authorised", () => { + // what capturePayment writes once PayPal reports the capture COMPLETED + assert.equal(isLockedForCustomer(order({ paymentAuthorised: true, paymentStatus: "authorised" })), true); +}); -- 2.36.6 From 2992e4805e761081752ff8fadbdebb007710b4f1 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Thu, 8 Oct 2026 22:55:13 +0200 Subject: [PATCH 03/16] fix(order): answer 404 instead of 500 for a missing order findOne and findOneByUuid mapped order.cart before checking that the order exists, and the service's findOneByUuid did the same, so a missing order threw a TypeError: GET /orders/:uuid/cart, add-product, remove-product, generate-invoice and generate-delivery-note answered 500. The service's findOneByUuid now returns null for a missing order, and each caller handles it: the controllers answer 404, sendInvoice and sendDeliveryNote throw a NotFoundError. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/api/order/controllers/order.ts | 6 ++++-- src/api/order/services/order.ts | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/api/order/controllers/order.ts b/src/api/order/controllers/order.ts index 52be99d..d0d2830 100644 --- a/src/api/order/controllers/order.ts +++ b/src/api/order/controllers/order.ts @@ -68,6 +68,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = where: { id }, ...orderDefaultParams }); + if (!order) return ctx.notFound("Order not found"); order.cart = order?.cart.map((item) => ({ ...item, product: { @@ -75,7 +76,6 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = totalProductPrice: calculateTotalProductPrice(item.product) } })); - if (!order) return ctx.notFound("Order not found"); return await sanitize.contentAPI.output(order, strapi.getModel("api::order.order")); }, @@ -85,6 +85,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = where: { uuid }, ...orderDefaultParams }); + if (!order) return ctx.notFound("Order not found"); order.cart = order?.cart.map((item) => ({ ...item, product: { @@ -92,7 +93,6 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = totalProductPrice: calculateTotalProductPrice(item.product) } })); - if (!order) return ctx.notFound("Order not found"); return await sanitize.contentAPI.output(order, strapi.getModel("api::order.order")); }, @@ -245,6 +245,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = const { uuid } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); strapi.log.info(`app:i:order-controller: Started invoice generation`); const orderUnsafe = (await strapi.service("api::order.order").findOneByUuid(uuid)) as Order; + if (!orderUnsafe) return ctx.notFound("Order not found"); if (!orderUnsafe.paymentAuthorised) { return ctx.locked("Payment not authorised, cannot generate invoice"); @@ -263,6 +264,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = const { uuid } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); strapi.log.info(`app:i:order-controller: Started delivery note generation`); const orderUnsafe = (await strapi.service("api::order.order").findOneByUuid(uuid)) as Order; + if (!orderUnsafe) return ctx.notFound("Order not found"); if (!orderUnsafe.paymentAuthorised) { return ctx.locked("Payment not authorised, cannot generate delivery note"); diff --git a/src/api/order/services/order.ts b/src/api/order/services/order.ts index dd846be..a73f856 100644 --- a/src/api/order/services/order.ts +++ b/src/api/order/services/order.ts @@ -1,7 +1,7 @@ import path from "path"; import fs from "fs"; import { factories } from "@strapi/strapi"; -import { sanitize } from "@strapi/utils"; +import { errors, sanitize } from "@strapi/utils"; import { ID } from "@strapi/database/dist/types"; import { vatDecimal, vatIncludedDecimal, baseUrl, adminEmail, shopName, shopContactName, shopAddress, shopPhone, shopEmail, shopLogoUrl, shopSecondaryLogoUrl } from "../../../../config/constants"; import { calculateTotalProductPrice, productDefaultParams } from "../../product/services/product"; @@ -32,6 +32,7 @@ export default factories.createCoreService("api::order.order", ({ strapi }) => ( where: { uuid }, ...orderDefaultParams }); + if (!orderUnsafe) return null; orderUnsafe.cart = orderUnsafe?.cart.map((item: CartProduct) => ({ ...item, product: { @@ -80,6 +81,7 @@ export default factories.createCoreService("api::order.order", ({ strapi }) => ( sendInvoice: async (uuid: string, blob?: any) => { strapi.log.info(`app:i:order-service: - Sending invoice for order ${uuid}`); const oderUnsafe = (await strapi.service("api::order.order").findOneByUuid(uuid)) as Partial; + if (!oderUnsafe) throw new errors.NotFoundError("Order not found"); const shopContext = { shopName, shopContactName, shopAddress, shopPhone, shopEmail, shopLogoUrl, shopSecondaryLogoUrl }; try { @@ -119,6 +121,7 @@ export default factories.createCoreService("api::order.order", ({ strapi }) => ( sendDeliveryNote: async (uuid: string, blob?: ArrayBuffer) => { strapi.log.info(`app:i:order-service: - Sending delivery note for order ${uuid}`); const oderUnsafe = (await strapi.service("api::order.order").findOneByUuid(uuid)) as Partial; + if (!oderUnsafe) throw new errors.NotFoundError("Order not found"); const shopContext = { shopName, shopContactName, shopAddress, shopPhone, shopEmail, shopLogoUrl, shopSecondaryLogoUrl }; try { const emailConfig: MessageBody = { -- 2.36.6 From 20afcd4416f780e8402c4ea9cd305453dc173a8f Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Thu, 8 Oct 2026 22:58:21 +0200 Subject: [PATCH 04/16] fix(order): write only customer fields on an order update PUT /orders/:uuid/cart passed the body through sanitize.contentAPI.input, which keeps every public writable attribute, to the order update. The shop forwards the browser's body unchanged, so a browser could set paymentAuthorised on its own order, and the order lifecycle then generated and mailed the invoice. The handler now parses the body with parseCustomerUpdate and writes only the fields it returns, plus the customer it finds or creates from the invoice address. A body with any other field is answered 400 with the error list, and the warning it logs names the fields, not their values. A delivery method that does not exist is answered 400. Once the payment is authorised, the order is locked for the customer: the update, add-product and remove-product answer 409. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/api/order/controllers/order.ts | 47 +++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/api/order/controllers/order.ts b/src/api/order/controllers/order.ts index d0d2830..1cc7cec 100644 --- a/src/api/order/controllers/order.ts +++ b/src/api/order/controllers/order.ts @@ -1,9 +1,15 @@ import { factories } from "@strapi/strapi"; import { sanitize } from "@strapi/utils"; +import type { ID } from "@strapi/database/dist/types"; import { Order } from "../../../../types"; import { paypalApi } from "../../../services/PayPalApi"; import { orderDefaultParams } from "../services/order"; import { calculateTotalProductPrice } from "../../product/services/product"; +import { parseCustomerUpdate } from "../../../checkout/customer-update"; +import type { CustomerUpdate } from "../../../checkout/customer-update"; +import { isLockedForCustomer } from "../../../checkout/order-state"; + +const ORDER_LOCKED = "Order can no longer be changed"; /** * The order controller is a funnel for the client to interact with the order service. @@ -96,31 +102,42 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = return await sanitize.contentAPI.output(order, strapi.getModel("api::order.order")); }, + /** + * The customer's checkout steps: contact details, terms, addresses and delivery method. Only these fields are written; + * payment state, totals, numbers and documents are written by the payment flow and the server alone. + */ updateByUuid: async (ctx) => { const { uuid } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); const order = await strapi.db.query("api::order.order").findOne({ where: { uuid } }); if (!order) return ctx.notFound("Order not found"); + if (isLockedForCustomer(order)) return ctx.conflict(ORDER_LOCKED); - const sanitizedBody = await sanitize.contentAPI.input(ctx.request.body, strapi.getModel("api::order.order")); + const update = parseCustomerUpdate(ctx.request.body, new Date()); + if (update.ok === false) { + strapi.log.warn("app:w:order-controller: Rejected order update", { order: order.id, fields: update.fields }); + return ctx.badRequest("Invalid order update", { errors: update.errors }); + } + const data: CustomerUpdate & { customer?: ID } = { ...update.data }; + + if (data.delivery !== undefined) { + const delivery = await strapi.db.query("api::delivery.delivery").findOne({ where: { id: data.delivery }, select: ["id"] }); + if (!delivery) { + strapi.log.warn("app:w:order-controller: Rejected order update", { order: order.id, fields: ["data.delivery"] }); + return ctx.badRequest("Invalid order update", { errors: ["data.delivery: no delivery method has this id"] }); + } + } // Handle customer creation/update if structured addresses are provided - // @ts-expect-error - if (sanitizedBody?.data?.invoiceAddressStructured) { + if (data.invoiceAddressStructured) { + const address = data.invoiceAddressStructured; const customerAddress = - // @ts-expect-error - sanitizedBody.data.invoiceAddress || - // @ts-expect-error - `${sanitizedBody.data.invoiceAddressStructured.givenName} ${sanitizedBody.data.invoiceAddressStructured.familyName}\n${sanitizedBody.data.invoiceAddressStructured.streetAddress}\n${sanitizedBody.data.invoiceAddressStructured.postalCode} ${sanitizedBody.data.invoiceAddressStructured.addressLevel2}`; - // @ts-expect-error - sanitizedBody.data.customer = await strapi - .service("api::customer.customer") - // @ts-expect-error - .findOrCreateCustomer(customerAddress, sanitizedBody.data.invoiceAddressStructured); + data.invoiceAddress || + `${address.givenName} ${address.familyName}\n${address.streetAddress}\n${address.postalCode} ${address.addressLevel2}`; + data.customer = await strapi.service("api::customer.customer").findOrCreateCustomer(customerAddress, address); } const orderUnsafe = await strapi.service("api::order.order").update(order.id, { - // @ts-expect-error - data: sanitizedBody?.data, + data, ...orderDefaultParams }); @@ -131,6 +148,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = const { uuid, productId } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); const order = (await strapi.service("api::order.order").findOneByUuid(uuid)) as Order; if (!order) return ctx.notFound("Order not found"); + if (isLockedForCustomer(order)) return ctx.conflict(ORDER_LOCKED); const product = parseInt(productId as string); const count = parseInt(ctx.query.count as string) || 1; @@ -162,6 +180,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = const product = parseInt(productId as string); if (!order) return ctx.notFound("Order not found"); + if (isLockedForCustomer(order)) return ctx.conflict(ORDER_LOCKED); strapi.log.info("app:v:order-controller: - Removing product from cart", { totalBefore: order.total }); const updatedCart = [...(order.cart || [])]; -- 2.36.6 From 2bbffeed2dc5f30e64a636ea3efaec1d88062792 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Thu, 8 Oct 2026 23:22:36 +0200 Subject: [PATCH 05/16] feat(order): add private attributes for the PayPal order and its capture paypalOrderId is the PayPal order the checkout created for the order, paypalCaptureId and paymentCapturedAt record the verified capture. All three are private, so the content API never returns them. Strapi adds the columns when it syncs the schema at startup. types/generated/contentTypes.d.ts is the output of strapi ts:generate-types, run in the release image without network. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/api/order/content-types/order/schema.json | 12 ++++++++++++ types/generated/contentTypes.d.ts | 3 +++ 2 files changed, 15 insertions(+) diff --git a/src/api/order/content-types/order/schema.json b/src/api/order/content-types/order/schema.json index 5f783eb..a58fa3f 100644 --- a/src/api/order/content-types/order/schema.json +++ b/src/api/order/content-types/order/schema.json @@ -83,6 +83,18 @@ "paymentStatus": { "type": "string" }, + "paypalOrderId": { + "type": "string", + "private": true + }, + "paypalCaptureId": { + "type": "string", + "private": true + }, + "paymentCapturedAt": { + "type": "datetime", + "private": true + }, "emailSent": { "type": "boolean", "default": false diff --git a/types/generated/contentTypes.d.ts b/types/generated/contentTypes.d.ts index fbd56db..5f83aed 100644 --- a/types/generated/contentTypes.d.ts +++ b/types/generated/contentTypes.d.ts @@ -389,7 +389,10 @@ export interface ApiOrderOrder extends Schema.CollectionType { invoiceSent: Attribute.Boolean & Attribute.DefaultTo; payment: Attribute.Relation<"api::order.order", "oneToOne", "api::payment.payment">; paymentAuthorised: Attribute.Boolean & Attribute.DefaultTo; + paymentCapturedAt: Attribute.DateTime & Attribute.Private; paymentStatus: Attribute.String; + paypalCaptureId: Attribute.String & Attribute.Private; + paypalOrderId: Attribute.String & Attribute.Private; subtotal: Attribute.Decimal; total: Attribute.Decimal; updatedAt: Attribute.DateTime; -- 2.36.6 From 4d77998b514c48f16682dd346d53d4bebd0c4301 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:12:20 +0200 Subject: [PATCH 06/16] feat(checkout): verify the approved PayPal order and its capture against the order verifyApprovedOrder accepts PayPal's order only if it is APPROVED, its first purchase unit's custom_id is the order's uuid, and its amount is the order total in EUR. verifyCompletedCapture accepts the order after the capture only if it and its first capture are COMPLETED for the order total in EUR, and returns the capture id. Both read PayPal's Orders v2 JSON in snake_case, as PayPalApi parses the response body. Money is compared in integer cents: PayPal's "12.34" is parsed strictly, the order total is Math.round(total * 100). For every total up to 3000.00 the amount the checkout sends (total.toFixed(2)) parses to the cents the capture compares. A rejection reason repeats only PayPal's statuses, currency codes and amounts, never the payer's or the shipping data of the response. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/checkout/paypal-verify.ts | 104 +++++++ .../unit/fixtures/paypal-order-approved.json | 88 ++++++ .../unit/fixtures/paypal-order-captured.json | 131 +++++++++ tests/unit/paypal-verify.test.ts | 254 ++++++++++++++++++ 4 files changed, 577 insertions(+) create mode 100644 src/checkout/paypal-verify.ts create mode 100644 tests/unit/fixtures/paypal-order-approved.json create mode 100644 tests/unit/fixtures/paypal-order-captured.json create mode 100644 tests/unit/paypal-verify.test.ts diff --git a/src/checkout/paypal-verify.ts b/src/checkout/paypal-verify.ts new file mode 100644 index 0000000..f0eefae --- /dev/null +++ b/src/checkout/paypal-verify.ts @@ -0,0 +1,104 @@ +// Checks PayPal's answers against the shop order before the order is marked paid: the PayPal order the buyer approved +// (GET /v2/checkout/orders/:id) and the order after its capture (POST /v2/checkout/orders/:id/capture, Prefer: return=representation). +// The JSON is PayPal's Orders v2 JSON in snake_case, as PayPalApi parses the raw response body. Money is compared in integer cents. +// A reason says what does not match. It repeats only PayPal's statuses, currency codes and amounts, never a name, an email address +// or an address from the response, because it is logged. + +/** The fields of the shop order the checks compare. */ +export type VerifiableOrder = { + uuid: string; + total: number | null; +}; + +export type ApprovedOrderCheck = { ok: true } | { ok: false; reason: string }; + +export type CompletedCaptureCheck = { ok: true; captureId: string } | { ok: false; reason: string }; + +const CURRENCY = "EUR"; + +type Json = Record; + +const isObject = (value: unknown): value is Json => typeof value === "object" && value !== null && !Array.isArray(value); + +/** PayPal's money value as integer cents: "12.34" is 1234. Null for anything but a non-negative amount with at most two decimals. */ +export const parseCents = (value: unknown): number | null => { + if (typeof value !== "string") return null; + const match = /^(\d{1,10})(?:\.(\d{1,2}))?$/.exec(value); + if (match === null) return null; + return Number(match[1]) * 100 + Number((match[2] ?? "").padEnd(2, "0")); +}; + +/** The order total in integer cents, or null if the order has no positive total. Strapi returns the decimal column as a number. */ +export const orderTotalCents = (order: { total: number | null }): number | null => + typeof order.total === "number" && Number.isFinite(order.total) && order.total > 0 ? Math.round(order.total * 100) : null; + +const formatCents = (cents: number): string => `${Math.floor(cents / 100)}.${String(cents % 100).padStart(2, "0")}`; + +// The values of OrderStatus and CaptureStatus in @paypal/paypal-server-sdk 0.6.1. A reason repeats these and ISO currency codes, +// and no other value from the response. +const ORDER_STATUSES = ["CREATED", "SAVED", "APPROVED", "VOIDED", "COMPLETED", "PAYER_ACTION_REQUIRED"]; +const CAPTURE_STATUSES = ["COMPLETED", "DECLINED", "PARTIALLY_REFUNDED", "PENDING", "REFUNDED", "FAILED"]; + +const shown = (value: unknown, isKnown: (value: string) => boolean): string => + typeof value === "string" && isKnown(value) ? value : "missing or unexpected"; +const orderStatus = (value: unknown): string => shown(value, (status) => ORDER_STATUSES.includes(status)); +const captureStatus = (value: unknown): string => shown(value, (status) => CAPTURE_STATUSES.includes(status)); +const currency = (value: unknown): string => shown(value, (code) => /^[A-Z]{3}$/.test(code)); + +const firstPurchaseUnit = (paypalOrder: Json): Json | null => { + const units = paypalOrder.purchase_units; + return Array.isArray(units) && isObject(units[0]) ? units[0] : null; +}; + +/** Why PayPal's money ({ currency_code, value }) is not the order total in EUR, or null if it is. */ +const amountMismatch = (subject: string, money: unknown, order: VerifiableOrder): string | null => { + if (!isObject(money)) return `${subject} has no amount`; + if (money.currency_code !== CURRENCY) return `${subject} currency is ${currency(money.currency_code)}, not ${CURRENCY}`; + const cents = parseCents(money.value); + if (cents === null) return `${subject} amount is not a money value`; + const total = orderTotalCents(order); + if (total === null) return "the order has no total"; + if (cents !== total) return `${subject} amount ${formatCents(cents)} is not the order total ${formatCents(total)}`; + return null; +}; + +/** + * The PayPal order may be captured for this order: the buyer approved it, it was created for this order (its custom_id is the + * order's uuid, see buildPayPalOrderRequest), and its amount is the order total in EUR. + */ +export const verifyApprovedOrder = (order: VerifiableOrder, paypalOrder: unknown): ApprovedOrderCheck => { + if (!isObject(paypalOrder)) return { ok: false, reason: "the PayPal order is not an object" }; + if (paypalOrder.status !== "APPROVED") { + return { ok: false, reason: `the PayPal order status is ${orderStatus(paypalOrder.status)}, not APPROVED` }; + } + + const unit = firstPurchaseUnit(paypalOrder); + if (unit === null) return { ok: false, reason: "the PayPal order has no purchase unit" }; + if (typeof order.uuid !== "string" || order.uuid === "" || unit.custom_id !== order.uuid) { + return { ok: false, reason: "the custom_id of the PayPal order is not the order's uuid" }; + } + + const mismatch = amountMismatch("the PayPal order", unit.amount, order); + return mismatch === null ? { ok: true } : { ok: false, reason: mismatch }; +}; + +/** + * PayPal captured the order total in EUR: the PayPal order and its first capture are COMPLETED. Returns the capture's id. + * A capture PayPal holds for review (PENDING) or declined (DECLINED) is not a payment. + */ +export const verifyCompletedCapture = (order: VerifiableOrder, captured: unknown): CompletedCaptureCheck => { + if (!isObject(captured)) return { ok: false, reason: "the capture response is not an object" }; + if (captured.status !== "COMPLETED") return { ok: false, reason: `the PayPal order status is ${orderStatus(captured.status)}, not COMPLETED` }; + + const unit = firstPurchaseUnit(captured); + const payments = unit !== null && isObject(unit.payments) ? unit.payments : null; + const capture = payments !== null && Array.isArray(payments.captures) && isObject(payments.captures[0]) ? payments.captures[0] : null; + if (capture === null) return { ok: false, reason: "the capture response has no capture" }; + if (capture.status !== "COMPLETED") return { ok: false, reason: `the capture status is ${captureStatus(capture.status)}, not COMPLETED` }; + + const mismatch = amountMismatch("the capture", capture.amount, order); + if (mismatch !== null) return { ok: false, reason: mismatch }; + if (typeof capture.id !== "string" || !/^[A-Za-z0-9-]{1,64}$/.test(capture.id)) return { ok: false, reason: "the capture has no valid id" }; + + return { ok: true, captureId: capture.id }; +}; diff --git a/tests/unit/fixtures/paypal-order-approved.json b/tests/unit/fixtures/paypal-order-approved.json new file mode 100644 index 0000000..5715338 --- /dev/null +++ b/tests/unit/fixtures/paypal-order-approved.json @@ -0,0 +1,88 @@ +{ + "id": "5O190127TN364715T", + "intent": "CAPTURE", + "status": "APPROVED", + "payment_source": { + "paypal": { + "email_address": "erika.buyer@example.org", + "account_id": "QYR5Z8XDVJNXQ", + "account_status": "VERIFIED", + "name": { + "given_name": "Erika", + "surname": "Mustermann" + }, + "address": { + "country_code": "DE" + } + } + }, + "purchase_units": [ + { + "reference_id": "11111111-2222-4333-8444-555555555555", + "amount": { + "currency_code": "EUR", + "value": "22.85", + "breakdown": { + "item_total": { + "currency_code": "EUR", + "value": "15.04" + }, + "shipping": { + "currency_code": "EUR", + "value": "4.95" + }, + "tax_total": { + "currency_code": "EUR", + "value": "2.86" + } + } + }, + "payee": { + "email_address": "merchant@example.com", + "merchant_id": "7KNGBPH2U58GQ" + }, + "custom_id": "11111111-2222-4333-8444-555555555555", + "invoice_id": "R-12-4711", + "shipping": { + "name": { + "full_name": "Erika Mustermann" + }, + "address": { + "address_line_1": "Musterstraße 1", + "admin_area_2": "Stuttgart", + "postal_code": "70190", + "country_code": "DE" + } + } + } + ], + "payer": { + "name": { + "given_name": "Erika", + "surname": "Mustermann" + }, + "email_address": "erika.buyer@example.org", + "payer_id": "QYR5Z8XDVJNXQ", + "address": { + "country_code": "DE" + } + }, + "create_time": "2026-10-08T19:12:40Z", + "links": [ + { + "href": "https://api.sandbox.paypal.com/v2/checkout/orders/5O190127TN364715T", + "rel": "self", + "method": "GET" + }, + { + "href": "https://api.sandbox.paypal.com/v2/checkout/orders/5O190127TN364715T", + "rel": "update", + "method": "PATCH" + }, + { + "href": "https://api.sandbox.paypal.com/v2/checkout/orders/5O190127TN364715T/capture", + "rel": "capture", + "method": "POST" + } + ] +} diff --git a/tests/unit/fixtures/paypal-order-captured.json b/tests/unit/fixtures/paypal-order-captured.json new file mode 100644 index 0000000..df157da --- /dev/null +++ b/tests/unit/fixtures/paypal-order-captured.json @@ -0,0 +1,131 @@ +{ + "id": "5O190127TN364715T", + "intent": "CAPTURE", + "status": "COMPLETED", + "payment_source": { + "paypal": { + "email_address": "erika.buyer@example.org", + "account_id": "QYR5Z8XDVJNXQ", + "account_status": "VERIFIED", + "name": { + "given_name": "Erika", + "surname": "Mustermann" + }, + "address": { + "country_code": "DE" + } + } + }, + "purchase_units": [ + { + "reference_id": "11111111-2222-4333-8444-555555555555", + "amount": { + "currency_code": "EUR", + "value": "22.85", + "breakdown": { + "item_total": { + "currency_code": "EUR", + "value": "15.04" + }, + "shipping": { + "currency_code": "EUR", + "value": "4.95" + }, + "tax_total": { + "currency_code": "EUR", + "value": "2.86" + } + } + }, + "payee": { + "email_address": "merchant@example.com", + "merchant_id": "7KNGBPH2U58GQ" + }, + "custom_id": "11111111-2222-4333-8444-555555555555", + "invoice_id": "R-12-4711", + "shipping": { + "name": { + "full_name": "Erika Mustermann" + }, + "address": { + "address_line_1": "Musterstraße 1", + "admin_area_2": "Stuttgart", + "postal_code": "70190", + "country_code": "DE" + } + }, + "payments": { + "captures": [ + { + "id": "3C679366HH908993F", + "status": "COMPLETED", + "amount": { + "currency_code": "EUR", + "value": "22.85" + }, + "final_capture": true, + "seller_protection": { + "status": "ELIGIBLE", + "dispute_categories": ["ITEM_NOT_RECEIVED", "UNAUTHORIZED_TRANSACTION"] + }, + "seller_receivable_breakdown": { + "gross_amount": { + "currency_code": "EUR", + "value": "22.85" + }, + "paypal_fee": { + "currency_code": "EUR", + "value": "0.92" + }, + "net_amount": { + "currency_code": "EUR", + "value": "21.93" + } + }, + "custom_id": "11111111-2222-4333-8444-555555555555", + "invoice_id": "R-12-4711", + "links": [ + { + "href": "https://api.sandbox.paypal.com/v2/payments/captures/3C679366HH908993F", + "rel": "self", + "method": "GET" + }, + { + "href": "https://api.sandbox.paypal.com/v2/payments/captures/3C679366HH908993F/refund", + "rel": "refund", + "method": "POST" + }, + { + "href": "https://api.sandbox.paypal.com/v2/checkout/orders/5O190127TN364715T", + "rel": "up", + "method": "GET" + } + ], + "create_time": "2026-10-08T19:13:02Z", + "update_time": "2026-10-08T19:13:02Z" + } + ] + } + } + ], + "payer": { + "name": { + "given_name": "Erika", + "surname": "Mustermann" + }, + "email_address": "erika.buyer@example.org", + "payer_id": "QYR5Z8XDVJNXQ", + "address": { + "country_code": "DE" + } + }, + "create_time": "2026-10-08T19:12:40Z", + "update_time": "2026-10-08T19:13:02Z", + "links": [ + { + "href": "https://api.sandbox.paypal.com/v2/checkout/orders/5O190127TN364715T", + "rel": "self", + "method": "GET" + } + ] +} diff --git a/tests/unit/paypal-verify.test.ts b/tests/unit/paypal-verify.test.ts new file mode 100644 index 0000000..10222aa --- /dev/null +++ b/tests/unit/paypal-verify.test.ts @@ -0,0 +1,254 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { orderTotalCents, parseCents, verifyApprovedOrder, verifyCompletedCapture } from "../../src/checkout/paypal-verify.ts"; +import { buildPayPalOrderRequest } from "../../src/checkout/paypal-order.ts"; + +const UUID = "11111111-2222-4333-8444-555555555555"; + +// PayPal's Orders v2 JSON as PayPalApi parses the response body: the PayPal order once the buyer approved it +// (GET /v2/checkout/orders/:id), and the PayPal order after its capture (POST /v2/checkout/orders/:id/capture with +// Prefer: return=representation). Both belong to the order below. Every call returns a fresh copy to change. +const fixture = (name: string) => JSON.parse(readFileSync(new URL(`./fixtures/${name}`, import.meta.url), "utf8")); +const approved = () => fixture("paypal-order-approved.json"); +const captured = () => fixture("paypal-order-captured.json"); + +// The order of paypal-order.test.ts: subtotal 17.90, delivery 4.95, total 22.85. +const order = (overrides = {}) => ({ + id: 4711, + uuid: UUID, + email: "erika@example.org", + subtotal: 17.9, + total: 22.85, + VAT: 2.86, + delivery: { id: 1, name: "DHL", price: 4.95 }, + invoiceNumber: "R-12-4711", + deliveryAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", + ...overrides +}); + +// What identifies the buyer or the merchant in the fixtures. A reason is logged, so it repeats none of it. +const PERSONAL_DATA = [ + "Erika", + "Mustermann", + "erika", + "QYR5Z8XDVJNXQ", + "Musterstraße", + "70190", + "Stuttgart", + "merchant@example.com", + "7KNGBPH2U58GQ" +]; + +/** The reason of a rejection, after checking that it repeats no personal data. */ +const rejection = (result: { ok: boolean; reason?: string }): string => { + assert.equal(result.ok, false, "expected a rejection"); + for (const value of PERSONAL_DATA) assert.ok(!result.reason.includes(value), `the reason repeats ${value}: ${result.reason}`); + return result.reason; +}; + +// Money + +test("parses PayPal's money values into integer cents", () => { + assert.equal(parseCents("22.85"), 2285); + assert.equal(parseCents("0.01"), 1); + assert.equal(parseCents("4.9"), 490); + assert.equal(parseCents("100"), 10000); + assert.equal(parseCents("0.00"), 0); + assert.equal(parseCents("1234567.89"), 123456789); +}); + +test("parses nothing but a non-negative amount with at most two decimals", () => { + for (const value of ["", "22.850", "-22.85", "22,85", " 22.85", "22.85 ", "2.285e1", "22.", ".85", "0x10", "Infinity", "NaN", "12345678901.00"]) { + assert.equal(parseCents(value), null, JSON.stringify(value)); + } + for (const value of [22.85, null, undefined, {}, ["22.85"]]) { + assert.equal(parseCents(value), null, String(value)); + } +}); + +test("turns the order total into integer cents, also where its float is a hair below the cent", () => { + // 22.85 * 100 is 2284.9999999999995 and 0.29 * 100 is 28.999999999999996 in floating point + assert.equal(orderTotalCents({ total: 22.85 }), 2285); + assert.equal(orderTotalCents({ total: 0.29 }), 29); + assert.equal(orderTotalCents({ total: 17.9 + 4.95 }), 2285); + for (const total of [null, undefined, 0, -4.95, NaN, Infinity]) { + assert.equal(orderTotalCents({ total }), null, String(total)); + } +}); + +test("the amount the checkout sends PayPal is the amount the capture accepts, for every total up to 3000.00", () => { + const mismatches: number[] = []; + for (let cents = 1; cents <= 300000; cents++) { + const total = cents / 100; + // buildPayPalOrderRequest sends order.total.toFixed(2); the order service rounds every total to the cent + if (parseCents(total.toFixed(2)) !== cents || orderTotalCents({ total }) !== cents) mismatches.push(total); + } + assert.equal(mismatches.length, 0, `e.g. ${mismatches.slice(0, 5).join(", ")}`); +}); + +test("the fixtures belong to the PayPal order the checkout requests for the order", () => { + const request = buildPayPalOrderRequest("https://shop.example/checkout/3", order()).purchaseUnits[0]; + + for (const paypalOrder of [approved(), captured()]) { + const unit = paypalOrder.purchase_units[0]; + assert.equal(unit.custom_id, request.customId); + assert.equal(unit.amount.value, request.amount.value); + assert.equal(unit.amount.currency_code, request.amount.currencyCode); + } +}); + +// The PayPal order the buyer approved + +test("accepts the PayPal order the buyer approved for the order", () => { + assert.deepEqual(verifyApprovedOrder(order(), approved()), { ok: true }); +}); + +test("rejects a PayPal order that is not approved", () => { + for (const status of ["CREATED", "SAVED", "PAYER_ACTION_REQUIRED", "VOIDED", "COMPLETED"]) { + assert.equal(rejection(verifyApprovedOrder(order(), { ...approved(), status })), `the PayPal order status is ${status}, not APPROVED`); + } + const { status, ...withoutStatus } = approved(); + assert.equal(rejection(verifyApprovedOrder(order(), withoutStatus)), "the PayPal order status is missing or unexpected, not APPROVED"); + // a value that is not one of PayPal's statuses is not repeated + assert.equal( + rejection(verifyApprovedOrder(order(), { ...approved(), status: "APPROVED BY ERIKA MUSTERMANN" })), + "the PayPal order status is missing or unexpected, not APPROVED" + ); +}); + +test("rejects the PayPal order of a cheap order captured for an expensive one", () => { + // The buyer pays order A, then calls the capture of order B with A's PayPal order. + const orderB = order({ id: 4712, uuid: "99999999-8888-4777-8666-555555555555", total: 249 }); + + assert.equal(rejection(verifyApprovedOrder(orderB, approved())), "the custom_id of the PayPal order is not the order's uuid"); +}); + +test("rejects a PayPal order without a custom_id, also for an order without a uuid", () => { + const paypalOrder = approved(); + delete paypalOrder.purchase_units[0].custom_id; + + assert.equal(rejection(verifyApprovedOrder(order(), paypalOrder)), "the custom_id of the PayPal order is not the order's uuid"); + // undefined is not compared with undefined, nor "" with "" + assert.equal( + rejection(verifyApprovedOrder(order({ uuid: undefined }), paypalOrder)), + "the custom_id of the PayPal order is not the order's uuid" + ); + paypalOrder.purchase_units[0].custom_id = ""; + assert.equal(rejection(verifyApprovedOrder(order({ uuid: "" }), paypalOrder)), "the custom_id of the PayPal order is not the order's uuid"); +}); + +test("rejects a PayPal order whose amount is not the order total", () => { + const withAmount = (value: unknown) => { + const paypalOrder = approved(); + paypalOrder.purchase_units[0].amount.value = value; + return paypalOrder; + }; + + assert.equal(rejection(verifyApprovedOrder(order(), withAmount("22.84"))), "the PayPal order amount 22.84 is not the order total 22.85"); + assert.equal(rejection(verifyApprovedOrder(order(), withAmount("22.86"))), "the PayPal order amount 22.86 is not the order total 22.85"); + assert.equal(rejection(verifyApprovedOrder(order(), withAmount("2285"))), "the PayPal order amount 2285.00 is not the order total 22.85"); + // the cart changed after the checkout + assert.equal(rejection(verifyApprovedOrder(order({ total: 249 }), approved())), "the PayPal order amount 22.85 is not the order total 249.00"); + for (const value of ["22.850", "22,85", "", 22.85, null]) { + assert.equal(rejection(verifyApprovedOrder(order(), withAmount(value))), "the PayPal order amount is not a money value", String(value)); + } +}); + +test("rejects a PayPal order in another currency or without an amount", () => { + const paypalOrder = approved(); + paypalOrder.purchase_units[0].amount.currency_code = "USD"; + assert.equal(rejection(verifyApprovedOrder(order(), paypalOrder)), "the PayPal order currency is USD, not EUR"); + + paypalOrder.purchase_units[0].amount.currency_code = "Euro"; + assert.equal(rejection(verifyApprovedOrder(order(), paypalOrder)), "the PayPal order currency is missing or unexpected, not EUR"); + + delete paypalOrder.purchase_units[0].amount; + assert.equal(rejection(verifyApprovedOrder(order(), paypalOrder)), "the PayPal order has no amount"); +}); + +test("rejects every PayPal order for an order without a total", () => { + for (const total of [null, 0]) { + assert.equal(rejection(verifyApprovedOrder(order({ total }), approved())), "the order has no total"); + } +}); + +test("rejects a PayPal order without a purchase unit, and anything that is not a PayPal order", () => { + assert.equal(rejection(verifyApprovedOrder(order(), { ...approved(), purchase_units: [] })), "the PayPal order has no purchase unit"); + const { purchase_units, ...withoutUnits } = approved(); + assert.equal(rejection(verifyApprovedOrder(order(), withoutUnits)), "the PayPal order has no purchase unit"); + + // "/result/error" is what PayPalApi.checkPaymentStatus answered for every error + for (const value of [null, undefined, "/result/error", [], 42]) { + assert.equal(rejection(verifyApprovedOrder(order(), value)), "the PayPal order is not an object", String(value)); + } +}); + +// The PayPal order after the capture + +test("accepts the completed capture of the order total and returns the capture id", () => { + assert.deepEqual(verifyCompletedCapture(order(), captured()), { ok: true, captureId: "3C679366HH908993F" }); +}); + +test("rejects a capture response whose PayPal order is not completed", () => { + for (const status of ["APPROVED", "PAYER_ACTION_REQUIRED", "VOIDED"]) { + assert.equal(rejection(verifyCompletedCapture(order(), { ...captured(), status })), `the PayPal order status is ${status}, not COMPLETED`); + } +}); + +test("rejects a capture PayPal holds for review, declined or refunded", () => { + for (const status of ["PENDING", "DECLINED", "FAILED", "REFUNDED", "PARTIALLY_REFUNDED"]) { + const response = captured(); + response.purchase_units[0].payments.captures[0].status = status; + if (status === "PENDING") response.purchase_units[0].payments.captures[0].status_details = { reason: "PENDING_REVIEW" }; + + assert.equal(rejection(verifyCompletedCapture(order(), response)), `the capture status is ${status}, not COMPLETED`); + } +}); + +test("rejects a capture response without a capture", () => { + // what PayPal answers with Prefer: return=minimal, which the capture used to ask for + const minimal = { id: "5O190127TN364715T", status: "COMPLETED", links: captured().links }; + assert.equal(rejection(verifyCompletedCapture(order(), minimal)), "the capture response has no capture"); + + const withoutPayments = captured(); + delete withoutPayments.purchase_units[0].payments; + assert.equal(rejection(verifyCompletedCapture(order(), withoutPayments)), "the capture response has no capture"); + + const withoutCaptures = captured(); + withoutCaptures.purchase_units[0].payments.captures = []; + assert.equal(rejection(verifyCompletedCapture(order(), withoutCaptures)), "the capture response has no capture"); + + assert.equal(rejection(verifyCompletedCapture(order(), { ...captured(), purchase_units: [] })), "the capture response has no capture"); +}); + +test("rejects a capture of another amount or in another currency", () => { + const lower = captured(); + lower.purchase_units[0].payments.captures[0].amount.value = "2.29"; + assert.equal(rejection(verifyCompletedCapture(order(), lower)), "the capture amount 2.29 is not the order total 22.85"); + + // the cart changed during the capture + assert.equal(rejection(verifyCompletedCapture(order({ total: 249 }), captured())), "the capture amount 22.85 is not the order total 249.00"); + + const usd = captured(); + usd.purchase_units[0].payments.captures[0].amount.currency_code = "USD"; + assert.equal(rejection(verifyCompletedCapture(order(), usd)), "the capture currency is USD, not EUR"); + + const withoutAmount = captured(); + delete withoutAmount.purchase_units[0].payments.captures[0].amount; + assert.equal(rejection(verifyCompletedCapture(order(), withoutAmount)), "the capture has no amount"); +}); + +test("rejects a capture without a valid id", () => { + for (const id of [undefined, "", 12345, "3C679366HH908993F; Erika Mustermann", "x".repeat(65)]) { + const response = captured(); + response.purchase_units[0].payments.captures[0].id = id; + assert.equal(rejection(verifyCompletedCapture(order(), response)), "the capture has no valid id", String(id)); + } +}); + +test("rejects anything that is not a capture response", () => { + for (const value of [null, undefined, "/result/error", [], 42]) { + assert.equal(rejection(verifyCompletedCapture(order(), value)), "the capture response is not an object", String(value)); + } +}); -- 2.36.6 From f599b2c84b4d97081e6df403b3e83115253ebab6 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:12:20 +0200 Subject: [PATCH 07/16] feat(logging): describe an error by its name, message and status code describeError keeps an error's name, the first line of its message, cut to 200 characters, and its HTTP status code. An error of the PayPal SDK or of axios carries its request, whose Authorization header holds the PayPal client credentials or an access token, so the error object itself must not reach the log. The SDK's response validation error quotes the response values on the lines after the first. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/logging/error.ts | 30 ++++++++++++++++ tests/unit/log-error.test.ts | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 src/logging/error.ts create mode 100644 tests/unit/log-error.test.ts diff --git a/src/logging/error.ts b/src/logging/error.ts new file mode 100644 index 0000000..0ea4ea6 --- /dev/null +++ b/src/logging/error.ts @@ -0,0 +1,30 @@ +/** What is logged of an error. */ +export type ErrorSummary = { + name: string; + message: string; + statusCode: number | undefined; +}; + +// A message is cut to its first line and to this length: the PayPal SDK's ResponseValidationError quotes the response values it +// rejected on the lines after its first. +const MAX_LENGTH = 200; + +const cut = (text: string): string => { + const line = text.split("\n")[0]; + return line.length > MAX_LENGTH ? `${line.slice(0, MAX_LENGTH)}…` : line; +}; + +/** + * The name, the first line of the message and the HTTP status code of an error, and nothing else. An error of the PayPal SDK + * (ApiError) or of axios carries its request, whose Authorization header holds the PayPal client credentials or an access token, + * so the error object itself is never logged. + */ +export const describeError = (error: unknown): ErrorSummary => { + if (typeof error !== "object" || error === null) return { name: typeof error, message: "", statusCode: undefined }; + const { name, message, statusCode } = error as { name?: unknown; message?: unknown; statusCode?: unknown }; + return { + name: typeof name === "string" ? cut(name) : "object", + message: typeof message === "string" ? cut(message) : "", + statusCode: typeof statusCode === "number" ? statusCode : undefined + }; +}; diff --git a/tests/unit/log-error.test.ts b/tests/unit/log-error.test.ts new file mode 100644 index 0000000..2407e96 --- /dev/null +++ b/tests/unit/log-error.test.ts @@ -0,0 +1,67 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { describeError } from "../../src/logging/error.ts"; +import { formatLogLine } from "../../src/logging/format.ts"; + +const TIMESTAMP = "2026-10-08 21:29:40.426"; +const CREDENTIALS = "RFVNTVlfQ0xJRU5UX0lEOkRVTU1ZX0NMSUVOVF9TRUNSRVQ="; // base64 of the dummy "DUMMY_CLIENT_ID:DUMMY_CLIENT_SECRET" + +// What axios throws when PayPal cannot be reached: the request config with its headers. +const networkError = () => + Object.assign(new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"), { + name: "AxiosError", + code: "EAI_AGAIN", + config: { method: "post", url: "https://api-m.paypal.com/v1/oauth2/token", headers: { Authorization: `Basic ${CREDENTIALS}` } } + }); + +test("keeps the name, message and status code of a PayPal SDK error, not its request, headers or body", () => { + // @apimatic/core's ApiError, which the SDK's CustomError and OAuthProviderError extend + const error = Object.assign(new Error("OAuth 2 Authorization endpoint returned error."), { + request: { method: "POST", url: "https://api-m.paypal.com/v1/oauth2/token", headers: { Authorization: `Basic ${CREDENTIALS}` } }, + statusCode: 401, + headers: { "paypal-debug-id": "f1a2b3c4d5e6f" }, + body: '{"error":"invalid_client","error_description":"Client Authentication failed"}' + }); + + assert.deepEqual(describeError(error), { name: "Error", message: "OAuth 2 Authorization endpoint returned error.", statusCode: 401 }); +}); + +test("keeps the name and message of an axios network error, not its config", () => { + assert.deepEqual(describeError(networkError()), { name: "AxiosError", message: "getaddrinfo EAI_AGAIN api-m.paypal.com", statusCode: undefined }); +}); + +test("keeps the first line of a message: the SDK's response validation error quotes the response after it", () => { + const error = new Error( + 'The response did not match the response schema.\n\n> Issue #1\n\nExpected value to be of type \'string\'.\n\nGiven value: {"email_address":"erika.buyer@example.org"}' + ); + + assert.deepEqual(describeError(error), { name: "Error", message: "The response did not match the response schema.", statusCode: undefined }); +}); + +test("cuts a long message", () => { + assert.equal(describeError(new Error("x".repeat(500))).message, `${"x".repeat(200)}…`); +}); + +test("describes a thrown value that is not an error by its type alone", () => { + assert.deepEqual(describeError("/result/error"), { name: "string", message: "", statusCode: undefined }); + assert.deepEqual(describeError(undefined), { name: "undefined", message: "", statusCode: undefined }); + assert.deepEqual(describeError(null), { name: "object", message: "", statusCode: undefined }); + assert.deepEqual(describeError({ statusCode: "500" }), { name: "object", message: "", statusCode: undefined }); +}); + +test("the log line of the description holds no credentials", () => { + // Compare log-format.test.ts: the line of the error object itself holds them, a known defect of the formatter. + const line = formatLogLine({ + timestamp: TIMESTAMP, + level: "error", + message: "app:e:order-controller: PayPal did not capture the payment", + order: 4711, + error: describeError(networkError()) + }); + + assert.equal( + line, + '2026-10-08 21:29:40.426Z error: app:e:order-controller: PayPal did not capture the payment {\n "order": 4711,\n "error": {\n "name": "AxiosError",\n "message": "getaddrinfo EAI_AGAIN api-m.paypal.com"\n }\n}' + ); + assert.ok(!line.includes(CREDENTIALS)); +}); -- 2.36.6 From d56ae4359758a0430db4a9aaf67422f05bab70da Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:12:20 +0200 Subject: [PATCH 08/16] feat(checkout): capture a payment through injected dependencies capturePayment(deps, { uuid, paypalOrderId, now }) is the capture flow without Strapi or the PayPal SDK, so it is tested with fakes: - a missing order is answered 404; - a paid order is answered alreadyCaptured for the PayPal order it was paid with, else 409, without a call or a write; - a PayPal order the checkout did not store on the order is answered 409 before PayPal is asked anything; - a PayPal order that does not verify as approved for the order is answered 409 without a capture; - the capture is sent with the PayPal-Request-Id capture--, so a retry captures once; - a capture that does not verify leaves the order unpaid, is logged as an error with the order id and the reason, and is answered 502: PayPal may have taken the money, so it needs a manual refund check; - the payment is written only while the order still carries the PayPal order id, so an order whose cart or address changed during the capture stays unpaid (409, logged with the capture id); - PayPal and network errors are answered 502 and logged by name, message and status code only. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/checkout/capture-payment.ts | 155 ++++++++++++ tests/unit/capture-payment.test.ts | 375 +++++++++++++++++++++++++++++ 2 files changed, 530 insertions(+) create mode 100644 src/checkout/capture-payment.ts create mode 100644 tests/unit/capture-payment.test.ts diff --git a/src/checkout/capture-payment.ts b/src/checkout/capture-payment.ts new file mode 100644 index 0000000..46716b3 --- /dev/null +++ b/src/checkout/capture-payment.ts @@ -0,0 +1,155 @@ +import { verifyApprovedOrder, verifyCompletedCapture } from "./paypal-verify"; +import type { VerifiableOrder } from "./paypal-verify"; +import { describeError } from "../logging/error"; + +// POST /orders/:uuid/capture/:paypalOrderId, which the shop calls once the buyer approved the payment in PayPal's window +// (libreshop/shop pages/checkout/3.vue, onApprove). The PayPal order id comes from the browser, so it must be the one the checkout +// stored on the order before PayPal is asked anything, and PayPal's answers are checked against the order before it is marked paid. +// The order controller passes Strapi and PayPalApi as the dependencies; the tests pass fakes. + +/** The fields of the order the capture reads. */ +export type CapturableOrder = VerifiableOrder & { + id: number | string; + paymentAuthorised?: boolean | null; + paypalOrderId?: string | null; +}; + +/** What a verified capture writes to the order. */ +export type CapturedPayment = { + paymentAuthorised: true; + paymentStatus: "captured"; + paypalCaptureId: string; + paymentCapturedAt: string; +}; + +export type CaptureLog = Record<"info" | "warn" | "error", (message: string, fields: Record) => void>; + +export type CapturePaymentDeps = { + /** The order with this uuid, or null. */ + findOrder: (uuid: string) => Promise; + /** PayPal's order as parsed JSON: GET /v2/checkout/orders/:id. */ + getPayPalOrder: (paypalOrderId: string) => Promise; + /** Captures the PayPal order and returns PayPal's order as parsed JSON: POST /v2/checkout/orders/:id/capture. */ + capturePayPalOrder: (paypalOrderId: string, requestId: string) => Promise; + /** Writes data to the order only while the order still carries this PayPal order id: the updated order, or null if it does not. */ + updateOrder: (id: O["id"], paypalOrderId: string, data: CapturedPayment) => Promise; + log: CaptureLog; +}; + +export type CapturePaymentResult = + | { status: 200; alreadyCaptured: false; order: R } + | { status: 200; alreadyCaptured: true } + | { status: 404 | 409 | 500 | 502; message: string }; + +export const NOT_FOUND = "Order not found"; +export const ALREADY_PAID = "Order is already paid"; +export const NOT_THIS_ORDER = "Payment does not belong to this order"; +export const NOT_MATCHING = "Payment does not match this order"; +export const PAYPAL_FAILED = "PayPal could not capture the payment"; +export const NOT_CONFIRMED = "Payment could not be confirmed"; +export const NOT_RECORDED = "Payment could not be recorded"; +export const ORDER_CHANGED = "Order changed during the payment"; + +/** + * The PayPal-Request-Id of the capture. PayPal answers a repeated id with the first capture's response instead of capturing again, + * so a capture that is retried, or sent twice, takes the money once. + */ +export const captureRequestId = (uuid: string, paypalOrderId: string): string => `capture-${uuid}-${paypalOrderId}`; + +/** + * Captures the PayPal order the checkout created for the order and marks the order paid, in this order: + * 1. a missing order is answered 404; + * 2. a paid order is answered alreadyCaptured if the PayPal order is the one it was paid with, else 409, without a write; + * 3. a PayPal order the checkout did not store on the order is answered 409, without asking PayPal; + * 4. a PayPal order that is not APPROVED for this order's uuid, total and currency is answered 409, without a capture; + * 5. the capture must be COMPLETED for the order total in EUR, else the order stays unpaid and 502 is answered; + * 6. the payment is written only while the order still carries this PayPal order id, so a cart or address changed during the + * capture is not marked paid. + * PayPal and network errors are answered 502 and logged with name, message and status code only. + */ +export const capturePayment = async ( + deps: CapturePaymentDeps, + { uuid, paypalOrderId, now }: { uuid: string; paypalOrderId: string; now: Date } +): Promise> => { + const { log } = deps; + + // An empty uuid finds no order. Strapi would drop an undefined condition and find any order. + const order = typeof uuid === "string" && uuid !== "" ? await deps.findOrder(uuid) : null; + if (!order) return { status: 404, message: NOT_FOUND }; + + const isStoredPayPalOrder = typeof order.paypalOrderId === "string" && order.paypalOrderId !== "" && order.paypalOrderId === paypalOrderId; + + if (order.paymentAuthorised) { + if (isStoredPayPalOrder) return { status: 200, alreadyCaptured: true }; + log.warn("Rejected a capture for a paid order with another PayPal order", { order: order.id }); + return { status: 409, message: ALREADY_PAID }; + } + if (!isStoredPayPalOrder) { + log.warn("Rejected a capture with a PayPal order the checkout did not create for the order", { + order: order.id, + hasPayPalOrder: typeof order.paypalOrderId === "string" && order.paypalOrderId !== "" + }); + return { status: 409, message: NOT_THIS_ORDER }; + } + + let paypalOrder: unknown; + try { + paypalOrder = await deps.getPayPalOrder(paypalOrderId); + } catch (error) { + log.error("Could not get the PayPal order", { order: order.id, error: describeError(error) }); + return { status: 502, message: PAYPAL_FAILED }; + } + + const approved = verifyApprovedOrder(order, paypalOrder); + if (approved.ok === false) { + log.warn("Rejected a capture: the PayPal order does not match the order", { order: order.id, reason: approved.reason }); + return { status: 409, message: NOT_MATCHING }; + } + + let captured: unknown; + try { + captured = await deps.capturePayPalOrder(paypalOrderId, captureRequestId(order.uuid, paypalOrderId)); + } catch (error) { + log.error("PayPal did not capture the payment", { order: order.id, error: describeError(error) }); + return { status: 502, message: PAYPAL_FAILED }; + } + + const capture = verifyCompletedCapture(order, captured); + if (capture.ok === false) { + // PayPal answered the capture, so it may have taken the money: the PayPal order needs a manual check, and a refund if it was paid. + log.error("The PayPal capture does not match the order, which stays unpaid: check the PayPal order for a refund", { + order: order.id, + reason: capture.reason + }); + return { status: 502, message: NOT_CONFIRMED }; + } + + const data: CapturedPayment = { + paymentAuthorised: true, + paymentStatus: "captured", + paypalCaptureId: capture.captureId, + paymentCapturedAt: now.toISOString() + }; + + let updated: R | null; + try { + updated = await deps.updateOrder(order.id, paypalOrderId, data); + } catch (error) { + log.error("PayPal captured the payment, but the order could not be marked paid: check the order", { + order: order.id, + capture: capture.captureId, + error: describeError(error) + }); + return { status: 500, message: NOT_RECORDED }; + } + if (updated === null || updated === undefined) { + log.error("PayPal captured the payment, but the order changed during the capture and stays unpaid: check the order for a refund", { + order: order.id, + capture: capture.captureId + }); + return { status: 409, message: ORDER_CHANGED }; + } + + log.info("Captured the payment", { order: order.id, capture: capture.captureId }); + return { status: 200, alreadyCaptured: false, order: updated }; +}; diff --git a/tests/unit/capture-payment.test.ts b/tests/unit/capture-payment.test.ts new file mode 100644 index 0000000..9d8ceb8 --- /dev/null +++ b/tests/unit/capture-payment.test.ts @@ -0,0 +1,375 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { + ALREADY_PAID, + NOT_CONFIRMED, + NOT_FOUND, + NOT_MATCHING, + NOT_RECORDED, + NOT_THIS_ORDER, + ORDER_CHANGED, + PAYPAL_FAILED, + capturePayment, + captureRequestId +} from "../../src/checkout/capture-payment.ts"; +import { formatLogLine } from "../../src/logging/format.ts"; + +const UUID = "11111111-2222-4333-8444-555555555555"; +const PAYPAL_ORDER_ID = "5O190127TN364715T"; +const OTHER_PAYPAL_ORDER_ID = "8MC585209K746392H"; +const CAPTURE_ID = "3C679366HH908993F"; +// The server's time when the capture request arrives. +const NOW = new Date("2026-10-08T19:13:05.000Z"); +const CREDENTIALS = "RFVNTVlfQ0xJRU5UX0lEOkRVTU1ZX0NMSUVOVF9TRUNSRVQ="; // base64 of the dummy "DUMMY_CLIENT_ID:DUMMY_CLIENT_SECRET" + +// PayPal's Orders v2 JSON for the order below, see paypal-verify.test.ts. Every call returns a fresh copy to change. +const fixture = (name: string) => JSON.parse(readFileSync(new URL(`./fixtures/${name}`, import.meta.url), "utf8")); + +// The row the capture reads with strapi.db.query("api::order.order").findOne({ where: { uuid } }) after a checkout stored the +// PayPal order it created: subtotal 17.90, delivery 4.95, total 22.85. +const orderRow = (overrides = {}) => ({ + id: 4711, + uuid: UUID, + email: "erika@example.org", + subtotal: 17.9, + total: 22.85, + VAT: 2.86, + paymentAuthorised: false, + paymentStatus: null, + invoiceSent: false, + paypalOrderId: PAYPAL_ORDER_ID, + paypalCaptureId: null, + paymentCapturedAt: null, + ...overrides +}); + +/** + * Fakes of the dependencies over one order row, which record every call and every log line. updateOrder writes like the + * controller's update with where { id, paypalOrderId }: only while the row still carries that PayPal order, else it answers null. + * duringCapture runs while PayPal captures, as a request that changes the order at that moment would. + */ +const setup = ({ + row = orderRow(), + paypalOrder = fixture("paypal-order-approved.json"), + captured = fixture("paypal-order-captured.json"), + getError = undefined, + captureError = undefined, + updateError = undefined, + duringCapture = (row) => {} +} = {}) => { + const calls = { findOrder: [], getPayPalOrder: [], capturePayPalOrder: [], updateOrder: [] }; + const logs = []; + const deps = { + findOrder: async (uuid) => { + calls.findOrder.push(uuid); + return row !== null && row.uuid === uuid ? { ...row } : null; + }, + getPayPalOrder: async (id) => { + calls.getPayPalOrder.push(id); + if (getError) throw getError; + return paypalOrder; + }, + capturePayPalOrder: async (id, requestId) => { + calls.capturePayPalOrder.push({ id, requestId }); + duringCapture(row); + if (captureError) throw captureError; + return captured; + }, + updateOrder: async (id, paypalOrderId, data) => { + calls.updateOrder.push({ id, paypalOrderId, data }); + if (updateError) throw updateError; + if (row.id !== id || row.paypalOrderId !== paypalOrderId) return null; + Object.assign(row, data); + return { ...row }; + }, + log: { + info: (message, fields) => logs.push({ level: "info", message, fields }), + warn: (message, fields) => logs.push({ level: "warn", message, fields }), + error: (message, fields) => logs.push({ level: "error", message, fields }) + } + }; + return { deps, calls, logs, row }; +}; + +const capture = (deps, { uuid = UUID, paypalOrderId = PAYPAL_ORDER_ID } = {}) => capturePayment(deps, { uuid, paypalOrderId, now: NOW }); + +const paypalCalls = (calls) => calls.getPayPalOrder.length + calls.capturePayPalOrder.length; + +// What must never reach the log: the credentials of a failed request, and the buyer's data in PayPal's responses. +const NEVER_LOGGED = [CREDENTIALS, "Authorization", "Erika", "Mustermann", "erika", "Musterstraße", "QYR5Z8XDVJNXQ", "INSTRUMENT_DECLINED"]; + +/** Checks the recorded log, and each line as the console shows it (config/logger.ts formats with formatLogLine). */ +const assertCleanLog = (logs) => { + const lines = logs.map(({ level, message, fields }) => + formatLogLine({ timestamp: "2026-10-08 19:13:05.000", level, message: `app:${level[0]}:order-controller: ${message}`, ...fields }) + ); + for (const text of [JSON.stringify(logs), ...lines]) { + for (const value of NEVER_LOGGED) assert.ok(!text.includes(value), `the log holds ${value}: ${text}`); + } +}; + +// What the PayPal SDK throws for an error response: @apimatic/core's ApiError, here the SDK's CustomError, with the request, whose +// Authorization header holds the client credentials (token request) or an access token, and PayPal's error body. +const paypalApiError = (statusCode: number, message: string) => + Object.assign(new Error(message), { + request: { + method: "POST", + url: `https://api-m.sandbox.paypal.com/v2/checkout/orders/${PAYPAL_ORDER_ID}/capture`, + headers: { Authorization: `Basic ${CREDENTIALS}`, "PayPal-Request-Id": captureRequestId(UUID, PAYPAL_ORDER_ID) } + }, + statusCode, + headers: { "paypal-debug-id": "f1a2b3c4d5e6f" }, + body: JSON.stringify({ name: "UNPROCESSABLE_ENTITY", details: [{ issue: "INSTRUMENT_DECLINED" }], debug_id: "f1a2b3c4d5e6f" }) + }); + +// What axios throws when PayPal cannot be reached: the request config with its headers. +const networkError = (message = "getaddrinfo EAI_AGAIN api-m.sandbox.paypal.com") => + Object.assign(new Error(message), { + name: "AxiosError", + code: "EAI_AGAIN", + config: { method: "post", url: "https://api-m.sandbox.paypal.com/v1/oauth2/token", headers: { Authorization: `Basic ${CREDENTIALS}` } } + }); + +// The capture + +test("captures the PayPal order the checkout stored and writes exactly the payment fields", async () => { + const { deps, calls, logs } = setup(); + const result = await capture(deps); + + assert.equal(result.status, 200); + assert.equal(result.alreadyCaptured, false); + assert.deepEqual(calls.findOrder, [UUID]); + assert.deepEqual(calls.getPayPalOrder, [PAYPAL_ORDER_ID]); + assert.deepEqual(calls.capturePayPalOrder, [{ id: PAYPAL_ORDER_ID, requestId: `capture-${UUID}-${PAYPAL_ORDER_ID}` }]); + assert.deepEqual(calls.updateOrder, [ + { + id: 4711, + paypalOrderId: PAYPAL_ORDER_ID, + data: { paymentAuthorised: true, paymentStatus: "captured", paypalCaptureId: CAPTURE_ID, paymentCapturedAt: "2026-10-08T19:13:05.000Z" } + } + ]); + // the controller answers the updated order, sanitized + assert.equal(result.order.paymentAuthorised, true); + assert.deepEqual(logs, [{ level: "info", message: "Captured the payment", fields: { order: 4711, capture: CAPTURE_ID } }]); + assertCleanLog(logs); +}); + +test("sends the same PayPal-Request-Id on every attempt, so that PayPal captures once", async () => { + assert.equal(captureRequestId(UUID, PAYPAL_ORDER_ID), "capture-11111111-2222-4333-8444-555555555555-5O190127TN364715T"); + // PayPal accepts at most 108 characters + assert.ok(captureRequestId(UUID, PAYPAL_ORDER_ID).length <= 108); + + // The first attempt times out after PayPal may have captured. The retry repeats the id, and PayPal answers the first capture. + const first = setup({ captureError: Object.assign(networkError("timeout of 30000ms exceeded"), { code: "ECONNABORTED" }) }); + assert.deepEqual(await capture(first.deps), { status: 502, message: PAYPAL_FAILED }); + const retry = setup(); + assert.equal((await capture(retry.deps)).status, 200); + + assert.equal(first.calls.capturePayPalOrder[0].requestId, retry.calls.capturePayPalOrder[0].requestId); +}); + +// Paying a cheap order, capturing an expensive one + +test("rejects a PayPal order the checkout did not store on the order, without asking PayPal", async () => { + // The buyer paid order A with PayPal order OTHER_PAYPAL_ORDER_ID and calls the capture of the expensive order B with it. + const { deps, calls, logs, row } = setup({ row: orderRow({ id: 4712, uuid: UUID, total: 249 }) }); + + assert.deepEqual(await capture(deps, { paypalOrderId: OTHER_PAYPAL_ORDER_ID }), { status: 409, message: NOT_THIS_ORDER }); + assert.equal(paypalCalls(calls), 0); + assert.equal(calls.updateOrder.length, 0); + assert.equal(row.paymentAuthorised, false); + assert.deepEqual(logs, [ + { + level: "warn", + message: "Rejected a capture with a PayPal order the checkout did not create for the order", + fields: { order: 4712, hasPayPalOrder: true } + } + ]); +}); + +test("rejects every PayPal order for an order without a stored one, without asking PayPal", async () => { + // no checkout yet, an order from before the checkout stored it, or a cart changed since the checkout + for (const paypalOrderId of [null, undefined, ""]) { + const { deps, calls, logs } = setup({ row: orderRow({ paypalOrderId }) }); + + assert.deepEqual(await capture(deps), { status: 409, message: NOT_THIS_ORDER }); + assert.equal(paypalCalls(calls) + calls.updateOrder.length, 0); + assert.equal(logs[0].fields.hasPayPalOrder, false); + } + + // a request without a PayPal order id + const { deps, calls } = setup(); + assert.deepEqual(await capture(deps, { paypalOrderId: "" }), { status: 409, message: NOT_THIS_ORDER }); + assert.equal(paypalCalls(calls), 0); +}); + +test("does not capture a PayPal order whose amount is not the order total", async () => { + const paypalOrder = fixture("paypal-order-approved.json"); + paypalOrder.purchase_units[0].amount.value = "2.29"; + const { deps, calls, logs } = setup({ paypalOrder }); + + assert.deepEqual(await capture(deps), { status: 409, message: NOT_MATCHING }); + assert.deepEqual(calls.getPayPalOrder, [PAYPAL_ORDER_ID]); + assert.equal(calls.capturePayPalOrder.length, 0); + assert.equal(calls.updateOrder.length, 0); + assert.deepEqual(logs, [ + { + level: "warn", + message: "Rejected a capture: the PayPal order does not match the order", + fields: { order: 4711, reason: "the PayPal order amount 2.29 is not the order total 22.85" } + } + ]); + assertCleanLog(logs); +}); + +test("does not capture a PayPal order created for another order, in another currency or not approved", async () => { + const cases: [(paypalOrder) => void, string][] = [ + [ + (paypalOrder) => (paypalOrder.purchase_units[0].custom_id = "99999999-8888-4777-8666-555555555555"), + "the custom_id of the PayPal order is not the order's uuid" + ], + [(paypalOrder) => (paypalOrder.purchase_units[0].amount.currency_code = "USD"), "the PayPal order currency is USD, not EUR"], + [(paypalOrder) => (paypalOrder.status = "CREATED"), "the PayPal order status is CREATED, not APPROVED"] + ]; + for (const [change, reason] of cases) { + const paypalOrder = fixture("paypal-order-approved.json"); + change(paypalOrder); + const { deps, calls, logs } = setup({ paypalOrder }); + + assert.deepEqual(await capture(deps), { status: 409, message: NOT_MATCHING }); + assert.equal(calls.capturePayPalOrder.length + calls.updateOrder.length, 0); + assert.equal(logs[0].fields.reason, reason); + assertCleanLog(logs); + } +}); + +// A paid order + +test("answers alreadyCaptured for a paid order and the PayPal order it was paid with, without a call or a write", async () => { + const { deps, calls, logs } = setup({ row: orderRow({ paymentAuthorised: true, paymentStatus: "captured", paypalCaptureId: CAPTURE_ID }) }); + + assert.deepEqual(await capture(deps), { status: 200, alreadyCaptured: true }); + assert.equal(paypalCalls(calls) + calls.updateOrder.length, 0); + assert.deepEqual(logs, []); +}); + +test("rejects another PayPal order for a paid order, without a call or a write", async () => { + const paidRows = [ + orderRow({ paymentAuthorised: true, paymentStatus: "captured", paypalCaptureId: CAPTURE_ID }), + // paid before the checkout stored the PayPal order + orderRow({ paymentAuthorised: true, paymentStatus: "authorised", paypalOrderId: null }) + ]; + for (const row of paidRows) { + const { deps, calls } = setup({ row }); + + assert.deepEqual(await capture(deps, { paypalOrderId: OTHER_PAYPAL_ORDER_ID }), { status: 409, message: ALREADY_PAID }); + assert.equal(paypalCalls(calls) + calls.updateOrder.length, 0); + } +}); + +test("answers 404 for a missing order, without asking PayPal", async () => { + const { deps, calls } = setup({ row: null }); + + assert.deepEqual(await capture(deps), { status: 404, message: NOT_FOUND }); + assert.equal(paypalCalls(calls), 0); + + // No uuid: no lookup at all, since Strapi drops an undefined where condition and would find any order. + for (const uuid of ["", undefined]) { + const { deps, calls } = setup(); + assert.deepEqual(await capturePayment(deps, { uuid, paypalOrderId: PAYPAL_ORDER_ID, now: NOW }), { status: 404, message: NOT_FOUND }); + assert.deepEqual(calls.findOrder, []); + } +}); + +// PayPal fails + +test("answers 502 when PayPal fails, and logs the error's name, message and status code but not its request", async () => { + const failures: [string, Error & { statusCode?: number }][] = [ + ["getError", paypalApiError(401, "Authentication failed due to missing authorization header, or invalid authentication credentials.")], + ["getError", networkError()], + ["captureError", paypalApiError(422, "The requested action could not be performed, semantically incorrect, or failed business validation.")], + ["captureError", networkError()] + ]; + for (const [failing, error] of failures) { + const { deps, calls, logs, row } = setup({ [failing]: error }); + + assert.deepEqual(await capture(deps), { status: 502, message: PAYPAL_FAILED }); + assert.equal(calls.capturePayPalOrder.length, failing === "getError" ? 0 : 1); + assert.equal(calls.updateOrder.length, 0); + assert.equal(row.paymentAuthorised, false); + assert.equal(logs.length, 1); + assert.equal(logs[0].level, "error"); + assert.deepEqual(logs[0].fields, { order: 4711, error: { name: error.name, message: error.message, statusCode: error.statusCode } }); + assertCleanLog(logs); + } +}); + +// After PayPal answered the capture + +test("keeps the order unpaid and answers 502 when the capture does not match, logging the order and the reason only", async () => { + const pending = fixture("paypal-order-captured.json"); + pending.purchase_units[0].payments.captures[0].status = "PENDING"; + const lower = fixture("paypal-order-captured.json"); + lower.purchase_units[0].payments.captures[0].amount.value = "2.29"; + const minimal = { id: PAYPAL_ORDER_ID, status: "COMPLETED", links: [] }; + + const cases: [unknown, string][] = [ + [pending, "the capture status is PENDING, not COMPLETED"], + [lower, "the capture amount 2.29 is not the order total 22.85"], + [minimal, "the capture response has no capture"] + ]; + for (const [captured, reason] of cases) { + const { deps, calls, logs, row } = setup({ captured }); + + assert.deepEqual(await capture(deps), { status: 502, message: NOT_CONFIRMED }); + assert.equal(calls.updateOrder.length, 0); + assert.equal(row.paymentAuthorised, false); + assert.deepEqual(logs, [ + { + level: "error", + message: "The PayPal capture does not match the order, which stays unpaid: check the PayPal order for a refund", + fields: { order: 4711, reason } + } + ]); + assertCleanLog(logs); + } +}); + +test("keeps the order unpaid when its cart or address changed during the capture", async () => { + // addProduct, removeProduct and updateByUuid clear paypalOrderId in the write that changes the order + const { deps, calls, logs, row } = setup({ + duringCapture: (row) => { + row.total = 249; + row.paypalOrderId = null; + } + }); + + assert.deepEqual(await capture(deps), { status: 409, message: ORDER_CHANGED }); + assert.equal(calls.updateOrder.length, 1); + assert.equal(row.paymentAuthorised, false); + assert.deepEqual(logs, [ + { + level: "error", + message: "PayPal captured the payment, but the order changed during the capture and stays unpaid: check the order for a refund", + fields: { order: 4711, capture: CAPTURE_ID } + } + ]); +}); + +test("answers 500 when the order cannot be marked paid after the capture, and logs the capture", async () => { + const { deps, logs } = setup({ updateError: new Error("Connection terminated unexpectedly") }); + + assert.deepEqual(await capture(deps), { status: 500, message: NOT_RECORDED }); + assert.deepEqual(logs, [ + { + level: "error", + message: "PayPal captured the payment, but the order could not be marked paid: check the order", + fields: { + order: 4711, + capture: CAPTURE_ID, + error: { name: "Error", message: "Connection terminated unexpectedly", statusCode: undefined } + } + } + ]); +}); -- 2.36.6 From c7a422621ab7fcca3ec4d53be14aeeef6fd8cf91 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:12:21 +0200 Subject: [PATCH 09/16] feat(order): name the fields an order still lacks for a checkout missingCheckoutFields returns which of email, invoiceAddress and delivery an order lacks, so the checkout can answer 400 before it creates a PayPal order. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/checkout/order-state.ts | 17 +++++++++++++++++ tests/unit/order-state.test.ts | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/checkout/order-state.ts b/src/checkout/order-state.ts index 7c66128..4388349 100644 --- a/src/checkout/order-state.ts +++ b/src/checkout/order-state.ts @@ -8,3 +8,20 @@ export type OrderLockState = { * so the customer can no longer change its contact details, addresses, delivery method or cart. */ export const isLockedForCustomer = (order: OrderLockState): boolean => Boolean(order.paymentAuthorised); + +/** The fields of an order a checkout needs: the contact and address steps of the shop's checkout, and a delivery method. */ +export type CheckoutFields = { + email?: string | null; + invoiceAddress?: string | null; + /** The populated delivery method, or null. */ + delivery?: unknown; +}; + +/** The names of the fields the order still lacks for a checkout, in this order: email, invoiceAddress, delivery. [] if none. */ +export const missingCheckoutFields = (order: CheckoutFields): string[] => { + const missing: string[] = []; + if (typeof order.email !== "string" || order.email.trim() === "") missing.push("email"); + if (typeof order.invoiceAddress !== "string" || order.invoiceAddress.trim() === "") missing.push("invoiceAddress"); + if (order.delivery === null || order.delivery === undefined) missing.push("delivery"); + return missing; +}; diff --git a/tests/unit/order-state.test.ts b/tests/unit/order-state.test.ts index 3274d24..c07261a 100644 --- a/tests/unit/order-state.test.ts +++ b/tests/unit/order-state.test.ts @@ -1,6 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { isLockedForCustomer } from "../../src/checkout/order-state.ts"; +import { isLockedForCustomer, missingCheckoutFields } from "../../src/checkout/order-state.ts"; // Shaped like the row PUT /orders/:uuid/cart reads with strapi.db.query("api::order.order").findOne before it updates. const order = (overrides = {}) => ({ @@ -21,6 +21,36 @@ test("an order is open for the customer until its payment is authorised", () => }); test("an order is locked once its payment is authorised", () => { - // what capturePayment writes once PayPal reports the capture COMPLETED + // what capturePayment writes once the capture is verified (src/checkout/capture-payment.ts) + assert.equal(isLockedForCustomer(order({ paymentAuthorised: true, paymentStatus: "captured" })), true); + // what it wrote before assert.equal(isLockedForCustomer(order({ paymentAuthorised: true, paymentStatus: "authorised" })), true); }); + +// Shaped like the row POST /orders/:uuid/checkout reads (strapi.db.query findOne with orderDefaultParams), after both checkout steps. +const readyForCheckout = (overrides = {}) => + order({ + invoiceAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", + deliveryAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", + delivery: { id: 1, name: "DHL", price: 4.95 }, + ...overrides + }); + +test("an order that went through both checkout steps lacks nothing for the checkout", () => { + assert.deepEqual(missingCheckoutFields(readyForCheckout()), []); +}); + +test("names the email, invoice address and delivery method an order still lacks for the checkout", () => { + // a new order: POST /orders creates it empty + assert.deepEqual(missingCheckoutFields({ email: null, invoiceAddress: null, delivery: null }), ["email", "invoiceAddress", "delivery"]); + assert.deepEqual(missingCheckoutFields({}), ["email", "invoiceAddress", "delivery"]); + // after step 1 alone + assert.deepEqual(missingCheckoutFields(readyForCheckout({ invoiceAddress: null, deliveryAddress: null, delivery: null })), [ + "invoiceAddress", + "delivery" + ]); + assert.deepEqual(missingCheckoutFields(readyForCheckout({ email: " " })), ["email"]); + assert.deepEqual(missingCheckoutFields(readyForCheckout({ invoiceAddress: "" })), ["invoiceAddress"]); + // a delivery method that was deleted is populated as null + assert.deepEqual(missingCheckoutFields(readyForCheckout({ delivery: null })), ["delivery"]); +}); -- 2.36.6 From 66e5800418e633914bb29fe4286dfdea86ede96f Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:12:31 +0200 Subject: [PATCH 10/16] fix(order): capture only the PayPal order the checkout stored, verified against the order The capture took the PayPal order id from the URL and captured it with Prefer: return=minimal, then marked the order paid on status COMPLETED. The checkout never stored its PayPal order (the paypalOrderId written at capture was no schema attribute and was dropped), and neither the order nor the amount was compared. A buyer could pay a cheap order and call the capture of an expensive one with that PayPal order id: the expensive order was marked paid and invoiced. checkPaymentStatus also turned every error into the string "/result/error", which failed the capture without a log. checkout now answers 409 for a paid order and 400 with the missing fields for an order without email, invoice address or delivery method. It stores the id of the PayPal order it created as paypalOrderId and answers { id, links } as before. A PayPal error is answered 502. capturePayment is an adapter for src/checkout/capture-payment.ts. It reads the PayPal order with GET, captures with Prefer: return=representation and a PayPal-Request-Id, and writes paymentAuthorised, paymentStatus "captured", paypalCaptureId and paymentCapturedAt with where { id, paypalOrderId }. updateByUuid, addProduct and removeProduct set paypalOrderId to null in the write that changes the order, so a changed cart or address needs a new checkout, and an earlier approval cannot be captured for it. PayPalApi gains getOrder and captureOrder, which return PayPal's parsed JSON and let errors propagate. createSessionOrThrow logs an error by name, message and status code instead of the error object, which holds the request with its Authorization header; its request building is unchanged. Refs https://git.librete.ch/libretech/mp/issues/71 --- src/api/order/controllers/order.ts | 104 ++++++++++++++++++----------- src/services/PayPalApi.ts | 35 +++++----- tests/unit/customer-update.test.ts | 9 ++- 3 files changed, 89 insertions(+), 59 deletions(-) diff --git a/src/api/order/controllers/order.ts b/src/api/order/controllers/order.ts index 1cc7cec..d68a0c2 100644 --- a/src/api/order/controllers/order.ts +++ b/src/api/order/controllers/order.ts @@ -7,10 +7,19 @@ import { orderDefaultParams } from "../services/order"; import { calculateTotalProductPrice } from "../../product/services/product"; import { parseCustomerUpdate } from "../../../checkout/customer-update"; import type { CustomerUpdate } from "../../../checkout/customer-update"; -import { isLockedForCustomer } from "../../../checkout/order-state"; +import { isLockedForCustomer, missingCheckoutFields } from "../../../checkout/order-state"; +import { capturePayment } from "../../../checkout/capture-payment"; +import type { CaptureLog } from "../../../checkout/capture-payment"; +import { describeError } from "../../../logging/error"; const ORDER_LOCKED = "Order can no longer be changed"; +const captureLog: CaptureLog = { + info: (message, fields) => strapi.log.info(`app:i:order-controller: ${message}`, fields), + warn: (message, fields) => strapi.log.warn(`app:w:order-controller: ${message}`, fields), + error: (message, fields) => strapi.log.error(`app:e:order-controller: ${message}`, fields) +}; + /** * The order controller is a funnel for the client to interact with the order service. * From creating until finalizing an order, the controller is responsible for handling @@ -117,7 +126,8 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = strapi.log.warn("app:w:order-controller: Rejected order update", { order: order.id, fields: update.fields }); return ctx.badRequest("Invalid order update", { errors: update.errors }); } - const data: CustomerUpdate & { customer?: ID } = { ...update.data }; + // A changed address or delivery method needs a new checkout: the PayPal order created before can no longer be captured. + const data: CustomerUpdate & { customer?: ID; paypalOrderId: null } = { ...update.data, paypalOrderId: null }; if (data.delivery !== undefined) { const delivery = await strapi.db.query("api::delivery.delivery").findOne({ where: { id: data.delivery }, select: ["id"] }); @@ -165,8 +175,9 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = strapi.log.info(`app:v:order-controller: - Adding product to cart ${order.id}`, { totalBefore: order.total }); + // A changed cart needs a new checkout: the PayPal order created before can no longer be captured. const orderUnsafe = await strapi.service("api::order.order").update(order.id, { - data: { cart: cart }, + data: { cart: cart, paypalOrderId: null }, ...orderDefaultParams }); strapi.log.info("app:v:order-controller: ✔ Adding product to cart", { totalAfter: orderUnsafe.total }); @@ -198,14 +209,19 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = updatedCart[existingIndex].count -= count; } + // A changed cart needs a new checkout: the PayPal order created before can no longer be captured. const orderUnsafe = await strapi.service("api::order.order").update(order.id, { - data: { cart: updatedCart }, + data: { cart: updatedCart, paypalOrderId: null }, ...orderDefaultParams }); strapi.log.info("app:v:order-controller: ✔ Removing product from cart", { totalAfter: order.total }); return await sanitize.contentAPI.output(orderUnsafe, strapi.getModel("api::order.order")); }, + /** + * Creates the PayPal order for the order and stores its id on the order: the capture accepts this PayPal order and no other. + * Answers PayPal's id and links, which the shop's PayPal button opens. + */ checkout: async (ctx) => { const { uuid } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); const { returnUrl } = await sanitize.contentAPI.query(ctx.query, strapi.getModel("api::order.order")); @@ -214,49 +230,57 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = ...orderDefaultParams }); if (!order) return ctx.notFound("Order not found"); + if (isLockedForCustomer(order)) return ctx.conflict(ORDER_LOCKED); - return await paypalApi.createSessionOrThrow(returnUrl as string, order); - }, + const missing = missingCheckoutFields(order); + if (missing.length > 0) return ctx.badRequest("Order is not ready for checkout", { missing }); - capturePayment: async (ctx) => { - const { uuid, paypalOrderId } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); - strapi.log.info(`app:i:order-controller: Capturing payment for order ${uuid} with PayPal order ${paypalOrderId}`); - - const order = await strapi.db.query("api::order.order").findOne({ - where: { uuid }, - ...orderDefaultParams - }); - if (!order) return ctx.notFound("Order not found"); - - if (order.paymentAuthorised) { - strapi.log.info(`app:i:order-controller: Payment already authorised for order ${uuid}`); - return { success: true, alreadyCaptured: true }; + let session: { id: string; links: unknown }; + try { + session = await paypalApi.createSessionOrThrow(returnUrl as string, order); + } catch (error) { + strapi.log.error("app:e:order-controller: Could not create the PayPal order", { order: order.id, error: describeError(error) }); + return ctx.badGateway("Could not create the PayPal order"); } - try { - // Capture the payment via PayPal server SDK - const captureResult = await paypalApi.checkPaymentStatus({ orderID: paypalOrderId as string }); + await strapi.db.query("api::order.order").update({ where: { id: order.id }, data: { paypalOrderId: session.id } }); + return { id: session.id, links: session.links }; + }, - if (captureResult.status === "COMPLETED") { - // Update order with payment info - const updatedOrder = await strapi.entityService.update("api::order.order", order.id, { - data: { - paymentAuthorised: true, - paymentStatus: "authorised", - paypalOrderId: paypalOrderId as string - }, - ...orderDefaultParams - }); + /** Captures the PayPal order the checkout stored and marks the order paid, see src/checkout/capture-payment.ts. */ + capturePayment: async (ctx) => { + const { uuid, paypalOrderId } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); - strapi.log.info(`app:i:order-controller: Payment captured successfully for order ${uuid}`); - return await sanitize.contentAPI.output(updatedOrder, strapi.getModel("api::order.order")); - } else { - strapi.log.error(`app:e:order-controller: Payment capture failed for order ${uuid}`, { captureResult }); - return ctx.badRequest("Payment capture failed", { status: captureResult.status }); + const result = await capturePayment( + { + findOrder: (orderUuid) => strapi.db.query("api::order.order").findOne({ where: { uuid: orderUuid } }), + getPayPalOrder: (id) => paypalApi.getOrder(id), + capturePayPalOrder: (id, requestId) => paypalApi.captureOrder(id, requestId), + // The where clause makes the write conditional: a cart or address change clears paypalOrderId, so an order changed + // during the capture is not found, and stays unpaid. Strapi's update answers null then and runs no afterUpdate. + updateOrder: (id, storedPayPalOrderId, data) => + strapi.db.query("api::order.order").update({ where: { id, paypalOrderId: storedPayPalOrderId }, data, ...orderDefaultParams }), + log: captureLog + }, + { + uuid: typeof uuid === "string" ? uuid : "", + paypalOrderId: typeof paypalOrderId === "string" ? paypalOrderId : "", + now: new Date() } - } catch (error) { - strapi.log.error(`app:e:order-controller: Error capturing payment for order ${uuid}`, { error }); - return ctx.internalServerError("Payment capture failed"); + ); + + switch (result.status) { + case 200: + if (result.alreadyCaptured === true) return { success: true, alreadyCaptured: true }; + return await sanitize.contentAPI.output(result.order, strapi.getModel("api::order.order")); + case 404: + return ctx.notFound(result.message); + case 409: + return ctx.conflict(result.message); + case 502: + return ctx.badGateway(result.message); + default: + return ctx.internalServerError(result.message); } }, diff --git a/src/services/PayPalApi.ts b/src/services/PayPalApi.ts index 304b392..c003e00 100644 --- a/src/services/PayPalApi.ts +++ b/src/services/PayPalApi.ts @@ -2,6 +2,7 @@ import { Client, Environment, LogLevel, OrdersController, OrderRequest } from "@ import { paypalClientId, paypalClientSecret, paypalEnvironment } from "../../config/constants"; import { Order } from "../../types"; import { buildPayPalOrderItems, buildPayPalOrderRequest } from "../checkout/paypal-order"; +import { describeError } from "../logging/error"; class PayPalApi { private readonly client: Client; @@ -24,7 +25,8 @@ class PayPalApi { this.ordersController = new OrdersController(this.client); } - async createSessionOrThrow(returnUrl: string, order: Order) { + /** Creates the PayPal order for the order and returns PayPal's id for it, which the capture accepts, and its links. */ + async createSessionOrThrow(returnUrl: string, order: Order): Promise<{ id: string; links: unknown }> { // Outside the try block, so these throws are not logged here. The items are computed but not sent. buildPayPalOrderItems(order); @@ -41,31 +43,32 @@ class PayPalApi { strapi.log.info("app:i:paypal-api: " + JSON.stringify({ session: sessionData })); + if (typeof sessionData?.id !== "string" || sessionData.id === "") throw new Error("PayPal answered without an order id"); return { id: sessionData.id, links: sessionData.links }; } catch (error) { - strapi.log.error("app:e:paypal-api: " + JSON.stringify({ error })); + // Never the error object: an SDK or axios error carries the request with its Authorization header. + strapi.log.error("app:e:paypal-api: Creating the PayPal order failed", { error: describeError(error) }); throw error; } } - async checkPaymentStatus(redirect: { orderID: string }) { - try { - const collect = { - id: redirect.orderID, - prefer: "return=minimal" - }; + /** PayPal's order as parsed JSON in PayPal's snake_case (GET /v2/checkout/orders/:id). Throws PayPal's and network errors. */ + async getOrder(id: string): Promise { + const { body } = await this.ordersController.ordersGet({ id }); + return JSON.parse(body as string); + } - const { body } = await this.ordersController.ordersCapture(collect); - const captureResult = JSON.parse(body as string); - strapi.log.info("app:i:paypal-api: " + JSON.stringify({ captureResult })); - - return captureResult; - } catch (error) { - return "/result/error"; - } + /** + * Captures the PayPal order and returns the whole order after the capture as parsed JSON in PayPal's snake_case + * (POST /v2/checkout/orders/:id/capture, Prefer: return=representation). PayPal answers a repeated requestId with the + * response of the first capture instead of capturing again. Throws PayPal's and network errors. + */ + async captureOrder(id: string, requestId: string): Promise { + const { body } = await this.ordersController.ordersCapture({ id, paypalRequestId: requestId, prefer: "return=representation" }); + return JSON.parse(body as string); } async handleWebhook(req: any) { diff --git a/tests/unit/customer-update.test.ts b/tests/unit/customer-update.test.ts index 9dd12af..6133477 100644 --- a/tests/unit/customer-update.test.ts +++ b/tests/unit/customer-update.test.ts @@ -46,12 +46,15 @@ const MAX = { }; const MAX_TEXT = "Max Mustermann\nHauptstraße 5\n10115 Berlin"; -// Every attribute of the order that only the server writes (src/api/order/content-types/order/schema.json), the fields Strapi -// manages, and the paypalOrderId the capture writes; with values a client would send to get a paid order for less. +// Every attribute of the order that only the server writes (src/api/order/content-types/order/schema.json), among them the PayPal +// order the checkout stores and the capture it verified, and the fields Strapi manages; with values a client would send to get a +// paid order for less. const SERVER_FIELDS = { paymentAuthorised: true, - paymentStatus: "authorised", + paymentStatus: "captured", paypalOrderId: "5O190127TN364715T", + paypalCaptureId: "3C679366HH908993F", + paymentCapturedAt: "2026-01-01T00:00:00.000Z", total: 0.01, subtotal: 0.01, VAT: 0, -- 2.36.6 From ab071cd222be9f25fd5eaf511a4a89ea4e4f50c5 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:41:34 +0200 Subject: [PATCH 11/16] fix(logging): write log fields without throwing and without credentials formatLogLine wrote the fields of a log call with JSON.stringify. A circular reference threw, so Strapi's error middleware lost the error it was logging: the axios error of a failed PayPal token request holds its request object, which refers to itself. An Error was written as {}, and the config.headers of an axios error carried "Authorization: Basic" with the base64 of the PayPal client id and secret into the line. The fields are now copied before they are written: - a field named like a credential (authorization, cookie, password, secret, token, api key) is written as "[redacted]", at any depth; - an Error is written as { name, message }, with the first line of the message as describeError has it, and its stack only with the stack option; - an object that contains itself is written once, the reference to it as "[Circular]"; - an instance of a class is written as its toJSON(), or as its class name without one: a request object keeps its headers in _header; - a bigint is written as its digits, an unreadable field as "[unreadable]". The three todo tests of the formatter pass and are regular tests now. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/logging/format.ts | 76 +++++++++++++++- tests/unit/log-error.test.ts | 2 +- tests/unit/log-format.test.ts | 167 ++++++++++++++++++++++++++-------- 3 files changed, 203 insertions(+), 42 deletions(-) diff --git a/src/logging/format.ts b/src/logging/format.ts index 693645a..254b264 100644 --- a/src/logging/format.ts +++ b/src/logging/format.ts @@ -1,3 +1,5 @@ +import { describeError } from "./error"; + /** What winston hands a printf template (logform's TransformableInfo); Strapi's default format has added `timestamp`. */ export type LogInfo = { level: string; @@ -5,10 +7,76 @@ export type LogInfo = { [key: string | symbol]: unknown; }; -/** One console line: `Z : `. */ -export const formatLogLine = ({ timestamp, level, message, ...rest }: LogInfo): string => { - let restString = JSON.stringify(rest, undefined, 2); +export type LogFormatOptions = { + /** Writes the stack of an error value too. config/logger.ts sets it at the debug level, the local one. */ + stack?: boolean; +}; + +// A field whose name matches holds a credential or a session, at any depth: an axios error carries the request headers in +// config.headers, among them "Authorization: Basic ". +const SENSITIVE_KEY = /authorization|cookie|password|secret|token|api[-_]?key/i; +const REDACTED = "[redacted]"; + +/** + * A copy of the value that JSON.stringify writes without throwing and without credentials: + * - a field whose name matches SENSITIVE_KEY becomes "[redacted]"; + * - an Error becomes { name, message } as describeError has them, the first line of the message only, plus the stack if asked; + * - an object that contains itself is written once, the reference to it as "[Circular]"; + * - an instance of a class other than Object and Array is written as its toJSON() if it has one, else as "[ClassName]": a request + * or socket object keeps the request headers, and with them the credentials, in fields such as _header; + * - a bigint, which JSON.stringify throws on, becomes its digits, and a value that cannot be read becomes "[unreadable]". + */ +const toLoggable = (value: unknown, options: LogFormatOptions, ancestors: object[]): unknown => { + if (typeof value === "bigint") return value.toString(); + if (typeof value !== "object" || value === null) return value; + if (ancestors.includes(value)) return "[Circular]"; + + ancestors.push(value); + try { + if (value instanceof Error) { + const { name, message } = describeError(value); + return options.stack && typeof value.stack === "string" ? { name, message, stack: value.stack } : { name, message }; + } + if (Array.isArray(value)) return value.map((item) => toLoggable(item, options, ancestors)); + + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + const { toJSON } = value as { toJSON?: unknown }; + if (typeof toJSON === "function") return toLoggable(toJSON.call(value), options, ancestors); + const name = (value as { constructor?: { name?: unknown } }).constructor?.name; + return `[${typeof name === "string" && name !== "" ? name : "object"}]`; + } + + const copy: Record = {}; + for (const key of Object.keys(value)) { + const field = (value as Record)[key]; + // Dropped as JSON.stringify drops them; a function named toJSON would also be called on the copy. + if (typeof field === "function") continue; + copy[key] = SENSITIVE_KEY.test(key) ? REDACTED : toLoggable(field, options, ancestors); + } + return copy; + } catch { + return "[unreadable]"; + } finally { + ancestors.pop(); + } +}; + +const asText = (value: unknown): string => { + try { + return String(value); + } catch { + return "[unreadable]"; + } +}; + +/** + * One console line: `Z : `. Never throws: a log call that + * throws loses the line, and in Strapi's error middleware the error it was logging. + */ +export const formatLogLine = ({ timestamp, level, message, ...rest }: LogInfo, options: LogFormatOptions = {}): string => { + let restString = JSON.stringify(toLoggable(rest, options, []), undefined, 2); restString = restString === "{}" ? "" : restString; - return `${timestamp}Z ${level}: ${message} ${restString}`; + return `${asText(timestamp)}Z ${asText(level)}: ${asText(message)} ${restString}`; }; diff --git a/tests/unit/log-error.test.ts b/tests/unit/log-error.test.ts index 2407e96..fa9df96 100644 --- a/tests/unit/log-error.test.ts +++ b/tests/unit/log-error.test.ts @@ -50,7 +50,7 @@ test("describes a thrown value that is not an error by its type alone", () => { }); test("the log line of the description holds no credentials", () => { - // Compare log-format.test.ts: the line of the error object itself holds them, a known defect of the formatter. + // The formatter now also keeps them out of the line of the error object itself, see log-format.test.ts. const line = formatLogLine({ timestamp: TIMESTAMP, level: "error", diff --git a/tests/unit/log-format.test.ts b/tests/unit/log-format.test.ts index 5265264..2a567ea 100644 --- a/tests/unit/log-format.test.ts +++ b/tests/unit/log-format.test.ts @@ -33,48 +33,141 @@ test("ignores the symbol keys winston adds", () => { assert.equal(formatLogLine(info), "2026-10-08 21:29:40.426Z info: hello "); }); -// Known defects: these assert the correct behaviour and fail today, which the todo marks as expected. +// Fixed defects of https://git.librete.ch/libretech/mp/issues/67. -test( - "does not throw on a circular reference", - { todo: "JSON.stringify throws on a circular reference — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - const request: Record = { method: "POST", path: "/v1/oauth2/token" }; - request.self = request; +test("does not throw on a circular reference", () => { + const request: Record = { method: "POST", path: "/v1/oauth2/token" }; + request.self = request; - assert.doesNotThrow(() => formatLogLine({ timestamp: TIMESTAMP, level: "error", message: "app:e:paypal-api", request })); + assert.doesNotThrow(() => formatLogLine({ timestamp: TIMESTAMP, level: "error", message: "app:e:paypal-api", request })); +}); + +test("does not write the Authorization header of an axios-like error into the line", () => { + const error = Object.assign(new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"), { + name: "AxiosError", + code: "EAI_AGAIN", + config: { method: "post", url: "https://api-m.paypal.com/v1/oauth2/token", headers: { Authorization: `Basic ${CREDENTIALS}` } } + }); + // What strapi.log.error(error) hands this formatter: Strapi's logErrors format spreads the error and appends its stack. + const line = formatLogLine({ ...error, message: `${error.message}\n${error.stack}`, level: "error", timestamp: TIMESTAMP }); + + assert.ok(!line.includes(CREDENTIALS), line); +}); + +test("renders an Error with its message instead of {}", () => { + const error = new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"); + // strapi.log.error("…", { error }), as in the order controller's capturePayment + const line = formatLogLine({ + timestamp: TIMESTAMP, + level: "error", + message: "app:e:order-controller: Error capturing payment for order x", + error + }); + + assert.ok(line.includes("getaddrinfo EAI_AGAIN api-m.paypal.com"), line); +}); + +// Safe serialisation. + +const fieldsOf = (line: string) => JSON.parse(line.slice(line.indexOf("{"))); + +test("redacts the fields named like a credential at any depth, whatever their case", () => { + const line = formatLogLine({ + timestamp: TIMESTAMP, + level: "debug", + message: "app:d:test", + token: "A21AAF…", + request: { + method: "POST", + headers: { Authorization: `Basic ${CREDENTIALS}`, Cookie: "koa.sess=abc", "X-API-Key": "k1", "Content-Type": "application/json" }, + body: { grant_type: "client_credentials", client_secret: "s3cr3t", user: { password: "hunter2", api_key: "k2", apikey: "k3" } }, + responses: [{ headers: { "set-cookie": ["koa.sess=def"] }, data: { access_token: "A21AAG…", expires_in: 32400 } }] + } + }); + + assert.deepEqual(fieldsOf(line), { + token: "[redacted]", + request: { + method: "POST", + headers: { Authorization: "[redacted]", Cookie: "[redacted]", "X-API-Key": "[redacted]", "Content-Type": "application/json" }, + body: { + grant_type: "client_credentials", + client_secret: "[redacted]", + user: { password: "[redacted]", api_key: "[redacted]", apikey: "[redacted]" } + }, + responses: [{ headers: { "set-cookie": "[redacted]" }, data: { access_token: "[redacted]", expires_in: 32400 } }] + } + }); +}); + +test("writes the other fields of an object that contains itself, and the reference to it as [Circular]", () => { + const request: Record = { method: "POST", path: "/v1/oauth2/token" }; + request.socket = { request, remoteAddress: "151.101.1.21" }; + const line = formatLogLine({ timestamp: TIMESTAMP, level: "error", message: "app:e:paypal-api", request, list: [request] }); + + assert.deepEqual(fieldsOf(line), { + request: { method: "POST", path: "/v1/oauth2/token", socket: { request: "[Circular]", remoteAddress: "151.101.1.21" } }, + list: [{ method: "POST", path: "/v1/oauth2/token", socket: { request: "[Circular]", remoteAddress: "151.101.1.21" } }] + }); +}); + +test("writes an object that is referenced twice, but not by itself, both times", () => { + const delivery = { id: 1, price: 4.95 }; + + assert.deepEqual(fieldsOf(formatLogLine({ timestamp: TIMESTAMP, level: "info", message: "m", before: delivery, after: delivery })), { + before: { id: 1, price: 4.95 }, + after: { id: 1, price: 4.95 } + }); +}); + +test("writes an Error as its name and the first line of its message, and its stack only when asked", () => { + const error = Object.assign(new Error('The response did not match the response schema.\n\nGiven value: {"email_address":"erika@example.org"}'), { + request: { url: "https://api-m.paypal.com/v2/checkout/orders", headers: { Authorization: "Bearer A21AAF…" } }, + statusCode: 201 + }); + const info = { timestamp: TIMESTAMP, level: "error", message: "app:e:paypal-api", error }; + + assert.deepEqual(fieldsOf(formatLogLine(info)), { error: { name: "Error", message: "The response did not match the response schema." } }); + const withStack = fieldsOf(formatLogLine(info, { stack: true })); + assert.equal(withStack.error.stack, error.stack); + assert.deepEqual(Object.keys(withStack.error), ["name", "message", "stack"]); +}); + +test("writes an instance of a class without toJSON as its class name, so the headers a request object keeps stay out", () => { + class ClientRequest { + _header = `POST /v1/oauth2/token HTTP/1.1\r\nAuthorization: Basic ${CREDENTIALS}\r\n\r\n`; + method = "POST"; } -); + const line = formatLogLine({ timestamp: TIMESTAMP, level: "error", message: "m", request: new ClientRequest(), seen: new Set([1]) }); -test( - "does not write the Authorization header of an axios-like error into the line", - { todo: "the PayPal client credentials are logged — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - const error = Object.assign(new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"), { - name: "AxiosError", - code: "EAI_AGAIN", - config: { method: "post", url: "https://api-m.paypal.com/v1/oauth2/token", headers: { Authorization: `Basic ${CREDENTIALS}` } } - }); - // What strapi.log.error(error) hands this formatter: Strapi's logErrors format spreads the error and appends its stack. - const line = formatLogLine({ ...error, message: `${error.message}\n${error.stack}`, level: "error", timestamp: TIMESTAMP }); + assert.deepEqual(fieldsOf(line), { request: "[ClientRequest]", seen: "[Set]" }); + assert.ok(!line.includes(CREDENTIALS), line); +}); - assert.ok(!line.includes(CREDENTIALS), line); +test("writes an instance of a class with toJSON as what toJSON returns, redacted", () => { + // Like axios's AxiosHeaders, which an axios error carries in config.headers. + class AxiosHeaders { + toJSON() { + return { Accept: "application/json", Authorization: `Basic ${CREDENTIALS}` }; + } } -); + const at = new Date("2026-10-08T19:13:05.000Z"); + const line = formatLogLine({ timestamp: TIMESTAMP, level: "error", message: "m", config: { headers: new AxiosHeaders() }, at }); -test( - "renders an Error with its message instead of {}", - { todo: "an Error is rendered as {} — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - const error = new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"); - // strapi.log.error("…", { error }), as in the order controller's capturePayment - const line = formatLogLine({ - timestamp: TIMESTAMP, - level: "error", - message: "app:e:order-controller: Error capturing payment for order x", - error - }); + assert.deepEqual(fieldsOf(line), { + config: { headers: { Accept: "application/json", Authorization: "[redacted]" } }, + at: "2026-10-08T19:13:05.000Z" + }); +}); - assert.ok(line.includes("getaddrinfo EAI_AGAIN api-m.paypal.com"), line); - } -); +test("writes a bigint as its digits and a field that cannot be read as [unreadable]", () => { + const unreadable = { + get value() { + throw new Error("no"); + } + }; + // The PayPal SDK keeps an OAuth token's expiry as a bigint, which JSON.stringify throws on. + const line = formatLogLine({ timestamp: TIMESTAMP, level: "debug", message: "m", expiry: 1791587585n, unreadable }); + + assert.deepEqual(fieldsOf(line), { expiry: "1791587585", unreadable: "[unreadable]" }); +}); -- 2.36.6 From c560de8ec0a73721ad6cbb33440c9ea5985193d5 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:42:17 +0200 Subject: [PATCH 12/16] fix(logging): log at info in production instead of silly Strapi 4.25 builds its logger from @strapi/logger's default configuration with config/logger.ts assigned over it. That default level is "silly", and config/logger.ts set none, so production wrote every level: the logger of the v0.1.1 image, built as Strapi builds it with NODE_ENV=production, reports level silly. Debug and silly lines carry order data, such as the PayPal order request with the buyer's name and address. The level is now info for NODE_ENV=production, which mp's compose.yml sets for the cms, and debug in every other environment. Strapi's request log lines are written at http, below info, so production no longer writes them. The stack of an error field is written at debug only. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- config/logger.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/config/logger.ts b/config/logger.ts index ece6c76..4215811 100644 --- a/config/logger.ts +++ b/config/logger.ts @@ -1,10 +1,17 @@ import { winston } from "@strapi/logger"; import { formatLogLine } from "../src/logging/format"; +// Strapi 4.25 creates its logger from @strapi/logger's default configuration with this object assigned over it +// (Strapi.js: createLogger(this.config.get("logger"))). Its default level is "silly", which writes every line, also the debug and +// silly lines with order data such as names and addresses. Production (NODE_ENV=production in mp's compose.yml) writes info and +// above, any other environment debug and above. +const level = process.env.NODE_ENV === "production" ? "info" : "debug"; + export default { + level, transports: [ new winston.transports.Console({ - format: winston.format.combine(winston.format.printf(formatLogLine)) + format: winston.format.combine(winston.format.printf((info) => formatLogLine(info, { stack: level === "debug" }))) }) ] }; -- 2.36.6 From 9e94d69e2edbb6592511531959ed15b79d8811ca Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:48:34 +0200 Subject: [PATCH 13/16] fix(checkout): reject an order PayPal cannot take with a typed error buildPayPalOrderRequest split the text delivery address at fixed offsets and called toFixed on the delivery price, and the items that were computed but never sent read the product of every cart line. A missing or two-line delivery address, a delivery method whose price is null, such as a pickup, and a cart line whose product was deleted each threw a TypeError, which the checkout answered 502 (D1 to D4). The request is now built in whole cents, with the shipping address from the first one entered of the structured delivery address, the structured invoice address and the text delivery address: - fullName is givenName and familyName, addressLine1 streetAddress, adminArea2 addressLevel2, countryCode the country upper-cased or DE; a structured address that is entered but incomplete is rejected instead of replaced by another address; - a text address needs three lines: the name, the street, and " " split at the first whitespace; lines between the name and the street become addressLine2; - a delivery price that is null counts as 0; - itemTotal is the net of the subtotal, round(subtotal cents * 100 / 119), taxTotal the order's VAT. The breakdown must add up to the total to the cent, as PayPal requires, so totals computed before a price changed are rejected here instead of by PayPal. The items are gone. With items, PayPal requires item_total and tax_total to be the sums of the amounts rounded per unit, which miss the net and the VAT of the subtotal that the invoice states: 3 x 12.50 gives 31.50 and 6.00, the subtotal 31.51 and 5.99. An empty cart, a missing total or VAT, totals that do not add up and a missing or unusable address throw CheckoutNotReadyError, which the checkout answers 400 "Order is not ready for checkout" with the reason. A deleted product throws ProductUnavailableError, answered 409 "A product in the cart is no longer available". Both are thrown before PayPal is asked and are logged as warnings; other errors stay 502. Tests: D1 to D4 are regular tests. The test of an order without a total expects CheckoutNotReadyError instead of a plain Error, and the item test is replaced by one that shows why no items are sent. The order of paypal-verify.test.ts gains the cart line the builder now requires, and its amount test builds the request for every total up to 3000.00 instead of assuming toFixed(2). Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/api/order/controllers/order.ts | 21 ++- src/checkout/paypal-order.ts | 288 ++++++++++++++++++++--------- src/services/PayPalApi.ts | 20 +- tests/unit/paypal-order.test.ts | 258 ++++++++++++++++++++------ tests/unit/paypal-verify.test.ts | 10 +- 5 files changed, 434 insertions(+), 163 deletions(-) diff --git a/src/api/order/controllers/order.ts b/src/api/order/controllers/order.ts index d68a0c2..6d45cfe 100644 --- a/src/api/order/controllers/order.ts +++ b/src/api/order/controllers/order.ts @@ -10,6 +10,7 @@ import type { CustomerUpdate } from "../../../checkout/customer-update"; import { isLockedForCustomer, missingCheckoutFields } from "../../../checkout/order-state"; import { capturePayment } from "../../../checkout/capture-payment"; import type { CaptureLog } from "../../../checkout/capture-payment"; +import { CHECKOUT_NOT_READY, checkoutFailure } from "../../../checkout/paypal-order"; import { describeError } from "../../../logging/error"; const ORDER_LOCKED = "Order can no longer be changed"; @@ -220,7 +221,8 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = /** * Creates the PayPal order for the order and stores its id on the order: the capture accepts this PayPal order and no other. - * Answers PayPal's id and links, which the shop's PayPal button opens. + * Answers PayPal's id and links, which the shop's PayPal button opens. An order PayPal cannot take, such as one without a usable + * delivery address, is answered 400 with the reason, a cart with a deleted product 409, and a PayPal or network error 502. */ checkout: async (ctx) => { const { uuid } = await sanitize.contentAPI.query(ctx.params, strapi.getModel("api::order.order")); @@ -233,14 +235,25 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = if (isLockedForCustomer(order)) return ctx.conflict(ORDER_LOCKED); const missing = missingCheckoutFields(order); - if (missing.length > 0) return ctx.badRequest("Order is not ready for checkout", { missing }); + if (missing.length > 0) return ctx.badRequest(CHECKOUT_NOT_READY, { missing }); let session: { id: string; links: unknown }; try { session = await paypalApi.createSessionOrThrow(returnUrl as string, order); } catch (error) { - strapi.log.error("app:e:order-controller: Could not create the PayPal order", { order: order.id, error: describeError(error) }); - return ctx.badGateway("Could not create the PayPal order"); + const failure = checkoutFailure(error); + const fields = { order: order.id, error: describeError(error) }; + switch (failure.status) { + case 400: + strapi.log.warn("app:w:order-controller: The order cannot be checked out as it is", fields); + return ctx.badRequest(failure.message, { reason: failure.reason }); + case 409: + strapi.log.warn("app:w:order-controller: A product in the cart is no longer available", fields); + return ctx.conflict(failure.message); + default: + strapi.log.error("app:e:order-controller: Could not create the PayPal order", fields); + return ctx.badGateway(failure.message); + } } await strapi.db.query("api::order.order").update({ where: { id: order.id }, data: { paypalOrderId: session.id } }); diff --git a/src/checkout/paypal-order.ts b/src/checkout/paypal-order.ts index 7cf509c..304214d 100644 --- a/src/checkout/paypal-order.ts +++ b/src/checkout/paypal-order.ts @@ -1,20 +1,28 @@ -import type { Order } from "../../types"; -import { calculateTotalProductPrice } from "./pricing"; +import type { CustomerStructuredAddress } from "./customer-update"; import { vatIncludedDecimal } from "./vat"; // The PayPal order that PayPalApi.createSessionOrThrow creates, built without the PayPal SDK so it can be unit tested. // String literals stand in for the SDK enums, with the values of @paypal/paypal-server-sdk 0.6.1. +// +// The request sends no items. With items, PayPal requires item_total to be the sum of unit_amount × quantity and tax_total the +// sum of tax × quantity, and amounts rounded per unit miss the net and the VAT of the subtotal, which the invoice states: +// 3 × 12.50 gives 3 × 10.50 = 31.50 net and 3 × 2.00 = 6.00 tax, the subtotal of 37.50 gives 31.51 net and 5.99 VAT. export type PayPalMoney = { currencyCode: string; value: string; }; -export type PayPalOrderItem = { - name: string; - unitAmount: PayPalMoney; - tax: PayPalMoney; - quantity: string; +export type PayPalShipping = { + name: { fullName: string }; + address: { addressLine1: string; addressLine2?: string; postalCode: string; adminArea2: string; countryCode: string }; +}; + +export type PayPalBreakdown = { + itemTotal: PayPalMoney; + taxTotal: PayPalMoney; + /** Undefined for an order without a delivery method. */ + shipping: PayPalMoney | undefined; }; export type PayPalOrderRequest = { @@ -22,17 +30,8 @@ export type PayPalOrderRequest = { intent: "CAPTURE"; purchaseUnits: { referenceId: string; - amount: PayPalMoney & { - breakdown: { - itemTotal: PayPalMoney; - taxTotal: PayPalMoney; - shipping: PayPalMoney | undefined; - }; - }; - shipping: { - name: { fullName: string }; - address: { addressLine1: string; postalCode: string; adminArea2: string; countryCode: string }; - }; + amount: PayPalMoney & { breakdown: PayPalBreakdown }; + shipping: PayPalShipping; customId: string; invoiceId: string; }[]; @@ -45,80 +44,195 @@ export type PayPalOrderRequest = { }; /** - * The part of PayPalApi.createSessionOrThrow that runs before its try block, so nothing thrown here is logged there. - * Rejects an order without a total, then computes one item per cart line. The items are not sent to PayPal, but computing - * them throws a TypeError for a cart line without a product. + * The fields of the order the request is built from, as the checkout reads the order (strapi.db.query findOne with + * orderDefaultParams). Each of them may be null. Prices are decimals, which Strapi returns as numbers. */ -export const buildPayPalOrderItems = (order: Order): PayPalOrderItem[] => { - if (!order.total) throw new Error("Cart is empty or has no total"); +export type PayPalOrderSource = { + uuid: string; + invoiceNumber?: string | null; + subtotal?: number | string | null; + total?: number | string | null; + VAT?: number | string | null; + delivery?: { price?: number | string | null } | null; + cart?: ({ product?: unknown } | null)[] | null; + deliveryAddress?: string | null; + deliveryAddressStructured?: CustomerStructuredAddress | null; + invoiceAddressStructured?: CustomerStructuredAddress | null; +}; - return order.cart.map((item) => { - const totalProductPrice = calculateTotalProductPrice(item.product); - const amount = totalProductPrice / vatIncludedDecimal; - const tax = totalProductPrice - amount; +/** The order lacks something PayPal needs, or its totals do not add up, so the customer has to complete or change it first. */ +export class CheckoutNotReadyError extends Error { + constructor(message: string) { + super(message); + this.name = "CheckoutNotReadyError"; + } +} - return { - name: item.product.name, - unitAmount: { - currencyCode: "EUR", - value: amount.toFixed(2) - }, - tax: { - currencyCode: "EUR", - value: tax.toFixed(2) - }, - quantity: item.count.toString() - }; - }); +/** A cart line whose product was deleted. */ +export class ProductUnavailableError extends Error { + constructor(message: string) { + super(message); + this.name = "ProductUnavailableError"; + } +} + +export const CHECKOUT_NOT_READY = "Order is not ready for checkout"; +export const PRODUCT_UNAVAILABLE = "A product in the cart is no longer available"; +export const PAYPAL_ORDER_FAILED = "Could not create the PayPal order"; + +export type CheckoutFailure = { status: 400; message: string; reason: string } | { status: 409 | 502; message: string }; + +/** + * The checkout's answer to an error of PayPalApi.createSessionOrThrow: 400 with the reason for an order that is not ready, 409 for a + * product that is no longer available. Any other error is PayPal's or the network's: 502. + */ +export const checkoutFailure = (error: unknown): CheckoutFailure => { + if (error instanceof CheckoutNotReadyError) return { status: 400, message: CHECKOUT_NOT_READY, reason: error.message }; + if (error instanceof ProductUnavailableError) return { status: 409, message: PRODUCT_UNAVAILABLE }; + return { status: 502, message: PAYPAL_ORDER_FAILED }; +}; + +// Money is computed in whole cents. + +/** A decimal amount of euros as whole cents, or null for anything that is not a finite number. */ +const toCents = (amount: unknown): number | null => { + const value = typeof amount === "string" && amount.trim() !== "" ? Number(amount) : amount; + return typeof value === "number" && Number.isFinite(value) ? Math.round(value * 100) : null; +}; + +/** Whole, non-negative cents as PayPal money: 1504 is "15.04". */ +const money = (cents: number): PayPalMoney => ({ + currencyCode: "EUR", + value: `${Math.floor(cents / 100)}.${String(cents % 100).padStart(2, "0")}` +}); + +// vatIncludedDecimal as a whole percentage (119), so that the net of the subtotal is a quotient of whole numbers. +const VAT_INCLUDED_PERCENT = Math.round(vatIncludedDecimal * 100); + +/** + * The breakdown of the order's total: the net of its subtotal, its VAT and the price of its delivery method, 0 when that price + * is not set. PayPal rejects a breakdown that does not add up to the amount, which happens when a price changed after the order + * service computed the totals, so such an order is rejected here. + */ +const breakdownFor = (order: PayPalOrderSource, totalCents: number): PayPalBreakdown => { + const subtotalCents = toCents(order.subtotal); + const taxCents = toCents(order.VAT); + if (subtotalCents === null || taxCents === null) throw new CheckoutNotReadyError("The order has no subtotal or no VAT"); + + const itemTotalCents = Math.round((subtotalCents * 100) / VAT_INCLUDED_PERCENT); + const shippingCents = order.delivery ? (toCents(order.delivery.price) ?? 0) : undefined; + + const parts = [itemTotalCents, taxCents, shippingCents ?? 0]; + const sum = parts.reduce((a, b) => a + b, 0); + if (parts.some((cents) => cents < 0) || sum !== totalCents) { + throw new CheckoutNotReadyError( + `The order's totals do not add up: total ${totalCents} cents, items ${itemTotalCents} + VAT ${taxCents} + shipping ${shippingCents ?? 0}` + ); + } + + return { + itemTotal: money(itemTotalCents), + taxTotal: money(taxCents), + shipping: shippingCents === undefined ? undefined : money(shippingCents) + }; +}; + +const text = (value: unknown): string => (typeof value === "string" ? value.trim() : ""); + +const STRUCTURED_COMPONENTS = ["givenName", "familyName", "streetAddress", "postalCode", "addressLevel2"] as const; + +/** Whether any component besides the country is set: Strapi stores the country with the default DE. */ +const isEntered = (address: CustomerStructuredAddress | null | undefined): boolean => + typeof address === "object" && address !== null && STRUCTURED_COMPONENTS.some((key) => text(address[key]) !== ""); + +/** The shipping address from a structured address, which needs a name, a street, a postal code and a city. */ +const fromStructured = (field: string, address: CustomerStructuredAddress): PayPalShipping => { + const fullName = [text(address.givenName), text(address.familyName)].filter((part) => part !== "").join(" "); + const addressLine1 = text(address.streetAddress); + const postalCode = text(address.postalCode); + const adminArea2 = text(address.addressLevel2); + const countryCode = (text(address.country) || "DE").toUpperCase(); + + const lacking = [ + [fullName, "givenName and familyName"], + [addressLine1, "streetAddress"], + [postalCode, "postalCode"], + [adminArea2, "addressLevel2"] + ] + .filter(([value]) => value === "") + .map(([, component]) => component); + if (lacking.length > 0) throw new CheckoutNotReadyError(`${field} lacks ${lacking.join(", ")}`); + if (!/^[A-Z]{2}$/.test(countryCode)) throw new CheckoutNotReadyError(`${field} has no two-letter country code`); + + return { name: { fullName }, address: { addressLine1, postalCode, adminArea2, countryCode } }; }; /** - * The order request body PayPalApi.createSessionOrThrow sends. It is built inside that try block, so whatever throws here is logged there. + * The shipping address from a text address as the shop writes it (libreshop/shop pages/checkout/2.vue, addressToString): the name, + * the street and " " on lines of their own. Lines between the name and the street, such as c/o, become + * addressLine2. Blank lines are skipped. The country is DE. */ -export const buildPayPalOrderRequest = (returnUrl: string, order: Order): PayPalOrderRequest => ({ - intent: "CAPTURE", - purchaseUnits: [ - { - // items: computed by buildPayPalOrderItems, but not sent - referenceId: order.uuid, - amount: { - currencyCode: "EUR", - value: order.total.toFixed(2), - breakdown: { - itemTotal: { - currencyCode: "EUR", - value: (Math.round((order.subtotal / vatIncludedDecimal) * 100) / 100).toFixed(2) - }, - taxTotal: { - currencyCode: "EUR", - value: order.VAT.toFixed(2) - }, - shipping: order.delivery - ? { - currencyCode: "EUR", - value: order.delivery.price.toFixed(2) - } - : undefined - } - }, - shipping: { - name: { - fullName: order.deliveryAddress.split("\n")[0] - }, - address: { - addressLine1: order.deliveryAddress.split("\n")[1], - postalCode: order.deliveryAddress.split("\n")[2].slice(0, 5), - adminArea2: order.deliveryAddress.split("\n")[2].slice(6), - countryCode: "DE" - } - }, - customId: order.uuid, - invoiceId: order.invoiceNumber +const fromText = (field: string, address: string): PayPalShipping => { + const lines = address + .split(/\r\n|\r|\n/) + .map((line) => line.trim()) + .filter((line) => line !== ""); + if (lines.length < 3) throw new CheckoutNotReadyError(`${field} needs the name, the street, and the postal code and city on lines of their own`); + + const city = /^(\S+)\s+(\S.*)$/.exec(lines[lines.length - 1]); + if (city === null) throw new CheckoutNotReadyError(`The last line of ${field} is not the postal code and the city`); + + const addressLine2 = lines.slice(1, -2).join(", "); + return { + name: { fullName: lines[0] }, + address: { + addressLine1: lines[lines.length - 2], + ...(addressLine2 !== "" ? { addressLine2 } : {}), + postalCode: city[1], + adminArea2: city[2], + countryCode: "DE" } - ], - applicationContext: { - returnUrl: returnUrl, - cancelUrl: returnUrl, - shippingPreference: "SET_PROVIDED_ADDRESS" - } -}); + }; +}; + +/** + * The address PayPal shows the buyer as the shipping address: the structured delivery address, else the structured invoice address, + * else the text delivery address. The shop sends the invoice address as the delivery address when the buyer gives no other. An + * address that is entered but incomplete is rejected rather than replaced by the next one. + */ +const shippingFor = (order: PayPalOrderSource): PayPalShipping => { + if (isEntered(order.deliveryAddressStructured)) return fromStructured("deliveryAddressStructured", order.deliveryAddressStructured); + if (isEntered(order.invoiceAddressStructured)) return fromStructured("invoiceAddressStructured", order.invoiceAddressStructured); + if (text(order.deliveryAddress) !== "") return fromText("deliveryAddress", order.deliveryAddress); + throw new CheckoutNotReadyError("The order has no delivery address"); +}; + +/** + * The order request body PayPalApi.createSessionOrThrow sends. Throws ProductUnavailableError for a cart line whose product was + * deleted, and CheckoutNotReadyError for an empty cart, a missing total, totals that do not add up or a missing or unusable + * delivery address. The messages name fields and amounts, never an address. + */ +export const buildPayPalOrderRequest = (returnUrl: string, order: PayPalOrderSource): PayPalOrderRequest => { + const cart = Array.isArray(order.cart) ? order.cart : []; + const totalCents = toCents(order.total); + if (cart.length === 0 || totalCents === null || totalCents <= 0) throw new CheckoutNotReadyError("Cart is empty or has no total"); + if (cart.some((line) => !line?.product)) throw new ProductUnavailableError(PRODUCT_UNAVAILABLE); + + return { + intent: "CAPTURE", + purchaseUnits: [ + { + referenceId: order.uuid, + amount: { ...money(totalCents), breakdown: breakdownFor(order, totalCents) }, + shipping: shippingFor(order), + customId: order.uuid, + invoiceId: order.invoiceNumber + } + ], + applicationContext: { + returnUrl: returnUrl, + cancelUrl: returnUrl, + shippingPreference: "SET_PROVIDED_ADDRESS" + } + }; +}; diff --git a/src/services/PayPalApi.ts b/src/services/PayPalApi.ts index c003e00..00c7415 100644 --- a/src/services/PayPalApi.ts +++ b/src/services/PayPalApi.ts @@ -1,7 +1,7 @@ import { Client, Environment, LogLevel, OrdersController, OrderRequest } from "@paypal/paypal-server-sdk"; import { paypalClientId, paypalClientSecret, paypalEnvironment } from "../../config/constants"; -import { Order } from "../../types"; -import { buildPayPalOrderItems, buildPayPalOrderRequest } from "../checkout/paypal-order"; +import { buildPayPalOrderRequest } from "../checkout/paypal-order"; +import type { PayPalOrderSource } from "../checkout/paypal-order"; import { describeError } from "../logging/error"; class PayPalApi { @@ -25,16 +25,16 @@ class PayPalApi { this.ordersController = new OrdersController(this.client); } - /** Creates the PayPal order for the order and returns PayPal's id for it, which the capture accepts, and its links. */ - async createSessionOrThrow(returnUrl: string, order: Order): Promise<{ id: string; links: unknown }> { - // Outside the try block, so these throws are not logged here. The items are computed but not sent. - buildPayPalOrderItems(order); + /** + * Creates the PayPal order for the order and returns PayPal's id for it, which the capture accepts, and its links. Throws + * CheckoutNotReadyError or ProductUnavailableError before PayPal is asked, for an order PayPal cannot take. + */ + async createSessionOrThrow(returnUrl: string, order: PayPalOrderSource): Promise<{ id: string; links: unknown }> { + // Outside the try block: an order that is not ready is the customer's to fix, not a PayPal failure. + const request = buildPayPalOrderRequest(returnUrl, order) as OrderRequest; try { - const collect = { - body: buildPayPalOrderRequest(returnUrl, order) as OrderRequest, - prefer: "return=minimal" - }; + const collect = { body: request, prefer: "return=minimal" }; strapi.log.silly("app:d:paypal-api: Creating PayPal order " + JSON.stringify({ collect })); diff --git a/tests/unit/paypal-order.test.ts b/tests/unit/paypal-order.test.ts index 104baea..b659567 100644 --- a/tests/unit/paypal-order.test.ts +++ b/tests/unit/paypal-order.test.ts @@ -1,6 +1,12 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { buildPayPalOrderItems, buildPayPalOrderRequest } from "../../src/checkout/paypal-order.ts"; +import { + CheckoutNotReadyError, + ProductUnavailableError, + PRODUCT_UNAVAILABLE, + buildPayPalOrderRequest, + checkoutFailure +} from "../../src/checkout/paypal-order.ts"; import { calculateTotalProductPrice } from "../../src/checkout/pricing.ts"; import { vatDecimal, vatIncludedDecimal } from "../../src/checkout/vat.ts"; @@ -43,11 +49,8 @@ const orderFor = (lines: [number, number][], deliveryPrice: number | null) => { }); }; -// Both halves in the order createSessionOrThrow runs them: what it would send to PayPal, or what it would throw. -const buildSession = (order) => { - buildPayPalOrderItems(order); - return buildPayPalOrderRequest(RETURN_URL, order); -}; +// What createSessionOrThrow would send to PayPal, or what it would throw before asking PayPal. +const buildSession = (order) => buildPayPalOrderRequest(RETURN_URL, order); const cents = (money) => (money ? Math.round(Number(money.value) * 100) : 0); @@ -71,6 +74,15 @@ const assertNoTypeError = (build: () => unknown) => { } }; +const thrown = (build: () => unknown): unknown => { + try { + build(); + } catch (error) { + return error; + } + assert.fail("did not throw"); +}; + // Characterization: what the builders do today. test("builds the PayPal order request for a valid order", () => { @@ -117,71 +129,51 @@ test("sends no shipping amount when the order has no delivery method", () => { }); }); -test("rejects an order without a total before computing the items", () => { - for (const total of [0, null, undefined]) { - assert.throws(() => buildPayPalOrderItems(validOrder({ total })), { name: "Error", message: "Cart is empty or has no total" }); +// The request used to be built after a list of items that was never sent; computing it threw "Cart is empty or has no total". +// That check now throws CheckoutNotReadyError, which the checkout answers 400 instead of 502. +test("rejects an empty cart or an order without a total as not ready for checkout", () => { + for (const overrides of [{ total: 0 }, { total: null }, { total: undefined }, { cart: [] }, { cart: null }]) { + assert.throws(() => buildSession(validOrder(overrides)), { name: "CheckoutNotReadyError", message: "Cart is empty or has no total" }); } }); -test("computes one item per cart line, which is not sent to PayPal", () => { - const order = validOrder({ - cart: [ - { id: 1, count: 1, product: notebook }, - { id: 2, count: 3, product: { id: 102, name: "Journaling Booklet", cover: { price: 12.5 } } } - ] - }); +test("sends no items: amounts rounded per unit would miss the net and the VAT of the subtotal", () => { + // 3 × 12.50: per unit 10.50 net and 2.00 tax, 31.50 and 6.00 for the three; the subtotal of 37.50 has 31.51 net and 5.99 VAT. + const unitNetCents = Math.round((1250 * 100) / 119); + const unit = buildSession(orderFor([[12.5, 3]], 4.95)).purchaseUnits[0]; - assert.deepEqual(buildPayPalOrderItems(order), [ - { - name: "Notizbuch A5 Punktraster", - unitAmount: { currencyCode: "EUR", value: "15.04" }, - tax: { currencyCode: "EUR", value: "2.86" }, - quantity: "1" - }, - { - name: "Journaling Booklet", - unitAmount: { currencyCode: "EUR", value: "10.50" }, - tax: { currencyCode: "EUR", value: "2.00" }, - quantity: "3" + assert.equal("items" in unit, false); + assert.deepEqual([3 * unitNetCents, 3 * (1250 - unitNetCents)], [3150, 600]); + assert.deepEqual(unit.amount, { + currencyCode: "EUR", + value: "42.45", + breakdown: { + itemTotal: { currencyCode: "EUR", value: "31.51" }, + taxTotal: { currencyCode: "EUR", value: "5.99" }, + shipping: { currencyCode: "EUR", value: "4.95" } } - ]); + }); }); -// Known defects: these assert the correct behaviour and fail today, which the todo marks as expected. +// Fixed defects of https://git.librete.ch/libretech/mp/issues/67. -test( - "D1: a missing delivery address is rejected with a clear validation error", - { todo: "null deliveryAddress throws a TypeError — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - assert.throws(() => buildSession(validOrder({ deliveryAddress: null })), isClearError(/address/i)); - } -); +test("D1: a missing delivery address is rejected with a clear validation error", () => { + assert.throws(() => buildSession(validOrder({ deliveryAddress: null })), isClearError(/address/i)); +}); -test( - "D2: a two-line delivery address does not throw a TypeError", - { todo: "a two-line address throws a TypeError — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - assertNoTypeError(() => buildSession(validOrder({ deliveryAddress: "Erika Mustermann\nMusterstraße 1" }))); - } -); +test("D2: a two-line delivery address does not throw a TypeError", () => { + assertNoTypeError(() => buildSession(validOrder({ deliveryAddress: "Erika Mustermann\nMusterstraße 1" }))); +}); -test( - "D3: a pickup whose delivery price is null is sent with shipping 0.00", - { todo: "delivery price null throws a TypeError — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - const order = validOrder({ delivery: { id: 2, name: "Abholung", price: null }, total: 17.9 }); +test("D3: a pickup whose delivery price is null is sent with shipping 0.00", () => { + const order = validOrder({ delivery: { id: 2, name: "Abholung", price: null }, total: 17.9 }); - assert.deepEqual(buildSession(order).purchaseUnits[0].amount.breakdown.shipping, { currencyCode: "EUR", value: "0.00" }); - } -); + assert.deepEqual(buildSession(order).purchaseUnits[0].amount.breakdown.shipping, { currencyCode: "EUR", value: "0.00" }); +}); -test( - "D4: a cart line whose product was deleted is rejected with a clear error", - { todo: "a deleted product throws a TypeError — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - assert.throws(() => buildSession(validOrder({ cart: [{ id: 1, count: 1, product: null }] })), isClearError(/product/i)); - } -); +test("D4: a cart line whose product was deleted is rejected with a clear error", () => { + assert.throws(() => buildSession(validOrder({ cart: [{ id: 1, count: 1, product: null }] })), isClearError(/product/i)); +}); test( "D5: with a payment surcharge the breakdown still adds up to the amount", @@ -195,7 +187,7 @@ test( } ); -// Not a todo: this holds today for every whole-cent subtotal (Strapi stores prices as decimal(10,2)). It guards the fixes above. +// This held before the fixes too, for every whole-cent subtotal (Strapi stores prices as decimal(10,2)). It guards the fixes above. test("D6: the breakdown adds up to the amount to the cent, also for the subtotals whose net is closest to half a cent", () => { const carts: { lines: [number, number][]; delivery: number | null }[] = [ { lines: [[17.9, 1]], delivery: 4.95 }, @@ -232,3 +224,149 @@ test("D6: the breakdown adds up to the amount to the cent, also for the subtotal // Compare the count, not the array: diffing tens of thousands of entries against [] takes many GB of memory. assert.equal(mismatches.length, 0, `${mismatches.length} carts do not add up, e.g. ${JSON.stringify(mismatches.slice(0, 3))}`); }); + +// Shipping address: the structured delivery address, else the structured invoice address, else the text delivery address. + +const structured = (overrides = {}) => ({ + givenName: "Erika", + familyName: "Mustermann", + streetAddress: "Musterstraße 1", + postalCode: "70190", + addressLevel2: "Stuttgart", + country: "DE", + ...overrides +}); +const berlin = structured({ givenName: "Max", streetAddress: "Hauptstraße 5", postalCode: "10115", addressLevel2: "Berlin" }); +const berlinShipping = { + name: { fullName: "Max Mustermann" }, + address: { addressLine1: "Hauptstraße 5", postalCode: "10115", adminArea2: "Berlin", countryCode: "DE" } +}; +// An address.structured-address component that was saved without components: Strapi fills in the default country. +const emptyStructured = { givenName: null, familyName: null, streetAddress: null, postalCode: null, addressLevel2: null, country: "DE" }; +const shippingOf = (order) => buildSession(order).purchaseUnits[0].shipping; + +test("takes the shipping address from the structured delivery address before the invoice and the text address", () => { + assert.deepEqual(shippingOf(validOrder({ deliveryAddressStructured: berlin, invoiceAddressStructured: structured() })), berlinShipping); +}); + +test("falls back to the structured invoice address when no structured delivery address is entered", () => { + for (const deliveryAddressStructured of [null, undefined, emptyStructured, { givenName: " ", country: "DE" }]) { + assert.deepEqual( + shippingOf(validOrder({ deliveryAddressStructured, invoiceAddressStructured: berlin, deliveryAddress: null })), + berlinShipping + ); + } +}); + +test("trims the components, names the buyer by the name parts there are and upper-cases the country, DE by default", () => { + const order = (address) => validOrder({ deliveryAddressStructured: { ...structured(), ...address } }); + + assert.deepEqual(shippingOf(order({ givenName: " Erika ", familyName: null, streetAddress: " Musterstraße 1 ", country: "at" })), { + name: { fullName: "Erika" }, + address: { addressLine1: "Musterstraße 1", postalCode: "70190", adminArea2: "Stuttgart", countryCode: "AT" } + }); + for (const country of [null, undefined, ""]) { + assert.equal(shippingOf(order({ country })).address.countryCode, "DE"); + } +}); + +test("rejects a structured address that is entered but incomplete instead of shipping to another address", () => { + const order = validOrder({ deliveryAddressStructured: structured({ streetAddress: "", postalCode: null }), invoiceAddressStructured: berlin }); + + assert.throws(() => buildSession(order), { name: "CheckoutNotReadyError", message: "deliveryAddressStructured lacks streetAddress, postalCode" }); + assert.throws(() => buildSession(validOrder({ invoiceAddressStructured: structured({ givenName: null, familyName: "" }) })), { + name: "CheckoutNotReadyError", + message: "invoiceAddressStructured lacks givenName and familyName" + }); + assert.throws(() => buildSession(validOrder({ deliveryAddressStructured: structured({ country: "Deutschland" }) })), { + name: "CheckoutNotReadyError", + message: "deliveryAddressStructured has no two-letter country code" + }); +}); + +test("parses the text delivery address when no structured address is entered", () => { + const shippingFor = (deliveryAddress) => shippingOf(validOrder({ deliveryAddress, deliveryAddressStructured: emptyStructured })); + + assert.deepEqual(shippingFor(" Erika Mustermann\r\nMusterstraße 1\r\n\r\n70190 Stuttgart-Bad Cannstatt \n"), { + name: { fullName: "Erika Mustermann" }, + address: { addressLine1: "Musterstraße 1", postalCode: "70190", adminArea2: "Stuttgart-Bad Cannstatt", countryCode: "DE" } + }); + // The street is the line above the postal code and city, as in a German address; the lines after the name go into addressLine2. + assert.deepEqual(shippingFor("Erika Mustermann\nc/o Muster GmbH\nHinterhaus\nMusterstraße 1\n70190 Stuttgart"), { + name: { fullName: "Erika Mustermann" }, + address: { + addressLine1: "Musterstraße 1", + addressLine2: "c/o Muster GmbH, Hinterhaus", + postalCode: "70190", + adminArea2: "Stuttgart", + countryCode: "DE" + } + }); +}); + +test("rejects a text delivery address it cannot parse, and a missing one, with a validation error", () => { + const cases: [unknown, string][] = [ + ["Erika Mustermann\nMusterstraße 1", "deliveryAddress needs the name, the street, and the postal code and city on lines of their own"], + ["Erika Mustermann\n\nMusterstraße 1\n", "deliveryAddress needs the name, the street, and the postal code and city on lines of their own"], + ["Erika Mustermann\nMusterstraße 1\n70190", "The last line of deliveryAddress is not the postal code and the city"], + [" \n ", "The order has no delivery address"], + ["", "The order has no delivery address"], + [null, "The order has no delivery address"], + [undefined, "The order has no delivery address"] + ]; + for (const [deliveryAddress, message] of cases) { + assert.throws(() => buildSession(validOrder({ deliveryAddress })), { name: "CheckoutNotReadyError", message }); + } +}); + +// Money in whole cents, and the errors the checkout answers. + +test("reads decimals that arrive as strings like numbers", () => { + const order = validOrder({ subtotal: "17.90", total: "22.85", VAT: "2.86", delivery: { id: 1, name: "DHL", price: "4.95" } }); + + assert.deepEqual(buildSession(order), buildSession(validOrder())); +}); + +test("rejects totals that do not add up, as after a price change, instead of sending PayPal a breakdown it rejects", () => { + // The delivery method costs 5.95 now; the order's total was computed when it cost 4.95. + assert.throws(() => buildSession(validOrder({ delivery: { id: 1, name: "DHL", price: 5.95 } })), { + name: "CheckoutNotReadyError", + message: "The order's totals do not add up: total 2285 cents, items 1504 + VAT 286 + shipping 595" + }); + assert.throws(() => buildSession(validOrder({ delivery: { id: 1, name: "DHL", price: -4.95 }, total: 12.95 })), { + name: "CheckoutNotReadyError", + message: "The order's totals do not add up: total 1295 cents, items 1504 + VAT 286 + shipping -495" + }); + for (const overrides of [{ VAT: null }, { subtotal: undefined }, { VAT: "n/a" }]) { + assert.throws(() => buildSession(validOrder(overrides)), { name: "CheckoutNotReadyError", message: "The order has no subtotal or no VAT" }); + } +}); + +test("rejects a deleted product next to available ones as unavailable", () => { + const order = validOrder({ cart: [{ id: 1, count: 1, product: notebook }, { id: 2, count: 1, product: null }, null] }); + + assert.throws( + () => buildSession(order), + (error) => error instanceof ProductUnavailableError && error.name === "ProductUnavailableError" && error.message === PRODUCT_UNAVAILABLE + ); +}); + +test("answers an order that is not ready 400 with the reason, a deleted product 409 and any other error 502", () => { + assert.deepEqual(checkoutFailure(thrown(() => buildSession(validOrder({ deliveryAddress: null })))), { + status: 400, + message: "Order is not ready for checkout", + reason: "The order has no delivery address" + }); + assert.equal(checkoutFailure(thrown(() => buildSession(validOrder({ deliveryAddress: "Erika Mustermann\nMusterstraße 1" })))).status, 400); + assert.equal(checkoutFailure(thrown(() => buildSession(validOrder({ total: 0 })))).status, 400); + assert.deepEqual(checkoutFailure(thrown(() => buildSession(validOrder({ cart: [{ id: 1, count: 1, product: null }] })))), { + status: 409, + message: "A product in the cart is no longer available" + }); + assert.ok(thrown(() => buildSession(validOrder({ total: 0 }))) instanceof CheckoutNotReadyError); + + const sdkError = Object.assign(new Error("OAuth 2 provider says client authentication failed."), { statusCode: 401 }); + for (const error of [sdkError, new TypeError("Cannot read properties of null (reading 'split')"), "/result/error", undefined]) { + assert.deepEqual(checkoutFailure(error), { status: 502, message: "Could not create the PayPal order" }); + } +}); diff --git a/tests/unit/paypal-verify.test.ts b/tests/unit/paypal-verify.test.ts index 10222aa..8ffe2dc 100644 --- a/tests/unit/paypal-verify.test.ts +++ b/tests/unit/paypal-verify.test.ts @@ -23,6 +23,9 @@ const order = (overrides = {}) => ({ VAT: 2.86, delivery: { id: 1, name: "DHL", price: 4.95 }, invoiceNumber: "R-12-4711", + cart: [ + { id: 1, count: 1, product: { id: 101, name: "Notizbuch A5 Punktraster", cover: { price: 14.9 }, pages: { price: 3 }, ruling: { price: 0 } } } + ], deliveryAddress: "Erika Mustermann\nMusterstraße 1\n70190 Stuttgart", ...overrides }); @@ -80,9 +83,12 @@ test("turns the order total into integer cents, also where its float is a hair b test("the amount the checkout sends PayPal is the amount the capture accepts, for every total up to 3000.00", () => { const mismatches: number[] = []; for (let cents = 1; cents <= 300000; cents++) { + // The order service rounds every total to the cent; an order without a delivery method has its subtotal as its total. const total = cents / 100; - // buildPayPalOrderRequest sends order.total.toFixed(2); the order service rounds every total to the cent - if (parseCents(total.toFixed(2)) !== cents || orderTotalCents({ total }) !== cents) mismatches.push(total); + const VAT = Math.round((total / 1.19) * 0.19 * 100) / 100; + const request = buildPayPalOrderRequest("https://shop.example/checkout/3", order({ subtotal: total, total, VAT, delivery: null })); + const { amount } = request.purchaseUnits[0]; + if (parseCents(amount.value) !== cents || orderTotalCents({ total }) !== cents) mismatches.push(total); } assert.equal(mismatches.length, 0, `e.g. ${mismatches.slice(0, 5).join(", ")}`); }); -- 2.36.6 From ba110a11d82e4ed4c8b6a9a803d0a025cc191332 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:50:02 +0200 Subject: [PATCH 14/16] fix(checkout): send a payment surcharge to PayPal as handling The order service adds the price of the order's payment method to the total, but the PayPal breakdown left it out, so an order with a surcharge was sent an amount its breakdown did not add up to, which PayPal rejects (D5). Since the breakdown is now checked against the total, such an order was answered 400. A payment price above 0 is now sent as handling, in whole cents, and item total, tax, shipping and handling add up to the amount to the cent. A payment method that costs nothing sends no handling. Every payment method and the pickup cost 0.00 today, and the shop cannot set a payment method (parseCustomerUpdate rejects the field), so this is robustness, not a live defect. The D5 test is a regular test, and a test checks the sum with a surcharge, also with a pickup without a price, for every whole-cent subtotal up to 300.00. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/checkout/paypal-order.ts | 18 ++++++--- tests/unit/paypal-order.test.ts | 65 +++++++++++++++++++++++++-------- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/checkout/paypal-order.ts b/src/checkout/paypal-order.ts index 304214d..a71df1c 100644 --- a/src/checkout/paypal-order.ts +++ b/src/checkout/paypal-order.ts @@ -23,6 +23,8 @@ export type PayPalBreakdown = { taxTotal: PayPalMoney; /** Undefined for an order without a delivery method. */ shipping: PayPalMoney | undefined; + /** The surcharge of the payment method, only when it is above 0. */ + handling?: PayPalMoney; }; export type PayPalOrderRequest = { @@ -54,6 +56,7 @@ export type PayPalOrderSource = { total?: number | string | null; VAT?: number | string | null; delivery?: { price?: number | string | null } | null; + payment?: { price?: number | string | null } | null; cart?: ({ product?: unknown } | null)[] | null; deliveryAddress?: string | null; deliveryAddressStructured?: CustomerStructuredAddress | null; @@ -110,9 +113,10 @@ const money = (cents: number): PayPalMoney => ({ const VAT_INCLUDED_PERCENT = Math.round(vatIncludedDecimal * 100); /** - * The breakdown of the order's total: the net of its subtotal, its VAT and the price of its delivery method, 0 when that price - * is not set. PayPal rejects a breakdown that does not add up to the amount, which happens when a price changed after the order - * service computed the totals, so such an order is rejected here. + * The breakdown of the order's total: the net of its subtotal, its VAT, the price of its delivery method, 0 when that price is not + * set, and the surcharge of its payment method as handling, which the order service adds to the total. PayPal rejects a breakdown + * that does not add up to the amount, which happens when a price changed after the order service computed the totals, so such an + * order is rejected here. */ const breakdownFor = (order: PayPalOrderSource, totalCents: number): PayPalBreakdown => { const subtotalCents = toCents(order.subtotal); @@ -121,19 +125,21 @@ const breakdownFor = (order: PayPalOrderSource, totalCents: number): PayPalBreak const itemTotalCents = Math.round((subtotalCents * 100) / VAT_INCLUDED_PERCENT); const shippingCents = order.delivery ? (toCents(order.delivery.price) ?? 0) : undefined; + const surchargeCents = toCents(order.payment?.price) ?? 0; - const parts = [itemTotalCents, taxCents, shippingCents ?? 0]; + const parts = [itemTotalCents, taxCents, shippingCents ?? 0, surchargeCents]; const sum = parts.reduce((a, b) => a + b, 0); if (parts.some((cents) => cents < 0) || sum !== totalCents) { throw new CheckoutNotReadyError( - `The order's totals do not add up: total ${totalCents} cents, items ${itemTotalCents} + VAT ${taxCents} + shipping ${shippingCents ?? 0}` + `The order's totals do not add up: total ${totalCents} cents, items ${itemTotalCents} + VAT ${taxCents} + shipping ${shippingCents ?? 0} + surcharge ${surchargeCents}` ); } return { itemTotal: money(itemTotalCents), taxTotal: money(taxCents), - shipping: shippingCents === undefined ? undefined : money(shippingCents) + shipping: shippingCents === undefined ? undefined : money(shippingCents), + ...(surchargeCents > 0 ? { handling: money(surchargeCents) } : {}) }; }; diff --git a/tests/unit/paypal-order.test.ts b/tests/unit/paypal-order.test.ts index b659567..37891de 100644 --- a/tests/unit/paypal-order.test.ts +++ b/tests/unit/paypal-order.test.ts @@ -33,7 +33,7 @@ const validOrder = (overrides = {}) => ({ }); // A cart of [price, count] lines with the totals the order service computes in update() (src/api/order/services/order.ts:151-173). -const orderFor = (lines: [number, number][], deliveryPrice: number | null) => { +const orderFor = (lines: [number, number][], deliveryPrice: number | null, paymentPrice: number | null = null) => { const cart = lines.map(([price, count], index) => ({ id: index + 1, count, @@ -43,9 +43,10 @@ const orderFor = (lines: [number, number][], deliveryPrice: number | null) => { return validOrder({ cart, subtotal, - total: Math.round((subtotal + (deliveryPrice ?? 0)) * 100) / 100, + total: Math.round((subtotal + (deliveryPrice ?? 0) + (paymentPrice ?? 0)) * 100) / 100, VAT: Math.round((subtotal / vatIncludedDecimal) * vatDecimal * 100) / 100, - delivery: deliveryPrice === null ? null : { id: 1, name: "DHL", price: deliveryPrice } + delivery: deliveryPrice === null ? null : { id: 1, name: "DHL", price: deliveryPrice }, + payment: paymentPrice === null ? null : { id: 1, name: "Vorkasse", price: paymentPrice } }); }; @@ -175,17 +176,13 @@ test("D4: a cart line whose product was deleted is rejected with a clear error", assert.throws(() => buildSession(validOrder({ cart: [{ id: 1, count: 1, product: null }] })), isClearError(/product/i)); }); -test( - "D5: with a payment surcharge the breakdown still adds up to the amount", - { todo: "the surcharge is in the amount but not in the breakdown — https://git.librete.ch/libretech/mp/issues/67" }, - () => { - // total = 17.90 + 4.95 shipping + 1.50 surcharge - const { amount } = buildSession(validOrder({ payment: { id: 1, name: "PayPal", price: 1.5 }, total: 24.35 })).purchaseUnits[0]; +test("D5: with a payment surcharge the breakdown still adds up to the amount", () => { + // total = 17.90 + 4.95 shipping + 1.50 surcharge + const { amount } = buildSession(validOrder({ payment: { id: 1, name: "PayPal", price: 1.5 }, total: 24.35 })).purchaseUnits[0]; - assert.equal(amount.value, "24.35"); - assert.equal(breakdownCents(amount.breakdown), cents(amount)); - } -); + assert.equal(amount.value, "24.35"); + assert.equal(breakdownCents(amount.breakdown), cents(amount)); +}); // This held before the fixes too, for every whole-cent subtotal (Strapi stores prices as decimal(10,2)). It guards the fixes above. test("D6: the breakdown adds up to the amount to the cent, also for the subtotals whose net is closest to half a cent", () => { @@ -331,17 +328,55 @@ test("rejects totals that do not add up, as after a price change, instead of sen // The delivery method costs 5.95 now; the order's total was computed when it cost 4.95. assert.throws(() => buildSession(validOrder({ delivery: { id: 1, name: "DHL", price: 5.95 } })), { name: "CheckoutNotReadyError", - message: "The order's totals do not add up: total 2285 cents, items 1504 + VAT 286 + shipping 595" + message: "The order's totals do not add up: total 2285 cents, items 1504 + VAT 286 + shipping 595 + surcharge 0" }); assert.throws(() => buildSession(validOrder({ delivery: { id: 1, name: "DHL", price: -4.95 }, total: 12.95 })), { name: "CheckoutNotReadyError", - message: "The order's totals do not add up: total 1295 cents, items 1504 + VAT 286 + shipping -495" + message: "The order's totals do not add up: total 1295 cents, items 1504 + VAT 286 + shipping -495 + surcharge 0" }); for (const overrides of [{ VAT: null }, { subtotal: undefined }, { VAT: "n/a" }]) { assert.throws(() => buildSession(validOrder(overrides)), { name: "CheckoutNotReadyError", message: "The order has no subtotal or no VAT" }); } }); +test("sends a payment surcharge as handling, and no handling for a payment method that costs nothing", () => { + const breakdownOf = (order) => buildSession(order).purchaseUnits[0].amount.breakdown; + + assert.deepEqual(breakdownOf(validOrder({ payment: { id: 1, name: "PayPal", price: 1.5 }, total: 24.35 })), { + itemTotal: { currencyCode: "EUR", value: "15.04" }, + taxTotal: { currencyCode: "EUR", value: "2.86" }, + shipping: { currencyCode: "EUR", value: "4.95" }, + handling: { currencyCode: "EUR", value: "1.50" } + }); + for (const payment of [null, undefined, { id: 1, name: "PayPal", price: 0 }, { id: 1, name: "PayPal", price: null }]) { + assert.equal("handling" in breakdownOf(validOrder({ payment })), false); + } + // A surcharge the total lacks, as when the price of the payment method changed after the totals were computed. + assert.throws(() => buildSession(validOrder({ payment: { id: 1, name: "PayPal", price: 1.5 } })), { + name: "CheckoutNotReadyError", + message: "The order's totals do not add up: total 2285 cents, items 1504 + VAT 286 + shipping 495 + surcharge 150" + }); +}); + +test("adds up to the cent with a surcharge, with a pickup without a price, for every whole-cent subtotal up to 300.00", () => { + const mismatches: string[] = []; + for (let cent = 1; cent <= 30000; cent++) { + for (const [delivery, payment] of [ + [4.95, 0.35], + [0, 1.5] + ]) { + const order = orderFor([[cent / 100, 1]], delivery, payment); + if (delivery === 0) order.delivery = { id: 2, name: "Abholung", price: null }; + const { amount } = buildSession(order).purchaseUnits[0]; + if (breakdownCents(amount.breakdown) !== cents(amount) || cents(amount.breakdown.handling) !== Math.round(payment * 100)) { + mismatches.push(`${cent}/${delivery}/${payment}`); + } + } + } + + assert.equal(mismatches.length, 0, `${mismatches.length} carts do not add up, e.g. ${mismatches.slice(0, 3).join(", ")}`); +}); + test("rejects a deleted product next to available ones as unavailable", () => { const order = validOrder({ cart: [{ id: 1, count: 1, product: notebook }, { id: 2, count: 1, product: null }, null] }); -- 2.36.6 From b67150376d3a945cedf5e83cc189dbf506203fdc Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:51:23 +0200 Subject: [PATCH 15/16] fix(paypal): log no request bodies, headers or PayPal orders The PayPal SDK client logged every request body to the console, and with it the buyer's name and address in the order request, as well as the response headers. createSessionOrThrow logged the whole request at silly, which production wrote, and PayPal's whole answer at info. The webhook logged its whole body, and the controller's webhook error handler the error object as JSON. The SDK now logs method, URL and status of each request, without bodies or headers. createSessionOrThrow logs the shop's order id, the PayPal order's id and its status, and an error by describeError with the order id. The webhook logs the event type and the id and status of its resource; its error handler describes the error. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/api/order/controllers/order.ts | 2 +- src/services/PayPalApi.ts | 46 +++++++++++++++++------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/api/order/controllers/order.ts b/src/api/order/controllers/order.ts index 6d45cfe..8c9302e 100644 --- a/src/api/order/controllers/order.ts +++ b/src/api/order/controllers/order.ts @@ -358,7 +358,7 @@ export default factories.createCoreController("api::order.order", ({ strapi }) = try { return await paypalApi.handleWebhook(ctx.request.body); } catch (error) { - strapi.log.error(`paypal-api: ${JSON.stringify({ error })}`); + strapi.log.error("app:e:order-controller: Handling the PayPal webhook failed", { error: describeError(error) }); return ctx.internalServerError(); } } diff --git a/src/services/PayPalApi.ts b/src/services/PayPalApi.ts index 00c7415..e5f2a15 100644 --- a/src/services/PayPalApi.ts +++ b/src/services/PayPalApi.ts @@ -16,10 +16,12 @@ class PayPalApi { }, timeout: 0, environment, + // The SDK logs method, URL and status of each request to the console. Not the bodies: a request body holds the buyer's name + // and address. Not the headers: they hold the token. logging: { logLevel: LogLevel.Info, - logRequest: { logBody: true }, - logResponse: { logHeaders: true } + logRequest: { logBody: false, logHeaders: false }, + logResponse: { logBody: false, logHeaders: false } } }); this.ordersController = new OrdersController(this.client); @@ -29,28 +31,25 @@ class PayPalApi { * Creates the PayPal order for the order and returns PayPal's id for it, which the capture accepts, and its links. Throws * CheckoutNotReadyError or ProductUnavailableError before PayPal is asked, for an order PayPal cannot take. */ - async createSessionOrThrow(returnUrl: string, order: PayPalOrderSource): Promise<{ id: string; links: unknown }> { + async createSessionOrThrow(returnUrl: string, order: PayPalOrderSource & { id?: number | string }): Promise<{ id: string; links: unknown }> { // Outside the try block: an order that is not ready is the customer's to fix, not a PayPal failure. const request = buildPayPalOrderRequest(returnUrl, order) as OrderRequest; try { - const collect = { body: request, prefer: "return=minimal" }; + const { body } = await this.ordersController.ordersCreate({ body: request, prefer: "return=minimal" }); + const session = JSON.parse(body as string); + if (typeof session?.id !== "string" || session.id === "") throw new Error("PayPal answered without an order id"); - strapi.log.silly("app:d:paypal-api: Creating PayPal order " + JSON.stringify({ collect })); - - const { body, ...httpResponse } = await this.ordersController.ordersCreate(collect); - const sessionData = JSON.parse(body as string); - - strapi.log.info("app:i:paypal-api: " + JSON.stringify({ session: sessionData })); - - if (typeof sessionData?.id !== "string" || sessionData.id === "") throw new Error("PayPal answered without an order id"); - return { - id: sessionData.id, - links: sessionData.links - }; + // The PayPal order's id and status only: the request and PayPal's order hold the buyer's name and address. + strapi.log.info("app:i:paypal-api: Created the PayPal order", { + order: order.id, + paypalOrderId: session.id, + status: typeof session.status === "string" ? session.status : undefined + }); + return { id: session.id, links: session.links }; } catch (error) { // Never the error object: an SDK or axios error carries the request with its Authorization header. - strapi.log.error("app:e:paypal-api: Creating the PayPal order failed", { error: describeError(error) }); + strapi.log.error("app:e:paypal-api: Creating the PayPal order failed", { order: order.id, error: describeError(error) }); throw error; } } @@ -71,9 +70,16 @@ class PayPalApi { return JSON.parse(body as string); } - async handleWebhook(req: any) { - // Implement PayPal webhook validation and processing - strapi.log.verbose("app:i:paypal-api: " + JSON.stringify({ req })); + async handleWebhook(req: unknown) { + // Implement PayPal webhook validation and processing. The event's resource holds the buyer's name and address, so only the + // event type and the resource's id and status are logged. + const event = (typeof req === "object" && req !== null ? req : {}) as { event_type?: unknown; resource?: { id?: unknown; status?: unknown } }; + const textOf = (value: unknown) => (typeof value === "string" ? value : undefined); + strapi.log.verbose("app:v:paypal-api: Webhook received", { + eventType: textOf(event.event_type), + resource: textOf(event.resource?.id), + status: textOf(event.resource?.status) + }); // Note: PayPal webhook handling requires specific implementation // This is a placeholder and should be replaced with actual PayPal webhook validation -- 2.36.6 From 7df72676c90bae4839d1f24c0934426b9129bccb Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Fri, 9 Oct 2026 01:55:06 +0200 Subject: [PATCH 16/16] fix(paypal): replace a broken SDK client and time out after 15 s The PayPal SDK keeps the promise of its OAuth token for the lifetime of its Client, also when the token request failed. After one failed token request, as during the DNS outage, every later call failed with that same error and without a request until the process restarted: once DNS was back, the investigators' harness (scenario S7) saw 0 DNS lookups. The client had no timeout either: the SDK's default of 0 means none. PayPalApi now reaches PayPal through createPayPalClient (src/checkout/paypal-client.ts). It creates the SDK's Client and OrdersController through an injected factory on the first call, and anew after an error of the token request, an error without an answer from PayPal (network, DNS, timeout), 401 or 5xx. PayPal's other answers, such as 422, keep the client. A call that fails on a client that was replaced meanwhile leaves its successor alone. A replacement is logged as a warning with the error described. The client times out after 15 s. paypalClientConfiguration builds its configuration, with the logging of bodies and headers off. The tests use a fake factory that, like the SDK, keeps a failed token request per client, and count constructions and token requests. With the real SDK in the harness, S7 looks DNS up again and every Client is created with a timeout of 15000. Refs https://git.librete.ch/libretech/mp/issues/71 Refs https://git.librete.ch/libretech/mp/issues/67 --- src/checkout/paypal-client.ts | 96 +++++++++++++++++ src/services/PayPalApi.ts | 37 +++---- tests/unit/paypal-client.test.ts | 171 +++++++++++++++++++++++++++++++ 3 files changed, 283 insertions(+), 21 deletions(-) create mode 100644 src/checkout/paypal-client.ts create mode 100644 tests/unit/paypal-client.test.ts diff --git a/src/checkout/paypal-client.ts b/src/checkout/paypal-client.ts new file mode 100644 index 0000000..901368f --- /dev/null +++ b/src/checkout/paypal-client.ts @@ -0,0 +1,96 @@ +import type { Configuration, Environment } from "@paypal/paypal-server-sdk"; + +// The PayPal SDK client behind PayPalApi, created through a factory so that it can be replaced: PayPalApi passes one that creates +// the SDK's Client and OrdersController, the tests a fake. +// +// The SDK keeps the promise of its OAuth token for the lifetime of a Client, also when the token request failed: every later call +// awaits the same rejected promise and fails with the same error without a request, until the process restarts. One failed token +// request, during a DNS outage for example, stopped every checkout until a restart. + +/** The SDK's default timeout is 0, which means none: a call to PayPal that hangs would hang the checkout with it. */ +export const PAYPAL_TIMEOUT_MS = 15000; + +/** + * The configuration of the SDK's Client. The SDK logs method, URL and status of each request to the console, but no bodies, since + * a request body holds the buyer's name and address, and no headers, since they hold the token. + */ +export const paypalClientConfiguration = ({ + clientId, + clientSecret, + environment +}: { + clientId: string; + clientSecret: string; + environment: Environment; +}): Partial => ({ + clientCredentialsAuthCredentials: { + oAuthClientId: clientId, + oAuthClientSecret: clientSecret + }, + timeout: PAYPAL_TIMEOUT_MS, + environment, + logging: { + logRequest: { logBody: false, logHeaders: false }, + logResponse: { logBody: false, logHeaders: false } + } +}); + +const TOKEN_PATH = "/v1/oauth2/token"; + +/** The URL of the request the error belongs to: an error of the SDK carries the request, an axios error its config. */ +const requestUrl = (error: object): string => { + const { request, config } = error as { request?: { url?: unknown }; config?: { url?: unknown } }; + const url = request?.url ?? config?.url; + return typeof url === "string" ? url : ""; +}; + +/** + * Whether the error may leave the client unusable, so that the next call needs a new one: any error of the token request, which + * the SDK keeps; an error without an answer from PayPal (network, DNS, timeout); 401, a token PayPal no longer accepts; and 5xx. + * PayPal's other answers, such as 422 for an order it rejects, leave the client as it is. + */ +export const breaksClient = (error: unknown): boolean => { + if (typeof error !== "object" || error === null) return true; + const { statusCode } = error as { statusCode?: unknown }; + if (typeof statusCode !== "number") return true; + if (statusCode === 401 || statusCode >= 500) return true; + return requestUrl(error).includes(TOKEN_PATH); +}; + +export type PayPalClient = { + /** Runs the request on the current controller. An error that breaksClient replaces the controller for the calls after it. */ + call(request: (controller: C) => Promise): Promise; +}; + +/** + * Creates the controller on the first call, and a new one, with a new client and token, on the first call after an error that + * breaksClient. onReplace is called with that error. + */ +export const createPayPalClient = ({ + configuration, + createController, + onReplace +}: { + configuration: Partial; + createController: (configuration: Partial) => C; + onReplace?: (error: unknown) => void; +}): PayPalClient => { + let current: C | undefined; + + return { + async call(request: (controller: C) => Promise): Promise { + current ??= createController(configuration); + const controller = current; + try { + return await request(controller); + } catch (error) { + // A call that failed on a controller which was replaced meanwhile leaves its successor alone. + if (breaksClient(error) && current === controller) { + current = undefined; + onReplace?.(error); + } + throw error; + } + } + }; +}; diff --git a/src/services/PayPalApi.ts b/src/services/PayPalApi.ts index e5f2a15..8951a57 100644 --- a/src/services/PayPalApi.ts +++ b/src/services/PayPalApi.ts @@ -1,30 +1,23 @@ -import { Client, Environment, LogLevel, OrdersController, OrderRequest } from "@paypal/paypal-server-sdk"; +import { Client, OrdersController } from "@paypal/paypal-server-sdk"; +import type { Environment, OrderRequest } from "@paypal/paypal-server-sdk"; import { paypalClientId, paypalClientSecret, paypalEnvironment } from "../../config/constants"; +import { createPayPalClient, paypalClientConfiguration } from "../checkout/paypal-client"; +import type { PayPalClient } from "../checkout/paypal-client"; import { buildPayPalOrderRequest } from "../checkout/paypal-order"; import type { PayPalOrderSource } from "../checkout/paypal-order"; import { describeError } from "../logging/error"; class PayPalApi { - private readonly client: Client; - private readonly ordersController: OrdersController; + // A new SDK Client and OrdersController after a failed token request, a network error, 401 or 5xx: see src/checkout/paypal-client.ts. + private readonly orders: PayPalClient; constructor(clientId: string, clientSecret: string, environment: Environment) { - this.client = new Client({ - clientCredentialsAuthCredentials: { - oAuthClientId: clientId, - oAuthClientSecret: clientSecret - }, - timeout: 0, - environment, - // The SDK logs method, URL and status of each request to the console. Not the bodies: a request body holds the buyer's name - // and address. Not the headers: they hold the token. - logging: { - logLevel: LogLevel.Info, - logRequest: { logBody: false, logHeaders: false }, - logResponse: { logBody: false, logHeaders: false } - } + this.orders = createPayPalClient({ + configuration: paypalClientConfiguration({ clientId, clientSecret, environment }), + createController: (configuration) => new OrdersController(new Client(configuration)), + onReplace: (error) => + strapi.log.warn("app:w:paypal-api: Replacing the PayPal client after a failed request", { error: describeError(error) }) }); - this.ordersController = new OrdersController(this.client); } /** @@ -36,7 +29,7 @@ class PayPalApi { const request = buildPayPalOrderRequest(returnUrl, order) as OrderRequest; try { - const { body } = await this.ordersController.ordersCreate({ body: request, prefer: "return=minimal" }); + const { body } = await this.orders.call((orders) => orders.ordersCreate({ body: request, prefer: "return=minimal" })); const session = JSON.parse(body as string); if (typeof session?.id !== "string" || session.id === "") throw new Error("PayPal answered without an order id"); @@ -56,7 +49,7 @@ class PayPalApi { /** PayPal's order as parsed JSON in PayPal's snake_case (GET /v2/checkout/orders/:id). Throws PayPal's and network errors. */ async getOrder(id: string): Promise { - const { body } = await this.ordersController.ordersGet({ id }); + const { body } = await this.orders.call((orders) => orders.ordersGet({ id })); return JSON.parse(body as string); } @@ -66,7 +59,9 @@ class PayPalApi { * response of the first capture instead of capturing again. Throws PayPal's and network errors. */ async captureOrder(id: string, requestId: string): Promise { - const { body } = await this.ordersController.ordersCapture({ id, paypalRequestId: requestId, prefer: "return=representation" }); + const { body } = await this.orders.call((orders) => + orders.ordersCapture({ id, paypalRequestId: requestId, prefer: "return=representation" }) + ); return JSON.parse(body as string); } diff --git a/tests/unit/paypal-client.test.ts b/tests/unit/paypal-client.test.ts new file mode 100644 index 0000000..4e9e53a --- /dev/null +++ b/tests/unit/paypal-client.test.ts @@ -0,0 +1,171 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { PAYPAL_TIMEOUT_MS, breaksClient, createPayPalClient, paypalClientConfiguration } from "../../src/checkout/paypal-client.ts"; + +const TOKEN_URL = "https://api-m.paypal.com/v1/oauth2/token"; +const ORDERS_URL = "https://api-m.paypal.com/v2/checkout/orders"; +const PAYPAL_ORDER = '{"id":"5O190127TN364715T","status":"CREATED"}'; + +// What axios throws when PayPal cannot be reached: no status code, the request in config. +const networkError = (url = TOKEN_URL) => + Object.assign(new Error("getaddrinfo EAI_AGAIN api-m.paypal.com"), { name: "AxiosError", code: "EAI_AGAIN", config: { method: "post", url } }); + +// What the SDK throws when PayPal answers with an error status (@apimatic/core's ApiError): the status code and the request. +const apiError = (statusCode: number, url = ORDERS_URL) => + Object.assign(new Error(`PayPal answered ${statusCode}`), { statusCode, request: { method: "POST", url }, headers: {}, body: "{}" }); + +/** + * A fake of the SDK's OrdersController over a Client. Like the SDK, every client requests its OAuth token on its first call and + * keeps the promise for its lifetime, also when the token request failed: every later call fails with that same error and + * requests nothing. The token request fails while dns.up is false. Each construction and token request is counted. + */ +const fakeSdk = () => { + const sdk = { dnsUp: false, constructions: 0, tokenRequests: 0, configurations: [] as unknown[] }; + const createController = (configuration: unknown) => { + sdk.constructions += 1; + sdk.configurations.push(configuration); + let token: Promise | undefined; + const authenticate = () => { + if (token === undefined) { + sdk.tokenRequests += 1; + token = sdk.dnsUp ? Promise.resolve("A21AAF…") : Promise.reject(networkError()); + token.catch(() => {}); + } + return token; + }; + return { + ordersCreate: async () => { + await authenticate(); + return { statusCode: 201, body: PAYPAL_ORDER }; + } + }; + }; + return { sdk, createController }; +}; + +test("a client of the SDK keeps a failed token request: every later call fails without a request", async () => { + // What the replacement below works around, as the harness's scenario S7 observed with the real SDK. + const { sdk, createController } = fakeSdk(); + const orders = createController({}); + + const first = await orders.ordersCreate().catch((error) => error); + sdk.dnsUp = true; + const second = await orders.ordersCreate().catch((error) => error); + + assert.equal(second, first); + assert.equal(sdk.tokenRequests, 1); +}); + +test("replaces the client after a failed token request, so that the next call requests a new token", async () => { + const { sdk, createController } = fakeSdk(); + const replaced: unknown[] = []; + const client = createPayPalClient({ configuration: {}, createController, onReplace: (error) => replaced.push(error) }); + + await assert.rejects( + client.call((orders) => orders.ordersCreate()), + { name: "AxiosError", message: "getaddrinfo EAI_AGAIN api-m.paypal.com" } + ); + assert.deepEqual([sdk.constructions, sdk.tokenRequests, replaced.length], [1, 1, 1]); + + sdk.dnsUp = true; + assert.deepEqual(await client.call((orders) => orders.ordersCreate()), { statusCode: 201, body: PAYPAL_ORDER }); + assert.deepEqual([sdk.constructions, sdk.tokenRequests], [2, 2]); + + // The new client keeps its token. + await client.call((orders) => orders.ordersCreate()); + assert.deepEqual([sdk.constructions, sdk.tokenRequests, replaced.length], [2, 2, 1]); +}); + +test("keeps the client after an answer of PayPal that leaves it usable", async () => { + let constructions = 0; + const client = createPayPalClient({ + configuration: {}, + createController: () => { + constructions += 1; + return { ordersCreate: async () => Promise.reject(apiError(422)) }; + } + }); + + for (let call = 0; call < 3; call++) { + const error = await client.call((orders) => orders.ordersCreate()).catch((rejection) => rejection); + assert.equal(error.statusCode, 422); + } + assert.equal(constructions, 1); +}); + +test("a call that fails on a replaced client does not replace its successor", async () => { + let constructions = 0; + const pending: ((error: unknown) => void)[] = []; + const replaced: unknown[] = []; + const client = createPayPalClient({ + configuration: {}, + createController: () => { + constructions += 1; + return { ordersGet: () => new Promise((_, reject) => pending.push(reject)) }; + }, + onReplace: (error) => replaced.push(error) + }); + + // Two calls run on the first client. The first one fails, which replaces the client, and a third call starts on the second. + const first = client.call((orders) => orders.ordersGet()); + const second = client.call((orders) => orders.ordersGet()); + pending[0](networkError(ORDERS_URL)); + await assert.rejects(first); + const third = client.call((orders) => orders.ordersGet()); + assert.equal(constructions, 2); + + // The second call fails on the first client only now: the second client stays. + pending[1](networkError(ORDERS_URL)); + await assert.rejects(second); + const fourth = client.call((orders) => orders.ordersGet()); + assert.deepEqual([constructions, replaced.length], [2, 1]); + + pending[2](apiError(404)); + pending[3](apiError(404)); + await Promise.all([assert.rejects(third), assert.rejects(fourth)]); +}); + +test("breaks the client with an error of the token request, without an answer, with 401 and with 5xx", () => { + for (const error of [ + networkError(TOKEN_URL), + networkError(ORDERS_URL), + Object.assign(new Error("timeout of 15000ms exceeded"), { name: "AxiosError", code: "ECONNABORTED", config: { url: ORDERS_URL } }), + apiError(400, TOKEN_URL), + apiError(429, TOKEN_URL), + apiError(401, TOKEN_URL), + apiError(401, ORDERS_URL), + apiError(500, ORDERS_URL), + apiError(503, ORDERS_URL), + // The SDK's ResponseValidationError carries the status of the answer it could not read. + Object.assign(new Error("The response did not match the response schema."), { statusCode: 200, request: { url: TOKEN_URL } }), + new Error("The HTTP call was aborted."), + "a string", + undefined + ]) { + assert.equal(breaksClient(error), true, String(error?.message ?? error)); + } + for (const error of [apiError(400), apiError(404), apiError(409), apiError(422), apiError(429)]) { + assert.equal(breaksClient(error), false, error.message); + } +}); + +test("configures the SDK client with a timeout of 15 s and without logging bodies or headers", async () => { + const configuration = paypalClientConfiguration({ clientId: "DUMMY_CLIENT_ID", clientSecret: "DUMMY_CLIENT_SECRET", environment: "Production" }); + + assert.equal(PAYPAL_TIMEOUT_MS, 15000); + assert.deepEqual(configuration, { + clientCredentialsAuthCredentials: { oAuthClientId: "DUMMY_CLIENT_ID", oAuthClientSecret: "DUMMY_CLIENT_SECRET" }, + timeout: 15000, + environment: "Production", + logging: { logRequest: { logBody: false, logHeaders: false }, logResponse: { logBody: false, logHeaders: false } } + }); + + // Every client the factory creates, also after a replacement, gets this configuration. + const { sdk, createController } = fakeSdk(); + const client = createPayPalClient({ configuration, createController }); + await client.call((orders) => orders.ordersCreate()).catch(() => {}); + sdk.dnsUp = true; + await client.call((orders) => orders.ordersCreate()); + assert.equal(sdk.constructions, 2); + for (const passed of sdk.configurations) assert.equal((passed as { timeout: number }).timeout, 15000); +}); -- 2.36.6