Skip to content

Commit 591df78

Browse files
author
epriestley
committed
Bind patches, file content and raw diffs bind policies to their originating objects
Summary: Fixes T4270. When you download raw file content, diffs, and patches we currently give them default (all users) visibility. Instead, bind them to the repository or revision in question. (This code could use a bit of cleanup at some point.) Test Plan: Hit the patch and content download links in Diffusion and the patch download link in Differential, got restricted files with accurate policy bindings. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4270 Differential Revision: https://secure.phabricator.com/D7849
1 parent 10d4eac commit 591df78

File tree

5 files changed

+45
-4
lines changed

5 files changed

+45
-4
lines changed

src/applications/differential/controller/DifferentialRevisionViewController.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public function processRequest() {
7070

7171
if ($request->getExists('download')) {
7272
return $this->buildRawDiffResponse(
73+
$revision,
7374
$changesets,
7475
$vs_changesets,
7576
$vs_map,
@@ -850,6 +851,7 @@ private function renderOtherRevisions(array $revisions) {
850851
* @return @{class:AphrontRedirectResponse}
851852
*/
852853
private function buildRawDiffResponse(
854+
DifferentialRevision $revision,
853855
array $changesets,
854856
array $vs_changesets,
855857
array $vs_map,
@@ -910,8 +912,16 @@ private function buildRawDiffResponse(
910912
$raw_diff,
911913
array(
912914
'name' => $file_name,
915+
'ttl' => (60 * 60 * 24),
916+
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
913917
));
914918

919+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
920+
$file->attachToObject(
921+
$this->getRequest()->getUser(),
922+
$revision->getPHID());
923+
unset($unguarded);
924+
915925
return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
916926

917927
}

src/applications/diffusion/controller/DiffusionBrowseFileController.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,12 +810,21 @@ private function renderInlines(array $inlines, $needs_blame, $engine) {
810810
}
811811

812812
private function loadFileForData($path, $data) {
813-
return PhabricatorFile::buildFromFileDataOrHash(
813+
$file = PhabricatorFile::buildFromFileDataOrHash(
814814
$data,
815815
array(
816816
'name' => basename($path),
817817
'ttl' => time() + 60 * 60 * 24,
818+
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
818819
));
820+
821+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
822+
$file->attachToObject(
823+
$this->getRequest()->getUser(),
824+
$this->getDiffusionRequest()->getRepository()->getPHID());
825+
unset($unguarded);
826+
827+
return $file;
819828
}
820829

821830
private function buildRawResponse($path, $data) {

src/applications/diffusion/controller/DiffusionCommitController.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,8 +1049,16 @@ private function buildRawDiffResponse(DiffusionRequest $drequest) {
10491049
$raw_diff,
10501050
array(
10511051
'name' => $drequest->getCommit().'.diff',
1052+
'ttl' => (60 * 60 * 24),
1053+
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
10521054
));
10531055

1056+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
1057+
$file->attachToObject(
1058+
$this->getRequest()->getUser(),
1059+
$drequest->getRepository()->getPHID());
1060+
unset($unguarded);
1061+
10541062
return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
10551063
}
10561064

src/applications/files/controller/PhabricatorFileInfoController.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ public function processRequest() {
3333

3434
$this->loadHandles($handle_phids);
3535
$header = id(new PHUIHeaderView())
36+
->setUser($user)
37+
->setPolicyObject($file)
3638
->setHeader($file->getName());
3739

3840
$ttl = $file->getTTL();

src/applications/files/storage/PhabricatorFile.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,17 @@ public static function newFileFromContentHash($hash, $params) {
175175
$file_ttl = idx($params, 'ttl');
176176
$authorPHID = idx($params, 'authorPHID');
177177

178-
$new_file = new PhabricatorFile();
178+
$new_file = new PhabricatorFile();
179179

180180
$new_file->setName($file_name);
181181
$new_file->setByteSize($copy_of_byteSize);
182182
$new_file->setAuthorPHID($authorPHID);
183183
$new_file->setTtl($file_ttl);
184184

185+
if (idx($params, 'viewPolicy')) {
186+
$new_file->setViewPolicy($params['viewPolicy']);
187+
}
188+
185189
$new_file->setContentHash($hash);
186190
$new_file->setStorageEngine($copy_of_storage_engine);
187191
$new_file->setStorageHandle($copy_of_storage_handle);
@@ -262,6 +266,10 @@ private static function buildFromFileData($data, array $params = array()) {
262266
$file->setTtl($file_ttl);
263267
$file->setContentHash(self::hashFileContent($data));
264268

269+
if (idx($params, 'viewPolicy')) {
270+
$file->setViewPolicy($params['viewPolicy']);
271+
}
272+
265273
$file->setStorageEngine($engine_identifier);
266274
$file->setStorageHandle($data_handle);
267275

@@ -877,8 +885,12 @@ public function getCapabilities() {
877885
}
878886

879887
public function getPolicy($capability) {
880-
// TODO: Implement proper per-object policies.
881-
return PhabricatorPolicies::POLICY_USER;
888+
switch ($capability) {
889+
case PhabricatorPolicyCapability::CAN_VIEW:
890+
return $this->getViewPolicy();
891+
case PhabricatorPolicyCapability::CAN_EDIT:
892+
return PhabricatorPolicies::POLICY_NOONE;
893+
}
882894
}
883895

884896
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {

0 commit comments

Comments
 (0)