* Password policy for the wiki.
* Structured as
* [
- * 'policies' => [ <group> => [ <policy> => <value>, ... ], ... ],
+ * 'policies' => [ <group> => [ <policy> => <settings>, ... ], ... ],
* '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.
+ * PHP callable implementing the policy check, <settings> is an array
+ * of options with the following keys:
+ * - value: (number, boolean or null) the value to pass to the callback
+ * - forceChange: (bool, default false) if the password is invalid, do
+ * not let the user log in without changing the password
+ * As a shorthand for [ 'value' => <value> ], simply <value> can be written.
+ * When multiple password policies are defined for a user, the settings
+ * arrays are merged, and for fields which are set in both arrays, the
+ * larger value (as understood by PHP's 'max' method) is taken.
*
* 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
$reset = $this->getPasswordResetData( $username, $data );
if ( !$reset && $this->config->get( 'InvalidPasswordReset' ) && !$status->isGood() ) {
+ $hard = $status->getValue()['forceChange'] ?? false;
$reset = (object)[
- 'msg' => $status->getMessage( 'resetpass-validity-soft' ),
- 'hard' => false,
+ 'msg' => $status->getMessage( $hard ? 'resetpass-validity' : 'resetpass-validity-soft' ),
+ 'hard' => $hard,
];
}
/**
* @param array $policies
* @param array $checks mapping statement to its checking function. Checking functions are
- * called with the policy value for this user, the user object, and the password to check.
+ * called with the policy value for this user, the user object, and the password to check.
*/
public function __construct( array $policies, array $checks ) {
if ( !isset( $policies['default'] ) ) {
* @param User $user who's policy we are checking
* @param string $password the password to check
* @return Status error to indicate the password didn't meet the policy, or fatal to
- * indicate the user shouldn't be allowed to login.
+ * indicate the user shouldn't be allowed to login. The status value will be an array,
+ * potentially with the following keys:
+ * - forceChange: do not allow the user to login without changing the password if invalid.
*/
public function checkUserPassword( User $user, $password ) {
$effectivePolicy = $this->getPoliciesForUser( $user );
* @param string $password the password to check
* @param array $groups list of groups to which we assume the user belongs
* @return Status error to indicate the password didn't meet the policy, or fatal to
- * indicate the user shouldn't be allowed to login.
+ * indicate the user shouldn't be allowed to login. The status value will be an array,
+ * potentially with the following keys:
+ * - forceChange: do not allow the user to login without changing the password if invalid.
*/
public function checkUserPasswordForGroups( User $user, $password, array $groups ) {
$effectivePolicy = self::getPoliciesForGroups(
* @return Status
*/
private function checkPolicies( User $user, $password, $policies, $policyCheckFunctions ) {
- $status = Status::newGood();
- foreach ( $policies as $policy => $value ) {
+ $status = Status::newGood( [] );
+ $forceChange = false;
+ foreach ( $policies as $policy => $settings ) {
if ( !isset( $policyCheckFunctions[$policy] ) ) {
throw new DomainException( "Invalid password policy config. No check defined for '$policy'." );
}
- $status->merge(
- call_user_func(
- $policyCheckFunctions[$policy],
- $value,
- $user,
- $password
- )
+ if ( !is_array( $settings ) ) {
+ // legacy format
+ $settings = [ 'value' => $settings ];
+ }
+ if ( !array_key_exists( 'value', $settings ) ) {
+ throw new DomainException( "Invalid password policy config. No value defined for '$policy'." );
+ }
+ $value = $settings['value'];
+ /** @var StatusValue $policyStatus */
+ $policyStatus = call_user_func(
+ $policyCheckFunctions[$policy],
+ $value,
+ $user,
+ $password
);
+ if ( !$policyStatus->isGood() && !empty( $settings['forceChange'] ) ) {
+ $forceChange = true;
+ }
+ $status->merge( $policyStatus );
+ }
+ if ( $status->isOK() && $forceChange ) {
+ $status->value['forceChange'] = true;
}
return $status;
}
/**
* Utility function to get a policy that is the most restrictive of $p1 and $p2. For
* simplicity, we setup the policy values so the maximum value is always more restrictive.
+ * It is also used recursively to merge settings within the same policy.
* @param array $p1
* @param array $p2
* @return array containing the more restrictive values of $p1 and $p2
$ret[$key] = $p2[$key];
} elseif ( !isset( $p2[$key] ) ) {
$ret[$key] = $p1[$key];
- } else {
+ } elseif ( !is_array( $p1[$key] ) && !is_array( $p2[$key] ) ) {
$ret[$key] = max( $p1[$key], $p2[$key] );
+ } else {
+ if ( !is_array( $p1[$key] ) ) {
+ $p1[$key] = [ 'value' => $p1[$key] ];
+ } elseif ( !is_array( $p2[$key] ) ) {
+ $p2[$key] = [ 'value' => $p2[$key] ];
+ }
+ $ret[$key] = self::maxOfPolicies( $p1[$key], $p2[$key] );
}
}
return $ret;
/**
* 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
$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 ] ) ) {
}
if ( $result === false ) {
- $status->merge( $upp->checkUserPassword( $this, $password ) );
+ $status->merge( $upp->checkUserPassword( $this, $password ), true );
return $status;
} elseif ( $result === true ) {
return $status;
"resetpass-abort-generic": "Password change has been aborted by an extension.",
"resetpass-expired": "Your password has expired. Please set a new password to log in.",
"resetpass-expired-soft": "Your password has expired and needs to be changed. Please choose a new password now, or click \"{{int:authprovider-resetpass-skip-label}}\" to change it later.",
+ "resetpass-validity": "Your password is not valid: $1\n\nPlease set a new password to log in.",
"resetpass-validity-soft": "Your password is not valid: $1\n\nPlease choose a new password now, or click \"{{int:authprovider-resetpass-skip-label}}\" to change it later.",
"passwordreset": "Reset password",
"passwordreset-text-one": "Complete this form to receive a temporary password via email.",
"resetpass-abort-generic": "Generic error message shown on [[Special:ChangePassword]] when an extension aborts a password change from a hook.",
"resetpass-expired": "Generic error message shown on [[Special:ChangePassword]] when a user's password is expired",
"resetpass-expired-soft": "Generic warning message shown on [[Special:ChangePassword]] when a user needs to reset their password, but they are not prevented from logging in at this time",
+ "resetpass-validity": "Warning message shown on [[Special:ChangePassword]] when a user needs to reset their password, because their password is not valid.\n\nParameters:\n* $1 - error message",
"resetpass-validity-soft": "Warning message shown on [[Special:ChangePassword]] when a user needs to reset their password, because their password is not valid.\n\nRefers to {{msg-mw|authprovider-resetpass-skip-label}}.\n\nParameters:\n* $1 - error message",
"passwordreset": "Title of [[Special:PasswordReset]].\n{{Identical|Reset password}}",
"passwordreset-text-one": "Text on [[Special:PasswordReset]] that appears when there is only one way of resetting the password.\n\n{{msg-mw|Passwordreset-text-many}} will be used, when there are multiple ways of resetting the password.",
public function testCheckPasswordValidity() {
$uppCalled = 0;
- $uppStatus = \Status::newGood();
+ $uppStatus = \Status::newGood( [] );
$this->setMwGlobals( [
'wgPasswordPolicy' => [
'policies' => [
$this->assertNotNull( $ret );
$this->assertSame( 'resetpass-validity-soft', $ret->msg->getKey() );
$this->assertFalse( $ret->hard );
+
+ $this->manager->removeAuthenticationSessionData( null );
+ $row->user_password_expires = null;
+ $status = \Status::newGood( [ 'forceChange' => true ] );
+ $status->error( 'testing' );
+ $providerPriv->setPasswordResetFlag( $userName, $status, $row );
+ $ret = $this->manager->getAuthenticationSessionData( 'reset-pass' );
+ $this->assertNotNull( $ret );
+ $this->assertSame( 'resetpass-validity', $ret->msg->getKey() );
+ $this->assertTrue( $ret->hard );
}
public function testAuthentication() {
protected $policies = [
'checkuser' => [
- 'MinimalPasswordLength' => 10,
+ 'MinimalPasswordLength' => [ 'value' => 10, 'forceChange' => true ],
'MinimumPasswordLengthToLogin' => 6,
'PasswordCannotMatchUsername' => true,
],
'MinimumPasswordLengthToLogin' => 1,
'PasswordCannotMatchBlacklist' => true,
'MaximalPasswordLength' => 4096,
+ // test null handling
+ 'PasswordCannotMatchUsername' => null,
],
];
public function testGetPoliciesForUser() {
$upp = $this->getUserPasswordPolicy();
- $user = User::newFromName( 'TestUserPolicy' );
- $user->addToDatabase();
- $user->addGroup( 'sysop' );
-
+ $user = $this->getTestUser( [ 'sysop' ] )->getUser();
$this->assertArrayEquals(
[
'MinimalPasswordLength' => 8,
'MinimumPasswordLengthToLogin' => 1,
- 'PasswordCannotMatchUsername' => 1,
+ 'PasswordCannotMatchUsername' => true,
+ 'PasswordCannotMatchBlacklist' => true,
+ 'MaximalPasswordLength' => 4096,
+ ],
+ $upp->getPoliciesForUser( $user )
+ );
+
+ $user = $this->getTestUser( [ 'sysop', 'checkuser' ] )->getUser();
+ $this->assertArrayEquals(
+ [
+ 'MinimalPasswordLength' => [ 'value' => 10, 'forceChange' => true ],
+ 'MinimumPasswordLengthToLogin' => 6,
+ 'PasswordCannotMatchUsername' => true,
'PasswordCannotMatchBlacklist' => true,
'MaximalPasswordLength' => 4096,
],
$this->assertArrayEquals(
[
- 'MinimalPasswordLength' => 10,
+ 'MinimalPasswordLength' => [ 'value' => 10, 'forceChange' => true ],
'MinimumPasswordLengthToLogin' => 6,
'PasswordCannotMatchUsername' => true,
'PasswordCannotMatchBlacklist' => true,
/**
* @dataProvider provideCheckUserPassword
*/
- public function testCheckUserPassword( $username, $groups, $password, $valid, $ok, $msg ) {
+ public function testCheckUserPassword( $groups, $password, StatusValue $expectedStatus ) {
$upp = $this->getUserPasswordPolicy();
-
- $user = User::newFromName( $username );
- $user->addToDatabase();
- foreach ( $groups as $group ) {
- $user->addGroup( $group );
- }
+ $user = $this->getTestUser( $groups )->getUser();
$status = $upp->checkUserPassword( $user, $password );
- $this->assertSame( $valid, $status->isGood(), $msg . ' - password valid' );
- $this->assertSame( $ok, $status->isOK(), $msg . ' - can login' );
+ $this->assertSame( $expectedStatus->isGood(), $status->isGood(), 'password valid' );
+ $this->assertSame( $expectedStatus->isOK(), $status->isOK(), 'can login' );
+ $this->assertSame( $expectedStatus->getValue(), $status->getValue(), 'flags' );
}
public function provideCheckUserPassword() {
+ $success = Status::newGood( [] );
+ $warning = Status::newGood( [] );
+ $forceChange = Status::newGood( [ 'forceChange' => true ] );
+ $fatal = Status::newGood( [] );
+ // the message does not matter, we only test for state and value
+ $warning->warning( 'invalid-password' );
+ $forceChange->warning( 'invalid-password' );
+ $warning->warning( 'invalid-password' );
+ $fatal->fatal( 'invalid-password' );
return [
- [
- 'PassPolicyUser',
+ 'No groups, default policy, password too short to login' => [
[],
'',
- false,
- false,
- 'No groups, default policy, password too short to login'
+ $fatal,
],
- [
- 'PassPolicyUser',
+ 'Default policy, short password' => [
[ 'user' ],
'aaa',
- false,
- true,
- 'Default policy, short password'
+ $warning,
],
- [
- 'PassPolicyUser',
+ 'Sysop with good password' => [
[ 'sysop' ],
'abcdabcdabcd',
- true,
- true,
- 'Sysop with good password'
+ $success,
],
- [
- 'PassPolicyUser',
+ 'Sysop with short password' => [
[ 'sysop' ],
'abcd',
- false,
- true,
- 'Sysop with short password'
+ $warning,
],
- [
- 'PassPolicyUser',
+ 'Checkuser with short password' => [
[ 'sysop', 'checkuser' ],
'abcdabcd',
- false,
- true,
- 'Checkuser with short password'
+ $forceChange,
],
- [
- 'PassPolicyUser',
+ 'Checkuser with too short password to login' => [
[ 'sysop', 'checkuser' ],
'abcd',
- false,
- false,
- 'Checkuser with too short password to login'
- ],
- [
- 'Useruser',
- [ 'user' ],
- 'Passpass',
- false,
- true,
- 'Username & password on blacklist'
+ $fatal,
],
];
}
+ public function testCheckUserPassword_blacklist() {
+ $upp = $this->getUserPasswordPolicy();
+ $user = User::newFromName( 'Useruser' );
+ $user->addToDatabase();
+
+ $status = $upp->checkUserPassword( $user, 'Passpass' );
+ $this->assertFalse( $status->isGood(), 'password invalid' );
+ $this->assertTrue( $status->isOK(), 'can login' );
+ }
+
/**
* @dataProvider provideMaxOfPolicies
*/
- public function testMaxOfPolicies( $p1, $p2, $max, $msg ) {
+ public function testMaxOfPolicies( $p1, $p2, $max ) {
$this->assertArrayEquals(
$max,
- UserPasswordPolicy::maxOfPolicies( $p1, $p2 ),
- $msg
+ UserPasswordPolicy::maxOfPolicies( $p1, $p2 )
);
}
public function provideMaxOfPolicies() {
return [
- [
+ 'Basic max in p1' => [
[ 'MinimalPasswordLength' => 8 ], // p1
[ 'MinimalPasswordLength' => 2 ], // p2
[ 'MinimalPasswordLength' => 8 ], // max
- 'Basic max in p1'
],
- [
+ 'Basic max in p2' => [
[ 'MinimalPasswordLength' => 2 ], // p1
[ 'MinimalPasswordLength' => 8 ], // p2
[ 'MinimalPasswordLength' => 8 ], // max
- 'Basic max in p2'
],
- [
- [ 'MinimalPasswordLength' => 8 ], // p1
+ 'Missing items in p1' => [
+ [
+ 'MinimalPasswordLength' => 8,
+ ], // p1
[
'MinimalPasswordLength' => 2,
'PasswordCannotMatchUsername' => 1,
'MinimalPasswordLength' => 8,
'PasswordCannotMatchUsername' => 1,
], // max
- 'Missing items in p1'
],
- [
+ 'Missing items in p2' => [
[
'MinimalPasswordLength' => 8,
'PasswordCannotMatchUsername' => 1,
'MinimalPasswordLength' => 8,
'PasswordCannotMatchUsername' => 1,
], // max
- 'Missing items in p2'
+ ],
+ 'complex value in p1' => [
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ ],
+ ], // p1
+ [
+ 'MinimalPasswordLength' => 2,
+ ], // p2
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ ],
+ ], // max
+ ],
+ 'complex value in p2' => [
+ [
+ 'MinimalPasswordLength' => 8,
+ ], // p1
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 2,
+ 'foo' => 1,
+ ],
+ ], // p2
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ ],
+ ], // max
+ ],
+ 'complex value in both p1 and p2' => [
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ 'baz' => false,
+ ],
+ ], // p1
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 2,
+ 'bar' => 2,
+ 'baz' => true,
+ ],
+ ], // p2
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ 'bar' => 2,
+ 'baz' => true,
+ ],
+ ], // max
],
];
}
dépôts / lhc / web / wiklou.git / commitdiff