diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index b15544429d..171f504b7b 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -20,7 +20,7 @@ final class ManiphestTask extends ManiphestDAO protected $subpriority = 0; protected $title = ''; - protected $originalTitle; + protected $originalTitle = ''; protected $description = ''; protected $originalEmailSource; protected $mailKey; diff --git a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php index 1544138ec9..81aaff88db 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php @@ -31,4 +31,117 @@ final class PhabricatorPolicyDataTestCase extends PhabricatorTestCase { $this->assertEqual(0, count($results)); } + + public function testCustomPolicyRuleUser() { + $user_a = $this->generateNewTestUser(); + $user_b = $this->generateNewTestUser(); + $author = $this->generateNewTestUser(); + + $policy = id(new PhabricatorPolicy()) + ->setRules( + array( + array( + 'action' => PhabricatorPolicy::ACTION_ACCEPT, + 'rule' => 'PhabricatorPolicyRuleUsers', + 'value' => array($user_a->getPHID()), + ), + )) + ->save(); + + $task = ManiphestTask::initializeNewTask($author); + $task->setViewPolicy($policy->getPHID()); + $task->save(); + + $can_a_view = PhabricatorPolicyFilter::hasCapability( + $user_a, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + + $this->assertEqual(true, $can_a_view); + + $can_b_view = PhabricatorPolicyFilter::hasCapability( + $user_b, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + + $this->assertEqual(false, $can_b_view); + } + + public function testCustomPolicyRuleAdministrators() { + $user_a = $this->generateNewTestUser(); + $user_a->setIsAdmin(true)->save(); + $user_b = $this->generateNewTestUser(); + $author = $this->generateNewTestUser(); + + $policy = id(new PhabricatorPolicy()) + ->setRules( + array( + array( + 'action' => PhabricatorPolicy::ACTION_ACCEPT, + 'rule' => 'PhabricatorPolicyRuleAdministrators', + 'value' => null, + ), + )) + ->save(); + + $task = ManiphestTask::initializeNewTask($author); + $task->setViewPolicy($policy->getPHID()); + $task->save(); + + $can_a_view = PhabricatorPolicyFilter::hasCapability( + $user_a, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + + $this->assertEqual(true, $can_a_view); + + $can_b_view = PhabricatorPolicyFilter::hasCapability( + $user_b, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + + $this->assertEqual(false, $can_b_view); + } + + public function testCustomPolicyRuleLunarPhase() { + $user_a = $this->generateNewTestUser(); + $author = $this->generateNewTestUser(); + + $policy = id(new PhabricatorPolicy()) + ->setRules( + array( + array( + 'action' => PhabricatorPolicy::ACTION_ACCEPT, + 'rule' => 'PhabricatorPolicyRuleLunarPhase', + 'value' => 'new', + ), + )) + ->save(); + + $task = ManiphestTask::initializeNewTask($author); + $task->setViewPolicy($policy->getPHID()); + $task->save(); + + $time_a = PhabricatorTime::pushTime(934354800, 'UTC'); + + $can_a_view = PhabricatorPolicyFilter::hasCapability( + $user_a, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + $this->assertEqual(true, $can_a_view); + + unset($time_a); + + + $time_b = PhabricatorTime::pushTime(1116745200, 'UTC'); + + $can_a_view = PhabricatorPolicyFilter::hasCapability( + $user_a, + $task, + PhabricatorPolicyCapability::CAN_VIEW); + $this->assertEqual(false, $can_a_view); + + unset($time_b); + } + } diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index 2ed1dec241..ef6108ceaa 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -7,6 +7,7 @@ final class PhabricatorPolicyFilter { private $capabilities; private $raisePolicyExceptions; private $userProjects; + private $customPolicies = array(); public static function mustRetainCapability( PhabricatorUser $user, @@ -85,6 +86,7 @@ final class PhabricatorPolicyFilter { } $need_projects = array(); + $need_policies = array(); foreach ($objects as $key => $object) { $object_capabilities = $object->getCapabilities(); foreach ($capabilities as $capability) { @@ -99,9 +101,17 @@ final class PhabricatorPolicyFilter { if ($type == PhabricatorProjectPHIDTypeProject::TYPECONST) { $need_projects[$policy] = $policy; } + + if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) { + $need_policies[$policy] = $policy; + } } } + if ($need_policies) { + $this->loadCustomPolicies(array_keys($need_policies)); + } + // If we need projects, check if any of the projects we need are also the // objects we're filtering. Because of how project rules work, this is a // common case. @@ -225,6 +235,12 @@ final class PhabricatorPolicyFilter { } else { $this->rejectObject($object, $policy, $capability); } + } else if ($type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) { + if ($this->checkCustomPolicy($policy)) { + return true; + } else { + $this->rejectObject($object, $policy, $capability); + } } else { // Reject objects with unknown policies. $this->rejectObject($object, false, $capability); @@ -316,4 +332,75 @@ final class PhabricatorPolicyFilter { throw $exception; } + + private function loadCustomPolicies(array $phids) { + $viewer = $this->viewer; + $viewer_phid = $viewer->getPHID(); + + $custom_policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->withPHIDs($phids) + ->execute(); + $custom_policies = mpull($custom_policies, null, 'getPHID'); + + + $classes = array(); + $values = array(); + foreach ($custom_policies as $policy) { + foreach ($policy->getCustomRuleClasses() as $class) { + $classes[$class] = $class; + $values[$class][] = $policy->getCustomRuleValues($class); + } + } + + foreach ($classes as $class => $ignored) { + $object = newv($class, array()); + $object->willApplyRules($viewer, array_mergev($values[$class])); + $classes[$class] = $object; + } + + foreach ($custom_policies as $policy) { + $policy->attachRuleObjects($classes); + } + + if (empty($this->customPolicies[$viewer_phid])) { + $this->customPolicies[$viewer_phid] = array(); + } + + $this->customPolicies[$viewer->getPHID()] += $custom_policies; + } + + private function checkCustomPolicy($policy_phid) { + $viewer = $this->viewer; + $viewer_phid = $viewer->getPHID(); + + $policy = $this->customPolicies[$viewer_phid][$policy_phid]; + + $objects = $policy->getRuleObjects(); + $action = null; + foreach ($policy->getRules() as $rule) { + $object = idx($objects, idx($rule, 'rule')); + if (!$object) { + // Reject, this policy has a bogus rule. + return false; + } + + // If the user matches this rule, use this action. + if ($object->applyRule($viewer, idx($rule, 'value'))) { + $action = idx($rule, 'action'); + break; + } + } + + if ($action === null) { + $action = $policy->getDefaultAction(); + } + + if ($action === PhabricatorPolicy::ACTION_ACCEPT) { + return true; + } + + return false; + } + } diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php index a58c085f54..f810c345f7 100644 --- a/src/applications/policy/query/PhabricatorPolicyQuery.php +++ b/src/applications/policy/query/PhabricatorPolicyQuery.php @@ -26,31 +26,19 @@ final class PhabricatorPolicyQuery extends PhabricatorQuery { PhabricatorPolicyInterface $object) { $results = array(); - $policies = null; - $global = self::getGlobalPolicies(); - $capabilities = $object->getCapabilities(); - foreach ($capabilities as $capability) { - $policy = $object->getPolicy($capability); - if (!$policy) { - continue; - } + $map = array(); + foreach ($object->getCapabilities() as $capability) { + $map[$capability] = $object->getPolicy($capability); + } - if (isset($global[$policy])) { - $results[$capability] = $global[$policy]; - continue; - } + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->withPHIDs($map) + ->execute(); - if ($policies === null) { - // This slightly overfetches data, but it shouldn't generally - // be a problem. - $policies = id(new PhabricatorPolicyQuery()) - ->setViewer($viewer) - ->setObject($object) - ->execute(); - } - - $results[$capability] = $policies[$policy]; + foreach ($map as $capability => $phid) { + $results[$capability] = $policies[$phid]; } return $results; @@ -75,72 +63,65 @@ final class PhabricatorPolicyQuery extends PhabricatorQuery { throw new Exception('Call setViewer() before execute()!'); } - $results = $this->getGlobalPolicies(); + if ($this->object && $this->phids) { + throw new Exception( + "You can not issue a policy query with both setObject() and ". + "setPHIDs()."); + } else if ($this->object) { + $phids = $this->loadObjectPolicyPHIDs(); + } else { + $phids = $this->phids; + } - if ($this->viewer->getPHID()) { - $projects = id(new PhabricatorProjectQuery()) - ->setViewer($this->viewer) - ->withMemberPHIDs(array($this->viewer->getPHID())) - ->execute(); - if ($projects) { - foreach ($projects as $project) { - $results[] = id(new PhabricatorPolicy()) - ->setType(PhabricatorPolicyType::TYPE_PROJECT) - ->setPHID($project->getPHID()) - ->setHref('/project/view/'.$project->getID().'/') - ->setName($project->getName()); - } + $phids = array_fuse($phids); + + $results = array(); + + // First, load global policies. + foreach ($this->getGlobalPolicies() as $phid => $policy) { + if (isset($phids[$phid])) { + $results[$phid] = $policy; + unset($phids[$phid]); } } - $results = mpull($results, null, 'getPHID'); - - $other_policies = array(); - if ($this->object) { - $capabilities = $this->object->getCapabilities(); - foreach ($capabilities as $capability) { - $policy = $this->object->getPolicy($capability); - if (!$policy) { - continue; + // If we still need policies, we're going to have to fetch data. Bucket + // the remaining policies into rule-based policies and handle-based + // policies. + if ($phids) { + $rule_policies = array(); + $handle_policies = array(); + foreach ($phids as $phid) { + $phid_type = phid_get_type($phid); + if ($phid_type == PhabricatorPolicyPHIDTypePolicy::TYPECONST) { + $rule_policies[$phid] = $phid; + } else { + $handle_policies[$phid] = $phid; } - $other_policies[$policy] = $policy; } - } - // If this install doesn't have "Public" enabled, remove it as an option - // unless the object already has a "Public" policy. In this case we retain - // the policy but enforce it as thought it was "All Users". - $show_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); - if (!$show_public && - empty($other_policies[PhabricatorPolicies::POLICY_PUBLIC])) { - unset($results[PhabricatorPolicies::POLICY_PUBLIC]); - } + if ($handle_policies) { + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($this->viewer) + ->withPHIDs($handle_policies) + ->execute(); + foreach ($handle_policies as $phid) { + $results[$phid] = PhabricatorPolicy::newFromPolicyAndHandle( + $phid, + $handles[$phid]); + } + } - $other_policies = array_diff_key($other_policies, $results); - - if ($other_policies) { - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->viewer) - ->withPHIDs($other_policies) - ->execute(); - foreach ($other_policies as $phid) { - $results[$phid] = PhabricatorPolicy::newFromPolicyAndHandle( - $phid, - $handles[$phid]); + if ($rule_policies) { + $rules = id(new PhabricatorPolicy())->loadAllWhere( + 'phid IN (%Ls)', + $rule_policies); + $results += mpull($rules, null, 'getPHID'); } } $results = msort($results, 'getSortKey'); - if ($this->phids) { - $phids = array_fuse($this->phids); - foreach ($results as $key => $result) { - if (empty($phids[$result->getPHID()])) { - unset($results[$key]); - } - } - } - return $results; } @@ -196,5 +177,43 @@ final class PhabricatorPolicyQuery extends PhabricatorQuery { } } + private function loadObjectPolicyPHIDs() { + $phids = array(); + + if ($this->viewer->getPHID()) { + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->viewer) + ->withMemberPHIDs(array($this->viewer->getPHID())) + ->execute(); + foreach ($projects as $project) { + $phids[] = $project->getPHID(); + } + } + + $capabilities = $this->object->getCapabilities(); + foreach ($capabilities as $capability) { + $policy = $this->object->getPolicy($capability); + if (!$policy) { + continue; + } + $phids[] = $policy; + } + + // If this install doesn't have "Public" enabled, don't include it as an + // option unless the object already has a "Public" policy. In this case we + // retain the policy but enforce it as though it was "All Users". + $show_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); + foreach ($this->getGlobalPolicies() as $phid => $policy) { + if ($phid == PhabricatorPolicies::POLICY_PUBLIC) { + if (!$show_public) { + continue; + } + } + $phids[] = $phid; + } + + return $phids; + } + } diff --git a/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php b/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php index a4f28aa5dc..7ac490ed28 100644 --- a/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php +++ b/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php @@ -8,7 +8,12 @@ final class PhabricatorPolicyRuleUsers } public function applyRule(PhabricatorUser $viewer, $value) { - return isset($value[$viewer->getPHID()]); + foreach ($value as $phid) { + if ($phid == $viewer->getPHID()) { + return true; + } + } + return false; } public function getValueControlType() { diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index b1f60f1268..e14f49c6f9 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -14,6 +14,8 @@ final class PhabricatorPolicy protected $rules = array(); protected $defaultAction = self::ACTION_DENY; + private $ruleObjects = self::ATTACHABLE; + public function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -228,4 +230,55 @@ final class PhabricatorPolicy return $desc; } } + + /** + * Return a list of custom rule classes (concrete subclasses of + * @{class:PhabricatorPolicyRule}) this policy uses. + * + * @return list List of class names. + */ + public function getCustomRuleClasses() { + $classes = array(); + + foreach ($this->getRules() as $rule) { + $class = idx($rule, 'rule'); + try { + if (class_exists($class)) { + $classes[$class] = $class; + } + } catch (Exception $ex) { + continue; + } + } + + return array_keys($classes); + } + + /** + * Return a list of all values used by a given rule class to implement this + * policy. This is used to bulk load data (like project memberships) in order + * to apply policy filters efficiently. + * + * @param string Policy rule classname. + * @return list List of values used in this policy. + */ + public function getCustomRuleValues($rule_class) { + $values = array(); + foreach ($this->getRules() as $rule) { + if ($rule['rule'] == $rule_class) { + $values[] = $rule['value']; + } + } + return $values; + } + + public function attachRuleObjects(array $objects) { + $this->ruleObjects = $objects; + return $this; + } + + public function getRuleObjects() { + return $this->assertAttached($this->ruleObjects); + } + }