amplify-swift
amplify-swift copied to clipboard
fix: add granular read ops enum
Issue #
https://github.com/aws-amplify/amplify-flutter/issues/2526 https://github.com/aws-amplify/amplify-swift/issues/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 accordinglytestThing_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.
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?
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)
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
orlisten
. ?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
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
orlisten
. ?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
Next steps, this comment https://github.com/aws-amplify/amplify-swift/pull/2720#discussion_r1313212958 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