Page MenuHomePhabricator

Respect the TitleBlacklist Extension in the FileImporter
Closed, ResolvedPublic5 Story Points

Description

Motivation:
Commons uploads are checked whether the title respects the rules of the TitleBlacklist extension. We should do the same.

Task
Respect the TitleBlacklist extension. That means, for users without the tboverwrite right, block all file moves when the TitleBlacklist contains the title.

@WMDE-Fisch already found out the following:
Currently the TitleBlacklist extension is not triggered when importing pages. The hook that the extension listens to is UserPermissionsErrorsExpensive triggered in TitlecheckPermissionHooks. We would need to implement that to make sure people do not upload bad titled images.

Event Timeline

Lea_WMDE created this task.Apr 17 2018, 3:13 PM
Restricted Application added a project: TCB-Team. · View Herald TranscriptApr 17 2018, 3:13 PM
WMDE-Fisch set the point value for this task to 5.
WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2018-04-17 board.

The Title::getUserPermissionsErrors method generally takes care of all things related to permission checks on title creation. Calling this method eventually triggers the hooks mentioned above. Errors in here can then be respected when importing and/or renaming files.

Triggering the right hooks here seems to be quite easy. But now I come across the question how to handle the error.

Right now ( like the AbuseFilter checks ) the TitleBlacklist check is done in the very last step when "really" importing the file. There is a default behavior for the permission errors that come out the checks run by the TItelBlacklist. These result in a page looking like that:

So question is: Do we keep it like that? Or do we want our own error handling ( and error view ) take over? In the latter we should at least stick to the error message I guess. :-)
@Lea_WMDE

After a quick talk to @Lea_WMDE we came to the conclusion, that this should best be treated as custom error because it should be recoverable like for example the duplicated title error. Therefore the validation will happen early in the title validation steps.

Change 427692 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Include title permission checks

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

Users with special are not blocked when using blacklisted words. The handling of that and the according warning message are part of T192933.

Change 427692 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Include title permission checks

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

@WMDE-Fisch Do I understand that right, that we are currently refusing everyone to move the file if the TitleBlacklist extension triggers something?

And @Tobi_WMDE_SW how can I test this? :)

@WMDE-Fisch Do I understand that right, that we are currently refusing everyone to move the file if the TitleBlacklist extension triggers something?

Nope, we only refuse blacklisted titles for users that do not have the "tboverwrite" right. So admins for example can still move files to blacklisted titles. ( that's also the normal behavior in the RL wiki world ). Everything else would be covered then by the T192933 ticket.

And @Tobi_WMDE_SW how can I test this? :)

So on http://mwfileimport.wmflabs.org/wiki/index.php/MediaWiki:Titleblacklist I have a blacklist configured that forbids the word "voldemort" in a title. .... So if you use a non-admin account you should get the blocking message when trying to set a title like that.

And @Tobi_WMDE_SW how can I test this? :)

So on http://mwfileimport.wmflabs.org/wiki/index.php/MediaWiki:Titleblacklist I have a blacklist configured that forbids the word "voldemort" in a title. .... So if you use a non-admin account you should get the blocking message when trying to set a title like that.

Or you try to import this nice picture of Voldemort with a mustache. :D https://commons.wikimedia.org/wiki/File:Voldemort.jpg

Lea_WMDE updated the task description. (Show Details)Apr 25 2018, 1:50 PM

Interesting picture! It did bring some styling issue into view though:

... also as you can see in the screenshot, the file name contains ".jpg" which results in ".jpg.jpg" once you save your new name. So as normally, we currently don't want to see the extension in the file name window.

Interesting picture! It did bring some styling issue into view though:

Yep, good point.

... also as you can see in the screenshot, the file name contains ".jpg" which results in ".jpg.jpg" once you save your new name. So as normally, we currently don't want to see the extension in the file name window.

Ohh that seems to be a general bug with that form. I will look into it.

Change 429200 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Allow parsed wikitext in RecoverableTitleException

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

Change 429201 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Always use filename without extension in ChangeFileNameForm

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

Change 429201 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Always use filename without extension in ChangeFileNameForm

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

Change 429200 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Allow parsed wikitext in RecoverableTitleException

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

Lea_WMDE closed this task as Resolved.May 3 2018, 1:51 PM
Lea_WMDE triaged this task as Normal priority.
Lea_WMDE moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2018-04-17 board.