When users have no password on their account, guide them through the "reset password" flow in the guise of "set password"

Summary:
Depends on D20119. Fixes T9512. When you don't have a password on your account, the "Password" panel in Settings is non-obviously useless: you can't provide an old password, so you can't change your password.

The correct remedy is to "Forgot password?" and go through the password reset flow. However, we don't guide you to this and it isn't really self-evident.

Instead:

  - Guide users to the password reset flow.
  - Make it work when you're already logged in.
  - Skin it as a "set password" flow.

We're still requiring you to prove you own the email associated with your account. This is a pretty weak requirement, but maybe stops attackers who use the computer at the library after you do in some bizarre emergency and forget to log out? It would probably be fine to just let users "set password", this mostly just keeps us from having two different pieces of code responsible for setting passwords.

Test Plan:
  - Set password as a logged-in user.
  - Reset password on the normal flow as a logged-out user.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: revi

Maniphest Tasks: T9512

Differential Revision: https://secure.phabricator.com/D20120

src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php

@@ -14,11 +14,6 @@ final class PhabricatorAuthOneTimeLoginController
     $key = $request->getURIData('key');
     $email_id = $request->getURIData('emailID');
 
-    if ($request->getUser()->isLoggedIn()) {
-      return $this->renderError(
-        pht('You are already logged in.'));
-    }
-
     $target_user = id(new PhabricatorPeopleQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withIDs(array($id))
@@ -27,6 +22,19 @@ final class PhabricatorAuthOneTimeLoginController
       return new Aphront404Response();
     }
 
+    // NOTE: We allow you to use a one-time login link for your own current
+    // login account. This supports the "Set Password" flow.
+
+    $is_logged_in = false;
+    if ($viewer->isLoggedIn()) {
+      if ($viewer->getPHID() !== $target_user->getPHID()) {
+        return $this->renderError(
+          pht('You are already logged in.'));
+      } else {
+        $is_logged_in = true;
+      }
+    }
+
     // NOTE: As a convenience to users, these one-time login URIs may also
     // be associated with an email address which will be verified when the
     // URI is used.
@@ -100,7 +108,7 @@ final class PhabricatorAuthOneTimeLoginController
         ->addCancelButton('/');
     }
 
-    if ($request->isFormPost()) {
+    if ($request->isFormPost() || $is_logged_in) {
       // If we have an email bound into this URI, 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.
@@ -121,6 +129,12 @@ final class PhabricatorAuthOneTimeLoginController
 
       $next_uri = $this->getNextStepURI($target_user);
 
+      // If the user is already logged in, we're just doing a "password set"
+      // flow. Skip directly to the next step.
+      if ($is_logged_in) {
+        return id(new AphrontRedirectResponse())->setURI($next_uri);
+      }
+
       PhabricatorCookies::setNextURICookie($request, $next_uri, $force = true);
 
       $force_full_session = false;

src/applications/auth/controller/PhabricatorEmailLoginController.php

@@ -9,20 +9,38 @@ final class PhabricatorEmailLoginController
 
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
+    $is_logged_in = $viewer->isLoggedIn();
 
     $e_email = true;
     $e_captcha = true;
     $errors = array();
 
-    $v_email = $request->getStr('email');
+    if ($is_logged_in) {
+      if (!$this->isPasswordAuthEnabled()) {
+        return $this->newDialog()
+          ->setTitle(pht('No Password Auth'))
+          ->appendParagraph(
+            pht(
+              'Password authentication is not enabled and you are already '.
+              'logged in. There is nothing for you here.'))
+          ->addCancelButton('/', pht('Continue'));
+      }
+
+      $v_email = $viewer->loadPrimaryEmailAddress();
+    } else {
+      $v_email = $request->getStr('email');
+    }
+
     if ($request->isFormPost()) {
       $e_email = null;
       $e_captcha = pht('Again');
 
-      $captcha_ok = AphrontFormRecaptchaControl::processCaptcha($request);
-      if (!$captcha_ok) {
-        $errors[] = pht('Captcha response is incorrect, try again.');
-        $e_captcha = pht('Invalid');
+      if (!$is_logged_in) {
+        $captcha_ok = AphrontFormRecaptchaControl::processCaptcha($request);
+        if (!$captcha_ok) {
+          $errors[] = pht('Captcha response is incorrect, try again.');
+          $e_captcha = pht('Invalid');
+        }
       }
 
       if (!strlen($v_email)) {
@@ -76,10 +94,24 @@ final class PhabricatorEmailLoginController
         }
 
         if (!$errors) {
-          $body = $this->newAccountLoginMailBody($target_user);
+          $body = $this->newAccountLoginMailBody(
+            $target_user,
+            $is_logged_in);
+
+          if ($is_logged_in) {
+            $subject = pht('[Phabricator] Account Password Link');
+            $instructions = pht(
+              'An email has been sent containing a link you can use to set '.
+              'a password for your account.');
+          } else {
+            $subject = pht('[Phabricator] Account Login Link');
+            $instructions = pht(
+              'An email has been sent containing a link you can use to log '.
+              'in to your account.');
+          }
 
           $mail = id(new PhabricatorMetaMTAMail())
-            ->setSubject(pht('[Phabricator] Account Login Link'))
+            ->setSubject($subject)
             ->setForceDelivery(true)
             ->addRawTos(array($target_email->getAddress()))
             ->setBody($body)
@@ -88,8 +120,7 @@ final class PhabricatorEmailLoginController
           return $this->newDialog()
             ->setTitle(pht('Check Your Email'))
             ->setShortTitle(pht('Email Sent'))
-            ->appendParagraph(
-              pht('An email has been sent with a link you can use to log in.'))
+            ->appendParagraph($instructions)
             ->addCancelButton('/', pht('Done'));
         }
       }
@@ -99,33 +130,47 @@ final class PhabricatorEmailLoginController
       ->setViewer($viewer);
 
     if ($this->isPasswordAuthEnabled()) {
-      $form->appendRemarkupInstructions(
-        pht(
-          'To reset your password, provide your email address. An email '.
-          'with a login link will be sent to you.'));
+      if ($is_logged_in) {
+        $title = pht('Set Password');
+        $form->appendRemarkupInstructions(
+          pht(
+            'A password reset link will be sent to your primary email '.
+            'address. Follow the link to set an account password.'));
+      } else {
+        $title = pht('Password Reset');
+        $form->appendRemarkupInstructions(
+          pht(
+            'To reset your password, provide your email address. An email '.
+            'with a login link will be sent to you.'));
+      }
     } else {
+      $title = pht('Email Login');
       $form->appendRemarkupInstructions(
         pht(
           'To access your account, provide your email address. An email '.
           'with a login link will be sent to you.'));
     }
 
+    if ($is_logged_in) {
+      $address_control = new AphrontFormStaticControl();
+    } else {
+      $address_control = id(new AphrontFormTextControl())
+        ->setName('email')
+        ->setError($e_email);
+    }
+
+    $address_control
+      ->setLabel(pht('Email Address'))
+      ->setValue($v_email);
+
     $form
-      ->appendControl(
-        id(new AphrontFormTextControl())
-          ->setLabel(pht('Email Address'))
-          ->setName('email')
-          ->setValue($v_email)
-          ->setError($e_email))
-      ->appendControl(
+      ->appendControl($address_control);
+
+    if (!$is_logged_in) {
+      $form->appendControl(
         id(new AphrontFormRecaptchaControl())
           ->setLabel(pht('Captcha'))
           ->setError($e_captcha));
-
-    if ($this->isPasswordAuthEnabled()) {
-      $title = pht('Password Reset');
-    } else {
-      $title = pht('Email Login');
     }
 
     return $this->newDialog()
@@ -137,7 +182,10 @@ final class PhabricatorEmailLoginController
       ->addSubmitButton(pht('Send Email'));
   }
 
-  private function newAccountLoginMailBody(PhabricatorUser $user) {
+  private function newAccountLoginMailBody(
+    PhabricatorUser $user,
+    $is_logged_in) {
+
     $engine = new PhabricatorAuthSessionEngine();
     $uri = $engine->getOneTimeLoginURI(
       $user,
@@ -148,7 +196,12 @@ final class PhabricatorEmailLoginController
     $have_passwords = $this->isPasswordAuthEnabled();
 
     if ($have_passwords) {
-      if ($is_serious) {
+      if ($is_logged_in) {
+        $body = pht(
+          'You can use this link to set a password on your account:'.
+          "\n\n  %s\n",
+          $uri);
+      } else if ($is_serious) {
         $body = pht(
           "You can use this link to reset your Phabricator password:".
           "\n\n  %s\n",

src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php

@@ -34,11 +34,6 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel {
 
     $content_source = PhabricatorContentSource::newFromRequest($request);
 
-    $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession(
-      $viewer,
-      $request,
-      '/settings/');
-
     $min_len = PhabricatorEnv::getEnvConfig('account.minimum-password-length');
     $min_len = (int)$min_len;
 
@@ -55,20 +50,25 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel {
       ->withPasswordTypes(array($account_type))
       ->withIsRevoked(false)
       ->execute();
-    if ($password_objects) {
-      $password_object = head($password_objects);
-    } else {
-      $password_object = PhabricatorAuthPassword::initializeNewPassword(
-        $user,
-        $account_type);
+    if (!$password_objects) {
+      return $this->newSetPasswordView($request);
     }
+    $password_object = head($password_objects);
 
     $e_old = true;
     $e_new = true;
     $e_conf = true;
 
     $errors = array();
-    if ($request->isFormPost()) {
+    if ($request->isFormOrHisecPost()) {
+      $workflow_key = sprintf(
+        'password.change(%s)',
+        $user->getPHID());
+
+      $hisec_token = id(new PhabricatorAuthSessionEngine())
+        ->setWorkflowKey($workflow_key)
+        ->requireHighSecurityToken($viewer, $request, '/settings/');
+
       // Rate limit guesses about the old password. This page requires MFA and
       // session compromise already, so this is mostly just to stop researchers
       // from reporting this as a vulnerability.
@@ -218,5 +218,27 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel {
     );
   }
 
+  private function newSetPasswordView(AphrontRequest $request) {
+    $viewer = $request->getUser();
+    $user = $this->getUser();
+
+    $form = id(new AphrontFormView())
+      ->setViewer($viewer)
+      ->appendRemarkupInstructions(
+        pht(
+          'Your account does not currently have a password set. You can '.
+          'choose a password by performing a password reset.'))
+      ->appendControl(
+        id(new AphrontFormSubmitControl())
+          ->addCancelButton('/login/email/', pht('Reset Password')));
+
+    $form_box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Set Password'))
+      ->setBackground(PHUIObjectBoxView::WHITE_CONFIG)
+      ->setForm($form);
+
+    return $form_box;
+  }
+
 
 }