From cc7409502adfe58c9f3e7d8c646af140dfa1e9ac Mon Sep 17 00:00:00 2001 From: Damien Fa Date: Tue, 2 Mar 2021 00:08:58 +0100 Subject: [PATCH 1/2] changes rebased --- .../Bundle/SecurityBundle/CHANGELOG.md | 1 + .../Factory/LoginThrottlingFactory.php | 3 ++- .../Tests/Functional/FormLoginTest.php | 27 ++++++++++++------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index d36da75e80e25..fe22755dd3099 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * [BC break] Add `login_throttling.lock_factory` setting defaulting to `null` (instead of `lock.factory`) + * Add a `login_throttling.interval` (in `security.firewalls`) option to change the default throttling interval. * Add the `debug:firewall` command. * Deprecate `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command, use `UserPasswordHashCommand` and `user:hash-password` instead diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php index f8e3416e68819..d9c3efd5f0faf 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php @@ -54,6 +54,7 @@ public function addConfiguration(NodeDefinition $builder) ->children() ->scalarNode('limiter')->info(sprintf('A service id implementing "%s".', RequestRateLimiterInterface::class))->end() ->integerNode('max_attempts')->defaultValue(5)->end() + ->scalarNode('interval')->defaultValue('1 minute')->end() ->scalarNode('lock_factory')->info('The service ID of the lock factory used by the login rate limiter (or null to disable locking)')->defaultNull()->end() ->end(); } @@ -76,7 +77,7 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal $limiterOptions = [ 'policy' => 'fixed_window', 'limit' => $config['max_attempts'], - 'interval' => '1 minute', + 'interval' => $config['interval'], 'lock_factory' => $config['lock_factory'], ]; FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php index f79028cb20719..5c1955f282c4c 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php @@ -112,7 +112,7 @@ public function testFormLoginRedirectsToProtectedResourceAfterLogin(array $optio * @dataProvider provideInvalidCredentials * @group time-sensitive */ - public function testLoginThrottling($username, $password) + public function testLoginThrottling(string $username, string $password, int $attemptIndex) { if (!class_exists(LoginThrottlingListener::class)) { $this->markTestSkipped('Login throttling requires symfony/security-http:^5.2'); @@ -125,19 +125,28 @@ public function testLoginThrottling($username, $password) $form['_password'] = $password; $client->submit($form); - $client->followRedirect()->selectButton('login')->form(); - $form['_username'] = $username; - $form['_password'] = $password; - $client->submit($form); - $text = $client->followRedirect()->text(null, true); - $this->assertStringMatchesFormat('%sToo many failed login attempts, please try again in %d minute%s', $text); + if (1 === $attemptIndex) { + // First attempt : Invalid credentials (OK) + $this->assertStringMatchesFormat('%sInvalid credentials%s', $text); + } elseif (2 === $attemptIndex) { + // Second attempt : login throttling ! + $this->assertStringMatchesFormat('%sToo many failed login attempts, please try again in 8 minutes%s', $text); + } elseif (3 === $attemptIndex) { + // Third attempt with unexisting username + $this->assertStringMatchesFormat('%sUsername could not be found.%s', $text); + } elseif (4 === $attemptIndex) { + // Fourth attempt : still login throttling ! + $this->assertStringMatchesFormat('%sToo many failed login attempts, please try again in 8 minutes%s', $text); + } } public function provideInvalidCredentials() { - yield 'invalid_password' => ['johannes', 'wrong']; - yield 'invalid_username' => ['wrong', 'wrong']; + yield 'invalid_password' => ['johannes', 'wrong', 1]; + yield 'invalid_password_again' => ['johannes', 'also_wrong', 2]; + yield 'invalid_username' => ['wrong', 'wrong', 3]; + yield 'invalid_password_again_bis' => ['johannes', 'wrong_again', 4]; } public function provideClientOptions() From d1a0342e1eaf23b6f698835f86945c2ddba80165 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Tue, 2 Mar 2021 14:27:07 +0100 Subject: [PATCH 2/2] Fix tests --- .../Tests/Functional/FormLoginTest.php | 60 ++++++++++--------- .../StandardFormLogin/login_throttling.yml | 1 + 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php index 5c1955f282c4c..05b27e82acf47 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php @@ -109,10 +109,9 @@ public function testFormLoginRedirectsToProtectedResourceAfterLogin(array $optio } /** - * @dataProvider provideInvalidCredentials * @group time-sensitive */ - public function testLoginThrottling(string $username, string $password, int $attemptIndex) + public function testLoginThrottling() { if (!class_exists(LoginThrottlingListener::class)) { $this->markTestSkipped('Login throttling requires symfony/security-http:^5.2'); @@ -120,35 +119,40 @@ public function testLoginThrottling(string $username, string $password, int $att $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'login_throttling.yml', 'enable_authenticator_manager' => true]); - $form = $client->request('GET', '/login')->selectButton('login')->form(); - $form['_username'] = $username; - $form['_password'] = $password; - $client->submit($form); - - $text = $client->followRedirect()->text(null, true); - if (1 === $attemptIndex) { - // First attempt : Invalid credentials (OK) - $this->assertStringMatchesFormat('%sInvalid credentials%s', $text); - } elseif (2 === $attemptIndex) { - // Second attempt : login throttling ! - $this->assertStringMatchesFormat('%sToo many failed login attempts, please try again in 8 minutes%s', $text); - } elseif (3 === $attemptIndex) { - // Third attempt with unexisting username - $this->assertStringMatchesFormat('%sUsername could not be found.%s', $text); - } elseif (4 === $attemptIndex) { - // Fourth attempt : still login throttling ! - $this->assertStringMatchesFormat('%sToo many failed login attempts, please try again in 8 minutes%s', $text); + $attempts = [ + ['johannes', 'wrong'], + ['johannes', 'also_wrong'], + ['wrong', 'wrong'], + ['johannes', 'wrong_again'], + ]; + foreach ($attempts as $i => $attempt) { + $form = $client->request('GET', '/login')->selectButton('login')->form(); + $form['_username'] = $attempt[0]; + $form['_password'] = $attempt[1]; + $client->submit($form); + + $text = $client->followRedirect()->text(null, true); + switch ($i) { + case 0: // First attempt : Invalid credentials (OK) + $this->assertStringContainsString('Invalid credentials', $text, 'Invalid response on 1st attempt'); + + break; + case 1: // Second attempt : login throttling ! + $this->assertStringContainsString('Too many failed login attempts, please try again in 8 minutes.', $text, 'Invalid response on 2nd attempt'); + + break; + case 2: // Third attempt with unexisting username + $this->assertStringContainsString('Username could not be found.', $text, 'Invalid response on 3rd attempt'); + + break; + case 3: // Fourth attempt : still login throttling ! + $this->assertStringContainsString('Too many failed login attempts, please try again in 8 minutes.', $text, 'Invalid response on 4th attempt'); + + break; + } } } - public function provideInvalidCredentials() - { - yield 'invalid_password' => ['johannes', 'wrong', 1]; - yield 'invalid_password_again' => ['johannes', 'also_wrong', 2]; - yield 'invalid_username' => ['wrong', 'wrong', 3]; - yield 'invalid_password_again_bis' => ['johannes', 'wrong_again', 4]; - } - public function provideClientOptions() { yield [['test_case' => 'StandardFormLogin', 'root_config' => 'config.yml', 'enable_authenticator_manager' => true]]; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/login_throttling.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/login_throttling.yml index 4848567cf3360..c445ce6963841 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/login_throttling.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/login_throttling.yml @@ -10,3 +10,4 @@ security: default: login_throttling: max_attempts: 1 + interval: '8 minutes'