Page MenuHomePhabricator

"Conflict of user rights changes!" message is too easily missed.
Closed, ResolvedPublic

Description

I've had user's complain that when they changed a user's rights, it didn't stick. I wasn't able to reproduce it, so I didn't think too much of it.

Now, though, I think I've found what they were talking about. The first time I tried to change a user's groups, I got an error 500. Unfortunately, nothing was stored in the log. The second time, the change "didn't stick" but I didn't notice until I saw no change was made in the log at the bottom of the page.

I looked for some indication of a problem and finally found it. The message was "Conflict of user rights changes! Please review and confirm your changes."

The message, though, is not highlighted in any way and is easily missed since there is no other indication of a problem except for the missing log entry.

Event Timeline

MarkAHershberger raised the priority of this task from to Needs Triage.
MarkAHershberger updated the task description. (Show Details)
MarkAHershberger added a subscriber: MarkAHershberger.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 18 2015, 3:39 PM

I was able to reproduce the original error on a testing server and got

Fatal error: Call to a member function toString() on a non-object in .../includes/mail/UserMailer.php on line 203

This is on a wiki with Flow and Echo installed.

The problem that causes the message is when

$request->getVal( 'conflictcheck-originalgroups' )  !== implode( ',', $targetUser->getGroups()

So when the form is created, it stores the user's groups in conflictcheck-originalgroups and checks the user's current groups on form submission. Strangely, the groups stored in "conflictcheck-originalgroups" contains the one that I added.

Another user reported that they didn't see the user in the group even though I did. Since we're still using a per-server cache (yes, I know I need to set up memcached) I tried clearing the cache.

That worked, I don't see the user in the group. Changing the user's groups again resulted in the error 500 and returning to the page showed they are in the group but clicking "Save user groups" results in the conflict message that started this whole exploration.

So, somehow, the cache is being updated before the database change is actually made.

Got a backtrace from the error:
[fdfad37c] /wiki/Special:UserRights MWException from line 203 of .../includes/mail/UserMailer.php: Not an object: array (
'replyTo' =>
MailAddress::__set_state(array(
'address' => 'apache@server',
'name' => 'No Reply',
'realName' => '',
)),
)

Backtrace:

#0 .../extensions/Echo/Notifier.php(105): UserMailer::send(MailAddress, MailAddress, string, string, array)
#1 [internal function]: EchoNotifier::notifyWithEmail(User, EchoEvent)
#2 .../extensions/Echo/includes/controller/NotificationController.php(262): call_user_func_array(array, array)
#3 .../extensions/Echo/includes/controller/NotificationController.php(100): EchoNotificationController::doNotification(EchoEvent, User, string)
#4 .../extensions/Echo/includes/model/Event.php(150): EchoNotificationController::notify(EchoEvent, boolean)
#5 .../extensions/Echo/Hooks.php(513): EchoEvent::create(array)
#6 [internal function]: EchoHooks::onUserRights(User, array, array)
#7 .../includes/Hooks.php(209): call_user_func_array(string, array)
#8 .../includes/specials/SpecialUserrights.php(275): Hooks::run(string, array)
#9 .../includes/specials/SpecialUserrights.php(216): UserrightsPage->doSaveUserGroups(User, array, array, string)
#10 .../includes/specials/SpecialUserrights.php(172): UserrightsPage->saveUserGroups(string, string, User)
#11 .../includes/specialpage/SpecialPage.php(384): UserrightsPage->execute(NULL)
#12 .../includes/specialpage/SpecialPageFactory.php(582): SpecialPage->run(NULL)
#13 .../includes.php(267): SpecialPageFactory::executePath(Title, RequestContext)
#14 .../includes.php(566): MediaWiki->performRequest()
#15 .../includes.php(414): MediaWiki->main()
#16 .../index.php(41): MediaWiki->run()
#17 {main}

The above leads us to this change https://gerrit.wikimedia.org/r/#/c/223368/ ... Which is something that probably should have an entry in the release notes.

So, the problem here looks like a DB commit should be made when the cache is updated and before a hook is called.

Glaisher added a subscriber: Glaisher.
Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptFeb 27 2016, 8:47 AM
Tgr added a subscriber: Tgr.Jan 7 2019, 1:28 AM

Yeah this is definitely hard to notice. At least it should be in red.

Change 482575 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Make user rights conflict error a bit more obvious

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

Change 482575 merged by jenkins-bot:
[mediawiki/core@master] Make user rights conflict error a bit more obvious

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

Tgr closed this task as Resolved.Jan 7 2019, 6:11 PM
Tgr claimed this task.