From 4f845d8f8c7749b536f32c2f26f1fdb7c32e2766 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 12 Sep 2019 09:28:56 -0700 Subject: [PATCH] When users encounter a policy exception for a non-view capability with a custom policy, inline the policy rules Summary: Fixes T13411. This looks like the last case where you hit a policy explanation and have permission to see the policy, but we don't currently show you the policy rules. This implementation is slightly clumsy, but likely harmless. Test Plan: {F6856421} Maniphest Tasks: T13411 Differential Revision: https://secure.phabricator.com/D20807 --- .../policy/filter/PhabricatorPolicyFilter.php | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index 63f3f98cec..d8f239e51a 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -653,14 +653,42 @@ final class PhabricatorPolicyFilter extends Phobject { $view_capability); } + // TODO: This is a bit clumsy. We're producing HTML and text versions of + // this message, but can't render the full policy rules in text today. + // Users almost never get a text-only version of this exception anyway. + + $head = null; + $more = null; + if ($show_details) { - $more = PhabricatorPolicy::getPolicyExplanation($viewer, $policy); + $head = PhabricatorPolicy::getPolicyExplanation($viewer, $policy); + + $policy_type = PhabricatorPolicyPHIDTypePolicy::TYPECONST; + $is_custom = (phid_get_type($policy) === $policy_type); + if ($is_custom) { + $policy_map = PhabricatorPolicyQuery::loadPolicies( + $viewer, + $object); + if (isset($policy_map[$capability])) { + require_celerity_resource('phui-policy-section-view-css'); + + $more = id(new PhabricatorPolicyRulesView()) + ->setViewer($viewer) + ->setPolicy($policy_map[$capability]); + + $more = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-rules', + ), + $more); + } + } } else { - $more = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy); + $head = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy); } - $more = (array)$more; - $more = array_filter($more); + $head = (array)$head; $exceptions = PhabricatorPolicy::getSpecialRules( $object, @@ -668,7 +696,10 @@ final class PhabricatorPolicyFilter extends Phobject { $capability, true); - $details = array_filter(array_merge($more, $exceptions)); + $text_details = array_filter(array_merge($head, $exceptions)); + $text_details = implode(' ', $text_details); + + $html_details = array($head, $more, $exceptions); $access_denied = $this->renderAccessDenied($object); @@ -677,7 +708,7 @@ final class PhabricatorPolicyFilter extends Phobject { $access_denied, $capability_name, $rejection, - implode(' ', $details)); + $text_details); $exception = id(new PhabricatorPolicyException($full_message)) ->setTitle($access_denied) @@ -685,7 +716,7 @@ final class PhabricatorPolicyFilter extends Phobject { ->setRejection($rejection) ->setCapability($capability) ->setCapabilityName($capability_name) - ->setMoreInfo($details); + ->setMoreInfo($html_details); throw $exception; }