Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Login check fails #36989

Closed
xf- opened this issue May 28, 2020 · 4 comments
Closed

Login check fails #36989

xf- opened this issue May 28, 2020 · 4 comments

Comments

@xf-
Copy link

xf- commented May 28, 2020

Symfony version(s) affected: 5.1.0-RC2 (PHP 7.4.6)

Description
Authentication passes, but i get logged out afterwards. I'm unsure why this happens.

How to reproduce

  1. I login and response (log & xdebug) says true.
  2. Redirected to dashboard route
  3. Redirected back to login

Used/tested permissions

  1. SensioFramework with IsGranted is used and at the beginning
  2. Removed IsGranted annotation and added a access_control to the security.yaml
  3. Remove all checks and got an exception that app.user is null (twig can't access app.user.username)

Downgrade to 5.1.0-RC1 resolves the issue/ works without a problem.

Additional context

[2020-05-28T02:49:50.403071+00:00] security.INFO: Guard authentication successful!
[2020-05-28T02:49:50.447793+00:00] request.INFO: Matched route "dashboard". {"route":"dashboard","route_parameters":{"_route":"dashboard","_controller":"App\\Controller\\DashboardController::index"},"request_uri":"https://dev.dev/dashboard","method":"GET"} []
[2020-05-28T02:49:50.449085+00:00] php.INFO: User Deprecated: Since symfony/security-http 5.1: The "Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface" interface is deprecated, create a listener for the "Symfony\Component\Security\Http\Event\LogoutEvent" event instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Since symfony/security-http 5.1: The \"Symfony\\Component\\Security\\Http\\Logout\\LogoutSuccessHandlerInterface\" interface is deprecated, create a listener for the \"Symfony\\Component\\Security\\Http\\Event\\LogoutEvent\" event instead. at /var/www/html/vendor/symfony/security-http/Logout/LogoutSuccessHandlerInterface.php:18)"} []
[2020-05-28T02:49:50.449329+00:00] php.INFO: User Deprecated: Since symfony/security-http 5.1: The "Symfony\Component\Security\Http\Logout\DefaultLogoutSuccessHandler" class is deprecated, use "Symfony\Component\Security\Http\EventListener\DefaultLogoutListener" instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Since symfony/security-http 5.1: The \"Symfony\\Component\\Security\\Http\\Logout\\DefaultLogoutSuccessHandler\" class is deprecated, use \"Symfony\\Component\\Security\\Http\\EventListener\\DefaultLogoutListener\" instead. at /var/www/html/vendor/symfony/security-http/Logout/DefaultLogoutSuccessHandler.php:18)"} []
[2020-05-28T02:49:50.449499+00:00] php.INFO: User Deprecated: Since symfony/security-http 5.1: Passing a logout success handler to "Symfony\Component\Security\Http\Firewall\LogoutListener::__construct" is deprecated, pass an instance of "Symfony\Contracts\EventDispatcher\EventDispatcherInterface" instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Since symfony/security-http 5.1: Passing a logout success handler to \"Symfony\\Component\\Security\\Http\\Firewall\\LogoutListener::__construct\" is deprecated, pass an instance of \"Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface\" instead. at /var/www/html/vendor/symfony/security-http/Firewall/LogoutListener.php:52)"} []
[2020-05-28T02:49:50.450213+00:00] php.INFO: User Deprecated: Since symfony/security-http 5.1: Calling "Symfony\Component\Security\Http\Firewall\LogoutListener::addHandler" is deprecated, register a listener on the "Symfony\Component\Security\Http\Event\LogoutEvent" event instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Since symfony/security-http 5.1: Calling \"Symfony\\Component\\Security\\Http\\Firewall\\LogoutListener::addHandler\" is deprecated, register a listener on the \"Symfony\\Component\\Security\\Http\\Event\\LogoutEvent\" event instead. at /var/www/html/vendor/symfony/security-http/Firewall/LogoutListener.php:81)"} []
[2020-05-28T02:49:50.450478+00:00] php.INFO: User Deprecated: Since symfony/security-http 5.1: Calling "Symfony\Component\Security\Http\Firewall\LogoutListener::addHandler" is deprecated, register a listener on the "Symfony\Component\Security\Http\Event\LogoutEvent" event instead. {"exception":"[object] (ErrorException(code: 0): User Deprecated: Since symfony/security-http 5.1: Calling \"Symfony\\Component\\Security\\Http\\Firewall\\LogoutListener::addHandler\" is deprecated, register a listener on the \"Symfony\\Component\\Security\\Http\\Event\\LogoutEvent\" event instead. at /var/www/html/vendor/symfony/security-http/Firewall/LogoutListener.php:81)"} []
[2020-05-28T02:49:50.452472+00:00] security.DEBUG: Checking for guard authentication credentials. {"firewall_key":"main","authenticators":1} []
[2020-05-28T02:49:50.452549+00:00] security.DEBUG: Checking support on guard authenticator. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-05-28T02:49:50.452584+00:00] security.DEBUG: Guard authenticator does not support the request. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-05-28T02:49:50.452877+00:00] security.DEBUG: Read existing security token from the session. {"key":"_security_main","token_class":"Symfony\\Component\\Security\\Guard\\Token\\PostAuthenticationGuardToken"} []
[2020-05-28T02:49:50.455954+00:00] doctrine.DEBUG: SELECT t0.id AS id_1,.......
[2020-05-28T02:49:50.457309+00:00] security.DEBUG: Cannot refresh token because user has changed. {"username":"test","provider":"Symfony\\Bridge\\Doctrine\\Security\\User\\EntityUserProvider"} []
[2020-05-28T02:49:50.457346+00:00] security.DEBUG: Token was deauthenticated after trying to refresh it. [] []
[2020-05-28T02:49:50.457487+00:00] security.INFO: Populated the TokenStorage with an anonymous Token. [] []
[2020-05-28T02:49:50.460327+00:00] security.DEBUG: Access denied, the user is not fully authenticated; redirecting to authentication entry point. {"exception":"[object] (Symfony\\Component\\Security\\Core\\Exception\\AccessDeniedException(code: 403): Access Denied. at /var/www/html/vendor/symfony/security-http/Firewall/AccessListener.php:117)"} []
[2020-05-28T02:49:50.460405+00:00] security.DEBUG: Calling Authentication entry point. [] []
[2020-05-28T02:49:50.496555+00:00] request.INFO: Matched route "app_login".
@wouterj
Copy link
Member

wouterj commented May 28, 2020

security.DEBUG: Cannot refresh token because user has changed. {"username":"test","provider":"Symfony\\Bridge\\Doctrine\\Security\\User\\EntityUserProvider"}

I think this is the most important log line. I think it somehow is related to #35944 Do you by any chance customize user roles in your application without saving it? Or can you maybe try to debug AbstractToken::hasUserChanged() to see why RC2 considers the user changed, while RC1 doesn't? (I'm also happy to help, but then we need some minimal application to reproduce the bug)

@xf-
Copy link
Author

xf- commented May 28, 2020

My test roles
Screenshot_20200528_201412

You are right
Screenshot_20200528_201524

role_hierarchy
Screenshot_20200528_201858

Enitity User
Screenshot_20200528_202155
(Docu changed a little bit, but the hierarchy should work)

Edit:
Looks like $this->user has only id email and password set. All other fields like active or createdAt are null/false

@wouterj
Copy link
Member

wouterj commented May 29, 2020

This has been reproduced in symfony/demo#1121, I'll take a look tonight

@stof
Copy link
Member

stof commented May 29, 2020

I think this happens when the serialization of the User class does not include roles.

nicolas-grekas added a commit that referenced this issue May 30, 2020
This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Security] Fixed AbstractToken::hasUserChanged()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36989
| License       | MIT
| Doc PR        | -

This PR completely reverts #35944.

That PR tried to fix a BC break (ref #35941, #35509) introduced by #31177. However, this broke many authentications (ref #36989), as the User is serialized in the session (as hinted by @stof). Many applications don't include the `roles` property in the serialization (at least, the MakerBundle doesn't include it).

In 5.2, we should probably deprecate having different roles in token and user, which fixes the BC breaks all together.

Commits
-------

f297beb [Security] Fixed AbstractToken::hasUserChanged()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants