From f396497c7e620dd2023a5319ad87bfb3ea6990e4 Mon Sep 17 00:00:00 2001 From: Calmfox Date: Sat, 10 Oct 2026 18:29:12 +0200 Subject: [PATCH 1/2] Keep the proxy prefix when going back from "Reset two-factor" and "Turn off" Behind a reverse proxy that mounts the shop under a prefix (X-Forwarded-Prefix, e.g. example.com/store), the forms on the grid and the edit page used app.request.requestUri: the bare request path, which Symfony never prefixes. After the operation the administrator landed at the root of the domain, where another site may live. The forms now carry the page's full address (app.request.uri) and the controller accepts it when it points at this site. The test application trusts the test client as a proxy, so a functional test can mount it under a prefix. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 4 ++ src/Controller/ManageAction.php | 18 ++++++- .../update/two_factor_forms.html.twig | 2 +- .../grid/action/reset_two_factor.html.twig | 2 +- tests/Functional/PanelManagementTest.php | 47 +++++++++++++++++++ tests/TestApplication/config/config.yaml | 6 +++ 6 files changed, 75 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aff70c3..4caa331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project are documented in this file. The format foll ## [Unreleased] +### Fixed + +- Behind a reverse proxy that mounts the shop under a prefix (`X-Forwarded-Prefix`, e.g. `example.com/store`), *Reset two-factor* and *Turn off* (grid and edit page) went back to the bare request path, lost the prefix and landed on whatever lives at the root of the domain. The forms now carry the page's full address and the controller accepts it as long as it points at this site. + ## [1.2.1] - 2026-10-05 ### Fixed diff --git a/src/Controller/ManageAction.php b/src/Controller/ManageAction.php index cd73a68..71ffe40 100644 --- a/src/Controller/ManageAction.php +++ b/src/Controller/ManageAction.php @@ -67,11 +67,25 @@ public function __invoke(Request $request, int|string $id, string $operation): R ]); } + return new RedirectResponse($this->redirectTarget($request)); + } + + /** + * Back where the button was (list or edit page). The form carries the page's full address: + * behind a proxy that mounts the shop under a prefix (`X-Forwarded-Prefix`), a bare path + * would lose that prefix and land on whatever else lives at the root of the domain. + * Only this site is a valid target, never another one. + */ + private function redirectTarget(Request $request): string + { $target = (string) $request->request->get('_redirect'); - return new RedirectResponse(str_starts_with($target, '/') && !str_starts_with($target, '//') + $local = str_starts_with($target, '/') && !str_starts_with($target, '//'); + $thisSite = str_starts_with($target, $request->getSchemeAndHttpHost() . '/'); + + return $local || $thisSite ? $target - : $this->urlGenerator->generate('sylius_admin_admin_user_index')); + : $this->urlGenerator->generate('sylius_admin_admin_user_index'); } public static function csrfTokenId(string $operation, int|string $id): string diff --git a/templates/admin/admin_user/update/two_factor_forms.html.twig b/templates/admin/admin_user/update/two_factor_forms.html.twig index eedfb5b..e79857a 100644 --- a/templates/admin/admin_user/update/two_factor_forms.html.twig +++ b/templates/admin/admin_user/update/two_factor_forms.html.twig @@ -3,6 +3,6 @@ {% for operation in ['reset', 'disable'] %} {% endfor %} diff --git a/templates/admin/grid/action/reset_two_factor.html.twig b/templates/admin/grid/action/reset_two_factor.html.twig index 07cf156..6075785 100644 --- a/templates/admin/grid/action/reset_two_factor.html.twig +++ b/templates/admin/grid/action/reset_two_factor.html.twig @@ -2,7 +2,7 @@
- + diff --git a/tests/Functional/PanelManagementTest.php b/tests/Functional/PanelManagementTest.php index f5b2837..d71b0b1 100644 --- a/tests/Functional/PanelManagementTest.php +++ b/tests/Functional/PanelManagementTest.php @@ -62,6 +62,53 @@ public function testResetRemovesTheMethodsAndRequiresANewPairing(): void self::assertResponseRedirects('/admin/two-factor/setup', null, 'a reset account pairs again even with an optional policy'); } + /** + * Behind a reverse proxy that mounts the shop under a prefix (`X-Forwarded-Prefix: /store`), + * the forms must carry the page's full address: a bare request path would lose the prefix + * and the browser would land on whatever else lives at the root of the domain. + */ + public function testBehindAPrefixTheFormsGoBackToTheFullAddress(): void + { + $prefix = ['HTTP_X_FORWARDED_PREFIX' => '/store']; + $target = $this->createAdmin(self::TOTP_SECRET); + $this->client->loginUser($this->createAdmin(self::TOTP_SECRET), 'admin'); + + $crawler = $this->client->request('GET', '/admin/users/', [], [], $prefix); + self::assertResponseIsSuccessful(); + self::assertSame( + 'http://localhost/store/admin/users/', + $crawler->filter('form[action="/store/admin/users/' . self::id($target) . '/two-factor/reset"] input[name="_redirect"]')->attr('value'), + ); + + $crawler = $this->client->request('GET', '/admin/users/' . self::id($target) . '/edit', [], [], $prefix); + $form = $crawler->filter('form#calmfox-admin-two-factor-reset'); + self::assertSame('http://localhost/store/admin/users/' . self::id($target) . '/edit', $form->filter('input[name="_redirect"]')->attr('value')); + + // the proxy strips the prefix before the request reaches the application + $this->client->request('POST', '/admin/users/' . self::id($target) . '/two-factor/reset', [ + '_csrf_token' => $form->filter('input[name="_csrf_token"]')->attr('value'), + '_redirect' => $form->filter('input[name="_redirect"]')->attr('value'), + ], [], $prefix); + + self::assertResponseRedirects('http://localhost/store/admin/users/' . self::id($target) . '/edit'); + self::assertTrue($this->reload($target)->isTwoFactorSetupRequired()); + } + + public function testAnotherSiteIsNeverARedirectTarget(): void + { + $target = $this->createAdmin(self::TOTP_SECRET); + $this->client->loginUser($this->createAdmin(self::TOTP_SECRET), 'admin'); + + $crawler = $this->client->request('GET', '/admin/users/' . self::id($target) . '/edit'); + $form = $crawler->filter('form#calmfox-admin-two-factor-disable'); + $this->client->request('POST', '/admin/users/' . self::id($target) . '/two-factor/disable', [ + '_csrf_token' => $form->filter('input[name="_csrf_token"]')->attr('value'), + '_redirect' => 'https://evil.example/', + ]); + + self::assertResponseRedirects('/admin/users/'); + } + public function testTurningOffRemovesTheMethodsAndLeavesTheRestToThePolicy(): void { $target = $this->createAdmin(self::TOTP_SECRET); diff --git a/tests/TestApplication/config/config.yaml b/tests/TestApplication/config/config.yaml index cb4807c..05482e8 100644 --- a/tests/TestApplication/config/config.yaml +++ b/tests/TestApplication/config/config.yaml @@ -36,3 +36,9 @@ security: check_path: calmfox_admin_two_factor_login_check default_target_path: sylius_admin_dashboard enable_csrf: true + +# The test client comes from 127.0.0.1: trust it, so a test can mount the application under a +# prefix with X-Forwarded-Prefix the way a reverse proxy does. +framework: + trusted_proxies: '127.0.0.1' + trusted_headers: ['x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-forwarded-port', 'x-forwarded-prefix'] From 8a134841a66ff19a0bff406ebd4e70b6a73fa026 Mon Sep 17 00:00:00 2001 From: Calmfox Date: Sat, 10 Oct 2026 18:33:43 +0200 Subject: [PATCH 2/2] Prepare 1.2.2 --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4caa331..641e8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to this project are documented in this file. The format foll ## [Unreleased] +## [1.2.2] - 2026-10-10 + ### Fixed - Behind a reverse proxy that mounts the shop under a prefix (`X-Forwarded-Prefix`, e.g. `example.com/store`), *Reset two-factor* and *Turn off* (grid and edit page) went back to the bare request path, lost the prefix and landed on whatever lives at the root of the domain. The forms now carry the page's full address and the controller accepts it as long as it points at this site. @@ -49,7 +51,8 @@ All notable changes to this project are documented in this file. The format foll - Access to the 2FA pages granted by route name; the policy condition and the admin form renderer apply to administrators only. - English and Polish translations. -[Unreleased]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.2.1...HEAD +[Unreleased]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.2.2...HEAD +[1.2.2]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.2.1...v1.2.2 [1.2.1]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.2.0...v1.2.1 [1.2.0]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/calmfoxpl/sylius-admin-two-factor-plugin/compare/v1.0.0...v1.1.0