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

[api] Model helpers should not pass null fields #2492

Closed
1 of 13 tasks
dnys1 opened this issue Dec 15, 2022 · 10 comments
Closed
1 of 13 tasks

[api] Model helpers should not pass null fields #2492

dnys1 opened this issue Dec 15, 2022 · 10 comments
Assignees
Labels
bug Something is not working; the issue has reproducible steps and has been reproduced fixed-in-release-candidate Issues that have been addressed in the current release-candidate branch GraphQL API Issues related to the API (GraphQL) Category

Comments

@dnys1
Copy link
Contributor

dnys1 commented Dec 15, 2022

Description

AppSync interprets explicit null values different than implicit ones in the variables map of a request. Model helpers explicitly setting these values can lead to unexpected errors when these fields are read-only or otherwise protected.

For example, consider the schema:

type Todo @model @auth(rules: [{ allow: public }, { allow: owner, ownerField: "owners" }]) {
  id: ID!
  name: String!
  owners: [String]
  private: String @auth(rules: [{ allow: owner, ownerField: "owners" }])
}

And a mutation to create a Todo:

mutation CreateTodo($input: CreateTodoInput!) {
  createTodo(input: $input) {
    id
    name
    owners
    private
    createdAt
    updatedAt
  }
}

The following input allows creation with no issues:

{
  "input": {
    "name": "test",
    "private": "secret"
  }
}

However, model helpers generates the following input (notice the inclusion of owners which cannot be written to):

{
  "input": {
    "name": "test",
    "owners": null,
    "private": "secret"
  }
}

This leads to the following error:

{
  "data": {
    "createTodo": null
  },
  "errors": [
    {
      "path": [
        "createTodo"
      ],
      "data": null,
      "errorType": "Unauthorized",
      "errorInfo": null,
      "locations": [
        {
          "line": 2,
          "column": 3,
          "sourceName": null
        }
      ],
      "message": "Not Authorized to access createTodo on type Todo"
    }
  ]
}

Categories

  • Analytics
  • API (REST)
  • API (GraphQL)
  • Auth
  • Authenticator
  • DataStore
  • Storage

Steps to Reproduce

No response

Screenshots

No response

Platforms

  • iOS
  • Android
  • Web
  • macOS
  • Windows
  • Linux

Android Device/Emulator API Level

No response

Environment

N/A

Dependencies

vNext

Device

N/A

OS

N/A

Deployment Method

Amplify CLI

CLI Version

No response

Additional Context

No response

Amplify Config

N/A

@dnys1 dnys1 added bug Something is not working; the issue has reproducible steps and has been reproduced GraphQL API Issues related to the API (GraphQL) Category labels Dec 15, 2022
@ragingsquirrel3
Copy link
Contributor

I made a PR for this. Noted in the PR description but IMO we should not remove all null values from mutation input (plenty of valid use cases for doing that), only null values when they are owner fields.

@okhomin
Copy link

okhomin commented Dec 24, 2022

I faced the same issue for my model:

type UserInfo @model @auth(rules: [{allow: owner}]) {
  id: ID!
  owner: String @auth(rules: [{ allow: owner, operations: [read, delete] }]) # prevent reassigning of user
  nickname: String
  firstLineAddress: String
  secondLineAddress: String
}

And amplify codegen models generates this code:

  Map<String, dynamic> toJson() => {
    'id': id, 'owner': _owner, 'nickname': _nickname, 'firstLineAddress': _firstLineAddress, 'secondLineAddress': _secondLineAddress, 'createdAt': _createdAt?.format(), 'updatedAt': _updatedAt?.format()
  };

So instead of a json without the owner field it sends a json with "owner": null, which leads to the Unauthorized error.

@okhomin
Copy link

okhomin commented Dec 24, 2022

@ragingsquirrel3 Is there a workaround before your fix?
Currently I have to manually fix the generated models.

@ragingsquirrel3
Copy link
Contributor

ragingsquirrel3 commented Dec 27, 2022

@okhomin yes there is a workaround. You basically have to remove the unwanted keys from the request input variables and copy to a new request like this:

final Blog blog = Blog(id: id, name: name);
final GraphQLRequest<Blog> req = ModelMutations.create<Blog>(blog);
final modifiedInputVariables =
    req.variables['input'] as Map<String, dynamic>;
modifiedInputVariables.remove('owners');
final modifiedVariables = {'input': modifiedInputVariables};
final modifiedRequest = GraphQLRequest<Blog>(
  document: req.document,
  decodePath: req.decodePath,
  modelType: req.modelType,
  variables: modifiedVariables,
);
final operation = Amplify.API.query(request: modifiedRequest);

Quick followup question: Is your use case a create mutation with ModelMutations.create()?

@ragingsquirrel3 ragingsquirrel3 added the pending-release Issues that have been addressed in main but have not been released label Jan 13, 2023
@theartofnonso
Copy link

Update as of today, I was experiencing this issue with the stable release (v0.6.12). However I switched to the developer preview version (v1.0.0-next.4).

I can confirm that the issue is still happening in the stable release but the dev preview is fine.

I guess I will be sticking to the working version.

@dnys1 dnys1 added the fixed-in-release-candidate Issues that have been addressed in the current release-candidate branch label Feb 17, 2023
@Jordan-Nelson Jordan-Nelson removed the pending-release Issues that have been addressed in main but have not been released label Mar 2, 2023
@Jordan-Nelson
Copy link
Contributor

This issue has been addressed in v1.0.0 of Amplify Flutter, which is now stable. This release also includes web and desktop support for Auth, API, Analytics, and Storage. You can see the list of new features and bug fixes in the release notes, and see more details on how to migrate in the upgrade guide.

@maziarzamani
Copy link

I have the same problem with amplify_flutter=1.3.1. Is this is a bug?

@dnys1
Copy link
Contributor Author

dnys1 commented Aug 21, 2023

Hi @maziarzamani. Would you be able to create a new issue with details about your schema and the issue you're facing?

@maziarzamani
Copy link

Hi @maziarzamani. Would you be able to create a new issue with details about your schema and the issue you're facing?

I am actually on a AWS Support right now, as I was able to fix it again by removing the field-level restriction, so there is definitely something weird going on in the SDK.

@maziarzamani
Copy link

Long story short, it seems to be a bug in the SDK, if any AWS personnel needs a reference to the case ID, please let me know. I'd really love if this bug was fixed as I have many read-only field permission in my GraphQL schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working; the issue has reproducible steps and has been reproduced fixed-in-release-candidate Issues that have been addressed in the current release-candidate branch GraphQL API Issues related to the API (GraphQL) Category
Projects
None yet
Development

No branches or pull requests

6 participants