Update mutation - Owner authorisation not working

[dylan-westbury]

Before opening, please confirm:

• [X] I have installed the latest version of the Amplify CLI (see above), and confirmed that the issue still persists.
• [X] I have searched for duplicate or closed issues.
• [X] I have read the guide for submitting bug reports.
• [X] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
• [X] I have removed any sensitive information from my code snippets and submission.

How did you install the Amplify CLI?

sudo npm install -g @aws-amplify/cli --unsafe-perm=true

If applicable, what version of Node.js are you using?

v16.13.1

Amplify CLI Version

9.1.0

What operating system are you using?

Mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes made

Amplify Categories

auth, storage

Amplify Commands

Not applicable

Describe the bug

Given models in appsync schema, the owner authorisation is not working for update mutation with cognito authorisation, in the code and in the AWS AppSync console. When the user is or isn't assigned to a cognito group.

Expected behavior

Owner of record should be able to update their record

Reproduction steps

1. Create model in AppSync schema (e.g. User)

2. Create user Dynamodb record with Lambda post confirmation workflow (that calls another lambda based on cyclic dependency issue) including with the owner field and adds user to a group called "Users"

3. Log in as the cognito user and attempt to update the record - unauthorised error shows

4. Remove "Users" group from the cognito, log out and in and attempt to update the record - unauthorised error also shows

GraphQL schema(s)

```graphql # **************************************************************************** # USER # **************************************************************************** type User @model(subscriptions: null, mutations: { create: null, update: "updateUser", delete: null }) @auth(rules: [ { allow: groups, groups: ["SuperAdmins"], operations: [read, update] } { allow: groups, groups: ["Admins"], operations: [read, update] } { allow: owner, operations: [read, update] } ]) { identityId: ID email: AWSEmail @auth(rules: [ { allow: groups, groups: ["SuperAdmins"], operations: [read, update] } { allow: groups, groups: ["Admins"], operations: [read, update] } { allow: owner, operations: [read, update] } ]) about: String firstName: String lastName: String phone: AWSPhone profileImg: String interests: [String] facebook: String instagram: String twitter: String linkedin: String createdAt: AWSDateTime updatedAt: AWSDateTime } ```

Project Identifier

e2e641d97a6320f5f38e4746bc43a19c

Log output

``` # Put your logs below this line ```

Additional information

There are various open / closed related issues, but many with no response from OP and other issues. Hoping to create a clean plate to resolve the issue.

We did recreate the model using amplify push --allow-destructive-graphql-schema-updates where we removed userId as the primary key, and let it auto create as id.

[josefaidt]

Hey @dylan-westbury :wave: thanks for raising this! With the more recent versions of the CLI the generated VTL resolvers will expect the auth identifier as sub::username, see https://docs.amplify.aws/cli/migration/identity-claim-changes/ for more info. Can you try saving this item with the following and re-run the query?

owner: `${userAttributes.sub}::${userAttributes.username}`

You can also change this behavior by explicitly noting the identityClaim as sub in your auth rule, see https://docs.amplify.aws/cli/graphql/authorization-rules/#per-user--owner-based-data-access

[dylan-westbury]

Hi @josefaidt

Since then I have created a new environment, also updated post confirmation to set the owner as you mentioned:

Which creates the owner as expected in the database

However I still get the error in aws AppSync console

Here is the cognito user

Here is the Mutation.updateUser.auth.1.res.vtl which is auto generated by amplify

## [Start] Authorization Steps. **
$util.qr($ctx.stash.put("hasAuth", true))
#if( $ctx.error )
  $util.error($ctx.error.message, $ctx.error.type)
#end
#set( $inputFields = $util.parseJson($util.toJson($ctx.args.input.keySet())) )
#set( $isAuthorized = false )
#set( $allowedFields = [] )
#set( $nullAllowedFields = [] )
#set( $deniedFields = {} )
#if( $util.authType() == "IAM Authorization" )
  #set( $adminRoles = ["TemplateCreateUser-temp","TemplateCreateAdmin-temp","TemplateDeleteAdmin-temp"] )
  #foreach( $adminRole in $adminRoles )
    #if( $ctx.identity.userArn.contains($adminRole) && $ctx.identity.userArn != $ctx.stash.authRole && $ctx.identity.userArn != $ctx.stash.unauthRole )
      #return($util.toJson({}))
    #end
  #end
$util.unauthorized()
#end
#if( $util.authType() == "User Pool Authorization" )
  #if( !$isAuthorized )
    #set( $staticGroupRoles = [{"claim":"cognito:groups","entity":"SuperAdmins","allowedFields":["identityId","email","about","firstName","lastName","phone","profileImg","interests","facebook","instagram","twitter","linkedin","conversations","createdAt","updatedAt"],"nullAllowedFields":[],"isAuthorizedOnAllFields":false},{"claim":"cognito:groups","entity":"Admins","allowedFields":["identityId","email","about","firstName","lastName","phone","profileImg","interests","facebook","instagram","twitter","linkedin","conversations","createdAt","updatedAt"],"nullAllowedFields":[],"isAuthorizedOnAllFields":false}] )
    #foreach( $groupRole in $staticGroupRoles )
      #set( $groupsInToken = $util.defaultIfNull($ctx.identity.claims.get($groupRole.claim), []) )
      #if( $groupsInToken.contains($groupRole.entity) )
        #if( $groupRole.isAuthorizedOnAllFields )
          #set( $isAuthorized = true )
          #break
        #else
          $util.qr($allowedFields.addAll($groupRole.allowedFields))
          $util.qr($nullAllowedFields.addAll($groupRole.nullAllowedFields))
        #end
      #end
    #end
  #end
  #if( !$isAuthorized )
    #set( $ownerEntity0 = $util.defaultIfNull($ctx.result.owner, null) )
    #set( $ownerClaim0 = $util.defaultIfNull($ctx.identity.claims.get("sub"), "___xamznone____") )
    #set( $currentClaim1 = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
    #set( $ownerClaim0 = "$ownerClaim0::$currentClaim1" )
    #set( $ownerClaimsList0 = [] )
    $util.qr($ownerClaimsList0.add($util.defaultIfNull($ctx.identity.claims.get("sub"), "___xamznone____")))
    $util.qr($ownerClaimsList0.add($util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____"))))
    #set( $ownerAllowedFields0 = ["identityId","email","about","firstName","lastName","phone","profileImg","interests","facebook","instagram","twitter","linkedin","conversations","createdAt","updatedAt"] )
    #set( $ownerNullAllowedFields0 = [] )
    #set( $isAuthorizedOnAllFields0 = false )
    #if( $ownerEntity0 == $ownerClaim0 || $ownerClaimsList0.contains($ownerEntity0) )
      #if( $isAuthorizedOnAllFields0 )
        #set( $isAuthorized = true )
      #else
        $util.qr($allowedFields.addAll($ownerAllowedFields0))
        $util.qr($nullAllowedFields.addAll($ownerNullAllowedFields0))
      #end
    #end
  #end
#end
#if( !$isAuthorized && $allowedFields.isEmpty() && $nullAllowedFields.isEmpty() )
$util.unauthorized()
#end
#if( !$isAuthorized )
  #foreach( $entry in $util.map.copyAndRetainAllKeys($ctx.args.input, $inputFields).entrySet() )
    #if( $util.isNull($entry.value) && !$nullAllowedFields.contains($entry.key) )
      $util.qr($deniedFields.put($entry.key, ""))
    #end
  #end
  #foreach( $deniedField in $util.list.copyAndRemoveAll($inputFields, $allowedFields) )
    $util.qr($deniedFields.put($deniedField, ""))
  #end
#end
#if( $deniedFields.keySet().size() > 0 )
  $util.error("Unauthorized on ${deniedFields.keySet()}", "Unauthorized")
#end
$util.toJson({})
## [End] Authorization Steps. **

[dylan-westbury]

Perhaps the resolver is throwing "unauthorized" because "id" is not allowed to be updated, however this is not the case as it's the primary key to update the record?

@josefaidt

[josefaidt]

Hey @dylan-westbury :wave: in the generated resolver we see it is generating the expected identityClaim using the new default sub::username:

#set( $ownerClaim0 = $util.defaultIfNull($ctx.identity.claims.get("sub"), "___xamznone____") )
#set( $currentClaim1 = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
#set( $ownerClaim0 = "$ownerClaim0::$currentClaim1" )

And looking at this line of your resolver, it appears id is not getting set as an "allowed field" when we do not also specify id on the model:

#set( $ownerAllowedFields0 = ["identityId","email","about","firstName","lastName","phone","profileImg","interests","facebook","instagram","twitter","linkedin","conversations","createdAt","updatedAt"] )

As workaround we should be able to manually include id: ID on the GraphQL model to mitigate this issue. For example, say we have the following schema

type User @model @auth(rules: [{ allow: owner }]) {
  name: String!
}

Upon running amplify api gql-compile we see the Mutation.updateUser.auth.1.res.vtl templated is generated with:

#set( $ownerAllowedFields0 = ["name"] )
#set( $ownerNullAllowedFields0 = ["name"] )

However if we specify ID on the model and re-run amplify api gql-compile we are presented with the correct list of "allowed fields"

type User @model @auth(rules: [{ allow: owner }]) {
  id: ID
  name: String!
}

#set( $ownerAllowedFields0 = ["id","name"] )
#set( $ownerNullAllowedFields0 = ["id","name"] )

Per our documentation it is not required to specify id on types annotated with @model. Marking as a bug 🙂

[dylan-westbury]

Thanks @josefaidt

Yes we choose not to specify id in the schema as we need it autogenerated on the server rather than by the client.

[josefaidt]

Hey @dylan-westbury if you do not specify that it is a required field on the model definition (i.e. with ID!) it will not be required on the generated mutation inputs. It will be auto-generated if the client does not supply an ID

[dylan-westbury]

@josefaidt I believe when using the @primaryKey directive it requires it to be generated on the client, originally I wasn't able to create records (when using adminId: ID! @primaryKey), without passing a uuid.

[dylan-westbury]

Hi @josefaidt,

Do you know when there would be a release for this bug?

It would be good to know just because we need to finalise testing before we can release a web app / mobile app.

Kind regards

[dylan-westbury]

This is error when not specifying as mandatory.