T135668: Need documentation on How to integrate AbuseFilter with xyz extension - captcha extension should be able to tell AbuseFilter that there should be a new action

We have the (not very documented) AbuseFilterCustomActionsHandlers global for this. It's used at https://phabricator.wikimedia.org/diffusion/EABF/browse/master/includes/AbuseFilterRunner.php$1025. It shouldn't be super-hard to integrate ext:captcha with it.
Basically, you have to:

• Register a handle in AbuseFilterCustomActionsHandlers (see code linked above for params)
• Add the action to AbuseFilterActions [1]
• Provide i18n for it (see here and here).

In general, I hope to expand my explanation above and put it somewhere on mw.org for T135668. Also, it's probably worthwhile to audit and overhaul this part of the code before documenting it (i.e. resolve footer note 1, clarify the handle system, etc; T215136 should also be addressed).

[1] - This isn't very clean, I admit. We should probably factor out SpecialAbuseLog::getAllActions, and reuse it in various places, instead of just using AbuseFilterActions.

I took a quick look, and it should be feasible, at least for edits. The handler method would be similar to confirmEditMerged, the main part being

$page->ConfirmEdit_ActivateCaptcha = true;

and then ConfirmEdit will show a captcha. The main problems are:

• Determine whether some user groups should be able to bypass the check. As I said in T69936, I believe the answer should be "definitely no".
• Pass the WikiPage passed in by the hook object onto the handler. We have to change the AF code to make this possible.

And I guess for other actions it'll be similar.

In T20110#5702776, @Daimona wrote:

I took a quick look, and it should be feasible, at least for edits. The handler method would be similar to confirmEditMerged, the main part being

$page->ConfirmEdit_ActivateCaptcha = true;

and then ConfirmEdit will show a captcha. The main problems are:

• Determine whether some user groups should be able to bypass the check. As I said in T69936, I believe the answer should be "definitely no".
• Pass the WikiPage passed in by the hook object onto the handler. We have to change the AF code to make this possible.

And I guess for other actions it'll be similar.

Respectfully, strongly disagree on point 1 - we shouldn't be showing captchas to users with skipcaptcha

In T20110#5702868, @DannyS712 wrote:

Respectfully, strongly disagree on point 1 - we shouldn't be showing captchas to users with skipcaptcha

OK, that's a fair point. I think that filter editors should already be checking user_groups; also, I imagine that a filter with "captcha" action enabled would mostly be meant to target anons or non-confirmed (i.e. to catch spambots). Maybe it would be even better to add a checkbox to the filter form, saying like

Show captcha to users with 'skipcaptcha'

but the UI doesn't allow that at the moment (it should be an "easy" fix though).

In T20110#5703214, @Daimona wrote:

In T20110#5702868, @DannyS712 wrote:

Respectfully, strongly disagree on point 1 - we shouldn't be showing captchas to users with skipcaptcha

OK, that's a fair point. I think that filter editors should already be checking user_groups; also, I imagine that a filter with "captcha" action enabled would mostly be meant to target anons or non-confirmed (i.e. to catch spambots). Maybe it would be even better to add a checkbox to the filter form, saying like

Show captcha to users with 'skipcaptcha'

but the UI doesn't allow that at the moment (it should be an "easy" fix though).

There are already issues with captcha and accessibility as it is - introducing the ability to override the skipcaptcha right should require Community-consensus-needed

In T20110#5703215, @DannyS712 wrote:

There are already issues with captcha and accessibility as it is - introducing the ability to override the skipcaptcha right should require Community-consensus-needed

Makes sense. I guess we can make it respect the skipcaptcha right for now, the implementation would be the same.

Having this follow 'skipcatcha' in its current implementation would quite cripple versatility; as filters support global groups if a filter writer wanted to exempt the actual people in the 'captcha-exempt' global group, they certainly could. (Perhaps they want to be able to target registered users who normally have skipcaptcha for the traditional captcha reasons).

Change #1027259 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/AbuseFilter@master] [WIP] Provide edit integration with ConfirmEdit

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

Change #1027260 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ConfirmEdit@master] [WIP] Allow showing a CAPTCHA in response to AbuseFilter consequence

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

Change #1027267 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] zuul: Add ConfirmEdit to phan dependencies for AbuseFilter

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

Change #1027267 merged by jenkins-bot:

[integration/config@master] zuul: Add ConfirmEdit to phan dependencies for AbuseFilter

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

Mentioned in SAL (#wikimedia-releng) [2024-05-05T06:25:43Z] <James_F> Zuul: Add ConfirmEdit to phan dependencies for AbuseFilter, for T20110

Change #1027516 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] zuul: Update AbuseFilter/ConfirmEdit phan and test dependencies

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

Change #1027516 merged by jenkins-bot:

[integration/config@master] zuul: Update AbuseFilter/ConfirmEdit phan and test dependencies

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

Demo of the functionality https://drive.google.com/file/d/1YKijFfbYPv7Y4BXc20IY4_gVq_gMijRf/view?usp=sharing, and a patch demo instance https://patchdemo.wmflabs.org/wikis/fbd86be54b/wiki/Main_Page.

Change #1027260 merged by jenkins-bot:

[mediawiki/extensions/ConfirmEdit@master] Allow showing a CAPTCHA in response to AbuseFilter consequence

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

Change #1027259 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Provide integration with ConfirmEdit to show CAPTCHA

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

Suggested text for user notice:

AbuseFilter can show a CAPTCHA to a user before they can proceed with an action. Not all actions may be supported, but the CAPTCHA action should work well for source and API edits. As this is a new action, please use with caution and report any issues as subtasks of T20110.

Thanks for implementing this! But I'm having trouble getting it to work on testwiki. This edit from an account without skipcaptcha rights tripped a showcaptcha filter, but did not require a CAPTCHA.

In T20110#9796395, @suffusion_of_yellow wrote:

Thanks for implementing this! But I'm having trouble getting it to work on testwiki. This edit from an account without skipcaptcha rights tripped a showcaptcha filter, but did not require a CAPTCHA.

Thanks for testing, and reporting the issue, I'm not sure why it's occurring. Looking into it...

In T20110#9796476, @kostajh wrote:

In T20110#9796395, @suffusion_of_yellow wrote:

Thanks for implementing this! But I'm having trouble getting it to work on testwiki. This edit from an account without skipcaptcha rights tripped a showcaptcha filter, but did not require a CAPTCHA.

Thanks for testing, and reporting the issue, I'm not sure why it's occurring. Looking into it...

Looking at debug logs for an edit that triggers this filter, we end up at "ConfirmEdit: no need to show captcha.". Looking at the relevant function in the XHGui output I can see that onConfirmEditTriggersCaptcha is invoked. Looking at the hook implementation for ConfirmEditTriggersCaptcha I can see that the AbuseFilter hook runs. So one possibility is that that setting the CaptchaConsequence on the WebRequest object in the RequestContext in one hook didn't persist when reading it back in the other hook.

Hey, noticed on testwiki that some of you are trying this from your main account; that probably has skipcaptcha rights so will never be shown one.

FWIW, I have also tried this on your patch demo, and also get no captcha. Also tried on a local wiki, and again get no captcha. I even tried loading AbuseFilter before ConfirmEdit (a bad idea, breaks the "throttle" filters and also causes T151116), and still no luck.

Change #1031549 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ConfirmEdit@master] CaptchaConsequence: Use SessionManager to communicate with AbuseFilter

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

Change #1031550 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/AbuseFilter@master] ConfirmEditHandler: Use Session instead of WebRequest

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

In T20110#9796913, @suffusion_of_yellow wrote:

Hey, noticed on testwiki that some of you are trying this from your main account; that probably has skipcaptcha rights so will never be shown one.

Yeah, I realized that and switched to using a logged out user for the test. Thanks for flagging though, it's easy to forget about that!

FWIW, I have also tried this on your patch demo, and also get no captcha. Also tried on a local wiki, and again get no captcha. I even tried loading AbuseFilter before ConfirmEdit (a bad idea, breaks the "throttle" filters and also causes T151116), and still no luck.

Thanks for that. I've uploaded an alternative approach that I think would be more robust.

In T20110#9797066, @kostajh wrote:

Thanks for that. I've uploaded an alternative approach that I think would be more robust.

Tried that on my local wiki (all edits made logged out):

• Loading ConfirmEdit before AbuseFilter (the same order as WMF wikis), I get no captcha on the first attempted edit. I do get a CAPTCHA, the next time I open a page for editing, even before I try to publish.
• Loading AbuseFilter before ConfirmEdit, I get a "Incorrect or missing CAPTCHA. " error (even though I would have had no idea a CAPTCHA was to be expected). Also, the message is at the top, but the CAPTCHA entry form is at the bottom. But reversing the order would be a very bad idea; it would cause "throttle" filters to trip on innocent users struggling with the CAPTCHA, and also prevent any user from adding an external link, if they also trip a "warn" filter.

In T20110#9797176, @suffusion_of_yellow wrote:

In T20110#9797066, @kostajh wrote:

Thanks for that. I've uploaded an alternative approach that I think would be more robust.

Tried that on my local wiki (all edits made logged out):

• Loading ConfirmEdit before AbuseFilter (the same order as WMF wikis), I get no captcha on the first attempted edit. I do get a CAPTCHA, the next time I open a page for editing, even before I try to publish.
• Loading AbuseFilter before ConfirmEdit, I get a "Incorrect or missing CAPTCHA. " error (even though I would have had no idea a CAPTCHA was to be expected). Also, the message is at the top, but the CAPTCHA entry form is at the bottom. But reversing the order would be a very bad idea; it would cause "throttle" filters to trip on innocent users struggling with the CAPTCHA, and also prevent any user from adding an external link, if they also trip a "warn" filter.

Aha. Yeah, I am loading AbuseFilter before ConfirmEdit in my local wiki. Thank you for testing the patches and leaving the detailed notes. OK, I'll look at this more closely tomorrow.

Alternatively, would it be possible to set a static flag in one of the ConfirmEdit classes? In theory, it doesn't even need to be static as long as the class in question is obtained through the service container. Something like AbuseFilter's own EditRevUpdater::$logIds. Another thing is that this could live all within the ConfirmEdit extension, and the property might be read in triggersCaptcha directly, without using a hook.

Won't ConfirmEdit need to be run twice, though? Once before AbuseFilter, to avoid filter trips from repeated bad CAPTCHAs, then again after AbuseFilter iff (A) No CAPTCHA was show on the first run, and (B) A showcaptcha filter was tripped.

Yeah, just to clarify, my comment was only in response to the switch from WebRequest to Session. But indeed, it does not address the bug above, which seems to be due to the order of execution.

FWIW, I have also tried this on your patch demo, and also get no captcha.

Correction. I didn't realize that the "Type 'patchdemo' here as an anti-spam measure." box was coming from QuestyCaptcha, and thought it was some generic anti-spam measure on all patchdemos.

So what actually happened is that I did get a request for a CAPTCHA, but the box was there before I even tried to click "Publish"! This cannot be coming from AbuseFilter; the filters haven't even been checked yet. Do you have ConfirmEdit enabled for all edits with something like $wgCaptchaTriggers['edit'] = true;?

In T20110#9791865, @kostajh wrote:

Suggested text for user notice:

AbuseFilter can show a CAPTCHA to a user before they can proceed with an action. Not all actions may be supported, but the CAPTCHA action should work well for source and API edits. As this is a new action, please use with caution and report any issues as subtasks of T20110.

The user notice should wait, because the functionality isn't working yet in production.

Change #1031757 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ConfirmEdit@master] AbuseFilterHooks: Provide feature flags for AF custom actions

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

Change #1031550 abandoned by Kosta Harlan:

[mediawiki/extensions/AbuseFilter@master] ConfirmEditHandler: Use Session instead of WebRequest

Reason:

Doesn't fix the underlying issue, which is due to order of extension loading in production.

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

Change #1031549 abandoned by Kosta Harlan:

[mediawiki/extensions/ConfirmEdit@master] CaptchaConsequence: Use SessionManager to communicate with AbuseFilter

Reason:

Doesn't fix the underlying issue, which is due to order of extension loading in production.

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

You could reorder the list of extensions to be loaded in production, it has been done before for the exact same problem but 1- It's not sustainable 2- it might break other things.

In T20110#9798669, @Ladsgroup wrote:

You could reorder the list of extensions to be loaded in production, it has been done before for the exact same problem but 1- It's not sustainable 2- it might break other things.

It will break other things.

I’m working on a patch that uses a different approach, will clean it up and push for review later today.

Change #1031885 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ConfirmEdit@master] [WIP] SimpleCaptcha: Allow invoking from other extensions

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

Change #1031886 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/AbuseFilter@master] [WIP] ConfirmEditHandler: Use SimpleCaptcha to invoke CAPTCHA display

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

Change #1031921 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ConfirmEdit@wmf/1.43.0-wmf.5] AbuseFilterHooks: Provide feature flags for AF custom actions

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

Change #1031757 merged by jenkins-bot:

[mediawiki/extensions/ConfirmEdit@master] AbuseFilterHooks: Provide feature flags for AF custom actions

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

Change #1031921 merged by jenkins-bot:

[mediawiki/extensions/ConfirmEdit@wmf/1.43.0-wmf.5] AbuseFilterHooks: Provide feature flags for AF custom actions

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

Mentioned in SAL (#wikimedia-operations) [2024-05-15T20:59:39Z] <samtar@deploy1002> Started scap: Backport for [[gerrit:1031921|AbuseFilterHooks: Provide feature flags for AF custom actions (T20110)]]

Mentioned in SAL (#wikimedia-operations) [2024-05-15T21:02:20Z] <samtar@deploy1002> samtar and kharlan: Backport for [[gerrit:1031921|AbuseFilterHooks: Provide feature flags for AF custom actions (T20110)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-05-15T21:16:10Z] <samtar@deploy1002> Finished scap: Backport for [[gerrit:1031921|AbuseFilterHooks: Provide feature flags for AF custom actions (T20110)]] (duration: 16m 31s)

In T20110#9797662, @suffusion_of_yellow wrote:

FWIW, I have also tried this on your patch demo, and also get no captcha.

Correction. I didn't realize that the "Type 'patchdemo' here as an anti-spam measure." box was coming from QuestyCaptcha, and thought it was some generic anti-spam measure on all patchdemos.

So what actually happened is that I did get a request for a CAPTCHA, but the box was there before I even tried to click "Publish"! This cannot be coming from AbuseFilter; the filters haven't even been checked yet. Do you have ConfirmEdit enabled for all edits with something like $wgCaptchaTriggers['edit'] = true;?

@suffusion_of_yellow Yeah, Patch Demo isn't a good environment for verifying these patches because it sets a CAPTCHA for all edits for not logged-in users, and gives all logged-in users skipcaptcha right. We'd need to make some adjustments. The config it uses (extensions-ConfirmEdit.php):

$wgCaptchaTriggers['edit'] = true;
$wgCaptchaTriggers['createaccount'] = true;
// Don't show CAPTCHA when logged in
$wgGroupPermissions['user']['skipcaptcha'] = true;

I've reworked the functionality for the AbuseFilter/ConfirmEdit integration in these two patches: https://gerrit.wikimedia.org/r/c/1031885 https://gerrit.wikimedia.org/r/c/1031886

I've reworked the functionality for the AbuseFilter/ConfirmEdit integration in these two patches: https://gerrit.wikimedia.org/r/c/1031885 https://gerrit.wikimedia.org/r/c/1031886

Tried these patches. I am using the 2010 editor, and these options:

wfLoadExtensions( ['ConfirmEdit', 'ConfirmEdit/QuestyCaptcha'] );
$wgCaptchaQuestions = [
    'What is the answer to Life, the Universe, and Everything?' => '42'
];
$wgConfirmEditEnabledAbuseFilterCustomActions = [ 'showcaptcha' ];
                                                                                
wfLoadExtension( 'AbuseFilter' );

First, I set a filter to "showcaptcha", and attempted an edit NOT adding external links. After clicking "publish" the first time, at the top of the page, I got the message "Incorrect or missing CAPTCHA." even though I was never asked for one in the first place. The actual CAPCTHA was at the bottom of the page. The page saved successfully when I answered correctly.

Second, I attempted an edit adding external links, keeping the filter set to "showcaptcha". This time, I got the standard "Your edit includes new external links..." message, and the captcha input was at the top, right below the message. (This is the expected behavior.)

Third, I set a filter to "warn" and "showcaptcha", and attempted an edit NOT adding external links. This time, I was bounced back and forth endlessly between the warning message and the request for a CAPTCHA. There was no way to save the page.

Fourth, I attempted an edit adding external links, keeping the filter set to "warn" and "showcaptcha". This time I got one request for a CAPTCHA, then the warning, then a second request, but the edit finally saved. (This is not optimal, but AFAIK an existing problem not caused by this patch.)

So three improvements are needed here:

• The message should say something other than "Incorrect or missing CAPTCHA." when no CAPTCHA was ever asked for.
• The CAPTCHA input should be at the top, near the message (same as when adding external links)
• There should be some way to save a page when a filter is set to warn+showcaptcha (other than adding a new link), or this option should be forbidden when saving a filter.