From f1f8ee8e6ad5f094fd1a4f65e0e6cc194ca21c09 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 15 Feb 2016 07:20:29 -0800 Subject: [PATCH] Improve subproject/milestone error handling for users who can't create projects Summary: Fixes T10357. - Show a better (more descriptive) error message when a user who can't create projects tries to create a subproject or milestone. - Disable the subproject actions if you don't have create permission. All this stuff was already enforced properly: this diff doesn't make any actual policy changes, just improves the UI for users who lack permission. Test Plan: - As an unprivileged user (no "Can Create Projects"), tried to create a subproject or milestone. - After patch, got a disabled action, with more specific and helpful error than before. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10357 Differential Revision: https://secure.phabricator.com/D15274 --- .../controller/PhabricatorProjectEditController.php | 5 +++++ .../controller/PhabricatorProjectSubprojectsController.php | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/applications/project/controller/PhabricatorProjectEditController.php b/src/applications/project/controller/PhabricatorProjectEditController.php index 5091135bec..7c041af93a 100644 --- a/src/applications/project/controller/PhabricatorProjectEditController.php +++ b/src/applications/project/controller/PhabricatorProjectEditController.php @@ -24,6 +24,11 @@ final class PhabricatorProjectEditController $id = $request->getURIData('id'); if (!$id) { + // This capability is checked again later, but checking it here + // explicitly gives us a better error message. + $this->requireApplicationCapability( + ProjectCreateProjectsCapability::CAPABILITY); + $parent_id = head($request->getArr('parent')); if (!$parent_id) { $parent_id = $request->getStr('parent'); diff --git a/src/applications/project/controller/PhabricatorProjectSubprojectsController.php b/src/applications/project/controller/PhabricatorProjectSubprojectsController.php index 36f9d641a8..eb32d00b92 100644 --- a/src/applications/project/controller/PhabricatorProjectSubprojectsController.php +++ b/src/applications/project/controller/PhabricatorProjectSubprojectsController.php @@ -181,6 +181,9 @@ final class PhabricatorProjectSubprojectsController $viewer = $this->getViewer(); $id = $project->getID(); + $can_create = $this->hasApplicationCapability( + ProjectCreateProjectsCapability::CAPABILITY); + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $project, @@ -198,7 +201,7 @@ final class PhabricatorProjectSubprojectsController $milestone_text = pht('Create Milestone'); } - $can_milestone = ($can_edit && $allows_milestones); + $can_milestone = ($can_create && $can_edit && $allows_milestones); $milestone_href = "/project/edit/?milestone={$id}"; $view->addAction( @@ -209,7 +212,7 @@ final class PhabricatorProjectSubprojectsController ->setDisabled(!$can_milestone) ->setWorkflow(!$can_milestone)); - $can_subproject = ($can_edit && $allows_subprojects); + $can_subproject = ($can_create && $can_edit && $allows_subprojects); // If we're offering to create the first subproject, we're going to warn // the user about the effects before moving forward.