mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-02 19:01:03 +01:00
Use an extended policy to bind column and board policies together
Summary: Ref T10349. Columns have the same policies as the projects they belong to. However, the current implementation just returns the policy directly. This usually works, but if the project has a policy like "Members of (This) Project", the policy filter tries to check if the viewer is a member of //the column itself//. That doesn't work, since columns don't have members. This leads to a situation where columns on "Editable By: Project Members" projects can not be edited. Instead, return a permissive base policy and then use an extended policy to bind the column policy to the project policy. Test Plan: - Edited a column on an "Editable By: Members of Project" board. - Added and ran a unit test covering this case. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10349 Differential Revision: https://secure.phabricator.com/D15268
This commit is contained in:
parent
3f50ba90f1
commit
329d03661f
4 changed files with 83 additions and 4 deletions
|
@ -7305,6 +7305,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorApplicationTransactionInterface',
|
'PhabricatorApplicationTransactionInterface',
|
||||||
'PhabricatorPolicyInterface',
|
'PhabricatorPolicyInterface',
|
||||||
'PhabricatorDestructibleInterface',
|
'PhabricatorDestructibleInterface',
|
||||||
|
'PhabricatorExtendedPolicyInterface',
|
||||||
),
|
),
|
||||||
'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController',
|
'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController',
|
||||||
'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController',
|
'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController',
|
||||||
|
|
|
@ -1013,6 +1013,51 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
|
||||||
$this->assertColumns($expect, $user, $board, $task);
|
$this->assertColumns($expect, $user, $board, $task);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testColumnExtendedPolicies() {
|
||||||
|
$user = $this->createUser();
|
||||||
|
$user->save();
|
||||||
|
|
||||||
|
$board = $this->createProject($user);
|
||||||
|
$column = $this->addColumn($user, $board, 0);
|
||||||
|
|
||||||
|
// At first, the user should be able to view and edit the column.
|
||||||
|
$column = $this->refreshColumn($user, $column);
|
||||||
|
$this->assertTrue((bool)$column);
|
||||||
|
|
||||||
|
$can_edit = PhabricatorPolicyFilter::hasCapability(
|
||||||
|
$user,
|
||||||
|
$column,
|
||||||
|
PhabricatorPolicyCapability::CAN_EDIT);
|
||||||
|
$this->assertTrue($can_edit);
|
||||||
|
|
||||||
|
// Now, set the project edit policy to "Members of Project". This should
|
||||||
|
// disable editing.
|
||||||
|
$members_policy = id(new PhabricatorProjectMembersPolicyRule())
|
||||||
|
->getObjectPolicyFullKey();
|
||||||
|
$board->setEditPolicy($members_policy)->save();
|
||||||
|
|
||||||
|
$column = $this->refreshColumn($user, $column);
|
||||||
|
$this->assertTrue((bool)$column);
|
||||||
|
|
||||||
|
$can_edit = PhabricatorPolicyFilter::hasCapability(
|
||||||
|
$user,
|
||||||
|
$column,
|
||||||
|
PhabricatorPolicyCapability::CAN_EDIT);
|
||||||
|
$this->assertFalse($can_edit);
|
||||||
|
|
||||||
|
// Now, join the project. This should make the column editable again.
|
||||||
|
$this->joinProject($board, $user);
|
||||||
|
|
||||||
|
$column = $this->refreshColumn($user, $column);
|
||||||
|
$this->assertTrue((bool)$column);
|
||||||
|
|
||||||
|
$can_edit = PhabricatorPolicyFilter::hasCapability(
|
||||||
|
$user,
|
||||||
|
$column,
|
||||||
|
PhabricatorPolicyCapability::CAN_EDIT);
|
||||||
|
$this->assertTrue($can_edit);
|
||||||
|
}
|
||||||
|
|
||||||
private function moveToColumn(
|
private function moveToColumn(
|
||||||
PhabricatorUser $viewer,
|
PhabricatorUser $viewer,
|
||||||
PhabricatorProject $board,
|
PhabricatorProject $board,
|
||||||
|
@ -1251,6 +1296,22 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function refreshColumn(
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
PhabricatorProjectColumn $column) {
|
||||||
|
|
||||||
|
$results = id(new PhabricatorProjectColumnQuery())
|
||||||
|
->setViewer($viewer)
|
||||||
|
->withIDs(array($column->getID()))
|
||||||
|
->execute();
|
||||||
|
|
||||||
|
if ($results) {
|
||||||
|
return head($results);
|
||||||
|
} else {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private function createProject(
|
private function createProject(
|
||||||
PhabricatorUser $user,
|
PhabricatorUser $user,
|
||||||
PhabricatorProject $parent = null,
|
PhabricatorProject $parent = null,
|
||||||
|
|
|
@ -73,8 +73,7 @@ final class PhabricatorProjectColumnDetailController
|
||||||
|
|
||||||
$header = id(new PHUIHeaderView())
|
$header = id(new PHUIHeaderView())
|
||||||
->setUser($viewer)
|
->setUser($viewer)
|
||||||
->setHeader($column->getDisplayName())
|
->setHeader($column->getDisplayName());
|
||||||
->setPolicyObject($column);
|
|
||||||
|
|
||||||
if ($column->isHidden()) {
|
if ($column->isHidden()) {
|
||||||
$header->setStatus('fa-ban', 'dark', pht('Hidden'));
|
$header->setStatus('fa-ban', 'dark', pht('Hidden'));
|
||||||
|
|
|
@ -5,7 +5,8 @@ final class PhabricatorProjectColumn
|
||||||
implements
|
implements
|
||||||
PhabricatorApplicationTransactionInterface,
|
PhabricatorApplicationTransactionInterface,
|
||||||
PhabricatorPolicyInterface,
|
PhabricatorPolicyInterface,
|
||||||
PhabricatorDestructibleInterface {
|
PhabricatorDestructibleInterface,
|
||||||
|
PhabricatorExtendedPolicyInterface {
|
||||||
|
|
||||||
const STATUS_ACTIVE = 0;
|
const STATUS_ACTIVE = 0;
|
||||||
const STATUS_HIDDEN = 1;
|
const STATUS_HIDDEN = 1;
|
||||||
|
@ -219,7 +220,14 @@ final class PhabricatorProjectColumn
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getPolicy($capability) {
|
public function getPolicy($capability) {
|
||||||
return $this->getProject()->getPolicy($capability);
|
// NOTE: Column policies are enforced as an extended policy which makes
|
||||||
|
// them the same as the project's policies.
|
||||||
|
switch ($capability) {
|
||||||
|
case PhabricatorPolicyCapability::CAN_VIEW:
|
||||||
|
return PhabricatorPolicies::getMostOpenPolicy();
|
||||||
|
case PhabricatorPolicyCapability::CAN_EDIT:
|
||||||
|
return PhabricatorPolicies::POLICY_USER;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
|
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
|
||||||
|
@ -233,6 +241,16 @@ final class PhabricatorProjectColumn
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/* -( PhabricatorExtendedPolicyInterface )--------------------------------- */
|
||||||
|
|
||||||
|
|
||||||
|
public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
|
||||||
|
return array(
|
||||||
|
array($this->getProject(), $capability),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( PhabricatorDestructibleInterface )----------------------------------- */
|
/* -( PhabricatorDestructibleInterface )----------------------------------- */
|
||||||
|
|
||||||
public function destroyObjectPermanently(
|
public function destroyObjectPermanently(
|
||||||
|
|
Loading…
Reference in a new issue