diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 63e0916c34..530101ae40 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2645,6 +2645,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsEditor' => 'applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php', 'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php', 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsSubscribeEmailCommand.php', + 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php', 'PhabricatorSubscriptionsTransactionController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsTransactionController.php', 'PhabricatorSubscriptionsUIEventListener' => 'applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php', 'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsUnsubscribeEmailCommand.php', @@ -6166,6 +6167,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor', 'PhabricatorSubscriptionsListController' => 'PhabricatorController', 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand', + 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'PhabricatorPolicyRule', 'PhabricatorSubscriptionsTransactionController' => 'PhabricatorController', 'PhabricatorSubscriptionsUIEventListener' => 'PhabricatorEventListener', 'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand', diff --git a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php index 2348ba13b1..f68eae2b49 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php @@ -208,5 +208,32 @@ final class PhabricatorPolicyDataTestCase extends PhabricatorTestCase { PhabricatorPolicyCapability::CAN_VIEW)); } + public function testObjectPolicyRuleSubscribers() { + $author = $this->generateNewTestUser(); + + $rule = new PhabricatorSubscriptionsSubscribersPolicyRule(); + + $task = ManiphestTask::initializeNewTask($author); + $task->setViewPolicy($rule->getObjectPolicyFullKey()); + $task->save(); + + $this->assertFalse( + PhabricatorPolicyFilter::hasCapability( + $author, + $task, + PhabricatorPolicyCapability::CAN_VIEW)); + + id(new PhabricatorSubscriptionsEditor()) + ->setActor($author) + ->setObject($task) + ->subscribeExplicit(array($author->getPHID())) + ->save(); + + $this->assertTrue( + PhabricatorPolicyFilter::hasCapability( + $author, + $task, + PhabricatorPolicyCapability::CAN_VIEW)); + } } diff --git a/src/applications/policy/rule/PhabricatorPolicyRule.php b/src/applications/policy/rule/PhabricatorPolicyRule.php index 919c511bb7..c4e47da827 100644 --- a/src/applications/policy/rule/PhabricatorPolicyRule.php +++ b/src/applications/policy/rule/PhabricatorPolicyRule.php @@ -97,6 +97,56 @@ abstract class PhabricatorPolicyRule { } +/* -( Transaction Hints )-------------------------------------------------- */ + + + /** + * Tell policy rules about upcoming transaction effects. + * + * Before transaction effects are applied, we try to stop users from making + * edits which will lock them out of objects. We can't do this perfectly, + * since they can set a policy to "the moon is full" moments before it wanes, + * but we try to prevent as many mistakes as possible. + * + * Some policy rules depend on complex checks against object state which + * we can't set up ahead of time. For example, subscriptions require database + * writes. + * + * In cases like this, instead of doing writes, you can pass a hint about an + * object to a policy rule. The rule can then look for hints and use them in + * rendering a verdict about whether the user will be able to see the object + * or not after applying the policy change. + * + * @param PhabricatorPolicyInterface Object to pass a hint about. + * @param PhabricatorPolicyRule Rule to pass hint to. + * @param wild Hint. + * @return void + */ + public static function passTransactionHintToRule( + PhabricatorPolicyInterface $object, + PhabricatorPolicyRule $rule, + $hint) { + + $cache = PhabricatorCaches::getRequestCache(); + $cache->setKey(self::getObjectPolicyCacheKey($object, $rule), $hint); + } + + protected function getTransactionHint( + PhabricatorPolicyInterface $object) { + + $cache = PhabricatorCaches::getRequestCache(); + return $cache->getKey(self::getObjectPolicyCacheKey($object, $this)); + } + + private static function getObjectPolicyCacheKey( + PhabricatorPolicyInterface $object, + PhabricatorPolicyRule $rule) { + $hash = spl_object_hash($object); + $rule = get_class($rule); + return 'policycache.'.$hash.'.'.$rule; + } + + /* -( Implementing Object Policies )--------------------------------------- */ diff --git a/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php b/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php new file mode 100644 index 0000000000..7da35f3703 --- /dev/null +++ b/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php @@ -0,0 +1,121 @@ +getPHID(); + if (!$viewer_phid) { + return; + } + + if (empty($this->subscribed[$viewer_phid])) { + $this->subscribed[$viewer_phid] = array(); + } + + // Load the project PHIDs the user is a member of. + if (!isset($this->sourcePHIDs[$viewer_phid])) { + $source_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $viewer_phid, + PhabricatorProjectMemberOfProjectEdgeType::EDGECONST); + $source_phids[] = $viewer_phid; + $this->sourcePHIDs[$viewer_phid] = $source_phids; + } + + // Look for transaction hints. + foreach ($objects as $key => $object) { + $cache = $this->getTransactionHint($object); + if ($cache === null) { + // We don't have a hint for this object, so we'll deal with it below. + continue; + } + + // We have a hint, so use that as the source of truth. + unset($objects[$key]); + + foreach ($this->sourcePHIDs[$viewer_phid] as $source_phid) { + if (isset($cache[$source_phid])) { + $this->subscribed[$viewer_phid][$object->getPHID()] = true; + break; + } + } + } + + $phids = mpull($objects, 'getPHID'); + if (!$phids) { + return; + } + + $edge_query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs($this->sourcePHIDs[$viewer_phid]) + ->withEdgeTypes( + array( + PhabricatorSubscribedToObjectEdgeType::EDGECONST, + )) + ->withDestinationPHIDs($phids); + + $edge_query->execute(); + + $subscribed = $edge_query->getDestinationPHIDs(); + if (!$subscribed) { + return; + } + + $this->subscribed[$viewer_phid] += array_fill_keys($subscribed, true); + } + + public function applyRule( + PhabricatorUser $viewer, + $value, + PhabricatorPolicyInterface $object) { + + $viewer_phid = $viewer->getPHID(); + if (!$viewer_phid) { + return false; + } + + if ($object->isAutomaticallySubscribed($viewer_phid)) { + return true; + } + + $subscribed = idx($this->subscribed, $viewer_phid); + return isset($subscribed[$object->getPHID()]); + } + + public function getValueControlType() { + return self::CONTROL_TYPE_NONE; + } + +} diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index a6f93ff761..6bf88b4e25 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2087,6 +2087,19 @@ abstract class PhabricatorApplicationTransactionEditor foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + $clone_xaction = clone $xaction; + $clone_xaction->setOldValue(array_values($this->subscribers)); + $clone_xaction->setNewValue( + $this->getPHIDTransactionNewValue( + $clone_xaction)); + + PhabricatorPolicyRule::passTransactionHintToRule( + $copy, + new PhabricatorSubscriptionsSubscribersPolicyRule(), + array_fuse($clone_xaction->getNewValue())); + + break; case PhabricatorTransactions::TYPE_SPACE: $space_phid = $this->getTransactionNewValue($object, $xaction); $copy->setSpacePHID($space_phid);