Page MenuHomePhabricator

Remove one-time login from username change email
ClosedPublic

Authored by epriestley on Feb 5 2019, 7:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 6:03 PM
Unknown Object (File)
Mon, Dec 9, 7:18 AM
Unknown Object (File)
Fri, Dec 6, 5:11 PM
Unknown Object (File)
Thu, Dec 5, 3:44 PM
Unknown Object (File)
Thu, Dec 5, 2:16 AM
Unknown Object (File)
Tue, Dec 3, 2:09 PM
Unknown Object (File)
Tue, Dec 3, 5:09 AM
Unknown Object (File)
Sun, Dec 1, 5:35 AM
Subscribers
None

Details

Summary

Depends on D20100. Ref T7732. Ref T13244. This is a bit of an adventure.

Long ago, passwords were digested with usernames as part of the salt. This was a mistake: it meant that your password becomes invalid if your username is changed.

(I think very very long ago, some other hashing may also have used usernames -- perhaps session hashing or CSRF hashing?)

To work around this, the "username change" email included a one-time login link and some language about resetting your password.

This flaw was fixed when passwords were moved to shared infrastructure (they're now salted more cleanly on a per-digest basis), and since D18908 (about a year ago) we've transparently upgraded password digests on use.

Although it's still technically possible that a username change could invalidate your password, it requires:

  • You set the password on a version of Phabricator earlier than ~2018 Week 5 (about a year ago).
  • You haven't logged into a version of Phabricator newer than that using your password since then.
  • Your username is changed.

This probably affects more than zero users, but I suspect not many more than zero. These users can always use "Forgot password?" to recover account access.

Since the value of this is almost certainly very near zero now and declining over time, just get rid of it. Also move the actual mail out of PhabricatorUser, ala the similar recent change to welcome mail in D19989.

Test Plan

Changed a user's username, reviewed resulting mail with bin/mail show-outbound.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable