GraphQL auth transformer: support dynamic groups on list entity field

[alex-vladut]

Description of changes

This is the next work to support parsing dynamic groups claim into a list of groups and with these changes those will be compared against an entity field of list type e.g.:

  type Post @model @auth(rules: [{ allow: groups, groupsField: "groups" }]) {
      id: ID!
      title: String!
      groups: [String!]
      createdAt: String
      updatedAt: String
  }

And the generated authorization logic will look something like this:

#if( $util.authType() == "User Pool Authorization" )
  #if( !$isAuthorized )
    #set( $authFilter = [] )
    #set( $groups0 = $util.defaultIfNull($ctx.identity.claims.get("cognito:groups"), []) )
    #if( $util.isString($groups0) )
      #if( $util.isList($util.parseJson($groups0)) )
        #set( $groups0 = $util.parseJson($groups0) )
      #else
        #set( $groups0 = [$groups0] )
      #end
    #end
    #foreach( $group in $groups0 )
      #if( !$group.isEmpty() )
        $util.qr($authFilter.add({"groups": { "contains": $group }}))
      #end
    #end
    #if( !$authFilter.isEmpty() )
      $util.qr($ctx.stash.put("authFilter", { "or": $authFilter }))
    #end
  #end
#end

I added as well a refactoring commit to separate the logic for adding authorization checks for owner strategy from the groups one. This change will be useful in my next PRs as the authorization checks are slightly different.

Issue #, if available

Previous PR with support for entity field of type String: https://github.com/aws-amplify/amplify-cli/pull/9466 Related issue ticket: aws-amplify/amplify-category-api#132

Description of how you validated changes

Added some unit tests and also manually tested it on a project by copying the built files.

Checklist

• [X] PR description included
• [X] yarn test passes
• [X] Tests are changed or added

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

[codecov-commenter]

Codecov Report

Merging #9535 (815ab95) into master (2b555b6) will decrease coverage by 0.00%. The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master    aws-amplify/amplify-cli#9535      +/-   ##
==========================================
- Coverage   52.76%   52.75%   -0.01%     
==========================================
  Files         821      821              
  Lines       45576    45587      +11     
  Branches     9731     9734       +3     
==========================================
+ Hits        24049    24051       +2     
- Misses      19531    19536       +5     
- Partials     1996     2000       +4     

Impacted Files	Coverage Δ
...fy-graphql-auth-transformer/src/resolvers/query.ts	66.37% <42.85%> (-5.20%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2b555b6...815ab95. Read the comment docs.

[n0rb]

@alex-vladut I am not 100% certain but I think this new PR does not handle the case either when the groupsField is set to a primary field, like id, ie

      {
        allow: groups
        groupsField: "id"
        groupClaim: "custom:fleets"
        operations: [read]
      }

the routes.ts file seems to handle this separately starting at line 108

/**
 * In the event that an owner/group field is the same as a primary field we can validate against the args if provided
 * if the field is not in the args we include it in the KeyConditionExpression which is formed as a part of the query
 * when it is formed as a part of the query we can consider the request authorized
 */
const generateAuthOnModelQueryExpression = (

I could probably fix it, but I dont want to mess into it while you're refactoring the transformer

[alex-vladut]

Hey @n0rb thank you so much for taking the time and reviewing this PR. This is expected, I wanted to only introduce a minimal set of changes in this PR and I already have some changes prepared for new PRs to address those cases. The refactoring I made here was to split the logic based on the strategy being owner or groups in order to the next PRs to be easier to reason about as my changes are going to be done for groups only, while the owner auth I am expecting will remain the same. Let me know if you think this approach makes sense or you would have a different suggestion. Thanks 👍

[alex-vladut]

Hey Amplify team, any chance someone could review my PR here please? Thanks

cc @danielleadams @johnpc not sure what is the process inside the team for picking these up, just tagging you as you were the ones reviewing my previous PR in that area :)

[danielleadams]

@alex-vladut I didn't get a chance to look at this this week, but I will next week. Thanks for the PR 👍🏼

[danielleadams]

@alex-vladut apologies - I didn't get around to this this week, but it's first on my TODO list for Monday.

[alex-vladut]

Sounds good, thanks for the update 👍

[alex-vladut]

Sounds great @Booligoosh, thank you for reviewing this PR. My initial intention was indeed to address those issues in a different PR, but as this one is already a month old it could make sense to add those changes here so that will hopefully fix the issue faster. Thanks, will merge your PR into this branch and let's see how it goes

[daniellorenzin]

Any update on this? We are patching the CLI just to get the graphql working within development. @danielleadams

[danielleadams]

Hi @alex-vladut, thanks for your work on this pull request! We are currently making changes to the GQL auth directive that will stall approving and merging open pull requests from contributors. For the time being, I would recommend holding off on any changes which may need to be reworked later once we complete more foundational changes to the transformer. Apologies for the delay and inconvenience this causes - we will be happy to circle back and work on getting this PR in once that work is completed.

[undefobj]

Reopening so that we can keep tracking the customer requested functionality as we continue infrastructure work mentioned by @danielleadams

[danielleadams]

@alex-vladut I know it's been awhile - but we finally got the code sorted out. The GQL transformers have moved to https://github.com/aws-amplify/amplify-category-api. If you are open to it, we'd love to have your @auth contributions over there. Thanks again for investigating this.

[alex-vladut]

Great news! I'll see if I can find some time over the next couple of weeks to look into it again and migrate the changes to the new repo 👍

[sdstolworthy]

Closing as https://github.com/aws-amplify/amplify-category-api is the repository where this will be tracked.