HomePhabricator

Remove requireCapabilities() from ApplicationTransactionEditor and require…

Description

Remove requireCapabilities() from ApplicationTransactionEditor and require CAN_EDIT by default

Summary:
Depends on D19585. Ref T13164.

Almost all transactions require CAN_EDIT on the object, but they generally do not enforce this directly today. Instead, this is effectively enforced by Controllers, API methods, and EditEngine doing a CAN_EDIT check when loading the object to be edited.

A small number of transactions do not require CAN_EDIT, and instead require only a weaker/lesser permission. These are:

  • Joining a project which you have CAN_JOIN on.
  • Leaving a project which isn't locked.
  • Joining a Conpherence thread you can see (today, no separate CAN_JOIN permission for Conpherence).
  • Leaving a Conpherence thread.
  • Unsubscribing.
  • Using the special !history command from email.

Additionally, these require CAN_INTERACT, which is weaker than CAN_EDIT:

  • Adding comments.
  • Subscribing.
  • Awarding tokens.

Soon, I want to add "disabling users" to this list, so that you can disable users if you have "Can Disable User" permission, even if you can not otherwise edit users.

It's possible this list isn't exhaustive, so this change might break something by adding a policy check to a place where we previously didn't have one. If so, we can go weaken that policy check to the appropriate level.

Enforcement of these special cases is currently weird:

  • We mostly don't actually enforce CAN_EDIT in the Editor; instead, it's enforced before you get to the editor (in EditEngine/Controllers).
  • To apply a weaker requirement (like leaving comments or leaving a project), we let you get through the Controller without CAN_EDIT, then apply the weaker policy check in the Editor.
  • Some transactions apply a confusing/redundant explicit CAN_EDIT policy check. These mostly got cleaned up in previous changes.

Instead, the new world order is:

  • Every transaction has capability/policy requirements.
  • The default is CAN_EDIT, but transactions can weaken this explicitly they want.
  • So now we'll get requirements right in the Editor, even if Controllers or API endpoints make a mistake.
  • And you don't have to copy/paste a bunch of code to say "yes, every transaction should require CAN_EDIT".

Test Plan:

  • Tried to add members to a Conpherence thread I could not edit (permissions error).
  • Left a Conpherence thread I could not edit (worked properly).
  • Joined a thread I could see but could not edit (worked properly).
  • Tried to join a thread I could not see (permissions error).
  • Implemented requireCapabilites() on ManiphestTransactionEditor and tried to edit a task (upgrade guidance error).
  • Mentioned an object I can not edit on another object (works).
  • Mentioned another object on an object I can not edit (works).
  • Added a {F...} reference to an object I can not edit (works).
  • Awarded tokens to an object I can not edit (works).
  • Subscribed/unsubscribed from an object I can not edit (works).
  • Muted/unmuted an object I can not edit (works).
  • Tried to do other types of edits to an object I can not edit (correctly results in a permissions error).
  • Joined and left a project I can not edit (works).
  • Tried to edit and add members to a project I can not edit (permissions error).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19586