AppSync queries print empty relations

[DarylBeattie]

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

18.15.0

Amplify CLI Version

12.12.2

What operating system are you using?

Windows

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No.

Describe the bug

Ever since upgrading to amplify-cli 12.12.2, when I query an object that is supposed to return a sub-object, the sub-object returned is null, which causes GraphQL errors of Cannot return null for non-nullable type: 'ID' within parent 'ChildObj' (/updateParentObj/childObj/id) on mutation queries.

This is discussed in further detail in Discord here and here.
Supposedly, it is related to PR #2536.

Expected behavior

I expect the GraphQL to a) return the nested objects, and b) not throw errors.

Reproduction steps

1. With this model:

type ParentObj @model @auth(rules: [{ allow: owner }, { allow: private, operations: [create, read] }, { allow: groups, groups: ["Admin"] }]) {
  id: ID! @primaryKey
  name: String
  childObj: ChildObj @hasOne
  owner: String
    @auth(rules: [{ allow: owner, operations: [read, delete] }, { allow: private, operations: [read] }, { allow: groups, groups: ["Admin"] }])
}

type ChildObj @model @auth(rules: [{ allow: owner }, { allow: groups, groups: ["Admin"] }]) {
  id: ID! @primaryKey
  parentObj: ParentObj! @belongsTo
  email: String
  owner: String @auth(rules: [{ allow: owner, operations: [read, delete] }, { allow: groups, groups: ["Admin"] }])
}

2. Create a parent and child with the same owner.

3. Execute an update on the parent, using the default Amplify-generated GQL that returns the child (which we want, and have permission to retrieve).

mutation UpdateParentObj {
  updateParentObj(input: {id: "721cd514-7cbd-4d66-abcd-0f9e555f5cf6", name: "Test"}) {
    id
    name
    childObj {
      id
      email
      owner
      createdAt
      updatedAt
      childObjParentObjId
      __typename
    }
    owner
    createdAt
    updatedAt
    parentObjChildObjId
    __typename
  }
}

You will get this response:

{
   "data":{
      "updateParentObj":{
         "id":"721cd514-7cbd-4d66-abcd-0f9e555f5cf6",
         "name":"Test",
         "childObj":null,
         "ParentObjChildObjId":"child-721cd514-7cbd-4d66-abcd-0f9e555f5cf6",
         "errors":[
      {
         "path":[
            "updateParentObj",
            "childObj",
            "id"
         ],
         "locations":null,
         "message":"Cannot return null for non-nullable type: 'ID' within parent 'ChildObj' (/updateParentObj/childObj/id)"
      },
      {
         "path":[
            "updateParentObj",
            "childObj",
            "createdAt"
         ],
         "locations":null,
         "message":"Cannot return null for non-nullable type: 'AWSDateTime' within parent 'ChildObj' (/updateParentObj/childObj/createdAt)"
      },
      {
         "path":[
            "updateParentObj",
            "childObj",
            "updatedAt"
         ],
         "locations":null,
         "message":"Cannot return null for non-nullable type: 'AWSDateTime' within parent 'ChildObj' (/updateParentObj/childObj/updatedAt)"
      },
      {
         "path":[
            "updateParentObj",
            "childObj",
            "childObjParentObjId"
         ],
         "locations":null,
         "message":"Cannot return null for non-nullable type: 'ID' within parent 'ChildObj' (/updateParentObj/childObj/childObjParentObjId)"
      }
   ]
}

Project Identifier

No response

Log output

# Put your logs below this line

Additional information

No response

Before submitting, please confirm:

• I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
• I have removed any sensitive information from my code snippets and submission.

[DarylBeattie]

Attn @AaronZyLee, @dpilch, @palpatim wrt #2536.

[dpilch]

This is the intended behavior for mutations with regards to the data returned. The error messages should not be present, but the nullification of child fields is intended.

[DarylBeattie]

Is it? Why would it be?

1. I should be able to request those fields back. I want and need them back, as I use the returned objects to replace the objects I have in memory.
2. This used to work, and was working fine, for years.
3. Amplify generates mutations that return those objects by default; so clearly returning them is intended.
4. Why should I have to make 2 calls; one to mutate the object and another to get the correct object+sub-objects back? That's ridiculous. Furthermore I'd have to manually check to see if I got the objects I asked for in order to know if I need to make another call.

As far as I'm concerned this is a straight-up bug, and should be repaired.
Please reverse the "the nullification of child fields is intended" change of intention.
Or at least, make it configurable? Or something?

Philosophically, why would the returned data behave differently from a mutation to a query of the same structure? That's completely unexpected, as per GraphQL itself, it is an intention that you can return nested types:

Just like in queries, if the mutation field returns an object type, you can ask for nested fields. This can be useful for fetching the new state of an object after an update.

(Which is exactly the pattern that I, and likely many other Amplify users, do.)

[DarylBeattie]

Similarly, if Subscriptions are broken in this way, then that's a show-stopper for me.

I rely on subscriptions to update the objects in memory when they are updated by other processes elsewhere. It would completely break my app if subscriptions were updating objects and suddenly setting their child objects to null. -- That breaks the entire pattern and purpose of subscriptions.

I do have the permissions (through the auth rules) to get these sub-objects returned, so they should come back when requested.

[MarlonJD]

It's seems, it's really important issues that customers in already in production. I hope it will fix. I also had this issue on gen2.

[matt-at-allera]

Is it? Why would it be?

1. I should be able to request those fields back. I want and need them back, as I use the returned objects to replace the objects I have in memory.
2. This used to work, and was working fine, for years.
3. Amplify generates mutations that return those objects by default; so clearly returning them is intended.
4. Why should I have to make 2 calls; one to mutate the object and another to get the correct object+sub-objects back? That's ridiculous. Furthermore I'd have to manually check to see if I got the objects I asked for in order to know if I need to make another call.

As far as I'm concerned this is a straight-up bug, and should be repaired. Please reverse the "the nullification of child fields is intended" change of intention. Or at least, make it configurable? Or something?

Philosophically, why would the returned data behave differently from a mutation to a query of the same structure? That's completely unexpected, as per GraphQL itself, it is an intention that you can return nested types:

Just like in queries, if the mutation field returns an object type, you can ask for nested fields. This can be useful for fetching the new state of an object after an update.

(Which is exactly the pattern that I, and likely many other Amplify users, do.)

Strong agreement on all points, especially #3. If the auto-generated mutations are going to list out all nested objects regardless of the field-level @auth rules applied, the behavior we are seeing should not be the default.

[matt-at-allera]

Do we have a way to work around this to get our current setup to work? Does it involve repeating auth rules for the subscription?

[DarylBeattie]

@matt-at-allera I "worked around it" by npm-installing amplify-cli version 12.11.1.
I am doing this temporarily while waiting for this bug to be fixed.

[chrisbonifacio]

Thank you for contacting us regarding the changes to the fields included in your subscription updates of your Amplify-generated AppSync API. With Amplify CLI @aws-amplify/cli@12.12.2 and API Category @aws-amplify/amplify-category-api@5.11.5, AWS Amplify made an improvement to how relational field data is sent over subscriptions where the relational field data is now always redacted when different authorization rules apply to related models in a schema. The values for the fields now appear as null or empty. If an authorized end-user needs access to these redacted fields, we recommend they perform a GraphQL query to read the application data [1].

[1] https://docs.amplify.aws/react/build-a-backend/data/query-data/#list-and-get-your-data

The link above is for reading application data with Amplify Gen 2. For Amplify Gen 1, please refer to this page:
https://docs.amplify.aws/gen1/react/build-a-backend/graphqlapi/query-data/#list-and-get-your-data

[DarylBeattie]

So, I disagree that this is "an improvement". I firmly assert that it is a "bug".

If you ask for data via GraphQL, from a subscription or an update-response, and you have permission to see that data as defined by the authorization rules' strategies (as documented here for Gen1, and here for Gen2), you should get those fields back.

This should happen regardless of whether "different authorization rules apply to the related models" so long as you have authorization to access that data.

If you would refer to the example I outlined in the bug reproduction-steps above, ParentObj and ChildObj have "different rules", but both specify that the owner can access the ChildObj. This can/should function, that if the owner is requesting the object, then the ChildObj gets returned, but if a "private" non-owner user requests the object, then the ChildObj is not returned as they don't have access to it. This is in accordance with how the @auth is supposed to work, and follows the authorization patterns.

Not only should this data be returned; but currently, an exception is thrown as well as the data not being returned. So now it has become an error to use the Amplify-generated code to request data that you have access to.

Imagine a world where you ask a database for data via SQL and it randomly decides to ignore your join and return null for the joined data? This would be a tremendous issue, right?

[PeteDuncanson]

Cross posting from #2636 as seem related but not sure which one should be the main issue. #2636 (comment)

[DarylBeattie]

Cross posting from #2636 as seem related but not sure which one should be the main issue. #2636 (comment)

I'd vote for this one to be the main issue, as it was logged 2 weeks ago. The other one is a duplicate logged later. (I will comment on it too!)
Both bugs have reproducible examples, which is good.

[NicholasAllen]

I asked about this ticket in the Amplify Office Hours session on Discord, but unfortunately it wasn't my own "office hours", so I couldn't discuss it further with them. However, the summary of what I got was:

• this was a security fix to prevent accidental data exposure
• the team is continuing to discuss it internally, but it didn't sound promising that they were going to change tack on this being a necessary change and not a defect
• it only applies where dynamic auth rules are used
• the two workarounds suggested I think have already been covered by people here: disable subscriptions on the model items or re-query for the data you need. I guess down-grading the Amplify version is only a temporary workaround, and by their reckoning, leaves you at risk of data security issues

They said something about changing the paradigm between gen1 & gen2 from GraphQL to something more REST like, but as I've not yet looked in detail at gen2, I'm not totally sure the significance - I think the inference was that the pattern we have all been relying upon with mutation queries returning joined in relations wasn't how they saw it being used in gen2.

Like I say, unfortunately I couldn't fully engage with the topic on this session, but thought I'd share what they'd said. They seemed aware of the issues it had caused and would be following up on it, but it might not make us happy as the fix we all want is not going to be forthcoming.

[DarylBeattie]

Hi @NicholasAllen , I really appreciate you speaking to them about this for all of us. Thank you for that! I know you likely disagree with their points, but in case anyone at Amplify is reading this, here's my take on those things:

this was a security fix to prevent accidental data exposure

Nobody wants or is advocating for accidental/incorrect data exposure.
And the fact that (previously) the update and subscription GraphQL was returning data that you didn't have permission to access is a bug.
However, the fix of, "Fine, then nobody ever gets any data back!" is simply unacceptable. It's not a fix, it's a cop-out.
The correct fix is to correctly check if the requester has permissions to see the child-objects and return the child data only if they have access to it. If they don't, then return null (don't "throw exceptions").
This is how it works for queries, so why not updates and subscriptions?

the team is continuing to discuss it internally, but it didn't sound promising that they were going to change tack on this being a necessary change and not a defect

That's disappointing.
Though as I just explained, the "necessary change" should be to correctly check permissions, not add a "defect" of returning null for everybody (plus throw exceptions).

it only applies where dynamic auth rules are used

What's a "dynamic auth rule"?
In the reproduction example above, all the auth-rules are defined in the schema, which would make them "static", right?
Or does it mean, "the auth rule has to be checked dynamically"?

the pattern we have all been relying upon with mutation queries returning joined in relations wasn't how they saw it being used in gen2.

That is literally what returns from mutations, and what subscriptions entirely, are meant for.
As I commented above, it even says so in the official GraphQL documentation.

[ryanbeaz]

we downgraded to 12.12.0, but still running into sporadic issues. Tried a manual push from a local downgraded CLI version too. Any ideas? smaller mutations seem to work, but larger ones still error

looks like a feature flag was added to amplify-cli, but I don't know how to configure it: aws-amplify/amplify-cli@4f9aadb. Any ideas?

[ConradMMoss]

we downgraded to 12.12.0, but still running into sporadic issues. Tried a manual push from a local downgraded CLI version too. Any ideas? smaller mutations seem to work, but larger ones still error

Hi @ryanbeaz I have been facing the same issue. I managed to get a fix by pushing a whitespace addition to my graphql schema file after downgrading the version. This may have caused appsync to then use the old amplify version and downgrade the resolvers fixing the issue. Hopefully this helps!

[anni1236012]

+1
I think this is a bug and should be fixed. It must return and redact the data based on the authorization rules. I am also relying on subscriptions and UI is breaking because the fields are redacted that were expected in the subscription.

[999roberto999]

+1 Should fix, we are going to downgrade until then.

[dpilch]

@aws-amplify/cli@12.12.4 has been released with a feature flag to opt-out of the new behavior.

Please see the updates to the following docs:

• https://docs.amplify.aws/gen1/javascript/build-a-backend/graphqlapi/subscribe-data/
• https://docs.amplify.aws/gen1/javascript/tools/cli/reference/feature-flags/#subscriptionsInheritPrimaryAuth.

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

[OperationalFallacy]

How does it work in gen2?

[dpilch]

The ability to opt-out is not exposed on Gen 2. There are currently no plans to expose this functionality.

[PeteDuncanson]

@dpilch that seems a bold stance to not roll it out to Gen2. I was in the process of assessing what work is needed to convert over to Gen2 from our older Gen1 app (which is about 3 years+ old). From what you say here it sounds like this improvement of erroring and returning nulls for nested results in a mutation is now going to be by design going forward. As a result am I right in thinking that if I want to upgrade to Gen2 I should think of the feature flag fix as just an escape hatch for now and at some point I will have to refactor all my code to do the recommended "refetch if you need it!" fix for nested content in mutations?

Obvs I'm aware that things change and some time we have to refactor stuff to keep moving forward but this change really does add in a lot of work that none of us planned for or knew about. Does seem very much like our hand is forced there.