Page MenuHomePhabricator

FileImporter thinks I’m an admin even though I’m not
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce:

  1. Go to a wiki where you’re not an admin.
  2. Find a local file that would otherwise be eligible for moving, e.g. https://en.wikipedia.org/wiki/File:CAEWW_12.jpeg (if you’re not an admin on enwiki).
  3. Click the Export to Wikimedia Commons tab.
  4. Scroll to the bottom of the page.

Actual Results:

  • There is a checkbox saying Delete file on source wiki in my name. (Selecting this will cause the file not to be deleted, of course, as the user has no right to do so.)
  • There is no checkbox to add {{NowCommons}} to the source description page.

Expected Results:

  • There’s a {{NowCommons}} checkbox but no delete checkbox.

Affected Users:

  • Everyone except source wiki admins.

Event Timeline

DannyS712 moved this task from Backlog to User rights on the MediaWiki-User-management board.

See also T256427 - something going on with user rights validation?

See also T256427

https://phabricator.wikimedia.org/T256427 is restricted, so I have no idea what’s in it.

Logan added a subscriber: Logan.Jun 26 2020, 12:19 AM
Ammarpad added a subscriber: Ammarpad.

Has nothing with that

Has nothing with that

Its user group/user right management and detection

Ammarpad claimed this task.Jun 26 2020, 6:31 AM
Ammarpad triaged this task as High priority.

There is a checkbox saying Delete file on source wiki in my name. (Selecting this will cause the file not to be deleted, of course, as the user has no right to do so.)

Of course. So this is not very urgent, but it should still be high. It's an error introduced by r606703

There is no checkbox to add {{NowCommons}} to the source description page.

They are mutually exclusive. So both can't be shown.

Change 607973 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/FileImporter@master] Set Status error if permission check returns false.

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

Change 607979 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Fix edit/delete permission checks allways succeeding

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

Change 608000 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] [POC] Clearly mark forbidden remote edits as not being ok/good

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

Change 607973 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Set Status error if permission check returns false.

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

Change 608048 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/FileImporter@master] Streamline status logic

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

Change 608000 abandoned by Thiemo Kreuz (WMDE):
[POC] Clearly mark forbidden remote edits as not being ok/good

Reason:
In favor of Ida16bec.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FileImporter/ /608000

Change 607979 abandoned by Thiemo Kreuz (WMDE):
Fix edit/delete permission checks allways succeeding

Reason:
In favor of Ida16bec.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FileImporter/ /607979

Change 608476 had a related patch set uploaded (by Awight; owner: Ammarpad):
[mediawiki/extensions/FileImporter@wmf/1.35.0-wmf.38] Set Status error if permission check returns false.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FileImporter/ /608476

awight updated the task description. (Show Details)Jun 30 2020, 10:07 AM
awight added a subscriber: awight.

I'm backporting this today.

Tacsipacsi updated the task description. (Show Details)Jun 30 2020, 10:12 AM

Change 608476 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@wmf/1.35.0-wmf.38] Set Status error if permission check returns false.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FileImporter/ /608476

Mentioned in SAL (#wikimedia-operations) [2020-06-30T11:26:39Z] <awight@deploy1001> Synchronized php-1.35.0-wmf.38/extensions/FileImporter: BACON: [[gerrit:608476|Set Status error if permission check returns false. (T256428)]] (duration: 00m 58s)

awight moved this task from Review to Demo on the WMDE-QWERTY-Sprint-2020-06-24 board.EditedJun 30 2020, 11:34 AM

The fix is live. Thanks again to @Tacsipacsi for this helpful bug report, and @Ammarpad for the patch!

Tacsipacsi closed this task as Resolved.Jun 30 2020, 3:04 PM

Thanks, it works.

Change 608048 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Streamline status logic

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