diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -117,9 +117,9 @@ // First, load the edges. $edge_type = PhabricatorEdgeConfig::TYPE_FILE_HAS_OBJECT; - $phids = mpull($files, 'getPHID'); + $file_phids = mpull($files, 'getPHID'); $edges = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs($phids) + ->withSourcePHIDs($file_phids) ->withEdgeTypes(array($edge_type)) ->execute(); @@ -131,6 +131,21 @@ $object_phids[$phid] = true; } } + + // If this file is a transform of another file, load that file too. If you + // can see the original file, you can see the thumbnail. + + // TODO: It might be nice to put this directly on PhabricatorFile and remove + // the PhabricatorTransformedFile table, which would be a little simpler. + + $xforms = id(new PhabricatorTransformedFile())->loadAllWhere( + 'transformedPHID IN (%Ls)', + $file_phids); + $xform_phids = mpull($xforms, 'getOriginalPHID', 'getTransformedPHID'); + foreach ($xform_phids as $derived_phid => $original_phid) { + $object_phids[$original_phid] = true; + } + $object_phids = array_keys($object_phids); // Now, load the objects. @@ -156,6 +171,27 @@ $file->attachObjects($file_objects); } + foreach ($files as $key => $file) { + $original_phid = idx($xform_phids, $file->getPHID()); + if ($original_phid == PhabricatorPHIDConstants::PHID_VOID) { + // This is a special case for builtin files, which are handled + // oddly. + $original = null; + } else if ($original_phid) { + $original = idx($objects, $original_phid); + if (!$original) { + // If the viewer can't see the original file, also prevent them from + // seeing the transformed file. + $this->didRejectResult($file); + unset($files[$key]); + continue; + } + } else { + $original = null; + } + $file->attachOriginalFile($original); + } + return $files; } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -50,6 +50,14 @@ private $objects = self::ATTACHABLE; private $objectPHIDs = self::ATTACHABLE; + private $originalFile = self::ATTACHABLE; + + public static function initializeNewFile() { + return id(new PhabricatorFile()) + ->attachOriginalFile(null) + ->attachObjects(array()) + ->attachObjectPHIDs(array()); + } public function getConfiguration() { return array( @@ -190,7 +198,7 @@ $copy_of_byteSize = $file->getByteSize(); $copy_of_mimeType = $file->getMimeType(); - $new_file = new PhabricatorFile(); + $new_file = PhabricatorFile::initializeNewFile(); $new_file->setByteSize($copy_of_byteSize); @@ -226,7 +234,7 @@ throw new Exception('No valid storage engines are available!'); } - $file = new PhabricatorFile(); + $file = PhabricatorFile::initializeNewFile(); $data_handle = null; $engine_identifier = null; @@ -848,6 +856,15 @@ return $this; } + public function getOriginalFile() { + return $this->assertAttached($this->originalFile); + } + + public function attachOriginalFile(PhabricatorFile $file = null) { + $this->originalFile = $file; + return $this; + } + public function getImageHeight() { if (!$this->isViewableImage()) { return null; @@ -943,6 +960,23 @@ /** + * Remove the policy edge between this file and some object. + * + * @param phid Object PHID to detach from. + * @return this + */ + public function detachFromObject($phid) { + $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_FILE; + + id(new PhabricatorEdgeEditor()) + ->removeEdge($phid, $edge_type, $this->getPHID()) + ->save(); + + return $this; + } + + + /** * Configure a newly created file object according to specified parameters. * * This method is called both when creating a file from fresh data, and @@ -1033,6 +1067,12 @@ switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: + // If you can see the file this file is a transform of, you can see + // this file. + if ($this->getOriginalFile()) { + return true; + } + // If you can see any object this file is attached to, you can see // the file. return (count($this->getObjects()) > 0); @@ -1049,6 +1089,9 @@ $out[] = pht( 'Files attached to objects are visible to users who can view '. 'those objects.'); + $out[] = pht( + 'Thumbnails are visible only to users who can view the original '. + 'file.'); break; } diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php --- a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php +++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php @@ -8,6 +8,119 @@ ); } + public function testFileVisibility() { + $engine = new PhabricatorTestStorageEngine(); + $data = Filesystem::readRandomCharacters(64); + + $author = $this->generateNewTestUser(); + $viewer = $this->generateNewTestUser(); + $users = array($author, $viewer); + + $params = array( + 'name' => 'test.dat', + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + 'authorPHID' => $author->getPHID(), + 'storageEngines' => array( + $engine, + ), + ); + + $file = PhabricatorFile::newFromFileData($data, $params); + $filter = new PhabricatorPolicyFilter(); + + // Test bare file policies. + $this->assertEqual( + array( + true, + false, + ), + $this->canViewFile($users, $file), + pht('File Visibility')); + + // Create an object and test object policies. + + $object = ManiphestTask::initializeNewTask($author); + $object->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()); + $object->save(); + + $this->assertTrue( + $filter->hasCapability( + $author, + $object, + PhabricatorPolicyCapability::CAN_VIEW), + pht('Object Visible to Author')); + + $this->assertTrue( + $filter->hasCapability( + $viewer, + $object, + PhabricatorPolicyCapability::CAN_VIEW), + pht('Object Visible to Others')); + + // Attach the file to the object and test that the association opens a + // policy exception for the non-author viewer. + + $file->attachToObject($author, $object->getPHID()); + + // Test the attached file's visibility. + $this->assertEqual( + array( + true, + true, + ), + $this->canViewFile($users, $file), + pht('Attached File Visibility')); + + // Create a "thumbnail" of the original file. + $params = array( + 'name' => 'test.thumb.dat', + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + 'storageEngines' => array( + $engine, + ), + ); + + $xform = PhabricatorFile::newFromFileData($data, $params); + + id(new PhabricatorTransformedFile()) + ->setOriginalPHID($file->getPHID()) + ->setTransform('test-thumb') + ->setTransformedPHID($xform->getPHID()) + ->save(); + + // Test the thumbnail's visibility. + $this->assertEqual( + array( + true, + true, + ), + $this->canViewFile($users, $xform), + pht('Attached Thumbnail Visibility')); + + // Detach the object and make sure it affects the thumbnail. + $file->detachFromObject($object->getPHID()); + + // Test the detached thumbnail's visibility. + $this->assertEqual( + array( + true, + false, + ), + $this->canViewFile($users, $xform), + pht('Detached Thumbnail Visibility')); + } + + private function canViewFile(array $users, PhabricatorFile $file) { + $results = array(); + foreach ($users as $user) { + $results[] = (bool)id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs(array($file->getPHID())) + ->execute(); + } + return $results; + } + public function testFileStorageReadWrite() { $engine = new PhabricatorTestStorageEngine();