diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1731,6 +1731,7 @@ 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', 'ManiphestTaskListHTTPParameterType' => 'applications/maniphest/httpparametertype/ManiphestTaskListHTTPParameterType.php', 'ManiphestTaskListView' => 'applications/maniphest/view/ManiphestTaskListView.php', + 'ManiphestTaskMFAEngine' => 'applications/maniphest/engine/ManiphestTaskMFAEngine.php', 'ManiphestTaskMailReceiver' => 'applications/maniphest/mail/ManiphestTaskMailReceiver.php', 'ManiphestTaskMergeInRelationship' => 'applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php', 'ManiphestTaskMergedFromTransaction' => 'applications/maniphest/xaction/ManiphestTaskMergedFromTransaction.php', @@ -2977,6 +2978,8 @@ 'PhabricatorEditEngineListController' => 'applications/transactions/controller/PhabricatorEditEngineListController.php', 'PhabricatorEditEngineLock' => 'applications/transactions/editengine/PhabricatorEditEngineLock.php', 'PhabricatorEditEngineLockableInterface' => 'applications/transactions/editengine/PhabricatorEditEngineLockableInterface.php', + 'PhabricatorEditEngineMFAEngine' => 'applications/transactions/editengine/PhabricatorEditEngineMFAEngine.php', + 'PhabricatorEditEngineMFAInterface' => 'applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php', 'PhabricatorEditEnginePointsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEnginePointsCommentAction.php', 'PhabricatorEditEngineProfileMenuItem' => 'applications/search/menuitem/PhabricatorEditEngineProfileMenuItem.php', 'PhabricatorEditEngineQuery' => 'applications/transactions/query/PhabricatorEditEngineQuery.php', @@ -7298,6 +7301,7 @@ 'DoorkeeperBridgedObjectInterface', 'PhabricatorEditEngineSubtypeInterface', 'PhabricatorEditEngineLockableInterface', + 'PhabricatorEditEngineMFAInterface', ), 'ManiphestTaskAssignHeraldAction' => 'HeraldAction', 'ManiphestTaskAssignOtherHeraldAction' => 'ManiphestTaskAssignHeraldAction', @@ -7336,6 +7340,7 @@ 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListHTTPParameterType' => 'AphrontListHTTPParameterType', 'ManiphestTaskListView' => 'ManiphestView', + 'ManiphestTaskMFAEngine' => 'PhabricatorEditEngineMFAEngine', 'ManiphestTaskMailReceiver' => 'PhabricatorObjectMailReceiver', 'ManiphestTaskMergeInRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskMergedFromTransaction' => 'ManiphestTaskTransactionType', @@ -8754,6 +8759,7 @@ 'PhabricatorEditEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorEditEngineListController' => 'PhabricatorEditEngineController', 'PhabricatorEditEngineLock' => 'Phobject', + 'PhabricatorEditEngineMFAEngine' => 'Phobject', 'PhabricatorEditEnginePointsCommentAction' => 'PhabricatorEditEngineCommentAction', 'PhabricatorEditEngineProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorEditEngineQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -212,6 +212,8 @@ status. - `locked` //Optional bool.// Lock tasks in this status, preventing users from commenting. + - `mfa` //Optional bool.// Require all edits to this task to be signed with + multi-factor authentication. Statuses will appear in the UI in the order specified. Note the status marked `special` as `duplicate` is not settable directly and will not appear in UI diff --git a/src/applications/maniphest/constants/ManiphestTaskStatus.php b/src/applications/maniphest/constants/ManiphestTaskStatus.php --- a/src/applications/maniphest/constants/ManiphestTaskStatus.php +++ b/src/applications/maniphest/constants/ManiphestTaskStatus.php @@ -160,6 +160,10 @@ return self::getStatusAttribute($status, 'locked', false); } + public static function isMFAStatus($status) { + return self::getStatusAttribute($status, 'mfa', false); + } + public static function getStatusActionName($status) { return self::getStatusAttribute($status, 'name.action'); } @@ -282,6 +286,7 @@ 'disabled' => 'optional bool', 'claim' => 'optional bool', 'locked' => 'optional bool', + 'mfa' => 'optional bool', )); } diff --git a/src/applications/maniphest/engine/ManiphestTaskMFAEngine.php b/src/applications/maniphest/engine/ManiphestTaskMFAEngine.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/engine/ManiphestTaskMFAEngine.php @@ -0,0 +1,11 @@ +getObject()->getStatus(); + return ManiphestTaskStatus::isMFAStatus($status); + } + +} diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -19,7 +19,8 @@ PhabricatorFerretInterface, DoorkeeperBridgedObjectInterface, PhabricatorEditEngineSubtypeInterface, - PhabricatorEditEngineLockableInterface { + PhabricatorEditEngineLockableInterface, + PhabricatorEditEngineMFAInterface { const MARKUP_FIELD_DESCRIPTION = 'markup:desc'; @@ -619,4 +620,12 @@ return new ManiphestTaskFerretEngine(); } + +/* -( PhabricatorEditEngineMFAInterface )---------------------------------- */ + + + public function newEditEngineMFAEngine() { + return new ManiphestTaskMFAEngine(); + } + } diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -8,7 +8,7 @@ $phid = $request->getURIData('phid'); $action = $request->getURIData('action'); - if (!$request->isFormPost()) { + if (!$request->isFormOrHisecPost()) { return new Aphront400Response(); } @@ -73,6 +73,7 @@ $editor = id($object->getApplicationTransactionEditor()) ->setActor($viewer) + ->setCancelURI($handle->getURI()) ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true) ->setContentSourceFromRequest($request); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1041,7 +1041,7 @@ } $validation_exception = null; - if ($request->isFormPost() && $request->getBool('editEngine')) { + if ($request->isFormOrHisecPost() && $request->getBool('editEngine')) { $submit_fields = $fields; foreach ($submit_fields as $key => $field) { diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineMFAEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngineMFAEngine.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/editengine/PhabricatorEditEngineMFAEngine.php @@ -0,0 +1,39 @@ +object = $object; + return $this; + } + + public function getObject() { + return $this->object; + } + + public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + public function getViewer() { + if (!$this->viewer) { + throw new PhutilInvalidStateException('setViewer'); + } + + return $this->viewer; + } + + final public static function newEngineForObject( + PhabricatorEditEngineMFAInterface $object) { + return $object->newEditEngineMFAEngine() + ->setObject($object); + } + + abstract public function shouldRequireMFA(); + +} diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php b/src/applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php @@ -0,0 +1,7 @@ +isNewObject = ($object->getPHID() === null); $this->validateEditParameters($object, $xactions); + $xactions = $this->newMFATransactions($object, $xactions); $actor = $this->requireActor(); @@ -4825,11 +4826,22 @@ $request = $this->getRequest(); if ($request === null) { - throw new Exception( - pht( - 'This transaction group requires MFA to apply, but the Editor was '. - 'not configured with a Request. This workflow can not perform an '. - 'MFA check.')); + $source_type = $this->getContentSource()->getSourceTypeConstant(); + $conduit_type = PhabricatorConduitContentSource::SOURCECONST; + $is_conduit = ($source_type === $conduit_type); + if ($is_conduit) { + throw new Exception( + pht( + 'This transaction group requires MFA to apply, but you can not '. + 'provide an MFA response via Conduit. Edit this object via the '. + 'web UI.')); + } else { + throw new Exception( + pht( + 'This transaction group requires MFA to apply, but the Editor was '. + 'not configured with a Request. This workflow can not perform an '. + 'MFA check.')); + } } $cancel_uri = $this->getCancelURI(); @@ -4850,4 +4862,46 @@ } } + private function newMFATransactions( + PhabricatorLiskDAO $object, + array $xactions) { + + $is_mfa = ($object instanceof PhabricatorEditEngineMFAInterface); + if (!$is_mfa) { + return $xactions; + } + + $engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object) + ->setViewer($this->getActor()); + $require_mfa = $engine->shouldRequireMFA(); + + if (!$require_mfa) { + return $xactions; + } + + $type_mfa = PhabricatorTransactions::TYPE_MFA; + + $has_mfa = false; + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() === $type_mfa) { + $has_mfa = true; + break; + } + } + + if ($has_mfa) { + return $xactions; + } + + $template = $object->getApplicationTransactionTemplate(); + + $mfa_xaction = id(clone $template) + ->setTransactionType($type_mfa) + ->setNewValue(true); + + array_unshift($xactions, $mfa_xaction); + + return $xactions; + } + }