mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 08:42:41 +01:00
Mask typed passwords as they are entered into 'accountadmin'
Summary: Currently, we echo the password as the user types it. This turns out to be a bit of an issue in over-the-shoulder installs. Instead, disable tty echo while the user is typing their password so nothing is shown (like how 'sudo' works). Also show a better error message if the user chooses a duplicate email; without testing for this we just throw a duplicate key exception when saving, which isn't easy to understand. The other duplicate key exception is duplicate username, which is impossible (the script updates rather than creating in this case). There's currently a bug where creating a user and setting their password at the same time doesn't work. This is because we hash the PHID into the password hash, but it's empty if the user hasn't been persisted yet. Make sure the user is persisted before setting their password. Finally, fix an issue where $original would have the new username set, creating a somewhat confusing summary at the end. I'm also going to improve the password behavior/explanation here once I add welcome emails ("Hi Joe, epriestley created an account for you on Phabricator, click here to login..."). Test Plan: - Typed a password and didn't have it echoed. I also tested this on Ubuntu without encountering problems. - Chose a duplicate email, got a useful error message instead of the exception I'd encountered earlier. - Created a new user with a password in one pass and logged in as that user, this worked properly. - Verified summary table does not contain username for new users. Reviewed By: jungejason Reviewers: jungejason, tuomaspelkonen, aran CC: moskov, jr, aran, jungejason Differential Revision: 358
This commit is contained in:
parent
fff08a9894
commit
bc71888249
2 changed files with 43 additions and 8 deletions
|
@ -22,6 +22,7 @@ require_once $root.'/scripts/__init_script__.php';
|
||||||
require_once $root.'/scripts/__init_env__.php';
|
require_once $root.'/scripts/__init_env__.php';
|
||||||
|
|
||||||
phutil_require_module('phutil', 'console');
|
phutil_require_module('phutil', 'console');
|
||||||
|
phutil_require_module('phutil', 'future/exec');
|
||||||
|
|
||||||
echo "Enter a username to create a new account or edit an existing account.";
|
echo "Enter a username to create a new account or edit an existing account.";
|
||||||
|
|
||||||
|
@ -36,6 +37,8 @@ $user = id(new PhabricatorUser())->loadOneWhere(
|
||||||
$username);
|
$username);
|
||||||
|
|
||||||
if (!$user) {
|
if (!$user) {
|
||||||
|
$original = new PhabricatorUser();
|
||||||
|
|
||||||
echo "There is no existing user account '{$username}'.\n";
|
echo "There is no existing user account '{$username}'.\n";
|
||||||
$ok = phutil_console_confirm(
|
$ok = phutil_console_confirm(
|
||||||
"Do you want to create a new '{$username}' account?",
|
"Do you want to create a new '{$username}' account?",
|
||||||
|
@ -47,6 +50,8 @@ if (!$user) {
|
||||||
$user = new PhabricatorUser();
|
$user = new PhabricatorUser();
|
||||||
$user->setUsername($username);
|
$user->setUsername($username);
|
||||||
} else {
|
} else {
|
||||||
|
$original = clone $user;
|
||||||
|
|
||||||
echo "There is an existing user account '{$username}'.\n";
|
echo "There is an existing user account '{$username}'.\n";
|
||||||
$ok = phutil_console_confirm(
|
$ok = phutil_console_confirm(
|
||||||
"Do you want to edit the existing '{$username}' account?",
|
"Do you want to edit the existing '{$username}' account?",
|
||||||
|
@ -57,8 +62,6 @@ if (!$user) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$original = clone $user;
|
|
||||||
|
|
||||||
$user_realname = $user->getRealName();
|
$user_realname = $user->getRealName();
|
||||||
if (strlen($user_realname)) {
|
if (strlen($user_realname)) {
|
||||||
$realname_prompt = ' ['.$user_realname.']';
|
$realname_prompt = ' ['.$user_realname.']';
|
||||||
|
@ -76,17 +79,34 @@ if (strlen($user_email)) {
|
||||||
} else {
|
} else {
|
||||||
$email_prompt = '';
|
$email_prompt = '';
|
||||||
}
|
}
|
||||||
$email = nonempty(
|
|
||||||
|
do {
|
||||||
|
$email = nonempty(
|
||||||
phutil_console_prompt("Enter user email address{$email_prompt}:"),
|
phutil_console_prompt("Enter user email address{$email_prompt}:"),
|
||||||
$user_email);
|
$user_email);
|
||||||
|
$duplicate = id(new PhabricatorUser())->loadOneWhere(
|
||||||
|
'email = %s',
|
||||||
|
$email);
|
||||||
|
if ($duplicate && $duplicate->getID() != $user->getID()) {
|
||||||
|
$duplicate_username = $duplicate->getUsername();
|
||||||
|
echo "ERROR: There is already a user with that email address ".
|
||||||
|
"({$duplicate_username}). Each user must have a unique email ".
|
||||||
|
"address.\n";
|
||||||
|
} else {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
} while (true);
|
||||||
$user->setEmail($email);
|
$user->setEmail($email);
|
||||||
|
|
||||||
$changed_pass = false;
|
$changed_pass = false;
|
||||||
|
// This disables local echo, so the user's password is not shown as they type
|
||||||
|
// it.
|
||||||
|
phutil_passthru('stty -echo');
|
||||||
$password = phutil_console_prompt(
|
$password = phutil_console_prompt(
|
||||||
"Enter a password for this user [blank to leave unchanged]:");
|
"Enter a password for this user [blank to leave unchanged]:");
|
||||||
|
phutil_passthru('stty echo');
|
||||||
if (strlen($password)) {
|
if (strlen($password)) {
|
||||||
$user->setPassword($password);
|
$changed_pass = $password;
|
||||||
$changed_pass = true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
$is_admin = $user->getIsAdmin();
|
$is_admin = $user->getIsAdmin();
|
||||||
|
@ -101,7 +121,10 @@ printf($tpl, null, 'OLD VALUE', 'NEW VALUE');
|
||||||
printf($tpl, 'Username', $original->getUsername(), $user->getUsername());
|
printf($tpl, 'Username', $original->getUsername(), $user->getUsername());
|
||||||
printf($tpl, 'Real Name', $original->getRealName(), $user->getRealName());
|
printf($tpl, 'Real Name', $original->getRealName(), $user->getRealName());
|
||||||
printf($tpl, 'Email', $original->getEmail(), $user->getEmail());
|
printf($tpl, 'Email', $original->getEmail(), $user->getEmail());
|
||||||
printf($tpl, 'Password', null, $changed_pass ? 'Updated' : 'Unchanged');
|
printf($tpl, 'Password', null,
|
||||||
|
($changed_pass !== false)
|
||||||
|
? 'Updated'
|
||||||
|
: 'Unchanged');
|
||||||
|
|
||||||
printf(
|
printf(
|
||||||
$tpl,
|
$tpl,
|
||||||
|
@ -117,5 +140,11 @@ if (!phutil_console_confirm("Save these changes?", $default_no = false)) {
|
||||||
}
|
}
|
||||||
|
|
||||||
$user->save();
|
$user->save();
|
||||||
|
if ($changed_pass !== false) {
|
||||||
|
// This must happen after saving the user because we use their PHID as a
|
||||||
|
// component of the password hash.
|
||||||
|
$user->setPassword($changed_pass);
|
||||||
|
$user->save();
|
||||||
|
}
|
||||||
|
|
||||||
echo "Saved changes.\n";
|
echo "Saved changes.\n";
|
||||||
|
|
|
@ -58,6 +58,12 @@ class PhabricatorUser extends PhabricatorUserDAO {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function setPassword($password) {
|
public function setPassword($password) {
|
||||||
|
if (!$this->getPHID()) {
|
||||||
|
throw new Exception(
|
||||||
|
"You can not set a password for an unsaved user because their PHID ".
|
||||||
|
"is a salt component in the password hash.");
|
||||||
|
}
|
||||||
|
|
||||||
if (!strlen($password)) {
|
if (!strlen($password)) {
|
||||||
$this->setPasswordHash('');
|
$this->setPasswordHash('');
|
||||||
} else {
|
} else {
|
||||||
|
|
Loading…
Reference in a new issue