diff --git a/CHANGELOG.md b/CHANGELOG.md index aff70c3..641e8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ 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. + ## [1.2.1] - 2026-10-05 ### Fixed @@ -45,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 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']