Page MenuHomePhabricator

hCaptcha risk scores: Hook handler for capturing when a block notice is presented in the Desktop editor
Closed, ResolvedPublic2 Estimated Story Points

Description

Summary

A mechanism is needed to trigger the risk score collection in ConfirmEdit when a blocked edit notice page is triggered from Core.

Specifically, we need a new BeforePageDisplayHookHandler hook handler in ConfirmEdit that loads the hCaptcha frontend module for block notices.

When an IP or range block is in place and the user attempts to edit, the handler:

  • injects a wgHCaptchaIpBlockRiskScore JS config object containing the passive mode sitekey, a mode identifier, and the IDs of blocks applying to the current page
  • then loads the ext.confirmEdit.hCaptcha frontend module in order to collect the risk score in the frontend.

Acceptance criteria:

  • A BeforePageDisplay hook handler in ConfirmEdit is triggered for edit pages where the user has an active IP or range block
  • The hook handler loads the hCaptcha frontend modules and passing the required config variables
  • wgHCaptchaIpBlockRiskScore is present on blocked edit pages with the correct siteKey, mode, and blockIds values
  • The sitekey provided for blocked edit pages comes from a different config param than the ones used for edits (since it needs to be the passive mode site key)
  • The handler does not fire on non-edit pages, or when no block is present
  • The handler ignores block types other than IP or IP range blocks
  • The ext.confirmEdit.hCaptcha frontend module is loaded on blocked edit pages for IPs and IP range blocks

Event Timeline

Change #1286851 had a related patch set uploaded (by Harroyo-wmf; author: Harroyo-wmf):

[mediawiki/extensions/ConfirmEdit@master] hCaptcha: Load hCaptcha modules for block notices in the Desktop editor

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

We are still making progress in this task, but the CI error is still happening after merging the change in T426392: hCaptcha: Add GlobalBlocking as a dependency for ConfirmEdit CI jobs

Change #1287907 had a related patch set uploaded (by Harroyo-wmf; author: Harroyo-wmf):

[mediawiki/extensions/ConfirmEdit@master] DNM Debug CI failure

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

Selenium tests for the patch associated with this task are failing, may be related to T276783: "Page should be undoable" selenium test is flaky

Happens even when modifying the the code in the hook handler to exit early and the new test file to skip the test right after it starts.

Change #1288016 had a related patch set uploaded (by Harroyo-wmf; author: Harroyo-wmf):

[mediawiki/extensions/ConfirmEdit@master] DNM Debug CI failure

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

Change #1287907 abandoned by Harroyo-wmf:

[mediawiki/extensions/ConfirmEdit@master] DNM Debug CI failure

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

A small version of the former patch https://gerrit.wikimedia.org/r/c/mediawiki/extensions/ConfirmEdit/+/1288016 (that exits early both in the handler and in the test itself) shows the same CI issue as the patch implementing the fix for this patch (https://gerrit.wikimedia.org/r/c/mediawiki/extensions/ConfirmEdit/+/1286851?tab=checks)

  1. Page should be undoable

19:50:06 [chrome-headless-shell 120.0.6099.224 linux #0-0] Can't call click on element with selector "#wpSave" because element wasn't found

EDIT: Unblocked

  • The CI failure was caused by a config variable name mismatch after renaming $wgHCaptchaPassiveModeSiteKey to $wgHCaptchaBlockedIpEditingScoreCollectionSiteKey per code review feedback. The rename was not done in extension.json, which caused the dependent extension tests to fail without a clear error message. This has been fixed now.
  • Renaming the variable was requested during code review after the initial implementation, and a simultaneous CI config change (also requested during review) made it harder to identify the root cause.
  • The original variable name was included in the initial draft shared on May 11th without objections at that time.

Change #1288016 abandoned by Harroyo-wmf:

[mediawiki/extensions/ConfirmEdit@master] DNM Debug CI failure

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

Change #1286851 merged by jenkins-bot:

[mediawiki/extensions/ConfirmEdit@master] hCaptcha: Load hCaptcha modules for block notices in the Desktop editor

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

QA will be done as part of the parent task.

Change #1290848 had a related patch set uploaded (by Harroyo-wmf; author: Harroyo-wmf):

[mediawiki/extensions/ConfirmEdit@master] hcaptcha: Split blocks in two lists of IDs (local and global)

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