Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -705,8 +705,14 @@ 'LiskMigrationIterator' => 'infrastructure/storage/lisk/LiskMigrationIterator.php', 'LiskRawMigrationIterator' => 'infrastructure/storage/lisk/LiskRawMigrationIterator.php', 'ManiphestBatchEditController' => 'applications/maniphest/controller/ManiphestBatchEditController.php', + 'ManiphestCapabilityBulkEdit' => 'applications/maniphest/capability/ManiphestCapabilityBulkEdit.php', 'ManiphestCapabilityDefaultEdit' => 'applications/maniphest/capability/ManiphestCapabilityDefaultEdit.php', 'ManiphestCapabilityDefaultView' => 'applications/maniphest/capability/ManiphestCapabilityDefaultView.php', + 'ManiphestCapabilityEditAssign' => 'applications/maniphest/capability/ManiphestCapabilityEditAssign.php', + 'ManiphestCapabilityEditPolicies' => 'applications/maniphest/capability/ManiphestCapabilityEditPolicies.php', + 'ManiphestCapabilityEditPriority' => 'applications/maniphest/capability/ManiphestCapabilityEditPriority.php', + 'ManiphestCapabilityEditProjects' => 'applications/maniphest/capability/ManiphestCapabilityEditProjects.php', + 'ManiphestCapabilityEditStatus' => 'applications/maniphest/capability/ManiphestCapabilityEditStatus.php', 'ManiphestConfiguredCustomField' => 'applications/maniphest/field/ManiphestConfiguredCustomField.php', 'ManiphestConstants' => 'applications/maniphest/constants/ManiphestConstants.php', 'ManiphestController' => 'applications/maniphest/controller/ManiphestController.php', @@ -2837,8 +2843,14 @@ 'LiskMigrationIterator' => 'PhutilBufferedIterator', 'LiskRawMigrationIterator' => 'PhutilBufferedIterator', 'ManiphestBatchEditController' => 'ManiphestController', + 'ManiphestCapabilityBulkEdit' => 'PhabricatorPolicyCapability', 'ManiphestCapabilityDefaultEdit' => 'PhabricatorPolicyCapability', 'ManiphestCapabilityDefaultView' => 'PhabricatorPolicyCapability', + 'ManiphestCapabilityEditAssign' => 'PhabricatorPolicyCapability', + 'ManiphestCapabilityEditPolicies' => 'PhabricatorPolicyCapability', + 'ManiphestCapabilityEditPriority' => 'PhabricatorPolicyCapability', + 'ManiphestCapabilityEditProjects' => 'PhabricatorPolicyCapability', + 'ManiphestCapabilityEditStatus' => 'PhabricatorPolicyCapability', 'ManiphestConfiguredCustomField' => array( 0 => 'ManiphestCustomField', Index: src/applications/maniphest/application/PhabricatorApplicationManiphest.php =================================================================== --- src/applications/maniphest/application/PhabricatorApplicationManiphest.php +++ src/applications/maniphest/application/PhabricatorApplicationManiphest.php @@ -67,7 +67,7 @@ 'export/(?P[^/]+)/' => 'ManiphestExportController', 'subpriority/' => 'ManiphestSubpriorityController', 'subscribe/(?Padd|rem)/(?P[1-9]\d*)/' - => 'ManiphestSubscribeController', + => 'ManiphestSubscribeController', ), ); } @@ -100,6 +100,18 @@ 'caption' => pht( 'Default edit policy for newly created tasks.'), ), + ManiphestCapabilityEditStatus::CAPABILITY => array( + ), + ManiphestCapabilityEditAssign::CAPABILITY => array( + ), + ManiphestCapabilityEditPolicies::CAPABILITY => array( + ), + ManiphestCapabilityEditPriority::CAPABILITY => array( + ), + ManiphestCapabilityEditProjects::CAPABILITY => array( + ), + ManiphestCapabilityBulkEdit::CAPABILITY => array( + ), ); } Index: src/applications/maniphest/capability/ManiphestCapabilityBulkEdit.php =================================================================== --- /dev/null +++ src/applications/maniphest/capability/ManiphestCapabilityBulkEdit.php @@ -0,0 +1,20 @@ +requireApplicationCapability( + ManiphestCapabilityBulkEdit::CAPABILITY); + $request = $this->getRequest(); $user = $request->getUser(); Index: src/applications/maniphest/controller/ManiphestTaskDetailController.php =================================================================== --- src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -165,6 +165,27 @@ ManiphestTransaction::TYPE_PROJECTS => pht('Associate Projects'), ); + // Remove actions the user doesn't have permission to take. + + $requires = array( + ManiphestTransaction::TYPE_OWNER => + ManiphestCapabilityEditAssign::CAPABILITY, + ManiphestTransaction::TYPE_PRIORITY => + ManiphestCapabilityEditPriority::CAPABILITY, + ManiphestTransaction::TYPE_PROJECTS => + ManiphestCapabilityEditProjects::CAPABILITY, + ManiphestTransaction::TYPE_STATUS => + ManiphestCapabilityEditStatus::CAPABILITY, + ); + + foreach ($transaction_types as $type => $name) { + if (isset($requires[$type])) { + if (!$this->hasApplicationCapability($requires[$type])) { + unset($transaction_types[$type]); + } + } + } + if ($task->getStatus() == ManiphestTaskStatus::STATUS_OPEN) { $resolution_types = array_select_keys( $resolution_types, Index: src/applications/maniphest/controller/ManiphestTaskEditController.php =================================================================== --- src/applications/maniphest/controller/ManiphestTaskEditController.php +++ src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -1,8 +1,5 @@ getRequest(); $user = $request->getUser(); + $can_edit_assign = $this->hasApplicationCapability( + ManiphestCapabilityEditAssign::CAPABILITY); + $can_edit_policies = $this->hasApplicationCapability( + ManiphestCapabilityEditPolicies::CAPABILITY); + $can_edit_priority = $this->hasApplicationCapability( + ManiphestCapabilityEditPriority::CAPABILITY); + $can_edit_projects = $this->hasApplicationCapability( + ManiphestCapabilityEditProjects::CAPABILITY); + $can_edit_status = $this->hasApplicationCapability( + ManiphestCapabilityEditStatus::CAPABILITY); + $files = array(); $parent_task = null; $template_id = null; @@ -40,20 +48,24 @@ if (!$request->isFormPost()) { $task->setTitle($request->getStr('title')); - $default_projects = $request->getStr('projects'); - if ($default_projects) { - $task->setProjectPHIDs(explode(';', $default_projects)); + if ($can_edit_projects) { + $default_projects = $request->getStr('projects'); + if ($default_projects) { + $task->setProjectPHIDs(explode(';', $default_projects)); + } } $task->setDescription($request->getStr('description')); - $assign = $request->getStr('assign'); - if (strlen($assign)) { - $assign_user = id(new PhabricatorUser())->loadOneWhere( - 'username = %s', - $assign); - if ($assign_user) { - $task->setOwnerPHID($assign_user->getPHID()); + if ($can_edit_assign) { + $assign = $request->getStr('assign'); + if (strlen($assign)) { + $assign_user = id(new PhabricatorUser())->loadOneWhere( + 'username = %s', + $assign); + if ($assign_user) { + $task->setOwnerPHID($assign_user->getPHID()); + } } } } @@ -122,7 +134,9 @@ $changes[ManiphestTransaction::TYPE_TITLE] = $new_title; $changes[ManiphestTransaction::TYPE_DESCRIPTION] = $new_desc; - $changes[ManiphestTransaction::TYPE_STATUS] = $new_status; + if ($can_edit_status) { + $changes[ManiphestTransaction::TYPE_STATUS] = $new_status; + } $owner_tokenizer = $request->getArr('assigned_to'); $owner_phid = reset($owner_tokenizer); @@ -173,17 +187,27 @@ $task->setProjectPHIDs($request->getArr('projects')); } else { - $changes[ManiphestTransaction::TYPE_PRIORITY] = - $request->getInt('priority'); - $changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid; + if ($can_edit_priority) { + $changes[ManiphestTransaction::TYPE_PRIORITY] = + $request->getInt('priority'); + } + if ($can_edit_assign) { + $changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid; + } + $changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc'); - $changes[ManiphestTransaction::TYPE_PROJECTS] = - $request->getArr('projects'); - $changes[PhabricatorTransactions::TYPE_VIEW_POLICY] = - $request->getStr('viewPolicy'); - $changes[PhabricatorTransactions::TYPE_EDIT_POLICY] = - $request->getStr('editPolicy'); + if ($can_edit_projects) { + $changes[ManiphestTransaction::TYPE_PROJECTS] = + $request->getArr('projects'); + } + + if ($can_edit_policies) { + $changes[PhabricatorTransactions::TYPE_VIEW_POLICY] = + $request->getStr('viewPolicy'); + $changes[PhabricatorTransactions::TYPE_EDIT_POLICY] = + $request->getStr('editPolicy'); + } if ($files) { $file_map = mpull($files, 'getPHID'); @@ -418,7 +442,7 @@ ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) ->setValue($task->getTitle())); - if ($task->getID()) { + if ($task->getID() && $can_edit_status) { // Only show this in "edit" mode, not "create" mode, since creating a // non-open task is kind of silly and it would just clutter up the // "create" interface. @@ -436,58 +460,73 @@ ->setObject($task) ->execute(); - $form - ->appendChild( + if ($can_edit_assign) { + $form->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Assigned To')) ->setName('assigned_to') ->setValue($assigned_value) ->setUser($user) ->setDatasource('/typeahead/common/users/') - ->setLimit(1)) + ->setLimit(1)); + } + + $form ->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('CC')) ->setName('cc') ->setValue($cc_value) ->setUser($user) - ->setDatasource('/typeahead/common/mailable/')) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Priority')) - ->setName('priority') - ->setOptions($priority_map) - ->setValue($task->getPriority())) - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setUser($user) - ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) - ->setPolicyObject($task) - ->setPolicies($policies) - ->setName('viewPolicy')) - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setUser($user) - ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) - ->setPolicyObject($task) - ->setPolicies($policies) - ->setName('editPolicy')) - ->appendChild( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Projects')) - ->setName('projects') - ->setValue($projects_value) - ->setID($project_tokenizer_id) - ->setCaption( - javelin_tag( - 'a', - array( - 'href' => '/project/create/', - 'mustcapture' => true, - 'sigil' => 'project-create', - ), - pht('Create New Project'))) - ->setDatasource('/typeahead/common/projects/')); + ->setDatasource('/typeahead/common/mailable/')); + + if ($can_edit_priority) { + $form + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Priority')) + ->setName('priority') + ->setOptions($priority_map) + ->setValue($task->getPriority())); + } + + if ($can_edit_policies) { + $form + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setUser($user) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) + ->setPolicyObject($task) + ->setPolicies($policies) + ->setName('viewPolicy')) + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setUser($user) + ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) + ->setPolicyObject($task) + ->setPolicies($policies) + ->setName('editPolicy')); + } + + if ($can_edit_projects) { + $form + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Projects')) + ->setName('projects') + ->setValue($projects_value) + ->setID($project_tokenizer_id) + ->setCaption( + javelin_tag( + 'a', + array( + 'href' => '/project/create/', + 'mustcapture' => true, + 'sigil' => 'project-create', + ), + pht('Create New Project'))) + ->setDatasource('/typeahead/common/projects/')); + } foreach ($aux_fields as $aux_field) { $aux_control = $aux_field->renderEditControl(); Index: src/applications/maniphest/controller/ManiphestTaskListController.php =================================================================== --- src/applications/maniphest/controller/ManiphestTaskListController.php +++ src/applications/maniphest/controller/ManiphestTaskListController.php @@ -46,7 +46,11 @@ $group_parameter, $handles); + $can_edit_priority = $this->hasApplicationCapability( + ManiphestCapabilityEditPriority::CAPABILITY); + $can_drag = ($order_parameter == 'priority') && + ($can_edit_priority) && ($group_parameter == 'none' || $group_parameter == 'priority'); if (!$viewer->isLoggedIn()) { @@ -91,11 +95,13 @@ )); } - Javelin::initBehavior( - 'maniphest-subpriority-editor', - array( - 'uri' => '/maniphest/subpriority/', - )); + if ($can_drag) { + Javelin::initBehavior( + 'maniphest-subpriority-editor', + array( + 'uri' => '/maniphest/subpriority/', + )); + } return phutil_tag( 'div', @@ -191,6 +197,11 @@ private function renderBatchEditor(PhabricatorSavedQuery $saved_query) { $user = $this->getRequest()->getUser(); + $batch_capability = ManiphestCapabilityBulkEdit::CAPABILITY; + if (!$this->hasApplicationCapability($batch_capability)) { + return null; + } + if (!$user->isLoggedIn()) { // Don't show the batch editor or excel export for logged-out users. // Technically we //could// let them export, but ehh. Index: src/applications/maniphest/editor/ManiphestTransactionEditor.php =================================================================== --- src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -81,9 +81,9 @@ case ManiphestTransaction::TYPE_EDGE: return $xaction->getNewValue(); } - } + protected function transactionHasEffect( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { @@ -249,6 +249,43 @@ } } + protected function requireCapabilities( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + parent::requireCapabilities($object, $xaction); + + $app_capability_map = array( + ManiphestTransaction::TYPE_PRIORITY => + ManiphestCapabilityEditPriority::CAPABILITY, + ManiphestTransaction::TYPE_STATUS => + ManiphestCapabilityEditStatus::CAPABILITY, + ManiphestTransaction::TYPE_PROJECTS => + ManiphestCapabilityEditProjects::CAPABILITY, + ManiphestTransaction::TYPE_OWNER => + ManiphestCapabilityEditAssign::CAPABILITY, + PhabricatorTransactions::TYPE_EDIT_POLICY => + ManiphestCapabilityEditPolicies::CAPABILITY, + PhabricatorTransactions::TYPE_VIEW_POLICY => + ManiphestCapabilityEditPolicies::CAPABILITY, + ); + + $transaction_type = $xaction->getTransactionType(); + $app_capability = idx($app_capability_map, $transaction_type); + + if ($app_capability) { + $app = id(new PhabricatorApplicationQuery()) + ->setViewer($this->getActor()) + ->withClasses(array('PhabricatorApplicationManiphest')) + ->executeOne(); + PhabricatorPolicyFilter::requireCapability( + $this->getActor(), + $app, + $app_capability); + } + } + + public static function getNextSubpriority($pri, $sub) { // TODO: T603 Figure out what the policies here should be once this gets Index: src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php =================================================================== --- src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -460,6 +460,12 @@ return array(); } + // Now that we've merged, filtered, and combined transactions, check for + // required capabilities. + foreach ($xactions as $xaction) { + $this->requireCapabilities($object, $xaction); + } + $xactions = $this->sortTransactions($xactions); if ($is_preview) { @@ -696,10 +702,6 @@ $actor, $object, PhabricatorPolicyCapability::CAN_VIEW); - - foreach ($xactions as $xaction) { - $this->requireCapabilities($object, $xaction); - } } protected function requireCapabilities(