Page MenuHomePhabricator

Use PermissionManager::addTemporaryUserRights() in MassMessage
Closed, ResolvedPublic

Description

MassMessage uses the UserGetRights hook to add a temporary user right. This is fragile (user rights might be cached by that point) and we now have a helper to do that, PermissionManager::addTemporaryUserRights(), so the usage should be replaced.

Details

Related Gerrit Patches:

Event Timeline

Tgr created this task.Jul 17 2019, 8:31 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 17 2019, 8:31 AM

I've tried to figure out how to patch it and looked at the docs but it was not clear to me how to do it so I gave up :)

Tgr added a comment.Jul 17 2019, 11:34 AM

Seems pretty straightforward to me:

$guard = $permissionManager->addTemporaryUserRights( $user, 'some-right' );
// do stuff that requires the rights
ScopedCallback::consume( $guard );

Maybe there should be some generic documentation about the scoped callback pattern that can be linked from methods returning a scoped callback; there's a ScopedCallback wiki page but it's not super informative.

MarcoAurelio added a comment.EditedJul 17 2019, 11:37 AM

@Tgr Thanks. It's not the docs but my lack of PHP skills :-)

I guessed this also required to specify at the top:

use MediaWiki\MediaWikiServices
use MediaWiki\Permissions\PermissionManager

But I'm not sure.

Legoktm claimed this task.Jul 18 2019, 12:32 AM

Change 524068 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/MassMessage@master] Use new PermissionManager::addTemporaryUserRights instead of hook

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

Change 524068 merged by jenkins-bot:
[mediawiki/extensions/MassMessage@master] Use new PermissionManager::addTemporaryUserRights instead of hook

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

Tgr closed this task as Resolved.Jul 19 2019, 9:23 AM