PasswordReset: remove $wgAllowRequiringEmailForResets feature flag

Update a few tests that relied on the feature flag to ignore
the 'requireemail' preference on "User1" to instead use "User2",
who doesn't have the preference set.

Bug: T242406
Change-Id: I996d3996272d704a071d1d2094c3568247b80f98

RELEASE-NOTES-1.43

@@ -84,6 +84,8 @@ For notes on 1.42.x and older releases, see HISTORY.
   [[Talk:Foo]]; [[Special:TalkPage/Project:Foo]] goes to [[Project talk:Foo]].
   This allows for links by templates and tools without having to parse what
   namespaces are valid and have associated talk pages.
+* New preference: "Send password reset emails only when both email address
+  and username are provided."
 * …
 
 === New features for sysadmins in 1.43 ===

docs/config-schema.yaml

@@ -4829,12 +4829,6 @@ config-schema:
             used. Keys in the array are ignored; the class name is conventionally
             used as the key to avoid collisions. Order is not significant.
             @since 1.27
-    AllowRequiringEmailForResets:
-        default: false
-        description: |-
-            Whether users will see a checkbox allowing them to require providing email during password resets.
-            @unstable EXPERIMENTAL This feature is under development, don't assume this flag's existence
-            or function outside of Wikimedia Foundation wikis.
     AutoCreateTempUser:
         properties:
             known: { type: boolean, default: false }

docs/config-vars.php

@@ -2663,12 +2663,6 @@ $wgAuthenticationTokenVersion = null;
  */
 $wgSessionProviders = null;
 
-/**
- * Config variable stub for the AllowRequiringEmailForResets setting, for use by phpdoc and IDEs.
- * @see MediaWiki\MainConfigSchema::AllowRequiringEmailForResets
- */
-$wgAllowRequiringEmailForResets = null;
-
 /**
  * Config variable stub for the AutoCreateTempUser setting, for use by phpdoc and IDEs.
  * @see MediaWiki\MainConfigSchema::AutoCreateTempUser

includes/MainConfigNames.php

@@ -2678,12 +2678,6 @@ class MainConfigNames {
 	 */
 	public const SessionProviders = 'SessionProviders';
 
-	/**
-	 * Name constant for the AllowRequiringEmailForResets setting, for use with Config::get()
-	 * @see MainConfigSchema::AllowRequiringEmailForResets
-	 */
-	public const AllowRequiringEmailForResets = 'AllowRequiringEmailForResets';
-
 	/**
 	 * Name constant for the AutoCreateTempUser setting, for use with Config::get()
 	 * @see MainConfigSchema::AutoCreateTempUser

includes/MainConfigSchema.php

@@ -7618,16 +7618,6 @@ class MainConfigSchema {
 		],
 	];
 
-	/**
-	 * Whether users will see a checkbox allowing them to require providing email during password resets.
-	 *
-	 * @unstable EXPERIMENTAL This feature is under development, don't assume this flag's existence
-	 * or function outside of Wikimedia Foundation wikis.
-	 */
-	public const AllowRequiringEmailForResets = [
-		'default' => false,
-	];
-
 	/**
 	 * Configuration for automatic creation of temporary accounts on page save.
 	 * This can be enabled to avoid exposing the IP addresses of casual editors who

includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php

@@ -55,9 +55,6 @@ class TemporaryPasswordPrimaryAuthenticationProvider
 	/** @var int */
 	protected $passwordReminderResendTime = null;
 
-	/** @var bool */
-	protected $allowRequiringEmail = null;
-
 	/** @var IConnectionProvider */
 	private $dbProvider;
 
@@ -89,9 +86,6 @@ class TemporaryPasswordPrimaryAuthenticationProvider
 		if ( isset( $params['passwordReminderResendTime'] ) ) {
 			$this->passwordReminderResendTime = $params['passwordReminderResendTime'];
 		}
-		if ( isset( $params['allowRequiringEmailForResets'] ) ) {
-			$this->allowRequiringEmail = $params['allowRequiringEmailForResets'];
-		}
 		$this->dbProvider = $dbProvider;
 		$this->userOptionsLookup = $userOptionsLookup;
 	}
@@ -101,8 +95,6 @@ class TemporaryPasswordPrimaryAuthenticationProvider
 		$this->newPasswordExpiry ??= $this->config->get( MainConfigNames::NewPasswordExpiry );
 		$this->passwordReminderResendTime ??=
 			$this->config->get( MainConfigNames::PasswordReminderResendTime );
-		$this->allowRequiringEmail ??=
-			$this->config->get( MainConfigNames::AllowRequiringEmailForResets );
 	}
 
 	protected function getPasswordResetData( $username, $data ) {
@@ -474,9 +466,7 @@ class TemporaryPasswordPrimaryAuthenticationProvider
 			'<' . Title::newMainPage()->getCanonicalURL() . '>',
 			round( $this->newPasswordExpiry / 86400 ) )->text();
 
-		if ( $this->allowRequiringEmail && !$this->userOptionsLookup
-			->getBoolOption( $user, 'requireemail' )
-		) {
+		if ( !$this->userOptionsLookup->getBoolOption( $user, 'requireemail' ) ) {
 			$body .= "\n\n";
 			$url = SpecialPage::getTitleFor( 'Preferences', false, 'mw-prefsection-personal-email' )
 				->getCanonicalURL();

includes/config-schema.php

@@ -1140,7 +1140,6 @@ return [
 					],
 				],
 			],
-			'AllowRequiringEmailForResets' => false,
 			'AutoCreateTempUser' => [
 				'known' => false,
 				'enabled' => false,

includes/preferences/DefaultPreferencesFactory.php

@@ -127,7 +127,6 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 	 * @internal For use by ServiceWiring
 	 */
 	public const CONSTRUCTOR_OPTIONS = [
-		MainConfigNames::AllowRequiringEmailForResets,
 		MainConfigNames::AllowUserCss,
 		MainConfigNames::AllowUserCssPrefs,
 		MainConfigNames::AllowUserJs,
@@ -571,8 +570,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 				] ),
 				'label-message' => 'yourpassword',
 				// email password reset feature only works for users that have an email set up
-				'help' => $this->options->get( MainConfigNames::AllowRequiringEmailForResets ) &&
-						$user->getEmail()
+				'help' => $user->getEmail()
 					? $context->msg( 'prefs-help-yourpassword',
 						'[[#mw-prefsection-personal-email|{{int:prefs-email}}]]' )->parse()
 					: '',
@@ -819,15 +817,13 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 
 			$disableEmailPrefs = false;
 
-			if ( $this->options->get( MainConfigNames::AllowRequiringEmailForResets ) ) {
-				$defaultPreferences['requireemail'] = [
-					'type' => 'toggle',
-					'label-message' => 'tog-requireemail',
-					'help-message' => 'prefs-help-requireemail',
-					'section' => 'personal/email',
-					'disabled' => !$user->getEmail(),
-				];
-			}
+			$defaultPreferences['requireemail'] = [
+				'type' => 'toggle',
+				'label-message' => 'tog-requireemail',
+				'help-message' => 'prefs-help-requireemail',
+				'section' => 'personal/email',
+				'disabled' => !$user->getEmail(),
+			];
 
 			if ( $this->options->get( MainConfigNames::EmailAuthentication ) ) {
 				if ( $user->getEmail() ) {

includes/user/PasswordReset.php

@@ -69,7 +69,6 @@ class PasswordReset implements LoggerAwareInterface {
 	 * @internal For use by ServiceWiring
 	 */
 	public const CONSTRUCTOR_OPTIONS = [
-		MainConfigNames::AllowRequiringEmailForResets,
 		MainConfigNames::EnableEmail,
 		MainConfigNames::PasswordResetRoutes,
 	];
@@ -236,10 +235,8 @@ class PasswordReset implements LoggerAwareInterface {
 		} elseif ( $email !== null ) {
 			foreach ( $this->getUsersByEmail( $email ) as $userIdent ) {
 				// Skip users whose preference 'requireemail' is on since username was not submitted
-				if ( $this->config->get( MainConfigNames::AllowRequiringEmailForResets ) ) {
-					if ( $this->userOptionsLookup->getBoolOption( $userIdent, 'requireemail' ) ) {
-						continue;
-					}
+				if ( $this->userOptionsLookup->getBoolOption( $userIdent, 'requireemail' ) ) {
+					continue;
 				}
 				$users[] = $this->userFactory->newFromUserIdentity( $userIdent );
 			}
@@ -267,8 +264,7 @@ class PasswordReset implements LoggerAwareInterface {
 		// the key '0' might have been unset from $users array by a hook handler.
 		$firstUser = reset( $users );
 
-		$requireEmail = $this->config->get( MainConfigNames::AllowRequiringEmailForResets )
-			&& $username !== null
+		$requireEmail = $username !== null
 			&& $firstUser
 			&& $this->userOptionsLookup->getBoolOption( $firstUser, 'requireemail' );
 		if ( $requireEmail && $email === null ) {

tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php

@@ -145,7 +145,6 @@ class TemporaryPasswordPrimaryAuthenticationProviderTest extends MediaWikiIntegr
 			MainConfigNames::EnableEmail => false,
 			MainConfigNames::NewPasswordExpiry => 100,
 			MainConfigNames::PasswordReminderResendTime => 101,
-			MainConfigNames::AllowRequiringEmailForResets => false,
 		] );
 
 		$provider = new TemporaryPasswordPrimaryAuthenticationProvider(
@@ -165,7 +164,6 @@ class TemporaryPasswordPrimaryAuthenticationProviderTest extends MediaWikiIntegr
 				'emailEnabled' => true,
 				'newPasswordExpiry' => 42,
 				'passwordReminderResendTime' => 43,
-				'allowRequiringEmailForResets' => true,
 			]
 		);
 		$providerPriv = TestingAccessWrapper::newFromObject( $provider );
@@ -173,7 +171,6 @@ class TemporaryPasswordPrimaryAuthenticationProviderTest extends MediaWikiIntegr
 		$this->assertSame( true, $providerPriv->emailEnabled );
 		$this->assertSame( 42, $providerPriv->newPasswordExpiry );
 		$this->assertSame( 43, $providerPriv->passwordReminderResendTime );
-		$this->assertSame( true, $providerPriv->allowRequiringEmail );
 	}
 
 	public function testTestUserCanAuthenticate() {

tests/phpunit/includes/user/PasswordResetTest.php

@@ -38,7 +38,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 	public function testIsAllowed( $passwordResetRoutes, $enableEmail,
 		$allowsAuthenticationDataChange, $canEditPrivate, $block, $isAllowed
 	) {
-		$config = $this->makeConfig( $enableEmail, $passwordResetRoutes, false );
+		$config = $this->makeConfig( $enableEmail, $passwordResetRoutes );
 
 		$authManager = $this->createMock( AuthManager::class );
 		$authManager->method( 'allowsAuthenticationDataChange' )
@@ -279,8 +279,8 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public function provideExecute() {
-		$defaultConfig = $this->makeConfig( true, [ 'username' => true, 'email' => true ], false );
-		$emailRequiredConfig = $this->makeConfig( true, [ 'username' => true, 'email' => true ], true );
+		// 'User1' has the 'requireemail' preference set (see testExecute()). Other users do not.
+		$defaultConfig = $this->makeConfig( true, [ 'username' => true, 'email' => true ] );
 		$performingUser = $this->makePerformingUser( self::VALID_IP, false );
 		$throttledUser = $this->makePerformingUser( self::VALID_IP, true );
 
@@ -296,7 +296,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Throttled, email required for resets, is invalid, pretend everything is ok' => [
 				'expectedError' => false,
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $throttledUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -323,7 +323,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Email route not enabled' => [
 				'expectedError' => 'passwordreset-nodata',
-				'config' => $this->makeConfig( true, [ 'username' => true ], false ),
+				'config' => $this->makeConfig( true, [ 'username' => true ] ),
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => '',
@@ -332,7 +332,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Username route not enabled' => [
 				'expectedError' => 'passwordreset-nodata',
-				'config' => $this->makeConfig( true, [ 'email' => true ], false ),
+				'config' => $this->makeConfig( true, [ 'email' => true ] ),
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -341,7 +341,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'No routes enabled' => [
 				'expectedError' => 'passwordreset-nodata',
-				'config' => $this->makeConfig( true, [], false ),
+				'config' => $this->makeConfig( true, [] ),
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -350,7 +350,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Email required for resets but is empty, pretend everything is OK' => [
 				'expectedError' => false,
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -359,7 +359,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Email required for resets but is invalid' => [
 				'expectedError' => 'passwordreset-invalidemail',
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -413,7 +413,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Email required for resets, no match' => [
 				'expectedError' => false,
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
 				'username' => 'User1',
@@ -433,8 +433,8 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 				'expectedError' => 'passwordreset-ignored',
 				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
-				'authManager' => $this->makeAuthManager( [ 'User1' ], 0, [ 'User1' ] ),
-				'username' => 'User1',
+				'authManager' => $this->makeAuthManager( [ 'User2' ], 0, [ 'User2' ] ),
+				'username' => 'User2',
 				'email' => '',
 				'usersWithEmail' => [],
 			],
@@ -452,7 +452,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager(),
-				'username' => 'User1',
+				'username' => 'User2',
 				'email' => '',
 				'usersWithEmail' => [],
 			],
@@ -479,23 +479,23 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 				'expectedError' => false,
 				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
-				'authManager' => $this->makeAuthManager( [ 'User1' ], 1 ),
+				'authManager' => $this->makeAuthManager( [ 'User2' ], 1 ),
 				'username' => '',
 				'email' => self::VALID_EMAIL,
-				'usersWithEmail' => [ 'User1' ],
+				'usersWithEmail' => [ 'User2' ],
 			],
 			'Reset multiple users via email' => [
 				'expectedError' => false,
 				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
-				'authManager' => $this->makeAuthManager( [ 'User1', 'User2' ], 2 ),
+				'authManager' => $this->makeAuthManager( [ 'User2', 'User3' ], 2 ),
 				'username' => '',
 				'email' => self::VALID_EMAIL,
-				'usersWithEmail' => [ 'User1', 'User2' ],
+				'usersWithEmail' => [ 'User2', 'User3' ],
 			],
-			"Email is required for resets, user didn't opt in" => [
+			"Email is not required for resets, this user didn't opt in" => [
 				'expectedError' => false,
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager( [ 'User2' ], 1 ),
 				'username' => 'User2',
@@ -504,7 +504,7 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 			],
 			'Reset three users via email that did not opt in, multiple users with same email' => [
 				'expectedError' => false,
-				'config' => $emailRequiredConfig,
+				'config' => $defaultConfig,
 				'performingUser' => $performingUser,
 				'authManager' => $this->makeAuthManager( [ 'User2', 'User3', 'User4' ], 3, [ 'User1' ] ),
 				'username' => '',
@@ -514,9 +514,8 @@ class PasswordResetTest extends MediaWikiIntegrationTestCase {
 		];
 	}
 
-	private function makeConfig( $enableEmail, array $passwordResetRoutes, $emailForResets ) {
+	private function makeConfig( $enableEmail, array $passwordResetRoutes ) {
 		$hash = [
-			MainConfigNames::AllowRequiringEmailForResets => $emailForResets,
 			MainConfigNames::EnableEmail => $enableEmail,
 			MainConfigNames::PasswordResetRoutes => $passwordResetRoutes,
 		];