Maniphest - prevent uneditable tasks from being able to be closed as duplicates

Summary:
Fixes T7923.

Prevent the user from finding tasks that they can't edit in merge workflows. Also ensure that we query properly on final merge action just in case.

Test Plan: Tried to find a task I couldn't edit in various searches under the "merge" dialogue and couldn't find the task. Removed this big of code and tried to merge in a task and after hitting "merge" observed the page reloaded with no task merged in.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7923

Differential Revision: https://secure.phabricator.com/D12872

src/applications/phid/query/PhabricatorHandleQuery.php

@@ -3,6 +3,7 @@
 final class PhabricatorHandleQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
+  private $objectCapabilities;
   private $phids = array();
 
   public function withPHIDs(array $phids) {
@@ -10,6 +11,18 @@ final class PhabricatorHandleQuery
     return $this;
   }
 
+  public function requireObjectCapabilities(array $capabilities) {
+    $this->objectCapabilities = $capabilities;
+    return $this;
+  }
+
+  protected function getRequiredObjectCapabilities() {
+    if ($this->objectCapabilities) {
+      return $this->objectCapabilities;
+    }
+    return $this->getRequiredCapabilities();
+  }
+
   protected function loadPage() {
     $types = PhabricatorPHIDType::getAllTypes();
 
@@ -20,6 +33,7 @@ final class PhabricatorHandleQuery
 
     $object_query = id(new PhabricatorObjectQuery())
       ->withPHIDs($phids)
+      ->requireCapabilities($this->getRequiredObjectCapabilities())
       ->setViewer($this->getViewer());
 
     $objects = $object_query->execute();

src/applications/search/application/PhabricatorSearchApplication.php

@@ -32,7 +32,7 @@ final class PhabricatorSearchApplication extends PhabricatorApplication {
         '(?:query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorSearchController',
         'attach/(?P<phid>[^/]+)/(?P<type>\w+)/(?:(?P<action>\w+)/)?'
           => 'PhabricatorSearchAttachController',
-        'select/(?P<type>\w+)/'
+        'select/(?P<type>\w+)/(?:(?P<action>\w+)/)?'
           => 'PhabricatorSearchSelectController',
         'index/(?P<phid>[^/]+)/' => 'PhabricatorSearchIndexController',
         'hovercard/(?P<mode>retrieve|test)/'

src/applications/search/controller/PhabricatorSearchAttachController.php

@@ -3,38 +3,22 @@
 final class PhabricatorSearchAttachController
   extends PhabricatorSearchBaseController {
 
-  private $phid;
-  private $type;
-  private $action;
-
-  const ACTION_ATTACH       = 'attach';
-  const ACTION_MERGE        = 'merge';
-  const ACTION_DEPENDENCIES = 'dependencies';
-  const ACTION_BLOCKS       = 'blocks';
-  const ACTION_EDGE         = 'edge';
-
-  public function willProcessRequest(array $data) {
-    $this->phid = $data['phid'];
-    $this->type = $data['type'];
-    $this->action = idx($data, 'action', self::ACTION_ATTACH);
-  }
-
-  public function processRequest() {
-
-    $request = $this->getRequest();
-    $user = $request->getUser();
+  public function handleRequest(AphrontRequest $request) {
+    $user        = $request->getUser();
+    $phid        = $request->getURIData('phid');
+    $attach_type = $request->getURIData('type');
+    $action      = $request->getURIData('action', self::ACTION_ATTACH);
 
     $handle = id(new PhabricatorHandleQuery())
       ->setViewer($user)
-      ->withPHIDs(array($this->phid))
+      ->withPHIDs(array($phid))
       ->executeOne();
 
     $object_type = $handle->getType();
-    $attach_type = $this->type;
 
     $object = id(new PhabricatorObjectQuery())
       ->setViewer($user)
-      ->withPHIDs(array($this->phid))
+      ->withPHIDs(array($phid))
       ->executeOne();
 
     if (!$object) {
@@ -42,7 +26,7 @@ final class PhabricatorSearchAttachController
     }
 
     $edge_type = null;
-    switch ($this->action) {
+    switch ($action) {
       case self::ACTION_EDGE:
       case self::ACTION_DEPENDENCIES:
       case self::ACTION_BLOCKS:
@@ -66,7 +50,7 @@ final class PhabricatorSearchAttachController
         }
 
         $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-          $this->phid,
+          $phid,
           $edge_type);
         $add_phids = $phids;
         $rem_phids = array_diff($old_phids, $add_phids);
@@ -100,7 +84,7 @@ final class PhabricatorSearchAttachController
     } else {
       if ($edge_type) {
         $phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-          $this->phid,
+          $phid,
           $edge_type);
       } else {
         // This is a merge.
@@ -108,7 +92,7 @@ final class PhabricatorSearchAttachController
       }
     }
 
-    $strings = $this->getStrings();
+    $strings = $this->getStrings($attach_type, $action);
 
     $handles = $this->loadViewerHandles($phids);
 
@@ -116,11 +100,11 @@ final class PhabricatorSearchAttachController
     $obj_dialog
       ->setUser($user)
       ->setHandles($handles)
-      ->setFilters($this->getFilters($strings))
+      ->setFilters($this->getFilters($strings, $attach_type))
       ->setSelectedFilter($strings['selected'])
-      ->setExcluded($this->phid)
+      ->setExcluded($phid)
       ->setCancelURI($handle->getURI())
-      ->setSearchURI('/search/select/'.$attach_type.'/')
+      ->setSearchURI('/search/select/'.$attach_type.'/'.$action.'/')
       ->setTitle($strings['title'])
       ->setHeader($strings['header'])
       ->setButtonText($strings['button'])
@@ -148,6 +132,11 @@ final class PhabricatorSearchAttachController
 
     $targets = id(new ManiphestTaskQuery())
       ->setViewer($user)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
       ->withPHIDs(array_keys($phids))
       ->needSubscriberPHIDs(true)
       ->needProjectPHIDs(true)
@@ -208,8 +197,8 @@ final class PhabricatorSearchAttachController
     return $response;
   }
 
-  private function getStrings() {
-    switch ($this->type) {
+  private function getStrings($attach_type, $action) {
+    switch ($attach_type) {
       case DifferentialRevisionPHIDType::TYPECONST:
         $noun = 'Revisions';
         $selected = 'created';
@@ -228,7 +217,7 @@ final class PhabricatorSearchAttachController
         break;
     }
 
-    switch ($this->action) {
+    switch ($action) {
       case self::ACTION_EDGE:
       case self::ACTION_ATTACH:
         $dialog_title = "Manage Attached {$noun}";
@@ -268,8 +257,8 @@ final class PhabricatorSearchAttachController
     );
   }
 
-  private function getFilters(array $strings) {
-    if ($this->type == PholioMockPHIDType::TYPECONST) {
+  private function getFilters(array $strings, $attach_type) {
+    if ($attach_type == PholioMockPHIDType::TYPECONST) {
       $filters = array(
         'created' => 'Created By Me',
         'all' => 'All '.$strings['target_plural_noun'],

src/applications/search/controller/PhabricatorSearchBaseController.php

@@ -2,6 +2,12 @@
 
 abstract class PhabricatorSearchBaseController extends PhabricatorController {
 
+  const ACTION_ATTACH       = 'attach';
+  const ACTION_MERGE        = 'merge';
+  const ACTION_DEPENDENCIES = 'dependencies';
+  const ACTION_BLOCKS       = 'blocks';
+  const ACTION_EDGE         = 'edge';
+
   public function buildStandardPageResponse($view, array $data) {
     $page = $this->buildStandardPageView();
 

src/applications/search/controller/PhabricatorSearchSelectController.php

@@ -3,22 +3,17 @@
 final class PhabricatorSearchSelectController
   extends PhabricatorSearchBaseController {
 
-  private $type;
-
-  public function willProcessRequest(array $data) {
-    $this->type = $data['type'];
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
+  public function handleRequest(AphrontRequest $request) {
     $user = $request->getUser();
+    $type = $request->getURIData('type');
+    $action = $request->getURIData('action');
 
     $query = new PhabricatorSavedQuery();
     $query_str = $request->getStr('query');
 
     $query->setEngineClassName('PhabricatorSearchApplicationSearchEngine');
     $query->setParameter('query', $query_str);
-    $query->setParameter('types', array($this->type));
+    $query->setParameter('types', array($type));
 
     $status_open = PhabricatorSearchRelationship::RELATIONSHIP_OPEN;
 
@@ -31,7 +26,7 @@ final class PhabricatorSearchSelectController
         $query->setParameter('authorPHIDs', array($user->getPHID()));
         // TODO - if / when we allow pholio mocks to be archived, etc
         // update this
-        if ($this->type != PholioMockPHIDType::TYPECONST) {
+        if ($type != PholioMockPHIDType::TYPECONST) {
           $query->setParameter('statuses', array($status_open));
         }
         break;
@@ -42,15 +37,25 @@ final class PhabricatorSearchSelectController
 
     $query->setParameter('excludePHIDs', array($request->getStr('exclude')));
 
+    $capabilities = array(PhabricatorPolicyCapability::CAN_VIEW);
+    switch ($action) {
+      case self::ACTION_MERGE:
+        $capabilities[] = PhabricatorPolicyCapability::CAN_EDIT;
+        break;
+      default:
+        break;
+    }
+
     $results = id(new PhabricatorSearchDocumentQuery())
       ->setViewer($user)
+      ->requireObjectCapabilities($capabilities)
       ->withSavedQuery($query)
       ->setOffset(0)
       ->setLimit(100)
       ->execute();
 
     $phids = array_fill_keys(mpull($results, 'getPHID'), true);
-    $phids += $this->queryObjectNames($query_str);
+    $phids += $this->queryObjectNames($query_str, $capabilities);
 
     $phids = array_keys($phids);
     $handles = $this->loadViewerHandles($phids);
@@ -64,12 +69,14 @@ final class PhabricatorSearchSelectController
     return id(new AphrontAjaxResponse())->setContent($data);
   }
 
-  private function queryObjectNames($query) {
-    $viewer = $this->getRequest()->getUser();
+  private function queryObjectNames($query, $capabilities) {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
 
     $objects = id(new PhabricatorObjectQuery())
       ->setViewer($viewer)
-      ->withTypes(array($this->type))
+      ->requireCapabilities($capabilities)
+      ->withTypes(array($request->getURIData('type')))
       ->withNames(array($query))
       ->execute();
 

src/applications/search/query/PhabricatorSearchDocumentQuery.php

@@ -4,17 +4,31 @@ final class PhabricatorSearchDocumentQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
   private $savedQuery;
+  private $objectCapabilities;
 
   public function withSavedQuery(PhabricatorSavedQuery $query) {
     $this->savedQuery = $query;
     return $this;
   }
 
+  public function requireObjectCapabilities(array $capabilities) {
+    $this->objectCapabilities = $capabilities;
+    return $this;
+  }
+
+  protected function getRequiredObjectCapabilities() {
+    if ($this->objectCapabilities) {
+      return $this->objectCapabilities;
+    }
+    return $this->getRequiredCapabilities();
+  }
+
   protected function loadPage() {
     $phids = $this->loadDocumentPHIDsWithoutPolicyChecks();
 
     $handles = id(new PhabricatorHandleQuery())
       ->setViewer($this->getViewer())
+      ->requireObjectCapabilities($this->getRequiredObjectCapabilities())
       ->withPHIDs($phids)
       ->execute();