1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00

Test for "CAN_INTERACT" on comment edits in a way that survives objects which only implement "CAN_VIEW"

Summary:
Ref T13289. See D20551. In D20551, I implemented some "CAN_INTERACT" checks against certain edits, but these checks end up testing "CAN_INTERACT" against objects like Conpherence threads which do not support a distinct "CAN_INTERACT" permission. I misrembered how the "CAN_INTERACT" fallback to "CAN_VIEW" actually works: it's not fully automatic, and needs some explicit "interact, or view if interact is not available" checks.

Use the "interact" wrappers to test these policies so they fall back to "CAN_VIEW" if an object does not support "CAN_INTERACT". Generally, objects which have a "locked" state have a separate "CAN_INTERACT" permission; objects which don't have a "locked" state do not.

Test Plan: Created and edited comments in Conpherence (or most applications other than Maniphest).

Reviewers: amckinley

Maniphest Tasks: T13289

Differential Revision: https://secure.phabricator.com/D20558
This commit is contained in:
epriestley 2019-05-28 10:11:09 -07:00
parent ce6fc5be90
commit 53b9acfb7d
4 changed files with 31 additions and 16 deletions

View file

@ -90,6 +90,29 @@ final class PhabricatorPolicyFilter extends Phobject {
PhabricatorUser $user, PhabricatorUser $user,
PhabricatorPolicyInterface $object) { PhabricatorPolicyInterface $object) {
$capabilities = self::getRequiredInteractCapabilities($object);
foreach ($capabilities as $capability) {
if (!self::hasCapability($user, $object, $capability)) {
return false;
}
}
return true;
}
public static function requireCanInteract(
PhabricatorUser $user,
PhabricatorPolicyInterface $object) {
$capabilities = self::getRequiredInteractCapabilities($object);
foreach ($capabilities as $capability) {
self::requireCapability($user, $object, $capability);
}
}
private static function getRequiredInteractCapabilities(
PhabricatorPolicyInterface $object) {
$capabilities = $object->getCapabilities(); $capabilities = $object->getCapabilities();
$capabilities = array_fuse($capabilities); $capabilities = array_fuse($capabilities);
@ -107,13 +130,7 @@ final class PhabricatorPolicyFilter extends Phobject {
$require[] = $can_interact; $require[] = $can_interact;
} }
foreach ($require as $capability) { return $require;
if (!self::hasCapability($user, $object, $capability)) {
return false;
}
}
return true;
} }
public function setViewer(PhabricatorUser $user) { public function setViewer(PhabricatorUser $user) {

View file

@ -36,10 +36,9 @@ final class PhabricatorApplicationTransactionCommentEditController
// auditing, and editing comments serves neither goal. // auditing, and editing comments serves neither goal.
$object = $xaction->getObject(); $object = $xaction->getObject();
$can_interact = PhabricatorPolicyFilter::hasCapability( $can_interact = PhabricatorPolicyFilter::canInteract(
$viewer, $viewer,
$object, $object);
PhabricatorPolicyCapability::CAN_INTERACT);
if (!$can_interact) { if (!$can_interact) {
return $this->newDialog() return $this->newDialog()
->setTitle(pht('Conversation Locked')) ->setTitle(pht('Conversation Locked'))

View file

@ -189,10 +189,10 @@ final class PhabricatorApplicationTransactionCommentEditor
$actor, $actor,
$xaction, $xaction,
PhabricatorPolicyCapability::CAN_EDIT); PhabricatorPolicyCapability::CAN_EDIT);
PhabricatorPolicyFilter::requireCapability(
PhabricatorPolicyFilter::requireCanInteract(
$actor, $actor,
$xaction->getObject(), $xaction->getObject());
PhabricatorPolicyCapability::CAN_INTERACT);
} }
} }

View file

@ -513,10 +513,9 @@ class PhabricatorApplicationTransactionView extends AphrontView {
} }
} }
$can_interact = PhabricatorPolicyFilter::hasCapability( $can_interact = PhabricatorPolicyFilter::canInteract(
$viewer, $viewer,
$xaction->getObject(), $xaction->getObject());
PhabricatorPolicyCapability::CAN_INTERACT);
$event->setCanInteract($can_interact); $event->setCanInteract($can_interact);
} }