Page MenuHomePhabricator

No way to force a user to change their password if it's invalid
Open, Needs TriagePublic

Description

So, if a users password is now invalid, and $wgInvalidPasswordReset is set, they're taken to the password reset form. Great. But they can then just click cancel and carry on with their business. Which doesn't help resolve the issue.

Please choose a new password now, or click "Cancel" to reset it later.

Of course, you can then prevent login to those with invalid passwords, and force them to do a password reset via email... Which isn't so nice.

So, we need a way to force users to do a password, and not let them do anything else (but allow them to login and reach the reset form)

Event Timeline

Reedy created this task.Nov 16 2015, 8:13 PM
Reedy raised the priority of this task from to Needs Triage.
Reedy updated the task description. (Show Details)
Reedy added a subscriber: Reedy.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptNov 16 2015, 8:13 PM
Reedy updated the task description. (Show Details)Nov 16 2015, 8:14 PM
Reedy set Security to None.
lcawte added a subscriber: lcawte.Dec 17 2016, 12:59 PM
Anomie added a subscriber: Anomie.EditedSep 1 2017, 5:50 PM

At the code level, it's just a matter of setting 'hard' to true instead of false here (and adjusting the message used) since AuthManager. Deciding when exactly to do that, if the answer isn't "always", is the tricky part.

Reedy added a comment.Sep 1 2017, 5:55 PM

I guess we need to do something with making it configurable in LocalSettings, possibly depending on user group...

There was previous discussion somewhere, where people felt it should more be, you need to change your password in X number of days, since sometime people might be at inconvenient spots to change their password.

Tgr added a subscriber: Tgr.Nov 27 2018, 12:15 AM

There was previous discussion somewhere, where people felt it should more be, you need to change your password in X number of days, since sometime people might be at inconvenient spots to change their password.

Still pretty easy to do, just decide where to configure it (and how to set that configuration in Wikimedia wikis), and then the abstract password provider can set a password expiration timestamp when the user clicks Skip (we already have the mechanism for that, it's just disabled on Wikimedia sites). Plus users with an expired password would have to be logged out, I imagine, which could be done by the same mechanism that checks user_token to validate the session.

There is not much point to it without regularly nagging the user during the grace period though, so that would have to be implemented.

Note that Wikimedia login last for a year (and default MediaWiki logins half a year I think) and even that period is not enforced if the user manually alters cookie expiry. So changes in password policy will take a long time to propagate, that's not something we can help.

Tgr added a comment.Dec 9 2018, 7:34 PM

Logging skips would probably be a good start, we don't seem to be doing that now. (Or are we worried that it might highlight who have vulnerable passwords?)

Tgr added a comment.Dec 21 2018, 1:13 AM

I imagine the minimum viable version would look something like this:

  • Create a new $wgPasswordPolicyGracePeriod config setting that gives you how much time users have to change their password (maybe per user group? per policy check?). Or preferably reuse $wgPasswordExpireGrace which is more or less the same thing but for manually set password end-of-life.
  • If the grace setting needs to be set per policy check, look it up in UserPasswordPolicy and hack it into the status (set the value to [ 'grace' => <time> ] or something); otherwise just check it in the caller (LocalPasswordPrimaryAuthenticationProvider and the CentralAuth equivalent should be the only ones that care).
  • In LocalPasswordPrimaryAuthenticationProvider, when the password check fails with a non-fatal and there is a grace period, set the password expiration time (user_password_expires in core; CentralAuth has no equivalent and needs a schema change, short of some horrible hack to reuse gu_password_reset_expiration with gu_password_reset_key = NULL). If we reuse $wgPasswordExpireGrace, just set it to the current date, if there is a separate config value that's split by group or policy then calculate the end of the expiration period and set it to that.
  • Replicate LocalPasswordPrimaryAuthenticationProvider's expiry check in CentralAuthPrimaryAuthenticationProvider.
  • Add a big warning about the expiration period to the password change dialog.
  • Probably add password expiration time lookup as a User method, add some hook for CentralAuth, and use that method to show a warning on the preferences page and maybe do more aggressive things like showing it on every page when there is very little time left.
  • In LocalPasswordPrimaryAuthenticationProvider::getPasswordResetData and the CentralAuth equivalent, when the expiry check fails hard, do a password validity check and if that fails use it to modify the message so it's clearer to the user why they are required to change passwords.

Some things about this that are not great:

  • Needs to be reimplemented in every password-based auth extension. To some extent that's necessary since the expiration time needs to be stored somewhere, but we could make a standard interface for getting/setting that and then the rest of the logic can probably live in AbstractPasswordPrimaryAuthenticationProvider.
  • If an attacker gets hold of the database, the expiration field tells them which passwords are weak. If that's a big problem (it probably is...) and it's acceptable that we lose the ability to list accounts which are in the grace period, we can encrypt the field value with the password (and set it on login whether the password was invalid or not). At which point it is probably better to not mess with user_password_expires and just make a new column.
  • Configuration changes are not handled great, e.g. if the policy is relaxed after the grace period has started, the user will still be locked out, only with a more confusing message. Probably too fringe an issue to be worth dealing with.

Change 481819 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Add force option to password policy

https://gerrit.wikimedia.org/r/481819

Change 481819 merged by jenkins-bot:
[mediawiki/core@master] Add force option to password policy

https://gerrit.wikimedia.org/r/481819

Tgr added a comment.Jan 7 2019, 6:05 PM

The minimum viable version is merged now, you need to set the policy to [ 'value' => ..., 'forceChange' => true ] and you will not be allowed to skip the password change screen on login. We should also add a grace period but as discussed above that's rather more complex.

Change 481819 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Add force option to password policy
https://gerrit.wikimedia.org/r/481819

We should find a way to expose this on Special:PasswordPolicies

Sub bullet point or something extra in brackets, or a mark/footnote thing - pretty open to suggestions... Basically highlight that if your password doesn't pass this policy, it will enforce a password change so that it does

Change 485482 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Show password policy flags on Special:PasswordPolicies

https://gerrit.wikimedia.org/r/485482

Quennlatifa0303 changed the task status from Open to Stalled.Feb 19 2019, 4:03 AM
Quennlatifa0303 assigned this task to MediaWikiAccount.
Quennlatifa0303 triaged this task as Normal priority.
Quennlatifa0303 set Due Date to Feb 19 2019, 12:00 AM.
Quennlatifa0303 removed a project: Security.
Restricted Application changed the subtype of this task from "Task" to "Deadline". · View Herald TranscriptFeb 19 2019, 4:03 AM
Krinkle changed the task status from Stalled to Open.Feb 19 2019, 4:10 AM
Krinkle removed MediaWikiAccount as the assignee of this task.
Krinkle raised the priority of this task from Normal to Needs Triage.
Krinkle removed Due Date.
Krinkle added a project: Security.
Krinkle added a subscriber: MediaWikiAccount.
Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptFeb 19 2019, 4:10 AM

Change 485482 merged by jenkins-bot:
[mediawiki/core@master] Show password policy flags on Special:PasswordPolicies

https://gerrit.wikimedia.org/r/485482

Tgr added a comment.Mar 16 2019, 1:34 AM

Do we consider this good enough for enabling on Wikimedia wikis and/or core, or are we holding out for a more complex version that includes a grace period?