PermissionManager::getUserPermissions() caches user rights by ID, which means all anonymous or locally-non-existent users share a single cache entry. That will break if rights are granted or revoked based on e.g. IP address (such as in the case of IP blocks).
Solution: use namespaced cache keys. If the user ID is not 0, use 'u:' . $user->getId(). If it is 0, use 'anon:' . $user->getName().
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| PermissionManager should not cache anonymous rights under ID 0 | mediawiki/core | master | +50 -21 |
Tagging CPT for triage. This issue was introduced when factoring out PermissionManager for decoupling.
I note that the cache in question is an in-process cache, and typically, the rights that get cached are for the current user. So there doesn't seem to be cause of immediate alarm. It is however a conceptual problem that could easily lead to issues, e.g. if the cache is made to persist between requests.
Proposed solution: just use the user name instead of the ID as the key to the cache array.
It probably won't break anything in production but it's not that hard to imagine some extension looking up the rights of some arbitrary anonymous user early on in the request, and that later affecting the context user. So IMO it's a bug, although a fringe one.
Proposed solution: just use the user name instead of the ID as the key to the cache array.
In theory you could have both a logged-in and an anonymous user called 1.2.3.4 on very legacy systems. Also a fringe problem, but should be easy to avoid by namespacing the keys to anon / user.
Change 531312 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] WIP: PermissionManager should not cache anonymous rights under ID 0
Change 531312 merged by jenkins-bot:
[mediawiki/core@master] PermissionManager should not cache anonymous rights under ID 0