...how do you lock down entire areas otherwise? Fixes T6496.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T6496: Phriction should check for edit permissions on x/y/ before letting you create x/y/z/
- Commits
- Restricted Diffusion Commit
rP6f971a0fc4d8: Phriction - if you can't edit x/y don't allow creating x/y/z
used user 1 to create x/y that user 2 can't edit. tried to create x/y/z as user 2 and got a big ole error dialogue.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/applications/phriction/editor/PhrictionTransactionEditor.php | ||
|---|---|---|
| 605–615 | This seems a little off -- if there's no parent doc, we check this doc, and then go on to check the parent doc anyway? I think I'd expect this to look like: if ($parent_doc) {
check parent;
}...and for the "$object" check to be handled by parent::requireCapabilities(). Is there something subtle here I'm missing? | |
| 617 | Specifically, if there's no $parent_doc, I think we end up executing this check using it anyway? | |
Comment Actions
I think its correct as is, albeit a bit tricky and maybe should be re-written.
Note I am using rejectObject "manually" to cover the case where we have no parent_doc -- 'cuz we can't see it? -- and the case where we can't edit the parent doc.
| src/applications/phriction/editor/PhrictionTransactionEditor.php | ||
|---|---|---|
| 605–613 | This block could be written in english to say "If we can't see the parent doc, raise a policy exception for the doc", Note if the conditional is hit execution stops. | |
| 605–615 | This section is saying "If we can't edit the parent doc, raise a policy exception for the doc." | |
Comment Actions
Add a bunch of comments. I had to look seriously at this code twice since I wrote it so it seems worth it.