Update Conduit Maniphest CRUD API(s) to not accept crud

Summary: see T1241, T1242, T1244 for some examples of crud getting saved

Test Plan: threw some crud in my conduit console and got reasonable errors back

Reviewers: mikaaay, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1241, T1242, T1244

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

src/applications/conduit/method/maniphest/base/ConduitAPI_maniphest_Method.php

@@ -21,6 +21,12 @@
  */
 abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
 
+  public function defineErrorTypes() {
+    return array(
+      'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.'
+    );
+  }
+
   protected function buildTaskInfoDictionary(ManiphestTask $task) {
     $results = $this->buildTaskInfoDictionaries(array($task));
     return idx($results, $task->getPHID());
@@ -88,32 +94,54 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
 
       $status = $request->getValue('status');
       if ($status !== null) {
+        $valid_statuses = ManiphestTaskStatus::getTaskStatusMap();
+        if (!isset($valid_statuses[$status])) {
+          throw id(new ConduitException('ERR-INVALID-PARAMETER'))
+            ->setErrorDescription('Status set to invalid value.');
+        }
         $changes[ManiphestTransactionType::TYPE_STATUS] = $status;
       }
     }
 
     $priority = $request->getValue('priority');
     if ($priority !== null) {
+      $valid_priorities = ManiphestTaskPriority::getTaskPriorityMap();
+      if (!isset($valid_priorities[$priority])) {
+        throw id(new ConduitException('ERR-INVALID-PARAMETER'))
+          ->setErrorDescription('Priority set to invalid value.');
+      }
       $changes[ManiphestTransactionType::TYPE_PRIORITY] = $priority;
     }
 
     $owner_phid = $request->getValue('ownerPHID');
     if ($owner_phid !== null) {
+      $this->validatePHIDList(array($owner_phid),
+                              PhabricatorPHIDConstants::PHID_TYPE_USER,
+                              'ownerPHID');
       $changes[ManiphestTransactionType::TYPE_OWNER] = $owner_phid;
     }
 
     $ccs = $request->getValue('ccPHIDs');
     if ($ccs !== null) {
+      $this->validatePHIDList($ccs,
+                              PhabricatorPHIDConstants::PHID_TYPE_USER,
+                              'ccPHIDS');
       $changes[ManiphestTransactionType::TYPE_CCS] = $ccs;
     }
 
     $project_phids = $request->getValue('projectPHIDs');
     if ($project_phids !== null) {
+      $this->validatePHIDList($project_phids,
+                              PhabricatorPHIDConstants::PHID_TYPE_PROJ,
+                              'projectPHIDS');
       $changes[ManiphestTransactionType::TYPE_PROJECTS] = $project_phids;
     }
 
     $file_phids = $request->getValue('filePHIDs');
     if ($file_phids !== null) {
+      $this->validatePHIDList($file_phids,
+                              PhabricatorPHIDConstants::PHID_TYPE_FILE,
+                              'filePHIDS');
       $file_map = array_fill_keys($file_phids, true);
       $attached = $task->getAttached();
       $attached[PhabricatorPHIDConstants::PHID_TYPE_FILE] = $file_map;
@@ -223,4 +251,24 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
     return $result;
   }
 
+  /**
+   * Note this is a temporary stop gap since its easy to make malformed Tasks.
+   * Long-term, the values set in @{method:defineParamTypes} will be used to
+   * validate data implicitly within the larger Conduit application.
+   *
+   * TODO -- remove this in favor of generalized Conduit hotness
+   */
+  private function validatePHIDList(array $phid_list, $phid_type, $field) {
+    $phid_groups = phid_group_by_type($phid_list);
+    unset($phid_groups[$phid_type]);
+    if (!empty($phid_groups)) {
+      throw id(new ConduitException('ERR-INVALID-PARAMETER'))
+        ->setErrorDescription(
+          'One or more PHIDs were invalid for '.$field.'.'
+      );
+    }
+
+    return true;
+  }
+
 }

src/applications/conduit/method/maniphest/base/__init__.php

@@ -7,6 +7,7 @@
 
 
 phutil_require_module('phabricator', 'applications/conduit/method/base');
+phutil_require_module('phabricator', 'applications/conduit/protocol/exception');
 phutil_require_module('phabricator', 'applications/maniphest/constants/priority');
 phutil_require_module('phabricator', 'applications/maniphest/constants/status');
 phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
@@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/auxiliary')
 phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
 phutil_require_module('phabricator', 'applications/metamta/contentsource/source');
 phutil_require_module('phabricator', 'applications/phid/constants');
+phutil_require_module('phabricator', 'applications/phid/utils');
 phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'infrastructure/events/constant/type');
 phutil_require_module('phabricator', 'infrastructure/events/event');

src/applications/conduit/method/maniphest/createtask/ConduitAPI_maniphest_createtask_Method.php

@@ -36,6 +36,7 @@ final class ConduitAPI_maniphest_createtask_Method
 
   public function defineErrorTypes() {
     return array(
+      'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.'
     );
   }
 

src/applications/conduit/method/maniphest/update/ConduitAPI_maniphest_update_Method.php

@@ -26,6 +26,13 @@ final class ConduitAPI_maniphest_update_Method
     return "Update an existing Maniphest task.";
   }
 
+  public function defineErrorTypes() {
+    return array(
+      'ERR-BAD-TASK'          => 'No such maniphest task exists.',
+      'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.'
+    );
+  }
+
   public function defineParamTypes() {
     return $this->getTaskFields($is_new = false);
   }
@@ -34,12 +41,6 @@ final class ConduitAPI_maniphest_update_Method
     return 'nonempty dict';
   }
 
-  public function defineErrorTypes() {
-    return array(
-      'ERR-BAD-TASK'  => 'No such task exists.',
-    );
-  }
-
   protected function execute(ConduitAPIRequest $request) {
     $id = $request->getValue('id');
     $phid = $request->getValue('phid');