[Security] Rework the remember me system

[wouterj]

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fixes part of #39308
License	MIT
Doc PR	tbd

As I said in #39308, I want to change the remember me system in Symfony 5.3. The remember me system has a couple big "problems":

1. It's hardwired into some Security classes like ContextListener. The RememberMeFactory adds a setRememberMe() method call to the DI config and the context listener calls methods on this. This is very coupled, instead of the decoupled nature of the rest of security.
2. Conditional conditions are combined with cookie creation in one class. This is especially hard in e.g. 2FA (where setting the cookie should be done after 2FA is completed, which is currently near impossible as it's directly bound to the conditional of being called after logging in).

The changes

• The first commits harden the current functional test suite of remember me, to avoid breaking it.
• I discovered a lot of similarity between remember me tokens and login links. That's why I've extracted the shared logic into a generic SignatureHasher in the 3rd commit.
• I then remodelled RememberMeAuthenticator to the login link system, which I think improves a lot and at least improves problem (2) - as the conditionals (RememberMeAuthenticator) is split from the cookie creation (RememberMeHandlerInterface).
• Finally, I added a new event (TokenDeauthenticatedEvent) to the ContextListener to avoid direct coupling - solving problem (1).

This removes any usage of remember me services, which can be deprecated along with the rest of the security system.

Usage

As with the authenticator manager: Nothing changes in the configuration

Usage of persistent token providers has been improved. First, configuration is provided (setting up services is no longer needed):

# before
services:
    Symfony\Bridge\Doctrine\Security\RememberMe\DoctrineTokenProvider:
        autowire: true

security:
    firewalls:
        main:
            remember_me:
                # ...
                token_provider: 'Symfony\Bridge\Doctrine\Security\RememberMe\DoctrineTokenProvider'

# after
security:
    firewalls:
        main:
            remember_me:
                # ...
                token_provider:
                    doctrine: true

Furthermore, a schema listener is created. Whenever the doctrine token provider is used, make:migration/doctrine:schema:update will automatically create the required table.

Some advanced usage of Remember me is also improved a lot (there is no real "before" here, consider looking at scheb/2fa to get an idea of the before). A few use-cases I took into account:

• If you ever need to programmatically create a remember me cookie, you can autowire RememberMeHandlerInterface and use createRememberMeCookie($user). This will make sure the remember me cookie is set on the final response (using the ResponseListener)
• The RememberMeListener previously was responsible for both determining if a cookie must be set and setting the cookie. This is now split in 2 listeners (checking is done by RememberMeConditionsListener). If RememberMeBadge is enabled, the cookie is set and otherwise it isn't. This allows e.g. SchebTwoFactorBundle to create a listener that catches whether remember me was requested, but suppress it until the 2nd factor is completed.

Todo

• Update UPGRADE and CHANGELOG
• Show before/after examples
• Investigate the conditional event registering of ContextListener. This forces to inject both the firewall and the global event dispatcher at the moment.
• Make sure old remember me tokens still function. As remember me tokens are long lived, we may need to provide backwards compatibility for at least Symfony 6.x. Update: it was decided to not include this for now: [Security] Rework the remember me system #40145 (comment)

cc @scheb @weaverryan as you both initiated this PR by sharing the problems with the current design.

[stof]

• This removes the capability of PersistentTokenRememberMeService. Do we want to reintroduce this, or do we think that this is secure enough as-is (considering the same ideas as with the login link).

PersistentTokenRememberMeService is the only way to be able to invalidate tokens. And unlike login links, the remember tokens cannot be short-lived as that would defeat their purpose. So I suspect that removing the feature might not be OK (even though it was never properly documented and so requires digging into the code to discover it, making it an advanced features).

[wouterj]

PersistentTokenRememberMeService is the only way to be able to invalidate tokens. And unlike login links, the remember tokens cannot be short-lived as that would defeat their purpose. So I suspect that removing the feature might not be OK (even though it was never properly documented and so requires digging into the code to discover it, making it an advanced features).

You can invalidate tokens by including properties in the signature. If any signature property changes, all previously made remember me tokens are invalidated. The difference being short-lived login links vs long-lived remember me tokens is an interesting one! I see the current persistent tokens are using the logic as explained in https://stackoverflow.com/a/244907/1149495 I'll see if I can find other examples/references about persistent remember me cookies and whether using signature properties can fulfill this (or we need to reintroduce a persistent token handler).

[weaverryan]

PersistentTokenRememberMeService is the only way to be able to invalidate tokens

As Wouter said, the signature properties could be used to invalidate tokens... but it could only be used to invalidate all tokens (e.g. by adding some lastLogoutAt property to User and having that in the signature properties). If I logged in at work and at home (using remember me on both) and then logged out at work, there would not be a way to fully invalidate JUST the remember me cookie for my work computer. We would only be able to invalidate all current remember me tokens. This is actually a UX problem with removing PersistentTokenRememberMeService for companies that want a more secure setup.

However, I will also say that the new system will make it easier for "everyone else" to have a more secure remember me system: we can document how to setup a system that would truly invalidate remember me tokens (via the signature properties) on log out (though, again, it would invalidate all remember me tokens for that user).

[weaverryan]

This looks incredibly promising!

[wouterj]

PR and description updated:

• I've reintroduced a persistent remember me implementation. I also tried to improve its use (default config + schema listener for automatic migration creation)
• I've reimplemented the conditions to set a remember me cookie (request parameter) and added a special way to opt-out.
• I've added lots of tests, CHANGELOG and PHPdoc.

[Nyholm]

Thank you.
I did my best reviewing this. I just had some minor comments and suggestions.

[weaverryan]

This looks ready to go 👍

[wouterj]

Thanks for the detailed review @chalasr :)

[chalasr]

Thank you @wouterj.

[scheb]

@wouterj Congratulations for having this merged! 🥳 Thank you for all of your work and thank you to everyone who gave feedback on this thread. I believe it's a great step forward!

[PhilETaylor]

cross posting scheb/2fa#75 for visibility :-) where a live app, upgraded to v5.3.0-BETA1 (locally, dont worry) now has a problem with the decorator and interface type hints.

[scheb]

cross posting scheb/2fa#75 for visibility :-) where a live app, upgraded to v5.3.0-BETA1 (locally, dont worry) now has a problem with the decorator and interface type hints.

As I mentioned on scheb/2fa#75, just to let the people here know, that's nothing to worry about since I've been in the loop from the beginning of this PR here and I have an (unfinished) branch preparred to make scheb/2fa-bundle compatible with Symfony 5.3. Whoever wants to test Symfony 5.3 with 2fa-bundle is welcome to use that branch.

[PhilETaylor]

Sorry :-) was too eager to test ... thanks everyone for the hard work - this looks amazing!

[zerkms]

What was the reason to serialise userFqcn in the cookie? It's not used anywhere and leaks application implementation details for no good reason. The same for expires.

[derrabus]

@zerkms Would you mind opening a new issue for this topic? Only few people monitor comments of already closed PRs.