Skip to content

Commit 86ec4d6

Browse files
hach-quehach-que
committed
Implement policies in Phragment
Summary: This implements support for enforcing and setting policies in Phragment. Test Plan: Set policies and ensured they were enforced successfully. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T4205 Differential Revision: https://secure.phabricator.com/D7751
1 parent 7f45824 commit 86ec4d6

23 files changed

+350
-47
lines changed

‎resources/sql/patches/20131211.phragmentedges.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
CREATE TABLE {$NAMESPACE}_phragment.edge (
2+
src VARCHAR(64) NOT NULL COLLATE utf8_bin,
3+
type VARCHAR(64) NOT NULL COLLATE utf8_bin,
4+
dst VARCHAR(64) NOT NULL COLLATE utf8_bin,
5+
dateCreated INT UNSIGNED NOT NULL,
6+
seq INT UNSIGNED NOT NULL,
7+
dataID INT UNSIGNED,
8+
PRIMARY KEY (src, type, dst),
9+
KEY (src, type, dateCreated, seq)
10+
) ENGINE=InnoDB, COLLATE utf8_general_ci;
11+
12+
CREATE TABLE {$NAMESPACE}_phragment.edgedata (
13+
id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
14+
data LONGTEXT NOT NULL COLLATE utf8_bin
15+
) ENGINE=InnoDB, COLLATE utf8_general_ci;

‎src/__phutil_library_map__.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2180,6 +2180,7 @@
21802180
'PhortuneTestPaymentProvider' => 'applications/phortune/provider/PhortuneTestPaymentProvider.php',
21812181
'PhortuneWePayPaymentProvider' => 'applications/phortune/provider/PhortuneWePayPaymentProvider.php',
21822182
'PhragmentBrowseController' => 'applications/phragment/controller/PhragmentBrowseController.php',
2183+
'PhragmentCapabilityCanCreate' => 'applications/phragment/capability/PhragmentCapabilityCanCreate.php',
21832184
'PhragmentController' => 'applications/phragment/controller/PhragmentController.php',
21842185
'PhragmentCreateController' => 'applications/phragment/controller/PhragmentCreateController.php',
21852186
'PhragmentDAO' => 'applications/phragment/storage/PhragmentDAO.php',
@@ -2193,6 +2194,7 @@
21932194
'PhragmentPHIDTypeSnapshot' => 'applications/phragment/phid/PhragmentPHIDTypeSnapshot.php',
21942195
'PhragmentPatchController' => 'applications/phragment/controller/PhragmentPatchController.php',
21952196
'PhragmentPatchUtil' => 'applications/phragment/util/PhragmentPatchUtil.php',
2197+
'PhragmentPolicyController' => 'applications/phragment/controller/PhragmentPolicyController.php',
21962198
'PhragmentRevertController' => 'applications/phragment/controller/PhragmentRevertController.php',
21972199
'PhragmentSnapshot' => 'applications/phragment/storage/PhragmentSnapshot.php',
21982200
'PhragmentSnapshotChild' => 'applications/phragment/storage/PhragmentSnapshotChild.php',
@@ -4790,6 +4792,7 @@
47904792
'PhortuneTestPaymentProvider' => 'PhortunePaymentProvider',
47914793
'PhortuneWePayPaymentProvider' => 'PhortunePaymentProvider',
47924794
'PhragmentBrowseController' => 'PhragmentController',
4795+
'PhragmentCapabilityCanCreate' => 'PhabricatorPolicyCapability',
47934796
'PhragmentController' => 'PhabricatorController',
47944797
'PhragmentCreateController' => 'PhragmentController',
47954798
'PhragmentDAO' => 'PhabricatorLiskDAO',
@@ -4811,6 +4814,7 @@
48114814
'PhragmentPHIDTypeSnapshot' => 'PhabricatorPHIDType',
48124815
'PhragmentPatchController' => 'PhragmentController',
48134816
'PhragmentPatchUtil' => 'Phobject',
4817+
'PhragmentPolicyController' => 'PhragmentController',
48144818
'PhragmentRevertController' => 'PhragmentController',
48154819
'PhragmentSnapshot' =>
48164820
array(

‎src/applications/files/application/PhabricatorApplicationFiles.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public function getRoutes() {
6161
'xform/(?P<transform>[^/]+)/(?P<phid>[^/]+)/(?P<key>[^/]+)/'
6262
=> 'PhabricatorFileTransformController',
6363
'uploaddialog/' => 'PhabricatorFileUploadDialogController',
64+
'download/(?P<phid>[^/]+)/' => 'PhabricatorFileDialogController',
6465
),
6566
);
6667
}

‎src/applications/files/controller/PhabricatorFileDataController.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ public function processRequest() {
6666
if ($is_viewable && !$force_download) {
6767
$response->setMimeType($file->getViewableMimeType());
6868
} else {
69-
if (!$request->isHTTPPost()) {
70-
// NOTE: Require POST to download files. We'd rather go full-bore and
71-
// do a real CSRF check, but can't currently authenticate users on the
72-
// file domain. This should blunt any attacks based on iframes, script
73-
// tags, applet tags, etc., at least. Send the user to the "info" page
74-
// if they're using some other method.
69+
if (!$request->isHTTPPost() && !$alt_domain) {
70+
// NOTE: Require POST to download files from the primary domain. We'd
71+
// rather go full-bore and do a real CSRF check, but can't currently
72+
// authenticate users on the file domain. This should blunt any
73+
// attacks based on iframes, script tags, applet tags, etc., at least.
74+
// Send the user to the "info" page if they're using some other method.
7575
return id(new AphrontRedirectResponse())
7676
->setURI(PhabricatorEnv::getProductionURI($file->getBestURI()));
7777
}

‎src/applications/phragment/application/PhabricatorApplicationPhragment.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public function getRoutes() {
3737
'browse/(?P<dblob>.*)' => 'PhragmentBrowseController',
3838
'create/(?P<dblob>.*)' => 'PhragmentCreateController',
3939
'update/(?P<dblob>.*)' => 'PhragmentUpdateController',
40+
'policy/(?P<dblob>.*)' => 'PhragmentPolicyController',
4041
'history/(?P<dblob>.*)' => 'PhragmentHistoryController',
4142
'zip/(?P<dblob>.*)' => 'PhragmentZIPController',
4243
'zip@(?P<snapshot>[^/]+)/(?P<dblob>.*)' => 'PhragmentZIPController',
@@ -56,5 +57,12 @@ public function getRoutes() {
5657
);
5758
}
5859

60+
protected function getCustomCapabilities() {
61+
return array(
62+
PhragmentCapabilityCanCreate::CAPABILITY => array(
63+
),
64+
);
65+
}
66+
5967
}
6068

‎src/applications/phragment/capability/PhragmentCapabilityCanCreate.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
final class PhragmentCapabilityCanCreate
4+
extends PhabricatorPolicyCapability {
5+
6+
const CAPABILITY = 'phragment.create';
7+
8+
public function getCapabilityKey() {
9+
return self::CAPABILITY;
10+
}
11+
12+
public function getCapabilityName() {
13+
return pht('Can Create Fragments');
14+
}
15+
16+
public function describeCapabilityRejection() {
17+
return pht('You do not have permission to create fragments.');
18+
}
19+
20+
}

‎src/applications/phragment/controller/PhragmentBrowseController.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ final class PhragmentBrowseController extends PhragmentController {
44

55
private $dblob;
66

7+
public function shouldAllowPublic() {
8+
return true;
9+
}
10+
711
public function willProcessRequest(array $data) {
812
$this->dblob = idx($data, "dblob", "");
913
}
@@ -24,11 +28,14 @@ public function processRequest() {
2428
}
2529

2630
$crumbs = $this->buildApplicationCrumbsWithPath($parents);
27-
$crumbs->addAction(
28-
id(new PHUIListItemView())
29-
->setName(pht('Create Fragment'))
30-
->setHref($this->getApplicationURI('/create/'.$path))
31-
->setIcon('create'));
31+
if ($this->hasApplicationCapability(
32+
PhragmentCapabilityCanCreate::CAPABILITY)) {
33+
$crumbs->addAction(
34+
id(new PHUIListItemView())
35+
->setName(pht('Create Fragment'))
36+
->setHref($this->getApplicationURI('/create/'.$path))
37+
->setIcon('create'));
38+
}
3239

3340
$current_box = $this->createCurrentFragmentView($current, false);
3441

@@ -79,6 +86,7 @@ public function processRequest() {
7986
return $this->buildApplicationPage(
8087
array(
8188
$crumbs,
89+
$this->renderConfigurationWarningIfRequired(),
8290
$current_box,
8391
$list),
8492
array(

‎src/applications/phragment/controller/PhragmentController.php

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected function createCurrentFragmentView($fragment, $is_history_view) {
8484
->withPHIDs(array($fragment->getLatestVersion()->getFilePHID()))
8585
->executeOne();
8686
if ($file !== null) {
87-
$file_uri = $file->getBestURI();
87+
$file_uri = $file->getDownloadURI();
8888
}
8989
}
9090

@@ -93,37 +93,53 @@ protected function createCurrentFragmentView($fragment, $is_history_view) {
9393
->setPolicyObject($fragment)
9494
->setUser($viewer);
9595

96+
$can_edit = PhabricatorPolicyFilter::hasCapability(
97+
$viewer,
98+
$fragment,
99+
PhabricatorPolicyCapability::CAN_EDIT);
100+
101+
$zip_uri = $this->getApplicationURI("zip/".$fragment->getPath());
102+
96103
$actions = id(new PhabricatorActionListView())
97104
->setUser($viewer)
98105
->setObject($fragment)
99106
->setObjectURI($fragment->getURI());
100107
$actions->addAction(
101108
id(new PhabricatorActionView())
102109
->setName(pht('Download Fragment'))
103-
->setHref($file_uri)
104-
->setDisabled($file === null)
110+
->setHref($this->isCorrectlyConfigured() ? $file_uri : null)
111+
->setDisabled($file === null || !$this->isCorrectlyConfigured())
105112
->setIcon('download'));
106113
$actions->addAction(
107114
id(new PhabricatorActionView())
108115
->setName(pht('Download Contents as ZIP'))
109-
->setHref($this->getApplicationURI("zip/".$fragment->getPath()))
110-
->setDisabled(false) // TODO: Policy
116+
->setHref($this->isCorrectlyConfigured() ? $zip_uri : null)
117+
->setDisabled(!$this->isCorrectlyConfigured())
111118
->setIcon('zip'));
112119
if (!$fragment->isDirectory()) {
113120
$actions->addAction(
114121
id(new PhabricatorActionView())
115122
->setName(pht('Update Fragment'))
116123
->setHref($this->getApplicationURI("update/".$fragment->getPath()))
117-
->setDisabled(false) // TODO: Policy
124+
->setDisabled(!$can_edit)
125+
->setWorkflow(!$can_edit)
118126
->setIcon('edit'));
119127
} else {
120128
$actions->addAction(
121129
id(new PhabricatorActionView())
122130
->setName(pht('Convert to File'))
123131
->setHref($this->getApplicationURI("update/".$fragment->getPath()))
124-
->setDisabled(false) // TODO: Policy
132+
->setDisabled(!$can_edit)
133+
->setWorkflow(!$can_edit)
125134
->setIcon('edit'));
126135
}
136+
$actions->addAction(
137+
id(new PhabricatorActionView())
138+
->setName(pht('Set Fragment Policies'))
139+
->setHref($this->getApplicationURI("policy/".$fragment->getPath()))
140+
->setDisabled(!$can_edit)
141+
->setWorkflow(!$can_edit)
142+
->setIcon('edit'));
127143
if ($is_history_view) {
128144
$actions->addAction(
129145
id(new PhabricatorActionView())
@@ -142,15 +158,16 @@ protected function createCurrentFragmentView($fragment, $is_history_view) {
142158
->setName(pht('Create Snapshot'))
143159
->setHref($this->getApplicationURI(
144160
"snapshot/create/".$fragment->getPath()))
145-
->setDisabled(false) // TODO: Policy
161+
->setDisabled(!$can_edit)
162+
->setWorkflow(!$can_edit)
146163
->setIcon('snapshot'));
147164
$actions->addAction(
148165
id(new PhabricatorActionView())
149166
->setName(pht('Promote Snapshot to Here'))
150167
->setHref($this->getApplicationURI(
151168
"snapshot/promote/latest/".$fragment->getPath()))
152169
->setWorkflow(true)
153-
->setDisabled(false) // TODO: Policy
170+
->setDisabled(!$can_edit)
154171
->setIcon('promote'));
155172

156173
$properties = id(new PHUIPropertyListView())
@@ -188,4 +205,33 @@ protected function createCurrentFragmentView($fragment, $is_history_view) {
188205
->addPropertyList($properties);
189206
}
190207

208+
function renderConfigurationWarningIfRequired() {
209+
$alt = PhabricatorEnv::getEnvConfig("security.alternate-file-domain");
210+
if ($alt === null) {
211+
return id(new AphrontErrorView())
212+
->setTitle(pht('security.alternate-file-domain must be configured!'))
213+
->setSeverity(AphrontErrorView::SEVERITY_ERROR)
214+
->appendChild(phutil_tag('p', array(), pht(
215+
'Because Phragment generates files (such as ZIP archives and '.
216+
'patches) as they are requested, it requires that you configure '.
217+
'the `security.alterate-file-domain` option. This option on it\'s '.
218+
'own will also provide additional security when serving files '.
219+
'across Phabricator.')));
220+
}
221+
return null;
222+
}
223+
224+
/**
225+
* We use this to disable the download links if the alternate domain is
226+
* not configured correctly. Although the download links will mostly work
227+
* for logged in users without an alternate domain, the behaviour is
228+
* reasonably non-consistent and will deny public users, even if policies
229+
* are configured otherwise (because the Files app does not support showing
230+
* the info page to viewers who are not logged in).
231+
*/
232+
function isCorrectlyConfigured() {
233+
$alt = PhabricatorEnv::getEnvConfig("security.alternate-file-domain");
234+
return $alt !== null;
235+
}
236+
191237
}

‎src/applications/phragment/controller/PhragmentCreateController.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ public function processRequest() {
123123
return $this->buildApplicationPage(
124124
array(
125125
$crumbs,
126+
$this->renderConfigurationWarningIfRequired(),
126127
$box),
127128
array(
128129
'title' => pht('Create Fragment'),

‎src/applications/phragment/controller/PhragmentHistoryController.php

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ final class PhragmentHistoryController extends PhragmentController {
44

55
private $dblob;
66

7+
public function shouldAllowPublic() {
8+
return true;
9+
}
10+
711
public function willProcessRequest(array $data) {
812
$this->dblob = idx($data, "dblob", "");
913
}
@@ -21,11 +25,14 @@ public function processRequest() {
2125
$path = $current->getPath();
2226

2327
$crumbs = $this->buildApplicationCrumbsWithPath($parents);
24-
$crumbs->addAction(
25-
id(new PHUIListItemView())
26-
->setName(pht('Create Fragment'))
27-
->setHref($this->getApplicationURI('/create/'.$path))
28-
->setIcon('create'));
28+
if ($this->hasApplicationCapability(
29+
PhragmentCapabilityCanCreate::CAPABILITY)) {
30+
$crumbs->addAction(
31+
id(new PHUIListItemView())
32+
->setName(pht('Create Fragment'))
33+
->setHref($this->getApplicationURI('/create/'.$path))
34+
->setIcon('create'));
35+
}
2936

3037
$current_box = $this->createCurrentFragmentView($current, true);
3138

@@ -44,6 +51,11 @@ public function processRequest() {
4451
->execute();
4552
$files = mpull($files, null, 'getPHID');
4653

54+
$can_edit = PhabricatorPolicyFilter::hasCapability(
55+
$viewer,
56+
$current,
57+
PhabricatorPolicyCapability::CAN_EDIT);
58+
4759
$first = true;
4860
foreach ($versions as $version) {
4961
$item = id(new PHUIObjectItemView());
@@ -58,7 +70,7 @@ public function processRequest() {
5870
$item->addAttribute('Deletion');
5971
}
6072

61-
if (!$first) {
73+
if (!$first && $can_edit) {
6274
$item->addAction(id(new PHUIListItemView())
6375
->setIcon('undo')
6476
->setRenderNameAsTooltip(true)
@@ -71,13 +83,15 @@ public function processRequest() {
7183
$disabled = !isset($files[$version->getFilePHID()]);
7284
$action = id(new PHUIListItemView())
7385
->setIcon('download')
74-
->setDisabled($disabled)
86+
->setDisabled($disabled || !$this->isCorrectlyConfigured())
7587
->setRenderNameAsTooltip(true)
7688
->setName(pht("Download"));
77-
if (!$disabled) {
78-
$action->setHref($files[$version->getFilePHID()]->getBestURI());
89+
if (!$disabled && $this->isCorrectlyConfigured()) {
90+
$action->setHref($files[$version->getFilePHID()]
91+
->getDownloadURI($version->getURI()));
7992
}
8093
$item->addAction($action);
94+
8195
$list->addItem($item);
8296

8397
$first = false;
@@ -86,6 +100,7 @@ public function processRequest() {
86100
return $this->buildApplicationPage(
87101
array(
88102
$crumbs,
103+
$this->renderConfigurationWarningIfRequired(),
89104
$current_box,
90105
$list),
91106
array(

0 commit comments

Comments
 (0)