Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting "Unauthorized on [field]" with update permissions if trying to modify a field to null #2487

Open
2 tasks done
AleksandarGT opened this issue Apr 23, 2024 · 3 comments
Labels
pending-response question Further information is requested

Comments

@AleksandarGT
Copy link

How did you install the Amplify CLI?

npm

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

v16.16.0

Amplify CLI Version

12.10.1

What operating system are you using?

Windows

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

No manual changes made

Describe the bug

When I give permissions for owners of a record to only be able to read and update it, they cannot update a field to null even if it was already null.

I need to give them permissions for create and delete in order to be able to modify the given field.

GraphQL model

type User
@model
@auth(
rules: [
# Everyone logged in can read data
{ allow: private, operations: [read] }
{ allow: groups, groups: ["Admin"] }
{ allow: owner, ownerField: "owner", identityClaim: "username", operations: [read, update] }
]
) {
id: ID!
about: String

Using Angular codegen, if I try to do

const res = await this.apiService.UpdateUser({
  id: id,
  about: null,
})

I will get the following error
image

Adding create and delete permissions seems to fix the problem. However, I don't want users to be able to create new User records or delete their existing User record.

Expected behavior

Owners of a record should be able to update any field on their record by using the update permissions.

Reproduction steps

Project Identifier

No response

Log output

# Put your logs below this line


Additional information

Project is running on amplify version
"aws-amplify": "^5.3.18",

Before submitting, please confirm:

  • I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • I have removed any sensitive information from my code snippets and submission.
@phani-srikar
Copy link
Contributor

Hi @AleksandarGT, can you try removing the field altogether from your selection set, instead of re-setting it to null?

@AleksandarGT
Copy link
Author

If I remove the field from an update query it will not update the field of the record so it will be fine. However, this is not the desired functionality, I just want to set a given value to null.

@palpatim palpatim added the bug Something isn't working label May 3, 2024
@palpatim
Copy link
Member

palpatim commented May 9, 2024

tl;dr: You need to give the owner delete permissions on the field while continuing to restrict delete on the model.

Background

Setting a field to null represents a deletion of that field, not an update of the field. I stipulate that this is a bit nonintuitive for a scalar field, but it's how we elected to implement

With that in mind, you can verify that owner doesn't have permissions to set that field to null by viewing the access control matrix for your type:

(Note the entry in userPools:owner:owner for about:delete)

% amplify status api -acm User
⚠️ WARNING: owners may reassign ownership for the following model(s) and role(s): User: [owner]. If this is not intentional, you may want to apply field-level authorization rules to these fields. To read more: https://docs.amplify.aws/cli/graphql/authorization-rules/#per-user--owner-based-data-access.
✅ GraphQL schema compiled successfully.

Edit your schema at <redacted>
userPools:private
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │ false  │ false  │ false  │ true │ true │  true  │  true  │ true │
  │  about  │ false  │ false  │ false  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘
userPools:staticGroup:Admin
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │  true  │  true  │  true  │ true │ true │  true  │  true  │ true │
  │  about  │  true  │  true  │  true  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘
userPools:owner:owner
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │ false  │  true  │ false  │ true │ true │  true  │  true  │ true │
  │  about  │ false  │  true  │ false  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘

To allow owners to delete a single field, you'll need to add field-level auth to specify that intent. (Don't forget that field-level auth rules aren't inherited: an auth rule on a field overrides any model-level auth. So you'll need to specify all allowable permissions on that field.)

type User
  @model
  @auth(
    rules: [
      # Everyone logged in can read data
      { allow: private, operations: [read] }
      { allow: groups, groups: ["Admin"] }
      {
        allow: owner
        ownerField: "owner"
        identityClaim: "username"
        operations: [read, update]
      }
    ]
  ) {
  id: ID!
  about: String
    @auth(
      rules: [
        # Everyone logged in can read data
        { allow: private, operations: [read] }
        { allow: groups, groups: ["Admin"] }
        {
          allow: owner
          ownerField: "owner"
          identityClaim: "username"
          operations: [read, update, delete]
        }
      ]
    )
}

Now the ACM looks as you expect:

% amplify status api -acm User
⚠️ WARNING: owners may reassign ownership for the following model(s) and role(s): User: [owner]. If this is not intentional, you may want to apply field-level authorization rules to these fields. To read more: https://docs.amplify.aws/cli/graphql/authorization-rules/#per-user--owner-based-data-access.
✅ GraphQL schema compiled successfully.

Edit your schema at <redacted>
userPools:private
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │ false  │ false  │ false  │ true │ true │  true  │  true  │ true │
  │  about  │ false  │ false  │ false  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘
userPools:staticGroup:Admin
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │  true  │  true  │  true  │ true │ true │  true  │  true  │ true │
  │  about  │  true  │  true  │  true  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘
userPools:owner:owner
  ┌─────────┬────────┬────────┬────────┬──────┬──────┬────────┬────────┬──────┐
  │ (index) │ create │ update │ delete │ get  │ list │ search │ listen │ sync │
  ├─────────┼────────┼────────┼────────┼──────┼──────┼────────┼────────┼──────┤
  │   id    │ false  │  true  │ false  │ true │ true │  true  │  true  │ true │
  │  about  │ false  │  true  │  true  │ true │ true │  true  │  true  │ true │
  └─────────┴────────┴────────┴────────┴──────┴──────┴────────┴────────┴──────┘

And setting the field to null succeeds as expected:

Update query

# Logged in as user1
mutation MyMutation {
  updateUser(input: {id: "uid-1", about: null}) {
    id
    owner
    about
  }
}

Update result

{
  "data": {
    "updateUser": {
      "id": "uid-1",
      "owner": "user1",
      "about": null
    }
  }
}

Delete query

# logged in as user1
mutation MyMutation {
  deleteUser(input: {id: "uid-1"}) {
    id
    owner
    about
  }
}

Delete result

{
  "data": {
    "deleteUser": null
  },
  "errors": [
    {
      "path": [
        "deleteUser"
      ],
      "data": null,
      "errorType": "Unauthorized",
      "errorInfo": null,
      "locations": [
        {
          "line": 2,
          "column": 3,
          "sourceName": null
        }
      ],
      "message": "Not Authorized to access deleteUser on type Mutation"
    }
  ]
}

Hope this helps.

@palpatim palpatim added question Further information is requested pending-response and removed bug Something isn't working pending-triage labels May 9, 2024
@palpatim palpatim removed their assignment May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-response question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants