From cc0b74b01a2c154445a11f502094d32ed2af579a Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 29 Aug 2012 11:07:29 -0700 Subject: [PATCH] bin/accountadmin - allow creation of system accounts and create workflow for system accounts that are in trouble Summary: the former is self explanatory. the latter is necessary for installations that require email verification. since many system agents are given bogus email address there can become a problem where these accounts can't be verified Test Plan: created system agent account from scratch. edited user and toggled system agent accountness. created system agent with unverified email address and verified it. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T1656 Differential Revision: https://secure.phabricator.com/D3401 --- scripts/user/account_admin.php | 55 ++++++++++++++++--- .../people/PhabricatorUserEditor.php | 46 +++++++++++++++- .../PhabricatorPeopleEditController.php | 14 +++-- .../people/storage/PhabricatorUserLog.php | 1 + 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/scripts/user/account_admin.php b/scripts/user/account_admin.php index d0ae23287b..2b7053777a 100755 --- a/scripts/user/account_admin.php +++ b/scripts/user/account_admin.php @@ -85,7 +85,7 @@ $user->setRealName($realname); // a reasonable CLI interface for editing multiple addresses and managing email // verification and primary addresses. -$new_email = null; +$create_email = null; if ($is_new) { do { $email = phutil_console_prompt("Enter user email address:"); @@ -100,7 +100,7 @@ if ($is_new) { } } while (true); - $new_email = $email; + $create_email = $email; } $changed_pass = false; @@ -114,6 +114,29 @@ if (strlen($password)) { $changed_pass = $password; } +$is_system_agent = $user->getIsSystemAgent(); +$set_system_agent = phutil_console_confirm( + 'Should this user be a system agent?', + $default_no = !$is_system_agent); + +$verify_email = null; +$set_verified = false; +// Allow administrators to verify primary email addresses at this time in edit +// scenarios. (Create will work just fine from here as we auto-verify email +// on create.) +if (!$is_new) { + $verify_email = $user->loadPrimaryEmail(); + if (!$verify_email->getIsVerified()) { + $set_verified = phutil_console_confirm( + 'Should the primary email address be verified?', + $default_no = true + ); + } else { + // already verified so let's not make a fuss + $verify_email = null; + } +} + $is_admin = $user->getIsAdmin(); $set_admin = phutil_console_confirm( 'Should this user be an administrator?', @@ -124,14 +147,28 @@ $tpl = "%12s %-30s %-30s\n"; printf($tpl, null, 'OLD VALUE', 'NEW VALUE'); printf($tpl, 'Username', $original->getUsername(), $user->getUsername()); printf($tpl, 'Real Name', $original->getRealName(), $user->getRealName()); -if ($new_email) { - printf($tpl, 'Email', '', $new_email); +if ($is_new) { + printf($tpl, 'Email', '', $create_email); } printf($tpl, 'Password', null, ($changed_pass !== false) ? 'Updated' : 'Unchanged'); +printf( + $tpl, + 'System Agent', + $original->getIsSystemAgent() ? 'Y' : 'N', + $set_system_agent ? 'Y' : 'N'); + +if ($verify_email) { + printf( + $tpl, + 'Verify Email', + $verify_email->getIsVerified() ? 'Y' : 'N', + $set_verified ? 'Y' : 'N'); +} + printf( $tpl, 'Admin', @@ -153,17 +190,21 @@ $user->openTransaction(); // this script to create the first user. $editor->setActor($user); - if ($new_email) { + if ($is_new) { $email = id(new PhabricatorUserEmail()) - ->setAddress($new_email) + ->setAddress($create_email) ->setIsVerified(1); $editor->createNewUser($user, $email); } else { - $editor->updateUser($user); + if ($verify_email) { + $verify_email->setIsVerified($set_verified ? 1 : 0); + } + $editor->updateUser($user, $verify_email); } $editor->makeAdminUser($user, $set_admin); + $editor->makeSystemAgentUser($user, $set_system_agent); if ($changed_pass !== false) { $envelope = new PhutilOpaqueEnvelope($changed_pass); diff --git a/src/applications/people/PhabricatorUserEditor.php b/src/applications/people/PhabricatorUserEditor.php index 1ad29c0919..5e6fd70544 100644 --- a/src/applications/people/PhabricatorUserEditor.php +++ b/src/applications/people/PhabricatorUserEditor.php @@ -103,7 +103,9 @@ final class PhabricatorUserEditor { /** * @task edit */ - public function updateUser(PhabricatorUser $user) { + public function updateUser( + PhabricatorUser $user, + PhabricatorUserEmail $email = null) { if (!$user->getID()) { throw new Exception("User has not been created yet!"); } @@ -111,6 +113,9 @@ final class PhabricatorUserEditor { $actor = $this->requireActor(); $user->openTransaction(); $user->save(); + if ($email) { + $email->save(); + } $log = PhabricatorUserLog::newLog( $actor, @@ -235,6 +240,45 @@ final class PhabricatorUserEditor { return $this; } + /** + * @task role + */ + public function makeSystemAgentUser(PhabricatorUser $user, $system_agent) { + $actor = $this->requireActor(); + + if (!$user->getID()) { + throw new Exception("User has not been created yet!"); + } + + $user->openTransaction(); + $user->beginWriteLocking(); + + $user->reload(); + if ($user->getIsSystemAgent() == $system_agent) { + $user->endWriteLocking(); + $user->killTransaction(); + return $this; + } + + $log = PhabricatorUserLog::newLog( + $actor, + $user, + PhabricatorUserLog::ACTION_SYSTEM_AGENT); + $log->setOldValue($user->getIsSystemAgent()); + $log->setNewValue($system_agent); + + $user->setIsSystemAgent($system_agent); + $user->save(); + + $log->save(); + + $user->endWriteLocking(); + $user->saveTransaction(); + + return $this; + } + + /** * @task role */ diff --git a/src/applications/people/controller/PhabricatorPeopleEditController.php b/src/applications/people/controller/PhabricatorPeopleEditController.php index f27d3ee6de..b15bb157c3 100644 --- a/src/applications/people/controller/PhabricatorPeopleEditController.php +++ b/src/applications/people/controller/PhabricatorPeopleEditController.php @@ -132,8 +132,9 @@ final class PhabricatorPeopleEditController $request = $this->getRequest(); if ($request->isFormPost()) { $welcome_checked = $request->getInt('welcome'); + $is_new = !$user->getID(); - if (!$user->getID()) { + if ($is_new) { $user->setUsername($request->getStr('username')); $new_email = $request->getStr('email'); @@ -147,9 +148,6 @@ final class PhabricatorPeopleEditController $e_email = null; } - if ($request->getStr('role') == 'agent') { - $user->setIsSystemAgent(true); - } } $user->setRealName($request->getStr('realname')); @@ -172,7 +170,6 @@ final class PhabricatorPeopleEditController if (!$errors) { try { - $is_new = !$user->getID(); if (!$is_new) { id(new PhabricatorUserEditor()) @@ -186,6 +183,13 @@ final class PhabricatorPeopleEditController id(new PhabricatorUserEditor()) ->setActor($admin) ->createNewUser($user, $email); + + if ($request->getStr('role') == 'agent') { + id(new PhabricatorUserEditor()) + ->setActor($admin) + ->makeSystemAgentUser($user, true); + } + } if ($welcome_checked) { diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php index 7e8a7df5ee..238165de7c 100644 --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -27,6 +27,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO { const ACTION_EDIT = 'edit'; const ACTION_ADMIN = 'admin'; + const ACTION_SYSTEM_AGENT = 'system-agent'; const ACTION_DISABLE = 'disable'; const ACTION_DELETE = 'delete';