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

Seems out of place that we are checking ModelOperation in isOwnerArgumentRequired #778

Closed
saltonmassally opened this issue Aug 31, 2020 · 1 comment · Fixed by #807
Closed
Labels
bug Something isn't working GraphQL API Related to the API (GraphQL) category/plugins

Comments

@saltonmassally
Copy link
Contributor

Looking at com.amplifyframework.api.aws.AWSApiPlugin#isOwnerArgumentRequired it seems out of place that we are checking ModelOperation before passing ownerField in our subscription request.

  1. Correct me if wrong but i don't think a user needs CREATE, UPDATE, DELETE, perms to be able to receive ON_CREATE, ON_UPDATE, ON_DELETE subscriptions. I don't have to give a user UPDATE permission to my api for them to be notified of an update made to a record they observe.

  2. amplify cli still build the subscriptions with the owner field... (will send examples shortly)

take this model definition as an example:

type Share
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["ForbiddenGroup"] }
      { allow: owner, ownerField: "accessUsers", operations: [read] }
      { allow: private, provider: iam }
    ]
  )
 {
  id: ID!
  accessUsers: [String!]! # users that have access to the record
}

this is what cli outputs

  onCreateShare(accessUsers: String): Share @aws_subscribe(mutations: ["createShare"]) @aws_iam @aws_cognito_user_pools
  onUpdateShare(accessUsers: String): Share @aws_subscribe(mutations: ["updateShare"]) @aws_iam @aws_cognito_user_pools
  onDeleteShare(accessUsers: String): Share @aws_subscribe(mutations: ["deleteShare"]) @aws_iam @aws_cognito_user_pools

Trying to sub from the android app will fail as owner field will not be passed in given the conditions set in the method in question.

I would think that all we need to check in this case is ModelOperations.READ

saltonmassally added a commit to mikashboks/amplify-android that referenced this issue Aug 31, 2020
@TrekSoft TrekSoft added GraphQL API Related to the API (GraphQL) category/plugins bug Something isn't working labels Sep 1, 2020
@richardmcclellan
Copy link
Contributor

richardmcclellan commented Sep 1, 2020

Thanks for reporting this @saltonmassally ! I believe your understanding is accurate. I'm going to resolve this issue, as I think it is a duplicate of #699, but I will take a look at your PR shortly as well. Before we finalize this change, we need to make sure iOS and JS are in sync with this logic as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GraphQL API Related to the API (GraphQL) category/plugins
Projects
None yet
3 participants