Page MenuHomePhabricator

Policy to define who can move cards in a workboard
Closed, WontfixPublic

Description

There doesn't seem to be any policy for moving cards in a workboard. As long as workboards are visible, All Users (registered) can move them, unless I have missed something. It's not the Editable By policy for a project, neither for a task.

In projects where a workboard is maintained, the position of the cards might contain a lot of meaning for the developer team. It's not funny if overnight someone came and played with your workboard. It's even less funny if it was not even a game but an intentional statement, a difference of opinions, a protest, etc.

Wikimedia developers and product owners ask how can they protect their board so only the team gets to decide where the cards sit. In Trello and Mingle they are in control. We have no precedents of abuse in Phabricator so far, but looking at our history with Bugzilla I will not be surprised the morning we find a first precedent.

For a simple UI solution, and as long as workboards are part of projects, it would make sense that the Editable By policy of a project would define who can move the cards of the related workboard. It already defines who can edit the columns themselves.

Original report: https://phabricator.wikimedia.org/T1178

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
chad added a subscriber: chad.Nov 8 2014, 3:48 PM

I'd lean towards separate policy. Editing a task is primarily changing the definition of a task, where Workboards are the planning/implementation of a Task.

Specifically, you'd lean toward requiring the separate policy only (you don't need to be able to edit the task, as long as you can edit the board)?

I think that's a reasonable thing to consider. Two thoughts:

  • It makes some of the JS/UI stuff a lot easier, since every task on a board is either draggable or not draggable.
  • It makes priority ordering of the board really messy, though, since you need "Can Edit" on a task to change task priority (and similarly for any ordering other than "Natural").
chad added a comment.Nov 8 2014, 3:53 PM

Yeah, conceptually priority is also task management/planning like workboards and everything else is task definition.

Thinking on it a little more, I like it conceptually but think we can't get away with it:

  • The ordered board case turns into a mess.
  • Triggers (T5474) also turn into a mess.

If we tried to implement that rule (no need for "Edit Task"), I think we'd frequently (and sort of inconsistently) end up back at requiring "Edit Task" permission because a trigger tried to change the assignee or status, or an ordered drag tried to change the priority.

I don't think the ordered drags are really that important and would consider throwing them under the bus, but I suspect triggers may be very useful/powerful and I'd at least like to make sure they're just-okay or not-so-great before we consider policy approaches which would make them cumbersome.

That said, I do think a separate permission is probably reasonable, but you'd need both ("Can Edit Workboard", "Can Edit Task") to drag.

chad added a comment.Nov 8 2014, 3:59 PM

Would there be any future path to ... say only Security can edit Security Workboard...

Yeah, just set "Can Edit Workboard" to "Members of Project 'Security'".

(I think that would be a reasonable default, although the actual default-default would have to be "members of current project", which doesn't exist until T5681.)

chad added a comment.Nov 8 2014, 4:14 PM

Oh Can Edit Workboard is a Project Policy, not a Maniphest Policy, correct?

Yep, exactly.

qgil added a comment.Nov 8 2014, 7:26 PM

Is this summary correct, then?

When a user tries to move a card in a workboard...

  1. Enforce project policy, is the user in the group that Can Edit Workboard?
    1. If not, keep the card and show a dialog.
    2. Else, a check to the task policy follows:
      1. Is the move triggering any change in the task attributes other than a change of column?
        1. If not, move the card.
        2. Else, is the user in the group that Can Edit Task?
          1. If not, keep the card and show a dialog.
          2. Else, move the card.
chad added a comment.Nov 8 2014, 7:28 PM

I feel like 'Can Edit Workboard' should grant 'Can Edit Task'.

In T6502#83049, @qgil wrote:

Is this summary correct, then?

When a user tries to move a card in a workboard...

No, it would just be: when a user tries to move a card in a workboard:

  • Does the user have permission to manage the workboard AND permission to edit the task?
    • If yes to both, they can move it.
    • If no to either, they can't.

I want to avoid a complex tree of cases where you can make some edits but not others based on what triggers a column happens to have, etc. I think this will be really confusing.

In T6502#83050, @chad wrote:

I feel like 'Can Edit Workboard' should grant 'Can Edit Task'.

I don't want adding projects to objects to ever affect permissions. In particular, tagging a sensitive task with something like "Easy" or "Windows" shouldn't let a ton of users suddenly edit it. (This scenario is probably not really that scary since they normally wouldn't be able to see it if it was sensitive, but in principle...)

qgil added a comment.Nov 8 2014, 7:42 PM

@epriestley, ok, that works too. You see, you wouldn't hire me as an engineer. :P

@chad, I had the same idea at first, but since a task can be in more than a project, perhaps some tasks in your project are more editable than others.

chad triaged this task as Normal priority.Nov 10 2014, 4:31 AM
qgil moved this task from Backlog to Important on the Wikimedia board.Dec 4 2014, 12:52 PM
rbalik added a subscriber: rbalik.Mar 6 2015, 5:53 PM
DeanBodart moved this task from Important to In Progress on the Wikimedia board.Apr 27 2015, 5:14 PM
eadler added a subscriber: eadler.May 22 2015, 10:14 PM
epriestley moved this task from Backlog to Projects v3 on the Projects board.
timor added a subscriber: timor.Sep 10 2015, 11:48 AM
epriestley moved this task from Backlog to Workboards v2 on the Workboards board.Dec 8 2015, 11:17 PM
ablekh added a subscriber: ablekh.Jan 30 2016, 4:47 AM
chad moved this task from Projects v3 to v3 on the Projects board.Feb 5 2016, 7:39 PM
chad edited projects, added Projects (v3); removed Projects.
chad moved this task from Backlog to Evan on the Projects (v3) board.

This is probably happening tomorrow, unless I am just toying with everyone's emotions.

This is probably happening tomorrow, unless I am just toying with everyone's emotions.

Looking deeper, I'm sort of wondering if we actually need this. Are you seeing (or still seeing) issues with this?

This should only impact open source installs, and is hopefully less of an issue now after changes in T7410 and maybe broadly in the "newness" of Phabricator and workboards wearing off a bit. We've seen very few issues related to board editing on this install (a handful of users very occasionally drag cards around to test things).

In the upstream, we've set the default edit policy for tasks to "Members of: Community", and the default edit policy for bug/feature tasks to "Members of Community + Task Author" going forward. So users generally can not move tasks around, anyway (they don't have permission to edit them).

It's even less funny if it was not even a game but an intentional statement, a difference of opinions, a protest, etc.

Has this actually happened? See also T10215. This doesn't seem hugely damaging to me and the user will leave audit logs so their protest will be short-lived. If they can mess up a workboard, they could also mess up the tasks individually (e.g., close all of them).

This policy is also impossible to implement in a complete and consistent way with the advent of milestones and subprojects, because editing a task's projects may move which column it is in. A user who wants to vandalize a workboard and has permission to edit the tasks on that board can just change the projects the tasks are in to effectively vandalize it.

If you are still seeing issues with this, I'd like to wait until you pick up the new EditEngine stuff and see if that helps before moving forward here. Mostly, I'm concerned that this is adding a fair amount of complexity and only really affects one install.

If the specific issue is intentional vandalism rather than testing/mistakes, this may just be slightly lower-hanging fruit than other approaches, but vandals might shift their strategies if we raised the bar slightly. I'd like to look at other things we might do instead (for example, implementing a "rewind time" tool to undo abuse) if this is primarily an intentional abuse/vandalism issue rather than a testing/mistakes/it-was-shiny-so-I-touched-it issue, since any fix here only raises the barrier to vandalism very slightly.

On the Wikimedia side, looking at https://phabricator.wikimedia.org/T1178, I see almost no support or consensus for this feature. Restricting open editing is in some ways antithetical to Wikimedia's values (we try to be as open as possible) and there are a lot of other issues with Phabricator that are higher priority for us.

I'm not sure if that thumbs-down is an objection to introducing this policy control or an objection to my hesitance to introduce this policy control.

(Oh, the WMF task got a similar token so it's presumably an objection to the control.)

epriestley lowered the priority of this task from Normal to Wishlist.Feb 6 2016, 8:37 PM
epriestley added a subscriber: Krenair.

Given the feedback from @Krenair / @MZMcBride, I'm going to shelve this for the time being since WMF is the major driver for it but it looks like some mixture of contentious/low-priority/hypothetical, and even if consensus/priority does develop from WMF I'd like to wait for EditEngine and make sure the problems survive the new capabilities it introduced.

I don't think this feature is unreasonable, and it's definitely something we might develop in the future, but it looks like the case for it isn't very strong right now.

One minor consideration is that the way EditEngine interacts with "New Task" actions on workboards is somewhat-awkwardly balancing several concerns right now. That may get more options or a separate policy eventually, which could interact here (e.g., "Can Create New Cards On This Board" and "Can Move Cards on This Board" might reasonably be the same, new policy). Adding actions to columns (T5474) might also impact this.

To the degree that this is primarily an abuse issue, I'd prefer to step back and look at resolving larger issues related to abuse cases (e.g., discuss things like a "Rewind Time" tool or a more granular/aggressive rate limiting system for the actions new users can take, or look at ways we might be able to make account registration have a higher cost, or whatever else we can come up with that addresses specific, real abuse scenarios encountered in the wild).

epriestley moved this task from v3 to Far Future on the Projects board.Feb 6 2016, 8:37 PM
epriestley edited projects, added Projects; removed Projects (v3).
aklapper moved this task from In Progress to Backlog on the Wikimedia board.Feb 7 2016, 10:22 PM

@epriestley: thanks for the thorough consideration.

I agree, the use-case isn't very strong. If we wanted to restrict things we should just restrict the project or the tasks. Neither of which we will be doing so there is little use restricting the workboards.

We might still need this. We use priority aggressively for feature planning, and make sure only team leads can triage and adjust priorities.

We also drag tasks around columns around a lot for "task state" -- (Backlog, Dev, CR Ready, Code Review, QA Ready, Complete)

Some of our users use priority view as a way of determining what tasks they should take next, and we've already had a few strange cases where dragging magically adjusts priority -- something they weren't expecting, nor is something they should be able to do.

See T10333 for plans to make dragging cards in sorted workboards more clear (and T8135 for additional discussion). Those UIs aren't as clear as they should be and we'd like them to be more clear.

(It's possible that the end state is still implementing this as a dedicated permission, but to the degree that the problem it's solving is order-dragging-clarity, we'd like to try solving that first and then see if we still need this.)

The concern that @magcius mentions is that we (Endless Computers) only want team leads to have permission to triage priority. We set that up by restricting who has access to forms that allow editing priority. But the priority-sorted workboard defeats that form-based protection. Is there some other way to configure who has the ability to triage priority?

Why are users who shouldn't be changing priorities doing it?

If it's an accident or confusing UI, I think the best solution is to make the UI more clear. I'd like to pursue that before pursuing a hard permission. If we can make the UI clear, that might solve this problem (or make it so rare that it isn't concerning).

If there's another reason, can you walk me through what that reason and context is? For example, do paid, trained employees know they aren't supposed to change priorities but are using this mechanism to get around the rules?

Right now, the main concern is trained employees moving cards on the workboard and accidentally changing priorities (due to sorting by priority), so I agree that improving the UI would be the higher priority. In the meantime, perhaps we'll patch our instance with https://secure.phabricator.com/T8135#130563 to prevent the surprise priority changes.

That said, I'd like to be able to open up viewing to less-trained personnel (e.g., sales teams outside the U.S.) without concern that they may accidentally (or maybe not) move tasks on the workboard. I think that most such users will feel more comfortable using Phabricator if it is structured so that they can't make changes that they are not supposed to. It's probably a small enough concern, though, that we could just clean up afterwards and kindly ask someone not to do it again on the rare occasion that it becomes an issue.

scode added a subscriber: scode.Apr 4 2016, 10:00 PM

For what its worth, the biggest violators at my company tend to be company executives who (sometimes) rightfully wish to skirt our process. But, when they see how trivial it is to skirt our process, they're tempted to do it more often. And then they do it more often.

If, however, we could restrict who's able to move cards on the work boards, the very tools we use would enforce our process while still allowing these stakeholders to petition for their task priority.

We have no precedents of abuse in Phabricator so far, but looking at our history with Bugzilla I will not be surprised the morning we find a first precedent.

^ I am incredibly jealous of your perspective and am really, really surprised you haven't had this problem yet!

MZMcBride added a comment.EditedAug 1 2016, 9:24 AM

We have no precedents of abuse in Phabricator so far, but looking at our history with Bugzilla I will not be surprised the morning we find a first precedent.

^ I am incredibly jealous of your perspective and am really, really surprised you haven't had this problem yet!

That comment was written in November 2014. I can assure you that Wikimedia's Phabricator installation has seen instances of abuse and misuse since then. :-)

urzds added a subscriber: urzds.Jul 12 2017, 11:16 AM
epriestley closed this task as Wontfix.EditedMar 11 2019, 2:18 PM
epriestley claimed this task.

To summarize the state of the world here:

  • The current behavior is "you can drag a task if you can edit that task, regardless of your ability to edit the project the board belongs to".
  • After D20274, the UI makes this state more clear.
  • I expect recent and queued changes connected to T10334, particularly D20242, will reduce or eliminate cases of accidental reprioritization by dragging.

For now, I'm not planning to pursue further changes here under the banner of this particular task. I think some variation of "separate permission required to move stuff on a board" is still on the table, but if we revisit it in the future I'd like to make sure we're solving a specific problem which is still occurring after the ongoing changes connected to T13074 land.

The problems this task mentions seem solved, likely solved, probably better solved in other ways, or relatively weak:

  • "Anyone Can Move Anything": No longer true, you must have edit permission on the task to move it now (I think this has been the case for a long time, although probably not when this was originally filed). D20242 makes this more clear.
  • "Vandalism": Might still be a problem, but anyone who can vandalize a board can vandalize the tasks directly, so it's not immediately clear that board permissions are a good fix. If this remains an issue, I'd like to look at vandalism/abuse in general unless there's some specific reason that this kind of vandalism is especially problematic (e.g., especially hard to repair or especially attractive to casual vandals or whatever else), and I might want to try to solve those problems instead of changing the permissions model.
  • "Accidental Priority Changes": I think this is likely resolved by T10334.
  • "Board Drags Allow Edits Not Possible via EditEngine Forms": Maybe still an issue, but this is likely also sort-of-a-feature after T5474. I'd like to make sure this is still a problem after the next iteration ships. In particular, T10333 may actually make this problem much worse, by enabling a wider range of edit operations via dragging. But I'd probably solve this by preventing certain drag operations (e.g., if you can't edit owners, allowing you to drag tasks between columns but not to a different owner in the same column). This also gets very tricky now that subtypes mean you may have access to a field for some tasks but not others. Whatever form T10335 takes may help, but I'd prefer to find some way to sidestep this problem over tracking every user's ability to edit every attribute of every task.
  • "Our C-Level Team are Buffoons Who We Must Constantly Work to Undermine": I suspect this problem has a cleaner cultural solution, e.g. maybe give them a toy board called "Important Decisions Board" with columns like "High Priority", "Very High Priority", "Very Very High Priority!", etc, and then ignore it? I'd probably prefer to deal with this problem in general as a subset of the vandalism problem (e.g., by adding a "Parental Controls" mode) over adding board-specific controls, unless C-Levels are uniquely drawn to boards for some reason.

Generally, the landscape here will probably look at least somewhat different after triggers are available, so I think all variations of this problem are likely unripe in a pre-trigger world.

(As an example, a possible solution to some of these problems is a trigger like "Reject edit unless actor <satisfies some permission>", but this isn't part of the possibility space today.)