amplify-codegen
amplify-codegen copied to clipboard
Amplify Codegen does not generate output in a consistent/deterministic order
First of all, I'm surprised I couldn't find an existing bug, so if I've missed it, apologies. But as I couldn't find one I'm creating this ticket as as surely I'm not the only one this drives nuts over time...
Describe the bug
Running amplify push and regenerating GraphQL code bindings, or simply running amplify codegen on a GraphQL schema that hasn't changed results in unnecessary changes to files such as graphql/schema.json simply due to reordering of the same output due to apparent non-deterministic ordering in the generated code.
Amplify CLI Version 4.35.0
To Reproduce
Run the commands listed above a few times and see unnecessary and noisy changes made to files such as graphql/schema.json.
Expected behavior All code generated should be generated in a consistent deterministic, possibly sorted, order so that:
- It is clear that a change in the generated file is caused by a change to the GraphQL schema that has just been pushed up, rather than changes in generated files unrelated to the schema change.
- Currently I have to either spend a lot of time eyeballing the diffs to and reverting hunks if I'm absolutely sure something didn't change, or otherwise include it in file history just to be safe.
- This is especially true because updating the
@aws-amplify/clipackage can cause expected changes to the generated files that are required even if not caused directly by user changes to their GraphQL schema, and so running a codegen after updating the package version without any GraphQL schema changes should be able to show if the version has introduced changes or not; currently that's not clear, it could just be non-deterministic reordering.
- Git Commit history is not polluted with a ton of unnecessary changes making it hard to browse file history.
Screenshots An example diff highlighting one such apparently unnecessary reordering:
Desktop (please complete the following information):
- OS: macOS 11.0.1
- Node Version: 15.2.0
Additional context Git commit history is important to in order to understand when particular bugs may have been introduced and by what changes, and so unnecessary noise in codegen files is more important than it may seem.
@spc16670 Its hard to understand the problem without the records and schemas. Could you share GraphQL schemas with v1 and v2 which can be used to reproduce this issue. Please ensure there isn't any confidential info in the schema.
Also include a few mutation which can be used to create records in V1 and query to retrieve those in V2.
My v1 version of the schema is not very different:
type Client @model
@auth(rules: [
{ allow: groups, groups: ["Root"] }
{ allow: groups, groups: ["Partner"], operations: [create] }
{ allow: groups, groupsField: "id", groupClaim: "client:owner" }
{ allow: groups, groupsField: "id", groupClaim: "client:custodian", operations: [read, update] }
{ allow: groups, groupsField: "id", groupClaim: "client:user", operations: [read] }
])
{
id: ID!
name: String!
createdAt: AWSDateTime!
updatedAt: AWSDateTime
}
I don't use any custom transformers - all stock amplify.
Is there any reason why the earlier schema and list operation done by a user with only client:custodian claim with a value of "[\"GVP8919\"]" should not work in v2 (given there is a record with id GVP8919 in the Client table)?
I am not sure. Could you also include the Mutations where I can insert record to client with GVP8919. Also include the list query used in both V1 and V2
I used standard mutations as generated by amplify - same with list queries. I tried the list query in app sync console as well and it is giving the same results.
Here is the list query:
query MyQuery {
listClients {
items {
createdAt
id
name
updatedAt
}
}
}
We have the same issue after upgrading to the new transformer.
Simplified model:
type Project
@model
@auth(rules: [
{ allow: owner },
{ allow: groups, groupsField: "id", operations: [read, update] }
])
{
id: ID!
name: String!
}
In the PreTokenGenerator's alter-claims.js function, after fetching the projects the user should have access to, we're adding their ids as groups:
event.response = {
claimsOverrideDetails: {
groupOverrideDetails: {
groupsToOverride: projectIDs
}
}
};
I can see that the group got added correctly to the client's JWT token, when decoded:
"cognito:groups": [
"e3673234-bc4b-4700-892c-fa282fbc1919"
]
However, the user gets unauthorized errors for querying that project.
This used to work previously.
Our current amplify-cli version is 7.6.21.
I subscribe to this, I am also facing multiple issues in that area. Not sure what Amplify team's plans are and if they are going to tackle it. I added a PR a while back to fix some of these issues but it seems they are not accepting any PRs at the moment due to some internal refactoring I understand https://github.com/aws-amplify/amplify-cli/pull/9535#issuecomment-1036700574
We are also having this problem as I have described in aws-amplify/amplify-cli#9466 here So I would also be more than happy to see a solution for this very soon! Moreover I think this problem needs to be stated somewhere in the docs as a "known limitation" as long as this is not fixed. I think for most of the people running in this issue it is not very understandable. As some other guys (thanks @n0rb) found out, the bug comes up because the authentication vtl rules for an "id" field are created in queries.ts by generateAuthOnModelQueryExpression method while the rules for the other "non id fields" were created by generateAuthFilter method. So you are seeing this issue as you are also using "groupsField: id". Our model looks like this and has the same issue:
type Location @model
@auth(rules: [
{ allow: groups, groups: ["Admin"], operations: [create, read, update, delete] }
{ allow: groups, groupsField: "id", groupClaim: "la", operations: [read, update, delete] }
]) {
id: ID!
}
cc @danielleadams (I'm not sure you are the right contact person but you helped in aws-amplify/amplify-cli#9535)
Thanks for this comment, @endoodev1. With this information I was able to create a workaround for our use case until this issue is properly fixed. I added a new "group" property to the Project table that I duplicate the id value into. (This requires the Create query followed by and Update query, as the id is autogenerated, thus unknown at the time of creation.) At least this way we can get all our app's collab features working again and unblock development.
type Project
@model
@auth(rules: [
{ allow: owner },
{ allow: groups, groupsField: "group", operations: [read, update] }
])
{
id: ID!
group: String! # duplication of id for @auth group claim
name: String!
}
Not sure group needs to necessarily be a required field, like I implemented it.
@spc16670 Thanks for reporting this (and thanks all for the chiming in) - I know you've provided a lot of information here, but can you provide specific reproduction steps to investigate this? I tried the following:
- Create api with transformer v1, the group ID is set as the record ID. example schema:
type Group @model @auth(rules: [
{ allow: owner, operations: [create, update, delete] }
{ allow: groups, groupsField: "id", operations: [read] }
]) {
id: ID!
name: String!
}
- Create 2 users in cognito and add them to a group
- Create records as user 1, set the record's group to the group's group name
- Update API to use transformer v2 with the following schema:
type Group @model @auth(rules: [
{ allow: owner }
{ allow: groups, groupsField: "id", operations: [read] }
]) {
id: ID!
name: String!
}
- amplify push
- Query list as user 1, returns list as expected with the previously created record
- Query list as user 2, returns list with user 1's previously created record as expected
I've also tried variations of this, and I'm not able to reproduce. What am I missing here?
We're having the issue with transformer v2. In v1 it worked fine.
On Tue., Feb. 22, 2022, 16:08 Danielle Adams, @.***> wrote:
@spc16670 https://github.com/spc16670 Thanks for reporting this (and thanks all for the chiming in) - I know you've provided a lot of information here, but can you provide specific reproduction steps to investigate this? I tried the following:
- Create api with transformer v1, the group ID is set as the record ID. example schema:
type Group @model @auth(rules: [ { allow: owner, operations: [create, update, delete] } { allow: groups, groupsField: "id", operations: [read] } ]) { id: ID! name: String! }
- Create 2 users in cognito and add them to a group
- Create records as user 1, set the record's group to the group's group name
- Update API to use transformer v2 with the following schema:
type Group @model @auth(rules: [ { allow: owner } { allow: groups, groupsField: "id", operations: [read] } ]) { id: ID! name: String! }
- amplify push
- Query list as user 1, returns list as expected with the previously created record
- Query list as user 2, returns list with user 1's previously created record as expected
I've also tried variations of this, and I'm not able to reproduce. What am I missing here?
— Reply to this email directly, view it on GitHub <aws-amplify/amplify-category-api#110>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGS3GEMQ6BWY3YXTZ255YJDU4QQRVANCNFSM5OUDGMPQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID: @.***>
@hanna-becker I am not able to reproduce the issue in v2, so I am asking for specific steps to duplicate the issue.
Hi @danielleadams
Thanks for looking into this!
Re: step 2, I'm not sure I understood you correctly here, but do not add the users to a static group. Instead do this: When adding/updating auth, the cli asks about whether it should add triggers at some point. Add a PreTokenGeneration lambda trigger, and add the id of the shared resource (Group in your case, Project in my example above) in this block in the generated "alter-claims.js" file.
claimsOverrideDetails: {
groupOverrideDetails: {
groupsToOverride: [<insert shared resource id>]
}
}
Normally we would get the ids to add here from a DB query, but for the sake of reproducing this, hard-coding to the id of an existing record should be sufficient.
In our case we recreated the app from scratch after the transformer upgrade, but this may be different for others, so I cannot say anything about whether or not creating the api with v1 and then updating to v2 will reproduce the issue. Maybe others can chime in here.
Otherwise the steps to me look like you should be able to reproduce the issue with them.
[Edit] Nit: Since these claims get added at login, be sure to log the test users out and in after amplify pushing the Pretoken lambda.
Hey @danielleadams, thanks for taking up this issue. As @hanna-becker already explained, it is a problem with the dynamic groups authentication policies in v2 transformer. In my previous post I also explained the exact position in code where this bug comes from. It has to do with the vtl rules which are created in the queries.ts and which do not receive the groups claim from the jwt token as a json array in generateAuthOnModelQueryExpression method when it is an id field. For other fields the vtl auth rules are created by generateAuthFilter method which is working correctly (receiving the group claim as a json array) also because of pr aws-amplify/amplify-cli#9466. In the PR this issue is explained much better. Unfortunately I'm not 100% sure how vtl language is working so I'm not able to create a good working pr on my own. This is vtl code of id field of query.getXXX.auth.1.req.vtl
#if( !$util.isNull($ctx.args.id) )
#set( $idClaim = $util.defaultIfNull($ctx.identity.claims.get("GroupClaim-name"), "___xamznone____") )
#if( $util.isString($ctx.args.id) )
#set( $idCondition = ($idClaim == $ctx.args.id) )
#else
#set( $idCondition = ($idClaim == $util.defaultIfNull($ctx.args.id.get("eq"), "___xamznone____")) )
#end
#if( $idCondition )
#set( $isAuthorized = true )
$util.qr($ctx.stash.put("authFilter", null))
#end
this is vtl code for non ID field:
#set( $role0 = $util.defaultIfNull($ctx.identity.claims.get("GroupClaim-name"), []) )
#if( $util.isString($role0) )
#if( $util.isList($util.parseJson($role0)) )
#set( $role0 = $util.parseJson($role0) )
#else
#set( $role0 = [$role0] )
#end
#end
#if( !$role0.isEmpty() )
$util.qr($authFilter.add({ "tenant_id": { "in": $role0 } }))
#end
#if( !$authFilter.isEmpty() )
$util.qr($ctx.stash.put("authFilter", { "or": $authFilter }))
#end
we have the similar issue on our production environment after upgrading to 7.6.22
type BroadcastLiveData @auth( rules: [ { allow: groups, groupsField: "editors", operations: [update] } { allow: public, provider: iam, operations: [read] } { allow: private, operations: [read] } ] ) @model(subscriptions: { level: public }) { id: ID! communicationState: AWSJSON editors: [String] }
We are getting "Not Authorized to access updateBroadcastLiveData on type Mutation"
edit: it was fixed as soon as I changed: { allow: groups, groupsField: "editors", operations: [update] } to this: { allow: groups, groupsField: "editors" }
@Ilya93 this seems to be another issue. This issue here is about the specific case that you have the groupsField=id
In your case it's groupsField=editors However it seems to be a strange behavior as the update mutation should be part of the "update" operation. Have you checked that the group is set correctly in jwt token. What is unclear to me is how the BroadcastLiveData is created as there is no rule allowing "create".
I would suggest to open another issue.
@danielleadams are there any news when this issue will be addressed / fixed?
@danielleadams I don't think that you have managed to reproduce in your example earlier what I have reported. Modify your example to use "groupsField" and "groupClaim" and ensure your id token has a custom group array populated as I have shown in my report.
It is concerning this has still not been marked as a bug and there is no acknowledgement from the staff.
Until the issue is resolved the documentation here:
https://docs.amplify.aws/cli/graphql/authorization-rules/#how-it-works
should clarify that "groupsField" does not work "groupClaim".
Please refer to @endoodev1 said, he is reporting exactly the same issue stating where the problem is.
@hanna-becker @endoodev1 @spc16670 Thanks for the clarification. I think this is a duplicate of aws-amplify/amplify-category-api#132. A contributor has been working on this, so it is in the process of getting fixed.
Please let me know if this does not seem correct or if you have any other thoughts. Thank you.
@danielleadams unfortunately with aws-amplify/amplify-category-api#132 the current state is:
Straubulous moved this from In progress to Stalled in Bug bash
Is it possible you both somehow jointly have a look at this issue? 👀 Frankly in my opinion this really is a burning issue 🔥 for a lot of developers... Thank you in advance!
This is a burning issue - without this being fixed multi-tenancy in our app doesn't work. We are keeping everything on transformer V1 for now until the issue is resolved. This is a such a core feature of the framework I would imagine this kind of a bug should justify marking V2 as experimental - or the Amplify users will continue loosing confidence in the framework.
Since Amplify is a part of AWS products family I would expect higher standards than from a regular open source project - and much quicker bug acknowledgment and resolution.
So how can we push this issue up here @alharris-at @Straubulous (you moved issue #132 bug bash?). I completely described the bug 4 of https://github.com/aws-amplify/amplify-category-api/issues/132#issuecomment-1129353659 and also where it needs to be resolved. I'm too bad in writing good vtl code but for someone with some background knowledge this can be fixed in under 2h. As this issue is key for several multi tenancy use cases I hope that this will be fixed soon?!
Bumping this issue. Is the only current fix to downgrade?
Thanks for this comment, @endoodev1. With this information I was able to create a workaround for our use case until this issue is properly fixed. I added a new "group" property to the Project table that I duplicate the id value into. (This requires the Create query followed by and Update query, as the id is autogenerated, thus unknown at the time of creation.) At least this way we can get all our app's collab features working again and unblock development.
type Project @model @auth(rules: [ { allow: owner }, { allow: groups, groupsField: "group", operations: [read, update] } ]) { id: ID! group: String! # duplication of id for @auth group claim name: String! }Not sure group needs to necessarily be a required field, like I implemented it.
I was in your shoes a couple of weeks ago and luckily found this workaround to be somewhat acceptable for our use case so as to not having to go through the pain of rolling back. Though I wish I had never upgraded to v2 in the first place, because the workarond is obviously less than ideal because of the additional synching overhead and more surface area for partial failures.
I really hope this issue gets fixed, soon!
Adding onto this, I noticed that dynamic group auth with custom claims also doesn't work on a field with a custom index. There may already be a separate issue open for this, but I'm adding it here as it looks related. We're working around this in a similar way, by dupicating the project id into a non-indexed field:
type ChatMessage
@model
@auth(rules: [
{ allow: owner },
{ allow: groups, groupsField: "group", operations: [read] }
])
{
id: ID!
content: String!
projectID: ID! @index(name: "gsi-Project.chatMessages", sortKeyFields: ["createdAt"], queryField: "chatMessageByProject")
group: ID! # copy of projectID, as @auth doesn't work on @indexed field
}
We would like to see this working again:
type ChatMessage
@model
@auth(rules: [
{ allow: owner },
{ allow: groups, groupsField: "projectID", operations: [read] }
])
{
id: ID!
content: String!
projectID: ID! @index(name: "gsi-Project.chatMessages", sortKeyFields: ["createdAt"], queryField: "chatMessageByProject")
}
I am using version 9.2.1 of the Amplify CLI and also have this issue, but was able to workaround it by overriding the auth vtl file for the query (listReportingUnits) that I needed to work.
My schema:
type ReportingUnit
@model
@auth(rules: [
{allow: groups, groups: ["Administrators"], operations: [read]},
{allow: owner, ownerField: "createdBy", operations: [read]},
{allow: groups, groupsField: "organizationId", operations: [read]}
]) {
organizationId: ID! @primaryKey(sortKeyFields: ["reportingUnitId"])
reportingUnitId: ID!
label: String
description: String
# ...
createdBy: String
ControlReportingUnit: ReportingUnit @hasOne
}
type Organization
@model
@auth(rules: [
{allow: groups, groups: ["Administrators"], operations: [read]},
{allow: owner, ownerField: "ownedBy", operations: [read] },
{allow: groups, groupsField: "organizationId", operations: [read]}
]) {
organizationId: ID! @primaryKey
label: String
description: String
# ...
ownedBy: String
}
And in a new file at amplify/backend/api/myapi/resolvers/Query.listReportingUnits.auth.1.req.vtl I put the following vtl:
## [Start] Authorization Steps. **
$util.qr($ctx.stash.put("hasAuth", true))
#set( $isAuthorized = false )
#set( $primaryFieldMap = {} )
#if( $util.authType() == "User Pool Authorization" )
#if( !$isAuthorized )
#set( $staticGroupRoles = [{"claim":"cognito:groups","entity":"Administrators"}] )
#foreach( $groupRole in $staticGroupRoles )
#set( $groupsInToken = $util.defaultIfNull($ctx.identity.claims.get($groupRole.claim), []) )
#if( $groupsInToken.contains($groupRole.entity) )
#set( $isAuthorized = true )
#break
#end
#end
#end
#if( !$isAuthorized )
#set( $groupsInToken = $util.defaultIfNull($ctx.identity.claims.get("cognito:groups"), []) )
#if( $groupsInToken.contains($ctx.args.organizationId) )
#set( $isAuthorized = true )
#end
#end
#if( !$isAuthorized )
#set( $authFilter = [] )
#set( $ownerClaim0 = $util.defaultIfNull($ctx.identity.claims.get("sub"), "___xamznone____") )
#set( $currentClaim1 = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
#set( $ownerClaim0 = "$ownerClaim0::$currentClaim1" )
#if( $role0 != "___xamznone____" )
$util.qr($authFilter.add({"createdBy": { "eq": $ownerClaim0 }}))
#end
#set( $role0_0 = $util.defaultIfNull($ctx.identity.claims.get("sub"), "___xamznone____") )
#if( $role0_0 != "___xamznone____" )
$util.qr($authFilter.add({"createdBy": { "eq": $role0_0 }}))
#end
#set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
#if( $role0_1 != "___xamznone____" )
$util.qr($authFilter.add({"createdBy": { "eq": $role0_1 }}))
#end
#if( !$authFilter.isEmpty() )
$util.qr($ctx.stash.put("authFilter", { "or": $authFilter }))
#end
#end
#end
#if( !$isAuthorized && $util.isNull($ctx.stash.authFilter) )
$util.unauthorized()
#end
$util.toJson({"version":"2018-05-29","payload":{}})
## [End] Authorization Steps. **
Hope this helps!
In case it helps, I've attached the auto-generated vtl file that I'm overriding: Query.listReportingUnits.auth.1.req.vtl.txt
Edit: I also had a related item that needed to be similarly adjusted, but instead of $ctx.args.organizationId, I had to use $ctx.source.reportingUnitControlReportingUnitId (which, confusingly, is the organizationId of the related entry, since that is the partition key) in ReportingUnit.ControlReportingUnit.auth.1.req.vtl
It has been 8 months since the issue was raised and there is still no official advice from Amplify. The community confirmed the issue and identified bugs in the vtl templates.
The Amplify docs have not been updated to mention the issue as a known limitation: https://docs.amplify.aws/cli/graphql/authorization-rules/#how-it-works
Is this going to be fixed? Is the official advice never to upgrade and stay on v1? Should there be and AWS support ticket raised for this instead of just a Github issue?
I am dealing with this bug by self-made walkaroud. The multy-tenancy is critical for serious projects.
@danielleadams Sorry to bump, but any news? It's been months and multiple issues have been created for this problem. Imho this is pretty high prio.
The answer is to ditch Amplify and start building out a solution that will offer solutions.
Learning a framework like Sveltekit or Next is not that difficult and will handle much of the backend / frontend communication.
Mongodb's realm service will handle multi-tenants architecture.
https://www.mongodb.com/community/forums/t/multi-tenant-application-using-node-js-mongo-db/154534
On Thu, Nov 17, 2022 at 4:32 AM jbreemhaar @.***> wrote:
@danielleadams https://github.com/danielleadams Sorry to bump, but any news? It's been months and multiple issues have been created for this problem. Imho this is pretty high prio.
— Reply to this email directly, view it on GitHub https://github.com/aws-amplify/amplify-category-api/issues/110#issuecomment-1318343132, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKJQCJZ6IFFXZMH3MUNVSCLWIX3RHANCNFSM5WGKFIZA . You are receiving this because you commented.Message ID: @.***>
Hi, forks,
I had the same problem with groups strategy.
Generated vtl resolver:
#if( !$isAuthorized )
#set( $authFilter = [] )
#foreach( $group in $util.defaultIfNull($ctx.identity.claims.get("Your-groupClaim-field"), []) )
#if( !$group.isEmpty() )
$util.qr($authFilter.add({"Your-groupsField-field": { "contains": $group }}))
#end
#end
#if( !$authFilter.isEmpty() )
$util.qr($ctx.stash.put("authFilter", { "or": $authFilter }))
#end
#end
I changed the generated Vtl resolver to handle groupClaim with parseJson,
#if( !$isAuthorized )
#set( $authFilter = [] )
#set( $groupClaim0 = $util.defaultIfNull($ctx.identity.claims.get("Your-groupClaim-field"), []) )
#if( $util.isString($groupClaim0) )
#if( $util.isList($util.parseJson($groupClaim0)) )
#set( $groupClaim0 = $util.parseJson($groupClaim0) )
#else
#set( $groupClaim0 = [$groupClaim0] )
#end
#end
#foreach( $group in $groupClaim0 )
#if( !$group.isEmpty() )
$util.qr($authFilter.add({"Your-groupsField-field": { "contains": $group }}))
#end
#end
#if( !$authFilter.isEmpty() )
$util.qr($ctx.stash.put("authFilter", { "or": $authFilter }))
#end
#end
This workaround worked for me.
I guess this code below does the transformation, but I'm not sure... https://github.com/aws-amplify/amplify-category-api/blob/main/packages/amplify-graphql-auth-transformer/src/resolvers/query.ts#L462
I hope this helps you all.
Hi, forks, ... I hope this helps you all.
@kulikala thank you for sharing. But how did you handle the necessity of adding this your VTL code into every generated auth vtl?