Fix message queries

[joshsmith]

What's in this PR?

Fixes some issues with messages.

WIP.

• needs tests for user_filter

[joshsmith]

An issue that this PR does not yet address is that we are not allowing a user to view the Message that initiated the Conversation where they are the target. For security reasons, either:

• the user should not have access to the Message; this would require us to duplicate the project_id across all the Conversation records
• we use a different view to generate a subset of the JSON scoped to the user's authorization level

[joshsmith]

To clarify the above, the CodeCorps.Policy.Message.scope/2 limits the messages queried to projects administered by users or messages where the user is the author.

If we were to go with the second option above, we would need to also query messages joined with conversations where the user is the target, and scope the messages queryable by that.

[joshsmith]

The former option is likely simpler but more inflexible. The second option will always provide us with the data we want but at a higher overhead.

[joshsmith]

Thinking about this further, there is, as of yet, no data on the message record that the user could not see that is not scoped in some other way. They can and should see the body, who initiated the conversation, the subject, the author, and the project. The conversations would be scoped if they tried to access the endpoints for them.

We may only need the query for now and can open up a follow-on issue to scope the JSON in the event we add more sensitive fields to the message record.

[begedin]

Valid points.

The way I see it, from a user's point of view, they care about the conversation and conversation part.

The message record is a special, project-only thing which allows projects to message multiple users at once.

I agree we may only need what we have for now, but if that becomes false, the most logical solution for me would be to clone the respective fields to the conversation and handle it that way.

The users then care about conversations and conversation parts only, while projects also care about messages.

That being said, I think then we need to think about needing the message in the first place. In the hypothetical scenario where we are messaging multiple users, we are creating multiple conversations and one message. Since all those conversations are duplicating fields on the message, the only role the message serves is some sort of strange "hard grouping" of conversations. We could easily go with "soft" grouping on the client. It would probably save us a bunch of database as well as API queries.

[joshsmith]

The messages are going to be necessary, particularly when we add in auto-messages. In the current UI they appear superfluous, but there is more coming there that does not make them superfluous.

[begedin]

@joshsmith I've pushed up the agreed-upon change in Message scope/policy, but it required another change.

The Conversation scope relied on the Message scope and was able to see all Conversation records belonging to any Message they are able to see. Since they can now also see messages where one of the child conversations is targeted at them, this would have allowed them to see all other conversations originating from the same message.

A more concrete example

Project admin sends message_a to user_a and user_b

With the old policy, user_a was able to see conversation_a due to targeting them, but not message_a, due to them not being the author of the message, nor being admin or higher on the message project and thus, not conversation_b

With the new message policy, user_a, without a change to Policy.Conversation
would be able to see conversation_a due to targeting them, as well as message_a (due to message_a being the parent of conversation_a), as well as conversation_b, due to being able to see all conversations belonging to a message in their message scope.

We don't want the user to see conversation_b, so Policy.Message.scope/2 and Policy.Conversation.scope/2 needed to diverge instead of building on top of each other.

[joshsmith]

Can't approve since my PR, but 👍 and will merge.