Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL: @auth on a single field gives write on all fields, delete on all entries #308

Closed
stelcheck opened this issue May 20, 2020 · 9 comments
Assignees

Comments

@stelcheck
Copy link

Note: If your question is regarding the AWS Amplify Console service, please log it in the
official AWS Amplify Console forum

Which Category is your question related to?

AppSync/GraphQL

Amplify CLI Version

4.18.1

What AWS Services are you utilizing?

AppSync, DynamoDB, Cognito, S3

Provide additional details e.g. code snippets

Ref: https://docs.amplify.aws/cli/graphql-transformer/directives#combining-authorization-rules

Given the following GraphQL schema:

type Post
  @model
  @auth(
    rules: [
      { allow: public, provider: iam, operations: [read] },
      { allow: private, provider: iam, operations: [read] }
    ]
  ) {
  id: ID!
  title: String!
  content: String
  viewCount: Int @auth(
    rules: [
      { allow: private, provider: iam, operations: [update] }
    ]
  )
}

I expect I should be able to mutate only the viewCount, but I am able to both mutate other fields and delete records. I don't know if this is a bug or it's supposed to work like this, but I was a bit surprised (scared?) when I found that out.

I'm assuming I misunderstood the documentation; isn't there a way to have a record be read-only, but still allow for updates on a single field?

@stelcheck
Copy link
Author

I should also mention that if I remove update from the field, I do end up with read-only for both private and public.

@attilah
Copy link
Contributor

attilah commented May 20, 2020

@stelcheck please give some details about the security configuration:

  • Default auth mode
  • Additional auth modes

@stelcheck
Copy link
Author

stelcheck commented May 20, 2020

@attilah Default auth mode is IAM, no other auth modes are configured.

Edit: If I recall I started with userpool but then changed to IAM by running amplify update ^ mentioning it in case this might have an impact.

@attilah
Copy link
Contributor

attilah commented May 20, 2020

I was thinking about this scenario and with IAM you cannot do field level grant based on operation and I think CLI should give a warning about this, when an operation is specified for IAM provider on a field. In the resolver where we could generate this logic, but in case of IAM there is no such thing as "user", however with Cognito I can't see why you would not be able to enable your use case.

Did you run into any issues with Cognito?

@stelcheck
Copy link
Author

stelcheck commented May 20, 2020

Edit: For my current use-case I cannot use UserPool, instead I am using the IdentityPool.

The documentation link I posted earlier has the following example:

type Post @model @auth (rules: [{ allow: private }]) {
  id: ID!
  title: String
  owner: String
  secret: String
    @auth (rules: [{ allow: private, provider: iam, operations: [create, update] }])
}

The case is a bit different, but here a field is clearly being made writeable for private IAM user. Does this work only because it is combined with UserPool?

At any rate, I don't think a warning would be enough if it is not supported, it should error out; right now by attempting to give rights on a field it somehow ends up with all table entries being updatable and deletable, and I cannot think of a case where a developer would want to ignore that.

@attilah
Copy link
Contributor

attilah commented May 21, 2020

@stelcheck I agree, perhaps an error would be better for this use case, but the checks around it making it complex, since it is kind of an edge case, we'll give some thoughts to it what would be the best solution. For field auth if you need a solution now, you can write a custom resolver template where you check the info property in the $context so if it contains other fields in the update resolver just throw an error or ignore the other fields when you create the DynamoDB update expression. If provides a solution for your specific use case without causing a security issue. Docs for info object is here: https://docs.aws.amazon.com/appsync/latest/devguide/resolver-context-reference.html

@stelcheck
Copy link
Author

I ended up going in the direction of using custom mutations/resolvers instead. It requires more files, and in this current use case you need to manually manage IAM rights, but in the end each portion is more readable. It was also a great way to learn a bit more about the system :)

Let me know if I can contribute more informations/opinions/etc, I can imagine it's not a trivial issue to tackle.

@loganpowell
Copy link

loganpowell commented Feb 23, 2022

I'm having a similar problem, but in my case adding provider: iam to a type with only read access grants everything but read access to that type, e.g.:

#...

type Edge 
    @model 
    @auth(rules: [
        { allow: owner, ownerField: "owner" } , 
        { allow: groups,                        groups: ["Admins", "Editors"]}
        { allow: groups, operations: [ read ],  groups: ["Viewers"]}
        { allow: public, operations: [ read ],  provider: iam }
    ]) 
{
    id          : ID!       @primaryKey
    createdAt   : AWSDateTime!
    objectID    : ID!       @index(name: "Objects_by_subject")
    subjectID   : ID!       @index(name: "Subjects_by_object")
    object      : Node!     @belongsTo(fields: ["objectID"])
    subject     : Node!     @belongsTo(fields: ["subjectID"])
    type        : EdgeType! @index(name: "Edges_by_type", queryField: "edgesByType", sortKeyFields: ["createdAt"])
    owner       : String
    weight      : Int
}  

make these permissions in IAM:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Action": "appsync:GraphQL",
            "Resource": [
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Edge/*",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/ModelEdgeConnection/*",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Query/fields/getEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Query/fields/listEdges",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Mutation/fields/createEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Mutation/fields/updateEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Mutation/fields/deleteEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Query/fields/edgesByType",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Subscription/fields/onCreateEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Subscription/fields/onUpdateEdge",
                "arn:aws:appsync:us-east-1:538069693173:apis/vsfu7jwfrjcp5ikxhpkrwj2usy/types/Subscription/fields/onDeleteEdge"
            ],
            "Effect": "Allow"
        }
    ]
}

I have tried different combinations of permissions for IAM, but the only way I can get anything to change in IAM is to remove it completely from the type.

@renebrandel
Copy link
Contributor

closing this as in Transformer v2, we've moved to a deny-by-default authorization principle and we enforce the iam' unauthrole name in the AppSync resolver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants