Fixes T7094 (last of many revisions). Its important to do this filtering ASAP so that users can't deduce the identify of an unknown / invisible project.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T7094: Clean up T603
- Commits
- Restricted Diffusion Commit
rPda1531f219d0: Policy - make ManiphestTaskQuery verify project visibility first thing
executed a query for tasks in project foo using user bar. using user foo, lock user bar out of project foo. reissued the query and saw "no data" as well as "restricted project" in the project typeahead.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/applications/maniphest/query/ManiphestTaskQuery.php | ||
|---|---|---|
| 270 | this is probably not necessary, it just seemed nice to normalize the array indices again | |
Comment Actions
I think this logic could move to willExecute(), which would mean fewer queries in the event that we need to page several times.
Comment Actions
The PhabricatorEmptyQueryException doesn't get caught if you throw it from willExecute. I figure two options
- no change
- move code to willExecute, but rather than throw set member variable "projectPolicyCheckFailed". loadPage() then throws the EmptyQueryException if this variable is set
I'll submit the latter for review in a second.
Comment Actions
I guess option (3) would be to try/catch for "EmptyQuery" earlier. That also seems reasonable to me, but is a more invasive change.
Comment Actions
Yeah, let's go with this and maybe I'll do (3) the next time we hit this. We do have test coverage and stuff so in theory it shouldn't be tooooo terrible, but it doesn't seem worth it to muck around there if we're only getting a tiny cleanup here.