Deprecate User::getPasswordValidity()
authorGergő Tisza <tgr.huwiki@gmail.com>
Thu, 20 Dec 2018 22:44:04 +0000 (14:44 -0800)
committerGergő Tisza <tgr.huwiki@gmail.com>
Fri, 21 Dec 2018 04:26:51 +0000 (20:26 -0800)
Unused, the return format does not seem useful.

Also improve the documentation of $wgPasswordPolicy
and PasswordPolicyChecks.

Change-Id: Ic01e80cfefc4cfb0eee1eccc6a66942f692278a0

RELEASE-NOTES-1.33
includes/DefaultSettings.php
includes/password/PasswordPolicyChecks.php
includes/user/User.php
tests/phpunit/includes/user/UserTest.php

index 5b6f6b2..7567252 100644 (file)
@@ -189,6 +189,8 @@ because of Phabricator reports.
   domain ID as a key component and use makeGlobalKey().
 * (T202094) Title::getUserCaseDBKey() is deprecated; instead, please use
   Title::getDBKey(), which doesn't vary case.
+* User::getPasswordValidity() is now deprecated. User::checkPasswordValidity()
+  returns the same information in a more useful format.
 * …
 
 === Other changes in 1.33 ===
index 2f1efbf..c76b8c1 100644 (file)
@@ -4451,29 +4451,49 @@ $wgCentralIdLookupProviders = [
 $wgCentralIdLookupProvider = 'local';
 
 /**
- * Password policy for local wiki users. A user's effective policy
- * is the superset of all policy statements from the policies for the
- * groups where the user is a member. If more than one group policy
- * include the same policy statement, the value is the max() of the
- * values. Note true > false. The 'default' policy group is required,
- * and serves as the minimum policy for all users. New statements can
- * be added by appending to $wgPasswordPolicy['checks'].
- * Statements:
- *     - MinimalPasswordLength - minimum length a user can set
- *     - MinimumPasswordLengthToLogin - passwords shorter than this will
+ * Password policy for the wiki.
+ * Structured as
+ * [
+ *     'policies' => [ <group> => [ <policy> => <value>, ... ], ... ],
+ *     'checks' => [ <policy> => <callback>, ... ],
+ * ]
+ * where <group> is a user group, <policy> is a password policy name
+ * (arbitrary string) defined in the 'checks' part, <callback> is the
+ * PHP callable implementing the policy check, <value> is a number,
+ * boolean or null that gets passed to the callback.
+ *
+ * A user's effective policy is the superset of all policy statements
+ * from the policies for the groups where the user is a member. If more
+ * than one group policy include the same policy statement, the value is
+ * the max() of the values. Note true > false. The 'default' policy group
+ * is required, and serves as the minimum policy for all users.
+ *
+ * Callbacks receive three arguments: the policy value, the User object
+ * and the password; and must return a StatusValue. A non-good status
+ * means the password will not be accepted for new accounts, and existing
+ * accounts will be prompted for password change or barred from logging in
+ * (depending on whether the status is a fatal or merely error/warning).
+ *
+ * The checks supported by core are:
+ *     - MinimalPasswordLength - Minimum length a user can set.
+ *     - MinimumPasswordLengthToLogin - Passwords shorter than this will
  *             not be allowed to login, regardless if it is correct.
  *     - MaximalPasswordLength - maximum length password a user is allowed
  *             to attempt. Prevents DoS attacks with pbkdf2.
- *     - PasswordCannotMatchUsername - Password cannot match username to
+ *     - PasswordCannotMatchUsername - Password cannot match the username.
  *     - PasswordCannotMatchBlacklist - Username/password combination cannot
- *             match a specific, hardcoded blacklist.
+ *             match a blacklist of default passwords used by MediaWiki in the past.
  *     - PasswordCannotBePopular - Blacklist passwords which are known to be
  *             commonly chosen. Set to integer n to ban the top n passwords.
  *             If you want to ban all common passwords on file, use the
  *             PHP_INT_MAX constant.
  *     - PasswordNotInLargeBlacklist - Password not in best practices list of
- *             100,000 commonly used passwords.
+ *             100,000 commonly used passwords. Due to the size of the list this
+ *      is a probabilistic test.
+ *
  * @since 1.26
+ * @see PasswordPolicyChecks
+ * @see User::checkPasswordValidity()
  */
 $wgPasswordPolicy = [
        'policies' => [
index 3c56535..81b8a0d 100644 (file)
@@ -25,13 +25,20 @@ use MediaWiki\MediaWikiServices;
 use Wikimedia\PasswordBlacklist;
 
 /**
- * Functions to check passwords against a policy requirement
+ * Functions to check passwords against a policy requirement.
+ *
+ * $policyVal is the value configured in $wgPasswordPolicy. If the return status is fatal,
+ * the user won't be allowed to login. If the status is not good but not fatal, the user
+ * will not be allowed to set the given password (on registration or password change),
+ * but can still log in after bypassing a warning.
+ *
  * @since 1.26
+ * @see $wgPasswordPolicy
  */
 class PasswordPolicyChecks {
 
        /**
-        * Check password is longer than minimum, not fatal
+        * Check password is longer than minimum, not fatal.
         * @param int $policyVal minimal length
         * @param User $user
         * @param string $password
@@ -46,7 +53,7 @@ class PasswordPolicyChecks {
        }
 
        /**
-        * Check password is longer than minimum, fatal
+        * Check password is longer than minimum, fatal.
         * @param int $policyVal minimal length
         * @param User $user
         * @param string $password
@@ -61,7 +68,8 @@ class PasswordPolicyChecks {
        }
 
        /**
-        * Check password is shorter than maximum, fatal
+        * Check password is shorter than maximum, fatal.
+        * Intended for preventing DoS attacks when using a more expensive password hash like PBKDF2.
         * @param int $policyVal maximum length
         * @param User $user
         * @param string $password
@@ -76,7 +84,7 @@ class PasswordPolicyChecks {
        }
 
        /**
-        * Check if username and password match
+        * Check if username and password are a (case-insensitive) match.
         * @param bool $policyVal true to force compliance.
         * @param User $user
         * @param string $password
@@ -95,7 +103,7 @@ class PasswordPolicyChecks {
        }
 
        /**
-        * Check if username and password are on a blacklist
+        * Check if username and password are on a blacklist of past MediaWiki default passwords.
         * @param bool $policyVal true to force compliance.
         * @param User $user
         * @param string $password
@@ -126,7 +134,8 @@ class PasswordPolicyChecks {
        }
 
        /**
-        * Ensure that password isn't in top X most popular passwords
+        * Ensure that password isn't in top X most popular passwords, as defined by
+        * $wgPopularPasswordFile.
         *
         * @param int $policyVal Cut off to use. Will automatically shrink to the max
         *   supported for error messages if set to more than max number of passwords on file,
@@ -135,6 +144,7 @@ class PasswordPolicyChecks {
         * @param string $password
         * @since 1.27
         * @return Status
+        * @see $wgPopularPasswordFile
         */
        public static function checkPopularPasswordBlacklist( $policyVal, User $user, $password ) {
                global $wgPopularPasswordFile, $wgSitename;
@@ -173,7 +183,9 @@ class PasswordPolicyChecks {
 
        /**
         * Ensure the password isn't in the list of passwords blacklisted by the
-        * wikimedia/password-blacklist library
+        * wikimedia/password-blacklist library, which contains (as of 0.1.4) the
+        * 100.000 top passwords from SecLists (as a Bloom filter, with an
+        * 0.000001 false positive ratio).
         *
         * @param bool $policyVal Whether to apply this policy
         * @param User $user
index 1412215..ec75f05 100644 (file)
@@ -1144,8 +1144,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,8 +1153,11 @@ 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;
index 4f8a7da..50a9e50 100644 (file)
@@ -392,6 +392,7 @@ class UserTest extends MediaWikiTestCase {
                                ],
                        ],
                ] );
+               $this->hideDeprecated( 'User::getPasswordValidity' );
 
                $user = static::getTestUser()->getUser();