Page MenuHomePhabricator

User::getRights() applies session rights restrictions to non-session users
Open, Needs TriagePublic

Description

The Session can filter the list of rights allowed to the session-user, which is used to implement grants in OAuth and BotPasswords.

But since all User objects fall back to $wgRequest, that means they all also fall back to the request Session and therefore the OAuth or BotPasswords rights restrictions apply to users it's not intended to.

Normally this probably makes little difference, since we seldom check the rights of users other than the session user. But it can be seen easily enough by using the Action API action=query&list=users&usprop=rights.

Ideally we'd only have User objects created by newFromSession() tied to the WebRequest, or at least to its Session. A potential complication is whether there's any code that loads the session-user via some method other than User::newFromSession() and checks that User object's rights.

Event Timeline

Anomie created this task.Mar 19 2019, 1:56 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 19 2019, 1:56 PM
Tgr added a subscriber: Tgr.Mar 19 2019, 4:49 PM

User::getBlockedStatus has some ugly checks to see whether the User object whose status has been queried is the same as the global user (to determine whether the IP from $wgRequest can be used to check for IP blocks).

Looks like that check was first added in rSVN56325, fixing a similar bug where it was using the request's IP when checking blocks for all User objects rather than only the session user. No indication as to whether they compared user names (versus mFrom === 'session' or the like) there for a specific reason or just because that seemed like the safest way to not maybe break something else.