aws-amplify / amplify-cli

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development.
Apache License 2.0
2.82k stars 821 forks source link

add auth field to connection directive #7260

Closed avlonder closed 3 years ago

avlonder commented 3 years ago

Is this related to a new or existing Amplify category?

api

Is this related to another service?

No response

Describe the feature you'd like to request

To protect the grapqhQL API against nesting attacks, the connection resolvers should have the option to specify an identity check.

A basic example of a nesting attack is: a user is allowed to query his friends. If the friendship joint table is not protected by some identity check, this user is also able to query friends of friends and so on.

Describe the solution you'd like

For example:

type User {
  id: ID!
  name: String
 friends: [Friendship] @connection(fields: ["id"])
}

type Friendship @key(fields: ["userId", "friendId"]) {
  userId: ID!
  friendId: ID!
  user: User @connection(fields: ["userId"], authFields: ["userId, friendId"])
  friend: User @connection(fields: ["friendId"], authFields: ["userId, friendId"])
}

And in the result resolver template

#set($authorized = false)
#foreach($entry in $authFields)
  #if($ctx.result[$entry] == $ctx.identity.username)
    #set($authorized = true)
  #end
#end
#if(!$authorized)
  $util.unauthorized()
#end

Describe alternatives you've considered

Set a maximum depth on the nesting of objects

Additional context

No response

Is this something that you'd be interested in working on?

attilah commented 3 years ago

@avlonder thanks for the feature request, @connection do support authorization on related data if queries are disabled on the related type (Friendship). As this is a very special use case to unblock you I'd suggest to implement this functionality with custom resolvers.

avlonder commented 3 years ago

@attilah, even if queries are disabled on the friendship type, a user can access data from friends of friends of friends of ... (you see where I'm going), because there's nothing validating the identity in the resolvers created by the connection directive.

avlonder commented 3 years ago

Indeed, I could write custom resolvers to exactly do this identity check, but then what's the point in offering a CLI with graphql-transformer. Also, I think this is a very common use case and many AppSync practitioners are unaware of the security leak.

avlonder commented 3 years ago

Ow, now I see the confusion... I had a misunderstanding about $ctx.source (I thought it was the object one level higher, but it is the very root object), so indeed, the solution I propose is not better than just not creating queries like you suggested. It must be $ctx.result instead of $ctx.source (I've edit my original post above).

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.