Page MenuHomePhabricator

Move account passwords to shared infrastructure
ClosedPublic

Authored by epriestley on Jan 22 2018, 1:12 AM.

Details

Summary

Ref T13043. This moves user account passwords to the new shared infrastructure.

There's a lot of code changes here, but essentially all of it is the same as the VCS password logic in D18898.

Test Plan
  • Ran migration.
  • Spot checked table for general sanity.
  • Logged in with an existing password.
  • Hit all error conditions on "change password", "set password", "register new account" flows.
  • Verified that changing password logs out other sessions.
  • Verified that revoked passwords of a different type can't be selected.
  • Changed passwords a bunch.
  • Verified that salt regenerates properly after password change.
  • Tried to login with the wrong password, which didn't work.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Jan 22 2018, 1:12 AM
epriestley requested review of this revision.Jan 22 2018, 1:13 AM
epriestley edited the test plan for this revision. (Show Details)Jan 22 2018, 1:14 AM
epriestley added inline comments.
resources/sql/autopatches/20180120.auth.03.vcsdata.sql
4–6

There's a little bit of juggling here to make sure that the migration runs okay if the unique key on phid gets added sooner than we expect. This just gives each new password a unique value, then the followup migrations overwrite them with real PHIDs.

src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php
73–76

It turns out this comment is out of date; CSRF tokens haven't depended on any secret shared with passwords in a long time.

amckinley accepted this revision.Jan 23 2018, 7:47 PM
This revision is now accepted and ready to land.Jan 23 2018, 7:47 PM
This revision was automatically updated to reflect the committed changes.