From 6a69a4eb733bd6129533c93b7a1ca4564b16ac32 Mon Sep 17 00:00:00 2001 From: csteipp Date: Tue, 14 Jul 2015 12:26:46 -0700 Subject: [PATCH] Add "purpose" to password validity check Allow callers to specify why they are checking a passwords validity, so some checks can be modified. Only check the default policy on creation, since the account doesn't exist it's not a member of any groups. Bug: T104615 Change-Id: I56b66002562aaa1493d94a90309bc8e4ae3841c8 --- docs/hooks.txt | 2 ++ includes/User.php | 5 +++-- includes/installer/WebInstallerPage.php | 12 ++---------- includes/password/UserPasswordPolicy.php | 23 ++++++++++++++--------- includes/specials/SpecialUserlogin.php | 7 +++++-- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/docs/hooks.txt b/docs/hooks.txt index 23df983240..2fd815e6e4 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -2313,6 +2313,8 @@ that tests continue to run properly. 'PasswordPoliciesForUser': Alter the effective password policy for a user. $user: User object whose policy you are modifying &$effectivePolicy: Array of policy statements that apply to this user +$purpose: string indicating purpose of the check, one of 'login', 'create', + or 'reset' 'PerformRetroactiveAutoblock': Called before a retroactive autoblock is applied to a user. diff --git a/includes/User.php b/includes/User.php index 95ac0f1ed1..7057af654b 100644 --- a/includes/User.php +++ b/includes/User.php @@ -838,10 +838,11 @@ class User implements IDBAccessObject { * able to set their password to this. * * @param string $password Desired password + * @param string $purpose one of 'login', 'create', 'reset' * @return Status * @since 1.23 */ - public function checkPasswordValidity( $password ) { + public function checkPasswordValidity( $password, $purpose = 'login' ) { global $wgPasswordPolicy; $upp = new UserPasswordPolicy( @@ -858,7 +859,7 @@ class User implements IDBAccessObject { } if ( $result === false ) { - $status->merge( $upp->checkUserPassword( $this, $password ) ); + $status->merge( $upp->checkUserPassword( $this, $password, $purpose ) ); return $status; } elseif ( $result === true ) { return $status; diff --git a/includes/installer/WebInstallerPage.php b/includes/installer/WebInstallerPage.php index 9aa6960cc8..f7910ba5de 100644 --- a/includes/installer/WebInstallerPage.php +++ b/includes/installer/WebInstallerPage.php @@ -911,16 +911,8 @@ class WebInstallerName extends WebInstallerPage { $pwd = $this->getVar( '_AdminPassword' ); $user = User::newFromName( $cname ); if ( $user ) { - $upp = new UserPasswordPolicy( - $wgPasswordPolicy['policies'], - $wgPasswordPolicy['checks'] - ); - $status = $upp->checkUserPasswordForGroups( - $user, - $pwd, - array( 'sysop', 'bureaucrat' ) - ); - $valid = $status->isGood(); + $status = $user->checkPasswordValidity( $pwd, 'create' ); + $valid = $status->isGood() ? true : $status->getMessage()->escaped(); } else { $valid = 'config-admin-name-invalid'; } diff --git a/includes/password/UserPasswordPolicy.php b/includes/password/UserPasswordPolicy.php index 70757acb79..80dc66920d 100644 --- a/includes/password/UserPasswordPolicy.php +++ b/includes/password/UserPasswordPolicy.php @@ -67,11 +67,12 @@ class UserPasswordPolicy { * Check if a passwords meets the effective password policy for a User. * @param User $user who's policy we are checking * @param string $password the password to check + * @param string $purpose one of 'login', 'create', 'reset' * @return Status error to indicate the password didn't meet the policy, or fatal to * indicate the user shouldn't be allowed to login. */ - public function checkUserPassword( User $user, $password ) { - $effectivePolicy = $this->getPoliciesForUser( $user ); + public function checkUserPassword( User $user, $password, $purpose = 'login' ) { + $effectivePolicy = $this->getPoliciesForUser( $user, $purpose ); return $this->checkPolicies( $user, $password, @@ -126,16 +127,20 @@ class UserPasswordPolicy { * Get the policy for a user, based on their group membership. Public so * UI elements can access and inform the user. * @param User $user + * @param string $purpose one of 'login', 'create', 'reset' * @return array the effective policy for $user */ - public function getPoliciesForUser( User $user ) { - $effectivePolicy = self::getPoliciesForGroups( - $this->policies, - $user->getEffectiveGroups(), - $this->policies['default'] - ); + public function getPoliciesForUser( User $user, $purpose = 'login' ) { + $effectivePolicy = $this->policies['default']; + if ( $purpose !== 'create' ) { + $effectivePolicy = self::getPoliciesForGroups( + $this->policies, + $user->getEffectiveGroups(), + $this->policies['default'] + ); + } - Hooks::run( 'PasswordPoliciesForUser', array( $user, &$effectivePolicy ) ); + Hooks::run( 'PasswordPoliciesForUser', array( $user, &$effectivePolicy, $purpose ) ); return $effectivePolicy; } diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index aa5141514e..4ca07fe4af 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -545,7 +545,7 @@ class LoginForm extends SpecialPage { } # check for password validity, return a fatal Status if invalid - $validity = $u->checkPasswordValidity( $this->mPassword ); + $validity = $u->checkPasswordValidity( $this->mPassword, 'create' ); if ( !$validity->isGood() ) { $validity->ok = false; // make sure this Status is fatal return $validity; @@ -948,7 +948,10 @@ class LoginForm extends SpecialPage { } elseif ( $wgInvalidPasswordReset && !$user->isValidPassword( $this->mPassword ) ) { - $status = $user->checkPasswordValidity( $this->mPassword ); + $status = $user->checkPasswordValidity( + $this->mPassword, + 'login' + ); $this->resetLoginForm( $status->getMessage( 'resetpass-validity-soft' ) ); -- 2.20.1