Password policy checks that fail and have `suggestChangeOnLogin` set to true will
prompt for a password change on login.
Below are some rules that apply to this setting in different scenarios:
- If only one policy fails and has `suggestChangeOnLogin = false`, a password change will
not be requested
- If more than one policy fails and one or more have `suggestChangeOnLogin` set to true`,
a password change will be requested
- If `forceChange` is present in any of the failing policies, `suggestChangeOnLogin` value
will be ignored and password change will be enforced
- if $wgInvalidPasswordReset is set to false `suggestChangeOnLogin` is ignored
IMPORTANT**
Before this patch, suggesting a password change was the default behavior (depending on
$wgInvalidPasswordReset), which means that the necessary changes to $wgPasswordPolicy
need to be in place before this patch is merged and gets to production.
Bug: T211621
Change-Id: I7a4a0a06273fa4e8bd0da3dac54cf5a1b78bb3fd
* - 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
+ * - suggestChangeOnLogin: (bool, default false) if true and the password is
+ * invalid, suggest a password change if logging in. If all the failing policies
+ * that apply to the user have this set to false, the password change
+ * screen will not be shown. 'forceChange' takes precedence over
+ * 'suggestChangeOnLogin' if they are both present.
* 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
'PasswordNotInLargeBlacklist' => true,
],
'default' => [
- 'MinimalPasswordLength' => 1,
- 'PasswordCannotMatchUsername' => true,
- 'PasswordCannotMatchBlacklist' => true,
- 'MaximalPasswordLength' => 4096,
+ 'MinimalPasswordLength' => [ 'value' => 1, 'suggestChangeOnLogin' => true ],
+ 'PasswordCannotMatchUsername' => [ 'value' => true, 'suggestChangeOnLogin' => true ],
+ 'PasswordCannotMatchBlacklist' => [ 'value' => true, 'suggestChangeOnLogin' => true ],
+ 'MaximalPasswordLength' => [ 'value' => 4096, 'suggestChangeOnLogin' => true ],
],
],
'checks' => [
if ( !$reset && $this->config->get( 'InvalidPasswordReset' ) && !$status->isGood() ) {
$hard = $status->getValue()['forceChange'] ?? false;
- $reset = (object)[
- 'msg' => $status->getMessage( $hard ? 'resetpass-validity' : 'resetpass-validity-soft' ),
- 'hard' => $hard,
- ];
+
+ if ( $hard || !empty( $status->getValue()['suggestChangeOnLogin'] ) ) {
+ $reset = (object)[
+ 'msg' => $status->getMessage( $hard ? 'resetpass-validity' : 'resetpass-validity-soft' ),
+ 'hard' => $hard,
+ ];
+ }
}
if ( $reset ) {
$this->loginOnly = !empty( $params['loginOnly'] );
}
+ /**
+ * Check if the password has expired and needs a reset
+ *
+ * @param string $username
+ * @param \stdClass $row A row from the user table
+ * @return \stdClass|null
+ */
protected function getPasswordResetData( $username, $row ) {
$now = wfTimestamp();
$expiration = wfTimestampOrNull( TS_UNIX, $row->user_password_expires );
* 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.
+ * - suggestChangeOnLogin: prompt for a password change on login if the password is invalid.
*/
public function checkUserPassword( User $user, $password ) {
$effectivePolicy = $this->getPoliciesForUser( $user );
* 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.
+ * - suggestChangeOnLogin: prompt for a password change on login if the password is invalid.
*/
public function checkUserPasswordForGroups( User $user, $password, array $groups ) {
$effectivePolicy = self::getPoliciesForGroups(
private function checkPolicies( User $user, $password, $policies, $policyCheckFunctions ) {
$status = Status::newGood( [] );
$forceChange = false;
+ $suggestChangeOnLogin = false;
foreach ( $policies as $policy => $settings ) {
if ( !isset( $policyCheckFunctions[$policy] ) ) {
throw new DomainException( "Invalid password policy config. No check defined for '$policy'." );
$user,
$password
);
- if ( !$policyStatus->isGood() && !empty( $settings['forceChange'] ) ) {
- $forceChange = true;
+
+ if ( !$policyStatus->isGood() ) {
+ if ( !empty( $settings['forceChange'] ) ) {
+ $forceChange = true;
+ }
+
+ if ( !empty( $settings['suggestChangeOnLogin'] ) ) {
+ $suggestChangeOnLogin = true;
+ }
}
$status->merge( $policyStatus );
}
- if ( $status->isOK() && $forceChange ) {
- $status->value['forceChange'] = true;
+
+ if ( $status->isOK() ) {
+ if ( $forceChange ) {
+ $status->value['forceChange'] = true;
+ } elseif ( $suggestChangeOnLogin ) {
+ $status->value['suggestChangeOnLogin'] = true;
+ }
}
+
return $status;
}
* - 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).
+ * - suggestChangeOnLogin (bool): if set to true, the user should be prompted for
+ * a password change on login.
*
* @param string $password Desired password
* @return Status
"passwordpolicies-policy-passwordcannotbepopular": "Password cannot be {{PLURAL:$1|the popular password|in the list of $1 popular passwords}}",
"passwordpolicies-policy-passwordnotinlargeblacklist": "Password cannot be in the list of 100,000 most commonly used passwords.",
"passwordpolicies-policyflag-forcechange": "must change on login",
+ "passwordpolicies-policyflag-suggestchangeonlogin": "suggest change on login",
"easydeflate-invaliddeflate": "Content provided is not properly deflated",
"unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage"
}
"passwordpolicies-policy-passwordcannotbepopular": "Password policy that enforces that a password is not in a list of $1 number of \"popular\" passwords. $1 - number of popular passwords the password will be checked against",
"passwordpolicies-policy-passwordnotinlargeblacklist": "Password policy that enforces that a password is not in a list of 100,000 number of \"popular\" passwords.",
"passwordpolicies-policyflag-forcechange": "Password policy flag that enforces changing invalid passwords on login.",
+ "passwordpolicies-policyflag-suggestchangeonlogin": "Password policy flag that suggests changing invalid passwords on login.",
"easydeflate-invaliddeflate": "Error message if the content passed to easydeflate was not deflated (compressed) properly",
"unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected"
}
$this->assertNull( $manager->getAuthenticationSessionData( 'reset-pass' ) );
$manager->removeAuthenticationSessionData( null );
- $status = \Status::newGood();
+ $status = \Status::newGood( [ 'suggestChangeOnLogin' => true ] );
$status->error( 'testing' );
$providerPriv->setPasswordResetFlag( 'Foo', $status );
$ret = $manager->getAuthenticationSessionData( 'reset-pass' );
$this->manager = new AuthManager( new \FauxRequest(), $config );
}
$this->validity = \Status::newGood();
-
$provider = $this->getMockBuilder( LocalPasswordPrimaryAuthenticationProvider::class )
->setMethods( [ 'checkPasswordValidity' ] )
->setConstructorArgs( [ [ 'loginOnly' => $loginOnly ] ] )
->getMock();
+
$provider->expects( $this->any() )->method( 'checkPasswordValidity' )
->will( $this->returnCallback( function () {
return $this->validity;
$this->manager->removeAuthenticationSessionData( null );
$row->user_password_expires = null;
- $status = \Status::newGood();
+ $status = \Status::newGood( [ 'suggestChangeOnLogin' => 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 );
+
+ $this->manager->removeAuthenticationSessionData( null );
+ $row->user_password_expires = null;
+ $status = \Status::newGood( [ 'suggestChangeOnLogin' => false, ] );
+ $status->error( 'testing' );
+ $providerPriv->setPasswordResetFlag( $userName, $status, $row );
+ $ret = $this->manager->getAuthenticationSessionData( 'reset-pass' );
+ $this->assertNull( $ret );
}
public function testAuthentication() {
// Successful auth with reset
$this->manager->removeAuthenticationSessionData( null );
+ $this->validity = \Status::newGood( [ 'suggestChangeOnLogin' => true ] );
$this->validity->error( 'arbitrary-warning' );
$this->assertEquals(
AuthenticationResponse::newPass( $userName ),
$ret = $provider->beginPrimaryAuthentication( $reqs );
$this->assertEquals( AuthenticationResponse::PASS, $ret->status, 'new password is set' );
}
-
}
'PasswordCannotMatchUsername' => true,
],
'sysop' => [
- 'MinimalPasswordLength' => 8,
+ 'MinimalPasswordLength' => [ 'value' => 8, 'suggestChangeOnLogin' => true ],
'MinimumPasswordLengthToLogin' => 1,
'PasswordCannotMatchUsername' => true,
],
+ 'bureaucrat' => [
+ 'MinimalPasswordLength' => [
+ 'value' => 6,
+ 'suggestChangeOnLogin' => false,
+ 'forceChange' => true,
+ ],
+ 'PasswordCannotMatchUsername' => true,
+ ],
'default' => [
'MinimalPasswordLength' => 4,
'MinimumPasswordLengthToLogin' => 1,
$user = $this->getTestUser( [ 'sysop' ] )->getUser();
$this->assertArrayEquals(
[
- 'MinimalPasswordLength' => 8,
+ 'MinimalPasswordLength' => [ 'value' => 8, 'suggestChangeOnLogin' => true ],
'MinimumPasswordLengthToLogin' => 1,
'PasswordCannotMatchUsername' => true,
'PasswordCannotMatchBlacklist' => true,
$user = $this->getTestUser( [ 'sysop', 'checkuser' ] )->getUser();
$this->assertArrayEquals(
[
- 'MinimalPasswordLength' => [ 'value' => 10, 'forceChange' => true ],
+ 'MinimalPasswordLength' => [
+ 'value' => 10,
+ 'forceChange' => true,
+ 'suggestChangeOnLogin' => true
+ ],
'MinimumPasswordLengthToLogin' => 6,
'PasswordCannotMatchUsername' => true,
'PasswordCannotMatchBlacklist' => true,
public function testGetPoliciesForGroups() {
$effective = UserPasswordPolicy::getPoliciesForGroups(
$this->policies,
- [ 'user', 'checkuser' ],
+ [ 'user', 'checkuser', 'sysop' ],
$this->policies['default']
);
$this->assertArrayEquals(
[
- 'MinimalPasswordLength' => [ 'value' => 10, 'forceChange' => true ],
+ 'MinimalPasswordLength' => [
+ 'value' => 10,
+ 'forceChange' => true,
+ 'suggestChangeOnLogin' => true
+ ],
'MinimumPasswordLengthToLogin' => 6,
'PasswordCannotMatchUsername' => true,
'PasswordCannotMatchBlacklist' => true,
$success = Status::newGood( [] );
$warning = Status::newGood( [] );
$forceChange = Status::newGood( [ 'forceChange' => true ] );
+ $suggestChangeOnLogin = Status::newGood( [ 'suggestChangeOnLogin' => true ] );
$fatal = Status::newGood( [] );
+
// the message does not matter, we only test for state and value
$warning->warning( 'invalid-password' );
$forceChange->warning( 'invalid-password' );
+ $suggestChangeOnLogin->warning( 'invalid-password' );
$warning->warning( 'invalid-password' );
$fatal->fatal( 'invalid-password' );
+
return [
'No groups, default policy, password too short to login' => [
[],
'abcdabcdabcd',
$success,
],
- 'Sysop with short password' => [
+ 'Sysop with short password and suggestChangeOnLogin set to true' => [
[ 'sysop' ],
'abcd',
- $warning,
+ $suggestChangeOnLogin,
],
'Checkuser with short password' => [
- [ 'sysop', 'checkuser' ],
+ [ 'checkuser' ],
'abcdabcd',
$forceChange,
],
+ 'Bureaucrat bad password with forceChange true, suggestChangeOnLogin false' => [
+ [ 'bureaucrat' ],
+ 'short',
+ $forceChange,
+ ],
'Checkuser with too short password to login' => [
[ 'sysop', 'checkuser' ],
'abcd',
],
], // max
],
+ 'complex value in both p1 and p2 #2' => [
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ 'baz' => false,
+ ],
+ ], // p1
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 2,
+ 'bar' => true
+ ],
+ ], // p2
+ [
+ 'MinimalPasswordLength' => [
+ 'value' => 8,
+ 'foo' => 1,
+ 'bar' => true,
+ 'baz' => false,
+ ],
+ ], // max
+ ],
];
}
// This won't actually find all flags, just the ones in use. Can't really be helped,
// other than adding the core flags here.
- $flags = [ 'forceChange' ];
+ $flags = [ 'forceChange', 'suggestChangeOnLogin' ];
foreach ( $wgPasswordPolicy['policies'] as $group => $checks ) {
foreach ( $checks as $check => $settings ) {
if ( is_array( $settings ) ) {
- $flags = array_merge( $flags, array_diff( $settings, [ 'value' ] ) );
+ $flags = array_unique(
+ array_merge( $flags, array_diff( array_keys( $settings ), [ 'value' ] ) )
+ );
}
}
}
+
foreach ( $flags as $flag ) {
yield [ $flag ];
}