Lockdown blocks extensions relying on API interaction
Open, HighPublic

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.EditedJun 16 2017, 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.

Is this really resolved? I see that the issue still occurs after the patch and another user has also reported the same:
https://www.mediawiki.org/wiki/Topic:Tcspqvnlstfztqw2

Kghbln added a comment.Jul 1 2017, 1:28 PM

Is this really resolved? I see that the issue still occurs after the patch and another user has also reported the same:

The situation is better than before, i.e. much less broken than earlier on. However if you need to call the API a second or even more times things indeed still go bellies up. I hope that this patchset will fix the situation but I am not sure since I am not a developer.

This is a back-burner issue since nobody raises his or her voice. So if you are the only clown things move forward slowly. At least I feel like it.

Kghbln reopened this task as Open.Jul 1 2017, 1:29 PM

Re-opening per discussion. I guess this reflects the situation better.

aaron removed aaron as the assignee of this task.Jul 2 2017, 11:13 AM
Cavila added a subscriber: Cavila.Jul 17 2017, 1:29 PM

Speaking on behalf of a lot more people than just myself, this is certainly one of the most impactful and enduring issues that I've had to encounter since last year, although much has been improved.

StasR added a subscriber: StasR.Aug 17 2017, 3:32 PM
Loman87 added a subscriber: Loman87.Oct 3 2017, 8:14 AM

Hi everybody,
is there any chance to see this bug fixed? Me and more than 500 other wikis (according to wikiapiary) are using and counting on the functionality of this extension and this is not a minor bug! It is almost a year since the first bug report, please do something!
Thanks,
Lorenzo

Kghbln awarded a token.Oct 8 2017, 7:17 PM

Perhaps it will also be an idea to write to the mailing list about this to attract somebody to this? What do the subscribers think?

Perhaps it will also be an idea to write to the mailing list about this to attract somebody to this? What do the subscribers think?

Yes, I think you are right. It's definitely time to write to the mailing list!

Since nobody else seems to do it I took the liberty of being the clown sending out the e-mail to the list. Keeping fingers crossed.

Since nobody else seems to do it I took the liberty of being the clown sending out the e-mail to the list. Keeping fingers crossed.

I am sorry, but I am not a developer and I am not able to properly describe the issue in a different way than 'Lockdown does not work with MW 1.27'.
However thanks for your initiative.
All the best,
Lorenzo

AdSvS added a subscriber: AdSvS.Oct 30 2017, 11:09 AM

Hi Karsten, we don't have the expertise available at this moment to solve this or 'adopt' this extension. But we do want to contribute in a way we can.
Ad

Hedius removed a subscriber: Hedius.Nov 19 2017, 8:39 AM

It looks like people are still having problems with this issue. However, I'm using 1.27.3 + Lockdown + VE, etc and haven't been getting any problem reports. Note that I did do some work on Lockdown, so that may have resolved the issue. Could someone try to use the latest Lockdown and see if they still have problems?

Kghbln added a comment.EditedMar 8 2018, 5:43 PM

I just tested on a fresh MW 1.27.4 and apparently the situation is worse with latest master of Lockdown than with REL1_27. Now the first edit to a page fails and only after reloading the browser window with F5 it is possible to edit. This starts from the beginning for every page I access. So it looks to me that Lockdown is not aware at the beginning of the user groups and thus blocks. With REL1_27 I can at least edit as long as no API action is involved.

I am really helpless when it come to helping to fix the issue so any effort will greatly be apprechiated.

Note that I use Memchached as the caching backend for general caching and the database of session caching. Perhaps that's part of the issue.

Just a note that this impacts potential extensions that rely on this type of functionality.

I just tested the latest version for REL1_27 which now uses extension registration. This one is completely broken again - worse than it ever was. It arbitrarily pick namespaces to show or hide disregarding the configuration completely. I have not even gotten around testing editing.

Please I will be very kind if someone could fix MediaWiki for this extension to work again. Please this will be much appreciated.

I have been playing a bit with MW 1.27.3 and 1.27.4 instances with last REL1_27 version. I noticed that by removing SearchableNamespaces line Hook at extension.json things seem to work far better (I didn't test why yet).

"Hooks": {
        "getUserPermissionsErrors": "MediaWiki\\Extensions\\Lockdown\\Hooks::onGetUserPermissionsErrors",
        "MediaWikiPerformAction": "MediaWiki\\Extensions\\Lockdown\\Hooks::onMediawikiPerformAction",
        "SearchableNamespaces": "MediaWiki\\Extensions\\Lockdown\\Hooks::onSearchableNamespaces",
        "SearchGetNearMatchComplete": "MediaWiki\\Extensions\\Lockdown\\Hooks::onSearchGetNearMatchComplete"
},

@Kghbln , might you try as well and give your feebdack?

Configuration is far from being straightforward, I think it would be worth thinking about preparing some continuous integration tests for different permission scenarios.

I noticed than in any load page, up to 3 times SearchableNamespaces Hook is called. First one is 0 user and it seems to break everything when $user->getEffectiveGroups() is called.

This workaround seems to work:

 		if ( $user->getId() > 0 ) {
			$ugroups = $user->getEffectiveGroups();
		} else {
			$ugroups = array('*');
			
		}

If someone else can make some testing...

Change 407888 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@master] Handle when people specify all groups allowed

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

Change 407888 merged by jenkins-bot:
[mediawiki/extensions/Lockdown@master] Handle when people specify all groups allowed

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

Please download and test the latest version.

Change 444760 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@REL1_31] Handle when people specify all groups allowed

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

Change 444760 merged by jenkins-bot:
[mediawiki/extensions/Lockdown@REL1_31] Handle when people specify all groups allowed

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

Kghbln added a comment.EditedTue, Jul 10, 2:47 PM

Please download and test the latest version.

This does not fix the issues with Lockdown either though API-access may very well be working in the meantime. I did not even get that far with testing.

Setup

  • MediaWiki 1.31.0 (9604c55) 02:20, 8. Jul. 2018
  • PHP 7.0.30-0+deb9u1 (apache2handler)
  • MariaDB 10.1.26-MariaDB-0+deb9u1
  • Lockdown – (dbd06b7) 01:11, 10. Jul. 2018
## Poweruser
$wgGroupPermissions['poweruser']['editlocked'] = true;

## Define namespaces
define( 'NS_LOCKED', 3010 );
define( 'NS_LOCKED_TALK', 3011 );

## Name namespaces
$wgExtraNamespaces[NS_LOCKED] = 'Locked';
$wgExtraNamespaces[NS_LOCKED_TALK] = 'Locked_talk';

## NamespaceProtection
$wgNamespaceProtection[NS_LOCKED] = [
	'editlocked'
	];
$wgNamespaceProtection[NS_LOCKED_TALK] = [
	'editlocked'
	];
$wgNonincludableNamespaces[] = NS_LOCKED;
$wgNonincludableNamespaces[] = NS_LOCKED_TALK;

## Lockdown
wfLoadExtension( 'Lockdown' );
$wgActionLockdown['purge'] = [
	'user'
	];
$wgNamespacePermissionLockdown[NS_LOCKED]['read'] = [
	'poweruser'
	];
$wgNamespacePermissionLockdown[NS_LOCKED_TALK]['read'] = [
	'poweruser'
	];
$wgSpecialPageLockdown['Export'] = [
        'poweruser'
        ];

What happens is that anonymous users are required to log in for NS_MAIN while other namespaces like NS_PROJECT remain readable. Logged in users not belonging to the "poweruser" group get permission errors: "The action you have requested is limited to users in the group: poweruser."

Perhaps I mis-configured but I do not think this is the case, at least something like this worked in MW 1.23

I think the source of the issue is T142295 so trying to fix Lockdown to work with broken MediaWiki core is probably not the best approach.