elide icon indicating copy to clipboard operation
elide copied to clipboard

JSON Patch Operation Read Permission Issue

Open justin-tay opened this issue 2 years ago • 2 comments

Encountering an issue where running a transaction with 2 operations fails with a read permission denied error which if split into 2 separate transactions succeeds.

Expected Behavior

There shouldn't be a difference in outcome if the 2 separate operations in 2 separate transactions succeeds but the same 2 operations in a single transaction fails.

Current Behavior

This is happening partially because relationships are updated in a separate step.

I have a sample project where I have defined 3 models.

  • Author -> AuthorBook -> Book

where AuthorBook is an entity and has a one to one relationship with Author and Book.

Author has a read permission that allows read if the user has access to Book. (The sample hardcodes this to allow access if the Book has id 1 or 2)

After the setup the state looks like this

  • Book(b1)
  • Book(b2)
  • Author(a1) with AuthorBook(a1,b1)

The operations executed essentially adds AuthorBook(a1,b2) to Author and remove AuthorBook(a1,b1). When executing the transaction the failure occurs when the post processing of the relationships are being performed when Author needs to be read to apply the relationships.

The state of Author at the point of time the expression is evaluated is

  • Author(a1) with AuthorBook(a1,null) which fails because the user doesn't have access to either b1 or b2 at this point in time

This is because AuthorBook(a1,b1) was already removed. In the prior step when AuthorBook(a1,b2) was to be set on Author what was set was AuthorBook(a1,null) because it was split into the 2 phases.

Possible Solution

I don't really know enough of how the permission evaluation works but if all inline checks are deferred, similarly to what's happening for newly created resources, when performing postProcessRelationships then it should work. I don't know if this is a proper design or will break anything else though.

Steps to Reproduce (for bugs)

I have committed a project at https://github.com/justin-tay/elide-spring-boot-example/tree/permission-issue_elide-7.x which demonstrates the issue. The queries are in the README.md but I have reproduced them below.

Setup

curl --location --request PATCH 'http://localhost:8080/api/v1' \
--header 'Content-Type: application/vnd.api+json; ext=jsonpatch' \
--data '[
    {
        "op": "add",
        "path": "/author",
        "value": {
            "type": "author",
            "id": "7f14b0c3-d0ed-436c-9843-6bfbf7342df6",
            "attributes": {
                "name": "Ernest Hemingway"
            }
        }
    },
    {
        "op": "add",
        "path": "/book",
        "value": {
            "type": "book",
            "id": "1",
            "attributes": {
                "title": "The Old Man and the Sea"
            },
            "relationships": {}
        }
    },
    {
        "op": "add",
        "path": "/book",
        "value": {
            "type": "book",
            "id": "2",
            "attributes": {
                "title": "For Whom the Bell Tolls"
            },
            "relationships": {}
        }
    },
    {
        "op": "add",
        "path": "/author/7f14b0c3-d0ed-436c-9843-6bfbf7342df6/authorBooks",
        "value": {
            "type": "authorBook",
            "id": "e2d0e822-d94d-4e36-a592-ef4f2af89f75",
            "attributes": {},
            "relationships": {
                "book": {
                    "data": {
                        "type": "book",
                        "id": "1"
                    }
                }
            }
        }
    }
]'

Operation with issue

curl --location --request PATCH 'http://localhost:8080/api/v1' \
--header 'Content-Type: application/vnd.api+json; ext=jsonpatch' \
--data '[
  {
    "op": "add",
    "path": "/author/1/authorBooks",
    "value": {
      "type": "authorBook",
      "id": "bd71bc1a-dc41-4adb-bf17-610149429f68",
      "attributes": {},
      "relationships": {
        "book": {
          "data": {
            "type": "book",
            "id": "2"
          }
        }
      }
    }
  },
  {
    "op": "remove",
    "path": "/author/1/authorBooks",
    "value": {
      "type": "authorBook",
      "id": "1",
      "attributes": {},
      "relationships": {}
    }
  }
]'

justin-tay avatar May 18 '23 02:05 justin-tay

The danger in derring inline checks is that an attacker can overwrite the value of a field in a transaction. The check would have failed but because it was deferred to after their write, it now succeeds.

For example. Imagine I have a user record with a field called role. The attacker's role is READ_ONLY. They can create a transaction which overwrites the role to WRITE and then perform another operation that depends on that role. Because all checks are deferred until the end, they now can perform actions with the WRITE role.

aklish avatar Aug 26 '23 21:08 aklish

Is there any way to fix this scenario then?

I see that actually the checks were already run once during the initial pass, so even with the pull request if it did the remove authorBook operation first instead of add the request would fail as expected.

If I deferred the check until the end of each post process relationship action instead of until the transaction commit would that work? Or is there some other way to handle this?

justin-tay avatar Aug 27 '23 04:08 justin-tay