From 83112cc2e8c79471f0d5ec31039c90b455c06310 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 3 Jun 2014 16:45:18 -0700 Subject: [PATCH] Move email verification into PhabricatorUserEditor Summary: Both email verify and welcome links now verify email, centralize them and record them in the user activity log. Test Plan: - Followed a "verify email" link and got verified. - Followed a "welcome" (verifying) link. - Followed a "reset" (non-verifying) link. - Looked in the activity log for the verifications. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D9284 --- .../PhabricatorAuthOneTimeLoginController.php | 15 +---- ...PhabricatorEmailVerificationController.php | 16 +---- .../people/editor/PhabricatorUserEditor.php | 59 +++++++++++++++++++ .../people/storage/PhabricatorUserLog.php | 2 + 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php index a0edc324bb..cb663344b2 100644 --- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php @@ -102,18 +102,9 @@ final class PhabricatorAuthOneTimeLoginController $token->delete(); if ($target_email) { - $target_user->openTransaction(); - $target_email->setIsVerified(1); - $target_email->save(); - - // If this was the user's primary email address, also mark their - // account as verified. - $primary_email = $target_user->loadPrimaryEmail(); - if ($primary_email->getID() == $target_email->getID()) { - $target_user->setIsEmailVerified(1); - $target_user->save(); - } - $target_user->saveTransaction(); + id(new PhabricatorUserEditor()) + ->setActor($target_user) + ->verifyEmail($target_user, $target_email); } unset($unguarded); diff --git a/src/applications/auth/controller/PhabricatorEmailVerificationController.php b/src/applications/auth/controller/PhabricatorEmailVerificationController.php index bca3f27f20..644b5ca7f5 100644 --- a/src/applications/auth/controller/PhabricatorEmailVerificationController.php +++ b/src/applications/auth/controller/PhabricatorEmailVerificationController.php @@ -52,20 +52,10 @@ final class PhabricatorEmailVerificationController 'This email address has already been verified.'); $continue = pht('Continue to Phabricator'); } else if ($request->isFormPost()) { - $email->openTransaction(); - $email->setIsVerified(1); - $email->save(); - - // If the user just verified their primary email address, mark their - // account as email verified. - $user_primary = $user->loadPrimaryEmail(); - if ($user_primary->getID() == $email->getID()) { - $user->setIsEmailVerified(1); - $user->save(); - } - - $email->saveTransaction(); + id(new PhabricatorUserEditor()) + ->setActor($user) + ->verifyEmail($user, $email); $title = pht('Address Verified'); $content = pht( diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index fa889f1223..21724b6e08 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -494,6 +494,65 @@ final class PhabricatorUserEditor extends PhabricatorEditor { } + /** + * Verify a user's email address. + * + * This verifies an individual email address. If the address is the user's + * primary address and their account was not previously verified, their + * account is marked as email verified. + * + * @task email + */ + public function verifyEmail( + PhabricatorUser $user, + PhabricatorUserEmail $email) { + $actor = $this->requireActor(); + + if (!$user->getID()) { + throw new Exception('User has not been created yet!'); + } + if (!$email->getID()) { + throw new Exception('Email has not been created yet!'); + } + + $user->openTransaction(); + $user->beginWriteLocking(); + + $user->reload(); + $email->reload(); + + if ($email->getUserPHID() != $user->getPHID()) { + throw new Exception(pht('User does not own email!')); + } + + if (!$email->getIsVerified()) { + $email->setIsVerified(1); + $email->save(); + + $log = PhabricatorUserLog::initializeNewLog( + $actor, + $user->getPHID(), + PhabricatorUserLog::ACTION_EMAIL_VERIFY); + $log->setNewValue($email->getAddress()); + $log->save(); + } + + if (!$user->getIsEmailVerified()) { + // If the user just verified their primary email address, mark their + // account as email verified. + $user_primary = $user->loadPrimaryEmail(); + if ($user_primary->getID() == $email->getID()) { + $user->setIsEmailVerified(1); + $user->save(); + } + } + + $user->endWriteLocking(); + $user->saveTransaction(); + + } + + /* -( Internals )---------------------------------------------------------- */ diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php index 5a0dfdfc39..4dcc888880 100644 --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -25,6 +25,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO const ACTION_EMAIL_PRIMARY = 'email-primary'; const ACTION_EMAIL_REMOVE = 'email-remove'; const ACTION_EMAIL_ADD = 'email-add'; + const ACTION_EMAIL_VERIFY = 'email-verify'; const ACTION_CHANGE_PASSWORD = 'change-password'; const ACTION_CHANGE_USERNAME = 'change-username'; @@ -67,6 +68,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO self::ACTION_EMAIL_PRIMARY => pht('Email: Change Primary'), self::ACTION_EMAIL_ADD => pht('Email: Add Address'), self::ACTION_EMAIL_REMOVE => pht('Email: Remove Address'), + self::ACTION_EMAIL_VERIFY => pht('Email: Verify'), self::ACTION_CHANGE_PASSWORD => pht('Change Password'), self::ACTION_CHANGE_USERNAME => pht('Change Username'), self::ACTION_ENTER_HISEC => pht('Hisec: Enter'),