Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15379803
D20023.id.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
6 KB
Referenced Files
None
Subscribers
None
D20023.id.diff
View Options
diff --git a/src/applications/auth/controller/contact/PhabricatorAuthContactNumberPrimaryController.php b/src/applications/auth/controller/contact/PhabricatorAuthContactNumberPrimaryController.php
--- a/src/applications/auth/controller/contact/PhabricatorAuthContactNumberPrimaryController.php
+++ b/src/applications/auth/controller/contact/PhabricatorAuthContactNumberPrimaryController.php
@@ -55,7 +55,16 @@
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true);
- $editor->applyTransactions($number, $xactions);
+ try {
+ $editor->applyTransactions($number, $xactions);
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ // This happens when you try to make a number into your primary
+ // number, but you have contact number MFA on your account.
+ return $this->newDialog()
+ ->setTitle(pht('Unable to Make Primary'))
+ ->setValidationException($ex)
+ ->addCancelButton($cancel_uri);
+ }
return id(new AphrontRedirectResponse())->setURI($cancel_uri);
}
diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorAuthFactor.php
@@ -62,6 +62,18 @@
return null;
}
+ /**
+ * Is this a factor which depends on the user's contact number?
+ *
+ * If a user has a "contact number" factor configured, they can not modify
+ * or switch their primary contact number.
+ *
+ * @return bool True if this factor should lock contact numbers.
+ */
+ public function isContactNumberFactor() {
+ return false;
+ }
+
abstract public function getEnrollDescription(
PhabricatorAuthFactorProvider $provider,
PhabricatorUser $user);
diff --git a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php
@@ -28,6 +28,10 @@
return 2000;
}
+ public function isContactNumberFactor() {
+ return true;
+ }
+
public function canCreateNewProvider() {
return $this->isSMSMailerConfigured();
}
diff --git a/src/applications/auth/xaction/PhabricatorAuthContactNumberNumberTransaction.php b/src/applications/auth/xaction/PhabricatorAuthContactNumberNumberTransaction.php
--- a/src/applications/auth/xaction/PhabricatorAuthContactNumberNumberTransaction.php
+++ b/src/applications/auth/xaction/PhabricatorAuthContactNumberNumberTransaction.php
@@ -83,6 +83,11 @@
}
}
+ $mfa_error = $this->newContactNumberMFAError($object, $xaction);
+ if ($mfa_error) {
+ $errors[] = $mfa_error;
+ continue;
+ }
}
return $errors;
diff --git a/src/applications/auth/xaction/PhabricatorAuthContactNumberPrimaryTransaction.php b/src/applications/auth/xaction/PhabricatorAuthContactNumberPrimaryTransaction.php
--- a/src/applications/auth/xaction/PhabricatorAuthContactNumberPrimaryTransaction.php
+++ b/src/applications/auth/xaction/PhabricatorAuthContactNumberPrimaryTransaction.php
@@ -41,6 +41,12 @@
$xaction);
continue;
}
+
+ $mfa_error = $this->newContactNumberMFAError($object, $xaction);
+ if ($mfa_error) {
+ $errors[] = $mfa_error;
+ continue;
+ }
}
return $errors;
diff --git a/src/applications/auth/xaction/PhabricatorAuthContactNumberStatusTransaction.php b/src/applications/auth/xaction/PhabricatorAuthContactNumberStatusTransaction.php
--- a/src/applications/auth/xaction/PhabricatorAuthContactNumberStatusTransaction.php
+++ b/src/applications/auth/xaction/PhabricatorAuthContactNumberStatusTransaction.php
@@ -46,6 +46,12 @@
continue;
}
+ $mfa_error = $this->newContactNumberMFAError($object, $xaction);
+ if ($mfa_error) {
+ $errors[] = $mfa_error;
+ continue;
+ }
+
// NOTE: Enabling a contact number may cause us to collide with another
// active contact number. However, there might also be a transaction in
// this group that changes the number itself. Since we can't easily
diff --git a/src/applications/auth/xaction/PhabricatorAuthContactNumberTransactionType.php b/src/applications/auth/xaction/PhabricatorAuthContactNumberTransactionType.php
--- a/src/applications/auth/xaction/PhabricatorAuthContactNumberTransactionType.php
+++ b/src/applications/auth/xaction/PhabricatorAuthContactNumberTransactionType.php
@@ -1,4 +1,72 @@
<?php
abstract class PhabricatorAuthContactNumberTransactionType
- extends PhabricatorModularTransactionType {}
+ extends PhabricatorModularTransactionType {
+
+ protected function newContactNumberMFAError($object, $xaction) {
+ // If a contact number is attached to a user and that user has SMS MFA
+ // configured, don't let the user modify their primary contact number or
+ // make another contact number into their primary number.
+
+ $primary_type =
+ PhabricatorAuthContactNumberPrimaryTransaction::TRANSACTIONTYPE;
+
+ if ($xaction->getTransactionType() === $primary_type) {
+ // We're trying to make a non-primary number into the primary number,
+ // so do MFA checks.
+ $is_primary = false;
+ } else if ($object->getIsPrimary()) {
+ // We're editing the primary number, so do MFA checks.
+ $is_primary = true;
+ } else {
+ // Editing a non-primary number and not making it primary, so this is
+ // fine.
+ return null;
+ }
+
+ $target_phid = $object->getObjectPHID();
+ $omnipotent = PhabricatorUser::getOmnipotentUser();
+
+ $user_configs = id(new PhabricatorAuthFactorConfigQuery())
+ ->setViewer($omnipotent)
+ ->withUserPHIDs(array($target_phid))
+ ->execute();
+
+ $problem_configs = array();
+ foreach ($user_configs as $config) {
+ $provider = $config->getFactorProvider();
+ $factor = $provider->getFactor();
+
+ if ($factor->isContactNumberFactor()) {
+ $problem_configs[] = $config;
+ }
+ }
+
+ if (!$problem_configs) {
+ return null;
+ }
+
+ $problem_config = head($problem_configs);
+
+ if ($is_primary) {
+ return $this->newInvalidError(
+ pht(
+ 'You currently have multi-factor authentication ("%s") which '.
+ 'depends on your primary contact number. You must remove this '.
+ 'authentication factor before you can modify or disable your '.
+ 'primary contact number.',
+ $problem_config->getFactorName()),
+ $xaction);
+ } else {
+ return $this->newInvalidError(
+ pht(
+ 'You currently have multi-factor authentication ("%s") which '.
+ 'depends on your primary contact number. You must remove this '.
+ 'authentication factor before you can designate a new primary '.
+ 'contact number.',
+ $problem_config->getFactorName()),
+ $xaction);
+ }
+ }
+
+}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Mar 14, 11:28 PM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7676116
Default Alt Text
D20023.id.diff (6 KB)
Attached To
Mode
D20023: Prevent users from editing, disabling, or swapping their primary contact number while they have SMS MFA
Attached
Detach File
Event Timeline
Log In to Comment