1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 10:12:41 +01:00

Fix join and remove policy checks for Conpherence

Summary:
I think these got munged when I removed CAN_JOIN.

 - If you can view the room, you can join it.
 - ~~If you can view the room, you can add others to it.~~ This rule adjustment was removed, see discussion on the revision.
 - If you are a participant in the room, you can remove yourself.
 - If you can edit a room, you can remove anyone.

Test Plan:
Normal feature set:

 - Create a new room that only I can edit, viewable by all users.
 - Leave room (bye k thx)
 - Create another room, myself only
 - Join room from second account
 - See ability to only remove myself
 - Remove myself
 - Rejoin
 - Add third account
 - Log into first account
 - Boot off randos
 - Test joining by green button, message, and by + sign.

Policy consistency:

  - As a user who can not edit the room, tried to add other members. Received policy exception. The `+` button is currently visible and enabled for all users (even users who have not joined the room) but this is pre-existing.

Reviewers: chad

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17696
This commit is contained in:
Chad Little 2017-04-15 06:08:00 -07:00 committed by epriestley
parent f0fbf7a7d3
commit c1e8b394cc
2 changed files with 16 additions and 9 deletions

View file

@ -147,9 +147,6 @@ final class ConpherenceViewController extends
$user = $this->getRequest()->getUser(); $user = $this->getRequest()->getUser();
$participating = $conpherence->getParticipantIfExists($user->getPHID()); $participating = $conpherence->getParticipantIfExists($user->getPHID());
if (!$participating && $user->isLoggedIn()) {
return null;
}
$draft = PhabricatorDraft::newFromUserAndKey( $draft = PhabricatorDraft::newFromUserAndKey(
$user, $user,
$conpherence->getPHID()); $conpherence->getPHID());

View file

@ -341,13 +341,23 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
$add = array_keys(array_diff_key($new_map, $old_map)); $add = array_keys(array_diff_key($new_map, $old_map));
$rem = array_keys(array_diff_key($old_map, $new_map)); $rem = array_keys(array_diff_key($old_map, $new_map));
$actor_phid = $this->requireActor()->getPHID(); $actor_phid = $this->getActingAsPHID();
// You need CAN_EDIT to change participants other than yourself. $is_join = (($add === array($actor_phid)) && !$rem);
PhabricatorPolicyFilter::requireCapability( $is_leave = (($rem === array($actor_phid)) && !$add);
$this->requireActor(),
$object, if ($is_join) {
PhabricatorPolicyCapability::CAN_EDIT); // Anyone can join a thread they can see.
} else if ($is_leave) {
// Anyone can leave a thread.
} else {
// You need CAN_EDIT to add or remove participants. For additional
// discussion, see D17696 and T4411.
PhabricatorPolicyFilter::requireCapability(
$this->requireActor(),
$object,
PhabricatorPolicyCapability::CAN_EDIT);
}
break; break;
case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE: case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE: