@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Extend PhabricatorPolicyCodex interface to handle "interesting" policy defaults

Summary:
Fixes T13128. Ref PHI590. This is a rough-and-ready implementation of a new `PhabricatorPolicyCodex->compareToDefaultPolicy()` method that subclasses can override to handle special cases of policy defaults. Also implements a `PolicyCodex` for Phriction documents, because the default policy of a Phriction document is the policy of the root document.

I might break this change into two parts, one of which maintains the current behavior and another which implements `PhrictionDocumentPolicyCodex`.

Test Plan: Created some Phriction docs, fiddled with policies, observed expected colors in the header. Will test more comprehensively after review for basic reasonable-ness.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, swisspol

Maniphest Tasks: T13128

Differential Revision: https://secure.phabricator.com/D19409

+216 -58
+5
src/__phutil_library_map__.php
··· 3867 3867 'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php', 3868 3868 'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php', 3869 3869 'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php', 3870 + 'PhabricatorPolicyStrengthConstants' => 'applications/policy/constants/PhabricatorPolicyStrengthConstants.php', 3870 3871 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php', 3871 3872 'PhabricatorPolicyTestObject' => 'applications/policy/__tests__/PhabricatorPolicyTestObject.php', 3872 3873 'PhabricatorPolicyType' => 'applications/policy/constants/PhabricatorPolicyType.php', ··· 4998 4999 'PhrictionDocumentMoveToTransaction' => 'applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php', 4999 5000 'PhrictionDocumentPHIDType' => 'applications/phriction/phid/PhrictionDocumentPHIDType.php', 5000 5001 'PhrictionDocumentPathHeraldField' => 'applications/phriction/herald/PhrictionDocumentPathHeraldField.php', 5002 + 'PhrictionDocumentPolicyCodex' => 'applications/phriction/codex/PhrictionDocumentPolicyCodex.php', 5001 5003 'PhrictionDocumentQuery' => 'applications/phriction/query/PhrictionDocumentQuery.php', 5002 5004 'PhrictionDocumentSearchConduitAPIMethod' => 'applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php', 5003 5005 'PhrictionDocumentSearchEngine' => 'applications/phriction/query/PhrictionDocumentSearchEngine.php', ··· 9669 9671 'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 9670 9672 'PhabricatorPolicyRule' => 'Phobject', 9671 9673 'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension', 9674 + 'PhabricatorPolicyStrengthConstants' => 'PhabricatorPolicyConstants', 9672 9675 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', 9673 9676 'PhabricatorPolicyTestObject' => array( 9674 9677 'Phobject', ··· 11060 11063 'PhabricatorProjectInterface', 11061 11064 'PhabricatorApplicationTransactionInterface', 11062 11065 'PhabricatorConduitResultInterface', 11066 + 'PhabricatorPolicyCodexInterface', 11063 11067 ), 11064 11068 'PhrictionDocumentAuthorHeraldField' => 'PhrictionDocumentHeraldField', 11065 11069 'PhrictionDocumentContentHeraldField' => 'PhrictionDocumentHeraldField', ··· 11076 11080 'PhrictionDocumentMoveToTransaction' => 'PhrictionDocumentTransactionType', 11077 11081 'PhrictionDocumentPHIDType' => 'PhabricatorPHIDType', 11078 11082 'PhrictionDocumentPathHeraldField' => 'PhrictionDocumentHeraldField', 11083 + 'PhrictionDocumentPolicyCodex' => 'PhabricatorPolicyCodex', 11079 11084 'PhrictionDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 11080 11085 'PhrictionDocumentSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 11081 11086 'PhrictionDocumentSearchEngine' => 'PhabricatorApplicationSearchEngine',
+82
src/applications/phriction/codex/PhrictionDocumentPolicyCodex.php
··· 1 + <?php 2 + 3 + final class PhrictionDocumentPolicyCodex 4 + extends PhabricatorPolicyCodex { 5 + 6 + public function getPolicySpecialRuleDescriptions() { 7 + $object = $this->getObject(); 8 + $strongest_policy = $this->getStrongestPolicy(); 9 + 10 + $rules = array(); 11 + $rules[] = $this->newRule() 12 + ->setDescription( 13 + pht('To view a wiki document, you must also be able to view all '. 14 + 'of its ancestors. The most-restrictive view policy of this '. 15 + 'document\'s ancestors is "%s".', 16 + $strongest_policy->getShortName())) 17 + ->setCapabilities(array(PhabricatorPolicyCapability::CAN_VIEW)); 18 + 19 + $rules[] = $this->newRule() 20 + ->setDescription( 21 + pht('To edit a wiki document, you must also be able to view all '. 22 + 'of its ancestors.')) 23 + ->setCapabilities(array(PhabricatorPolicyCapability::CAN_EDIT)); 24 + 25 + return $rules; 26 + } 27 + 28 + public function getDefaultPolicy() { 29 + $ancestors = $this->getObject()->getAncestors(); 30 + if ($ancestors) { 31 + $root = head($ancestors); 32 + } else { 33 + $root = $this->getObject(); 34 + } 35 + 36 + $root_policy_phid = $root->getPolicy($this->getCapability()); 37 + 38 + return id(new PhabricatorPolicyQuery()) 39 + ->setViewer($this->getViewer()) 40 + ->withPHIDs(array($root_policy_phid)) 41 + ->executeOne(); 42 + } 43 + 44 + public function compareToDefaultPolicy(PhabricatorPolicy $policy) { 45 + $root_policy = $this->getDefaultPolicy(); 46 + $strongest_policy = $this->getStrongestPolicy(); 47 + 48 + // Note that we never return 'weaker', because Phriction documents can 49 + // never have weaker permissions than their parents. If this object has 50 + // been set to weaker permissions anyway, return 'adjusted'. 51 + if ($root_policy == $strongest_policy) { 52 + $strength = null; 53 + } else if ($strongest_policy->isStrongerThan($root_policy)) { 54 + $strength = PhabricatorPolicyStrengthConstants::STRONGER; 55 + } else { 56 + $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; 57 + } 58 + return $strength; 59 + } 60 + 61 + private function getStrongestPolicy() { 62 + $ancestors = $this->getObject()->getAncestors(); 63 + $ancestors[] = $this->getObject(); 64 + 65 + $strongest_policy = $this->getDefaultPolicy(); 66 + foreach ($ancestors as $ancestor) { 67 + $ancestor_policy_phid = $ancestor->getPolicy($this->getCapability()); 68 + 69 + $ancestor_policy = id(new PhabricatorPolicyQuery()) 70 + ->setViewer($this->getViewer()) 71 + ->withPHIDs(array($ancestor_policy_phid)) 72 + ->executeOne(); 73 + 74 + if ($ancestor_policy->isStrongerThan($strongest_policy)) { 75 + $strongest_policy = $ancestor_policy; 76 + } 77 + } 78 + 79 + return $strongest_policy; 80 + } 81 + 82 + }
+9 -4
src/applications/phriction/controller/PhrictionEditController.php
··· 219 219 ->execute(); 220 220 $view_capability = PhabricatorPolicyCapability::CAN_VIEW; 221 221 $edit_capability = PhabricatorPolicyCapability::CAN_EDIT; 222 + $codex = id(PhabricatorPolicyCodex::newFromObject($document, $viewer)) 223 + ->setCapability($view_capability); 224 + 225 + $view_capability_description = $codex->getPolicySpecialRuleForCapability( 226 + PhabricatorPolicyCapability::CAN_VIEW)->getDescription(); 227 + $edit_capability_description = $codex->getPolicySpecialRuleForCapability( 228 + PhabricatorPolicyCapability::CAN_EDIT)->getDescription(); 222 229 223 230 $form = id(new AphrontFormView()) 224 231 ->setUser($viewer) ··· 264 271 ->setPolicyObject($document) 265 272 ->setCapability($view_capability) 266 273 ->setPolicies($policies) 267 - ->setCaption( 268 - $document->describeAutomaticCapability($view_capability))) 274 + ->setCaption($view_capability_description)) 269 275 ->appendChild( 270 276 id(new AphrontFormPolicyControl()) 271 277 ->setName('editPolicy') 272 278 ->setPolicyObject($document) 273 279 ->setCapability($edit_capability) 274 280 ->setPolicies($policies) 275 - ->setCaption( 276 - $document->describeAutomaticCapability($edit_capability))) 281 + ->setCaption($edit_capability_description)) 277 282 ->appendChild( 278 283 id(new AphrontFormTextControl()) 279 284 ->setLabel(pht('Edit Notes'))
+11 -17
src/applications/phriction/storage/PhrictionDocument.php
··· 11 11 PhabricatorFerretInterface, 12 12 PhabricatorProjectInterface, 13 13 PhabricatorApplicationTransactionInterface, 14 - PhabricatorConduitResultInterface { 14 + PhabricatorConduitResultInterface, 15 + PhabricatorPolicyCodexInterface { 15 16 16 17 protected $slug; 17 18 protected $depth; ··· 200 201 return false; 201 202 } 202 203 203 - public function describeAutomaticCapability($capability) { 204 - 205 - switch ($capability) { 206 - case PhabricatorPolicyCapability::CAN_VIEW: 207 - return pht( 208 - 'To view a wiki document, you must also be able to view all '. 209 - 'of its parents.'); 210 - case PhabricatorPolicyCapability::CAN_EDIT: 211 - return pht( 212 - 'To edit a wiki document, you must also be able to view all '. 213 - 'of its parents.'); 214 - } 215 - 216 - return null; 217 - } 218 - 219 204 220 205 /* -( PhabricatorSubscribableInterface )----------------------------------- */ 221 206 ··· 328 313 ->setAttachmentKey('content'), 329 314 ); 330 315 } 316 + 317 + /* -( PhabricatorPolicyCodexInterface )------------------------------------ */ 318 + 319 + 320 + public function newPolicyCodex() { 321 + return new PhrictionDocumentPolicyCodex(); 322 + } 323 + 324 + 331 325 }
+21
src/applications/policy/codex/PhabricatorPolicyCodex.php
··· 29 29 return array(); 30 30 } 31 31 32 + public function getDefaultPolicy() { 33 + return PhabricatorPolicyQuery::getDefaultPolicyForObject( 34 + $this->viewer, 35 + $this->object, 36 + $this->capability); 37 + } 38 + 39 + public function compareToDefaultPolicy(PhabricatorPolicy $policy) { 40 + return null; 41 + } 42 + 43 + final public function getPolicySpecialRuleForCapability($capability) { 44 + foreach ($this->getPolicySpecialRuleDescriptions() as $rule) { 45 + if (in_array($capability, $rule->getCapabilities())) { 46 + return $rule; 47 + } 48 + } 49 + 50 + return null; 51 + } 52 + 32 53 final protected function newRule() { 33 54 return new PhabricatorPolicyCodexRuleDescription(); 34 55 }
+9
src/applications/policy/constants/PhabricatorPolicyStrengthConstants.php
··· 1 + <?php 2 + 3 + final class PhabricatorPolicyStrengthConstants 4 + extends PhabricatorPolicyConstants { 5 + 6 + const WEAKER = 'weaker'; 7 + const STRONGER = 'stronger'; 8 + const ADJUSTED = 'adjusted'; 9 + }
+31 -9
src/applications/policy/controller/PhabricatorPolicyExplainController.php
··· 169 169 $capability) { 170 170 $viewer = $this->getViewer(); 171 171 172 - $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( 173 - $viewer, 174 - $object, 175 - $capability); 176 - if (!$default_policy) { 177 - return; 172 + 173 + $strength = null; 174 + if ($object instanceof PhabricatorPolicyCodexInterface) { 175 + $codex = id(PhabricatorPolicyCodex::newFromObject($object, $viewer)) 176 + ->setCapability($capability); 177 + $strength = $codex->compareToDefaultPolicy($policy); 178 + $default_policy = $codex->getDefaultPolicy(); 179 + } else { 180 + $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( 181 + $viewer, 182 + $object, 183 + $capability); 184 + 185 + if ($default_policy) { 186 + if ($default_policy->getPHID() == $policy->getPHID()) { 187 + return; 188 + } 189 + 190 + if ($default_policy->getPHID() != $policy->getPHID()) { 191 + if ($default_policy->isStrongerThan($policy)) { 192 + $strength = PhabricatorPolicyStrengthConstants::WEAKER; 193 + } else if ($policy->isStrongerThan($default_policy)) { 194 + $strength = PhabricatorPolicyStrengthConstants::STRONGER; 195 + } else { 196 + $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; 197 + } 198 + } 199 + } 178 200 } 179 201 180 - if ($default_policy->getPHID() == $policy->getPHID()) { 202 + if (!$strength) { 181 203 return; 182 204 } 183 205 184 - if ($default_policy->isStrongerThan($policy)) { 206 + if ($strength == PhabricatorPolicyStrengthConstants::WEAKER) { 185 207 $info = pht( 186 208 'This object has a less restrictive policy ("%s") than the default '. 187 209 'policy for similar objects (which is "%s").', 188 210 $policy->getShortName(), 189 211 $default_policy->getShortName()); 190 - } else if ($policy->isStrongerThan($default_policy)) { 212 + } else if ($strength == PhabricatorPolicyStrengthConstants::STRONGER) { 191 213 $info = pht( 192 214 'This object has a more restrictive policy ("%s") than the default '. 193 215 'policy for similar objects (which is "%s").',
+10 -7
src/applications/policy/storage/PhabricatorPolicy.php
··· 434 434 $capability, 435 435 $active_only) { 436 436 437 + $exceptions = array(); 437 438 if ($object instanceof PhabricatorPolicyCodexInterface) { 438 - $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); 439 + $codex = id(PhabricatorPolicyCodex::newFromObject($object, $viewer)) 440 + ->setCapability($capability); 439 441 $rules = $codex->getPolicySpecialRuleDescriptions(); 440 442 441 - $exceptions = array(); 442 443 foreach ($rules as $rule) { 443 444 $is_active = $rule->getIsActive(); 444 445 if ($is_active) { ··· 467 468 468 469 $exceptions[] = $description; 469 470 } 470 - } else if (method_exists($object, 'describeAutomaticCapability')) { 471 - $exceptions = (array)$object->describeAutomaticCapability($capability); 472 - $exceptions = array_filter($exceptions); 473 - } else { 474 - $exceptions = array(); 471 + } 472 + 473 + if (!$exceptions) { 474 + if (method_exists($object, 'describeAutomaticCapability')) { 475 + $exceptions = (array)$object->describeAutomaticCapability($capability); 476 + $exceptions = array_filter($exceptions); 477 + } 475 478 } 476 479 477 480 return $exceptions;
+38 -21
src/view/phui/PHUIHeaderView.php
··· 473 473 // policy differs from the default policy. If it does, we'll call it out 474 474 // as changed. 475 475 if (!$use_space_policy) { 476 - $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( 477 - $viewer, 478 - $object, 479 - $view_capability); 480 - if ($default_policy) { 481 - if ($default_policy->getPHID() != $policy->getPHID()) { 482 - $container_classes[] = 'policy-adjusted'; 483 - if ($default_policy->isStrongerThan($policy)) { 484 - // The policy has strictly been weakened. For example, the 485 - // default might be "All Users" and the current policy is "Public". 486 - $container_classes[] = 'policy-adjusted-weaker'; 487 - } else if ($policy->isStrongerThan($default_policy)) { 488 - // The policy has strictly been strengthened, and is now more 489 - // restrictive than the default. For example, "All Users" has 490 - // been replaced with "No One". 491 - $container_classes[] = 'policy-adjusted-stronger'; 492 - } else { 493 - // The policy has been adjusted but not strictly strengthened 494 - // or weakened. For example, "Members of X" has been replaced with 495 - // "Members of Y". 496 - $container_classes[] = 'policy-adjusted-different'; 476 + $strength = null; 477 + if ($object instanceof PhabricatorPolicyCodexInterface) { 478 + $codex = id(PhabricatorPolicyCodex::newFromObject($object, $viewer)) 479 + ->setCapability($view_capability); 480 + $strength = $codex->compareToDefaultPolicy($policy); 481 + } else { 482 + $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( 483 + $viewer, 484 + $object, 485 + $view_capability); 486 + 487 + if ($default_policy) { 488 + if ($default_policy->getPHID() != $policy->getPHID()) { 489 + if ($default_policy->isStrongerThan($policy)) { 490 + $strength = PhabricatorPolicyStrengthConstants::WEAKER; 491 + } else if ($policy->isStrongerThan($default_policy)) { 492 + $strength = PhabricatorPolicyStrengthConstants::STRONGER; 493 + } else { 494 + $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; 495 + } 497 496 } 497 + } 498 + } 499 + 500 + if ($strength) { 501 + if ($strength == PhabricatorPolicyStrengthConstants::WEAKER) { 502 + // The policy has strictly been weakened. For example, the 503 + // default might be "All Users" and the current policy is "Public". 504 + $container_classes[] = 'policy-adjusted-weaker'; 505 + } else if ($strength == PhabricatorPolicyStrengthConstants::STRONGER) { 506 + // The policy has strictly been strengthened, and is now more 507 + // restrictive than the default. For example, "All Users" has 508 + // been replaced with "No One". 509 + $container_classes[] = 'policy-adjusted-stronger'; 510 + } else { 511 + // The policy has been adjusted but not strictly strengthened 512 + // or weakened. For example, "Members of X" has been replaced with 513 + // "Members of Y". 514 + $container_classes[] = 'policy-adjusted-different'; 498 515 } 499 516 } 500 517 }