Page MenuHomePhabricator

Remove "set password" from `bin/accountadmin` and let `bin/auth recover` recover anyone
ClosedPublic

Authored by epriestley on Jan 22 2018, 12:08 AM.
Tags
None
Referenced Files
F14000412: D18901.diff
Thu, Oct 24, 9:31 PM
F13990432: D18901.diff
Tue, Oct 22, 3:36 AM
F13990248: D18901.diff
Tue, Oct 22, 2:13 AM
F13990214: D18901.diff
Tue, Oct 22, 2:01 AM
F13989485: D18901.id45357.diff
Mon, Oct 21, 8:50 PM
F13986631: D18901.id.diff
Mon, Oct 21, 4:26 AM
F13986629: D18901.id45317.diff
Mon, Oct 21, 4:24 AM
F13986617: D18901.id45357.diff
Mon, Oct 21, 4:18 AM
Subscribers

Details

Summary

Ref T13043. This cleans some things up to prepare for moving account passwords to shared infrastructure.

Currently, the (very old, fairly unusual) bin/accountadmin tool can set account passwords. This is a bit weird, generally not great, and makes upgrading to shared infrastructure more difficult. Just get rid of this to simplify things. Many installs don't have passwords and this is pointless and unhelpful in those cases.

Instead, let bin/auth recover recover any account, not just administrator accounts. This was a guardrail against administrative abuse, but it has always seemed especially flimsy (since anyone who can run the tool can easily comment out the checks) and I use this tool in cluster support with some frequency, occasionally just commenting out the checks. This is generally a better solution than actually setting a password on accounts anyway. Just get rid of the check and give users enough rope to shoot themselves in the foot with if they truly desire.

Test Plan
  • Ran bin/accountadmin, didn't get prompted to swap passwords anymore.
  • Ran bin/auth recover to recover a non-admin account.

Diff Detail

Repository
rP Phabricator
Branch
revoke11
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 19104
Build 25789: Run Core Tests
Build 25788: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Jan 23 2018, 6:25 PM
This revision was automatically updated to reflect the committed changes.

Working great here, but it appears that the documentation needs to be updated to include this "top secret" :-) information.

Working great here, but it appears that the documentation needs to be updated to include this "top secret" :-) information.

Thanks for the report. Fixed in D20007.