Do not disable password reset for blocks meant to force login

Also remove resetpassword right (killed in I3ab5962d) from tests.

Bug: T161860
Change-Id: Ic7e7e9b4ff7fe94001578a895962ef732b690384

diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php
index 4ee256c..dd16fb7 100644 (file)
--- a/includes/user/PasswordReset.php
+++ b/includes/user/PasswordReset.php
@@ -100,9 +100,10 @@ class PasswordReset implements LoggerAwareInterface {
                        } elseif ( !$user->isAllowed( 'editmyprivateinfo' ) ) {
                                // Maybe not all users have permission to change private data
                                $status = StatusValue::newFatal( 'badaccess' );
-                       } elseif ( $user->isBlocked() ) {
+                       } elseif ( $this->isBlocked( $user ) ) {
                                // Maybe the user is blocked (check this here rather than relying on the parent
-                               // method as we have a more specific error message to use here
+                               // method as we have a more specific error message to use here and we want to
+                               // ignore some types of blocks)
                                $status = StatusValue::newFatal( 'blocked-mailpassword' );
                        }
 
@@ -250,6 +251,37 @@ class PasswordReset implements LoggerAwareInterface {
                return StatusValue::newGood( $passwords );
        }
 
+       /**
+        * Check whether the user is blocked.
+        * Ignores certain types of system blocks that are only meant to force users to log in.
+        * @param User $user
+        * @return bool
+        * @since 1.30
+        */
+       protected function isBlocked( User $user ) {
+               $block = $user->getBlock() ?: $user->getGlobalBlock();
+               if ( !$block ) {
+                       return false;
+               }
+               $type = $block->getSystemBlockType();
+               if ( in_array( $type, [ null, 'global-block' ], true ) ) {
+                       // Normal block. Maybe it was meant for someone else and the user just needs to log in;
+                       // or maybe it was issued specifically to prevent some IP from messing with password
+                       // reset? Go out on a limb and use the registration allowed flag to decide.
+                       return $block->prevents( 'createaccount' );
+               } elseif ( $type === 'proxy' ) {
+                       // we disallow actions through proxy even if the user is logged in
+                       // so it makes sense to disallow password resets as well
+                       return true;
+               } elseif ( in_array( $type, [ 'dnsbl', 'wgSoftBlockRanges' ], true ) ) {
+                       // these are just meant to force login so let's not prevent that
+                       return false;
+               } else {
+                       // some extension - we'll have to guess
+                       return true;
+               }
+       }
+
        /**
         * @param string $email
         * @return User[]

diff --git a/tests/phpunit/includes/user/PasswordResetTest.php b/tests/phpunit/includes/user/PasswordResetTest.php
index 3363bca..53f02df 100644 (file)
--- a/tests/phpunit/includes/user/PasswordResetTest.php
+++ b/tests/phpunit/includes/user/PasswordResetTest.php
@@ -10,8 +10,7 @@ class PasswordResetTest extends PHPUnit_Framework_TestCase {
         * @dataProvider provideIsAllowed
         */
        public function testIsAllowed( $passwordResetRoutes, $enableEmail,
-               $allowsAuthenticationDataChange, $canEditPrivate, $canSeePassword,
-               $userIsBlocked, $isAllowed
+               $allowsAuthenticationDataChange, $canEditPrivate, $block, $globalBlock, $isAllowed
        ) {
                $config = new HashConfig( [
                        'PasswordResetRoutes' => $passwordResetRoutes,
@@ -25,13 +24,12 @@ class PasswordResetTest extends PHPUnit_Framework_TestCase {
 
                $user = $this->getMockBuilder( User::class )->getMock();
                $user->expects( $this->any() )->method( 'getName' )->willReturn( 'Foo' );
-               $user->expects( $this->any() )->method( 'isBlocked' )->willReturn( $userIsBlocked );
+               $user->expects( $this->any() )->method( 'getBlock' )->willReturn( $block );
+               $user->expects( $this->any() )->method( 'getGlobalBlock' )->willReturn( $globalBlock );
                $user->expects( $this->any() )->method( 'isAllowed' )
-                       ->will( $this->returnCallback( function ( $perm ) use ( $canEditPrivate, $canSeePassword ) {
+                       ->will( $this->returnCallback( function ( $perm ) use ( $canEditPrivate ) {
                                if ( $perm === 'editmyprivateinfo' ) {
                                        return $canEditPrivate;
-                               } elseif ( $perm === 'passwordreset' ) {
-                                       return $canSeePassword;
                                } else {
                                        $this->fail( 'Unexpected permission check' );
                                }
@@ -44,67 +42,103 @@ class PasswordResetTest extends PHPUnit_Framework_TestCase {
 
        public function provideIsAllowed() {
                return [
-                       [
+                       'no routes' => [
                                'passwordResetRoutes' => [],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => true,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => false,
+                               'block' => null,
+                               'globalBlock' => null,
                                'isAllowed' => false,
                        ],
-                       [
+                       'email disabled' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => false,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => true,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => false,
+                               'block' => null,
+                               'globalBlock' => null,
                                'isAllowed' => false,
                        ],
-                       [
+                       'auth data change disabled' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => false,
                                'canEditPrivate' => true,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => false,
+                               'block' => null,
+                               'globalBlock' => null,
                                'isAllowed' => false,
                        ],
-                       [
+                       'cannot edit private data' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => false,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => false,
+                               'block' => null,
+                               'globalBlock' => null,
                                'isAllowed' => false,
                        ],
-                       [
+                       'blocked with account creation disabled' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => true,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => true,
+                               'block' => new Block( [ 'createAccount' => true ] ),
+                               'globalBlock' => null,
                                'isAllowed' => false,
                        ],
-                       [
+                       'blocked w/o account creation disabled' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => true,
-                               'canSeePassword' => false,
-                               'userIsBlocked' => false,
+                               'block' => new Block( [] ),
+                               'globalBlock' => null,
                                'isAllowed' => true,
                        ],
-                       [
+                       'using blocked proxy' => [
                                'passwordResetRoutes' => [ 'username' => true ],
                                'enableEmail' => true,
                                'allowsAuthenticationDataChange' => true,
                                'canEditPrivate' => true,
-                               'canSeePassword' => true,
-                               'userIsBlocked' => false,
+                               'block' => new Block( [ 'systemBlock' => 'proxy' ] ),
+                               'globalBlock' => null,
+                               'isAllowed' => false,
+                       ],
+                       'globally blocked with account creation disabled' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => null,
+                               'globalBlock' => new Block( [ 'systemBlock' => 'global-block', 'createAccount' => true ] ),
+                               'isAllowed' => false,
+                       ],
+                       'globally blocked with account creation not disabled' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => null,
+                               'globalBlock' => new Block( [ 'systemBlock' => 'global-block', 'createAccount' => false ] ),
+                               'isAllowed' => true,
+                       ],
+                       'blocked via wgSoftBlockRanges' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => new Block( [ 'systemBlock' => 'wgSoftBlockRanges', 'anonOnly' => true ] ),
+                               'globalBlock' => null,
+                               'isAllowed' => true,
+                       ],
+                       'all OK' => [
+                               'passwordResetRoutes' => [ 'username' => true ],
+                               'enableEmail' => true,
+                               'allowsAuthenticationDataChange' => true,
+                               'canEditPrivate' => true,
+                               'block' => null,
+                               'globalBlock' => null,
                                'isAllowed' => true,
                        ],
                ];