Page MenuHomePhabricator

D19899.diff
No OneTemporary

D19899.diff

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 @@
+<?php
+
+final class ManiphestTaskMFAEngine
+ extends PhabricatorEditEngineMFAEngine {
+
+ public function shouldRequireMFA() {
+ $status = $this->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 @@
+<?php
+
+abstract class PhabricatorEditEngineMFAEngine
+ extends Phobject {
+
+ private $object;
+ private $viewer;
+
+ public function setObject(PhabricatorEditEngineMFAInterface $object) {
+ $this->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 @@
+<?php
+
+interface PhabricatorEditEngineMFAInterface {
+
+ public function newEditEngineMFAEngine();
+
+}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -953,6 +953,7 @@
$this->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;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 16, 9:02 AM (2 d, 22 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6738598
Default Alt Text
D19899.diff (11 KB)

Event Timeline