aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
89 stars 77 forks source link

Sample Data Access Pattern - Simple strategy or VTL Mod necessary? #2424

Open charlieforward9 opened 6 months ago

charlieforward9 commented 6 months ago

Amplify CLI Version

12.10.1

Question

originally posted in Amplify Discord help channel

Most of my data models conform to this auth strategy

# Access to everything
{ allow: groups, groups: ["RootAdmin"] } 
# Authenticated users can see what they own
{ allow: owner, ownerField: "owners", operations: [create, update, read] } 
# Custom access to sample data when not authenticated
{ allow: custom, provider: function, operations: [read] }  

I need Authenticated users to see the sample data. All the sample data has owners: "ALL" attached. The custom Lambda authorizer checks for this for unauthenticated users. However, with userPool authentication, I cannot run that through the custom lambda. Instead I have to modify the VTL resolver. (I think?)

With this strategy, I would need to modify ALL of my auth resolvers to allow access to sample private data by initializing the authFilter array with {contains: "ALL"}, in order to search owners for "ALL" when using userPools auth strategy.

What is the best way to modify all of my (45) auth VTL files? Or, is there a better approach?

mtliendo commented 6 months ago

Hey @charlieforward9, typically I'd model this kind of access pattern as such:

# Access to everything
{ allow: groups, groups: ["RootAdmin"] } 
# Authenticated users can see what they own
{ allow: private,  operations: [create, update, read] } 
# Custom access to sample data when not authenticated
{ allow: public, provider: iam, operations: [read] }  

This all uses cognito to handle authentication. Just curious how this model works compared to what you have. Happy to discuss if your example is for sure what you need.

ykethan commented 6 months ago

Hey, 👋 thanks for raising this! I'm going to transfer this over to our API repository for better assistance 🙂. @mtliendo thank you for chiming in on this issue.

charlieforward9 commented 6 months ago

Hey @mtliendo , thank you for getting back to me on this.

This strategy won't work for my use case, because only the samples should be viewed by everyone, whereas this appears to show make all entries readable.

Ideally, I would like to avoid modifying the VTL, so if there's a better option that would take less modifications, I'm all in.

AaronZyLee commented 6 months ago

@charlieforward9 Have you considered using the dynamic(or static) group auth in your case? Looks like you assign owners: 'ALL' to all the sample data, in which case I think group is more suitable rather than owner.

On the other hand, you can try to override the resolver by following the document here. In your use case, you can find the built resolvers in amplify/backend/api/${api_name}/build/resolvers. Then just find those with .auth in the file name and add the override file in amplify/backend/api/${api_name}/resolvers.

In addition, if you consider using the automation strategy, you can also refer to the command hooks in Amplify CLI. In your use case, you will need to add a post command hook for amplify api gql-compile to modify the generated resolvers.

AnilMaktala commented 5 months ago

Hey 👋 , This issue is being closed due to inactivity. If you are still experiencing the same problem and need further assistance, please feel free to leave a comment. This will enable us to reopen the issue and provide you with the necessary support.

github-actions[bot] commented 5 months ago

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.

charlieforward9 commented 4 months ago

@charlieforward9 Have you considered using the dynamic(or static) group auth in your case? Looks like you assign owners: 'ALL' to all the sample data, in which case I think group is more suitable rather than owner.

@AaronZyLee Thank you for your feedback. Finally able to get back to this issue after a while of working on other features. Can it please be re-opened until I understand the best way selectively access entries within a model?

I need help understanding how I should enable ALL users to access only the SAMPLE data within the schema, which is stored alongside private data.

For example, we have an AccountProperty model, that can have SAMPLE and private entries. Sample entries should be accessible by everybody (Unauthorized/Authorized/AllGroups/etc). Private entries should only be accessed by its owner and RootAdmins.

The most brute force solution seems to be editing the VTL files, but I would prefer to find a more elegant solution if possible.

I will open a new issue if necessary, but this could also just be re-opened.

charlieforward9 commented 4 months ago

@AnilMaktala please reopen this issue

charlieforward9 commented 4 months ago

@mtliendo Id like to get a clear understanding on how I can enable this public access to records without an owner while keeping the records with owners safe. It seems like these docs imply that all users would be able to read all the records in this model...?

typically I'd model this kind of access pattern as such:

# Access to everything
{ allow: groups, groups: ["RootAdmin"] } 
# Authenticated users can see what they own
{ allow: private,  operations: [create, update, read] } 
# Custom access to sample data when not authenticated
{ allow: public, provider: iam, operations: [read] }  

This all uses cognito to handle authentication. Just curious how this model works compared to what you have. Happy to discuss if your example is for sure what you need.

charlieforward9 commented 4 months ago

Thank you for reopening. I am spending the rest of the day researching this. I would really appreciate some feedback to accelerate this development. I am happy to provide more details if necessary.

@mtliendo @AaronZyLee

charlieforward9 commented 4 months ago

After much thought, I was able to "work around" this by making two generateClient's when a user is not an admin but authenticated, one for the userPool auth and another for the lambda auth. I then had to make a combineResponses method.

Definitely not the most efficient method for DB access, but it works for now... I would like to find a better solution long term.

AnilMaktala commented 3 months ago

Hi @charlieforward9, Another workaround is, you can try creating the one custom query for public access and remove the public auth configuration from the model auth rules. Let us know if this workaround resolves your issue.

charlieforward9 commented 3 months ago

@AnilMaktala This does not work because I need users with private credentials to be able to access the public data. If they are signed in, then it will not run the custom function, as far as i am aware.

phani-srikar commented 3 months ago

Hi @charlieforward9, thanks for explaining your use-case. As per my understanding of your comment,

For example, we have an AccountProperty model, that can have SAMPLE and private entries. Sample entries should be accessible by everybody (Unauthorized/Authorized/AllGroups/etc). Private entries should only be accessed by its owner and RootAdmins.

You would like to extend the behavior of owner Auth such that if the value of owner field is a custom pre-defined value like "ALL" in your case, the request should be authorized for any cognito user, signed-in or not. The non signed-in or public Auth uses IAM mode which does not support "owner" like dynamic data based authorization.

After much thought, I was able to "work around" this by making two generateClient's when a user is not an admin but authenticated, one for the userPool auth and another for the lambda auth.

I'm glad that using the custom Lambda Auth filled in the gap for you temporarily. I do not have a better alternative and am marking this as a feature request.

charlieforward9 commented 3 months ago

Thanks for your input, @phani-srikar . With almost 500 feature requests in this repo alone, seems like it will be some time before this comes about. Is functionality like this on the timeline in any shape or form down the road? With a workaround available, I am good on my end, simply asking out of curiosity.