Page MenuHomePhabricator

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

Description

Reproduction steps:

  • ApplicationsFilesEdit PoliciesDefault 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)

Event Timeline

chad added a subscriber: chad.Apr 5 2016, 3:09 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.)

chad added a comment.Apr 5 2016, 4:24 PM

[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?

chad added a comment.Apr 6 2016, 6:03 AM

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

chad added a comment.Apr 6 2016, 6:05 AM

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?

chad added a comment.Apr 6 2016, 6:25 AM

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.

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?