Differential D18896

Consolidate password verification/revocation logic in a new PhabricatorAuthPasswordEngine
ClosedPublic
Actions

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

Tags

None

Referenced Files

	F13045891: D18896.id.diff
	Thu, Apr 18, 4:20 AM

	Unknown Object (File)
	Sun, Apr 14, 5:08 PM

	Unknown Object (File)
	Tue, Apr 2, 4:46 PM

	Unknown Object (File)
	Sun, Mar 31, 7:52 PM

	Unknown Object (File)
	Feb 23 2024, 7:18 PM

	Unknown Object (File)
	Feb 18 2024, 2:04 PM

	Unknown Object (File)
	Jan 18 2024, 3:10 AM

	Unknown Object (File)
	Dec 22 2023, 8:50 PM

View All 58 Files

Subscribers

None

Details

Reviewers

Maniphest Tasks

T13043: Improve authentication revocation behaviors

Commits

rPc280c85772a3: Consolidate password verification/revocation logic in a new…

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Jan 21 2018, 3:27 AM

Harbormaster completed remote builds in B19098: Diff 45311.Jan 21 2018, 3:28 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

Good catch!

Fix copy/paste in test case.

Harbormaster completed remote builds in B19130: Diff 45343.Jan 23 2018, 4:12 AM

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

Closed by commit rPc280c85772a3: Consolidate password verification/revocation logic in a new… (authored by epriestley). · Explain WhyJan 23 2018, 6:54 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

__phutil_library_map__.php

6 lines

applications/

auth/

__tests__/

PhabricatorAuthPasswordTestCase.php

92 lines

engine/

PhabricatorAuthPasswordEngine.php

240 lines

storage/

PhabricatorAuthPassword.php

42 lines

PhabricatorAuthPasswordTransaction.php

2 lines

xaction/

PhabricatorAuthPasswordUpgradeTransaction.php

24 lines

Diff 45352

src/__phutil_library_map__.php

Loading...

src/applications/auth/tests/PhabricatorAuthPasswordTestCase.php

Loading...

src/applications/auth/engine/PhabricatorAuthPasswordEngine.php

Loading...

src/applications/auth/storage/PhabricatorAuthPassword.php

Loading...

src/applications/auth/storage/PhabricatorAuthPasswordTransaction.php

Loading...

src/applications/auth/xaction/PhabricatorAuthPasswordUpgradeTransaction.php

Loading...