feature #33308 [SecurityGuard] Deprecate returning non-boolean values from checkCredentials() (derrabus)
This PR was merged into the 4.4 branch.
Discussion
----------
[SecurityGuard] Deprecate returning non-boolean values from checkCredentials()
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | yes
| Tests pass? | yes
| Fixed tickets | prepares #33228
| License | MIT
| Doc PR | TODO
This PR suggests to deprecate implementations of `Symfony\Component\Security\Guard\AuthenticatorInterface::checkCredentials()` that return non-boolean values. This will allow us to add `bool` as return type declaration in the future.
https://github.com/nicolas-grekas/symfony/pull/26#discussion_r317084918
/cc @nicolas-grekas @weaverryan
Commits
-------
a0ca3afeca
Deprecate returning non-boolean values from checkCredentials().
This commit is contained in:
commit
610a4e978f
@ -194,6 +194,7 @@ Security
|
||||
|
||||
* The `LdapUserProvider` class has been deprecated, use `Symfony\Component\Ldap\Security\LdapUserProvider` instead.
|
||||
* Implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface` should add a new `needsRehash()` method
|
||||
* Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`. Please explicitly return `false` to indicate invalid credentials.
|
||||
|
||||
Stopwatch
|
||||
---------
|
||||
|
@ -467,6 +467,7 @@ Security
|
||||
* The `BCryptPasswordEncoder` class has been removed, use `NativePasswordEncoder` instead.
|
||||
* Classes implementing the `TokenInterface` must implement the two new methods
|
||||
`__serialize` and `__unserialize`
|
||||
* Implementations of `Guard\AuthenticatorInterface::checkCredentials()` must return a boolean value now. Please explicitly return `false` to indicate invalid credentials.
|
||||
|
||||
SecurityBundle
|
||||
--------------
|
||||
|
@ -11,6 +11,7 @@ CHANGELOG
|
||||
* Added `Guard\PasswordAuthenticatedInterface`, an optional interface
|
||||
for "guard" authenticators that deal with user passwords
|
||||
* Marked all dispatched event classes as `@final`
|
||||
* Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`.
|
||||
|
||||
4.3.0
|
||||
-----
|
||||
|
@ -83,9 +83,8 @@ interface AuthenticatorInterface extends AuthenticationEntryPointInterface
|
||||
/**
|
||||
* Returns true if the credentials are valid.
|
||||
*
|
||||
* If any value other than true is returned, authentication will
|
||||
* fail. You may also throw an AuthenticationException if you wish
|
||||
* to cause authentication to fail.
|
||||
* If false is returned, authentication will fail. You may also throw
|
||||
* an AuthenticationException if you wish to cause authentication to fail.
|
||||
*
|
||||
* The *credentials* are the return value from getCredentials()
|
||||
*
|
||||
|
@ -113,7 +113,11 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
|
||||
}
|
||||
|
||||
$this->userChecker->checkPreAuth($user);
|
||||
if (true !== $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
|
||||
if (true !== $checkCredentialsResult = $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
|
||||
if (false !== $checkCredentialsResult) {
|
||||
@trigger_error(sprintf('%s::checkCredentials() must return a boolean value. You returned %s. This behavior is deprecated in Symfony 4.4 and will trigger a TypeError in Symfony 5.', \get_class($guardAuthenticator), \is_object($checkCredentialsResult) ? \get_class($checkCredentialsResult) : \gettype($checkCredentialsResult)), E_USER_DEPRECATED);
|
||||
}
|
||||
|
||||
throw new BadCredentialsException(sprintf('Authentication failed because %s::checkCredentials() did not return true.', \get_class($guardAuthenticator)));
|
||||
}
|
||||
if ($this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordEncoder && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && method_exists($this->passwordEncoder, 'needsRehash') && $this->passwordEncoder->needsRehash($user)) {
|
||||
|
@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Guard\Tests\Provider;
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
||||
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
|
||||
use Symfony\Component\Security\Core\User\UserInterface;
|
||||
use Symfony\Component\Security\Guard\AuthenticatorInterface;
|
||||
use Symfony\Component\Security\Guard\Provider\GuardAuthenticationProvider;
|
||||
@ -87,6 +88,41 @@ class GuardAuthenticationProviderTest extends TestCase
|
||||
$this->assertSame($authedToken, $actualAuthedToken);
|
||||
}
|
||||
|
||||
public function testCheckCredentialsReturningFalseFailsAuthentication()
|
||||
{
|
||||
$this->expectException(BadCredentialsException::class);
|
||||
$providerKey = 'my_uncool_firewall';
|
||||
|
||||
$authenticator = $this->createMock(AuthenticatorInterface::class);
|
||||
|
||||
// make sure the authenticator is used
|
||||
$this->preAuthenticationToken->expects($this->any())
|
||||
->method('getGuardProviderKey')
|
||||
// the 0 index, to match the only authenticator
|
||||
->willReturn('my_uncool_firewall_0');
|
||||
|
||||
$this->preAuthenticationToken->expects($this->atLeastOnce())
|
||||
->method('getCredentials')
|
||||
->willReturn('non-null-value');
|
||||
|
||||
$mockedUser = $this->createMock(UserInterface::class);
|
||||
$authenticator->expects($this->once())
|
||||
->method('getUser')
|
||||
->willReturn($mockedUser);
|
||||
// checkCredentials is called
|
||||
$authenticator->expects($this->once())
|
||||
->method('checkCredentials')
|
||||
// authentication fails :(
|
||||
->willReturn(false);
|
||||
|
||||
$provider = new GuardAuthenticationProvider([$authenticator], $this->userProvider, $providerKey, $this->userChecker);
|
||||
$provider->authenticate($this->preAuthenticationToken);
|
||||
}
|
||||
|
||||
/**
|
||||
* @group legacy
|
||||
* @expectedDeprecation %s::checkCredentials() must return a boolean value. You returned NULL. This behavior is deprecated in Symfony 4.4 and will trigger a TypeError in Symfony 5.
|
||||
*/
|
||||
public function testCheckCredentialsReturningNonTrueFailsAuthentication()
|
||||
{
|
||||
$this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException');
|
||||
|
Reference in New Issue
Block a user