Page MenuHomePhabricator

Prevent users from taking "edit"-like actions via comment forms if they don't have edit permission
ClosedPublic

Authored by epriestley on Mar 2 2017, 8:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 10:34 PM
Unknown Object (File)
Thu, Apr 11, 7:06 AM
Unknown Object (File)
Sun, Apr 7, 11:45 PM
Unknown Object (File)
Tue, Apr 2, 8:38 AM
Unknown Object (File)
Feb 24 2024, 4:58 PM
Unknown Object (File)
Feb 14 2024, 6:05 PM
Unknown Object (File)
Feb 9 2024, 4:53 AM
Unknown Object (File)
Feb 6 2024, 6:19 AM
Subscribers
None

Details

Summary

Ref T12335. Fixes T11207. Edit-like interactions which are not performed via "Edit <object>" are a bit of a grey area, policy-wise.

For example, you can correctly do these things to an object you can't edit:

  • Comment on it.
  • Award tokens.
  • Subscribe or unsubscribe.
  • Subscribe other users by mentioning them.
  • Perform review.
  • Perform audit.
  • (Maybe some other stuff.)

These behaviors are all desirable and correct. But, particularly now that we offer stacked actions, you can do a bunch of other stuff which you shouldn't really be able to, like changing the status and priority of tasks you can't edit, as long as you submit the change via the comment form.

(Before the advent of stacked actions there were fewer things you could do via the comment form, and more of them were very "grey area", especially since "Change Subscribers" was just "Add Subscribers", which you can do via mentions.)

This isn't too much of a problem in practice because we won't show you those actions if the edit form you'd end up on doesn't have those fields. So on intalls like ours where we've created simple + advanced flows, users who shouldn't be changing task priorities generally don't see an option to do so, even though they technically could if they mucked with the HTML.

Change this behavior to be more strict: unless an action explicitly says that it doesn't need edit permission (comment, review, audit) don't show it to users who don't have edit permission and don't let them take the action.

Test Plan
  • As a user who could not edit a task, tried to change status via comment form; received policy exception.
  • As a user who could not edit a task, viewed a comment form: no actions available (just "comment").
  • As a user who could not edit a revision, viewed a revision form: only "review" actions available (accept, resign, etc).
  • Viewed a commit form but these are kind of moot because there's no separate edit permission.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable