diff --git a/src/applications/auth/controller/PhabricatorEmailTokenController.php b/src/applications/auth/controller/PhabricatorEmailTokenController.php index b4f998113e..f4cdfa96d2 100644 --- a/src/applications/auth/controller/PhabricatorEmailTokenController.php +++ b/src/applications/auth/controller/PhabricatorEmailTokenController.php @@ -16,19 +16,14 @@ final class PhabricatorEmailTokenController public function processRequest() { $request = $this->getRequest(); + if ($request->getUser()->isLoggedIn()) { + return $this->renderError( + pht('You are already logged in.')); + } + $token = $this->token; $email = $request->getStr('email'); - // NOTE: We need to bind verification to **addresses**, not **users**, - // because we verify addresses when they're used to login this way, and if - // we have a user-based verification you can: - // - // - Add some address you do not own; - // - request a password reset; - // - change the URI in the email to the address you don't own; - // - login via the email link; and - // - get a "verified" address you don't control. - $target_email = id(new PhabricatorUserEmail())->loadOneWhere( 'address = %s', $email); @@ -40,6 +35,16 @@ final class PhabricatorEmailTokenController $target_email->getUserPHID()); } + // NOTE: We need to bind verification to **addresses**, not **users**, + // because we verify addresses when they're used to login this way, and if + // we have a user-based verification you can: + // + // - Add some address you do not own; + // - request a password reset; + // - change the URI in the email to the address you don't own; + // - login via the email link; and + // - get a "verified" address you don't control. + if (!$target_email || !$target_user || !$target_user->validateEmailToken($target_email, $token)) { @@ -65,29 +70,59 @@ final class PhabricatorEmailTokenController )); } - // Verify email so that clicking the link in the "Welcome" email is good - // enough, without requiring users to go through a second round of email - // verification. + if ($request->isFormPost()) { + // Verify email so that clicking the link in the "Welcome" email is good + // enough, without requiring users to go through a second round of email + // verification. - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $target_email->setIsVerified(1); - $target_email->save(); - unset($unguarded); + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $target_email->setIsVerified(1); + $target_email->save(); + unset($unguarded); - $next = '/'; - if (!PhabricatorAuthProviderPassword::getPasswordProvider()) { - $next = '/settings/panel/external/'; - } else if (PhabricatorEnv::getEnvConfig('account.editable')) { - $next = (string)id(new PhutilURI('/settings/panel/password/')) - ->setQueryParams( - array( - 'token' => $token, - 'email' => $email, - )); + $next = '/'; + if (!PhabricatorAuthProviderPassword::getPasswordProvider()) { + $next = '/settings/panel/external/'; + } else if (PhabricatorEnv::getEnvConfig('account.editable')) { + $next = (string)id(new PhutilURI('/settings/panel/password/')) + ->setQueryParams( + array( + 'token' => $token, + 'email' => $email, + )); + } + + PhabricatorCookies::setNextURICookie($request, $next, $force = true); + + return $this->loginUser($target_user); } - PhabricatorCookies::setNextURICookie($request, $next, $force = true); + // NOTE: We need to CSRF here so attackers can't generate an email link, + // then log a user in to an account they control via sneaky invisible + // form submissions. - return $this->loginUser($target_user); + // TODO: Since users can arrive here either through password reset or + // through welcome emails, it might be nice to include the workflow type + // in the URI or query params so we can tailor the messaging. Right now, + // it has to be generic enough to make sense in either workflow, which + // leaves it feeling a little awkward. + + $dialog = id(new AphrontDialogView()) + ->setUser($request->getUser()) + ->setTitle(pht('Login to Phabricator')) + ->addHiddenInput('email', $email) + ->appendParagraph( + pht( + 'Use the button below to log in as: %s', + phutil_tag('strong', array(), $email))) + ->appendParagraph( + pht( + 'After logging in you should set a password for your account, or '. + 'link your account to an external account that you can use to '. + 'authenticate in the future.')) + ->addSubmitButton(pht('Login (%s)', $email)) + ->addCancelButton('/'); + + return id(new AphrontDialogResponse())->setDialog($dialog); } }