Update allows updating resource that is not within read / write scope

eevaturkka commented 7 years ago

Still using Hapi 2.4

We have created scopes like:

builder.allow("read:" + scope).read().resourcesOfType(Observation.class).inCompartment("Patient", userIdPatientId);

and

builder.write("write:" + scope).write().resourcesOfType(Observation.class).inCompartment("Patient", userIdPatientId);

Lets say there is an Observation 1 with patient A (44a12254-b28d-42f9-8bec-4a468473ef9f) that's been saved with Access token with Patient A as the resource owner. Now it is possible for Patient B to update Observation 1 to with permissions from her access token if the subject of the resource in the update request is Patient B (21bb8e2a-673e-42f0-8843-ac90d18d8222).

Shouldn't there be also a check in the update operation that the resource being updated is within the given rules? Are we missing some rule here?

POSTing first version of a resource and reading a resource with GET work correctly with the rules here.

POST Patient 44a12254-b28d-42f9-8bec-4a468473ef9f

POST https://localhost/baseStu3/Observation HTTP/1.1
Authorization: Bearer eyJraWQiOiJyc2ExIiwiYWxnIjoiUlMyNTYifQ.eyJzdWIiOiI0NGExMjI1NC1iMjhkLTQyZjktOGJlYy00YTQ2ODQ3M2VmOWYiLCJhenAiOiJUSU1JUFJIIiwiaXNzIjoiaHR0cHM6XC9cL3Boci1hdXRoLmlnLmthbnRhLmZpXC9waHItYXV0aHNlcnZlci1zYW5kYm94XC8iLCJleHAiOjE0OTY4MzUzMzEsImlhdCI6MTQ5NjgzMTczMSwianRpIjoiYmMzMzdkYmEtMWNmYi00YzJhLWJkZGUtYjA3YTBhMmUzOWMyIn0.rkJ_Th_XgOATG3I6FgCMCah7SrZbdzooEWSfNW-HCCoEPrbT9MWJFdEvaE_eQqbkSjxRn0zJFEIqLwYKXQ9WdsSdPVZm47ypPTIsLRIF96iXeeWc9X0L3bqpM_xJQwlX-UVTH8VRxbXRn1yehtTxdLHPZ5HLYaZMQo5IwB-GkMQ
{
    "resourceType":"Observation",
    "meta":{
        "profile":[
            "http://phr.kanta.fi/StructureDefinition/fiphr-bodyheight-stu3"
        ]
    },
    "status":"final",
  "category": [{
    "coding": [
      {
        "system": "http://hl7.org/fhir/observation-category",
   "version": "1",
        "code": "vital-signs",
        "display": "Vital Sign",
  "userSelected": "true"
      }
    ],
    "text": "Catogory text"
  }],
    "code":{
        "coding":[
            {
                "system":"http://loinc.org/",
                "code":"8302-2",
    "display":"Body height"
            }
        ]
    },
    "subject":{
        "reference":"Patient/44a12254-b28d-42f9-8bec-4a468473ef9f"
    },
    "effectiveDateTime":"2017-05-02",
"issued":"2017-05-02T10:00:00.936+02:00",
  "performer": [{
    "reference": "Patient/44a12254-b28d-42f9-8bec-4a468473ef9f"
  }],
    "valueQuantity":{
        "value":165,
  "comparator":">",
        "unit":"cm",
  "system":"http://unitsofmeasure.org/",
  "code":"cm"

    }
}

response:

HTTP/1.1 201 Created
{
  "resourceType": "OperationOutcome",
  "issue": [
    {
      "severity": "information",
      "code": "informational",
      "diagnostics": "Successfully created resource \"Observation/79286/_history/1\" in 9ms"
    }
  ]
}

PUT with Patient B (21bb8e2a-673e-42f0-8843-ac90d18d8222)

PUT https://loicalhost/baseStu3/Observation/79286?_format=json&_pretty=true HTTP/1.1
Authorization: Bearer eyJraWQiOiJyc2ExIiwiYWxnIjoiUlMyNTYifQ.eyJzdWIiOiIyMWJiOGUyYS02NzNlLTQyZjAtODg0My1hYzkwZDE4ZDgyMjIiLCJhenAiOiJUSU1JUFJIIiwiaXNzIjoiaHR0cHM6XC9cL3Boci1hdXRoLmlnLmthbnRhLmZpXC9waHItYXV0aHNlcnZlci1zYW5kYm94XC8iLCJleHAiOjE0OTY4MzU0OTIsImlhdCI6MTQ5NjgzMTg5MiwianRpIjoiMzg0ZWNjMjktYjRhNi00Y2M1LThiNWUtOTZiZWY4Y2FhODFlIn0.w3HCm08ZwdYAJGYIt92Ogu2MeJ5rRXIuNv-1Do3z7f3y1bmYFN7Q8atRXu-WCrsRGnvHHewlHZ3Pig3HyEy_Nbxuo8juRHVCys4vALs8b_BWTDTN8YuZFDHEdrjACPPtaa4ymitHS3eWxJLrCjAhqfDyemEwsZ74r2G8fKFyDuo
{
    "resourceType":"Observation",
"id":"79286",
    "meta":{
        "profile":[
            "http://phr.kanta.fi/StructureDefinition/fiphr-bodyheight-stu3"
        ]
    },
    "status":"final",
   "category": [{
    "coding": [
      {
        "system": "http://hl7.org/fhir/observation-category",
   "version": "1",
        "code": "vital-signs",
        "display": "Vital Sign",
  "userSelected": "true"
      }
    ],
    "text": "Catogory text"
  }],
    "code":{
        "coding":[
            {
                "system":"http://loinc.org/",
                "code":"8302-2",
    "display":"Body height"
            }
        ]
    },
    "subject":{
        "reference":"Patient/21bb8e2a-673e-42f0-8843-ac90d18d8222"
    },
    "effectiveDateTime":"2017-05-02",
"issued":"2017-05-02T10:00:00.936+02:00",
  "performer": [{
    "reference": "Patient/21bb8e2a-673e-42f0-8843-ac90d18d8222"
  }],
    "valueQuantity":{
        "value":120,
  "comparator":">",
        "unit":"cm",
  "system":"http://unitsofmeasure.org/",
  "code":"cm"

    }
}

response for the put with patient B.

HTTP/1.1 200 OK
{
  "resourceType": "OperationOutcome",
  "issue": [
    {
      "severity": "information",
      "code": "informational",
      "diagnostics": "Successfully created resource \"Observation/79286/_history/2\" in 18ms"
    }
  ]
}

jamesagnew commented 7 years ago

Interesting....

Yes, I would say this is an issue. Let me try and get a unit test written.

jamesagnew commented 7 years ago

Ok, reproduced. I will check in a fix a bit later.

I'm inclined to say that if a resource is being updated from A to B, the client needs to have write permission for both A and B. That seems like the easiest way to fix this.

hapifhir / hapi-fhir

Update allows updating resource that is not within read / write scope #667