Page MenuHomePhabricator
Files F15378752

D20340.diff
No OneTemporary
Actions

D20340.diff
View Options

diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
@@ -29,7 +29,9 @@
$handles = $viewer->loadHandles(array($phid));
$obj_handle = $handles[$phid];
- if ($request->isDialogFormPost()) {
+ $done_uri = $obj_handle->getURI();
+
+ if ($request->isFormOrHisecPost()) {
$text = $request->getStr('text');
$comment = $xaction->getApplicationTransactionCommentObject();
@@ -41,29 +43,42 @@
$editor = id(new PhabricatorApplicationTransactionCommentEditor())
->setActor($viewer)
->setContentSource(PhabricatorContentSource::newFromRequest($request))
+ ->setRequest($request)
+ ->setCancelURI($done_uri)
->applyEdit($xaction, $comment);
if ($request->isAjax()) {
return id(new AphrontAjaxResponse())->setContent(array());
} else {
- return id(new AphrontReloadResponse())->setURI($obj_handle->getURI());
+ return id(new AphrontReloadResponse())->setURI($done_uri);
}
}
+ $errors = array();
+ if ($xaction->getIsMFATransaction()) {
+ $message = pht(
+ 'This comment was signed with MFA, so you will be required to '.
+ 'provide MFA credentials to make changes.');
+
+ $errors[] = id(new PHUIInfoView())
+ ->setSeverity(PHUIInfoView::SEVERITY_MFA)
+ ->setErrors(array($message));
+ }
+
$form = id(new AphrontFormView())
->setUser($viewer)
->setFullWidth(true)
->appendControl(
id(new PhabricatorRemarkupControl())
- ->setName('text')
- ->setValue($xaction->getComment()->getContent()));
+ ->setName('text')
+ ->setValue($xaction->getComment()->getContent()));
return $this->newDialog()
->setTitle(pht('Edit Comment'))
- ->addHiddenInput('anchor', $request->getStr('anchor'))
+ ->appendChild($errors)
->appendForm($form)
->addSubmitButton(pht('Save Changes'))
- ->addCancelButton($obj_handle->getURI());
+ ->addCancelButton($done_uri);
}
}
diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php
@@ -30,20 +30,24 @@
->withPHIDs(array($obj_phid))
->executeOne();
- if ($request->isDialogFormPost()) {
+ $done_uri = $obj_handle->getURI();
+
+ if ($request->isFormOrHisecPost()) {
$comment = $xaction->getApplicationTransactionCommentObject()
->setContent('')
->setIsRemoved(true);
$editor = id(new PhabricatorApplicationTransactionCommentEditor())
->setActor($viewer)
+ ->setRequest($request)
+ ->setCancelURI($done_uri)
->setContentSource(PhabricatorContentSource::newFromRequest($request))
->applyEdit($xaction, $comment);
if ($request->isAjax()) {
return id(new AphrontAjaxResponse())->setContent(array());
} else {
- return id(new AphrontReloadResponse())->setURI($obj_handle->getURI());
+ return id(new AphrontReloadResponse())->setURI($done_uri);
}
}
@@ -54,7 +58,6 @@
->setTitle(pht('Remove Comment'));
$dialog
- ->addHiddenInput('anchor', $request->getStr('anchor'))
->appendParagraph(
pht(
"Removing a comment prevents anyone (including you) from reading ".
@@ -65,7 +68,7 @@
$dialog
->addSubmitButton(pht('Remove Comment'))
- ->addCancelButton($obj_handle->getURI());
+ ->addCancelButton($done_uri);
return $dialog;
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
@@ -5,6 +5,9 @@
private $contentSource;
private $actingAsPHID;
+ private $request;
+ private $cancelURI;
+ private $isNewComment;
public function setActingAsPHID($acting_as_phid) {
$this->actingAsPHID = $acting_as_phid;
@@ -27,6 +30,33 @@
return $this->contentSource;
}
+ public function setRequest(AphrontRequest $request) {
+ $this->request = $request;
+ return $this;
+ }
+
+ public function getRequest() {
+ return $this->request;
+ }
+
+ public function setCancelURI($cancel_uri) {
+ $this->cancelURI = $cancel_uri;
+ return $this;
+ }
+
+ public function getCancelURI() {
+ return $this->cancelURI;
+ }
+
+ public function setIsNewComment($is_new) {
+ $this->isNewComment = $is_new;
+ return $this;
+ }
+
+ public function getIsNewComment() {
+ return $this->isNewComment;
+ }
+
/**
* Edit a transaction's comment. This method effects the required create,
* update or delete to set the transaction's comment to the provided comment.
@@ -39,6 +69,8 @@
$actor = $this->requireActor();
+ $this->applyMFAChecks($xaction, $comment);
+
$comment->setContentSource($this->getContentSource());
$comment->setAuthorPHID($this->getActingAsPHID());
@@ -160,5 +192,94 @@
}
}
+ private function applyMFAChecks(
+ PhabricatorApplicationTransaction $xaction,
+ PhabricatorApplicationTransactionComment $comment) {
+ $actor = $this->requireActor();
+
+ // We don't do any MFA checks here when you're creating a comment for the
+ // first time (the parent editor handles them for us), so we can just bail
+ // out if this is the creation flow.
+ if ($this->getIsNewComment()) {
+ return;
+ }
+
+ $request = $this->getRequest();
+ if (!$request) {
+ throw new PhutilInvalidStateException('setRequest');
+ }
+
+ $cancel_uri = $this->getCancelURI();
+ if (!strlen($cancel_uri)) {
+ throw new PhutilInvalidStateException('setCancelURI');
+ }
+
+ // If you're deleting a comment, we try to prompt you for MFA if you have
+ // it configured, but do not require that you have it configured. In most
+ // cases, this is administrators removing content.
+
+ // See PHI1173. If you're editing a comment you authored and the original
+ // comment was signed with MFA, you MUST have MFA on your account and you
+ // MUST sign the edit with MFA. Otherwise, we can end up with an MFA badge
+ // on different content than what was signed.
+
+ $want_mfa = false;
+ $need_mfa = false;
+
+ if ($comment->getIsRemoved()) {
+ // Try to prompt on removal.
+ $want_mfa = true;
+ }
+
+ if ($xaction->getIsMFATransaction()) {
+ if ($actor->getPHID() === $xaction->getAuthorPHID()) {
+ // Strictly require MFA if the original transaction was signed and
+ // you're the author.
+ $want_mfa = true;
+ $need_mfa = true;
+ }
+ }
+
+ if (!$want_mfa) {
+ return;
+ }
+
+ if ($need_mfa) {
+ $factors = id(new PhabricatorAuthFactorConfigQuery())
+ ->setViewer($actor)
+ ->withUserPHIDs(array($this->getActingAsPHID()))
+ ->withFactorProviderStatuses(
+ array(
+ PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE,
+ PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED,
+ ))
+ ->execute();
+ if (!$factors) {
+ $error = new PhabricatorApplicationTransactionValidationError(
+ $xaction->getTransactionType(),
+ pht('No MFA'),
+ pht(
+ 'This comment was signed with MFA, so edits to it must also be '.
+ 'signed with MFA. You do not have any MFA factors attached to '.
+ 'your account, so you can not sign this edit. Add MFA to your '.
+ 'account in Settings.'),
+ $xaction);
+
+ throw new PhabricatorApplicationTransactionValidationException(
+ array(
+ $error,
+ ));
+ }
+ }
+
+ $workflow_key = sprintf(
+ 'comment.edit(%s, %d)',
+ $xaction->getPHID(),
+ $xaction->getComment()->getID());
+
+ $hisec_token = id(new PhabricatorAuthSessionEngine())
+ ->setWorkflowKey($workflow_key)
+ ->requireHighSecurityToken($actor, $request, $cancel_uri);
+ }
}
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
@@ -1113,7 +1113,8 @@
$comment_editor = id(new PhabricatorApplicationTransactionCommentEditor())
->setActor($actor)
->setActingAsPHID($this->getActingAsPHID())
- ->setContentSource($this->getContentSource());
+ ->setContentSource($this->getContentSource())
+ ->setIsNewComment(true);
if (!$transaction_open) {
$object->openTransaction();

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 14, 4:46 PM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7662508
Default Alt Text
D20340.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility