Lockdown blocks extensions relying on API interaction
Closed, ResolvedPublic

Description

Edit: Extension that make use of the API error out with readapidenied, e.g. WikiEditor, MultimediaViewer, UploadWizard, etc

Using the latest version of Lockdown for 1.27 causes several errors in MediaWiki 1.27.1 .
This errors appear as soon as you load the extensions. So even without locking down any page.

  1. You are not able to use Visual Editor for edits. Pressing the save page button in the window "save changes" generates the following message:

    "Permission denied"
  1. It is also not possible to mark edits as patrolled. Pressing the link "Mark this page as patrolled" generates the markedaspatrollederrornotify error message:

    "Marking as patrolled failed."
Hedius created this task.Oct 18 2016, 8:11 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 18 2016, 8:11 PM
Aklapper updated the task description. (Show Details)Oct 22 2016, 8:54 PM
Kghbln added a subscriber: Kghbln.Oct 24 2016, 7:58 AM
Kghbln added a subscriber: daniel.Oct 24 2016, 8:01 AM

There seems to be a general issue when using the api to do actions. See

  1. Not working in MW 1.27 via API
  2. Permission denied error with MsUpload and Uploadwizard

So we probably need to rework the issue description and need help from @daniel to tackle this issue

If I understand correctly T115333 solves this issue since the origin of it is MediaWiki itself. So we need to wait for MW 1.27.2?

Kghbln renamed this task from Lockdown blocks Visual Editor and Patrolled edit to Lockdown blocks extensions relying on API interaction.Nov 20 2016, 4:41 PM
Kghbln triaged this task as High priority.EditedNov 20 2016, 4:51 PM

I have just tested with

  • MediaWiki 1.27.1 (577a8b6) 21:59, 16. Nov. 2016
  • PHP 5.6.27-0+deb8u1 (apache2handler)
  • Lockdown – (0d8aa13) 13:32, 6. Aug. 2016

and adjusted the task title. The following thread describes the issue best, i.e. as soon as an extension makes use of the API it errors out with readapidenied, e.g. WikiEditor, MultimediaViewer, UploadWizard, etc. This issue not only affects the restricted namespaces but all namespaces.

I had hoped that fixing T115333 would fix this issue, but it does not.

@daniel It will be really cool if you could have a peep at this!

Kghbln updated the task description. (Show Details)Nov 20 2016, 4:57 PM

I think there might still be an issue related to getEffectiveGroups():

  • Using api.php?action=query&meta=userinfo&uiprop=groups, I'm not getting all the groups of my user.
  • Using api.php?action=query&list=users&ususers=my_username&usprop=groups retrieves the correct groups.

Well, according to my testing just now, backporting Daniel's change 303369 to REL1_27 is enough to fix this issue. @Kghbln, can you confirm?

Well, according to my testing just now, backporting Daniel's change 303369 to REL1_27 is enough to fix this issue. @Kghbln, can you confirm?

Affirmative. After commenting out these two lines 1358 and 1359 on REL1_27 the WikiEditor extension came back to live. I assumed that 303369 was only necessary for REL1_28 onwards.

I was pretty naughty and added the MW 1.27 release tag to this issue.to make sure the backport of https://gerrit.wikimedia.org/r/#/c/303369/ is not being forgotten with the next release.

Well, I didn't just backport it myself because I wanted to make sure with @daniel that this isn't dependant on other changes, so I fully agree with your tagging :-)

This fix should not depend on any other changes, but I'm not sure the patch will apply cleanly.

Also, while I think it's fine to backport it, but I don't think it warrants a new bugfix release. So I don't know when it would actually become part of the official release.

aaron added a subscriber: aaron.Dec 6 2016, 3:38 PM

I backported it, but won't be it's own release I assume.

Kghbln closed this task as Resolved.Dec 6 2016, 4:46 PM
Kghbln assigned this task to aaron.

@daniel @aaron Indeed this alone does most probably not justify an immediate release. I believe we all agree on this from the start. I just added the tag to be sure that this issue does not get lost on the way when the next release is done, i.e. 1.27.2

@all involved here: I believe this is great collaboration what we have done here. Awesome! I am very happy and so will be a lot of other users for sure! Many thanks!!!!!

Kghbln reopened this task as Open.EditedDec 6 2016, 4:51 PM

Hmm, I am not sure what the best practice way is. Let's keep this open until 1.27.2 is out. Thus it stays visible on the board for the release.

Kghbln added a comment.Dec 6 2016, 5:17 PM

Hmm, I am not sure what the best practice way is. Let's keep this open until 1.27.2 is out. Thus it stays visible on the board for the release.

Well the backport is not merged yet so... I guess I was just to enthusiastic to notice this.

aaron added a comment.Dec 7 2016, 10:36 PM

Looks like the unit test failure in https://gerrit.wikimedia.org/r/#/c/325566/ is unrelated.

Kghbln added a comment.Dec 8 2016, 8:04 PM

Looks like the unit test failure in https://gerrit.wikimedia.org/r/#/c/325566/ is unrelated.

So hopefully there is a way out to get the code in. Keeping fingers crossed!

aaron closed this task as Resolved.Dec 20 2016, 3:02 PM

Finally merged.

@aaron Thanks a lot for your help! Greatly appreciated!!! Cool that hip hop is finally doing the test backflip too. :)

Emwiemaikel added a subscriber: Emwiemaikel.EditedFri, Jun 16, 11:16 AM

We looked into this issue and found that apparently the error occurs, if we hava sth. like this in your local settings.

$wgGroupPermissions['*']['read'] = false;

Lockdown seems to assume that the anonymous user is calling the API and will check this uer's effective group, which will also be '*' and thus Lockdown will most likely block the access.

We have a potential (hot) fix. I would like to push it to git and post a pull request. What is the best way to do that? Creating a new "bugfix/T148582" branch?

We have a potential (hot) fix. I would like to push it to git and post a pull request. What is the best way to do that? Creating a new "bugfix/T148582" branch?

We use gerrit, send a patch against master: https://www.mediawiki.org/wiki/Gerrit/Tutorial
The local branch name doesn't matter much.

Thanks for your contributions.