fix: add granular read ops enum

[AaronZyLee]

Issue #

aws-amplify/amplify-flutter#2526
#3038

Description

Add granular read operation enums in model auth operation rule
Refer https://docs.amplify.aws/cli/graphql/authorization-rules/#how-it-works

The purpose of adding enum in library is to ensure the model files generated by modelgen will be compiled. However, this is not intended for Datastore usage but for those who use modelgen files along with API plugin. It is not recommended to use them in the datastore due to the prerequisite of sync and listen. This is documented in the callout

General Checklist

• Added new tests to cover change, if needed
• Build succeeds with all target using Swift Package Manager
• All unit tests pass
• All integration tests pass
• Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
• Documentation update for the change if required
• PR title conforms to conventional commit style
• New or updated tests include Given When Then inline code documentation and are named accordingly testThing_condition_expectation()
• If breaking change, documentation/changelog update with migration instructions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[phani-srikar]

Are there any existing tests associated with these additional operations?

[5d]

Are there any existing tests associated with these additional operations?

@phani-srikar ; No, we don't have test cases cover these additional operations.

@AaronZyLee ; Have you performed manual testing to ensure that these additional operation types are functioning as expected?

[lawmicha]

Can we start off by attaching the generated Model swift types that utilizes these new granular read access cases? By doing so, we can update the source schema that we use to reference the generated types.

Is there an sort of relevant API calls that we can write, either in the unit or integration tests, utilizing these models, and asserting the expected behavior?

[AaronZyLee]

The example schema using it is

type Todo @model
@auth(rules:[
  { allow: public, operations: [get, list, listen, sync, search] },
  { allow: owner }
])
{
  id: ID!
  name: String
}

The model files can be generated by the current version of modelgen, which has the following line generated

    model.authRules = [
      rule(allow: .public, operations: [.get, .list, .listen, .sync, .search]),
      rule(allow: .owner, ownerField: "owner", identityClaim: "cognito:username", provider: .userPools, operations: [.create, .update, .delete, .read])
    ]

[lawmicha]

In DataStore, we get the authtypes based on .read. Since developers can use this schema with DataStore as long as they allow listen and sync. then this code probably needs to be updated to getting auth types for .read, sync or listen. ?

var authTypes = await authModeStrategy.authTypesFor(schema: modelSchema, operation: .read)

[lawmicha]

I bet a lot of logic is determined by checking if the operation is .read and does something. Now that we allow compiling the more granular "read" types, that means all logic based on "read" should now be updated to

1. If the previous check for read expected all granular read access then update the code to "if operation == read || operations.containsAll(list, sync, listen, search)"
2. If the previous check for read expected just the sync API (for example DataStore's sync operation), then code can be updated to "if operation == read || operation == sync)"
3. If previous check for read expected that the subscription API is used then --> "operation == read or operation == "listen"

[AaronZyLee]

In DataStore, we get the authtypes based on .read. Since developers can use this schema with DataStore as long as they allow listen and sync. then this code probably needs to be updated to getting auth types for .read, sync or listen. ?

var authTypes = await authModeStrategy.authTypesFor(schema: modelSchema, operation: .read)

The datastore usage is out of scope of this PR as mentioned in the description. This is for the API users to get the modelgen files compiled

[lawmicha]

In DataStore, we get the authtypes based on .read. Since developers can use this schema with DataStore as long as they allow listen and sync. then this code probably needs to be updated to getting auth types for .read, sync or listen. ?

var authTypes = await authModeStrategy.authTypesFor(schema: modelSchema, operation: .read)

The datastore usage is out of scope of this PR as mentioned in the description. This is for the API users to get the modelgen files compiled

Customers can use DataStore, and if they have more granular access including listen and sync, then their models may not work as expected because DataStore's sync code checks for "read" access, not "read" or "sync" access. By allowing them to compile the App with the generated models, will cause other unintended behaviors when using DataStore at runtime

[lawmicha]

Next steps, this comment #2720 (comment) captures some the concern over introducing more granular operations that was "sub-operations" of read. Logic that checks for "read" should consider checking for "read" OR one or more of the granular operations

[marcoboerner]

Is there any update on this? I'm using Amplify Gen2 as well and don't get any errors when adding get to my schema, the backend deploys just fine, but then the code generation fails / produces unknown enum cases.