diff --git a/RELEASE-NOTES-1.43 b/RELEASE-NOTES-1.43 index 9d6545ceca9..57cb5cbb7ae 100644 --- a/RELEASE-NOTES-1.43 +++ b/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 === diff --git a/docs/config-schema.yaml b/docs/config-schema.yaml index 54f75777874..9bda0851fe7 100644 --- a/docs/config-schema.yaml +++ b/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 } diff --git a/docs/config-vars.php b/docs/config-vars.php index 9b109b4c640..1678c9dc840 100644 --- a/docs/config-vars.php +++ b/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 diff --git a/includes/MainConfigNames.php b/includes/MainConfigNames.php index a3353d62cc5..b20eba1c98e 100644 --- a/includes/MainConfigNames.php +++ b/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 diff --git a/includes/MainConfigSchema.php b/includes/MainConfigSchema.php index eded970dcf4..1a2cd6f4f55 100644 --- a/includes/MainConfigSchema.php +++ b/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 diff --git a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php b/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php index e842af9b249..188ad3896c9 100644 --- a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php +++ b/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(); diff --git a/includes/config-schema.php b/includes/config-schema.php index 852991d2842..4fda1fb62b4 100644 --- a/includes/config-schema.php +++ b/includes/config-schema.php @@ -1140,7 +1140,6 @@ return [ ], ], ], - 'AllowRequiringEmailForResets' => false, 'AutoCreateTempUser' => [ 'known' => false, 'enabled' => false, diff --git a/includes/preferences/DefaultPreferencesFactory.php b/includes/preferences/DefaultPreferencesFactory.php index edc0da81658..90dca507c29 100644 --- a/includes/preferences/DefaultPreferencesFactory.php +++ b/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() ) { diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php index b89c3d5850c..eb630ff6ed7 100644 --- a/includes/user/PasswordReset.php +++ b/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 ) { diff --git a/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php b/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php index b4803229849..2dd83548108 100644 --- a/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php +++ b/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() { diff --git a/tests/phpunit/includes/user/PasswordResetTest.php b/tests/phpunit/includes/user/PasswordResetTest.php index e4a59a68fae..f1bb0705dd2 100644 --- a/tests/phpunit/includes/user/PasswordResetTest.php +++ b/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, ];