From 55a94d8aba2a7a7ab6f8499af491b14301469b3b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 20 Feb 2014 08:12:04 -0800
Subject: [PATCH] Don't prompt to upgrade unset passwords

Summary:
Fixes T4463. When your VCS or account password is not set, we test it for upgrade anyway. This doesn't make sense and throws shortly into the process because the empty hash isn't parseable.

Instead, only show upgrade prompts when the password exists.

Test Plan:
  - Added a password to an existing account with no password via password reset.
  - Added a VCS password to an existing account with no VCS password.
  - Observed no fatals / nonsense behaviors.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4463

Differential Revision: https://secure.phabricator.com/D8282
---
 .../diffusion/panel/DiffusionSetPasswordPanel.php    | 12 +++++++-----
 .../panel/PhabricatorSettingsPanelPassword.php       | 12 +++++++-----
 .../util/password/PhabricatorPasswordHasher.php      |  5 +++++
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php
index 0f4829d2ec..e896a03184 100644
--- a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php
+++ b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php
@@ -178,11 +178,13 @@ final class DiffusionSetPasswordPanel extends PhabricatorSettingsPanel {
         ->setLabel(pht('Best Available Algorithm'))
         ->setValue(PhabricatorPasswordHasher::getBestAlgorithmName()));
 
-    if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
-      $errors[] = pht(
-        'The strength of your stored VCS password hash can be upgraded. '.
-        'To upgrade, either: use the password to authenticate with a '.
-        'repository; or change your password.');
+    if (strlen($hash_envelope->openEnvelope())) {
+      if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
+        $errors[] = pht(
+          'The strength of your stored VCS password hash can be upgraded. '.
+          'To upgrade, either: use the password to authenticate with a '.
+          'repository; or change your password.');
+      }
     }
 
     $object_box = id(new PHUIObjectBoxView())
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php
index 769ab03a90..bc229fbaab 100644
--- a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php
@@ -113,11 +113,13 @@ final class PhabricatorSettingsPanelPassword
     }
 
     $hash_envelope = new PhutilOpaqueEnvelope($user->getPasswordHash());
-    if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
-      $errors[] = pht(
-        'The strength of your stored password hash can be upgraded. '.
-        'To upgrade, either: log out and log in using your password; or '.
-        'change your password.');
+    if (strlen($hash_envelope->openEnvelope())) {
+      if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
+        $errors[] = pht(
+          'The strength of your stored password hash can be upgraded. '.
+          'To upgrade, either: log out and log in using your password; or '.
+          'change your password.');
+      }
     }
 
     $len_caption = null;
diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php
index b344893689..338a7615ee 100644
--- a/src/infrastructure/util/password/PhabricatorPasswordHasher.php
+++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php
@@ -333,6 +333,11 @@ abstract class PhabricatorPasswordHasher extends Phobject {
    * @task hashing
    */
   public static function canUpgradeHash(PhutilOpaqueEnvelope $hash) {
+    if (!strlen($hash->openEnvelope())) {
+      throw new Exception(
+        pht('Expected a password hash, received nothing!'));
+    }
+
     $current_hasher = self::getHasherForHash($hash);
     $best_hasher = self::getBestHasher();