Allow "Default View" policies to be set to Public
Summary: Ref T603. Currently, we hard-code defense against setting policies to "Public" in several places, and special case only the CAN_VIEW policy. In fact, other policies (like Default View) should also be able to be set to public. Instead of hard-coding this, move it to the capability definitions. Test Plan: Set default view policy in Maniphest to "Public", created a task, verified default policy. Reviewers: btrahan, asherkin Reviewed By: asherkin CC: asherkin, aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7276
This commit is contained in:
		| @@ -13,4 +13,8 @@ final class DifferentialCapabilityDefaultView | |||||||
|     return pht('Default View Policy'); |     return pht('Default View Policy'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function shouldAllowPublicPolicySetting() { | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -13,4 +13,8 @@ final class ManiphestCapabilityDefaultView | |||||||
|     return pht('Default View Policy'); |     return pht('Default View Policy'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function shouldAllowPublicPolicySetting() { | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -51,9 +51,9 @@ final class PhabricatorApplicationEditController | |||||||
|           continue; |           continue; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|         if ($new == PhabricatorPolicies::POLICY_PUBLIC && |         $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); | ||||||
|             $capability != PhabricatorPolicyCapability::CAN_VIEW) { |         if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) { | ||||||
|           // Can't set policies other than "view" to public. |           // Can't set non-public policies to public. | ||||||
|           continue; |           continue; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -40,6 +40,15 @@ abstract class PhabricatorPolicyCapability extends Phobject { | |||||||
|     return null; |     return null; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   /** | ||||||
|  |    * Can this capability be set to "public"? Broadly, this is only appropriate | ||||||
|  |    * for view and view-related policies. | ||||||
|  |    * | ||||||
|  |    * @return bool True to allow the "public" policy. Returns false by default. | ||||||
|  |    */ | ||||||
|  |   public function shouldAllowPublicPolicySetting() { | ||||||
|  |     return false; | ||||||
|  |   } | ||||||
|  |  | ||||||
|   final public static function getCapabilityByKey($key) { |   final public static function getCapabilityByKey($key) { | ||||||
|     return idx(self::getCapabilityMap(), $key); |     return idx(self::getCapabilityMap(), $key); | ||||||
|   | |||||||
| @@ -15,4 +15,8 @@ final class PhabricatorPolicyCapabilityCanView | |||||||
|     return pht('You do not have permission to view this object.'); |     return pht('You do not have permission to view this object.'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function shouldAllowPublicPolicySetting() { | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -173,9 +173,10 @@ final class PhabricatorPolicyFilter { | |||||||
|         $policy = PhabricatorPolicies::POLICY_USER; |         $policy = PhabricatorPolicies::POLICY_USER; | ||||||
|       } |       } | ||||||
|  |  | ||||||
|       // If the object is set to "public" but the capability is anything other |       // If the object is set to "public" but the capability is not a public | ||||||
|       // than "view", restrict the policy to "user". |       // capability, restrict the policy to "user". | ||||||
|       if ($capability != PhabricatorPolicyCapability::CAN_VIEW) { |       $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); | ||||||
|  |       if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) { | ||||||
|         $policy = PhabricatorPolicies::POLICY_USER; |         $policy = PhabricatorPolicies::POLICY_USER; | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -36,13 +36,17 @@ final class AphrontFormPolicyControl extends AphrontFormControl { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function getOptions() { |   protected function getOptions() { | ||||||
|  |     $capability = $this->capability; | ||||||
|  |  | ||||||
|     $options = array(); |     $options = array(); | ||||||
|     foreach ($this->policies as $policy) { |     foreach ($this->policies as $policy) { | ||||||
|       if (($policy->getPHID() == PhabricatorPolicies::POLICY_PUBLIC) && |       if ($policy->getPHID() == PhabricatorPolicies::POLICY_PUBLIC) { | ||||||
|           ($this->capability != PhabricatorPolicyCapability::CAN_VIEW)) { |         // Never expose "Public" for capabilities which don't support it. | ||||||
|         // Never expose "Public" for anything except "Can View". |         $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); | ||||||
|  |         if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) { | ||||||
|           continue; |           continue; | ||||||
|         } |         } | ||||||
|  |       } | ||||||
|  |  | ||||||
|       $type_name = PhabricatorPolicyType::getPolicyTypeName($policy->getType()); |       $type_name = PhabricatorPolicyType::getPolicyTypeName($policy->getType()); | ||||||
|       $options[$type_name][$policy->getPHID()] = $policy->getFullName(); |       $options[$type_name][$policy->getPHID()] = $policy->getFullName(); | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley