Page MenuHomePhabricator

Don't put "spacePHID IN (...)" constraints in queries which will raise policy exceptions
ClosedPublic

Authored by epriestley on Jan 28 2019, 3:52 PM.
Tags
None
Referenced Files
F13099506: D20042.diff
Fri, Apr 26, 3:30 PM
Unknown Object (File)
Thu, Apr 25, 3:22 AM
Unknown Object (File)
Wed, Apr 24, 5:54 PM
Unknown Object (File)
Mon, Apr 22, 2:02 AM
Unknown Object (File)
Sat, Apr 20, 10:50 AM
Unknown Object (File)
Thu, Apr 11, 10:43 AM
Unknown Object (File)
Wed, Apr 10, 3:47 PM
Unknown Object (File)
Mar 28 2024, 1:12 AM
Subscribers
None

Details

Summary

See T13240. Ref T13242. When we're issuing a query that will raise policy exceptions (i.e., give the user a "You Shall Not Pass" dialog if they can not see objects it loads), don't do space filtering in MySQL: when objects are filtered out in MySQL, we can't distinguish between "bad/invalid ID/object" and "policy filter", so we can't raise a policy exception.

This leads to cases where viewing an object shows "You Shall Not Pass" if you can't see it for any non-Spaces reason, but "404" if the reason is Spaces.

There's no product reason for this, it's just that spacePHID IN (...) is important for non-policy-raising queries (like a list of tasks) to reduce how much application filtering we need to do.

Test Plan

Before:

$ git pull
phabricator-ssh-exec: No repository "spellbook" exists!
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

After:

$ git pull
phabricator-ssh-exec: [You Shall Not Pass: Unknown Object (Repository)] This object is in a space you do not have permission to access.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Diff Detail

Repository
rP Phabricator
Branch
space1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21710
Build 29621: Run Core Tests
Build 29620: arc lint + arc unit