Page MenuHomePhabricator
Files F15384409

D14993.diff
No OneTemporary
Actions

D14993.diff
View Options

diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php
--- a/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php
+++ b/src/applications/policy/__tests__/PhabricatorPolicyTestCase.php
@@ -285,6 +285,76 @@
pht('Extended Policy with Cycle'));
}
+
+ /**
+ * Test bulk checks of extended policies.
+ *
+ * This is testing an issue with extended policy filtering which allowed
+ * unusual inputs to slip objects through the filter. See D14993.
+ */
+ public function testBulkExtendedPolicies() {
+ $object1 = $this->buildObject(PhabricatorPolicies::POLICY_USER)
+ ->setPHID('PHID-TEST-1');
+ $object2 = $this->buildObject(PhabricatorPolicies::POLICY_USER)
+ ->setPHID('PHID-TEST-2');
+ $object3 = $this->buildObject(PhabricatorPolicies::POLICY_USER)
+ ->setPHID('PHID-TEST-3');
+
+ $extended = $this->buildObject(PhabricatorPolicies::POLICY_ADMIN)
+ ->setPHID('PHID-TEST-999');
+
+ $object1->setExtendedPolicies(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW => array(
+ array(
+ $extended,
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ),
+ ),
+ ),
+ ));
+
+ $object2->setExtendedPolicies(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW => array(
+ array($extended, PhabricatorPolicyCapability::CAN_VIEW),
+ ),
+ ));
+
+ $object3->setExtendedPolicies(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW => array(
+ array(
+ $extended,
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ),
+ ),
+ ),
+ ));
+
+ $user = $this->buildUser('user');
+
+ $visible = id(new PhabricatorPolicyFilter())
+ ->setViewer($user)
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ ))
+ ->apply(
+ array(
+ $object1,
+ $object2,
+ $object3,
+ ));
+
+ $this->assertEqual(array(), $visible);
+ }
+
+
/**
* An omnipotent user should be able to see even objects with invalid
* policies.
@@ -430,10 +500,12 @@
$object->setCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
));
$object->setPolicies(
array(
PhabricatorPolicyCapability::CAN_VIEW => $policy,
+ PhabricatorPolicyCapability::CAN_EDIT => $policy,
));
return $object;
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
@@ -321,7 +321,7 @@
$objects_in = array();
foreach ($structs as $struct) {
$extended_key = $struct['key'];
- if (empty($extended_objects[$key])) {
+ if (empty($extended_objects[$extended_key])) {
// If this object has already been rejected by an earlier filtering
// pass, we don't need to do any tests on it.
continue;
@@ -335,8 +335,8 @@
// We weren't able to load the corresponding object, so just
// reject this result outright.
- $reject = $extended_objects[$key];
- unset($extended_objects[$key]);
+ $reject = $extended_objects[$extended_key];
+ unset($extended_objects[$extended_key]);
// TODO: This could be friendlier.
$this->rejectObject($reject, false, '<bad-ref>');

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 15, 7:48 PM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7650461
Default Alt Text
D14993.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility