Skip to content

Commit

Permalink
feature #46118 [Security] Don't allow empty username or empty passwor…
Browse files Browse the repository at this point in the history
…d (bikalbasnet)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[Security] Don't allow empty username or empty password

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | yes
| Tickets       |  #46100
| License       | MIT
| Doc PR        | -
<!--
Replace this notice by a short README for your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against the latest branch.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Reopened from #46109 into `6.1` branch as this is not a bug rather a security feature

Commits
-------

db5afbd [Security] Don't allow empty username or empty password
  • Loading branch information
fabpot committed Jul 20, 2022
2 parents 2949e9c + db5afbd commit 5d9b6d2
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 0 deletions.
1 change: 1 addition & 0 deletions UPGRADE-6.2.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Security
* Add maximum username length enforcement of 4096 characters in `UserBadge` to
prevent [session storage flooding](https://symfony.com/blog/cve-2016-4423-large-username-storage-in-session)
* Deprecate the `Symfony\Component\Security\Core\Security` class and service, use `Symfony\Bundle\SecurityBundle\Security\Security` instead
* Passing empty username or password parameter when using `JsonLoginAuthenticator` is not supported anymore

Validator
---------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ private function getCredentials(Request $request)
throw new BadRequestHttpException(sprintf('The key "%s" must be provided.', $this->options['password_path']), $e);
}

if ('' === $credentials['username'] || '' === $credentials['password']) {
trigger_deprecation('symfony/security', '6.2', 'Passing an empty string as username or password parameter is deprecated.');
}

return $credentials;
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/Security/Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* Add maximum username length enforcement of 4096 characters in `UserBadge`
* Add `#[IsGranted()]`
* Deprecate empty username or password when using when using `JsonLoginAuthenticator`

6.0
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Security\Http\Tests\Authenticator;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
Expand All @@ -26,6 +27,8 @@

class JsonLoginAuthenticatorTest extends TestCase
{
use ExpectDeprecationTrait;

private $userProvider;
/** @var JsonLoginAuthenticator */
private $authenticator;
Expand Down Expand Up @@ -126,6 +129,26 @@ public function provideInvalidAuthenticateData()
yield [$request, 'Username too long.', BadCredentialsException::class];
}

/**
* @dataProvider provideEmptyAuthenticateData
*
* @group legacy
*/
public function testAuthenticationForEmptyCredentialDeprecation($request)
{
$this->expectDeprecation('Since symfony/security 6.2: Passing empty string for username or password is deprecated and will be removed in 7.0');
$this->setUpAuthenticator();

$this->authenticator->authenticate($request);
}

public function provideEmptyAuthenticateData()
{
$request = new Request([], [], [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json'], '{"username": "", "password": "notempty"}');
yield [$request];

}

public function testAuthenticationFailureWithoutTranslator()
{
$this->setUpAuthenticator();
Expand Down

0 comments on commit 5d9b6d2

Please sign in to comment.