Page MenuHomePhabricator

Rethink AuthManager::securitySensitiveOperationStatus
Open, MediumPublic


Some actions like changing the password or the email address require elevated security. The old approach was to plug in a password field into whatever form was responsible for the action, and then maybe provide a hook for extensions which provide extra authentication security (such as two-factor). This assumed that the user has a password (not necessarily true with AuthManager) and would have led to a profileration of hooks if more actions start using elevated security, so AuthManager tried a different approach.

That approach consists of calling AuthManager::securitySensitiveOperationStatus(<operation>) (there are various helpers to do this such as SpecialPage::getLoginSecurityLevel) which would return "ok" or "denied" or "needs reauthentication", based on site configuration. (Currently that configuration specifies max acceptable time since last login.) If it returns "ok", the user is allowed to do the action, without any extra password field.

This system could use more consideration. (Per @csteipp: For security, I'd like to talk about the change password form more, and maybe do a followup at some point. I don't like that a session hijacking can lead to password loss, even if the time is limited.

Event Timeline

Some options:

  • add a new action type and a new begin/continueXXX method to AuthManager and change the providers to support it. This seems like the only way to preserve the old UX intact, but would overcomplicate things horribly (both in code and in UX, especially with multi-step submission) and it would be hard to guarantee that the subset of providers that gets involved is a sane choice. (E.g. a "Login with Facebook" provider should probably not add its own button because anyone hijacking the Wikipedia session will likely hijack the Facebook session as well, so it would moot the point of preventing password change after a session hijack. On the other hand, what if that is the only provider on the wiki? That would lock users out from all security-requiring actions.)
  • embed the full login form (with the possible exception of pre-authentication providers) and use the normal login flow to verify the user. Has the same problems as the previous bullet point.
  • per Anomie: The only other way to do it that I can think of would be to have the user input their new password, then send them to re-login every time. Which would be complicated by needing to do the same thing via the API where we can't just set 'returnto' and 'returntoquery' to get them back to the special page. Ie. have the user do the action, stash it, go to the login page, have the user authenticate, on success redirect back and perform the original action. Also very complex and confusing, especially for complex actions (e.g. CheckUser where the user makes lots of requests and all of those are security-sensitive but sending them through a login for each step would be madness).

None of that seems really feasible.

chasemp triaged this task as Medium priority.Dec 9 2019, 5:22 PM