Maybe a good time to also fix T180888: All permission checks should be able to return a custom error message?

Assuming that one of the goals is to avoid hidden dependencies on global state, the interface seems a bit optimistic about what a permission check will depend on. The session will have to be included (for Session::getAllowedUserRights()) and also the request ( for the IP, but possibly also things like whether it is cross-origin) - I think the current logic just assumes these always match the global context, so that can be done initially, but but's far from optional.

There's also User::isAllowed() which is probably used more often than it should be, but there are always going to be permission checks which do not relate to a page. And User::isEveryoneAllowed(). And User::getBlock()/User::getBlockedStatus() (fun fact: $user->getBlock()->preventsEdit( $title ) is not the same as $user->isBlockedFrom( $title, 'edit' )). And User::isLocallyBlockedProxy() although that can probably be deprecated as a public method. And User::getGlobalBlock()/User::isBlockedGlobally() (which is not checked by User::getBlock() because that would be too simple). And User::isLocked() (not sure if this counts as permission and it does not involve titles, but exposing it in the permission manager interface would at least reduce the number of security bugs where people forget it exists).

Also Title::userCan() has a $rigor parameter, which is not pretty, but I doubt you can just drop it.

Also there is the whole protection/restriction system, which is implemented completely independently, but conceptually very much belongs to permission management.

See also T212311: Blocks should be an implementation detail of an abstract authorization system and T212341: Action authorization has multiple code paths to different results (and my comment there) if there's appetite to do a bigger refactoring of the authz system instead of piecemeal changes.

Another thing that would be nice to figure out if there's an authz refactoring is T212639: Separate could do / can do / is doing permission checks in MediaWiki.

In T208768#4844079, @Tgr wrote:

Another thing that would be nice to figure out if there's an authz refactoring is T212639: Separate could do / can do / is doing permission checks in MediaWiki.

This could perhaps be done by adding an optional $mode parameter to userCan(): userCan( UserIdentity, Title, $permission, $mode ); Possibly modes would be "could"/"can"/"do" as you suggest, or "quick"/"full"/"final", which more fits with the model used by Title now. We'd want constants of course, not strings. The default mode would be "can" resp "full": no corners cut, and no side effects triggered.

Change 495832 had a related patch set uploaded (by Vedmaka Wakalaka; owner: Vedmaka Wakalaka):
[mediawiki/core@master] PermissionManager service draft

https://gerrit.wikimedia.org/r/495832

Change 495832 had a related patch set uploaded (by Vedmaka Wakalaka; owner: Vedmaka Wakalaka):
[mediawiki/core@master] PermissionManager service draft

https://gerrit.wikimedia.org/r/495832

AHT are discussing implementing a BlockManager: T219441

Change 495832 merged by jenkins-bot:
[mediawiki/core@master] Introduce PermissionManager service

https://gerrit.wikimedia.org/r/495832

T220623: Unable to view certain pages on incubator.wikimedia.org (Fatal error: operator not supported) seems related.

In T208768#5101644, @Tgr wrote:

T220623: Unable to view certain pages on incubator.wikimedia.org (Fatal error: operator not supported) seems related.

I don't see how - the stack trace points to Title->checkPermissionHooks(string, User, array, string, boolean). This method is removed by the patch that introduces PermissionManager, I94479b44afb3. So the error must have happened before PermissionManager was introduced. In fact, the ticket says that "Users have encountered this since at least php-1.33.0-wmf.21", so long before PermissionManager.

But the fix for whatever is going wrong will probably now involve PermissionManager, since the relevant code moved there.

In T208768#5101644, @Tgr wrote:

T220623 seems related.

Probably not; per Timo, those issues have been in production since before this code was written, much less merged.

Since the related change was merged, do you think we should track this task as Resolved?

@Vedmaka: This task has subtasks which are still open.

In T208768#5149810, @Aklapper wrote:

@Vedmaka: This task has subtasks which are still open.

I think they are intended to be parent tasks. :)