Merge "Allow extensions to send password resets without a local user/email"

RELEASE-NOTES-1.43

@@ -738,6 +738,8 @@ because of Phabricator reports.
   rights from namespace protection ($wgNamespaceProtection) since that's
   only meant to prevent write actions.
 * Image captions are no longer trimmed.
+* 'SpecialPasswordResetOnSubmit' hook handlers can now receive both the
+  username and the email provided by the user, instead of only one of them.
 * …
 
 == Compatibility ==

includes/user/Hook/SpecialPasswordResetOnSubmitHook.php

@@ -16,6 +16,14 @@ interface SpecialPasswordResetOnSubmitHook {
 	/**
 	 * This hook is called when executing a form submission on Special:PasswordReset.
 	 *
+	 * The data submitted by the user ($data) is an associative array with the keys 'Username' and
+	 * 'Email', whose values are already validated user input (a valid username, and a valid email
+	 * address), or null if not given by the user. At least one of the values is not null.
+	 *
+	 * Since MediaWiki 1.43, hook handlers should check each user's 'requireemail' preference, and
+	 * if it is enabled by the user, only return that user if both username and email were present.
+	 * Until MediaWiki 1.42 only one of username and email could be present (the other would be null).
+	 *
 	 * @since 1.35
 	 *
 	 * @param User[] &$users

includes/user/PasswordReset.php

@@ -230,7 +230,16 @@ class PasswordReset implements LoggerAwareInterface {
 		$users = [];
 
 		if ( $username !== null ) {
-			$users[] = $this->userFactory->newFromName( $username );
+			$user = $this->userFactory->newFromName( $username );
+			// User must have an email address to attempt sending a password reset email
+			if ( $user && $user->isRegistered() && $user->getEmail() && (
+				!$this->userOptionsLookup->getBoolOption( $user, 'requireemail' ) ||
+				$user->getEmail() === $email
+			) ) {
+				// Either providing the email in the form is not required to request a reset,
+				// or the correct email was provided
+				$users[] = $user;
+			}
 
 		} elseif ( $email !== null ) {
 			foreach ( $this->getUsersByEmail( $email ) as $userIdent ) {
@@ -249,10 +258,7 @@ class PasswordReset implements LoggerAwareInterface {
 		// Check for hooks (captcha etc), and allow them to modify the users list
 		$data = [
 			'Username' => $username,
-			// Email is not provided to the hooks when we're resetting by username.
-			// We check for 'requireemail' below rather than relying on the hooks to do it.
-			// (However, we rely on the hooks doing it when resetting by email? That's a bit weird.)
-			'Email' => $username === null ? $email : null,
+			'Email' => $email,
 		];
 
 		$error = [];
@@ -260,40 +266,15 @@ class PasswordReset implements LoggerAwareInterface {
 			return StatusValue::newFatal( Message::newFromSpecifier( $error ) );
 		}
 
+		if ( !$users ) {
+			// Don't reveal whether or not a username or email address is in use
+			return StatusValue::newGood();
+		}
+
 		// Get the first element in $users by using `reset` function since
 		// the key '0' might have been unset from $users array by a hook handler.
 		$firstUser = reset( $users );
 
-		$requireEmail = $username !== null
-			&& $firstUser
-			&& $this->userOptionsLookup->getBoolOption( $firstUser, 'requireemail' );
-		if ( $requireEmail && $email === null ) {
-			// Email is required but not supplied: pretend everything's fine.
-			return StatusValue::newGood();
-		}
-
-		if ( !$users ) {
-			if ( $username === null ) {
-				// Don't reveal whether or not an email address is in use
-				return StatusValue::newGood();
-			} else {
-				return StatusValue::newFatal( 'noname' );
-			}
-		}
-
-		// If the user doesn't exist, or if the user doesn't have an email address,
-		// don't disclose the information. We want to pretend everything is ok per T238961.
-		// Note that all the users will have the same email address (or none),
-		// so there's no need to check more than the first.
-		if ( !$firstUser || !$firstUser->getId() || !$firstUser->getEmail() ) {
-			return StatusValue::newGood();
-		}
-
-		// Email is required but the email doesn't match: pretend everything's fine.
-		if ( $requireEmail && $firstUser->getEmail() !== $email ) {
-			return StatusValue::newGood();
-		}
-
 		$this->hookRunner->onUser__mailPasswordInternal( $performingUser, $ip, $firstUser );
 
 		$result = StatusValue::newGood();

tests/phpunit/includes/user/PasswordResetTest.php

@@ -619,12 +619,19 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 		$badUser->method( 'isRegistered' )->willReturn( true );
 		$badUser->method( 'getEmail' )->willReturn( '' );
 
+		$nonexistUser = $this->createMock( User::class );
+		$nonexistUser->method( 'getName' )->willReturn( 'Nonexistent user' );
+		$nonexistUser->method( 'getId' )->willReturn( 0 );
+		$nonexistUser->method( 'isRegistered' )->willReturn( false );
+		$nonexistUser->method( 'getEmail' )->willReturn( '' );
+
 		return [
 			'User1' => $user1,
 			'User2' => $user2,
 			'User3' => $user3,
 			'User4' => $user4,
 			'BadUser' => $badUser,
+			'Nonexistent user' => $nonexistUser,
 		];
 	}
 }