amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

DataStore does not update data when @Auth directive used with multiple owners

Open danrivett opened this issue 3 years ago • 18 comments

Summary

At the 11th hour in our app development we have just discovered that after following the official AWS documentation use cases of adding @Auth directives on my GraphQL schema in order to allow for read-only access to a selection of users, and also built it using DataStore, which is promoted as best practice for mobile app development, the two are not compatible with each other.

Example

Here's a simplified view of the schema and the use case:

type WorkerTimesheet
  @model
  @auth(
    rules: [
      { allow: owner, ownerField: "workerCognitoUserId" }
      { allow: owner, ownerField: "approverCognitoUserIds", operations: [read] }
    ]
  ) {
  id: ID!
  workerCognitoUserId: ID!
  approverCognitoUserIds: [ID!]
  ...
}

Details

In a pure GraphQL subscription, taking DataStore out of the equation, I can only get a subscription to fire when a WorkerTimesheet is updated, if I subscribe with a matching workerCognitoUserId field as follows:

subscription onWorkerTimesheetUpdated {
  onUpdateWorkerTimesheet(workerCognitoUserId: "owner-cognito-user-id") {
    id
    workerCognitoUserId
    approverCognitoUserIds
  }
}

That fires when the workerCognitoUserId parameter matches the owner. But how to inform the approvers?

Well this is the GraphQL subscription automatically generated by Amplify:

onUpdateWorkerTimesheet(workerCognitoUserId: String, approverCognitoUserIds: String): WorkerTimesheet @aws_subscribe(mutations: ["updateWorkerTimesheet"]) @aws_iam @aws_cognito_user_pools

So perhaps I can specify approverCognitoUserIds parameter - it's a String so that's promising:

subscription onWorkerTimesheetUpdated {
  onUpdateWorkerTimesheet(approverCognitoUserIds: "approver-cognito-user-id") {
    id
    workerCognitoUserId
    approverCognitoUserIds
  }
}

This doesn't work, apparently due to quite a large limitation in how AppSync subscriptions work with array fields that I've since found in my increasingly desperate research has been around for many years. This begs the question how @Auth directives using multiple owners were ever anticpated would work, since subscriptions are pretty fundamental to AppSync and GraphQL, and absolutely critical for DataStore.

What is confusing to me is why the generated subscription has a approverCognitoUserIds: String field in the first place if it's never possible to satisfy it given the targeted field is an array. Must be a bug, but this functionality has been available in Amplify for years, without exageration.

This is made much worse by the fact that turning on Amplify debug logging shows that DataStore is using that field for its subscription:

[DEBUG] 47:34.112 AWSAppSyncRealTimeProvider - subscription ready for
 {"query":"subscription operation($approverCognitoUserIds: String!) {
onUpdateWorkerTimesheet(approverCognitoUserIds: $approverCognitoUserIds) ...

And so even for WorkerTimesheets that are created with a matching workerCognitoUserId, DataStore never gets any updates for that type.

The only way it shows up is if we force it through a DataStore.stop() followed by a DataStore.start() and then it pulls the data for the matching workerCognitoUserId as well as for any user logged in that appears in the approverCognitoUserIds list. But that's because of the initial load of the data rather than through any subscription.

If this is all correct it basically makes using the @Auth directive with a list of authorized users incompatible with using DataStore and yet I never read about any such limitation. We are at the 11th hour in our app development and it looks like we have a fundamental design change required. This is not something to be treated lightly since it's going to affect both frontend and backend teams who have integrated around this design.

I don't understand how this sort design limitation has been tolerated for so long and viewed as an acceptable user experience by the Amplify + AppSync teams. At the very least there should be clear documentation about these limitations to avoid wasting dozens of hours on implementations that won't work. But even that wouldn't really be good enough for something like this. If DataStore is not compatible with @Auth directives like this, it should fail fast, perhaps on DataStore.start() and not just silently never update its DataStore. Honestly, it makes a mockery of the first-class experience Amplify and DataStore is intending to provide.

This report may perhaps sound a bit overblown, but we have kept running into fairly serious impediments as we've wholeheartedly attempted to embrace both Amplify and AppSync in our latest application. I have refused to believe that a roll-your-own, undifferentiated heavy-lifting approach is better, and actively promoted using higher-level services such as AppSync and libraries such as DataStore, but after several years of them being available and these sorts of issues largely ignored/overlooked/deprioritized, I am struggling to reconcile my beliefs with my experiences unfortunately.

References to other related issues:

  • https://github.com/aws-amplify/amplify-js/issues/7069
  • https://github.com/aws-amplify/amplify-cli/issues/4794

This scenario I'm documenting here is far from an edge case.

References of other issues we encountered in the very recent past:

  • aws-amplify/amplify-category-api#218
  • https://github.com/aws-amplify/amplify-cli/issues/6352

danrivett avatar Mar 25 '21 02:03 danrivett

Hi Dan, thank you for opening this issue and providing your feedback. I understand the frustration you’re experiencing. The CLI docs you’re linking in the issue are for the API category specifically. Not all of those Auth rules will be fully compatible with DataStore. The rules listed in the DataStore docs are the ones we currently support https://docs.amplify.aws/lib/datastore/setup-auth-rules/q/platform/js.

We’re actively working on bringing DataStore functionality to parity with the API category and making all of the rules from the CLI docs available, however, some of this work requires new service-level (i.e., AppSync) functionality.

I think we can certainly do a better job with being more explicit in our documentation and we will be adding a Frequently Asked Questions section that helps guide our customers with what use cases can and cannot be implemented with DataStore currently.

iartemiev avatar Mar 26 '21 15:03 iartemiev

@iartemiev Thanks for your quick reply and explanation, apologies for my delayed response. It wasn't due to a lack of importance or attention on my part, it was actually the complete opposite as we scrambled as a team to rearchitect and implement a different solution compatible with the current DataStore limitations. We've now managed to implement that end to end, and test that and fortunately it appears to be working well.

I'll explain the solution we went with for other people's possible benefit in a minute, but I first wanted to reiterate why I think this current limitation is so serious as it is indicative of a larger challenge that I think is eminently solvable given appropriate focus.


The challenge I'm talking about is the paradox of what Amplify, and DataStore in particular is designed to address - which is to make it as simple as possible to implement full-stack applications without needing to worry about high-ceremony and creating lots of boilerplate or undifferentiated code. This is a great goal, but what I have found using the tools for a couple of years is while for 70% of the use cases it delivers, the final 30% are very painful to overcome and the effort and workarounds normally required to address the 30% of use cases as the app grows in functionality has a very high likelihood of erasing a large part of the previous productivity gains.

This is especially true when the limitations are not clearly known and called out - both in documentation, but even better at compile or runtime - because for this ticket I really do not think we made a wrong assumption about DataStore working with this @Auth scenario, and if it isn't supported, then it should fail fast and notify the developer before they spend a lot of time going down the wrong path.

In the end we had to write a significant amount of code on the backend - certainly undifferentiated code - at very short notice in order to work around the limitation, and so I would strongly encourage you to prioritize this use case and other similar ones as I think it will make a world of difference in terms of developer productivity and enjoyment.

I would go one further and say field-level auth restrictions (aws-amplify/amplify-category-api#218) is woefully inadequate, and while a lot of the blame can be laid at AppSync's door here, AppSync has been out a number of years now and so really should be addressing these types of problems, and Amplify should be leading the charge strongly to get these enhancements delivered there and then subsequently in Amplify.

I may be preaching to the choir here, or perhaps an echo chamber, but I really do feel moving in this direction would take Amplify to a whole new level, and certainly make this developer very happy.

Thank you for reading, I will summarise the solution we implemented for this particular problem in the next comment.

danrivett avatar Apr 10 '21 01:04 danrivett

The solution we implemented is a standard replication pattern where for our Timesheet scenario a worker would create a timesheet and own it themselves, and then a backend lambda would subscribe to the DynamoDB stream determine a Timesheet has been created and then identify the appropriate approvers and then add duplicate/replica records for each approver so that each record has one owner: a master record owned by a worker and 1 or more replica records owned by each approver.

Sounds great, but the amount of backend logic required to handle controlled 2 way replication of updates was not trivial. Especially because we want to avoid feedback loops, but still have an update made in one replica record by an approver update all the other replica records (as well as master) for each approver.

And we haven't even tackled the complexity of optimistic locking to handle scenarios that 2 approvers edit their own replicas at the same time. You see, when the burden is put on the application developer to solve these problems, it quickly sucks so much of the productivity gains from using the tooling initially.


We use an Event Driven Architecture on the backend so that simplified being notified of when a Timesheet was created/updated/deleted (but that's a whole another level of undifferentiated code we had to write to achieve that - aws-amplify/amplify-category-api#829).

So the irony of all this is in order to use Amplify, and DataStore so that we simplify frontend programming patterns (which it largely has) we've had to create a lot more complexity on the backend in order to architect the backend in a scalable, flexible and maintainable fashion in its own right and not just create a big ball of mud there.

I would love our team to be able to avoid implementing so much undifferentiated code on the backend especially, and I think it would be very achievable by the AWS teams if that was a serious goal. There's lots of talks about EDA and its advantages, but I see little evidence of that from AWS' offerings (e.g. why does EventBridge talk about 500ms latencies for event delivery, why doesn't it support ordered event delivery, why isn't there a first class event store service for supporting event sourced architectures etc)

Anyway massive tangent, but needless to say this issue we ran into on this ticket is but one of many issues we've had to solve with undifferentiated solutions that I would seriously love AWS to solve for us as you guys are way more capable of doing so than us.

danrivett avatar Apr 10 '21 02:04 danrivett

Huge +1, this has been a show-stopper for my app, too.

laurenzfg avatar Jun 03 '21 21:06 laurenzfg

+1, we also need this in our upcoming application. Would be nice if this gets a prio bump.

evertson90 avatar Jul 25 '21 18:07 evertson90

+1 , would love to see this feature being implemented.

jdcas89 avatar Jul 25 '21 18:07 jdcas89

I managed to do it with Amplify API (AppSync) instead of DataStore. You have to use @model(subscriptions: { level: public }) and work around the fact that everyone receives all updates for that model.

fr-an-k avatar Jul 25 '21 19:07 fr-an-k

Basically DataStore should detect public subscriptions. An example can then be documented for owner arrays on which RBAC can be implemented. Optionally a separate @model(subscriptions: null) for private data queried after notification.

fr-an-k avatar Jul 25 '21 19:07 fr-an-k

Then the next step is to allow dynamic groups @auth (Cognito group id array) by extending the generated vtl and graphql queries.

fr-an-k avatar Jul 25 '21 20:07 fr-an-k

@laurenzfg @evertson90 @jdcas89 - this feature is prioritized (please see https://github.com/aws-amplify/amplify-js/issues/7534#issuecomment-847366391).

@frank-unovica - DataStore supports public subscriptions as well when those are configured via level: public. Were you running into issues with them? If so, please open a new issue that details the unexpected behavior and include your schema.graphql, package.json/envinfo output, and any relevant app code.

iartemiev avatar Jul 26 '21 13:07 iartemiev

@iartemiev I was using Datastore with the owner arrays feature from AppSync (unsupported in DataStore), but the subscriptions were not receiving server updates or errors due to some subscription query issue, so I switched to "raw" Amplify API.graphql.

But if public subscriptions make it work, this DataStore use case should be possible, so I suggest adding an example to the documentation to resolve this issue. All notifications are public, but write access is protected. Data can be private through a separate @model(subscriptions: null)

fr-an-k avatar Jul 27 '21 08:07 fr-an-k

@frank-unovica @model(subscriptions: null) will not work with DataStore at the moment, and just having subscriptions: {level: public} is not an option for most developers that are using e.g., owner or group authorization in their app due to the effect public subscriptions will have on data privacy. If it makes sense for your use case - great! But I would not feel comfortable suggesting that as a viable workaround for most customers.

iartemiev avatar Jul 27 '21 12:07 iartemiev

Hi @iartemiev

Exactly same here, we want to allow CRUD operations authorised based on dynamic and static groups to enable collaboration among invited members to a group and admins Our approach for now is to add the below to the shareable models, then at start-up fetch which data the user should have access to (if online otherwise bummer) and then set syncExpressions on each sharable model

schema.graphql

///

@model
  @auth(
    rules: [
      { allow: private }
    ]
  )

///

After hours trying to solve the privacy component here which is a huge concern, we came to your very same conclusion on the subscription For the aws team, quite frankly in this thread we have been talking of a very common use case and we would really appreciate some assistance. At this point in time, for my team use case, sadly, Datastore is not production ready

Thanks!

Gab1988 avatar Sep 28 '21 23:09 Gab1988

Same for me. In my case, I've detected that DataStore successfully subscribe to changes if the current user belongs to the "last" auth rule (so I had to swap mine due to priorities. Let me explain. This is my model:

type Appointment
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["Admins"] }
      { allow: owner, ownerField: "practitionerId", operations: [read] }
      { allow: owner, ownerField: "userId", operations: [read] }
    ]
  ) {
  id: ID!
  datetime: AWSDateTime!
  status: AppointmentStatus!
  userId: ID!
  user: User @belongsTo(fields: ["userId"])
  practitionerId: ID!
  practitioner: Practitioner! @belongsTo(fields: ["practitionerId"])
}

In this case, my user "practitioner" received live updates from DataStore subscription, but not my user "user".

Since in my app, users are the ones making the appointments (and therefore I want to give them live feedback on screen), I changes the last four lines as:

  practitionerId: ID!
  practitioner: Practitioner! @belongsTo(fields: ["practitionerId"])
  userId: ID!
  user: User @belongsTo(fields: ["userId"])

So my user "user" is the one receiving live updates. For now, the user "practitioner" will have to "refresh" the page, but tbh I don't expect them to be online waiting for appointments so I can live with this. Next time they visit there'll be a full sync and they will get the data anyway.

Maybe this workaround helps someone.

menendezjaume avatar Apr 29 '22 16:04 menendezjaume

+1 this issue, it's holding back a bunch of functionality in our app

bobalucard avatar Jul 05 '22 02:07 bobalucard

Check out AWS official docs for the the priority of multiple auth config.

https://docs.amplify.aws/lib/datastore/setup-auth-rules/q/platform/flutter/#configure-multiple-authorization-types

ErnestDaDev avatar Jul 25 '22 07:07 ErnestDaDev

Check out AWS official docs for the the priority of multiple auth config.

https://docs.amplify.aws/lib/datastore/setup-auth-rules/q/platform/flutter/#configure-multiple-authorization-types

Thanks for link but not relevant to what the issue is. Issue is with using multiple owners or owner fields.

bobalucard avatar Jul 25 '22 10:07 bobalucard

Check out AWS official docs for the the priority of multiple auth config. https://docs.amplify.aws/lib/datastore/setup-auth-rules/q/platform/flutter/#configure-multiple-authorization-types

Thanks for link but not relevant to what the issue is. Issue is with using multiple owners or owner fields.

Yes that's true, it is a link supporting the comment by @menendezjaume https://github.com/aws-amplify/amplify-js/issues/7989#issuecomment-1113523688

ErnestDaDev avatar Jul 26 '22 11:07 ErnestDaDev

Hi sorry for not getting back to this issue earlier but this issue has now been closed. It works for both GraphQL API category + DataStore. Here is our launch announcement https://aws.amazon.com/blogs/mobile/build-real-time-multi-user-experiences-using-graphql-on-aws-amplify/

Feel free to @-mention me if there's more on this particular issue that I've missed. Thank you all for continuously bringing up use cases that helped us to shape the solution.

renebrandel avatar Jun 28 '23 15:06 renebrandel

@renebrandel

When we have multiple owners on a model, DataStore subscription don't works.

DataStore still fails to have real time subscription working.

Sync works fine. That is not enough, we are forced to induce sync by restarting DataStore.

our amplify version: aws-amplify@^5.3.7

Example:

Following is our model, conversationMembers are owner array in which we have producer and consumer of the message.

type ChatMessage @model @auth(rules: [ {allow: owner, ownerField: "conversationMembers"}, {allow: groups, groups: ["admin"]}, {allow: public, provider: iam}]) { id: ID! content: String conversationMembers: [String] }

rcbose avatar Aug 18 '23 21:08 rcbose