Skip to content

Commit aa3b582

Browse files
author
epriestley
committed
Remove "set password" from bin/accountadmin and let bin/auth recover recover anyone
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. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13043 Differential Revision: https://secure.phabricator.com/D18901
1 parent 5a8a56f commit aa3b582

File tree

2 files changed

+4
-51
lines changed

2 files changed

+4
-51
lines changed

scripts/user/account_admin.php

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,6 @@
112112
$create_email = $email;
113113
}
114114

115-
$changed_pass = false;
116-
// This disables local echo, so the user's password is not shown as they type
117-
// it.
118-
phutil_passthru('stty -echo');
119-
$password = phutil_console_prompt(
120-
pht('Enter a password for this user [blank to leave unchanged]:'));
121-
phutil_passthru('stty echo');
122-
if (strlen($password)) {
123-
$changed_pass = $password;
124-
}
125-
126115
$is_system_agent = $user->getIsSystemAgent();
127116
$set_system_agent = phutil_console_confirm(
128117
pht('Is this user a bot?'),
@@ -158,10 +147,6 @@
158147
if ($is_new) {
159148
printf($tpl, pht('Email'), '', $create_email);
160149
}
161-
printf($tpl, pht('Password'), null,
162-
($changed_pass !== false)
163-
? pht('Updated')
164-
: pht('Unchanged'));
165150

166151
printf(
167152
$tpl,
@@ -218,11 +203,6 @@
218203
$editor->makeAdminUser($user, $set_admin);
219204
$editor->makeSystemAgentUser($user, $set_system_agent);
220205

221-
if ($changed_pass !== false) {
222-
$envelope = new PhutilOpaqueEnvelope($changed_pass);
223-
$editor->changePassword($user, $envelope);
224-
}
225-
226206
$user->saveTransaction();
227207

228208
echo pht('Saved changes.')."\n";

src/applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ protected function didConstruct() {
99
->setExamples('**recover** __username__')
1010
->setSynopsis(
1111
pht(
12-
'Recover access to an administrative account if you have locked '.
13-
'yourself out of Phabricator.'))
12+
'Recover access to an account if you have locked yourself out '.
13+
'of Phabricator.'))
1414
->setArguments(
1515
array(
1616
'username' => array(
@@ -21,23 +21,6 @@ protected function didConstruct() {
2121
}
2222

2323
public function execute(PhutilArgumentParser $args) {
24-
25-
$can_recover = id(new PhabricatorPeopleQuery())
26-
->setViewer($this->getViewer())
27-
->withIsAdmin(true)
28-
->execute();
29-
if (!$can_recover) {
30-
throw new PhutilArgumentUsageException(
31-
pht(
32-
'This Phabricator installation has no recoverable administrator '.
33-
'accounts. You can use `%s` to create a new administrator '.
34-
'account or make an existing user an administrator.',
35-
'bin/accountadmin'));
36-
}
37-
$can_recover = mpull($can_recover, 'getUsername');
38-
sort($can_recover);
39-
$can_recover = implode(', ', $can_recover);
40-
4124
$usernames = $args->getArg('username');
4225
if (!$usernames) {
4326
throw new PhutilArgumentUsageException(
@@ -57,18 +40,8 @@ public function execute(PhutilArgumentParser $args) {
5740
if (!$user) {
5841
throw new PhutilArgumentUsageException(
5942
pht(
60-
'No such user "%s". Recoverable administrator accounts are: %s.',
61-
$username,
62-
$can_recover));
63-
}
64-
65-
if (!$user->getIsAdmin()) {
66-
throw new PhutilArgumentUsageException(
67-
pht(
68-
'You can only recover administrator accounts, but %s is not an '.
69-
'administrator. Recoverable administrator accounts are: %s.',
70-
$username,
71-
$can_recover));
43+
'No such user "%s" to recover.',
44+
$username));
7245
}
7346

7447
if (!$user->canEstablishWebSessions()) {

0 commit comments

Comments
 (0)