feat(guest): guest checkout without login (Swish + QR) #17
Loading…
Reference in a new issue
No description provided.
Delete branch "feature/guest-checkout"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Anonymous customers currently can't order a bilhälsning without creating an account. This adds a guest checkout flow with Swish payment (QR + payment link).
Changes
Backend
GuestOrderController—POST /api/guest-orders(public, no auth)CreateGuestOrderRequest/GuestOrderResponseDTOsOrderentity —guest_email,guest_token(UUID), nullableuser_idOrderRepository—findByGuestToken,findByGuestEmailOrderService—createGuestOrder,getGuestOrderby tokenSecurityConfig—/api/guest-orders/**permitAllV12__add_guest_order_columns.sql— dropsuser_id NOT NULL, addsguest_email+guest_tokenwith partial unique index (backfill-safe for existing user-owned orders)Frontend
GuestCheckoutPage.vue— plate lookup + order form (no login)GuestPaymentRedirect.vue— Swish QR + payment link + status pollingGuestOrderPage.vue— order status by guest tokenguestOrders.ts— API clientrouter/index.ts—/guest/*public routesvite.config.ts— dev proxy for/api/guest-ordersVerification
vue-tsctype-check passes (exit 0)POST /api/guest-orders→ 201 → Swish → statusFrontend type-checks but the backend has NOT been compiled or run yet. Opening for review; backend smoke test pending in a docker environment.
Verdict: Well-structured guest-checkout scaffold — validation parity with the authed flow, defensive token lifecycle (only guest orders get a token, lookups refuse to serve user-owned orders), and a partial-unique-index migration that is backfill-safe. Frontend type-checks pass per the PR. Two blockers worth resolving before merge: the payment-confirmation path is honor-system (carried over from the authed path, but now on a public, JWT-less route), and the three new
OrderServicemethods have no tests.Critical
POST /api/guest-orders/{token}/payflips the order toPROCESSINGwithout verifying any Swish payment was received.confirmGuestPaymentsimply transitionsPENDING_PAYMENT → PROCESSINGon a button click. This is already the case on the authedconfirmPaymentpath, so it is pre-existing rather than introduced here — but exposing it on a public route where the only credential is a guess-resistant-but-not-secret token makes the impact worse, and a guest has no account to hold liable. The Javadoc already names it (Honor-system payment confirmation), so the author is aware. Cheapest mitigation that adds value without a full Swish Commerce integration: recordamountPaid+ apaid_attimestamp on confirm so finance can reconcile against the Swish payout report, and/or move confirmed-but-unverified orders into a distinctpending_reviewstatus visible in admin tooling rather thanPROCESSING.Warnings
OrderJavadoc states "Either userId or guestToken is set; never both, never neither", but onlyonCreate()enforces it in Java. A stray INSERT (admin tooling, future script, manual SQL) could violate it silently. SuggestALTER TABLE orders ADD CONSTRAINT chk_user_or_guest CHECK ((user_id IS NULL) <> (guest_token IS NULL));to enforce both halves at the DB.amountPaidis never set on payment confirmation (guest or authed).confirmGuestPaymentsets status toPROCESSINGbut leavesamountPaidnull, soGuestOrderResponse.amountPaidfor a paid order reads null. Same bug exists on the authed path, but this is the moment to fix both — setorder.setAmountPaid(BigDecimal.valueOf(swishAmount))(pull fromSwishInfo) before save.Suggestions
OrderServicemethods.OrderServiceTestalready exercisescreateOrder/confirmPayment/cancelOrder/updatePendingOrderwith the Mockito pattern. Mirror that forcreateGuestOrder(plate uppercased, email lowercased,guestTokenpopulated viaonCreate),getOrderByGuestToken(the 404-when-userId-non-null guard deserves its own regression test), andconfirmGuestPayment(transition +notifyOrderProcessingcall).request()auto-attachesAuthorization: Bearerto/guest-orders/*when the visitor is logged in. Harmless (permitAllignores it) but surprising for a no-JWT flow. OptionalnoAuthopt-out flag.route.query.token as stringcast inGuestPaymentRedirect.vue— vue-router returnsstring | string[]for query params. If a URL with?token=a&token=bis shared, the cast silently takes the array. Low impact (the fetch 404s), butArray.isArray(token) ? token[0] : token || ''is the safe form.guest_tokenonce the order reaches a terminal state (sent/delivered/cancelled) so stale magic links stop working; ensure server access logs for/gast-order/:tokenare rotated normally.vite.config.tshost: truebinds the dev server on all interfaces. Fine inside Docker, undesirable if anyone runsviteon a shared host. Consider gating onprocess.env.VITE_API_PROXY_TARGETorprocess.env.VITE_HOST.Looks Good
guest_token WHERE guest_token IS NOT NULLis the correct Postgres pattern and backfill-safe for the existing user-owned orders (all NULL tokens).onCreate()token generation is correctly scoped to guest orders only (guestToken == null && userId == null), so user-owned orders stay JWT-only — no cross-contamination of lookup paths.getOrderByGuestTokenrefuses to serve user-owned orders via a guest token (treats as 404) — good defensive design against future data corruption; deserves a regression test.CreateGuestOrderRequestvalidation mirrorsCreateOrderRequestexactly: same plate regex, same 1–1000 char envelope, plus@Emailand@Size(max=255)on the new email field.GuestOrderResponse.guestTokenis documented as returned only on create/by-token lookup — good principle even though the controller currently returns it uniformly.SecurityConfigplaces/api/guest-orders/**permitAll beforeanyRequest().authenticated()— correct matcher ordering.GuestPaymentRedirect.vuedefends against refresh on an already-paid order (bounces to the status page viafetchGuestOrder(token)pre-flight check).FlywayMigrationTestand confirmingmvn testgreen before merge.🔴 Critical — honor-system payment on a public route.
confirmGuestPaymenttransitionsPENDING_PAYMENT → PROCESSINGpurely on the client's button click, with no Swish payment verification. Pre-existing on the authedconfirmPayment, but this guest endpoint is JWT-less, so the only barrier is knowing the guest token. At minimum setamountPaid(and apaid_at) here so finance can reconcile against the Swish payout report; ideally gate the transition on a Swish Commerce payment request + callback.🟢 No tests for the new guest methods.
OrderServiceTestalready coverscreateOrder/confirmPayment/cancelOrder/updatePendingOrderwith the existing Mockito pattern. Please mirror that forcreateGuestOrder(plate uppercased, email lowercased,guestTokenset viaonCreate),getOrderByGuestToken(404 whenuserIdis non-null — the defensive guard deserves a regression test of its own), andconfirmGuestPayment(transition +notifyOrderProcessinginvocation).🟡 Missing DB-level orphan guard. The
OrderJavadoc asserts "Either userId or guestToken is set; never both, never neither" — but onlyonCreate()enforces this in Java. A stray INSERT (admin tooling, future script) can violate it. Suggest adding after the column additions:CI not passing
CI fix: added backend unit tests for guest checkout
Root cause: The PR added 3 new
OrderServicemethods,GuestOrderController(3 endpoints), and 2 DTO records — all without unit tests. This dropped backend coverage below thejacocoTestCoverageVerificationthresholds (70% line / 60% branch), failing theLint, type check, unit tests, coverageCI job.What I added (
a0cefb2):OrderServiceTest— 10 new tests:createGuestOrder: correct fields + PENDING_PAYMENT, plate normalization (uppercase+trim), email normalization (lowercase+trim), null email handlinggetOrderByGuestToken: successful lookup, not-found throwsOrderNotFoundException, security check — refuses to serve user-owned orders via guest tokenconfirmGuestPayment: success path (→PROCESSING + notification), non-pending status throwsInvalidOrderStateException, unknown token throwsOrderNotFoundExceptionGuestOrderControllerTest(new file) — 8 tests:POST /api/guest-orders: create without auth (201), validation rejections (bad plate, blank email, invalid email, blank letterText)GET /api/guest-orders/{token}: lookup (200), not-found (404)POST /api/guest-orders/{token}/pay: confirm (200), conflict (409), not-found (404)Verification:
vue-tsc --noEmit✅All tests follow existing patterns (MockitoExtension for service, SpringBootTest+MockMvc for controller).
Verdict — solid, well-structured guest checkout. The security model (opaque 122-bit token, partial unique index, refuse-to-serve user-owned orders via the guest path) is thoughtful and the backend cleanly parallels
OrderControllerwithout touching the JWT path. The main concerns are an unauthenticated create endpoint with no abuse protection, the guest token riding in the URL query string, and the honor-system/paythat should be hardened before real money. Advisory only — no blocking changes requested.Critical
POST /api/guest-ordersispermitAll, requires no auth, captcha, or proof-of-work, and writes a DB row per call. A trivial script can flood theorderstable (and any downstream cleanup/notification). Consider an IP rate limit, per-email throttle, requiring a successful vehicle/plate lookup first, or a lightweight captcha for the public create path. (inline)Warnings
POST /api/guest-orders/{token}/pay→confirmGuestPaymentflips the order toPROCESSINGwithout verifying any Swish payment was received. Anyone holding the token (or the customer themselves) can mark an order paid without paying — BilHej then eats the PostNord cost for a free letter. This is consistent with the existing authenticatedconfirmPaymentand acknowledged as Phase 0, but it's the core integrity gap to close (Swish Commerce API callback verification) before accepting real payments. Side note: neither this nor the authenticated path setsamountPaidor thePAIDstatus — worth aligning when payment verification lands. (inline)?token=…, so it lands in browser history and the nginx access log (and could leak viaRefererunless the defaultstrict-origin-when-cross-originpolicy holds). The magic-link landing/gast-order/:tokenis inherently URL-based, but the payment page doesn't have to be — prefersessionStorage/ a Pinia store / route state (or at minimum the URL fragment) so the credential isn't in a query string. (inline)Suggestions
idx_orders_guest_emailis indexed but never queried. The PR body listsOrderRepository — findByGuestToken, findByGuestEmail, but onlyfindByGuestTokenwas actually added — there's no read path onguest_emailyet (email confirmation is deferred). Either add the lookup now or defer the index to avoid an unused/unmeasured schema object. (inline)GuestCheckoutPageuses/\S+@\S+\.\S+/(e.g.a@borx@.ypass) while the DTO uses@Email, so inputs can pass client-side then fail server-side with a confusing message. Mirror a stricter regex or drop the client check and surface the backend message. (inline)PaymentRedirect.spec.ts/payment-redirect.spec.tsfor the authenticated path), but none ofGuestCheckoutPage,GuestPaymentRedirect,GuestOrderPage, orguestOrders.tshave specs. AGuestPaymentRedirectunit spec + a guest-checkout e2e would match the bar and protect the payment/token-handling logic.onMountedsequencing inGuestPaymentRedirect.fetchSwishInfo()andfetchGuestOrder(token)run sequentially in onetry/catch; if swish-info fails, the "already paid → redirect to status page" check never runs and the user is stuck on an error with no path to their order. Consider making the order-status check independent of swish-info loading.swishAmountis initialised to a hardcoded49— fine as a last-resort default, but iffetchSwishInfo()ever fails the page silently shows a stale price. Prefer a config/const or render the amount only after a successful fetch.Looks Good
WHERE guest_token IS NOT NULL, so existing NULL user-owned rows are untouched/backfill-safe), andgetOrderByGuestTokenreturning 404 when a token resolves to auserId-bearing order — actively prevents the guest path leaking user-owned order data.CreateOrderRequestexactly (plateregex,letterTextsize 1–1000); email normalised to lower-case/trimmed in the service. Validation tests cover invalid plate, blank email, invalid email, and blank text./api/guest-orders/**; theJwtAuthenticationFiltertolerates stale/malformed bearer tokens on public paths (swallowsJwtException, continues), so a lingeringlocalStoragetoken on a guest session doesn't break anything.DROP NOT NULLkeeps the FK, partial unique index is correct, backfill-safe for the existing dataset.{{ }}interpolation, nov-html), has a sensible already-paid redirect guard, and is honest that email confirmation is a later phase.OrderController/PaymentControllerwithout modifying the authenticated flows.🔴 Critical — public unauthenticated create with no abuse protection. This endpoint is
permitAlland writes a DB row per call with no rate limit, captcha, or pre-check. Trivial to abuse (orders-table flooding / cleanup cost). Suggest an IP rate limit + per-email throttle, requiring a successful plate/vehicle lookup first, or a lightweight captcha for the public create path.🟠 Honor-system pay — no payment verification.
confirmGuestPaymentmarks the orderPROCESSINGwithout confirming any Swish payment landed. Anyone with the token (or the customer) can mark an order paid without paying → BilHej mails a free letter. Acknowledged as Phase 0 and consistent with the authenticatedconfirmPayment, but harden with Swish Commerce callback verification before real money. Also:amountPaid/PAIDstatus are never set on either path.🔵 Unused index.
idx_orders_guest_emailis created butfindByGuestEmailwas never added toOrderRepository(the PR body lists it, but onlyfindByGuestTokenis there), so there's no read path onguest_emailyet. Either add the lookup now or defer the index until the email-link phase to avoid an unused schema object.🟠 Guest token in the URL query string. The token is the customer's sole credential but here it's pushed into
?token=…, so it lands in browser history and the nginx access log (and risksRefererleakage). The magic-link/gast-order/:tokenis inherently URL-based, but this payment page needn't be — prefersessionStorage/ Pinia store / route state (or a#fragment) to keep the credential out of the query string.🔵 Client email regex weaker than backend
@Email./\S+@\S+\.\S+/letsa@b/x@.ythrough client-side, only to fail server-side with a confusing message. Mirror a stricter pattern or drop the client check and surface the backend@Emailmessage.CI fails
Advisory review — guest checkout (Swish Tier 0 + QR)
Solid implementation with strong security guards (token entropy, cross-path leak prevention, input validation, good test coverage). The Swish QR config will cause scanning failures on desktop, and payment confirmation has no verification or rate limiting on the anonymous path — both worth addressing before this ships to real customers.
🔴 Critical
Swish QR won't scan on desktop —
GuestPaymentRedirect.vue:54-57:margin: 2(must be 4 per ISO/IEC 18004) + dark gray#111827(Swish spec says "black and white" →#000000). The Swish app's built-in scanner is stricter than a phone camera — a 2-module quiet zone fails to lock the finder pattern when scanning a QR displayed on a screen. This is the documented root cause of Swish-QR outages. Desktop users (the QR target audience) literally can't pay.width: 224is also borderline for a ~80–90 char pre-fill URL — prefer ≥256. Same config as the existingPaymentRedirect.vue; worth fixing both together. (See inline.)🟡 Warnings
POST /api/guest-orders— fully public, no throttle. An attacker can flood order creation (DB bloat/DoS) or mass-generate self-confirmed "paid" orders for free letters. Unlike the authenticated/api/payment/{id}/paypath there's no user account to ban. Add per-IP rate limiting (bucket4j / Spring@RateLimiter/ nginxlimit_req). (See inline.)POST /{token}/pay→confirmGuestPaymentsets status toPROCESSING+ triggersnotifyOrderProcessing(fulfillment) purely on self-confirmation, no Swish callback/API check. This mirrors the existing authenticated path (accepted Phase-0 design), but the guest path removes even account-level traceability. Until Tier 1 (Swish Commerce API + mTLS callback) lands, consider gating fulfillment — or flagging self-confirmed guest orders for manual review before mailing. (See inline.)amountPaidnever persisted —confirmGuestPaymentsets status but never callsorder.setAmountPaid(...). Every guest order response returnsamountPaid: null, even after "payment." Record the expected amount (fromapp.payment.letter-price) on confirmation so reconciliation can distinguish paid from never-attempted. (See inline.)normalizeSwishNumber(payment.ts:52) doesn't strip a leading+— ifapp.payment.swish-numberis stored as+46…, the+leaks into theswparam (URL-encoded%2B), which Swish rejects. Pre-existing inpayment.ts(not changed here) but now exercised by the guest flow. Addtrimmed.replace(/^\+/, '')before the prefix checks.🔵 Suggestions
QRCode.toDataURLsits inside theonMountedtry (line 38); if it throws, the catch at line 60 shows "Kunde inte ladda betalningsinformation," hiding the Swish link + manual fallback too. Wrap only the QR call so a QR failure still renders the payment link + manual number. (See inline.)findByGuestEmaillisted in PR body but not implemented — onlyfindByGuestTokenexists. The email→magic-link feature is deferred (noted inGuestPaymentRedirect.vue:181). Minor: update the PR body or add a TODO.?token=…) — lands in access logs, browser history, potentialRefererleakage. Acceptable for Phase 0; flag for when real payment verification lands.payGuestOrderposts to/pay.✅ Looks Good
getOrderByGuestTokenrefuses to serve user-owned orders (OrderService:56-59). Excellent defensive coding, with a dedicated test (shouldThrowWhenGuestTokenPointsToUserOwnedOrder).user_id NOT NULL(backfill-safe), partial unique index onguest_token WHERE NOT NULL, indexesguest_email. Clean and correct.@PrePersist. Well-documented in controller Javadoc.CreateGuestOrderRequest:@NotBlank,@Email,@Pattern(Swedish plate format),@Size(max=1000)on letterText. Frontend mirrors the same regex./api/guest-orders/**permitAll is narrow; admin routes still requireADMIN; everything else still authenticated.CODING_GUIDELINES.md. Tests cover happy + sad paths (validation rejections, 404, 409, normalization, null-email).🟡 No rate limiting on this public endpoint.
POST /api/guest-ordersis fully public (permitAll) with no throttle. An attacker can flood order creation (DB bloat / DoS) or mass-generate self-confirmed "paid" orders for free letters — and unlike the authenticated/api/payment/{id}/paypath, there's no user account to ban or audit.Add per-IP rate limiting (bucket4j, Spring
@RateLimiter, or nginxlimit_req).🟡 Honor-system pay — no payment verification.
confirmGuestPaymentsets status toPROCESSING+ triggersnotifyOrderProcessing(fulfillment) purely on self-confirmation. No Swish callback or API check. This mirrors the existing authenticated path (accepted Phase-0 design), but the guest path removes even account-level traceability — a customer gets a letter printed & mailed for 0 SEK with zero identity.Until Tier 1 (Swish Commerce API + mTLS callback) lands, consider not auto-triggering fulfillment here, or flagging self-confirmed guest orders for manual review before mailing.
🟡
amountPaidis never set on confirmation.confirmGuestPaymentsets status toPROCESSINGbut never callsorder.setAmountPaid(...). Every guest order response returnsamountPaid: null, even after "payment." Record the expected amount (fromapp.payment.letter-price) on confirmation so billing/reconciliation can distinguish paid-value from never-attempted.🔴 Swish QR quiet zone too small — won't scan on desktop.
margin: 2is half the ISO/IEC 18004 minimum of 4 modules. The Swish app's scanner is stricter than a phone camera and fails to lock the finder pattern when scanning a QR shown on a screen — this is the documented root cause of Swish-QR outages.Fix:
margin: 4. Also changecolor.darkfrom#111827(dark gray) to#000000(Swish spec: "black and white"), and bumpwidthto ≥256 for an ~80–90 char pre-fill URL. Same config asPaymentRedirect.vue— fix both.🔵 Isolate QR generation in its own try/catch.
This
toDataURLcall is inside theonMountedtry (line 38). If it throws, the catch at line 60 shows "Kunde inte ladda betalningsinformation" — hiding the Swish link + manual fallback too. Wrap only the QR call so a QR failure still renders the payment link and manual number.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.