8000 [Security] REMEMBERME cookie does not get deleted using the "logout_on_user_change" option · Issue #26379 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] REMEMBERME cookie does not get deleted using the "logout_on_user_change" option #26379

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.

Sign up for GitHub

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

Jump to bottom
Closed
egonolieux opened this issue Mar 2, 2018 · 7 comments
Closed

[Security] REMEMBERME cookie does not get deleted using the "logout_on_user_change" option #26379

egonolieux opened this issue Mar 2, 2018 · 7 comments
Labels
Bug Security Status: Reviewed

Comments

@egonolieux
Copy link
egonolieux commented Mar 2, 2018 •
edited by javiereguiluz
Loading
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.4.4

To check if a user has changed in a security context, my User entity implements the EquatableInterface. Because the logout_on_user_change is set to true, I expect the user to be logged out if the isEqualTo method returns false. This is indeed the case, but only the session cookie seems to be reset and not the REMEMBERME cookie, causing the user to be logged back in. I consider this to be a security flaw and not consistent with manually logging out (e.g. /logout URL), in which the REMEMBERME cookie does get deleted.

It seems like an issue for this already exists, but was closed as fixed in 3.4.0.

This is from my log files:

[2018-03-01 01:43:10] request.INFO: Matched route "account". {"route":"account","route_parameters":{"_controller":"AppBundle\\Controller\\AccountController::accountAction","_route":"account"},"request_uri":"https://example.com/account","method":"GET"} []
[2018-03-01 01:43:10] security.DEBUG: Read existing security token from the session. {"key":"_security_main","token_class":"Symfony\\Component\\Security\\Core\\Authentication\\Token\\RememberMeToken"} []
[2018-03-01 01:43:10] security.DEBUG: Token was deauthenticated after trying to refresh it. {"username":"user@email.com","provider":"Symfony\\Bridge\\Doctrine\\Security\\User\\EntityUserProvider"} []
[2018-03-01 01:43:10] security.DEBUG: Remember-me cookie detected. [] []
[2018-03-01 01:43:10] security.INFO: Remember-me cookie accepted. [] []
[2018-03-01 01:43:10] security.DEBUG: Populated the token storage with a remember-me token. [] []
[2018-03-01 01:43:10] security.DEBUG: Stored the security token in the session. {"key":"_security_main"} []

My security config is pretty basic:

security:

    encoders:
        AppBundle\Entity\User:
            algorithm: bcrypt
            cost: 12

    firewalls:
        api:
            pattern: ^/api
            stateless: true
            anonymous: ~
            http_basic:
                provider: user_entity_provider
                realm: API
        content:
            pattern: ^/content
            security: false
        dev:
            pattern: ^/_profiler
            security: false
        main:
            provider: user_entity_provider
            stateless: false
            anonymous: ~
            logout_on_user_change: true
            form_login:
                login_path: /account/sign-in
                check_path: /account/sign-in
                default_target_path: /
                csrf_token_generator: security.csrf.token_manager
            logout: 
                path: /account/sign-out
                target: /
            remember_me:
                name: REMEMBERME
                secret: '%secret%'
                lifetime: 31536000 # 1 year.

    providers:
        user_entity_provider:
            entity:
                class: AppBundle:User
                property: emailAddress
@egonolieux egonolieux mentioned this issue Mar 2, 2018
Closed
< 10000 /div>
@dmaicher
Copy link
Contributor
dmaicher commented Mar 6, 2018 •
edited
Loading

We could invalidate the remember-me cookie in the case you mentioned above where the user changes in the middle of an active session. But there is also the case with starting a new session only based on the remember-me cookie:

Let's say the user does not have an active session but he only has a remember-me cookie. And somehow since the last active session the user "changed".

Now if the user starts a new session and is logged in only based on the remember-me cookie it seems very tricky to support isEqualTo to invalidate the cookie.

The cookie currently only contains 4 things:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php#L38

And for the "checksum" hash generation it uses the username and the password:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php#L56

So quite impossible to invalidate the token here by checking isEqualTo as there is no user to compare it to 😢

@aschempp
Copy link
Contributor

But there is also the case with starting a new session only based on the remember-me cookie

I don't think this is a problematic case. Logging the user in from rememberme is the same as login through username and password, a new token is created. The user must only be logged out on change if there is already a token in the session.

@Simperfit Simperfit mentioned this issue Apr 18, 2019
Closed
@Simperfit
Copy link
Contributor
Simperfit commented Apr 18, 2019 •
edited
Loading

@egonolieux could you please test #31172 and tell me if it's working for you, if it's the case I'm gonna commit tests into it. As far as I tested it seems to not authenticate though rememberme after I added the loginFail call after the token has been refreshed to null.

@chalasr chalasr mentioned this issue Aug 11, 2019
Closed
@Claicon
Copy link
Claicon commented Nov 26, 2019

This bug is still a thing in 4.4. Anything new here and possible ways to get that fixed? Remember me cookie still doesn't get deleted when using EquatableInterface and the new addition that automatically detects role changes is still doing the same.

@chalasr
Copy link
Member
chalasr commented Nov 27, 2019 •
edited
Loading

@dmaicher is right here: RememberMeListener is called before ContextListener, which implies that when a rememberme cookie is found, the user is never fetched from the session nor refreshed, it is just loaded using the username present in the cookie checksum.
So at this stage we have no original user to compare. This is definitely a bug that needs to be fixed, but it's not possible with the way the remember-me feature is currently designed, a patc would imply some important refactoring.
I'm looking at it, any suggestion welcome

Nevermind

@chalasr chalasr mentioned this issue Nov 28, 2019
Merged
@chalasr
Copy link
Member
chalasr commented Nov 28, 2019

See #34671

@fabpot fabpot closed this as completed Nov 30, 2019
fabpot added a commit that referenced this issue Nov 30, 2019
@fabpot
bug #34671 [Security] Fix clearing remember-me cookie after deauthent…
d2a5c05 
…ication (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Fix clearing remember-me cookie after deauthentication

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

If you are using the `remember_me` listener and the refreshed user is deauthenticated, you are still logged in because the remember-me cookie does not get cleared.
This fixes it.

Commits
-------

d625a73 [Security] Fix clearing remember-me cookie after deauthentication
@Claicon
Copy link
Claicon commented Jan 2, 2020

So this seems to be still a thing here (4.4.2) but only when changing the role. When changing the role of an active user in a session and reloading the page the remember-me cookie is being cleared but there's a new remember-me cookie detected afterwards and accepted again and session is still kinda active (is_fully_authenticated() returns false, is_authenticated() returns true).

[2020-01-02 13:08:48] security.DEBUG: Read existing security token from the session. {"key":"_security_main","token_class":"Symfony\\Component\\Security\\Guard\\Token\\PostAuthenticationGuardToken"} []
[2020-01-02 13:08:48] security.DEBUG: Cannot refresh token because user has changed. {"username":"Claicon","provider":"Symfony\\Bridge\\Doctrine\\Security\\User\\EntityUserProvider"} []
[2020-01-02 13:08:48] security.DEBUG: Token was deauthenticated after trying to refresh it. [] []
[2020-01-02 13:08:48] security.DEBUG: Clearing remember-me cookie. {"name":"REMEMBERME"} []
[2020-01-02 13:08:48] security.DEBUG: Checking for guard authentication credentials. {"firewall_key":"main","authenticators":1} []
[2020-01-02 13:08:48] security.DEBUG: Checking support on guard authenticator. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-01-02 13:08:48] security.DEBUG: Guard authenticator does not support the request. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-01-02 13:08:48] security.DEBUG: Remember-me cookie detected. [] []
[2020-01-02 13:08:48] security.INFO: Remember-me cookie accepted. [] []
[2020-01-02 13:08:48] security.DEBUG: Populated the token storage with a remember-me token. [] []
[2020-01-02 13:08:48] security.DEBUG: Stored the security token in the session. {"key":"_security_main"} []

After changing the role again and reloading the page it seems to be fine, remember-me cookie is cleared and token populated with an anonymous Token.

[2020-01-02 13:15:31] security.DEBUG: Read existing security token from the session. {"key":"_security_main","token_class":"Symfony\\Component\\Security\\Core\\Authentication\\Token\\RememberMeToken"} []
[2020-01-02 13:15:31] security.DEBUG: Cannot refresh token because user has changed. {"username":"Claicon","provider":"Symfony\\Bridge\\Doctrine\\Security\\User\\EntityUserProvider"} []
[2020-01-02 13:15:31] security.DEBUG: Token was deauthenticated after trying to refresh it. [] []
[2020-01-02 13:15:31] security.DEBUG: Clearing remember-me cookie. {"name":"REMEMBERME"} []
[2020-01-02 13:15:31] security.DEBUG: Checking for guard authentication credentials. {"firewall_key":"main","authenticators":1} []
[2020-01-02 13:15:31] security.DEBUG: Checking support on guard authenticator. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-01-02 13:15:31] security.DEBUG: Guard authenticator does not support the request. {"firewall_key":"main","authenticator":"App\\Security\\LoginFormAuthenticator"} []
[2020-01-02 13:15:31] security.INFO: Populated the TokenStorage with an anonymous Token. [] []

Can someone actually confirm this on their application? Works fine with changing username, active/banned flag and so on.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@fabpot @javiereguiluz @dmaicher @aschempp @Simperfit @egonolieux @chalasr @carsonbot @Claicon
0