Description
Description
This is related to #18384 - when using persistent remember me cookies there is currently an issue when multiple requests are processed in parallel, this is the workflow:
- User logs in with remember me enabled, receives cookie with token A
- Session expires
- User then sends two or more requests in parallel, both containing the
rememberme=Acookie - The first request logs in the user again using the remember me cookie, and generates a new cookie with token B, which gets updated in the db.
- The other requests also come with an expired session, and
rememberme=Acookie, but now the stored token for the remember me series isB, so they fail to authenticate and those requests fail with aCookieTheftException
This leads to a few problems:
- First of all the CookieTheftException is really misleading in this case as there was no cookie theft.
- We are failing requests for the user which should work just fine
Application-level fix
We found a way to fix this by storing outdated tokens for a short time and treating them as still valid, so that the subsequent requests will simply update the token again. This still leaves the door open for race conditions if you're really unlucky, but it fixes most of the occurrences of this issue.
This is done like this:
- Read/store (in memory) rememberme cookie value before authenticators run
- Read the value after authenticators run, if it changed write an old value => new value pointer into the app cache
- Before authenticators run, if there is a value in cache for the current cookie's value we need to update the cookie in memory to make it look like it was sent with the new token.
Feature request
The above works fine for us, although a proper fix with optimistic locking might be even better to avoid multiple requests updating the cookie over and over..
The main issue is that this is done around the authenticators with kernel.request listeners and is therefore a little messy. PersistentRememberMeHandler is marked final so one can't extend the class to hook into things that way. It'd be great to have either have events when the cookie gets loaded so we could hook there and act only when the cookie is actually needed..
Or alternatively it would be also possible to do this cleaner if loadTokenBySeries was receiving the $token as well as the series, so we could inside our token provider return the old token as if it was still valid when a request with the old token shows up within seconds of a token update.
Note: I also sent #40972 to resolve a related issue
/cc @wouterj @chalasr it'd be great if we can move this forward before 5.3.0 if possible, I'm happy to help if needed.