Page MenuHomePhabricator
Files F15423562

D10410.id25053.diff
No OneTemporary
Actions

D10410.id25053.diff
View Options

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();

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 23, 4:05 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7696870
Default Alt Text
D10410.id25053.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility