From 33ed2a390c8c556aaf19b9438fd47c9d9ad131aa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 3 Feb 2019 06:36:49 -0800
Subject: [PATCH] (stable) Fix an issue where Duo validation could incorrectly
 apply to other factor types

See <https://discourse.phabricator-community.org/t/configuring-mfa-provider-totp-fails-for-missing-duo-only-options/2355>.

Test Plan: Created a TOTP provider; created a Duo provider (with missing and supplied values).
---
 ...catorAuthFactorProviderDuoCredentialTransaction.php |  4 ++++
 ...ricatorAuthFactorProviderDuoHostnameTransaction.php |  4 ++++
 .../PhabricatorAuthFactorProviderTransactionType.php   | 10 +++++++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php
index 532fc271f4..f5a52cb90f 100644
--- a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php
+++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php
@@ -27,6 +27,10 @@ final class PhabricatorAuthFactorProviderDuoCredentialTransaction
     $actor = $this->getActor();
     $errors = array();
 
+    if (!$this->isDuoProvider($object)) {
+      return $errors;
+    }
+
     $old_value = $this->generateOldValue($object);
     if ($this->isEmptyTextTransaction($old_value, $xactions)) {
       $errors[] = $this->newRequiredError(
diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php
index ce1838594e..27ae271137 100644
--- a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php
+++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php
@@ -26,6 +26,10 @@ final class PhabricatorAuthFactorProviderDuoHostnameTransaction
   public function validateTransactions($object, array $xactions) {
     $errors = array();
 
+    if (!$this->isDuoProvider($object)) {
+      return $errors;
+    }
+
     $old_value = $this->generateOldValue($object);
     if ($this->isEmptyTextTransaction($old_value, $xactions)) {
       $errors[] = $this->newRequiredError(
diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php
index fe17eee545..3f16249e0c 100644
--- a/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php
+++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php
@@ -1,4 +1,12 @@
 <?php
 
 abstract class PhabricatorAuthFactorProviderTransactionType
-  extends PhabricatorModularTransactionType {}
+  extends PhabricatorModularTransactionType {
+
+  final protected function isDuoProvider(
+    PhabricatorAuthFactorProvider $provider) {
+    $duo_key = id(new PhabricatorDuoAuthFactor())->getFactorKey();
+    return ($provider->getProviderFactorKey() === $duo_key);
+  }
+
+}