diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -602,12 +602,13 @@ PhabricatorPolicyInterface $object, $policy, $capability) { + $viewer = $this->viewer; if (!$this->raisePolicyExceptions) { return; } - if ($this->viewer->isOmnipotent()) { + if ($viewer->isOmnipotent()) { // Never raise policy exceptions for the omnipotent viewer. Although we // will never normally issue a policy rejection for the omnipotent // viewer, we can end up here when queries blanket reject objects that @@ -634,7 +635,30 @@ $capability); } - $more = PhabricatorPolicy::getPolicyExplanation($this->viewer, $policy); + // See T13411. If you receive a policy exception because you can't view + // an object, we also want to avoid disclosing too many details about the + // actual policy (for example, the names of projects in the policy). + + // If you failed a "CAN_VIEW" check, or failed some other check and don't + // have "CAN_VIEW" on the object, we give you an "opaque" explanation. + // Otherwise, we give you a more detailed explanation. + + $view_capability = PhabricatorPolicyCapability::CAN_VIEW; + if ($capability === $view_capability) { + $show_details = false; + } else { + $show_details = self::hasCapability( + $viewer, + $object, + $view_capability); + } + + if ($show_details) { + $more = PhabricatorPolicy::getPolicyExplanation($viewer, $policy); + } else { + $more = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy); + } + $more = (array)$more; $more = array_filter($more); diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -220,6 +220,25 @@ PhabricatorUser $viewer, $policy) { + $type = phid_get_type($policy); + if ($type === PhabricatorProjectProjectPHIDType::TYPECONST) { + $handle = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($policy)) + ->executeOne(); + + return pht( + 'Members of the project "%s" can take this action.', + $handle->getFullName()); + } + + return self::getOpaquePolicyExplanation($viewer, $policy); + } + + public static function getOpaquePolicyExplanation( + PhabricatorUser $viewer, + $policy) { + $rule = PhabricatorPolicyQuery::getObjectPolicyRule($policy); if ($rule) { return $rule->getPolicyExplanation(); @@ -245,7 +264,9 @@ $type = phid_get_type($policy); if ($type == PhabricatorProjectProjectPHIDType::TYPECONST) { return pht( - 'Members of the project "%s" can take this action.', + 'Members of a particular project can take this action. (You '. + 'can not see this object, so the name of this project is '. + 'restricted.)', $handle->getFullName()); } else if ($type == PhabricatorPeopleUserPHIDType::TYPECONST) { return pht(