Page MenuHomePhabricator

Give files uploaded to objects a very restrictive view policy
ClosedPublic

Authored by epriestley on Aug 2 2014, 8:22 PM.
Tags
None
Referenced Files
F18724417: D10131.id24368.diff
Tue, Sep 30, 3:40 AM
F18621715: D10131.id24368.diff
Mon, Sep 15, 9:15 AM
F18601028: D10131.diff
Sat, Sep 13, 11:20 AM
F18596530: D10131.diff
Sat, Sep 13, 12:56 AM
F18503904: D10131.id.diff
Thu, Sep 4, 11:28 PM
F18495607: D10131.diff
Thu, Sep 4, 4:33 PM
F18105784: D10131.id24368.diff
Aug 10 2025, 7:36 PM
F18065237: D10131.id24368.diff
Aug 4 2025, 2:04 PM

Details

Summary

Fixes T4589. This implements much better policy behavior for files that aligns with user expectations.

Currently, all files have permissive visibility.

The new behavior is:

  • Files uploaded via drag-and-drop to the home page or file upload page get permissive visibility, for ease of quickly sharing things like screenshots.
  • Files uploaded via the manual file upload control get permissive visibility by default, but the user can select the policy they want at upload time in an explicit/obvious way.
  • Files uploaded via drag-and-drop anywhere else (e.g., comments or Pholio) get restricted visibility (only the uploader).
    • When the user applies a transaction to the object which uses the file, we attach the file to the object and punch a hole through the policies: if you can see the object, you can see the file.
    • This rule requires things to use ApplicationTransactions, which is why this took so long to fix.
    • The "attach stuff to the object" code has been in place for a long time and works correctly.

I'll land D8498 after this lands, too.

Test Plan
  • Uploaded via global homepage upload and file drag-and-drop upload, saw permissive visibility.
  • Uploaded via comment area, saw restricted visibility.
  • After commenting, verified links were established and the file became visible to users who could see the attached object.
  • Verified Pholio (which is a bit of a special case) correctly attaches images.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Give files uploaded to objects a very restrictive view policy.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.

This is awesome. =D

Do you think we need a User Guide: Policy or something at some point? I think this makes intuitive sense while also striking me as something that should be written down somewhere.

This revision is now accepted and ready to land.Aug 2 2014, 9:14 PM
epriestley updated this revision to Diff 24368.

Closed by commit rP9181929ebc2f (authored by @epriestley).