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

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

Closed
eevaturkka opened this Issue Jun 7, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@eevaturkka

eevaturkka commented Jun 7, 2017

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

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Jun 8, 2017

Owner

Interesting....

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

Owner

jamesagnew commented Jun 8, 2017

Interesting....

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

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Jun 9, 2017

Owner

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.

Owner

jamesagnew commented Jun 9, 2017

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.

@jamesagnew jamesagnew closed this in c2e5fa3 Jun 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment