Can't access binary files of differential revision
Open, Needs TriagePublic
Actions

Assigned To

None

Authored By

	sascha-egerer
	Apr 5 2016, 2:49 PM

Description

Reproduction steps:

Applications → Files → Edit Policies → Default View Policy
Select Subscribers
Use user A and create a differential revision with a binary file
Use user B and try to view that binary file in differential or try to arc patch the revision.

Expected result:

User B should be able to checkout the revision or view the file in the web interface.

Actual result:

Try to apply patch

$ arc patch D1
Created and checked out branch arcpatch-D1.

Downloading binary data for 'foo.woff'...
Exception
ERR-CONDUIT-CORE: [You Shall Not Pass: Restricted File] (Can View) You do not have permission to view this object. // Subscribers can take this action. The user who uploaded a file can always view and edit it. Files attached to objects are visible to users who can view those objects. Thumbnails are visible only to users who can view the original file.
(Run with `--trace` for a full exception trace.)

Try to open the file in the web interface

Version

phabricator 130e1d1f68a4dc4c41559be11971301cbb656317 (Sun, Mar 20)
arcanist 3d7ac867f53892b8210339bbb0c5fd91e0e36d78 (Tue, Mar 15)
phutil b4f38af384807b7dcd1e3d1a96e9e9069e4d6964 (Fri, Mar 18)

Related Objects

Mentioned Here: rARC3d7ac867f538: Make callsigns optional in arcanist
rPHUb4f38af38480: Implement a stream iterator and hashing iterator

Event Timeline

sascha-egerer created this task.Apr 5 2016, 2:49 PM

What's the relationship between user b and the patch, specifically?

This is probably the "object policy evaluated in the context of a different object" issue we've seen occasionally. If so, setting the visibility policy to any non-object-policy value (e.g., "All Users", or a specific list of users, etc -- anything which does not depend on the revision) should fix the error.

The problem is that the diff directly inherits the revision's policy, which just says "this object's subscribers". The problem is that diffs don't have subscribers, so the policy fails when evaluated in the context of the diff.

One solution is to use an "extended policy" instead of directly inheriting the policy, but then the UI will show the wrong policy. Sometimes this is fine (e.g., because there's no UI) but sometimes not.

Another solution is to adjust the policy framework to accommodate this "policy-in-the-context-of-an-object" sort of policy. This is probably better in some sense, but likely also a lot of work.

(That explanation might make approximately zero sense, I am not having a high-brain day today.)

[Explanation Intensifies]

In T10725#168508, @chad wrote:

What's the relationship between user b and the patch, specifically?

Both in the same projects. But i think that doesn't matter, right?

My question was what is the relation to User B and the Diff. Is User B a reviewer, a subscriber, or ... ?

But seems like Evan has a handle on this, regardless.

What does setting view policy of files to subscribers even do for you, out of curiosity?

In T10725#168645, @chad wrote:

My question was what is the relation to User B and the Diff. Is User B a reviewer, a subscriber, or ... ?

Ah sorry. User B is a reviewer of that diff.

What does setting view policy of files to subscribers even do for you, out of curiosity?

I've changed them now. Currently playing with the settings as i used phabricator in an agency and have to prevent that clients access data of other clients. If i set the default permissions of a file to "all users" then everybody can access the file, right?

Files attached to other objects take on permissions of the object itself. So if you add a file to a restricted task, the restriction is also applied to the file.

In T10725#168658, @chad wrote:

Files attached to other objects take on permissions of the object itself. So if you add a file to a restricted task, the restriction is also applied to the file.

Ok, wasn't sure if that is always the case but good to know.

I've also added myself as a subscriber (even if i was auto subscribed because i'm the reviewer) but that doesn't change anything. Still no permissions.
The problem can also be reproduced when clicking on Download Raw Diff in the interface.

joshuaspence added a subscriber: joshuaspence.Apr 8 2016, 8:16 AM

chad added a project: Differential.May 2 2016, 7:56 PM

Herald added a subscriber: eadler. · View Herald TranscriptMay 2 2016, 7:56 PM

eadler added a project: Restricted Project.Jul 7 2016, 5:19 PM

I have that problem again. There is no relation between the file and the Diff. Don't know how that can happen...

I've been running into the same issue for a while now. Using the Default Policy set to Subscribers for Files works well when adding files to Phriction pages and Maniphest tickets. However, when adding binary files as part of a Diff the creator of that Diff needs to change each files permissions by hand before a reviewer can download the Diff. Moreover, this needs to be repeated for every new version of that Diff.

Maybe things have changed in the meantime and this setup is just not the right way to go about it?

Can't access binary files of differential revisionOpen, Needs TriagePublicActions