feature #53851 [Security] Ignore empty username or password login attempts (llupa)

chalasr · chalasr · commit 12468903d7e2 · 2024-03-05T21:48:40.000+01:00
This PR was squashed before being merged into the 7.1 branch. Discussion ---------- [Security] Ignore empty username or password login attempts | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes/no/improvement | Deprecations? | no | Issues | N/A | License | MIT ## Context A person going through Symfony docs for the first time wanted to create their own `LoginFormType` as a next step in their learning Symfony journey and noticed that you can submit empty username/password with form login. They wanted to disallow this and tried to add validation. To validate a login form is not so straight forward as it either needs to be done with a custom authenticator (complex validation) _or_ user provider if the data checks are simple. After a simple discussion in Symfony slack with `@wouterj` I am opening this PR. The approach to immediately ignore login attempts with empty username or empty password has already been introduced in Symfony 7.0 in [this commit](a3a3856#diff-87efdd98e1b9520c9397f27a3e11df0be6e12737a6d1c25ac3321f9dfdc9c5c4). As always, I am happy to apply changes as requested! Commits ------- a804ca1 [Security] Ignore empty username or password login attempts
diff --git a/src/Symfony/Component/Security/Http/Authenticator/FormLoginAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/FormLoginAuthenticator.php
@@ -129,12 +129,20 @@ private function getCredentials(Request $request): array
 
         $credentials['username'] = trim($credentials['username']);
 
+        if ('' === $credentials['username']) {
+            throw new BadRequestHttpException(sprintf('The key "%s" must be a non-empty string.', $this->options['username_parameter']));
+        }
+
         $request->getSession()->set(SecurityRequestAttributes::LAST_USERNAME, $credentials['username']);
 
         if (!\is_string($credentials['password']) && (!\is_object($credentials['password']) || !method_exists($credentials['password'], '__toString'))) {
             throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['password_parameter'], \gettype($credentials['password'])));
         }
 
+        if ('' === (string) $credentials['password']) {
+            throw new BadRequestHttpException(sprintf('The key "%s" must be a non-empty string.', $this->options['password_parameter']));
+        }
+
         return $credentials;
     }
 
diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md
@@ -6,6 +6,7 @@ CHANGELOG
 
  * Add `#[IsCsrfTokenValid]` attribute
  * Add CAS 2.0 access token handler
+ * Make empty username or empty password on form login attempts return Bad Request (400)
 
 7.0
 ---
diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/FormLoginAuthenticatorTest.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/FormLoginAuthenticatorTest.php
@@ -42,6 +42,30 @@ protected function setUp(): void
         $this->failureHandler = $this->createMock(AuthenticationFailureHandlerInterface::class);
     }
 
+    public function testHandleWhenUsernameEmpty()
+    {
+        $this->expectException(BadRequestHttpException::class);
+        $this->expectExceptionMessage('The key "_username" must be a non-empty string.');
+
+        $request = Request::create('/login_check', 'POST', ['_username' => '', '_password' => 's$cr$t']);
+        $request->setSession($this->createSession());
+
+        $this->setUpAuthenticator();
+        $this->authenticator->authenticate($request);
+    }
+
+    public function testHandleWhenPasswordEmpty()
+    {
+        $this->expectException(BadRequestHttpException::class);
+        $this->expectExceptionMessage('The key "_password" must be a non-empty string.');
+
+        $request = Request::create('/login_check', 'POST', ['_username' => 'foo', '_password' => '']);
+        $request->setSession($this->createSession());
+
+        $this->setUpAuthenticator();
+        $this->authenticator->authenticate($request);
+    }
+
     /**
      * @dataProvider provideUsernamesForLength
      */
