diff --git a/CHANGELOG.md b/CHANGELOG.md index 49306f3..fffca2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ Latest * [#89](https://github.com/cleverage/ui-process-bundle/issues/89) `ProcessConfigurationsManager`: resolve the `ui.default` option with a normalizer instead of nested options defined with `setDefault()` (deprecated since symfony/options-resolver 7.3, removed in 8.0). With Symfony 8, a process launched with the UI form (`ui_launch_mode: form`) without `ui.default` no longer fails (`Cannot use object of type Closure as array`), and `ui.default` is validated again. Add tests. * [#91](https://github.com/cleverage/ui-process-bundle/issues/91) `LoginController`: pass `error` and `last_username` (`AuthenticationUtils`) to the login template, so that a failed login displays the error message and keeps the email. Test updated. +## BC break +* [#93](https://github.com/cleverage/ui-process-bundle/issues/93) Protect the login form and the logout against CSRF: `enable_csrf` on the prepended `form_login` and `logout` configurations, `csrf_token_intention` passed to the login template, `symfony/security-csrf` declared. Add tests. +* Please follow [UPGRADE.md v4.0](UPGRADE.md#v40) + v3.0.2 ------ diff --git a/UPGRADE.md b/UPGRADE.md index cff141b..e35d141 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,6 +1,53 @@ Upgrade Guide ============= +## v4.0 + +### CSRF protection of the login form + +The `form_login` configuration prepended by the bundle now enables `enable_csrf` (token id `authenticate`, parameter +`_csrf_token`), and the login template renders the `_csrf_token` field. The Symfony CSRF protection must be enabled +(`framework.csrf_protection`, enabled by default when the session is). + +If you override `@CleverAgeUiProcess/admin/login.html.twig` or submit the login form yourself, add the token: + +```twig + +``` + +To keep the previous behaviour, disable it on your `main` firewall: + +```yaml +# config/packages/security.yaml +security: + firewalls: + main: + form_login: + enable_csrf: false +``` + +### CSRF protection of the logout + +The `logout` configuration prepended by the bundle now enables `enable_csrf` (token id `logout`, parameter +`_csrf_token`): `/process/logout` without a valid token is refused (403). The logout link of the UI menu carries the +token. If you link to the logout from your own templates, use `logout_path()` (or `logout_url()`) instead of +`path('process_logout')`: + +```twig +Logout +``` + +To keep the previous behaviour, disable it on your `main` firewall: + +```yaml +# config/packages/security.yaml +security: + firewalls: + main: + logout: + enable_csrf: false +``` + ## v3.0 ### Import routes diff --git a/composer.json b/composer.json index c538091..23b471f 100644 --- a/composer.json +++ b/composer.json @@ -68,6 +68,7 @@ "symfony/scheduler": "^6.4 || ^7.4 || ^8", "symfony/security-bundle": "^6.4 || ^7.4 || ^8", "symfony/security-core": "^6.4 || ^7.4 || ^8", + "symfony/security-csrf": "^6.4 || ^7.4 || ^8", "symfony/security-http": "^6.4 || ^7.4 || ^8", "symfony/serializer": "^6.4 || ^7.4 || ^8", "symfony/string": "^6.4 || ^7.4 || ^8", diff --git a/docs/reference/03-users_and_security.md b/docs/reference/03-users_and_security.md index 696e148..db8a573 100644 --- a/docs/reference/03-users_and_security.md +++ b/docs/reference/03-users_and_security.md @@ -20,10 +20,12 @@ security: form_login: login_path: process_login check_path: process_login + enable_csrf: true logout: path: process_logout target: process_login clear_site_data: '*' + enable_csrf: true ``` So your application firewall must be named `main`. The only required application configuration is a password @@ -38,6 +40,14 @@ security: The UI pages are protected by `#[IsGranted]` attributes, no `access_control` rule is required. +The login form is protected by a CSRF token (token id `authenticate`, parameter `_csrf_token`): the Symfony CSRF +protection must be enabled (`framework.csrf_protection`, enabled by default when the session is). If you override the +login template, keep the `_csrf_token` field (the `csrf_token_intention` variable passed by `LoginController`). + +The logout is also protected by a CSRF token (token id `logout`, parameter `_csrf_token`): the logout link of the UI +menu carries it. To link to the logout from your own templates, use `logout_path()` (or `logout_url()`), which adds the +token, rather than `path('process_logout')`. + Roles ----- diff --git a/src/Controller/Admin/Security/LoginController.php b/src/Controller/Admin/Security/LoginController.php index b6ee314..e276bd0 100644 --- a/src/Controller/Admin/Security/LoginController.php +++ b/src/Controller/Admin/Security/LoginController.php @@ -30,6 +30,8 @@ public function __invoke(AuthenticationUtils $authenticationUtils): Response 'target_path' => '/process', 'error' => $authenticationUtils->getLastAuthenticationError(), 'last_username' => $authenticationUtils->getLastUsername(), + // Token id checked by the form_login authenticator (enable_csrf, see CleverAgeUiProcessExtension) + 'csrf_token_intention' => 'authenticate', ] ); } diff --git a/src/DependencyInjection/CleverAgeUiProcessExtension.php b/src/DependencyInjection/CleverAgeUiProcessExtension.php index 7d6f725..b7d22c3 100644 --- a/src/DependencyInjection/CleverAgeUiProcessExtension.php +++ b/src/DependencyInjection/CleverAgeUiProcessExtension.php @@ -124,11 +124,13 @@ public function prepend(ContainerBuilder $container): void 'form_login' => [ 'login_path' => 'process_login', 'check_path' => 'process_login', + 'enable_csrf' => true, ], 'logout' => [ 'path' => 'process_logout', 'target' => 'process_login', 'clear_site_data' => '*', + 'enable_csrf' => true, ], ], ], diff --git a/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php index eaf7d7a..e826928 100644 --- a/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php +++ b/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php @@ -179,8 +179,8 @@ public function testPrepend(): void 'main' => [ 'provider' => 'process_user_provider', 'custom_authenticator' => ['cleverage_ui_process.security.http_process_execution_authenticator'], - 'form_login' => ['login_path' => 'process_login', 'check_path' => 'process_login'], - 'logout' => ['path' => 'process_logout', 'target' => 'process_login', 'clear_site_data' => '*'], + 'form_login' => ['login_path' => 'process_login', 'check_path' => 'process_login', 'enable_csrf' => true], + 'logout' => ['path' => 'process_logout', 'target' => 'process_login', 'clear_site_data' => '*', 'enable_csrf' => true], ], ], $security[0]['firewalls'] diff --git a/tests/Functional/SecurityTest.php b/tests/Functional/SecurityTest.php index 6fd0e18..e084930 100644 --- a/tests/Functional/SecurityTest.php +++ b/tests/Functional/SecurityTest.php @@ -133,14 +133,46 @@ public function testLoginWithAnInvalidPassword(): void self::assertResponseRedirects('http://localhost/process/login'); } + public function testLoginWithAnInvalidCsrfToken(): void + { + $this->createUser('admin@example.com', ['ROLE_ADMIN'], 'secret'); + + $crawler = $this->client->request('GET', '/process/login'); + self::assertCount(1, $crawler->filter('input[type="hidden"][name="_csrf_token"]')); + $form = $crawler->filter('form')->form(['_username' => 'admin@example.com', '_password' => 'secret']); + $form['_csrf_token'] = 'invalid'; + $this->client->submit($form); + + self::assertResponseRedirects('http://localhost/process/login'); + $this->client->followRedirect(); + self::assertSelectorTextContains('.alert-danger', 'Invalid CSRF token.'); + $this->client->request('GET', '/process'); + self::assertResponseRedirects('http://localhost/process/login'); + } + public function testLogout(): void { $this->login(); - $this->client->request('GET', '/process/logout'); + // Logout link of the user menu, with the CSRF token + $crawler = $this->client->request('GET', '/process/process-execution'); + $link = $crawler->filter('a[href^="/process/logout?_csrf_token="]'); + self::assertGreaterThan(0, $link->count()); + $this->client->click($link->first()->link()); self::assertResponseRedirects('http://localhost/process/login'); $this->client->request('GET', '/process'); self::assertResponseRedirects('http://localhost/process/login'); } + + public function testLogoutWithoutCsrfToken(): void + { + $this->login(); + + $this->client->request('GET', '/process/logout'); + self::assertResponseStatusCodeSame(403); + + $this->client->request('GET', '/process/process-execution'); + self::assertResponseIsSuccessful(); + } }