💡 Test gap + readability nit. This branch is the load-bearing fix for #18 (re-fires the 'Behandlas' notification only when an admin vouches for the order), and it is not covered by AdminOrderWorkflowServiceTest — the notifyOrderFailed branch above is tested, this one isn't. Consider adding shouldNotifyProcessingWhenAdminAdvancesFromPendingReview.
💡 Frontend test gap. GuestOrderPage.spec.ts asserts labels for paid/sent/delivered/failed/cancelled but not for the new pending_review case (renders 'Betalning mottagen, under granskning'). The switch default keeps existing tests green, but this new label is uncovered — worth adding it('shows "Betalning mottagen, under granskning" for pending_review status').
Verdict: Approve with minor test-coverage suggestions.
💡 The rate limiter is wired in but never actually tested. No test fires createLimit + 1 requests to verify a 429 is returned. For a security control, consider a dedicated GuestOrderRateLimitFilterTest with low limits that exercises the actual rejection path.
🟡 Unbounded map growth. Stale IP keys (pruned deques) are never removed from the map. With X-Forwarded-For spoofing, an attacker can cause unbounded memory growth by sending unique header values. Consider a scheduled cleanup task or a bounded cache (Caffeine maximumSize).
💡 Add Retry-After header. response.setHeader("Retry-After", "60") is trivial and helps well-behaved clients back off. RFC 6585 recommends it for 429 responses.
💡 Use the existing ErrorResponse DTO. The codebase has ErrorResponse(String message) and SecurityConfig uses objectMapper.writeValueAsString(new ErrorResponse(message)) for all error responses. Inject ObjectMapper and follow the same pattern to avoid hardcoded JSON and keep responses consistent.
🟡 X-Forwarded-For is trusted unconditionally. If the app is directly reachable (no stripping reverse proxy), an attacker can rotate this header per request to bypass rate limiting entirely. Consider a config flag like app.rate-limit.trust-forwarded-headers (default false).
Advisory review: ship with follow-up tracked for Phase 0 hardening.
💡 Suggestion: The backward-compat fallback (`sessionStorage.getItem('guestToken')
💡 Suggestion: <a href="#" @click.prevent> degrades screen-reader semantics (a link to # / nowhere). A <button> styled to match, or keeping <RouterLink> with a custom navigation guard, would preserve accessibility without changing the visual result.
💡 Suggestion: Consider sessionStorage.removeItem('guestToken') right after payGuestOrder(token) resolves, before the router.push. The token has been consumed at this point and lingering in sessionStorage only extends the credential's lifetime unnecessarily.