DMs: Secure messages

[GeorgeBellTMH]

Figure out a way to make sure the API doesn't return messages you shouldn't be able to read (ie that are not part of a room you are not in). May need to make a custom VTL for this.

┆Issue is synchronized with this Wrike Task

[JonEsparaz]

@GeorgeBellTMH I think we can avoid the VTL. What do you think of this?

type DirectMessage
@model
@key(name: "byMessageUser", fields: ["messageRoomID", "when"], queryField: "directMessagesByRoom")
@key(name: "dmsByUser", fields: ["userId", "when"])
{
  id: ID!
  content: String!
    @auth(rules: [
      { allow: owner, ownerField: "userId", operations: [create, update, read] },
      { allow: owner, ownerField: "recipients", operations: [create, update, read] },
    ])
  attachment: String 
    @auth(rules: [
      { allow: owner, ownerField: "userId", operations: [create, update, read] },
      { allow: owner, ownerField: "recipients", operations: [create, update, read] },
    ])
  attachmentName: String
    @auth(rules: [
      { allow: owner, ownerField: "userId", operations: [create, update, read] },
      { allow: owner, ownerField: "recipients", operations: [create, update, read] },
    ])
  when: String!
  recipients: [String]!
  userId: ID!
  author: User @connection(fields: ["userId"])
  messageRoomID:ID!
  messageRoom:DirectMessageRoom @connection(fields:["messageRoomID"])
}

[GeorgeBellTMH]

Not sure if you can use arrays, but I'm willing to try it and see what happens...

[GeorgeBellTMH]

aws-amplify/amplify-category-api#365

[GeorgeBellTMH]

I'm fine if we leave this for now...unless you feel adventurous and want to do an Amplify PR....

[JonEsparaz]

The docs suggest that this should work, but I not 100% sure: https://docs.amplify.aws/cli/graphql-transformer/directives#multiple-authorization-rules

[GeorgeBellTMH]

k, doesn't hurt to try ;-)

[GeorgeBellTMH]

Looks like we will need to at least handle the exception...we may need to write a script as well...

[JonEsparaz]

@GeorgeBellTMH the auth rules seem to work, but it causes the subscriptions to break... The message content is null when pulled by the subscription (it's fine if you reload the page).

We might need a custom subscription with auth rules... I'm not sure though.

[GeorgeBellTMH]

It did give me an error when I pushed it about subscriptions and required fields. That’s why I made one of the fields optional - to get it to push the changes successfully...

Sent from my iPhone

On Jul 25, 2020, at 9:30 PM, Jon Esparaz notifications@github.com wrote:



@GeorgeBellTMHhttps://github.com/GeorgeBellTMH the auth rules seem to work, but it causes the subscriptions to break... The message content is null when pulled by the subscription (it's fine if you reload the page).

We might need a custom subscription with auth rules... I'm not sure though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/jesus-collective/mobile/issues/242#issuecomment-663924293, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALMJUYRN3GQJRMETFA2WS63R5OBLHANCNFSM4OQXIVDQ.

[JonEsparaz]

I read the issue you posted in aws-amplify/amplify-cli#277. Seems like there's issues using Cognito and subscriptions too: https://github.com/aws-amplify/amplify-cli/issues/4936

I think the best way forward is to store everyone (sender and recipients) in a single array, then use custom VTL as you originally suggested and only return if the current user is in said array.

[JonEsparaz]

@GeorgeBellTMH how do you want to approach this so we can publish to Beta without breaking DMs? @lucastbelem and I discussed and it would be nice to get aws-amplify/amplify-cli#345 published sometime next week.

[GeorgeBellTMH]

I did a bunch of changes to get it to render in beta and added recipients to the DM's...it posts but the content for new messages is null for some reason...refresh and it's fine...

[lucastbelem]

@JonEsparaz @GeorgeBellTMH it's been a while since we looked at this, but this problem is very much still a thing, especially now that we're adding more and more people to the platform. I have people reporting receiving messages that aren't theirs. @JonEsparaz would you be in a position currently to:

• have time to tackle this?
• be interested in it?
• have the answers you need to resolve the issue?

I know you and George were going back and forth on a solution that didn't break things. Not sure if you guys arrived at one.

[JonEsparaz]

@lucastbelem I'll have to pass at this time.

[lucastbelem]

George said something about including the ID of who you're sending to instead of the index.