Give files uploaded to objects a very restrictive view policy

Summary:
Fixes T4589. This implements much better policy behavior for files that aligns with user expectations.

Currently, all files have permissive visibility.

The new behavior is:

- Files uploaded via drag-and-drop to the home page or file upload page get permissive visibility, for ease of quickly sharing things like screenshots.
- Files uploaded via the manual file upload control get permissive visibility by default, but the user can select the policy they want at upload time in an explicit/obvious way.
- Files uploaded via drag-and-drop anywhere else (e.g., comments or Pholio) get restricted visibility (only the uploader).
- When the user applies a transaction to the object which uses the file, we attach the file to the object and punch a hole through the policies: if you can see the object, you can see the file.
- This rule requires things to use ApplicationTransactions, which is why this took so long to fix.
- The "attach stuff to the object" code has been in place for a long time and works correctly.

I'll land D8498 after this lands, too.

Test Plan:
- Uploaded via global homepage upload and file drag-and-drop upload, saw permissive visibility.
- Uploaded via comment area, saw restricted visibility.
- After commenting, verified links were established and the file became visible to users who could see the attached object.
- Verified Pholio (which is a bit of a special case) correctly attaches images.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4589

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

epriestley 11 years ago 9181929e 1f1828e0

+45 -27

5 changed files

expand all

resources

celerity

map.php

src

applications

files

controller

PhabricatorFileDropUploadController.php

view

PhabricatorGlobalUploadTargetView.php

webroot

rsrc

core

DragAndDropFileUpload.js

behavior-global-drag-and-drop.js

+21 -21

resources/celerity/map.php

··· 8 8 return array( 9 9 'names' => array( 10 10 'core.pkg.css' => '66ada2ec', 11 - 'core.pkg.js' => '8cd3cd8c', 11 + 'core.pkg.js' => '4c28870b', 12 12 'darkconsole.pkg.js' => 'df001cab', 13 13 'differential.pkg.css' => '4a93db37', 14 - 'differential.pkg.js' => '79503aa4', 14 + 'differential.pkg.js' => 'eb182ccd', 15 15 'diffusion.pkg.css' => '591664fa', 16 16 'diffusion.pkg.js' => 'bfc0737b', 17 17 'maniphest.pkg.css' => 'f5d89daf', ··· 440 440 'rsrc/js/application/uiexample/gesture-example.js' => '558829c2', 441 441 'rsrc/js/application/uiexample/notification-example.js' => '7a9677fc', 442 442 'rsrc/js/core/Busy.js' => '6453c869', 443 - 'rsrc/js/core/DragAndDropFileUpload.js' => 'a575f592', 443 + 'rsrc/js/core/DragAndDropFileUpload.js' => 'f61aa8ec', 444 444 'rsrc/js/core/DraggableList.js' => '2cad29d1', 445 445 'rsrc/js/core/FileUpload.js' => 'a4ae61bf', 446 446 'rsrc/js/core/Hovercard.js' => '7e8468ae', ··· 465 465 'rsrc/js/core/behavior-file-tree.js' => '88236f00', 466 466 'rsrc/js/core/behavior-form.js' => '5c54cbf3', 467 467 'rsrc/js/core/behavior-gesture.js' => '3ab51e2c', 468 - 'rsrc/js/core/behavior-global-drag-and-drop.js' => '3672899b', 468 + 'rsrc/js/core/behavior-global-drag-and-drop.js' => '07f199d8', 469 469 'rsrc/js/core/behavior-high-security-warning.js' => '8fc1c918', 470 470 'rsrc/js/core/behavior-history-install.js' => '7ee2b591', 471 471 'rsrc/js/core/behavior-hovercard.js' => 'f36e01af', ··· 589 589 'javelin-behavior-doorkeeper-tag' => 'e5822781', 590 590 'javelin-behavior-error-log' => 'a5d7cf86', 591 591 'javelin-behavior-fancy-datepicker' => 'a5573bcd', 592 - 'javelin-behavior-global-drag-and-drop' => '3672899b', 592 + 'javelin-behavior-global-drag-and-drop' => '07f199d8', 593 593 'javelin-behavior-herald-rule-editor' => '7ebaeed3', 594 594 'javelin-behavior-high-security-warning' => '8fc1c918', 595 595 'javelin-behavior-history-install' => '7ee2b591', ··· 715 715 'phabricator-countdown-css' => '86b7b0a0', 716 716 'phabricator-crumbs-view-css' => '7fbf25b8', 717 717 'phabricator-dashboard-css' => 'a2bfdcbf', 718 - 'phabricator-drag-and-drop-file-upload' => 'a575f592', 718 + 'phabricator-drag-and-drop-file-upload' => 'f61aa8ec', 719 719 'phabricator-draggable-list' => '2cad29d1', 720 720 'phabricator-fatal-config-template-css' => '25d446d6', 721 721 'phabricator-feed-css' => '4e544db4', ··· 868 868 'javelin-mask', 869 869 'javelin-util', 870 870 'phabricator-busy', 871 + ), 872 + '07f199d8' => array( 873 + 'javelin-behavior', 874 + 'javelin-dom', 875 + 'javelin-uri', 876 + 'javelin-mask', 877 + 'phabricator-drag-and-drop-file-upload', 871 878 ), 872 879 '09eee344' => array( 873 880 'javelin-behavior', ··· 1042 1049 'javelin-behavior', 1043 1050 'javelin-dom', 1044 1051 ), 1045 - '3672899b' => array( 1046 - 'javelin-behavior', 1047 - 'javelin-dom', 1048 - 'javelin-uri', 1049 - 'javelin-mask', 1050 - 'phabricator-drag-and-drop-file-upload', 1051 - ), 1052 1052 '3915d490' => array( 1053 1053 'javelin-install', 1054 1054 'javelin-util', ··· 1490 1490 'javelin-stratcom', 1491 1491 'javelin-vector', 1492 1492 ), 1493 - 'a575f592' => array( 1494 - 'javelin-install', 1495 - 'javelin-util', 1496 - 'javelin-request', 1497 - 'javelin-dom', 1498 - 'javelin-uri', 1499 - 'phabricator-file-upload', 1500 - ), 1501 1493 'a5b67173' => array( 1502 1494 'javelin-dom', 1503 1495 'javelin-util', ··· 1876 1868 'phabricator-prefab', 1877 1869 'multirow-row-manager', 1878 1870 'javelin-json', 1871 + ), 1872 + 'f61aa8ec' => array( 1873 + 'javelin-install', 1874 + 'javelin-util', 1875 + 'javelin-request', 1876 + 'javelin-dom', 1877 + 'javelin-uri', 1878 + 'phabricator-file-upload', 1879 1879 ), 1880 1880 'f6555212' => array( 1881 1881 'javelin-install',

+12 -2

src/applications/files/controller/PhabricatorFileDropUploadController.php

··· 8 8 */ 9 9 public function processRequest() { 10 10 $request = $this->getRequest(); 11 - $user = $request->getUser(); 11 + $viewer = $request->getUser(); 12 12 13 13 // NOTE: Throws if valid CSRF token is not present in the request. 14 14 $request->validateCSRF(); ··· 16 16 $data = PhabricatorStartup::getRawInput(); 17 17 $name = $request->getStr('name'); 18 18 19 + // If there's no explicit view policy, make it very restrictive by default. 20 + // This is the correct policy for files dropped onto objects during 21 + // creation, comment and edit flows. 22 + 23 + $view_policy = $request->getStr('viewPolicy'); 24 + if (!$view_policy) { 25 + $view_policy = $viewer->getPHID(); 26 + } 27 + 19 28 $file = PhabricatorFile::newFromXHRUpload( 20 29 $data, 21 30 array( 22 31 'name' => $request->getStr('name'), 23 - 'authorPHID' => $user->getPHID(), 32 + 'authorPHID' => $viewer->getPHID(), 33 + 'viewPolicy' => $view_policy, 24 34 'isExplicitUpload' => true, 25 35 )); 26 36

src/applications/files/view/PhabricatorGlobalUploadTargetView.php

··· 28 28 'instructions' => $instructions_id, 29 29 'uploadURI' => '/file/dropupload/', 30 30 'browseURI' => '/file/query/authored/', 31 + 'viewPolicy' => PhabricatorPolicies::getMostOpenPolicy(), 31 32 )); 32 33 33 34 return phutil_tag(

+9 -3

webroot/rsrc/js/core/DragAndDropFileUpload.js

··· 174 174 175 175 var up_uri = JX.$U(this.getURI()) 176 176 .setQueryParam('name', file.getName()) 177 - .setQueryParam('__upload__', 1) 178 - .toString(); 177 + .setQueryParam('__upload__', 1); 178 + 179 + if (this.getViewPolicy()) { 180 + up_uri.setQueryParam('viewPolicy', this.getViewPolicy()); 181 + } 182 + 183 + up_uri = up_uri.toString(); 179 184 180 185 var onupload = JX.bind(this, function(r) { 181 186 if (r.error) { ··· 235 240 }, 236 241 properties: { 237 242 URI : null, 238 - activatedClass : null 243 + activatedClass : null, 244 + viewPolicy : null 239 245 } 240 246 });

+2 -1

webroot/rsrc/js/core/behavior-global-drag-and-drop.js

··· 21 21 } 22 22 23 23 var drop = new JX.PhabricatorDragAndDropFileUpload(document.documentElement) 24 - .setURI(config.uploadURI); 24 + .setURI(config.uploadURI) 25 + .setViewPolicy(config.viewPolicy); 25 26 26 27 drop.listen('didBeginDrag', function() { 27 28 JX.Mask.show();

Configure Feed

Configure Feed