From 517372a1e0c48d0bc1961af06e132b8537776565 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Sat, 15 Apr 2017 06:08:00 -0700
Subject: [PATCH] (stable) 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
---
 .../controller/ConpherenceViewController.php  |  3 ---
 .../conpherence/editor/ConpherenceEditor.php  | 22 ++++++++++++++-----
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php
index 6b953b9de4..7a152a1e2e 100644
--- a/src/applications/conpherence/controller/ConpherenceViewController.php
+++ b/src/applications/conpherence/controller/ConpherenceViewController.php
@@ -147,9 +147,6 @@ final class ConpherenceViewController extends
     $user = $this->getRequest()->getUser();
 
     $participating = $conpherence->getParticipantIfExists($user->getPHID());
-    if (!$participating && $user->isLoggedIn()) {
-      return null;
-    }
     $draft = PhabricatorDraft::newFromUserAndKey(
       $user,
       $conpherence->getPHID());
diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php
index e358700348..c53aab7655 100644
--- a/src/applications/conpherence/editor/ConpherenceEditor.php
+++ b/src/applications/conpherence/editor/ConpherenceEditor.php
@@ -341,13 +341,23 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
         $add = array_keys(array_diff_key($new_map, $old_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.
-        PhabricatorPolicyFilter::requireCapability(
-          $this->requireActor(),
-          $object,
-          PhabricatorPolicyCapability::CAN_EDIT);
+        $is_join = (($add === array($actor_phid)) && !$rem);
+        $is_leave = (($rem === array($actor_phid)) && !$add);
+
+        if ($is_join) {
+          // 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;
       case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE: