Page MenuHomePhabricator

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."

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
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.EditedJul 10 2018, 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.

I'm seeing something related to this (maybe?) now: ...api.php?format=json&callback=HotCat.start&action=query&rawcontinue=&titles=...&prop=info|revisions&rvprop=content|timestamp|ids&meta=siteinfo&rvlimit=1&rvstartid=113408

Removing the callback parameter returns information. Is this related or a new bug?

@MarkAHershberger having the callback parameter in the URL makes the API think it is a JSON-P call. In that case no user context is available (only anonymous [1][2]). Maybe that's the reason.

[1] https://github.com/wikimedia/mediawiki/blob/1f664ea4ebc686f3879e806d2059a85df18e3cd2/includes/api/ApiMain.php#L316-L331
[2] https://github.com/wikimedia/mediawiki/blob/1f664ea4ebc686f3879e806d2059a85df18e3cd2/includes/api/ApiMain.php#L228-L236

So this looks like another issue. Actually this very issue was resolved but by doing so a couple of new issues were introduced. This extension is now completely broken for MW 1.27, 1.28, 1.29, 1.30 and 1.31. I am really in panic and at the same time unable to contribute code to help the cause. If it cannot be fixed we should archive the extension and delete the code since it does not make sense to me to keep a zombie for another couple of years of brokenness. A bit sad since it used to be among the ten most popular extensions to MediaWiki, so the need for it seems to be present.

Osnard added a comment.EditedSep 26 2018, 11:18 AM

This is really interesting. We use this extension on quite a lot MediaWiki REL1_27 installations. There were no API related issues. We use REL1_27 branch of Lockdown.

edit: For our upcoming REL1_31 installations we actually removed Lockdown from the distribution package and implemented own permission checks. But that was for another reason.

This is really interesting. We use this extension on quite a lot MediaWiki REL1_27 installations. There were no API related issues. We use REL1_27 branch of Lockdown.

I just had to remove Lockdown due to continued issues. The customer was actually blaming BlueSpice for the issues but I was able to pinpoint Lockdown. I am surprised it works for you. Lucky you. Anyways 1.27 is no longer of big interest. I would rather like to see a working version for 1.31

Very strange, indeed. On this installation, which Lockdown version has been used in the first place? REL1_27? Which BlueSpice version has been used?

Kghbln <no-reply@phabricator.wikimedia.org> writes:

This extension is now completely broken for MW 1.27, 1.28, 1.29, 1.30
and 1.31.

I'm using this extension on 1.27 and about to deploy it on 1.31. Could
you help me prioritize the issues you know about?

Kghbln added a comment.EditedSep 27 2018, 7:37 AM

Very strange, indeed. On this installation, which Lockdown version has been used in the first place? REL1_27? Which BlueSpice version has been used?

Well it was unrelated to BlueSpice. What I was trying to convey was that since BlueSpice claims to have access control feature it took the blame even though not being responsible and this is not a preferable situation in either way. The version I use is this one. Everything after this renders the respective wiki being completely unusable. This created an armada of issue that overwhelmed me. I rather have no API intersection but an otherwise somewhat working wiki. But this is 1.27

I'm using this extension on 1.27 and about to deploy it on 1.31. Could you help me prioritize the issues you know about?

That's basically my report in this post. T148582#4412138 This happens on sandbox.s-wm.o - that's why I removed the extension again - as well as on a naked wiki with just custom namespaces and Lockdown.

@Kghbln thanks for giving me a simple case even if I should have seen it while leaving the comment.

I've see the following problems:

  • anon & unprivileged visits to the main page prompt for login
  • anon & unprivileged users can read Locked namespace

My current guess is that the read permission is the problem.

Could you go to extension.json in the Lockdown extension and change the following:

	"config": {
		"NamespacePermissionLockdown": [],
		"SpecialPageLockdown": [],
		"ActionLockdown": []
	},

to

	"config": {
		"NamespacePermissionLockdown": null,
		"SpecialPageLockdown": null,
		"ActionLockdown": null
	},

From my tests, that should fix the problem.

This might also explain why @Osnard and I didn't see the problem -- we are configuring these variables in another extension.

Change 463534 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@master] Specify null instead of empty array for extension.json

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

Change 463536 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@master] Fix problem with extension.json and initialization

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

Change 463538 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@master] Fix problem with extension.json and initialization

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

Change 463539 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@master] Fix https://phabricator.wikimedia.org/T148582#4412138

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

Change 463538 abandoned by MarkAHershberger:
Fix problem with extension.json and initialization

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

Change 463534 restored by MarkAHershberger:
Specify null instead of empty array for extension.json

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

Change 463583 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@REL1_27] Specify null instead of empty array for extension.json

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

Change 463629 had a related patch set uploaded (by MarkAHershberger; owner: MarkAHershberger):
[mediawiki/extensions/Lockdown@REL1_31] Specify merge strategy instead of just an empty array

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

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.

@Kghbln Can you check if the patchset resolves the issues you mentioned?

Change 463534 merged by jenkins-bot:
[mediawiki/extensions/Lockdown@master] Specify merge strategy for NamespacePermissionLockdown

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

Kghbln added a comment.EditedOct 1 2018, 9:09 PM

I have now pulled in master to <sandbox.semantic-mediawiki.org> and this indeed seemed to have solved issues to a large extent. Thanks a lot for working on this!!!

I have now tested the configuration as specified here: T148582#4412138. Moreover I tested if the API works by using the VisualEditor. On non-protected namespaces like "Extra" it works, however on the protected namespace "Locked" I get "apierror-visualeditor-docserver-http: HTTP 500" suggesting that API interaction is broken in this particular namespace.

The VisualEditor configuration in addition to what I have posted at T148582#4412138:

## VisualEditor
wfLoadExtension( 'VisualEditor' );
$wgVisualEditorAvailableNamespaces = [
	'File' => false,
	'Category' => false,
	'Extra' => true,
	'Locked' => true
	];
$wgVisualEditorEnableWikitext = true;
$wgDefaultUserOptions['visualeditor-newwikitext'] = 1;

Still this is great progress since before the API interaction was broken everywhere after performing the first action via API.

If you would like to test for yourself you are welcome to do so.

Kghbln <no-reply@phabricator.wikimedia.org> writes:

I get "apierror-visualeditor-docserver-http: HTTP 500" suggesting that
API interaction is broken in this particular namespace.

Could you post your php error log that is produced?

Kghbln added a comment.EditedOct 2 2018, 5:36 PM

Could you post your php error log that is produced?

Unfortunately I cannot since no error is logged to the files. Error reporting remains completely silent. Apparently I have no idea how to debug the ill-fated VisualEdior. These are my settings:

### Exception settings

## Activate exception reporting
error_reporting( -1 );
ini_set( 'display_errors', 1 );

## Show exceptions
$wgShowExceptionDetails = true;
$wgShowSQLErrors = true;
$wgShowDBErrorBacktrace = true;
// $wgDebugToolbar = true;

## Log exceptions and events
// $wgDebugLogFile = "/var/log/mediawiki/{$wgDBname}-debug-all.log";
$wgDebugLogGroups  = [
	'authentication' => "/var/log/mediawiki/{$wgDBname}-debug-authentication.log",
	'error' => "/var/log/mediawiki/{$wgDBname}-debug-error.log",
	'exception' => "/var/log/mediawiki/{$wgDBname}-debug-exception.log",
	'resourceloader' => "/var/log/mediawiki/{$wgDBname}-debug-resourceloader.log",
	'smw' => "/var/log/mediawiki/{$wgDBname}-debug-smw.log",
	'smw-elastic' => "var/log/mediawiki/{$wgDBname}-debug-smw-elastic.log"
	];

From your question I understand that a similar setup on your test wiki does not create issues. Still I find it strange that VisualEdior is working on the non protected namespaces but not in the protected namespaces on sandbox.semantic-mediawiki.org.

I'm not sure that this is caused by lockdown, though. I'm trying to duplicate it and I'm running into the same error even without Lockdown.

Kghbln added a comment.Oct 4 2018, 8:54 AM

Hmm, I did not test this connection since the correlation appeared to be to high. Anyways I disabled Lockdown and was indeed able to edit with VisualEditor as a result. Thus it still appears to me that the issue is connected with Lockdown.

Still present at the time of the test:

## Poweruser
$wgGroupPermissions['poweruser']['editlocked'] = true;

## Protect namespaces
$wgNamespaceProtection[NS_LOCKED] = [
        'editlocked'
        ];
$wgNamespaceProtection[NS_LOCKED_TALK] = [
        'editlocked'
        ];
$wgNonincludableNamespaces[] = NS_LOCKED;
$wgNonincludableNamespaces[] = NS_LOCKED_TALK;
MarkAHershberger added a comment.EditedOct 4 2018, 9:54 PM

Do you have cookie forwarding set up as the documentation for VE suggests?

(My previous problem was caused by my own misconfiguration.)

Kghbln added a comment.Oct 5 2018, 7:37 AM

No, since sandbox is a public wiki. The docu explicitly states not to do this. However I now adapted the way how permissions are set as detailed in method 2.2 but this did not help (It does in my private wiki setups without having Lockdown present). Hmm, so in the end Lockdown and VisualEditor do not mix on public wikis? Hopefully this does at least on private wikis which will be the more common use case.

I've finally been able to get the error 500 you described. (I was writing out my configuration and found the problem that was keeping me from seeing it.)

This message shows up in Parsoid's log when it happens:

"longMsg": "API response Error for TemplateRequest: request=; error={\"code\":\"rvaccessdenied\",\"info\":\"The current user is not allowed to read Locked:Main\",\"*\":\"See http://wikidev.localdomain/core/api.php for API usage\"}",

The actual api call that parsoid is sending is
.../api.php?format=json&action=query&prop=info%7Crevisions&rawcontinue=1&rvprop=content%7Cids%7Ctimestamp%7Csize%7Csha1%7Ccontentmodel&revids=2

Without cookies, I get the message:

{"error":{"code":"rvaccessdenied","info":"The current user is not allowed to read Locked:Main","*":"See http://wikidev.localdomain/core/api.php for API usage"}}

With cookies, I get the proper response.

MarkAHershberger added a comment.EditedOct 5 2018, 4:33 PM

This isn't really a lockdown problem.

You need parsoid to act as your proxy so it has to have your cookies.

Allowing parsoid to access the server without authenticating as you would mean that lockdown could be defeated just by using VisualEditor.

If parsoid and MW are on the same server, forwarding cookies should be relatively safe since your concern then is protecting the server itself from attack (which you have to do anyway) rather than worry about exposing credentials to network traffic.

MarkAHershberger closed this task as Resolved.Oct 5 2018, 7:31 PM
MarkAHershberger claimed this task.
Kghbln added a comment.Oct 5 2018, 8:20 PM

This isn't really a lockdown problem.

Ok, great that this in now clear.

You need parsoid to act as your proxy so it has to have your cookies.

Allowing parsoid to access the server without authenticating as you would mean that lockdown could be defeated just by using VisualEditor.

Fair enough!

If parsoid and MW are on the same server, forwarding cookies should be relatively safe since your concern then is protecting the server itself from attack (which you have to do anyway) rather than worry about exposing credentials to network traffic.

I trust you on this one!

I would like to thank you and all others involoved for digging into this and making it work again. I took the liberty to update the extension's page for the new status.

Change 463629 merged by jenkins-bot:
[mediawiki/extensions/Lockdown@REL1_31] Specify merge strategy for NamespacePermissionLockdown

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

Change 463583 merged by jenkins-bot:
[mediawiki/extensions/Lockdown@REL1_27] Specify merge strategy for NamespacePermissionLockdown

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