Bring new password validation into AuthPasswordEngine

Summary:
Ref T13043. We have ~4 copies of this logic (registration, lost password recovery, set password, set VCS password).

Currently it varies a bit from case to case, but since it's all going to be basically identical once account passwords swap to the new infrastructure, bring it into the Engine so it can live in one place.

This also fixes VCS passwords not being affected by `account.minimum-password-length`.

Test Plan: Hit all errors in "VCS Password" panel. Successfully changed password.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

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

src/__phutil_library_map__.php

@@ -2092,6 +2092,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthPassword' => 'applications/auth/storage/PhabricatorAuthPassword.php',
     'PhabricatorAuthPasswordEditor' => 'applications/auth/editor/PhabricatorAuthPasswordEditor.php',
     'PhabricatorAuthPasswordEngine' => 'applications/auth/engine/PhabricatorAuthPasswordEngine.php',
+    'PhabricatorAuthPasswordException' => 'applications/auth/password/PhabricatorAuthPasswordException.php',
     'PhabricatorAuthPasswordPHIDType' => 'applications/auth/phid/PhabricatorAuthPasswordPHIDType.php',
     'PhabricatorAuthPasswordQuery' => 'applications/auth/query/PhabricatorAuthPasswordQuery.php',
     'PhabricatorAuthPasswordResetTemporaryTokenType' => 'applications/auth/tokentype/PhabricatorAuthPasswordResetTemporaryTokenType.php',
@@ -7387,6 +7388,7 @@ phutil_register_library_map(array(
     ),
     'PhabricatorAuthPasswordEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhabricatorAuthPasswordEngine' => 'Phobject',
+    'PhabricatorAuthPasswordException' => 'Exception',
     'PhabricatorAuthPasswordPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorAuthPasswordQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorAuthPasswordResetTemporaryTokenType' => 'PhabricatorAuthTemporaryTokenType',

src/applications/auth/engine/PhabricatorAuthPasswordEngine.php

@@ -54,6 +54,79 @@ final class PhabricatorAuthPasswordEngine
     return $this->upgradeHashers;
   }
 
+  public function checkNewPassword(
+    PhutilOpaqueEnvelope $password,
+    PhutilOpaqueEnvelope $confirm,
+    $can_skip = false) {
+
+    $raw_password = $password->openEnvelope();
+
+    if (!strlen($raw_password)) {
+      if ($can_skip) {
+        throw new PhabricatorAuthPasswordException(
+          pht('You must choose a password or skip this step.'),
+          pht('Required'));
+      } else {
+        throw new PhabricatorAuthPasswordException(
+          pht('You must choose a password.'),
+          pht('Required'));
+      }
+    }
+
+    $min_len = PhabricatorEnv::getEnvConfig('account.minimum-password-length');
+    $min_len = (int)$min_len;
+    if ($min_len) {
+      if (strlen($raw_password) < $min_len) {
+        throw new PhabricatorAuthPasswordException(
+          pht(
+            'The selected password is too short. Passwords must be a minimum '.
+            'of %s characters long.',
+            new PhutilNumber($min_len)),
+          pht('Too Short'));
+      }
+    }
+
+    $raw_confirm = $confirm->openEnvelope();
+
+    if (!strlen($raw_confirm)) {
+      throw new PhabricatorAuthPasswordException(
+        pht('You must confirm the selected password.'),
+        null,
+        pht('Required'));
+    }
+
+    if ($raw_password !== $raw_confirm) {
+      throw new PhabricatorAuthPasswordException(
+        pht('The password and confirmation do not match.'),
+        pht('Invalid'),
+        pht('Invalid'));
+    }
+
+    if (PhabricatorCommonPasswords::isCommonPassword($raw_password)) {
+      throw new PhabricatorAuthPasswordException(
+        pht(
+          'The selected password is very weak: it is one of the most common '.
+          'passwords in use. Choose a stronger password.'),
+        pht('Very Weak'));
+    }
+
+    if ($this->isRevokedPassword($password)) {
+      throw new PhabricatorAuthPasswordException(
+        pht(
+          'The password you entered has been revoked. You can not reuse '.
+          'a password which has been revoked. Choose a new password.'),
+        pht('Revoked'));
+    }
+
+    if (!$this->isUniquePassword($password)) {
+      throw new PhabricatorAuthPasswordException(
+        pht(
+          'The password you entered is the same as another password '.
+          'associated with your account. Each password must be unique.'),
+        pht('Not Unique'));
+    }
+  }
+
   public function isValidPassword(PhutilOpaqueEnvelope $envelope) {
     $this->requireSetup();
 

src/applications/auth/password/PhabricatorAuthPasswordException.php

@@ -0,0 +1,28 @@
+<?php
+
+final class PhabricatorAuthPasswordException
+  extends Exception {
+
+  private $passwordError;
+  private $confirmErorr;
+
+  public function __construct(
+    $message,
+    $password_error,
+    $confirm_error = null) {
+
+    $this->passwordError = $password_error;
+    $this->confirmError = $confirm_error;
+
+    parent::__construct($message);
+  }
+
+  public function getPasswordError() {
+    return $this->passwordError;
+  }
+
+  public function getConfirmError() {
+    return $this->confirmError;
+  }
+
+}

src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php

@@ -58,6 +58,19 @@ final class DiffusionSetPasswordSettingsPanel extends PhabricatorSettingsPanel {
     $e_password = true;
     $e_confirm = true;
 
+    $content_source = PhabricatorContentSource::newFromRequest($request);
+
+    // NOTE: This test is against $viewer (not $user), so that the error
+    // message below makes sense in the case that the two are different,
+    // and because an admin reusing their own password is bad, while
+    // system agents generally do not have passwords anyway.
+
+    $engine = id(new PhabricatorAuthPasswordEngine())
+      ->setViewer($viewer)
+      ->setContentSource($content_source)
+      ->setObject($viewer)
+      ->setPasswordType($vcs_type);
+
     if ($request->isFormPost()) {
       if ($request->getBool('remove')) {
         if ($vcspassword->getID()) {
@@ -68,71 +81,26 @@ final class DiffusionSetPasswordSettingsPanel extends PhabricatorSettingsPanel {
 
       $new_password = $request->getStr('password');
       $confirm = $request->getStr('confirm');
-      if (!strlen($new_password)) {
-        $e_password = pht('Required');
-        $errors[] = pht('Password is required.');
-      } else {
-        $e_password = null;
-      }
 
-      if (!strlen($confirm)) {
-        $e_confirm = pht('Required');
-        $errors[] = pht('You must confirm the new password.');
-      } else {
+      $envelope = new PhutilOpaqueEnvelope($new_password);
+      $confirm_envelope = new PhutilOpaqueEnvelope($confirm);
+
+      try {
+        $engine->checkNewPassword($envelope, $confirm_envelope);
+        $e_password = null;
         $e_confirm = null;
+      } catch (PhabricatorAuthPasswordException $ex) {
+        $errors[] = $ex->getMessage();
+        $e_password = $ex->getPasswordError();
+        $e_confirm = $ex->getConfirmError();
       }
 
       if (!$errors) {
-        $envelope = new PhutilOpaqueEnvelope($new_password);
-        $content_source = PhabricatorContentSource::newFromRequest($request);
+        $vcspassword
+          ->setPassword($envelope, $user)
+          ->save();
 
-        // NOTE: This test is against $viewer (not $user), so that the error
-        // message below makes sense in the case that the two are different,
-        // and because an admin reusing their own password is bad, while
-        // system agents generally do not have passwords anyway.
-
-        $engine = id(new PhabricatorAuthPasswordEngine())
-          ->setViewer($viewer)
-          ->setContentSource($content_source)
-          ->setObject($viewer)
-          ->setPasswordType($vcs_type);
-
-        $same_password = !$engine->isUniquePassword($envelope);
-        $revoked_password = $engine->isRevokedPassword($envelope);
-
-        if ($new_password !== $confirm) {
-          $e_password = pht('Does Not Match');
-          $e_confirm = pht('Does Not Match');
-          $errors[] = pht('Password and confirmation do not match.');
-        } else if ($revoked_password) {
-          $e_password = pht('Revoked');
-          $e_confirm = pht('Revoked');
-          $errors[] = pht(
-            'This password has been revoked. You must choose a new, unique '.
-            'password.');
-        } else if ($same_password) {
-          $e_password = pht('Not Unique');
-          $e_confirm = pht('Not Unique');
-          $errors[] = pht(
-            'This password is the same as another password associated '.
-            'with your account. You must use a unique password for '.
-            'VCS access.');
-        } else if (
-          PhabricatorCommonPasswords::isCommonPassword($new_password)) {
-          $e_password = pht('Very Weak');
-          $e_confirm = pht('Very Weak');
-          $errors[] = pht(
-            'This password is extremely weak: it is one of the most common '.
-            'passwords in use. Choose a stronger password.');
-        }
-
-
-        if (!$errors) {
-          $vcspassword->setPassword($envelope, $user);
-          $vcspassword->save();
-
-          return id(new AphrontRedirectResponse())->setURI($panel_uri);
-        }
+        return id(new AphrontRedirectResponse())->setURI($panel_uri);
       }
     }