Remove separate checks for global blocks from PasswordReset
Closed, ResolvedPublic2 Estimated Story Points
Actions

Assigned To

Authored By

	Tchanders
	Sep 8 2022, 3:54 PM

Description

Background

PasswordReset::isBlocked checks for global blocks separately, and decides the user is blocked if ::appliesToPasswordReset returns true.

::appliesToPasswordReset never returns true for a GlobalBlock, so a global block alone does not block password reset (see appliesToPasswordReset definitions and isCreateAccountBlocked usage).

Additionally, after T257701: Add global blocks into CompositeBlocks rather than treating them separately, global blocks would not need to be checked separately anyway.

Acceptance criteria

PasswordReset::isBlocked no longer checks separately for global blocks.

Notes

Related question: Local blocks made by the GlobalBlocking extension do block password reset, by blocking account creation (example). Should global blocks block password reset and account creation? If so, this can be filed in a follow-up task and worked on separately.

Details

	Subject	Repo	Branch	Lines +/-
	Remove separate Global blocks check from PasswordReset::isBlocked	mediawiki/core	master	+1 -1

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Open		None	T231109 Remove Core coupling with GlobalBlocking extension
Open		None	T319069 Remove global-blocks-specific methods and hook from User class
Open	Spike	None	T315644 Global blocks should be treated as a subtype of blocks
Resolved		Cyndymediawiksim	T317337 Deprecate global-blocks-specific methods and hook
Resolved		• AGueyte	T317334 Remove separate checks for global blocks from PasswordReset
Resolved		Tchanders	T257701 Add global blocks into CompositeBlocks rather than treating them separately
Resolved		Tchanders	T317308 GlobalBlocking should specify block error message key
Resolved		• AGueyte	T317201 Allow extensions to modify block error message key via a hook
Resolved		STran	T317190 GlobalBlock should extend AbstractBlock instead of DatabaseBlock

Event Timeline

Tchanders created this task.Sep 8 2022, 3:54 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 8 2022, 3:54 PM

Tchanders updated the task description. (Show Details)Sep 8 2022, 3:54 PM

Tchanders mentioned this in T317337: Deprecate global-blocks-specific methods and hook.Sep 8 2022, 4:08 PM

Tchanders added a parent task: T317337: Deprecate global-blocks-specific methods and hook.Sep 8 2022, 4:16 PM

Password reset is a (minor) abuse vector, and it can be triggered from any wiki so normal blocks can't really prevent it. I think global blocks should apply to it.

Tchanders moved this task from Untriaged to Triage/To be Estimated on the Anti-Harassment board.Sep 9 2022, 3:37 PM

Tchanders mentioned this in T317636: Global blocks should apply to password reset.Sep 13 2022, 1:22 PM

In T317334#8223449, @Tgr wrote:

Password reset is a (minor) abuse vector, and it can be triggered from any wiki so normal blocks can't really prevent it. I think global blocks should apply to it.

Ok makes sense - I've filled T317636: Global blocks should apply to password reset for that.

Have also blocked this task on T257701: Add global blocks into CompositeBlocks rather than treating them separately to ensure global blocks are still checked by password reset, until global blocks are checked together with other blocks.

Tchanders set the point value for this task to 2.Oct 11 2022, 5:42 PM

• AGueyte claimed this task.Oct 14 2022, 1:34 PM

Change 842828 had a related patch set uploaded (by AGueyte; author: AGueyte):

[mediawiki/core@master] Remove Global blocks check from PasswordReset::isBlocked

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

gerritbot added a project: Patch-For-Review.Oct 14 2022, 3:02 PM

• AGueyte moved this task from Triage/To be Estimated to Cards ready for development on the Anti-Harassment board.Oct 14 2022, 3:36 PM

• AGueyte moved this task from Cards ready for development to AHaT Sprint 18: The Imperial State Crown on the Anti-Harassment board.

• AGueyte edited projects, added Anti-Harassment (AHaT Sprint 18: The Imperial State Crown); removed Anti-Harassment.

• AGueyte moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to Code Review 🔍 on the Anti-Harassment (AHaT Sprint 18: The Imperial State Crown) board.

Moving to blocked/stalled until the subtask is merged

ARamirez_WMF edited projects, added Anti-Harassment (AHaT Sprint 19: The Deerstalker); removed Anti-Harassment (AHaT Sprint 18: The Imperial State Crown).Oct 25 2022, 4:53 PM

ARamirez_WMF moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to Blocked/Stalled 🚧 on the Anti-Harassment (AHaT Sprint 19: The Deerstalker) board.

ARamirez_WMF edited projects, added Anti-Harassment (AHaT Sprint 19: The Sanbenito); removed Anti-Harassment (AHaT Sprint 19: The Deerstalker).Nov 8 2022, 5:24 AM

ARamirez_WMF moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to Blocked/Stalled 🚧 on the Anti-Harassment (AHaT Sprint 19: The Sanbenito) board.

Tchanders moved this task from Blocked/Stalled 🚧 to Code Review 🔍 on the Anti-Harassment (AHaT Sprint 19: The Sanbenito) board.Nov 9 2022, 4:23 PM

Testing notes:

This can be tested on Beta. (I tested locally.)

I tested this by visiting Special:PasswordReset from a globally blocked IP address, with no other block, including no local block. As expected, a block error is encountered. Doing the same from an IP address with no block, I see the form.

Note that this requires the latest version of the GlobalBlocking extension.

Change 842828 merged by jenkins-bot:

[mediawiki/core@master] Remove separate Global blocks check from PasswordReset::isBlocked

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

Maintenance_bot removed a project: Patch-For-Review.Nov 9 2022, 5:30 PM

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.10; 2022-11-14).Nov 9 2022, 6:00 PM

@Tchanders I tested Global block, Composite Block, Range IPs, XFF, Sitewide and Partial blocks which all came up with the same block error as seen in the screenshots below.

The only thing that I came up with was that same error of the uncheck in Single:Block from T317325
If that error is ok, can I move this task to Done?

Beta test for Global block, Composite Block, Range IPs, Sitewide and Partial blocks

T317334_GB_PasswordReset_Chrome.png (1×2 px, 186 KB)

Local test for XFF

T317334_GB_PasswordReset_Chrome2.png (1×2 px, 162 KB)

In T317334#8390247, @GMikesell-WMF wrote:

The only thing that I came up with was that same error of the uncheck in Single:Block from T317325
If that error is ok, can I move this task to Done?

Sounds good to me - see T317325#8396613.

@Tchanders Same non issue as T317325#8396613. I will move this to Done. Thanks!

GMikesell-WMF moved this task from QA/Testing 🐞 to Done Q2 2022-2023 on the Anti-Harassment (AHaT Sprint 19: The Sanbenito) board.Nov 15 2022, 7:11 PM

• Niharika closed this task as Resolved.Dec 8 2022, 1:17 AM

• Niharika closed subtask T257701: Add global blocks into CompositeBlocks rather than treating them separately as Resolved.Mar 1 2023, 1:39 AM

	F35756567: T317334_GB_PasswordReset_Chrome2.png
	Nov 11 2022, 10:15 PM

	F35756558: T317334_GB_PasswordReset_Chrome.png
	Nov 11 2022, 10:15 PM

Remove separate checks for global blocks from PasswordResetClosed, ResolvedPublic2 Estimated Story PointsActions