Page MenuHomePhabricator

Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for
Closed, ResolvedPublic

Description

Via HackerOne. The severity of this is extremely low so I'm not sure when I'll get around to fixing it, but here's a UI approach:

  • Alice joins room General Chat.
  • Alice loads General Chat in her browser.
  • In another window, kick Alice out of the room and change "Can Join" to something like "No One".
  • Send a message from the first window.

The message sends, which is incorrect. There's a more general version of this with raw HTTP requests but it's more cumbersome.

This is an outgrowth of CAN_VIEW usually being sufficient to comment in other applications (e.g., tasks don't have a "Can Join").

Maybe one fix for this is really just to remove the "Can Join" permission completely? But if we don't, we should check for it before accepting messages.

Related Objects

Event Timeline

A related issue is that CAN_EDIT does not imply CAN_JOIN, so users with CAN_EDIT but no CAN_JOIN can get a policy error while trying to join.

However, they can add themselves to the participant list or edit the join policy, add themselves, join, then revert the join policy, so this error is pointless and somewhat confusing. Instead, CAN_EDIT should just imply CAN_JOIN.

(Or we should just remove CAN_JOIN -- is there any actual use case for read-only rooms? If there is, shouldn't users still be able to join those rooms in order to follow them, just not send messages?)

chad added a comment.Apr 13 2017, 3:49 AM

I think we should remove the CAN_JOIN. If you can see it, you can join it. I don't see any parallel in Slack and looks like there are mods that add it forceably by kicking you off a channel if you leave a message. lulz.

chad claimed this task.Apr 13 2017, 3:50 AM
chad moved this task from Backlog to v4 on the Conpherence board.Apr 13 2017, 4:47 AM
chad edited projects, added Conpherence (v4); removed Conpherence.

That seems reasonable to me. We could implement a "read-only" equivalent later by having a flag like "require edit permission to send messages to this room" if we need it. That probably makes more sense anyway -- I can't really think of any use cases where a room is sort-of read-only, but the users who can post to it are different than users who can edit it.

The best I can come up with is something like this:

  • Room is "Design Decrees".
  • @chad is king.
  • But a few subjects who he trusts only a tiny amount may also post announcements.

But I think it's fine to say "just let them edit the room too and fire them if they abuse their power, since you're already trusting them a lot anyway".

(And I think "Conpherence as an announcement tool" is a very marginal use case anyway -- today, Phame is probably better already?)