Page MenuHomePhabricator

Consolidate password verification/revocation logic in a new PhabricatorAuthPasswordEngine
ClosedPublic

Authored by epriestley on Jan 21 2018, 3:27 AM.

Details

Summary

Ref T13043. This provides a new piece of shared infrastructure that VCS passwords and account passwords can use to validate passwords that users enter.

This isn't reachable by anything yet.

The test coverage of the "upgrade" flow (where we rehash a password to use a stronger hasher) isn't great in this diff, I'll expand that in the next change and then start migrating things.

Test Plan

Added a bunch of unit tests.

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 21 2018, 3:27 AM
epriestley requested review of this revision.Jan 21 2018, 3:28 AM

Most of this logic is just adapted from the VCS password flow rather than new code, and I have a diff on top of this for converting that flow to use the new Engine. Once that makes it off my machine it may make reviewing this one a little easier.

src/applications/auth/xaction/PhabricatorAuthPasswordUpgradeTransaction.php
10

I'm not entirely sure this actually works so it may change in the next diff. This transaction is a little unusual.

amckinley requested changes to this revision.Jan 23 2018, 2:49 AM
amckinley added inline comments.
src/applications/auth/__tests__/PhabricatorAuthPasswordTestCase.php
87

Redundant check/copy paste error?

This revision now requires changes to proceed.Jan 23 2018, 2:49 AM
epriestley updated this revision to Diff 45343.Jan 23 2018, 4:11 AM
  • Fix copy/paste in test case.
amckinley accepted this revision.Jan 23 2018, 6:20 PM
This revision is now accepted and ready to land.Jan 23 2018, 6:20 PM
This revision was automatically updated to reflect the committed changes.