diff --git a/resources/sql/autopatches/20151023.harborpolicy.1.sql b/resources/sql/autopatches/20151023.harborpolicy.1.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20151023.harborpolicy.1.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildplan + ADD viewPolicy VARBINARY(64) NOT NULL; + +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildplan + ADD editPolicy VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20151023.harborpolicy.2.php b/resources/sql/autopatches/20151023.harborpolicy.2.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20151023.harborpolicy.2.php @@ -0,0 +1,21 @@ +establishConnection('w'); + +$view_policy = PhabricatorPolicies::getMostOpenPolicy(); +queryfx( + $conn_w, + 'UPDATE %T SET viewPolicy = %s WHERE viewPolicy = %s', + $table->getTableName(), + $view_policy, + ''); + +$edit_policy = id(new PhabricatorHarbormasterApplication()) + ->getPolicy(HarbormasterCreatePlansCapability::CAPABILITY); +queryfx( + $conn_w, + 'UPDATE %T SET editPolicy = %s WHERE editPolicy = %s', + $table->getTableName(), + $edit_policy, + ''); 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 @@ -991,6 +991,8 @@ 'HarbormasterBuildPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPHIDType.php', 'HarbormasterBuildPlan' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php', 'HarbormasterBuildPlanDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildPlanDatasource.php', + 'HarbormasterBuildPlanDefaultEditCapability' => 'applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php', + 'HarbormasterBuildPlanDefaultViewCapability' => 'applications/harbormaster/capability/HarbormasterBuildPlanDefaultViewCapability.php', 'HarbormasterBuildPlanEditor' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditor.php', 'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php', 'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php', @@ -1036,6 +1038,7 @@ 'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php', 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', + 'HarbormasterCreatePlansCapability' => 'applications/harbormaster/capability/HarbormasterCreatePlansCapability.php', 'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php', 'HarbormasterDrydockBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php', 'HarbormasterDrydockCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php', @@ -1049,7 +1052,6 @@ 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php', 'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php', 'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php', - 'HarbormasterManagePlansCapability' => 'applications/harbormaster/capability/HarbormasterManagePlansCapability.php', 'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php', 'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php', 'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php', @@ -4815,6 +4817,8 @@ 'PhabricatorSubscribableInterface', ), 'HarbormasterBuildPlanDatasource' => 'PhabricatorTypeaheadDatasource', + 'HarbormasterBuildPlanDefaultEditCapability' => 'PhabricatorPolicyCapability', + 'HarbormasterBuildPlanDefaultViewCapability' => 'PhabricatorPolicyCapability', 'HarbormasterBuildPlanEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -4874,6 +4878,7 @@ 'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', + 'HarbormasterCreatePlansCapability' => 'PhabricatorPolicyCapability', 'HarbormasterDAO' => 'PhabricatorLiskDAO', 'HarbormasterDrydockBuildStepGroup' => 'HarbormasterBuildStepGroup', 'HarbormasterDrydockCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation', @@ -4887,7 +4892,6 @@ 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterLintMessagesController' => 'HarbormasterController', 'HarbormasterLintPropertyView' => 'AphrontView', - 'HarbormasterManagePlansCapability' => 'PhabricatorPolicyCapability', 'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow', diff --git a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php --- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php +++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php @@ -95,8 +95,16 @@ protected function getCustomCapabilities() { return array( - HarbormasterManagePlansCapability::CAPABILITY => array( - 'caption' => pht('Can create and manage build plans.'), + HarbormasterCreatePlansCapability::CAPABILITY => array( + 'default' => PhabricatorPolicies::POLICY_ADMIN, + ), + HarbormasterBuildPlanDefaultViewCapability::CAPABILITY => array( + 'template' => HarbormasterBuildPlanPHIDType::TYPECONST, + 'capability' => PhabricatorPolicyCapability::CAN_VIEW, + ), + HarbormasterBuildPlanDefaultEditCapability::CAPABILITY => array( + 'template' => HarbormasterBuildPlanPHIDType::TYPECONST, + 'capability' => PhabricatorPolicyCapability::CAN_EDIT, 'default' => PhabricatorPolicies::POLICY_ADMIN, ), ); diff --git a/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php b/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php @@ -0,0 +1,12 @@ +getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($request->getURIData('id'))) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php @@ -5,9 +5,6 @@ public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $id = $request->getURIData('id'); if ($id) { $plan = id(new HarbormasterBuildPlanQuery()) @@ -23,23 +20,42 @@ return new Aphront404Response(); } } else { + $this->requireApplicationCapability( + HarbormasterCreatePlansCapability::CAPABILITY); + $plan = HarbormasterBuildPlan::initializeNewBuildPlan($viewer); } $e_name = true; $v_name = $plan->getName(); + $v_view = $plan->getViewPolicy(); + $v_edit = $plan->getEditPolicy(); $validation_exception = null; if ($request->isFormPost()) { $xactions = array(); $v_name = $request->getStr('name'); + $v_view = $request->getStr('viewPolicy'); + $v_edit = $request->getStr('editPolicy'); + $e_name = null; + $type_name = HarbormasterBuildPlanTransaction::TYPE_NAME; + $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; + $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; $xactions[] = id(new HarbormasterBuildPlanTransaction()) ->setTransactionType($type_name) ->setNewValue($v_name); + $xactions[] = id(new HarbormasterBuildPlanTransaction()) + ->setTransactionType($type_view) + ->setNewValue($v_view); + + $xactions[] = id(new HarbormasterBuildPlanTransaction()) + ->setTransactionType($type_edit) + ->setNewValue($v_edit); + $editor = id(new HarbormasterBuildPlanEditor()) ->setActor($viewer) ->setContinueOnNoEffect(true) @@ -71,19 +87,37 @@ $save_button = pht('Save Build Plan'); } + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($plan) + ->execute(); + $form = id(new AphrontFormView()) ->setUser($viewer) - ->appendChild( + ->appendControl( id(new AphrontFormTextControl()) ->setLabel(pht('Plan Name')) ->setName('name') ->setError($e_name) - ->setValue($v_name)); - - $form->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue($save_button) - ->addCancelButton($cancel_uri)); + ->setValue($v_name)) + ->appendControl( + id(new AphrontFormPolicyControl()) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) + ->setPolicyObject($plan) + ->setPolicies($policies) + ->setValue($v_view) + ->setName('viewPolicy')) + ->appendControl( + id(new AphrontFormPolicyControl()) + ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) + ->setPolicyObject($plan) + ->setPolicies($policies) + ->setValue($v_edit) + ->setName('editPolicy')) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->setValue($save_button) + ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) ->setHeaderText($title) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanListController.php b/src/applications/harbormaster/controller/HarbormasterPlanListController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanListController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanListController.php @@ -42,7 +42,7 @@ $crumbs = parent::buildApplicationCrumbs(); $can_create = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); + HarbormasterCreatePlansCapability::CAPABILITY); $crumbs->addAction( id(new PHUIListItemView()) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php @@ -4,19 +4,16 @@ public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan_id = $request->getURIData('id'); - // NOTE: At least for now, this only requires the "Can Manage Plans" - // capability, not the "Can Edit" capability. Possibly it should have - // a more stringent requirement, though. - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($plan_id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$plan) { return new Aphront404Response(); diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -3,7 +3,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { public function handleRequest(AphrontRequest $request) { - $viewer = $this->getviewer(); + $viewer = $this->getViewer(); $id = $request->getURIData('id'); $plan = id(new HarbormasterBuildPlanQuery()) @@ -81,16 +81,11 @@ ->execute(); $steps = mpull($steps, null, 'getPHID'); - $has_manage = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - - $has_edit = PhabricatorPolicyFilter::hasCapability( + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $plan, PhabricatorPolicyCapability::CAN_EDIT); - $can_edit = ($has_manage && $has_edit); - $step_list = id(new PHUIObjectItemListView()) ->setUser($viewer) ->setNoDataString( @@ -252,16 +247,11 @@ ->setObject($plan) ->setObjectURI($this->getApplicationURI("plan/{$id}/")); - $has_manage = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - - $has_edit = PhabricatorPolicyFilter::hasCapability( + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $plan, PhabricatorPolicyCapability::CAN_EDIT); - $can_edit = ($has_manage && $has_edit); - $list->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Plan')) @@ -288,7 +278,7 @@ ->setIcon('fa-ban')); } - $can_run = ($has_manage && $plan->canRunManually()); + $can_run = ($can_edit && $plan->canRunManually()); $list->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php --- a/src/applications/harbormaster/controller/HarbormasterStepAddController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -5,9 +5,6 @@ public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($request->getURIData('id'))) diff --git a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php --- a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php @@ -5,9 +5,6 @@ public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $id = $request->getURIData('id'); $step = id(new HarbormasterBuildStepQuery()) diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -6,9 +6,6 @@ $viewer = $this->getViewer(); $id = $request->getURIData('id'); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - if ($id) { $step = id(new HarbormasterBuildStepQuery()) ->setViewer($viewer) diff --git a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php --- a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php @@ -15,7 +15,8 @@ $types = parent::getTransactionTypes(); $types[] = HarbormasterBuildPlanTransaction::TYPE_NAME; $types[] = HarbormasterBuildPlanTransaction::TYPE_STATUS; - $types[] = PhabricatorTransactions::TYPE_COMMENT; + $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; + $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -12,6 +12,8 @@ protected $name; protected $planStatus; protected $planAutoKey; + protected $viewPolicy; + protected $editPolicy; const STATUS_ACTIVE = 'active'; const STATUS_DISABLED = 'disabled'; @@ -19,10 +21,22 @@ private $buildSteps = self::ATTACHABLE; public static function initializeNewBuildPlan(PhabricatorUser $actor) { + $app = id(new PhabricatorApplicationQuery()) + ->setViewer($actor) + ->withClasses(array('PhabricatorHarbormasterApplication')) + ->executeOne(); + + $view_policy = $app->getPolicy( + HarbormasterBuildPlanDefaultViewCapability::CAPABILITY); + $edit_policy = $app->getPolicy( + HarbormasterBuildPlanDefaultEditCapability::CAPABILITY); + return id(new HarbormasterBuildPlan()) ->setName('') ->setPlanStatus(self::STATUS_ACTIVE) - ->attachBuildSteps(array()); + ->attachBuildSteps(array()) + ->setViewPolicy($view_policy) + ->setEditPolicy($edit_policy); } protected function getConfiguration() { @@ -156,16 +170,15 @@ public function getPolicy($capability) { switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - return PhabricatorPolicies::getMostOpenPolicy(); + if ($this->isAutoplan()) { + return PhabricatorPolicies::getMostOpenPolicy(); + } + return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: - // NOTE: In practice, this policy is always limited by the "Mangage - // Build Plans" policy. - if ($this->isAutoplan()) { return PhabricatorPolicies::POLICY_NOONE; } - - return PhabricatorPolicies::getMostOpenPolicy(); + return $this->getEditPolicy(); } }