Paths

Table of Contentst

Differential D18903

Move account passwords to shared infrastructure
ClosedPublic
Actions

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

Tags

None

Referenced Files

	Unknown Object (File)
	Sun, Feb 9, 4:59 AM

	Unknown Object (File)
	Sun, Feb 9, 4:59 AM

	Unknown Object (File)
	Sun, Feb 9, 4:59 AM

	Unknown Object (File)
	Sun, Feb 9, 4:59 AM

	Unknown Object (File)
	Mon, Feb 3, 11:13 PM

	Unknown Object (File)
	Tue, Jan 28, 3:02 AM

	Unknown Object (File)
	Tue, Jan 28, 3:02 AM

	Unknown Object (File)
	Tue, Jan 28, 3:02 AM

Subscribers

None

Details

Reviewers

Maniphest Tasks

T13043: Improve authentication revocation behaviors

Commits

rPabc030fa008b: Move account passwords to shared infrastructure

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

Branch

revoke13

Lint

Lint Passed

Severity	Location	Code	Message
Advice	src/applications/people/storage/PhabricatorUser.php:1624	XHP16	TODO Comment

Unit

Tests Passed

Build Status

Buildable 19106
Build 25793: Run Core Tests
Build 25792: arc lint + arc unit

Event Timeline

epriestley created this revision.Jan 22 2018, 1:12 AM

Harbormaster completed remote builds in B19106: Diff 45319.Jan 22 2018, 1:13 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–5	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.

epriestley mentioned this in D18904: Remove "passwordHash" and "passwordSalt" from User objects.Jan 22 2018, 1:20 AM

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

Closed by commit rPabc030fa008b: Move account passwords to shared infrastructure (authored by epriestley). · Explain WhyJan 23 2018, 9:43 PM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in rPcab2bba6f2f7: Remove "passwordHash" and "passwordSalt" from User objects.

Revision Contents
Changeset List

Path

Size

resources/

sql/

autopatches/

20180120.auth.03.vcsdata.sql

3 lines

20180120.auth.04.vcsphid.php

4 lines

20180121.auth.03.accountdata.sql

7 lines

	20180121.auth.04.accountphid.php
	20180120.auth.04.vcsphid.php

8 lines

src/

applications/

auth/

controller/

PhabricatorAuthRegisterController.php

47 lines

PhabricatorAuthSetPasswordController.php

82 lines

engine/

PhabricatorAuthPasswordEngine.php

33 lines

provider/

PhabricatorPasswordAuthProvider.php

23 lines

storage/

PhabricatorAuthPassword.php

2 lines

people/

editor/

PhabricatorUserEditor.php

27 lines

storage/

PhabricatorUser.php

78 lines

settings/

panel/

PhabricatorPasswordSettingsPanel.php

76 lines

Diff 45319

resources/sql/autopatches/20180120.auth.03.vcsdata.sql

Loading...

resources/sql/autopatches/20180120.auth.04.vcsphid.php

Loading...

resources/sql/autopatches/20180121.auth.03.accountdata.sql

Loading...

resources/sql/autopatches/20180121.auth.04.accountphid.php

Loading...

src/applications/auth/controller/PhabricatorAuthRegisterController.php

Loading...

src/applications/auth/controller/PhabricatorAuthSetPasswordController.php

Loading...

src/applications/auth/engine/PhabricatorAuthPasswordEngine.php

Loading...

src/applications/auth/provider/PhabricatorPasswordAuthProvider.php

Loading...

src/applications/auth/storage/PhabricatorAuthPassword.php

Loading...

src/applications/people/editor/PhabricatorUserEditor.php

Loading...

src/applications/people/storage/PhabricatorUser.php

Loading...

src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php

Loading...