From 2e54b3ff57a5019225987bf751b1e753bb9c1939 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 10 May 2015 10:55:01 -0700 Subject: [PATCH] Fix excessively-conservative feed story policy checks Summary: When checking if a user can see a feed story about an object, we currently use the object's primary policy but ignore automatic capabilities. Instead, proxy both primary policies and automatic capabilities. Test Plan: - Before this patch, users could not see stories about events they were invited to but not permitted to see by the primary policy (this is currently the default for newly created events). - After this patch, these invited users can now see the stories. Reviewers: btrahan Reviewed By: btrahan Subscribers: lpriestley, epriestley Differential Revision: https://secure.phabricator.com/D12785 --- .../feed/story/PhabricatorFeedStory.php | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index a115985704..0c989d4948 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -453,15 +453,9 @@ abstract class PhabricatorFeedStory * @task policy */ public function getPolicy($capability) { - // If this story's primary object is a policy-aware object, use its policy - // to control story visiblity. - - $primary_phid = $this->getPrimaryObjectPHID(); - if (isset($this->objects[$primary_phid])) { - $object = $this->objects[$primary_phid]; - if ($object instanceof PhabricatorPolicyInterface) { - return $object->getPolicy($capability); - } + $policy_object = $this->getPrimaryPolicyObject(); + if ($policy_object) { + return $policy_object->getPolicy($capability); } // TODO: Remove this once all objects are policy-aware. For now, keep @@ -476,6 +470,11 @@ abstract class PhabricatorFeedStory * @task policy */ public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + $policy_object = $this->getPrimaryPolicyObject(); + if ($policy_object) { + return $policy_object->hasAutomaticCapability($capability, $viewer); + } + return false; } @@ -484,6 +483,26 @@ abstract class PhabricatorFeedStory } + /** + * Get the policy object this story is about, if such a policy object + * exists. + * + * @return PhabricatorPolicyInterface|null Policy object, if available. + * @task policy + */ + private function getPrimaryPolicyObject() { + $primary_phid = $this->getPrimaryObjectPHID(); + if (empty($this->objects[$primary_phid])) { + $object = $this->objects[$primary_phid]; + if ($object instanceof PhabricatorPolicyInterface) { + return $object; + } + } + + return null; + } + + /* -( PhabricatorMarkupInterface Implementation )--------------------------- */