From 6f90fbdef806db579dfea6b02bffd8bf3abf49a1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Feb 2015 06:06:09 -0800 Subject: [PATCH] Send emails for email invites Summary: Ref T7152. Ref T3554. - When an administrator clicks "send invites", queue tasks to send the invites. - Then, actually send the invites. - Make the links in the invites work properly. - Also provide `bin/worker execute` to make debugging one-off workers like this easier. - Clean up some UI, too. Test Plan: We now get as far as the exception which is a placeholder for a registration workflow. {F291213} {F291214} {F291215} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T3554, T7152 Differential Revision: https://secure.phabricator.com/D11736 --- src/__phutil_library_map__.php | 4 ++ .../auth/data/PhabricatorAuthInviteAction.php | 18 ++++++ .../engine/PhabricatorAuthInviteEngine.php | 7 ++- .../auth/query/PhabricatorAuthInviteQuery.php | 1 + .../PhabricatorAuthInviteSearchEngine.php | 16 ++++- .../auth/storage/PhabricatorAuthInvite.php | 10 ++- .../worker/PhabricatorAuthInviteWorker.php | 60 ++++++++++++++++++ .../PhabricatorPeopleInviteSendController.php | 43 +++++++++++-- ...ricatorWorkerManagementExecuteWorkflow.php | 61 +++++++++++++++++++ 9 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 src/applications/auth/worker/PhabricatorAuthInviteWorker.php create mode 100644 src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index faf1dfddf2..325887d1cc 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1361,6 +1361,7 @@ phutil_register_library_map(array( 'PhabricatorAuthInviteSearchEngine' => 'applications/auth/query/PhabricatorAuthInviteSearchEngine.php', 'PhabricatorAuthInviteTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthInviteTestCase.php', 'PhabricatorAuthInviteVerifyException' => 'applications/auth/exception/PhabricatorAuthInviteVerifyException.php', + 'PhabricatorAuthInviteWorker' => 'applications/auth/worker/PhabricatorAuthInviteWorker.php', 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', 'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php', 'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php', @@ -2626,6 +2627,7 @@ phutil_register_library_map(array( 'PhabricatorWorkerDAO' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php', 'PhabricatorWorkerLeaseQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php', 'PhabricatorWorkerManagementCancelWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php', + 'PhabricatorWorkerManagementExecuteWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php', 'PhabricatorWorkerManagementFloodWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementFloodWorkflow.php', 'PhabricatorWorkerManagementFreeWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementFreeWorkflow.php', 'PhabricatorWorkerManagementRetryWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php', @@ -4591,6 +4593,7 @@ phutil_register_library_map(array( 'PhabricatorAuthInviteSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorAuthInviteTestCase' => 'PhabricatorTestCase', 'PhabricatorAuthInviteVerifyException' => 'PhabricatorAuthInviteDialogException', + 'PhabricatorAuthInviteWorker' => 'PhabricatorWorker', 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController', @@ -5959,6 +5962,7 @@ phutil_register_library_map(array( 'PhabricatorWorkerDAO' => 'PhabricatorLiskDAO', 'PhabricatorWorkerLeaseQuery' => 'PhabricatorQuery', 'PhabricatorWorkerManagementCancelWorkflow' => 'PhabricatorWorkerManagementWorkflow', + 'PhabricatorWorkerManagementExecuteWorkflow' => 'PhabricatorWorkerManagementWorkflow', 'PhabricatorWorkerManagementFloodWorkflow' => 'PhabricatorWorkerManagementWorkflow', 'PhabricatorWorkerManagementFreeWorkflow' => 'PhabricatorWorkerManagementWorkflow', 'PhabricatorWorkerManagementRetryWorkflow' => 'PhabricatorWorkerManagementWorkflow', diff --git a/src/applications/auth/data/PhabricatorAuthInviteAction.php b/src/applications/auth/data/PhabricatorAuthInviteAction.php index 0fe9dd1dd1..10b625642a 100644 --- a/src/applications/auth/data/PhabricatorAuthInviteAction.php +++ b/src/applications/auth/data/PhabricatorAuthInviteAction.php @@ -189,4 +189,22 @@ final class PhabricatorAuthInviteAction extends Phobject { return $results; } + public function sendInvite(PhabricatorUser $actor, $template) { + if (!$this->willSend()) { + throw new Exception(pht('Invite action is not a send action!')); + } + + if (!preg_match('/{\$INVITE_URI}/', $template)) { + throw new Exception(pht('Invite template does not include invite URI!')); + } + + PhabricatorWorker::scheduleTask( + 'PhabricatorAuthInviteWorker', + array( + 'address' => $this->getEmailAddress(), + 'template' => $template, + 'authorPHID' => $actor->getPHID(), + )); + } + } diff --git a/src/applications/auth/engine/PhabricatorAuthInviteEngine.php b/src/applications/auth/engine/PhabricatorAuthInviteEngine.php index 341d9aa502..2745bb51e0 100644 --- a/src/applications/auth/engine/PhabricatorAuthInviteEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthInviteEngine.php @@ -35,9 +35,10 @@ final class PhabricatorAuthInviteEngine extends Phobject { public function processInviteCode($code) { $viewer = $this->getViewer(); - $invite = id(new PhabricatorAuthInvite())->loadOneWhere( - 'verificationHash = %s', - PhabricatorHash::digestForIndex($code)); + $invite = id(new PhabricatorAuthInviteQuery()) + ->setViewer($viewer) + ->withVerificationCodes(array($code)) + ->executeOne(); if (!$invite) { throw id(new PhabricatorAuthInviteInvalidException( pht('Bad Invite Code'), diff --git a/src/applications/auth/query/PhabricatorAuthInviteQuery.php b/src/applications/auth/query/PhabricatorAuthInviteQuery.php index 28232fb140..f4d86a4885 100644 --- a/src/applications/auth/query/PhabricatorAuthInviteQuery.php +++ b/src/applications/auth/query/PhabricatorAuthInviteQuery.php @@ -88,6 +88,7 @@ final class PhabricatorAuthInviteQuery foreach ($this->verificationCodes as $code) { $hashes[] = PhabricatorHash::digestForIndex($code); } + $where[] = qsprintf( $conn_r, 'verificationHash IN (%Ls)', diff --git a/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php b/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php index 8a01f598bc..af0cc9790c 100644 --- a/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php +++ b/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php @@ -86,7 +86,21 @@ final class PhabricatorAuthInviteSearchEngine ); } - $table = new AphrontTableView($rows); + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Email Address'), + pht('Sent By'), + pht('Accepted By'), + pht('Invited'), + )) + ->setColumnClasses( + array( + '', + '', + 'wide', + 'right', + )); return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Email Invitations')) diff --git a/src/applications/auth/storage/PhabricatorAuthInvite.php b/src/applications/auth/storage/PhabricatorAuthInvite.php index 85b6477280..f1b6d7ffec 100644 --- a/src/applications/auth/storage/PhabricatorAuthInvite.php +++ b/src/applications/auth/storage/PhabricatorAuthInvite.php @@ -38,15 +38,21 @@ final class PhabricatorAuthInvite PhabricatorAuthInvitePHIDType::TYPECONST); } + public function regenerateVerificationCode() { + $this->verificationCode = Filesystem::readRandomCharacters(16); + $this->verificationHash = null; + return $this; + } + public function getVerificationCode() { - if (!$this->getVerificationHash()) { + if (!$this->verificationCode) { if ($this->verificationHash) { throw new Exception( pht( 'Verification code can not be regenerated after an invite is '. 'created.')); } - $this->verificationCode = Filesystem::readRandomCharacters(16); + $this->regenerateVerificationCode(); } return $this->verificationCode; } diff --git a/src/applications/auth/worker/PhabricatorAuthInviteWorker.php b/src/applications/auth/worker/PhabricatorAuthInviteWorker.php new file mode 100644 index 0000000000..7e9e29be50 --- /dev/null +++ b/src/applications/auth/worker/PhabricatorAuthInviteWorker.php @@ -0,0 +1,60 @@ +getTaskData(); + $viewer = PhabricatorUser::getOmnipotentUser(); + + $address = idx($data, 'address'); + $author_phid = idx($data, 'authorPHID'); + + $author = id(new PhabricatorPeopleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($author_phid)) + ->executeOne(); + if (!$author) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Invite has invalid author PHID ("%s").', $author_phid)); + } + + $invite = id(new PhabricatorAuthInviteQuery()) + ->setViewer($viewer) + ->withEmailAddresses(array($address)) + ->executeOne(); + if ($invite) { + // If we're inviting a user who has already been invited, we just + // regenerate their invite code. + $invite->regenerateVerificationCode(); + } else { + // Otherwise, we're creating a new invite. + $invite = id(new PhabricatorAuthInvite()) + ->setEmailAddress($address); + } + + // Whether this is a new invite or not, tag this most recent author as + // the invite author. + $invite->setAuthorPHID($author_phid); + + $code = $invite->getVerificationCode(); + $invite_uri = '/auth/invite/'.$code.'/'; + $invite_uri = PhabricatorEnv::getProductionURI($invite_uri); + + $template = idx($data, 'template'); + $template = str_replace('{$INVITE_URI}', $invite_uri, $template); + + $invite->save(); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addRawTos(array($invite->getEmailAddress())) + ->setForceDelivery(true) + ->setSubject( + pht( + '[Phabricator] %s has invited you to join Phabricator', + $author->getFullName())) + ->setBody($template) + ->saveAndSend(); + } + +} diff --git a/src/applications/people/controller/PhabricatorPeopleInviteSendController.php b/src/applications/people/controller/PhabricatorPeopleInviteSendController.php index 6debefb1c0..5add6ec07d 100644 --- a/src/applications/people/controller/PhabricatorPeopleInviteSendController.php +++ b/src/applications/people/controller/PhabricatorPeopleInviteSendController.php @@ -44,9 +44,8 @@ final class PhabricatorPeopleInviteSendController $any_valid = false; $all_valid = true; - $action_send = PhabricatorAuthInviteAction::ACTION_SEND; foreach ($actions as $action) { - if ($action->getAction() == $action_send) { + if ($action->willSend()) { $any_valid = true; } else { $all_valid = false; @@ -72,8 +71,44 @@ final class PhabricatorPeopleInviteSendController } if ($any_valid && $request->getBool('confirm')) { - throw new Exception( - pht('TODO: This workflow is not yet fully implemented.')); + + // TODO: The copywriting on this mail could probably be more + // engaging and we could have a fancy HTML version. + + $template = array(); + $template[] = pht( + '%s has invited you to join Phabricator.', + $viewer->getFullName()); + + if (strlen(trim($message))) { + $template[] = $message; + } + + $template[] = pht( + 'To register an account and get started, follow this link:'); + + // This isn't a variable; it will be replaced later on in the + // daemons once they generate the URI. + $template[] = '{$INVITE_URI}'; + + $template[] = pht( + 'If you already have an account, you can follow the link to '. + 'quickly verify this email address.'); + + $template = implode("\n\n", $template); + + foreach ($actions as $action) { + if ($action->willSend()) { + $action->sendInvite($viewer, $template); + } + } + + // TODO: This is a bit anticlimactic. We don't really have anything + // to show the user because the action is happening in the background + // and the invites won't exist yet. After T5166 we can show a + // better progress bar. + return id(new AphrontRedirectResponse()) + ->setURI($this->getApplicationURI()); } } } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php new file mode 100644 index 0000000000..c90fac5897 --- /dev/null +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php @@ -0,0 +1,61 @@ +setName('execute') + ->setExamples('**execute** --id __id__') + ->setSynopsis( + pht( + 'Execute a task explicitly. This command ignores leases, is '. + 'dangerous, and may cause work to be performed twice.')) + ->setArguments($this->getTaskSelectionArguments()); + } + + public function execute(PhutilArgumentParser $args) { + $console = PhutilConsole::getConsole(); + $tasks = $this->loadTasks($args); + + foreach ($tasks as $task) { + $can_execute = !$task->isArchived(); + if (!$can_execute) { + $console->writeOut( + "** %s ** %s\n", + pht('ARCHIVED'), + pht( + '%s is already archived, and can not be executed.', + $this->describeTask($task))); + continue; + } + + // NOTE: This ignores leases, maybe it should respect them without + // a parameter like --force? + + $task->setLeaseOwner(null); + $task->setLeaseExpires(PhabricatorTime::getNow()); + $task->save(); + + $task_data = id(new PhabricatorWorkerTaskData())->loadOneWhere( + 'id = %d', + $task->getDataID()); + $task->setData($task_data->getData()); + + $id = $task->getID(); + $class = $task->getTaskClass(); + + $console->writeOut("Executing task {$id} ({$class})..."); + + $task->executeTask(); + $ex = $task->getExecutionException(); + + if ($ex) { + throw $ex; + } + } + + return 0; + } + +}