Page MenuHomePhabricator

WikiForum overwrites $wgCaptchaTriggers when adding a new permission
Closed, ResolvedPublic

Description

Or so I am told. None of my lazy sysadmins want to file the ticket themselves.

Maybe in extension.json, CaptchaTriggers should be +CaptchaTriggers, so that the configuration variable merges?

Event Timeline

labster created this task.Dec 11 2016, 10:03 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 11 2016, 10:03 PM
Restricted Application added a project: Social-Tools. · View Herald TranscriptDec 11 2016, 10:04 PM
SamanthaNguyen moved this task from Backlog to Bugs on the WikiForum board.Dec 11 2016, 10:13 PM
SamanthaNguyen moved this task from Backlog to WikiForum on the Social-Tools board.

Updates on this task?

@Reception123: If there were any updates they'd be in this task, so your question is too broad. Do you have a more specific question?

@Aklapper I meant if this task will be fixed soon, as this extension is important for some of our wiki users.

Reedy added a subscriber: Reedy.Mar 23 2017, 6:54 PM

FWIW +CaptchaTriggers won't do anything in extension.json

@Reedy So then what would you suggest for $wgCaptchaTriggers not to be overriden?

Shouldn't that be done by the developers of the extension? (this is why the issue was posted here)

adding @UltrasonicNXT and pinging @ashley as they are devs

Reedy added a comment.Apr 10 2017, 9:31 PM

As always, as no comments have been added, no patches uploaded... It won't have been fixed

Reedy added a comment.Apr 10 2017, 9:34 PM

Though, I do wonder if this is related to having a prefix of "" and then putting wg on nearly everything else

	"config": {
		"_prefix": "",
		"wgCaptchaWhitelistIP": false,
		"wgCaptcha": null,
		"wgCaptchaClass": "SimpleCaptcha",
		"wgCaptchaTriggers": {
			"edit": false,
			"create": false,
			"sendemail": false,
			"addurl": true,
			"createaccount": true,
			"badlogin": true,
			"badloginperuser": true,
			"_merge_strategy": "array_plus"
		},
		"wgCaptchaTriggersOnNamespace": {
			"_merge_strategy": "array_plus_2d"
		},
		"wgCaptchaStorageClass": "CaptchaSessionStore",
		"wgCaptchaSessionExpiration": 1800,
		"wgCaptchaBadLoginExpiration": 300,
		"wgCaptchaBadLoginPerUserExpiration": 600,
		"ceAllowConfirmedEmail": false,
		"wgCaptchaBadLoginAttempts": 3,
		"wgCaptchaBadLoginPerUserAttempts": 20,
		"wgCaptchaWhitelist": false,
		"wgCaptchaRegexes": []
	},
Florian claimed this task.Apr 17 2017, 8:18 PM
Florian added subscribers: Legoktm, Florian.

The problem isn't related to the prefix. ExtensionRegistration isn't meant to add the same config option in different extensions and merge the default values. While extension registration has the ability to merge values from an extension.json and LocalSettings.php (or any other source of a global value) in different ways, it doesn't do that for values that are set in different extension.json definitions. That means, that this is one rare casse, where even the extension registration has a race condition and is error prone depending on the order of loading extensions. What I mean is:

This error highly depends on the order of wfLoadExtensions calls in your LocalSettings.php. If WikiForum is loaded _after_ ConfirmEdit, the CaptchaTriggers will be taken from WikiForum, while the ones from ConfirmEdit will be overwritten. If the WikiForum extension is laoded _before_ the ConfirmEdit extension it would be vice versa: The triggers from wikiforum will be overwritten by the ones from ConfirmEdit.

However, this is a very very rare case and I, for myself, would consider this as a bad development practice. An extension, that relies on a speicifc configuration of another extension, should mention this in the setup steps for this extension, so that the sysadmin does it. Or, if the extension really want to do that automatically, it should do this in a saner way, e.g. by merging the values by themself. However, the config section of extension.sjon is definitely the worst place doing it. It is meant for describing the configuration option, which are provided by this extension (and their default values), not for setting a specific value for another extensions config option(s). I'll add/ping @Legoktm to check my opinion about that, but from my point of view, we will not support this in extension registration.

However, I'll submit to changes in some minutes: One will fix WikiForum (in combination with ConfirmEdit), so that this problem should not occur. Another one for mediawiki, so that extension registration will at least warn about this problem, so that this unexpected behaviour will be spotted easier, hopefully :)

Thanks for posting this task! One suggestion for the next time: The problem was more or less obvious this time and was a bit easy to spot while looking at both extensions, however, I'm still not sure, if this is really your problem (I think so, but I'm not sure). The problem is, that you haven't described your problem very good, there're no steps to reproduce (e.g. lines from your LocalSettings.php or something you do in your wiki), there're also no expected outcomes and the current result you see, which could be your problem. I hope these points help you for the next time, when you file a task :) And, of course, allow me one little joke regarding:

None of my lazy sysadmins want to file the ticket themselves

You should try to find new ones, filing tasks to software a system administrator uses, sounds like a part of a the job description to me :P

Well, I'll fire our sysadmins just as soon as I start paying them :P

Sorry about the bad reporting. This was something that was dumped on my plate; I should have tried to better understand the issue before posting anyway. Understanding issues is part of the title of a software developer, and all on me.

Change 348577 had a related patch set uploaded (by Florianschmidtwelzow):
[mediawiki/core@master] Prevent setting an extension configuration twice

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

Change 348582 had a related patch set uploaded (by Florianschmidtwelzow):
[mediawiki/extensions/ConfirmEdit@master] Don't set CaptchaClass in ConfirmEdit\extension.json

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

Change 348584 had a related patch set uploaded (by Florianschmidtwelzow):
[mediawiki/extensions/WikiForum@master] Don't set the default value of another extension in extension.json

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

Reception123 triaged this task as High priority.May 7 2017, 2:16 PM

Not sure if this priority is appropriate, but we are nearing the next MediaWiki update and this change should be "approved" and merged until then.

Sorry if this isn't the priority, and feel free to change it

@Florian There's been some comments on the PRs.

(Sorry to bother about this, but users are asking about the extension and this is a issue that needs to be resolved)

Change 348584 abandoned by Florianschmidtwelzow:
Don't set the default value of another extension in extension.json

Reason:
see Iaef32efab397e82ff70ddca8ac79c545c5b7d2bb

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

Change 348582 abandoned by Florianschmidtwelzow:
Don't set CaptchaClass in ConfirmEdit\extension.json

Reason:
see Iaef32efab397e82ff70ddca8ac79c545c5b7d2bb

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

Hi @Recetion123: Yeah, I've to say, that I forget the comments from @Krinkle (sorry for that, again! :(). I've abandoned my first idea (not completely, but no changes are required in the extensions anymore), and updated the change to MediaWiki core (just for information for you here, so that you're not confused by the abandoned messages here :]).

that's for what I thanked you for :)

Ah, haven't seen that as I was writing my comment :]

Krinkle removed a subscriber: Krinkle.Jul 27 2017, 4:35 AM

Can we make CaptchaTriggers into an attribute?

I'm not completely sure, how Attributes in extensionr egistration work, however, how would a wiki sysadmin disable a trigger, if it is automatically assigned by an extension with an attribute?

Have a captcha trigger set to false in the config variable override the true in attributes?

$triggers = $wgCaptchaTriggers + ExtensionRegistry::getInstance()->getAttribute('CaptchaTriggers');

Change 371947 had a related patch set uploaded (by Florianschmidtwelzow; owner: Florianschmidtwelzow):
[mediawiki/extensions/ConfirmEdit@master] Allow other extensions to setup triggers using attributes

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

Change 371948 had a related patch set uploaded (by Florianschmidtwelzow; owner: Florianschmidtwelzow):
[mediawiki/extensions/WikiForum@master] Use CaptchaTriggers attribute to declare wikiforum Captcha trigger

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

Change 348577 merged by jenkins-bot:
[mediawiki/core@master] registration: Only allow one extension to set a specific config setting

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

Change 374334 had a related patch set uploaded (by Florianschmidtwelzow; owner: Florianschmidtwelzow):
[mediawiki/core@master] registration: Only allow one extension to set a specific config setting

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

The revert of the change 50d7546057c1089228a2a6d7d4a49d78d72887e3 was caused by

120:14:32 PHP Fatal error: Uncaught exception 'RuntimeException' with message 'The configuration setting 'wgCaptchaClass' was already set by another extension, and cannot be set again.' in /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php:494
220:14:32 Stack trace:
320:14:32 #0 /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php(453): ExtensionProcessor->addConfigGlobal('wgCaptchaClass', 'FancyCaptcha')
420:14:32 #1 /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php(193): ExtensionProcessor->extractConfig1(Array)
520:14:32 #2 /srv/mediawiki-staging/php-master/includes/registration/ExtensionRegistry.php(246): ExtensionProcessor->extractInfo('/srv/mediawiki-...', Array, 1)
620:14:32 #3 /srv/mediawiki-staging/php-master/includes/registration/ExtensionRegistry.php(148): ExtensionRegistry->readFromQueue(Array)
720:14:32 #4 /srv/mediawiki-staging/php-master/includes/Setup.php(40): ExtensionRegistry->loadFromQueue()
820:14:32 #5 /srv/mediawiki-staging/php-master/maintenance/doMaintenance.php(98): require_once('/srv/mediawiki-... in /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php on line 494
920:14:32 Fatal error: Uncaught exception 'RuntimeException' with message 'The configuration setting 'wgCaptchaClass' was already set by another extension, and cannot be set again.' in /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php:494
1020:14:32 Stack trace:
1120:14:32 #0 /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php(453): ExtensionProcessor->addConfigGlobal('wgCaptchaClass', 'FancyCaptcha')
1220:14:32 #1 /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php(193): ExtensionProcessor->extractConfig1(Array)
1320:14:32 #2 /srv/mediawiki-staging/php-master/includes/registration/ExtensionRegistry.php(246): ExtensionProcessor->extractInfo('/srv/mediawiki-...', Array, 1)
1420:14:32 #3 /srv/mediawiki-staging/php-master/includes/registration/ExtensionRegistry.php(148): ExtensionRegistry->readFromQueue(Array)
1520:14:32 #4 /srv/mediawiki-staging/php-master/includes/Setup.php(40): ExtensionRegistry->loadFromQueue()
1620:14:32 #5 /srv/mediawiki-staging/php-master/maintenance/doMaintenance.php(98): require_once('/srv/mediawiki-... in /srv/mediawiki-staging/php-master/includes/registration/ExtensionProcessor.php on line 494
which should be fixed with https://gerrit.wikimedia.org/r/371947 I re-created the change with 29a5fc72e3ab7d3da28bc4cc91d4bb051b9a690a and made it dependant upon the change in ConfirmEdit.

Add ConfirmEdit since there is a patch for that too.

Change 371947 merged by jenkins-bot:
[mediawiki/extensions/ConfirmEdit@master] Allow other extensions to setup triggers using attributes

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

Change 374334 merged by jenkins-bot:
[mediawiki/core@master] registration: Only allow one extension to set a specific config setting

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

Change 371948 merged by jenkins-bot:
[mediawiki/extensions/WikiForum@master] Use CaptchaTriggers attribute to declare wikiforum Captcha trigger

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

Reception123 closed this task as Resolved.EditedOct 25 2017, 9:03 AM

With the above getting +2, this issue is now fixed, and the captcha triggers are no longer overridden.