Page MenuHomePhabricator
Files F71071

D7292.diff
All Users
Actions

D7292.diff
View Options

diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@@ -20,7 +20,7 @@
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
--- a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php
+++ b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php
@@ -31,4 +31,117 @@
$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
--- a/src/applications/policy/filter/PhabricatorPolicyFilter.php
+++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php
@@ -7,6 +7,7 @@
private $capabilities;
private $raisePolicyExceptions;
private $userProjects;
+ private $customPolicies = array();
public static function mustRetainCapability(
PhabricatorUser $user,
@@ -85,6 +86,7 @@
}
$need_projects = array();
+ $need_policies = array();
foreach ($objects as $key => $object) {
$object_capabilities = $object->getCapabilities();
foreach ($capabilities as $capability) {
@@ -99,9 +101,17 @@
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 @@
} 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 @@
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
--- a/src/applications/policy/query/PhabricatorPolicyQuery.php
+++ b/src/applications/policy/query/PhabricatorPolicyQuery.php
@@ -26,31 +26,19 @@
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 @@
throw new Exception('Call setViewer() before execute()!');
}
- $results = $this->getGlobalPolicies();
-
- 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());
- }
- }
+ 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;
}
- $results = mpull($results, null, 'getPHID');
+ $phids = array_fuse($phids);
- $other_policies = array();
- if ($this->object) {
- $capabilities = $this->object->getCapabilities();
- foreach ($capabilities as $capability) {
- $policy = $this->object->getPolicy($capability);
- if (!$policy) {
- continue;
- }
- $other_policies[$policy] = $policy;
+ $results = array();
+
+ // First, load global policies.
+ foreach ($this->getGlobalPolicies() as $phid => $policy) {
+ if (isset($phids[$phid])) {
+ $results[$phid] = $policy;
+ unset($phids[$phid]);
}
}
- // 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 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 = array_diff_key($other_policies, $results);
+ 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]);
+ }
+ }
- 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 @@
}
}
+ 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
--- a/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRuleUsers.php
@@ -8,7 +8,12 @@
}
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
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -14,6 +14,8 @@
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 @@
return $desc;
}
}
+
+ /**
+ * Return a list of custom rule classes (concrete subclasses of
+ * @{class:PhabricatorPolicyRule}) this policy uses.
+ *
+ * @return list<string> 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<wild> 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);
+ }
+
}

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/ji/pn/e53qcssah2wdveff
Default Alt Text
D7292.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility