Page MenuHomePhabricator

Undefined offset: 0 when resetting password with Special:PasswordReset [medium]
Closed, ResolvedPublicBUG REPORT

Description

As a Wikimedia user, I want the undefined offset bug fixed, so that the password reset process can run more smoothly.

Background: We have conducted some tests and looked into data on this issue. So far, this does not seem to be a user-facing bug and it is unlikely to occur. However, we want to be cautious, so we will be fixing this bug before we enable Enhanced Password Reset on other wikis.

Acceptance Criteria:

  • Fix the bug that throws an error (in the back-end) when user has an email associated with 2 accounts, enables enhanced password reset on 1 account, and then inputs only email address on Special:PasswordReset.
What is the problem?

Sometimes, when resetting password via Special:PasswordReset, the below exception occurs.

I think what is happening is:

  1. When Special:PasswordReset is submitted, it finds all users who match the username/email and puts them all into the $users array (starting here?)
  2. If only username or email was submitted, it removes from the $users array any user who has the Enhanced Password Reset preference enabled in Special:Preferences (here)
  3. This can lead to a $users array where the first item is undefined
  4. The SpecialPasswordResetOnSubmit hook is called, which passes the $users array as a parameter (here)
  5. CentralAuthHooks::onSpecialPasswordResetOnSubmit is one of the hooks that gets called, but it assumes that $users is either empty or has its first item defined (here)

So the solutions could be:

  1. Change the PasswordReset code to remove undefined items from the $users array
  2. Change the CentralAuthHooks code to not assume all items in the $users array are defined

Stack trace:

[error] [0913efa8abf52f9f07bcaaeb] /wiki/Special:PasswordReset   ErrorException from line 388 of /vagrant/mediawiki/extensions/CentralAuth/includes/CentralAuthHooks.php: PHP Notice: Undefined offset: 0
#0 /vagrant/mediawiki/extensions/CentralAuth/includes/CentralAuthHooks.php(388): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /vagrant/mediawiki/includes/Hooks.php(174): CentralAuthHooks::onSpecialPasswordResetOnSubmit(array, array, array)
#2 /vagrant/mediawiki/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
#3 /vagrant/mediawiki/includes/user/PasswordReset.php(206): Hooks::run(string, array)
#4 /vagrant/mediawiki/includes/specials/SpecialPasswordReset.php(151): PasswordReset->execute(User, NULL, string)
#5 /vagrant/mediawiki/includes/htmlform/HTMLForm.php(694): SpecialPasswordReset->onSubmit(array, OOUIHTMLForm)
#6 /vagrant/mediawiki/includes/htmlform/HTMLForm.php(586): HTMLForm->trySubmit()
#7 /vagrant/mediawiki/includes/htmlform/HTMLForm.php(601): HTMLForm->tryAuthorizedSubmit()
#8 /vagrant/mediawiki/includes/specialpage/FormSpecialPage.php(187): HTMLForm->show()
#9 /vagrant/mediawiki/includes/specials/SpecialPasswordReset.php(83): FormSpecialPage->execute(NULL)
#10 /vagrant/mediawiki/includes/specialpage/SpecialPage.php(575): SpecialPasswordReset->execute(NULL)
#11 /vagrant/mediawiki/includes/specialpage/SpecialPageFactory.php(622): SpecialPage->run(NULL)
#12 /vagrant/mediawiki/includes/MediaWiki.php(299): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#13 /vagrant/mediawiki/includes/MediaWiki.php(973): MediaWiki->performRequest()
#14 /vagrant/mediawiki/includes/MediaWiki.php(535): MediaWiki->main()
#15 /vagrant/mediawiki/index.php(47): MediaWiki->run()
#16 /var/www/w/index.php(5): require(string)
#17 {main}
Steps to reproduce problem

With $wgAllowRequiringEmailForResets = true;:

  1. Find (or create) two users who have the same email (e.g. email@email.com)
  2. For the user who was created earliest, enable enhanced password reset (Special:Preferences > "Send password reset emails only when both email address and username are provided.")
  3. For the user who was created latest, disable enhanced password reset (should be disabled by default)
  4. Go to Special:PasswordReset and submit just with email from step 1 (i.e. email@email.com)

Expected behavior: The user who was created latest gets their password reset, the other does not.
Observed behavior: Exception as noted.

Environment

Wiki(s): MediaWiki 1.35.0-alpha (9b125a0)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 8 2020, 1:07 PM
dom_walden updated the task description. (Show Details)Apr 8 2020, 1:09 PM
ARamirez_WMF renamed this task from Undefined offset: 0 when resetting password with Special:PasswordReset to Undefined offset: 0 when resetting password with Special:PasswordReset [medium].Apr 8 2020, 5:13 PM
ARamirez_WMF moved this task from New & TBD Tickets to Estimated on the Community-Tech board.
HMonroy claimed this task.Apr 8 2020, 7:34 PM
HMonroy moved this task from Ready to In Development on the Community-Tech (Kanban-2019-20-Q4) board.
ifried updated the task description. (Show Details)Apr 8 2020, 8:51 PM
ifried updated the task description. (Show Details)

Change 587612 had a related patch set uploaded (by HMonroy; owner: HMonroy):
[mediawiki/core@master] Handle the undefined offset in Special:PasswordReset

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

Change 587612 merged by jenkins-bot:
[mediawiki/core@master] Handle the undefined offset in Special:PasswordReset

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

On https://en.wikivoyage.beta.wmflabs.org (where $wgAllowRequiringEmailForResets = true; MediaWiki 1.35.0-alpha (ec55eb4) 06:40, 20 April 2020), when I repeat the reproduction steps, an emails gets sent out to the last created user. However, I cannot see whether or not there are exceptions on beta.

On my local environment, where I can see exceptions, the bug is no longer reproducible when I repeat the reproduction steps.

I did experiments similar to T246844#5991422, but where the test users all have the same email and are created in a random order on the wiki (as it matters for this bug which order users are created.)

As this issue is related to the SpecialPasswordResetOnSubmit hook, I searched the codebase and found two places where this hook is called: CentralAuth and GlobalBlocking.

Therefore, I also added some test users who existed globally but not on the wiki I was testing (to test CentralAuth). (To do this, you create a user on a wiki which is connected to the test wiki.)

And, I also repeated the experiments with the IP of the user performing the reset being globally blocked.

I did this a few times for different sets of users, each time randomising the order they were created in.

I did not see any exceptions in the logs, and all the emails which should have been sent out were.

ifried closed this task as Resolved.Wed, Jun 10, 1:15 AM
ifried moved this task from Product sign-off to Done on the Community-Tech (Kanban-2019-20-Q4) board.
ifried added a subscriber: ifried.

This has been tested on production, so I'm marking this work as Done.