This is pretty basic allowing a user to set the
policy as a valid string ('no-one' or 'users') or
as a valid PHID. Without an explicit policy
a permissive one is set.
Details
Details
- Reviewers
epriestley 20after4 - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T5685: Allow file data to be inaccessible, not undiscoverable
- Commits
- Restricted Diffusion Commit
rP6a69b4699ecb: file.upload set policy explicitly
Tested using the python-phabricator module (very basic api wrapper).
The arc cli syntax was evading me.
import base64
from phabricator import Phabricator
phab = Phabricator()
with open('mypic.jpg') as f:
encoded = base64.b64encode(f.read())
//set no-one as viewer which really means author only?
phab.file.upload(name='mypicnoone.jpg',
data_base64=encoded,
viewPolicy='no-one')
//set a specific phid as policy in this case a project
phab.file.upload(name='mypicphid.jpg',
data_base64=encoded,
viewPolicy='PHID-PROJ-fgvvnafmhvkgn2d5a4rf')
//no set policy ends up as 'users' i.e. ('all users')
phab.file.upload(name='mypicdefault.jpg', data_base64=encoded)Not able to really test canCDN attribute but it should be
fine and I tried to make it all consistent with D10166
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- arcpatch-D10164
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2087 Build 2091: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
Minor parameter naming nit.
| src/applications/files/conduit/FileUploadConduitAPIMethod.php | ||
|---|---|---|
| 17 | This should be viewPolicy, to avoid ambiguity with editPolicy. While Files may not ever have a mutable editPolicy, other object types will, and it's more consistent if they all end up with their view policy called viewPolicy than if it's sometimes viewPolicy and sometimes policy. | |
Comment Actions
adding can_cdn parameter and fixing up the policy->view_policy nit....I hope this isn't stepping on @rush898's toes too much :)
Comment Actions
- policy becomes view_policy
- Added can_cdn flag since I was about to do that in a separate diff, this seemed appropriate..
| src/applications/files/conduit/FileUploadConduitAPIMethod.php | ||
|---|---|---|
| 47 | @epriestley: what do you think about setting can_cdn=true when the default view policy is public? if ($view_policy == PhabricatorPolicies::POLICY_PUBLIC) {
$cdn = true;
} | |
Comment Actions
I like the explicitness of not coupling canCDN/viewPolicy -- I think API callers likely benefit most from an explicit API. I think we should default it off (i.e., make the default more secure), though, like file uploads elsewhere. This looks good otherwise, just kill those three lines?
| src/applications/files/conduit/FileUploadConduitAPIMethod.php | ||
|---|---|---|
| 44–46 | I think it's OK to leave this off by default -- it's off by default for web UI uploads. | |
| src/applications/files/conduit/FileUploadConduitAPIMethod.php | ||
|---|---|---|
| 44–46 | don't make it explicit false by default then? | |
Comment Actions
Closed by commit rP6a69b4699ecb (authored by cpettet <rush@wikimedia.org>, committed by @epriestley).