diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index 7126f63995..e047e17b3f 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -164,9 +164,23 @@ class AphrontDefaultApplicationConfiguration return $login_controller->processRequest(); } - $content = hsprintf( - '
%s
', - $ex->getMessage()); + $list = $ex->getMoreInfo(); + foreach ($list as $key => $item) { + $list[$key] = phutil_tag('li', array(), $item); + } + if ($list) { + $list = phutil_tag('ul', array(), $list); + } + + $content = phutil_tag( + 'div', + array( + 'class' => 'aphront-policy-exception', + ), + array( + $ex->getMessage(), + $list, + )); $dialog = new AphrontDialogView(); $dialog diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 6a2a4133c9..fb36174a47 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -416,9 +416,13 @@ final class DifferentialRevisionQuery // The revision has an associated repository, and the viewer can't see // it, and the viewer has no special capabilities. Filter out this // revision. + $this->didRejectResult($revision); unset($revisions[$key]); } + if (!$revisions) { + return array(); + } $table = new DifferentialRevision(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index ba9e7a0383..2533a3789c 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -338,7 +338,7 @@ final class DifferentialRevision extends DifferentialDAO case PhabricatorPolicyCapability::CAN_VIEW: $description[] = pht( "A revision's reviewers can always view it."); - if ($this->getRepository()) { + if ($this->getRepositoryPHID()) { $description[] = pht( 'This revision belongs to a repository. Other users must be able '. 'to view the repository in order to view this revision.'); diff --git a/src/applications/policy/exception/PhabricatorPolicyException.php b/src/applications/policy/exception/PhabricatorPolicyException.php index 2e43cc0ed3..89649a3b8c 100644 --- a/src/applications/policy/exception/PhabricatorPolicyException.php +++ b/src/applications/policy/exception/PhabricatorPolicyException.php @@ -2,4 +2,15 @@ final class PhabricatorPolicyException extends Exception { + private $moreInfo = array(); + + public function setMoreInfo(array $more_info) { + $this->moreInfo = $more_info; + return $this; + } + + public function getMoreInfo() { + return $this->moreInfo; + } + } diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index ec8b7af87a..b5907e6a7a 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -248,28 +248,64 @@ final class PhabricatorPolicyFilter { "This object has an impossible {$verb} policy."); } - private function rejectObject($object, $policy, $capability) { + public function rejectObject( + PhabricatorPolicyInterface $object, + $policy, + $capability) { + if (!$this->raisePolicyExceptions) { return; } - // TODO: clean this up - $verb = $capability; - - $message = "You do not have permission to {$verb} this object."; + $more = array(); + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $message = pht( + 'This object exists, but you do not have permission to view it.'); + break; + case PhabricatorPolicyCapability::CAN_EDIT: + $message = pht('You do not have permission to edit this object.'); + break; + case PhabricatorPolicyCapability::CAN_JOIN: + $message = pht('You do not have permission to join this object.'); + break; + } switch ($policy) { case PhabricatorPolicies::POLICY_PUBLIC: - $who = "This is curious, since anyone can {$verb} the object."; + // Presumably, this is a bug, so we don't bother specializing the + // strings. + $more = pht('This object is public.'); break; case PhabricatorPolicies::POLICY_USER: - $who = "To {$verb} this object, you must be logged in."; + // We always raise this as "log in", so we don't need to specialize. + $more = pht('This object is available to logged in users.'); break; case PhabricatorPolicies::POLICY_ADMIN: - $who = "To {$verb} this object, you must be an administrator."; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $more = pht('Administrators can view this object.'); + break; + case PhabricatorPolicyCapability::CAN_EDIT: + $more = pht('Administrators can edit this object.'); + break; + case PhabricatorPolicyCapability::CAN_JOIN: + $more = pht('Administrators can join this object.'); + break; + } break; case PhabricatorPolicies::POLICY_NOONE: - $who = "No one can {$verb} this object."; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $more = pht('By default, no one can view this object.'); + break; + case PhabricatorPolicyCapability::CAN_EDIT: + $more = pht('By default, no one can edit this object.'); + break; + case PhabricatorPolicyCapability::CAN_JOIN: + $more = pht('By default, no one can join this object.'); + break; + } break; default: $handle = id(new PhabricatorHandleQuery()) @@ -279,16 +315,55 @@ final class PhabricatorPolicyFilter { $type = phid_get_type($policy); if ($type == PhabricatorProjectPHIDTypeProject::TYPECONST) { - $who = "To {$verb} this object, you must be a member of project ". - "'".$handle->getFullName()."'."; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $more = pht( + 'This object is visible to members of the project "%s".', + $handle->getFullName()); + break; + case PhabricatorPolicyCapability::CAN_EDIT: + $more = pht( + 'This object can be edited by members of the project "%s".', + $handle->getFullName()); + break; + case PhabricatorPolicyCapability::CAN_JOIN: + $more = pht( + 'This object can be joined by members of the project "%s".', + $handle->getFullName()); + break; + } } else if ($type == PhabricatorPeoplePHIDTypeUser::TYPECONST) { - $who = "Only '".$handle->getFullName()."' can {$verb} this object."; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $more = pht( + '%s can view this object.', + $handle->getFullName()); + break; + case PhabricatorPolicyCapability::CAN_EDIT: + $more = pht( + '%s can edit this object.', + $handle->getFullName()); + break; + case PhabricatorPolicyCapability::CAN_JOIN: + $more = pht( + '%s can join this object.', + $handle->getFullName()); + break; + } + } else { - $who = "It is unclear who can {$verb} this object."; + $who = pht("This object has an unknown policy setting."); } break; } - throw new PhabricatorPolicyException("{$message} {$who}"); + $more = array_merge( + array($more), + array_filter((array)$object->describeAutomaticCapability($capability))); + + $exception = new PhabricatorPolicyException($message); + $exception->setMoreInfo($more); + + throw $exception; } } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index a5e74c6a2b..f985d43e67 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -190,18 +190,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { $results = array(); - $filter = new PhabricatorPolicyFilter(); - $filter->setViewer($this->viewer); - - if (!$this->capabilities) { - $capabilities = array( - PhabricatorPolicyCapability::CAN_VIEW, - ); - } else { - $capabilities = $this->capabilities; - } - $filter->requireCapabilities($capabilities); - $filter->raisePolicyExceptions($this->shouldRaisePolicyExceptions()); + $filter = $this->getPolicyFilter(); $offset = (int)$this->getOffset(); $limit = (int)$this->getLimit(); @@ -287,6 +276,29 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { return $results; } + private function getPolicyFilter() { + $filter = new PhabricatorPolicyFilter(); + $filter->setViewer($this->viewer); + if (!$this->capabilities) { + $capabilities = array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } else { + $capabilities = $this->capabilities; + } + $filter->requireCapabilities($capabilities); + $filter->raisePolicyExceptions($this->shouldRaisePolicyExceptions()); + + return $filter; + } + + protected function didRejectResult(PhabricatorPolicyInterface $object) { + $this->getPolicyFilter()->rejectObject( + $object, + $object->getPolicy(PhabricatorPolicyCapability::CAN_VIEW), + PhabricatorPolicyCapability::CAN_VIEW); + } + /* -( Policy Query Implementation )---------------------------------------- */