Skip to content

Commit fbfc5e2

Browse files
authored
Validate QR shop endpoints via session instead of order token (#356)
2 parents d1f7753 + d3b7f90 commit fbfc5e2

5 files changed

Lines changed: 96 additions & 57 deletions

File tree

UPGRADE-2.2.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
# UPGRADE FROM 2.2.8 TO 2.2.9
2+
3+
1. Run `yarn install` and `yarn build` to rebuild the shop assets. The bundled
4+
`assets/shop/js/mollie/app.js` has changed and the shop will not work correctly until it is
5+
rebuilt.
6+
7+
1. The QR-code and thank-you shop endpoints no longer use the order `tokenValue` introduced in
8+
2.2.8. Ownership is now proven through the shop session, so both endpoints again accept
9+
`?orderId=` and validate it against the session:
10+
11+
- `GET /{_locale}/get-code` serves the current session cart and rejects a foreign `orderId`
12+
with `HTTP 403`; its JSON response returns `orderId`.
13+
- `GET /{_locale}/thank-you` expects `?orderId=`, validates it against the session, and
14+
returns `HTTP 404` when it is missing or does not match.
15+
16+
If you have overridden `app.js` or link to these endpoints yourself, switch back from
17+
`orderToken` to `orderId`.
18+
119
# UPGRADE FROM 2.2.7 TO 2.2.8
220

321
1. The shop payment webhook now verifies that the Mollie payment belongs to the referenced

assets/shop/js/mollie/app.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ $(function () {
44
var disableValidationMollieComponents = false;
55
let selectedValue = false;
66
let mollieData = $('.online-online-payment__container');
7-
let orderToken = null;
7+
let orderId = null;
88
let qrCodeInterval = null;
99
const initialOrderTotal = $('#sylius-summary-grand-total').text();
1010
const cardActiveClass = 'online-payment__item--active';
@@ -148,8 +148,8 @@ $(function () {
148148
.then((response) => response.json())
149149
.then((data) => {
150150
let qrCode = data.qrCode;
151-
if (orderToken === null) {
152-
orderToken = data.orderToken;
151+
if (orderId === null) {
152+
orderId = data.orderId;
153153
}
154154

155155
if (qrCode) {
@@ -158,7 +158,7 @@ $(function () {
158158
qrCodeMethod = 'iDeal';
159159
}
160160
createPopup(qrCode, qrCodeMethod);
161-
qrCodeInterval = setInterval(() => checkQrCode(url + '?orderToken=' + orderToken), 1000);
161+
qrCodeInterval = setInterval(() => checkQrCode(url + '?orderId=' + orderId), 1000);
162162
}
163163
});
164164
}
@@ -172,7 +172,7 @@ $(function () {
172172
let cartVariantDetails = document.getElementById('cart-variant-details')
173173
if (cartVariantDetails) {
174174
let thankYouPageUrl = cartVariantDetails.getAttribute('data-thankYouPage');
175-
window.location.href = thankYouPageUrl + '?orderToken=' + orderToken;
175+
window.location.href = thankYouPageUrl + '?orderId=' + orderId;
176176
}
177177
}
178178
});

src/Controller/Shop/PageRedirectController.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class PageRedirectController
2525
{
2626
private const ORDER_COMPLETED_STATE = 'completed';
2727

28+
private const QR_ORDER_ID_SESSION_KEY = 'sylius_mollie_qr_order_id';
29+
2830
public function __construct(
2931
private readonly RouterInterface $router,
3032
private readonly OrderRepositoryInterface $orderRepository,
@@ -33,30 +35,34 @@ public function __construct(
3335

3436
public function thankYouAction(Request $request, SessionInterface $session): RedirectResponse
3537
{
36-
$orderToken = $request->get('orderToken');
37-
$thankYouPageUrl = $this->router->generate('sylius_shop_order_thank_you');
38+
$orderId = $request->get('orderId');
3839

39-
if (null === $orderToken || '' === $orderToken) {
40-
throw new NotFoundHttpException('Order token is required.');
40+
if (null === $orderId || '' === $orderId ||
41+
(string) $session->get(self::QR_ORDER_ID_SESSION_KEY) !== (string) $orderId) {
42+
throw new NotFoundHttpException('Order not found.');
4143
}
4244

4345
/** @var OrderInterface|null $order */
44-
$order = $this->orderRepository->findOneByTokenValue($orderToken);
46+
$order = $this->orderRepository->findOneBy(['id' => $orderId]);
4547

4648
if (null === $order) {
47-
throw new NotFoundHttpException(sprintf('Order with token "%s" does not exist.', $orderToken));
49+
throw new NotFoundHttpException('Order not found.');
4850
}
4951

5052
$session->set('sylius_order_id', $order->getId());
51-
$payment = $order->getLastPayment();
52-
$tokenValue = $order->getTokenValue();
5353

54-
if ($payment?->getState() === self::ORDER_COMPLETED_STATE) {
55-
return new RedirectResponse($thankYouPageUrl);
54+
if ($order->getLastPayment()?->getState() === self::ORDER_COMPLETED_STATE) {
55+
return new RedirectResponse($this->router->generate('sylius_shop_order_thank_you'));
5656
}
5757

58-
$cartSummaryUrl = $this->router->generate('sylius_shop_order_show', ['tokenValue' => $tokenValue]);
58+
$tokenValue = $order->getTokenValue();
59+
60+
if (null === $tokenValue) {
61+
throw new NotFoundHttpException('Order not found.');
62+
}
5963

60-
return new RedirectResponse($cartSummaryUrl);
64+
return new RedirectResponse(
65+
$this->router->generate('sylius_shop_order_show', ['tokenValue' => $tokenValue]),
66+
);
6167
}
6268
}

src/Controller/Shop/QrCodeAction.php

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
final class QrCodeAction
3636
{
37+
private const QR_ORDER_ID_SESSION_KEY = 'sylius_mollie_qr_order_id';
38+
3739
public function __construct(
3840
private readonly MollieLoggerActionInterface $loggerAction,
3941
private readonly CartContextInterface $cartContext,
@@ -66,6 +68,7 @@ public function createPayment(Request $request): Response
6668
$qrCodeObject = $payment->details->qrCode;
6769
$this->setQrCodeOnOrder($order, $qrCodeObject->src);
6870
$this->setMolliePaymentIdOnOrder($order, $payment->id);
71+
$request->getSession()->set(self::QR_ORDER_ID_SESSION_KEY, $order->getId());
6972

7073
return new JsonResponse(['qrCode' => $qrCodeObject], Response::HTTP_OK);
7174
} catch (\Exception $e) {
@@ -80,15 +83,15 @@ public function fetchQrCodeFromOrder(Request $request): JsonResponse
8083
{
8184
/** @var OrderInterface|null $order */
8285
$order = $this->cartContext->getCart();
83-
$orderToken = $request->get('orderToken');
86+
$orderId = $request->get('orderId');
8487

85-
if (null !== $orderToken && '' !== $orderToken &&
86-
(null === $order || $order->getTokenValue() !== $orderToken)) {
87-
return new JsonResponse([], Response::HTTP_FORBIDDEN);
88+
if (null !== $orderId &&
89+
(string) $request->getSession()->get(self::QR_ORDER_ID_SESSION_KEY) !== (string) $orderId) {
90+
return new JsonResponse([], Response::HTTP_NOT_FOUND);
8891
}
8992

9093
return new JsonResponse(
91-
['qrCode' => $order?->getQrCode(), 'orderToken' => $order?->getTokenValue()],
94+
['qrCode' => $order?->getQrCode(), 'orderId' => $order?->getId()],
9295
Response::HTTP_OK,
9396
);
9497
}

tests/Unit/Controller/Shop/PageRedirectControllerTest.php

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
final class PageRedirectControllerTest extends TestCase
2828
{
29+
private const QR_ORDER_ID_SESSION_KEY = 'sylius_mollie_qr_order_id';
30+
2931
private MockObject&RouterInterface $router;
3032

3133
private MockObject&OrderRepositoryInterface $orderRepository;
@@ -37,97 +39,107 @@ protected function setUp(): void
3739
$this->router = $this->createMock(RouterInterface::class);
3840
$this->orderRepository = $this->createMock(OrderRepositoryInterface::class);
3941
$this->session = $this->createMock(SessionInterface::class);
42+
43+
$this->router->method('generate')->willReturnCallback(
44+
static fn (string $name, array $parameters = []): string => match ($name) {
45+
'sylius_shop_order_thank_you' => '/en_US/order/thank-you',
46+
'sylius_shop_order_show' => '/en_US/order/' . ($parameters['tokenValue'] ?? ''),
47+
default => '/',
48+
},
49+
);
4050
}
4151

42-
public function testItThrowsNotFoundWhenOrderTokenIsMissing(): void
52+
public function testItThrowsNotFoundWhenOrderIdIsMissing(): void
4353
{
4454
$this->expectException(NotFoundHttpException::class);
4555

46-
$this->orderRepository->expects(self::never())->method('findOneByTokenValue');
56+
$this->orderRepository->expects(self::never())->method('findOneBy');
4757

4858
$controller = $this->createController();
4959
$controller->thankYouAction(new Request(), $this->session);
5060
}
5161

52-
public function testItThrowsNotFoundWhenOrderTokenIsEmpty(): void
62+
public function testItThrowsNotFoundWhenOrderIdIsEmpty(): void
5363
{
5464
$this->expectException(NotFoundHttpException::class);
5565

56-
$this->orderRepository->expects(self::never())->method('findOneByTokenValue');
66+
$this->orderRepository->expects(self::never())->method('findOneBy');
5767

5868
$controller = $this->createController();
59-
$controller->thankYouAction(new Request(['orderToken' => '']), $this->session);
69+
$controller->thankYouAction(new Request(['orderId' => '']), $this->session);
6070
}
6171

62-
public function testItThrowsNotFoundWhenOrderTokenIsUnknown(): void
72+
public function testItThrowsNotFoundWhenOrderIdDoesNotMatchSession(): void
6373
{
6474
$this->expectException(NotFoundHttpException::class);
6575

66-
$this->orderRepository
67-
->expects(self::once())
68-
->method('findOneByTokenValue')
69-
->with('unknown-token')
70-
->willReturn(null);
76+
$this->session->method('get')->with(self::QR_ORDER_ID_SESSION_KEY)->willReturn(99);
77+
$this->orderRepository->expects(self::never())->method('findOneBy');
7178

7279
$controller = $this->createController();
73-
$controller->thankYouAction(new Request(['orderToken' => 'unknown-token']), $this->session);
80+
$controller->thankYouAction(new Request(['orderId' => 42]), $this->session);
7481
}
7582

76-
public function testItRedirectsToThankYouPageWhenPaymentIsCompleted(): void
83+
public function testItThrowsNotFoundWhenOrderDoesNotExist(): void
7784
{
78-
$order = $this->createOrderMock(42, 'abc123', 'completed');
85+
$this->expectException(NotFoundHttpException::class);
86+
87+
$this->session->method('get')->with(self::QR_ORDER_ID_SESSION_KEY)->willReturn(42);
88+
$this->orderRepository->method('findOneBy')->with(['id' => 42])->willReturn(null);
89+
90+
$controller = $this->createController();
91+
$controller->thankYouAction(new Request(['orderId' => 42]), $this->session);
92+
}
7993

80-
$this->orderRepository->method('findOneByTokenValue')->with('abc123')->willReturn($order);
94+
public function testItRedirectsToThankYouPageWhenPaymentIsCompleted(): void
95+
{
96+
$this->session->method('get')->with(self::QR_ORDER_ID_SESSION_KEY)->willReturn(42);
8197
$this->session->expects(self::once())->method('set')->with('sylius_order_id', 42);
82-
$this->router->method('generate')->with('sylius_shop_order_thank_you')->willReturn('/en_US/order/thank-you');
98+
99+
$order = $this->createOrderMock(42, 'abc123', 'completed');
100+
$this->orderRepository->method('findOneBy')->with(['id' => 42])->willReturn($order);
83101

84102
$controller = $this->createController();
85-
$response = $controller->thankYouAction(new Request(['orderToken' => 'abc123']), $this->session);
103+
$response = $controller->thankYouAction(new Request(['orderId' => 42]), $this->session);
86104

87105
self::assertSame(302, $response->getStatusCode());
88106
self::assertSame('/en_US/order/thank-you', $response->getTargetUrl());
89107
}
90108

91109
public function testItRedirectsToOrderShowWhenPaymentIsNotCompleted(): void
92110
{
93-
$order = $this->createOrderMock(42, 'abc123', 'new');
94-
95-
$this->orderRepository->method('findOneByTokenValue')->with('abc123')->willReturn($order);
111+
$this->session->method('get')->with(self::QR_ORDER_ID_SESSION_KEY)->willReturn(42);
96112
$this->session->expects(self::once())->method('set')->with('sylius_order_id', 42);
97-
$this->router->method('generate')->willReturnMap([
98-
['sylius_shop_order_thank_you', [], 1, '/en_US/order/thank-you'],
99-
['sylius_shop_order_show', ['tokenValue' => 'abc123'], 1, '/en_US/order/abc123'],
100-
]);
113+
114+
$order = $this->createOrderMock(42, 'abc123', 'new');
115+
$this->orderRepository->method('findOneBy')->with(['id' => 42])->willReturn($order);
101116

102117
$controller = $this->createController();
103-
$response = $controller->thankYouAction(new Request(['orderToken' => 'abc123']), $this->session);
118+
$response = $controller->thankYouAction(new Request(['orderId' => 42]), $this->session);
104119

105120
self::assertSame(302, $response->getStatusCode());
106121
self::assertSame('/en_US/order/abc123', $response->getTargetUrl());
107122
}
108123

109-
public function testItSetsSessionFromOrderIdNotFromRequestParameter(): void
124+
public function testItThrowsNotFoundWhenOrderHasNoTokenValue(): void
110125
{
111-
$order = $this->createOrderMock(99, 'abc123', 'completed');
126+
$this->expectException(NotFoundHttpException::class);
112127

113-
$this->orderRepository->method('findOneByTokenValue')->willReturn($order);
114-
$this->router->method('generate')->willReturn('/en_US/order/thank-you');
128+
$this->session->method('get')->with(self::QR_ORDER_ID_SESSION_KEY)->willReturn(42);
115129

116-
$this->session
117-
->expects(self::once())
118-
->method('set')
119-
->with('sylius_order_id', 99);
130+
$order = $this->createOrderMock(42, null, 'new');
131+
$this->orderRepository->method('findOneBy')->with(['id' => 42])->willReturn($order);
120132

121133
$controller = $this->createController();
122-
$controller->thankYouAction(new Request(['orderToken' => 'abc123']), $this->session);
134+
$controller->thankYouAction(new Request(['orderId' => 42]), $this->session);
123135
}
124136

125137
private function createController(): PageRedirectController
126138
{
127139
return new PageRedirectController($this->router, $this->orderRepository);
128140
}
129141

130-
private function createOrderMock(int $id, string $tokenValue, string $paymentState): MockObject&OrderInterface
142+
private function createOrderMock(int $id, ?string $tokenValue, string $paymentState): MockObject&OrderInterface
131143
{
132144
$payment = $this->createMock(PaymentInterface::class);
133145
$payment->method('getState')->willReturn($paymentState);

0 commit comments

Comments
 (0)