Collapse some else statements and nested if statements
[lhc/web/wiklou.git] / includes / user / User.php
index 1412215..f820861 100644 (file)
@@ -58,7 +58,7 @@ class User implements IDBAccessObject, UserIdentity {
        /**
         * @const int Serialized record version.
         */
-       const VERSION = 12;
+       const VERSION = 13;
 
        /**
         * Exclude user options that are set to their default value.
@@ -589,14 +589,15 @@ class User implements IDBAccessObject, UserIdentity {
                $name = self::getCanonicalName( $name, $validate );
                if ( $name === false ) {
                        return false;
-               } else {
-                       // Create unloaded user object
-                       $u = new User;
-                       $u->mName = $name;
-                       $u->mFrom = 'name';
-                       $u->setItemLoaded( 'name' );
-                       return $u;
                }
+
+               // Create unloaded user object
+               $u = new User;
+               $u->mName = $name;
+               $u->mFrom = 'name';
+               $u->setItemLoaded( 'name' );
+
+               return $u;
        }
 
        /**
@@ -1126,12 +1127,12 @@ class User implements IDBAccessObject, UserIdentity {
                }
 
                // Preg yells if you try to give it an empty string
-               if ( $wgInvalidUsernameCharacters !== '' ) {
-                       if ( preg_match( '/[' . preg_quote( $wgInvalidUsernameCharacters, '/' ) . ']/', $name ) ) {
-                               wfDebugLog( 'username', __METHOD__ .
-                                       ": '$name' invalid due to wgInvalidUsernameCharacters" );
-                               return false;
-                       }
+               if ( $wgInvalidUsernameCharacters !== '' &&
+                       preg_match( '/[' . preg_quote( $wgInvalidUsernameCharacters, '/' ) . ']/', $name )
+               ) {
+                       wfDebugLog( 'username', __METHOD__ .
+                               ": '$name' invalid due to wgInvalidUsernameCharacters" );
+                       return false;
                }
 
                return self::isUsableName( $name );
@@ -1144,8 +1145,8 @@ class User implements IDBAccessObject, UserIdentity {
         * @return bool
         */
        public function isValidPassword( $password ) {
-               // simple boolean wrapper for getPasswordValidity
-               return $this->getPasswordValidity( $password ) === true;
+               // simple boolean wrapper for checkPasswordValidity
+               return $this->checkPasswordValidity( $password )->isGood();
        }
 
        /**
@@ -1153,38 +1154,44 @@ class User implements IDBAccessObject, UserIdentity {
         *
         * @param string $password Desired password
         * @return bool|string|array True on success, string or array of error message on failure
+        * @deprecated since 1.33, use checkPasswordValidity
         */
        public function getPasswordValidity( $password ) {
+               wfDeprecated( __METHOD__, '1.33' );
+
                $result = $this->checkPasswordValidity( $password );
                if ( $result->isGood() ) {
                        return true;
-               } else {
-                       $messages = [];
-                       foreach ( $result->getErrorsByType( 'error' ) as $error ) {
-                               $messages[] = $error['message'];
-                       }
-                       foreach ( $result->getErrorsByType( 'warning' ) as $warning ) {
-                               $messages[] = $warning['message'];
-                       }
-                       if ( count( $messages ) === 1 ) {
-                               return $messages[0];
-                       }
-                       return $messages;
                }
+
+               $messages = [];
+               foreach ( $result->getErrorsByType( 'error' ) as $error ) {
+                       $messages[] = $error['message'];
+               }
+               foreach ( $result->getErrorsByType( 'warning' ) as $warning ) {
+                       $messages[] = $warning['message'];
+               }
+               if ( count( $messages ) === 1 ) {
+                       return $messages[0];
+               }
+
+               return $messages;
        }
 
        /**
         * Check if this is a valid password for this user
         *
-        * Create a Status object based on the password's validity.
-        * The Status should be set to fatal if the user should not
-        * be allowed to log in, and should have any errors that
-        * would block changing the password.
-        *
-        * If the return value of this is not OK, the password
-        * should not be checked. If the return value is not Good,
-        * the password can be checked, but the user should not be
-        * able to set their password to this.
+        * Returns a Status object with a set of messages describing
+        * problems with the password. If the return status is fatal,
+        * the action should be refused and the password should not be
+        * checked at all (this is mainly meant for DoS mitigation).
+        * If the return value is OK but not good, the password can be checked,
+        * but the user should not be able to set their password to this.
+        * The value of the returned Status object will be an array which
+        * can have the following fields:
+        * - forceChange (bool): if set to true, the user should not be
+        *   allowed to log with this password unless they change it during
+        *   the login process (see ResetPasswordSecondaryAuthenticationProvider).
         *
         * @param string $password Desired password
         * @return Status
@@ -1198,7 +1205,7 @@ class User implements IDBAccessObject, UserIdentity {
                        $wgPasswordPolicy['checks']
                );
 
-               $status = Status::newGood();
+               $status = Status::newGood( [] );
                $result = false; // init $result to false for the internal checks
 
                if ( !Hooks::run( 'isValidPassword', [ $password, &$result, $this ] ) ) {
@@ -1207,14 +1214,16 @@ class User implements IDBAccessObject, UserIdentity {
                }
 
                if ( $result === false ) {
-                       $status->merge( $upp->checkUserPassword( $this, $password ) );
+                       $status->merge( $upp->checkUserPassword( $this, $password ), true );
                        return $status;
-               } elseif ( $result === true ) {
+               }
+
+               if ( $result === true ) {
                        return $status;
-               } else {
-                       $status->error( $result );
-                       return $status; // the isValidPassword hook set a string $result and returned true
                }
+
+               $status->error( $result );
+               return $status; // the isValidPassword hook set a string $result and returned true
        }
 
        /**
@@ -1460,12 +1469,13 @@ class User implements IDBAccessObject, UserIdentity {
                        $this->mGroupMemberships = null; // deferred
                        $this->getEditCount(); // revalidation for nulls
                        return true;
-               } else {
-                       // Invalid user_id
-                       $this->mId = 0;
-                       $this->loadDefaults();
-                       return false;
                }
+
+               // Invalid user_id
+               $this->mId = 0;
+               $this->loadDefaults();
+
+               return false;
        }
 
        /**
@@ -1574,7 +1584,7 @@ class User implements IDBAccessObject, UserIdentity {
 
                if ( is_array( $data ) ) {
                        if ( isset( $data['user_groups'] ) && is_array( $data['user_groups'] ) ) {
-                               if ( !count( $data['user_groups'] ) ) {
+                               if ( $data['user_groups'] === [] ) {
                                        $this->mGroupMemberships = [];
                                } else {
                                        $firstGroup = reset( $data['user_groups'] );
@@ -1640,7 +1650,7 @@ class User implements IDBAccessObject, UserIdentity {
                }
 
                $toPromote = Autopromote::getAutopromoteOnceGroups( $this, $event );
-               if ( !count( $toPromote ) ) {
+               if ( $toPromote === [] ) {
                        return [];
                }
 
@@ -1970,10 +1980,10 @@ class User implements IDBAccessObject, UserIdentity {
                                if ( $blockIsValid && $useBlockCookie ) {
                                        // Use the block.
                                        return $tmpBlock;
-                               } else {
-                                       // If the block is not valid, remove the cookie.
-                                       Block::clearCookie( $this->getRequest()->response() );
                                }
+
+                               // If the block is not valid, remove the cookie.
+                               Block::clearCookie( $this->getRequest()->response() );
                        } else {
                                // If the block doesn't exist, remove the cookie.
                                Block::clearCookie( $this->getRequest()->response() );
@@ -1992,11 +2002,9 @@ class User implements IDBAccessObject, UserIdentity {
        public function isDnsBlacklisted( $ip, $checkWhitelist = false ) {
                global $wgEnableDnsBlacklist, $wgDnsBlacklistUrls, $wgProxyWhitelist;
 
-               if ( !$wgEnableDnsBlacklist ) {
-                       return false;
-               }
-
-               if ( $checkWhitelist && in_array( $ip, $wgProxyWhitelist ) ) {
+               if ( !$wgEnableDnsBlacklist ||
+                       ( $checkWhitelist && in_array( $ip, $wgProxyWhitelist ) )
+               ) {
                        return false;
                }
 
@@ -2040,9 +2048,9 @@ class User implements IDBAccessObject, UserIdentity {
                                        wfDebugLog( 'dnsblacklist', "Hostname $host is {$ipList[0]}, it's a proxy says $basename!" );
                                        $found = true;
                                        break;
-                               } else {
-                                       wfDebugLog( 'dnsblacklist', "Requested $host, not found in $basename." );
                                }
+
+                               wfDebugLog( 'dnsblacklist', "Requested $host, not found in $basename." );
                        }
                }
 
@@ -2162,11 +2170,9 @@ class User implements IDBAccessObject, UserIdentity {
                        if ( isset( $limits['anon'] ) ) {
                                $keys[$cache->makeKey( 'limiter', $action, 'anon' )] = $limits['anon'];
                        }
-               } else {
+               } elseif ( isset( $limits['user'] ) ) {
                        // limits for logged-in users
-                       if ( isset( $limits['user'] ) ) {
-                               $userLimit = $limits['user'];
-                       }
+                       $userLimit = $limits['user'];
                }
 
                // limits for anons and for newbie logged-in users
@@ -2300,14 +2306,25 @@ class User implements IDBAccessObject, UserIdentity {
                                // Special handling for a user's own talk page. The block is not aware
                                // of the user, so this must be done here.
                                if ( $title->equals( $this->getTalkPage() ) ) {
-                                       // If the block is sitewide, then whatever is set is what is honored.
                                        if ( $block->isSitewide() ) {
+                                               // If the block is sitewide, whatever is set is what is honored.
+                                               // This must be checked here, because Block::appliesToPage will
+                                               // return true for a sitewide block.
                                                $blocked = $block->prevents( 'editownusertalk' );
                                        } else {
-                                               // If the block is partial, ignore 'editownusertalk' unless
-                                               // there is a restriction on the user talk namespace.
-                                               // TODO: To be implemented with Namespace restrictions
-                                               $blocked = $block->appliesToTitle( $title );
+                                               // The page restrictions always take precedence over the namespace
+                                               // restrictions. If the user is explicity blocked from their own
+                                               // talk page, nothing can change that.
+                                               $blocked = $block->appliesToPage( $title->getArticleID() );
+
+                                               // If the block applies to the user talk namespace, then whatever is
+                                               // set is what is honored.
+                                               if ( !$blocked && $block->appliesToNamespace( NS_USER_TALK ) ) {
+                                                       $blocked = $block->prevents( 'editownusertalk' );
+                                               }
+
+                                               // If another type of restriction is added, it should be checked
+                                               // here.
                                        }
                                } else {
                                        $blocked = $block->appliesToTitle( $title );
@@ -2445,7 +2462,9 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $this->mId === null && $this->mName !== null && self::isIP( $this->mName ) ) {
                        // Special case, we know the user is anonymous
                        return 0;
-               } elseif ( !$this->isItemLoaded( 'id' ) ) {
+               }
+
+               if ( !$this->isItemLoaded( 'id' ) ) {
                        // Don't load if this was initialized from an ID
                        $this->load();
                }
@@ -2470,14 +2489,15 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $this->isItemLoaded( 'name', 'only' ) ) {
                        // Special case optimisation
                        return $this->mName;
-               } else {
-                       $this->load();
-                       if ( $this->mName === false ) {
-                               // Clean up IPs
-                               $this->mName = IP::sanitizeIP( $this->getRequest()->getIP() );
-                       }
-                       return $this->mName;
                }
+
+               $this->load();
+               if ( $this->mName === false ) {
+                       // Clean up IPs
+                       $this->mName = IP::sanitizeIP( $this->getRequest()->getIP() );
+               }
+
+               return $this->mName;
        }
 
        /**
@@ -2623,7 +2643,9 @@ class User implements IDBAccessObject, UserIdentity {
                $talks = [];
                if ( !Hooks::run( 'UserRetrieveNewTalks', [ &$user, &$talks ] ) ) {
                        return $talks;
-               } elseif ( !$this->getNewtalk() ) {
+               }
+
+               if ( !$this->getNewtalk() ) {
                        return [];
                }
                $utp = $this->getTalkPage();
@@ -2651,19 +2673,19 @@ class User implements IDBAccessObject, UserIdentity {
        public function getNewMessageRevisionId() {
                $newMessageRevisionId = null;
                $newMessageLinks = $this->getNewMessageLinks();
-               if ( $newMessageLinks ) {
-                       // Note: getNewMessageLinks() never returns more than a single link
-                       // and it is always for the same wiki, but we double-check here in
-                       // case that changes some time in the future.
-                       if ( count( $newMessageLinks ) === 1
-                               && WikiMap::isCurrentWikiId( $newMessageLinks[0]['wiki'] )
-                               && $newMessageLinks[0]['rev']
-                       ) {
-                               /** @var Revision $newMessageRevision */
-                               $newMessageRevision = $newMessageLinks[0]['rev'];
-                               $newMessageRevisionId = $newMessageRevision->getId();
-                       }
+
+               // Note: getNewMessageLinks() never returns more than a single link
+               // and it is always for the same wiki, but we double-check here in
+               // case that changes some time in the future.
+               if ( $newMessageLinks && count( $newMessageLinks ) === 1
+                       && WikiMap::isCurrentWikiId( $newMessageLinks[0]['wiki'] )
+                       && $newMessageLinks[0]['rev']
+               ) {
+                       /** @var Revision $newMessageRevision */
+                       $newMessageRevision = $newMessageLinks[0]['rev'];
+                       $newMessageRevisionId = $newMessageRevision->getId();
                }
+
                return $newMessageRevisionId;
        }
 
@@ -2703,10 +2725,10 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $dbw->affectedRows() ) {
                        wfDebug( __METHOD__ . ": set on ($field, $id)\n" );
                        return true;
-               } else {
-                       wfDebug( __METHOD__ . " already set ($field, $id)\n" );
-                       return false;
                }
+
+               wfDebug( __METHOD__ . " already set ($field, $id)\n" );
+               return false;
        }
 
        /**
@@ -2723,10 +2745,10 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $dbw->affectedRows() ) {
                        wfDebug( __METHOD__ . ": killed on ($field, $id)\n" );
                        return true;
-               } else {
-                       wfDebug( __METHOD__ . ": already gone ($field, $id)\n" );
-                       return false;
                }
+
+               wfDebug( __METHOD__ . ": already gone ($field, $id)\n" );
+               return false;
        }
 
        /**
@@ -3007,25 +3029,30 @@ class User implements IDBAccessObject, UserIdentity {
                if ( !$this->mToken ) {
                        // The user doesn't have a token, return null to indicate that.
                        return null;
-               } elseif ( $this->mToken === self::INVALID_TOKEN ) {
+               }
+
+               if ( $this->mToken === self::INVALID_TOKEN ) {
                        // We return a random value here so existing token checks are very
                        // likely to fail.
                        return MWCryptRand::generateHex( self::TOKEN_LENGTH );
-               } elseif ( $wgAuthenticationTokenVersion === null ) {
+               }
+
+               if ( $wgAuthenticationTokenVersion === null ) {
                        // $wgAuthenticationTokenVersion not in use, so return the raw secret
                        return $this->mToken;
-               } else {
-                       // $wgAuthenticationTokenVersion in use, so hmac it.
-                       $ret = MWCryptHash::hmac( $wgAuthenticationTokenVersion, $this->mToken, false );
-
-                       // The raw hash can be overly long. Shorten it up.
-                       $len = max( 32, self::TOKEN_LENGTH );
-                       if ( strlen( $ret ) < $len ) {
-                               // Should never happen, even md5 is 128 bits
-                               throw new \UnexpectedValueException( 'Hmac returned less than 128 bits' );
-                       }
-                       return substr( $ret, -$len );
                }
+
+               // $wgAuthenticationTokenVersion in use, so hmac it.
+               $ret = MWCryptHash::hmac( $wgAuthenticationTokenVersion, $this->mToken, false );
+
+               // The raw hash can be overly long. Shorten it up.
+               $len = max( 32, self::TOKEN_LENGTH );
+               if ( strlen( $ret ) < $len ) {
+                       // Should never happen, even md5 is 128 bits
+                       throw new \UnexpectedValueException( 'Hmac returned less than 128 bits' );
+               }
+
+               return substr( $ret, -$len );
        }
 
        /**
@@ -3114,19 +3141,17 @@ class User implements IDBAccessObject, UserIdentity {
                $type = $oldaddr != '' ? 'changed' : 'set';
                $notificationResult = null;
 
-               if ( $wgEmailAuthentication ) {
+               if ( $wgEmailAuthentication && $type === 'changed' ) {
                        // Send the user an email notifying the user of the change in registered
                        // email address on their previous email address
-                       if ( $type == 'changed' ) {
-                               $change = $str != '' ? 'changed' : 'removed';
-                               $notificationResult = $this->sendMail(
-                                       wfMessage( 'notificationemail_subject_' . $change )->text(),
-                                       wfMessage( 'notificationemail_body_' . $change,
-                                               $this->getRequest()->getIP(),
-                                               $this->getName(),
-                                               $str )->text()
-                               );
-                       }
+                       $change = $str != '' ? 'changed' : 'removed';
+                       $notificationResult = $this->sendMail(
+                               wfMessage( 'notificationemail_subject_' . $change )->text(),
+                               wfMessage( 'notificationemail_body_' . $change,
+                                       $this->getRequest()->getIP(),
+                                       $this->getName(),
+                                       $str )->text()
+                       );
                }
 
                $this->setEmail( $str );
@@ -3196,9 +3221,9 @@ class User implements IDBAccessObject, UserIdentity {
 
                if ( array_key_exists( $oname, $this->mOptions ) ) {
                        return $this->mOptions[$oname];
-               } else {
-                       return $defaultOverride;
                }
+
+               return $defaultOverride;
        }
 
        /**
@@ -3528,14 +3553,15 @@ class User implements IDBAccessObject, UserIdentity {
                global $wgSecureLogin;
                if ( !$wgSecureLogin ) {
                        return false;
-               } else {
-                       $https = $this->getBoolOption( 'prefershttps' );
-                       Hooks::run( 'UserRequiresHTTPS', [ $this, &$https ] );
-                       if ( $https ) {
-                               $https = wfCanIPUseHTTPS( $this->getRequest()->getIP() );
-                       }
-                       return $https;
                }
+
+               $https = $this->getBoolOption( 'prefershttps' );
+               Hooks::run( 'UserRequiresHTTPS', [ $this, &$https ] );
+               if ( $https ) {
+                       $https = wfCanIPUseHTTPS( $this->getRequest()->getIP() );
+               }
+
+               return $https;
        }
 
        /**
@@ -3598,7 +3624,8 @@ class User implements IDBAccessObject, UserIdentity {
        /**
         * Get the list of explicit group memberships this user has.
         * The implicit * and user groups are not included.
-        * @return array Array of String internal group names
+        *
+        * @return string[] Array of internal group names (sorted since 1.33)
         */
        public function getGroups() {
                $this->load();
@@ -3916,10 +3943,10 @@ class User implements IDBAccessObject, UserIdentity {
        public function getRequest() {
                if ( $this->mRequest ) {
                        return $this->mRequest;
-               } else {
-                       global $wgRequest;
-                       return $wgRequest;
                }
+
+               global $wgRequest;
+               return $wgRequest;
        }
 
        /**
@@ -4091,19 +4118,18 @@ class User implements IDBAccessObject, UserIdentity {
                $learnerRegistration = wfTimestamp( TS_MW, $now - $wgLearnerMemberSince * 86400 );
                $experiencedRegistration = wfTimestamp( TS_MW, $now - $wgExperiencedUserMemberSince * 86400 );
 
-               if (
-                       $editCount < $wgLearnerEdits ||
-                       $registration > $learnerRegistration
-               ) {
+               if ( $editCount < $wgLearnerEdits ||
+               $registration > $learnerRegistration ) {
                        return 'newcomer';
-               } elseif (
-                       $editCount > $wgExperiencedUserEdits &&
+               }
+
+               if ( $editCount > $wgExperiencedUserEdits &&
                        $registration <= $experiencedRegistration
                ) {
                        return 'experienced';
-               } else {
-                       return 'learner';
                }
+
+               return 'learner';
        }
 
        /**
@@ -4539,6 +4565,7 @@ class User implements IDBAccessObject, UserIdentity {
        /**
         * Get whether the user is blocked from using Special:Upload
         *
+        * @since 1.33
         * @return bool
         */
        public function isBlockedFromUpload() {
@@ -4919,9 +4946,9 @@ class User implements IDBAccessObject, UserIdentity {
                                return false;
                        }
                        return true;
-               } else {
-                       return $confirmed;
                }
+
+               return $confirmed;
        }
 
        /**
@@ -5197,9 +5224,9 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $title ) {
                        return MediaWikiServices::getInstance()
                                ->getLinkRenderer()->makeLink( $title, $text );
-               } else {
-                       return htmlspecialchars( $text );
                }
+
+               return htmlspecialchars( $text );
        }
 
        /**
@@ -5222,9 +5249,9 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $title ) {
                        $page = $title->getFullText();
                        return "[[$page|$text]]";
-               } else {
-                       return $text;
                }
+
+               return $text;
        }
 
        /**
@@ -5693,9 +5720,9 @@ class User implements IDBAccessObject, UserIdentity {
 
                if ( $groups ) {
                        return Status::newFatal( 'badaccess-groups', $wgLang->commaList( $groups ), count( $groups ) );
-               } else {
-                       return Status::newFatal( 'badaccess-group0' );
                }
+
+               return Status::newFatal( 'badaccess-group0' );
        }
 
        /**