diff --git a/rest/python/server/cart_test.py b/rest/python/server/cart_test.py index 80e6f61..62e4407 100644 --- a/rest/python/server/cart_test.py +++ b/rest/python/server/cart_test.py @@ -343,6 +343,147 @@ async def seed_discount() -> None: self.assertEqual(discount, -200) self.assertEqual(total, 1800) + def test_create_cart_does_not_adopt_client_supplied_omit_members( + self, + ) -> None: + """Cart create carrying omit members must not adopt them or 500. + + cart.json marks ucp, currency, totals, continue_url, expires_at, messages, + and links as ucp_request: omit, and id as omit on create. The create handler + must exclude them from cart_data so keyword collisions (TypeError) and + client value leaks are avoided. + """ + client_values = { + "currency": "XTS", + "id": "cart_client_chosen", + "totals": [{"type": "subtotal", "amount": 9999}], + "continue_url": "https://platform.example/client-continue", + "expires_at": "2030-01-01T00:00:00Z", + "messages": [ + { + "type": "info", + "code": "custom", + "content": "client text", + "severity": "recoverable", + } + ], + "links": [{"type": "terms_of_use", "url": "https://example.com/tos"}], + } + + with self.client: + payload = self._create_cart_payload([("rose", 1)]).model_dump( + mode="json", exclude_none=True + ) + payload.update(client_values) + payload["line_items"][0]["id"] = "client_line_1" + + response = self.client.post( + "/carts", + headers=self._get_headers( + idempotency_key="cart_omit_1", request_id="co1" + ), + json=payload, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + + self.assertEqual(body.get("currency"), "USD") + self.assertNotEqual(body.get("id"), client_values["id"]) + self.assertNotEqual( + body.get("continue_url"), client_values["continue_url"] + ) + self.assertNotEqual(body.get("expires_at"), client_values["expires_at"]) + contents = [ + m.get("content") + for m in body.get("messages", []) + if isinstance(m, dict) + ] + self.assertNotIn("client text", contents) + self.assertNotEqual(body.get("links"), client_values["links"]) + self.assertNotEqual(body["line_items"][0].get("id"), "client_line_1") + + # Verify persistence: GET /carts/{cart_id} + cart_id = body["id"] + get_res = self.client.get( + f"/carts/{cart_id}", + headers=self._get_headers(request_id="co1_get"), + ) + self.assertEqual(get_res.status_code, 200, f"Response: {get_res.text}") + stored = get_res.json() + self.assertEqual(stored.get("currency"), "USD") + self.assertNotEqual( + stored.get("continue_url"), client_values["continue_url"] + ) + self.assertNotEqual(stored.get("expires_at"), client_values["expires_at"]) + stored_contents = [ + m.get("content") + for m in stored.get("messages", []) + if isinstance(m, dict) + ] + self.assertNotIn("client text", stored_contents) + self.assertNotEqual(stored.get("links"), client_values["links"]) + + def test_create_cart_ignores_non_string_members(self) -> None: + """A cart create carrying non-string members must never 500.""" + with self.client: + response = self.client.post( + "/carts", + headers=self._get_headers( + idempotency_key="cart_non_str_1", request_id="cns1" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1, "id": 123}], + "currency": 123, + "id": 123, + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + self.assertEqual(body.get("currency"), "USD") + self.assertIsInstance(body.get("id"), str) + self.assertIsInstance(body["line_items"][0].get("id"), str) + + def test_cart_with_attribution_converts_to_checkout(self) -> None: + """A cart carrying attribution converts to checkout successfully.""" + with self.client: + payload = self._create_cart_payload([("rose", 1)]).model_dump( + mode="json", exclude_none=True + ) + payload["attribution"] = { + "campaign_id": "123", + "campaign_source": "newsletter", + } + + response = self.client.post( + "/carts", + headers=self._get_headers( + idempotency_key="cart_attr_1", request_id="ca1" + ), + json=payload, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + cart = response.json() + cart_id = cart["id"] + self.assertEqual(cart.get("attribution", {}).get("campaign_id"), "123") + + # Convert to checkout + checkout_payload = { + "cart_id": cart_id, + } + res = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="cart_attr_conv_1", request_id="ca_conv1" + ), + json=checkout_payload, + ) + self.assertEqual(res.status_code, 201, f"Response: {res.text}") + checkout = res.json() + self.assertIsNotNone(checkout.get("attribution")) + self.assertEqual( + checkout.get("attribution", {}).get("campaign_id"), "123" + ) + if __name__ == "__main__": absltest.main() diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 88de62d..55b0ba6 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -56,9 +56,6 @@ checkout_complete_request as checkout_comp_req, payment_complete_request as payment_comp_req, ) -from ucp_sdk.models.schemas.shopping.types import ( - payment_instrument as payment_instr_type, -) from ucp_sdk.models.schemas.shopping.ap2_mandate import Checkout as Ap2Checkout from ucp_sdk.models.schemas.shopping.buyer_consent import ( Checkout as BuyerConsentCheckoutResp, @@ -288,13 +285,13 @@ def _create_payment_payload(self) -> dict: payload = checkout_comp_req.CheckoutCompleteRequest( payment=payment_comp_req.PaymentCompleteRequest( instruments=[ - payment_instr_type.SelectedPaymentInstrument( - id="instr_1", - handler_id="mock_payment_handler", - type="card", - display={"brand": "Visa", "last_digits": "1234"}, - credential={"type": "token", "token": "success_token"}, - ) + { + "id": "instr_1", + "handler_id": "mock_payment_handler", + "type": "card", + "display": {"brand": "Visa", "last_digits": "1234"}, + "credential": {"type": "token", "token": "success_token"}, + } ] ), risk_signals={}, @@ -1530,6 +1527,251 @@ def _body(**extra: object) -> dict: self.assertEqual(response.status_code, 201, f"Response: {response.text}") self.assertIsInstance(response.json().get("id"), str) + def test_create_does_not_adopt_client_supplied_omit_members(self) -> None: + """A create carrying ucp_request: omit members must not adopt them. + + checkout.json marks continue_url, expires_at, messages and order as + ucp_request: omit, so the business owns them on the response. The create + handler must drop them from the request payload so they never echo in + the 201 response or persist into the stored session. + """ + client_values = { + "continue_url": "https://platform.example/client-chosen", + "expires_at": "2030-01-01T00:00:00Z", + "messages": [ + { + "type": "info", + "code": "custom", + "content": "client supplied text", + "severity": "recoverable", + } + ], + "order": { + "id": "order_client_chosen", + "checkout_session_id": "fake", + "permalink_url": "https://platform.example/order", + }, + } + + with self.client: + payload = self._create_checkout_payload( + "test_omit_members", [("rose", "Red Rose", 1000, 1)] + ).model_dump(mode="json", exclude_none=True) + payload.update(client_values) + + headers = self._get_headers(idempotency_key="omit_1", request_id="omit_1") + response = self.client.post( + "/checkout-sessions", + headers=headers, + json=payload, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + + self.assertNotEqual( + body.get("continue_url"), + client_values["continue_url"], + "continue_url is business owned", + ) + self.assertNotEqual( + body.get("expires_at"), + client_values["expires_at"], + "expires_at is business owned", + ) + contents = [ + m.get("content") + for m in body.get("messages", []) + if isinstance(m, dict) + ] + self.assertNotIn( + "client supplied text", + contents, + "messages are business owned", + ) + order = body.get("order") or {} + self.assertNotEqual( + order.get("id"), + client_values["order"]["id"], + "order is business owned", + ) + + # Verify persistence: read session back with GET + checkout_id = self.get_resource_id(body["id"]) + get_res = self.client.get( + f"/checkout-sessions/{checkout_id}", + headers=headers, + ) + self.assertEqual(get_res.status_code, 200, f"Response: {get_res.text}") + stored = get_res.json() + self.assertNotEqual( + stored.get("continue_url"), client_values["continue_url"] + ) + self.assertNotEqual(stored.get("expires_at"), client_values["expires_at"]) + stored_contents = [ + m.get("content") + for m in stored.get("messages", []) + if isinstance(m, dict) + ] + self.assertNotIn("client supplied text", stored_contents) + stored_order = stored.get("order") or {} + self.assertNotEqual(stored_order.get("id"), client_values["order"]["id"]) + + def test_create_ignores_client_currency_and_non_string_currency( + self, + ) -> None: + """Create with client/non-string currency must not override or 500. + + checkout.json marks currency with ucp_request: omit -- the merchant + determines it via config.get_default_currency(). Client-supplied string + currency (e.g. 'XTS') must be ignored, and non-string currency (e.g. 123) + must not raise ValidationError. + """ + with self.client: + # Client-supplied string currency is ignored (default 'USD' is used). + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="curr_1", request_id="curr_1" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1}], + "currency": "XTS", + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + self.assertEqual(response.json().get("currency"), "USD") + + # Non-string currency does not cause 500 ValidationError. + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="curr_2", request_id="curr_2" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1}], + "currency": 123, + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + self.assertEqual(response.json().get("currency"), "USD") + + def test_create_ignores_client_line_item_id_and_non_string_id(self) -> None: + """Create with line_items[].id assigns server ID; non-string never 500. + + types/line_item.json marks id with create: omit -- the server assigns it. + Client-supplied string id is ignored, and non-string id does not raise + ValidationError. + """ + with self.client: + # Client-supplied string line item id is ignored (server assigns UUID). + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="li_id_1", request_id="li_id_1" + ), + json={ + "line_items": [ + {"item": {"id": "rose"}, "quantity": 1, "id": "client_line_1"} + ], + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + line_items = body.get("line_items", []) + self.assertEqual(len(line_items), 1) + self.assertIsInstance(line_items[0].get("id"), str) + self.assertNotEqual(line_items[0].get("id"), "client_line_1") + + # Non-string line item id does not cause 500 ValidationError. + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="li_id_2", request_id="li_id_2" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1, "id": 123}], + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + line_items = body.get("line_items", []) + self.assertEqual(len(line_items), 1) + self.assertIsInstance(line_items[0].get("id"), str) + + def test_create_checkout_with_attribution(self) -> None: + """A checkout create carrying attribution returns 201 and persists.""" + attribution_data = { + "campaign_id": "18234567890", + "campaign_source": "google", + "campaign_medium": "cpc", + "campaign_name": "spring_2026", + "gclid": "EAIaIQobChMI...", + } + with self.client: + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="attr_1", request_id="attr_1" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1}], + "attribution": attribution_data, + }, + ) + self.assertEqual(response.status_code, 201, f"Response: {response.text}") + body = response.json() + self.assertIsNotNone(body.get("attribution")) + self.assertEqual( + body.get("attribution", {}).get("campaign_id"), "18234567890" + ) + + # Verify persistence + checkout_id = self.get_resource_id(body["id"]) + get_res = self.client.get( + f"/checkout-sessions/{checkout_id}", + headers=self._get_headers(request_id="attr_1_get"), + ) + self.assertEqual(get_res.status_code, 200, f"Response: {get_res.text}") + stored = get_res.json() + self.assertEqual( + stored.get("attribution", {}).get("campaign_id"), "18234567890" + ) + + def test_validation_failure_answers_with_ucp_envelope(self) -> None: + """A validation failure answers with the UCP envelope, not detail.""" + with self.client: + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="val_err_1", request_id="val_err_1" + ), + json={"line_items": "not-an-array"}, + ) + self.assertEqual(response.status_code, 422) + self.assertIn( + "application/json", response.headers.get("content-type", "") + ) + data = response.json() + self.assertNotIn("detail", data, "flat detail shape must be gone") + self.assertEqual( + data.get("ucp", {}).get("status"), "error", "ucp.status must be 'error'" + ) + self.assertEqual(data.get("ucp", {}).get("version"), app.version) + messages = data.get("messages", []) + self.assertTrue( + isinstance(messages, list) and len(messages) > 0, + "messages[] must carry the failure", + ) + msg = messages[0] + self.assertEqual(msg.get("type"), "error") + self.assertEqual(msg.get("code"), "INVALID_REQUEST") + self.assertEqual(msg.get("severity"), "unrecoverable") + self.assertIn( + "line_items", + msg.get("content", ""), + "content must name the offending member", + ) + if __name__ == "__main__": absltest.main() diff --git a/rest/python/server/server.py b/rest/python/server/server.py index 69d290f..2d828b1 100644 --- a/rest/python/server/server.py +++ b/rest/python/server/server.py @@ -22,6 +22,7 @@ from exceptions import UcpError from fastapi import FastAPI from fastapi import Request +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse import generated_routes.ucp_routes from routes.discovery import router as discovery_router @@ -43,6 +44,57 @@ ) +def _format_validation_loc(loc: tuple[int | str, ...]) -> str: + parts = list(loc) + if parts and parts[0] in ("body", "query", "path", "header"): + parts = parts[1:] + if not parts: + return str(loc[0]) if loc else "request" + path = "" + for p in parts: + if isinstance(p, int): + path += f"[{p}]" + else: + path = f"{path}.{p}" if path else str(p) + return path + + +@app.exception_handler(RequestValidationError) +async def request_validation_exception_handler( + request: Request, exc: RequestValidationError +): + """Handle validation errors and convert to the UCP error envelope.""" + del request # Unused. + error_lines = [] + for err in exc.errors(): + path = _format_validation_loc(err.get("loc", ())) + msg = err.get("msg", "Validation error") + error_lines.append(f"✖ {msg}\n → at {path}") + + error_content = ( + "\n".join(error_lines) if error_lines else "Request validation failed." + ) + logger.warning("Request payload failed validation:\n%s", error_content) + + return JSONResponse( + status_code=422, + content={ + "ucp": { + "version": config.get_server_version(), + "status": "error", + }, + "messages": [ + { + "type": "error", + "code": "INVALID_REQUEST", + "content": error_content, + "severity": "unrecoverable", + } + ], + }, + ) + + @app.exception_handler(UcpError) async def ucp_exception_handler(request: Request, exc: UcpError): """Handle UCP-specific exceptions and converts them to JSON responses.""" diff --git a/rest/python/server/services/cart_service.py b/rest/python/server/services/cart_service.py index 87998bf..9023093 100644 --- a/rest/python/server/services/cart_service.py +++ b/rest/python/server/services/cart_service.py @@ -110,9 +110,19 @@ async def create_cart( ) ) + # Exclude base and omit fields to prevent keyword argument collisions and + # ensure client-supplied omit members are dropped. cart_data = cart_req.model_dump( exclude={ "line_items", + "ucp", + "id", + "currency", + "totals", + "continue_url", + "expires_at", + "messages", + "links", } ) @@ -130,7 +140,7 @@ async def create_cart( ), id=cart_id, line_items=line_items, - currency="USD", + currency=config.get_default_currency(), totals=[ {"type": "subtotal", "amount": 0}, {"type": "total", "amount": 0}, diff --git a/rest/python/server/services/checkout_service.py b/rest/python/server/services/checkout_service.py index 32da3e4..730b4f9 100644 --- a/rest/python/server/services/checkout_service.py +++ b/rest/python/server/services/checkout_service.py @@ -210,7 +210,8 @@ async def create_checkout( source_context = checkout_req.context source_signals = checkout_req.signals source_attribution = checkout_req.attribution - source_currency = getattr(checkout_req, "currency", None) or "USD" + # `currency` carries `ucp_request: omit`, so the merchant determines it. + source_currency = config.get_default_currency() source_discounts = checkout_req.discounts # `id` carries `ucp_request: omit`, so the server assigns it and never @@ -227,7 +228,14 @@ async def create_checkout( item_id = li.item.id quantity = li.quantity parent_id = getattr(li, "parent_id", None) - li_id = getattr(li, "id", None) or str(uuid.uuid4()) + # When converting from a cart, preserve the cart line item id. + # On direct create, line item `id` carries `create: omit` so + # the server assigns it. + li_id = ( + li.id + if cart_id and hasattr(li, "id") and isinstance(li.id, str) + else str(uuid.uuid4()) + ) line_items.append( LineItemResponse( id=li_id, @@ -242,20 +250,19 @@ async def create_checkout( ) ) - # We exclude fields that the service explicitly manages or overrides to - # avoid keyword argument conflicts when constructing the response model. - # By excluding only these 'base' fields, we allow extension fields (like - # 'buyer' or 'discounts') to pass through dynamically via **checkout_data. + # We exclude fields that the service explicitly manages or overrides, as + # well as fields marked as `ucp_request: omit` in checkout.json + # (continue_url, expires_at, messages, order) to ensure the server is the + # authoritative source and client values do not bleed into the response. # # * Conflict Prevention: If we didn't exclude currency, id, or payment, # passing them via **checkout_data while also specifying them as keyword # arguments (e.g., currency=checkout_req.currency) would raise a # TypeError: multiple values for keyword argument. - # * Server Authority: Fields like status, totals, and links might be - # present in a client request (even if they shouldn't be), but the server - # is the source of truth. We exclude them from the dumped data to ensure - # we start with a "clean" calculated state (e.g., - # status=CheckoutStatus.IN_PROGRESS, totals=[]). + # * Server Authority: Fields like status, totals, links, continue_url, + # expires_at, messages, and order are merchant-owned. We exclude them from + # the dumped data to ensure we start with a clean calculated state and + # client-supplied omit members are dropped. # * Model Transformation: ucp in the request is usually just version # negotiation info, but in the response, it's a complex ResponseCheckout # object with capability metadata. We exclude the request version to @@ -277,6 +284,10 @@ async def create_checkout( "attribution", "cart_id", "discounts", + "continue_url", + "expires_at", + "messages", + "order", } ) @@ -395,21 +406,21 @@ async def create_checkout( platform=platform_config, fulfillment=fulfillment_resp, buyer=source_buyer.model_dump(exclude_none=True) - if source_buyer - else None, + if hasattr(source_buyer, "model_dump") + else source_buyer, context=source_context.model_dump(exclude_none=True) - if source_context - else None, + if hasattr(source_context, "model_dump") + else source_context, signals=source_signals.model_dump(exclude_none=True) - if source_signals - else None, + if hasattr(source_signals, "model_dump") + else source_signals, attribution=source_attribution.model_dump(exclude_none=True) - if source_attribution - else None, + if hasattr(source_attribution, "model_dump") + else source_attribution, cart_id=cart_id, discounts=source_discounts.model_dump(exclude_none=True) - if source_discounts - else None, + if hasattr(source_discounts, "model_dump") + else source_discounts, **checkout_data, )