fix(security): gate guest self-confirmed orders behind PENDING_REVIEW
Some checks failed
CI / Lint, type check, unit tests, coverage (pull_request) Successful in 2m47s
CI / E2E browser tests (pull_request) Failing after 1m44s

POST /api/guest-orders/{token}/pay is public (no JWT) and
confirmGuestPayment simply transitions PENDING_PAYMENT -> PROCESSING
without verifying any Swish payment was received. A guest can mark an
order paid without paying, and BilHej eats the PostNord cost for a
free letter.

Interim fix (Tier 1 Swish Commerce API integration is a separate,
larger effort): self-confirmed guest orders now enter PENDING_REVIEW
instead of PROCESSING, gating fulfillment on manual admin
confirmation.

Changes:
  - OrderStatus: add PENDING_REVIEW("pending_review")
  - OrderService.confirmGuestPayment: set PENDING_REVIEW instead of
    PROCESSING; do NOT call notifyOrderProcessing (no fulfillment yet)
  - AdminOrderStatusRules: allow PENDING_REVIEW -> PROCESSING,
    CANCELLED, FAILED; canRegisterShipment returns false for
    PENDING_REVIEW
  - AdminOrderWorkflowService: when admin advances PENDING_REVIEW ->
    PROCESSING, call notifyOrderProcessing to trigger fulfillment
  - GuestOrderPage.vue: status label 'Betalning mottagen, under
    granskning' for pending_review
  - OrderServiceTest: assert PENDING_REVIEW status + no notification
    after guest self-confirm
  - AdminOrderStatusRulesTest: 3 new tests for PENDING_REVIEW
    transitions (allowed targets, valid PROCESSING transition,
    rejected SENT transition)

The authenticated confirmPayment path is unchanged (acknowledged
Phase 0 honor-system with user traceability).

Closes #18
This commit is contained in:
Hermes Agent 2026-07-18 11:49:28 +00:00
parent 48c2a50c5d
commit 5825723420
7 changed files with 45 additions and 6 deletions

View file

@ -3,6 +3,7 @@ package se.bilhalsning.entity;
public enum OrderStatus { public enum OrderStatus {
PENDING_PAYMENT("pending_payment"), PENDING_PAYMENT("pending_payment"),
PAID("paid"), PAID("paid"),
PENDING_REVIEW("pending_review"),
PROCESSING("processing"), PROCESSING("processing"),
SENT("sent"), SENT("sent"),
DELIVERED("delivered"), DELIVERED("delivered"),

View file

@ -53,6 +53,7 @@ public final class AdminOrderStatusRules {
private static List<OrderStatus> allowedTargets(OrderStatus from, Order order) { private static List<OrderStatus> allowedTargets(OrderStatus from, Order order) {
return switch (from) { return switch (from) {
case PENDING_PAYMENT -> List.of(OrderStatus.FAILED); case PENDING_PAYMENT -> List.of(OrderStatus.FAILED);
case PENDING_REVIEW -> List.of(OrderStatus.PROCESSING, OrderStatus.CANCELLED, OrderStatus.FAILED);
case PROCESSING -> List.of(OrderStatus.FAILED); case PROCESSING -> List.of(OrderStatus.FAILED);
case SENT -> List.of(OrderStatus.DELIVERED, OrderStatus.FAILED); case SENT -> List.of(OrderStatus.DELIVERED, OrderStatus.FAILED);
case DELIVERED -> List.of(OrderStatus.FAILED); case DELIVERED -> List.of(OrderStatus.FAILED);

View file

@ -34,6 +34,9 @@ public class AdminOrderWorkflowService {
if (newStatus == OrderStatus.FAILED && previousStatus != OrderStatus.FAILED) { if (newStatus == OrderStatus.FAILED && previousStatus != OrderStatus.FAILED) {
orderNotificationService.notifyOrderFailed(saved); orderNotificationService.notifyOrderFailed(saved);
} }
if (newStatus == OrderStatus.PROCESSING && previousStatus == OrderStatus.PENDING_REVIEW) {
orderNotificationService.notifyOrderProcessing(saved);
}
return saved; return saved;
} }

View file

@ -64,6 +64,14 @@ public class OrderService {
* Honor-system payment confirmation for guest orders. Mirrors * Honor-system payment confirmation for guest orders. Mirrors
* {@link #confirmPayment(UUID, UUID)} but authenticates via the * {@link #confirmPayment(UUID, UUID)} but authenticates via the
* guest token instead of {@code userId}. * guest token instead of {@code userId}.
*
* <p>Guest self-confirmed orders enter {@link OrderStatus#PENDING_REVIEW}
* rather than {@link OrderStatus#PROCESSING} because the payment is
* not verified the customer simply clicked "I have paid" on the
* payment page. An admin must manually review and advance the order
* to PROCESSING (which triggers fulfillment notification). This
* prevents a guest from marking an order paid without paying and
* having it automatically sent to fulfillment.
*/ */
public Order confirmGuestPayment(UUID guestToken) { public Order confirmGuestPayment(UUID guestToken) {
Order order = getOrderByGuestToken(guestToken); Order order = getOrderByGuestToken(guestToken);
@ -71,10 +79,8 @@ public class OrderService {
throw new InvalidOrderStateException( throw new InvalidOrderStateException(
"Beställningen kan inte ändras i detta tillstånd"); "Beställningen kan inte ändras i detta tillstånd");
} }
order.setStatus(OrderStatus.PROCESSING); order.setStatus(OrderStatus.PENDING_REVIEW);
Order saved = orderRepository.save(order); return orderRepository.save(order);
orderNotificationService.notifyOrderProcessing(saved);
return saved;
} }
public List<Order> getOrdersByUserId(UUID userId) { public List<Order> getOrdersByUserId(UUID userId) {

View file

@ -30,6 +30,32 @@ class AdminOrderStatusRulesTest {
assertFalse(AdminOrderStatusRules.canRegisterShipment(order)); assertFalse(AdminOrderStatusRules.canRegisterShipment(order));
} }
@Test
void shouldAllowProcessingCancelledFailedFromPendingReview() {
Order order = orderWithStatus(OrderStatus.PENDING_REVIEW);
assertEquals(
java.util.List.of("pending_review", "processing", "cancelled", "failed"),
AdminOrderStatusRules.allowedStatusValues(order));
assertFalse(AdminOrderStatusRules.canRegisterShipment(order));
}
@Test
void shouldValidatePendingReviewToProcessingTransition() {
Order order = orderWithStatus(OrderStatus.PENDING_REVIEW);
assertDoesNotThrow(() ->
AdminOrderStatusRules.validateTransition(order, OrderStatus.PROCESSING));
}
@Test
void shouldRejectPendingReviewToSentTransition() {
Order order = orderWithStatus(OrderStatus.PENDING_REVIEW);
assertThrows(se.bilhalsning.exception.InvalidOrderStateException.class,
() -> AdminOrderStatusRules.validateTransition(order, OrderStatus.SENT));
}
@Test @Test
void shouldExposeFailedRecoveryOptionsWhenTrackingExists() { void shouldExposeFailedRecoveryOptionsWhenTrackingExists() {
Order order = orderWithStatus(OrderStatus.FAILED); Order order = orderWithStatus(OrderStatus.FAILED);

View file

@ -344,8 +344,8 @@ class OrderServiceTest {
Order result = orderService.confirmGuestPayment(token); Order result = orderService.confirmGuestPayment(token);
assertEquals(OrderStatus.PROCESSING, result.getStatus()); assertEquals(OrderStatus.PENDING_REVIEW, result.getStatus());
verify(orderNotificationService).notifyOrderProcessing(result); verify(orderNotificationService, never()).notifyOrderProcessing(any());
} }
@Test @Test

View file

@ -15,6 +15,8 @@ const statusLabel = computed(() => {
switch (order.value.status) { switch (order.value.status) {
case 'pending_payment': case 'pending_payment':
return 'Väntar på betalning' return 'Väntar på betalning'
case 'pending_review':
return 'Betalning mottagen, under granskning'
case 'paid': case 'paid':
case 'processing': case 'processing':
return 'Behandlas' return 'Behandlas'