Page MenuHomePhabricator
Files F13310704

D8079.diff
No OneTemporary
Actions

D8079.diff
View Options

Index: src/applications/auth/controller/PhabricatorEmailTokenController.php
===================================================================
--- src/applications/auth/controller/PhabricatorEmailTokenController.php
+++ src/applications/auth/controller/PhabricatorEmailTokenController.php
@@ -16,19 +16,14 @@
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 @@
$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 @@
));
}
- // 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);
-
- $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,
- ));
+ 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);
+
+ $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);
-
- return $this->loginUser($target_user);
+ // 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.
+
+ // 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);
}
}

File Metadata

Mime Type
text/plain
Expires
Tue, Jun 11, 5:27 PM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6281484
Default Alt Text
D8079.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility