Conpherence - fix policy bug on comments with files in rooms

Summary: Fixes T7840. Add some unit tests. One explicitly covers the case in T7840. Other cases are "base" cases for rooms and threads, plus the version if T7840 for a thread, where a policy exception should be thrown.

Test Plan: `arc unit` and was able to successfully add a file to a conpherence room i did not have edit permissions on

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7840

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

src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php

@@ -126,6 +126,29 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase {
     }
   }
 
+  public function testAddMessageWithFileAttachments() {
+    $creator = $this->generateNewTestUser();
+    $friend_1 = $this->generateNewTestUser();
+    $join_via_add = $this->generateNewTestUser();
+
+    $participant_map = array(
+      $creator->getPHID() => $creator,
+      $friend_1->getPHID() => $friend_1,
+    );
+
+    $conpherence = $this->createRoom(
+      $creator,
+      array_keys($participant_map));
+
+    foreach ($participant_map as $phid => $user) {
+      $xactions = $this->addMessageWithFile($user, $conpherence);
+      $this->assertEqual(2, count($xactions));
+    }
+
+    $xactions = $this->addMessageWithFile($join_via_add, $conpherence);
+    $this->assertEqual(2, count($xactions));
+  }
+
   private function createRoom(
     PhabricatorUser $creator,
     array $participant_phids) {

src/applications/conpherence/__tests__/ConpherenceTestCase.php

@@ -33,5 +33,43 @@ abstract class ConpherenceTestCase extends PhabricatorTestCase {
       ->applyTransactions($conpherence, $xactions);
   }
 
+  protected function addMessageWithFile(
+    PhabricatorUser $actor,
+    ConpherenceThread $conpherence) {
+
+    $file = $this->generateTestFile($actor);
+    $message = Filesystem::readRandomCharacters(64).
+      sprintf(' {%s} ', $file->getMonogram());
+
+    $editor = id(new ConpherenceEditor())
+      ->setActor($actor)
+      ->setContentSource(PhabricatorContentSource::newConsoleSource());
+
+    $xactions = $editor->generateTransactionsFromText(
+      $actor,
+      $conpherence,
+      $message);
+
+    return $editor->applyTransactions($conpherence, $xactions);
+  }
+
+  private function generateTestFile(PhabricatorUser $actor) {
+    $engine = new PhabricatorTestStorageEngine();
+    $data = Filesystem::readRandomCharacters(64);
+
+    $params = array(
+      'name' => 'test.'.$actor->getPHID(),
+      'viewPolicy' => $actor->getPHID(),
+      'authorPHID' => $actor->getPHID(),
+      'storageEngines' => array(
+        $engine,
+      ),
+    );
+
+    $file = PhabricatorFile::newFromFileData($data, $params);
+    $file->save();
+
+    return $file;
+  }
 
 }

src/applications/conpherence/__tests__/ConpherenceThreadTestCase.php

@@ -112,6 +112,47 @@ final class ConpherenceThreadTestCase extends ConpherenceTestCase {
     }
   }
 
+  public function testAddMessageWithFileAttachments() {
+    $creator = $this->generateNewTestUser();
+    $friend_1 = $this->generateNewTestUser();
+    $policy_exception_user = $this->generateNewTestUser();
+
+    $participant_map = array(
+      $creator->getPHID() => $creator,
+      $friend_1->getPHID() => $friend_1,
+    );
+
+    $conpherence = $this->createThread(
+      $creator,
+      array_keys($participant_map));
+
+    foreach ($participant_map as $phid => $user) {
+      $xactions = $this->addMessageWithFile($user, $conpherence);
+      $this->assertEqual(2, count($xactions), pht('hi'));
+    }
+
+    $caught = null;
+    try {
+      $xactions = $this->addMessageWithFile(
+        $policy_exception_user,
+        $conpherence);
+    } catch (PhabricatorPolicyException $ex) {
+      $caught = $ex;
+    }
+    $this->assertTrue(
+      $caught instanceof PhabricatorPolicyException,
+      pht(
+        'User not participating in thread should get policy exception '.
+        'trying to add message.'));
+    $this->assertTrue(
+      $conpherence->establishConnection('w')->isReadLocking(),
+      pht(
+        'Conpherence object should still be read locked from policy '.
+        'exception.'));
+    $conpherence->endReadLocking();
+    $conpherence->killTransaction();
+  }
+
   private function createThread(
     PhabricatorUser $creator,
     array $participant_phids) {

src/applications/conpherence/editor/ConpherenceEditor.php

@@ -441,7 +441,14 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
             PhabricatorPolicyCapability::CAN_EDIT);
         }
         break;
+      // This is similar to PhabricatorTransactions::TYPE_COMMENT so
+      // use CAN_VIEW
       case ConpherenceTransactionType::TYPE_FILES:
+        PhabricatorPolicyFilter::requireCapability(
+          $this->requireActor(),
+          $object,
+          PhabricatorPolicyCapability::CAN_VIEW);
+        break;
       case ConpherenceTransactionType::TYPE_TITLE:
         PhabricatorPolicyFilter::requireCapability(
           $this->requireActor(),