Support Binary.securityContext and X-Security-Context for Binary resources

[lmsurpre]

Is your feature request related to a problem? Please describe. from http://hl7.org/fhir/binary.html#rest

Very often, the content of a Binary resource is sensitive, and the server should apply appropriate access control to the content. When the server itself generates the content, it implicitly knows what access control to apply. When the client provides the binary to the server itself, it uses the securityContext element (or the matching X-Security-Context HTTP header) to inform the server that the Binary resource should be treated as if it was the other resource. Typically, the other resource is a DocumentReference or similar resource that refers directly to the Binary resource, but that is not mandatory

In our default config, we use basic auth and security/permissions models must be added on top. However, in cases where the fhir-smart interceptor is used, it would be nice to support this special feature for Binary resources.

Describe the solution you'd like Support this from our fhir-smart interceptor; probably in the afterX methods. If a given access token wouldn't grant access to the securityContext target, then it shouldn't give access to this Binary resource either.

In my opinion, X-Security-Context should only be considered when the contentType is non-FHIR (see #3715), but we'll need to document our approach...especially in cases where both Binary.securityContext and the header are present.

Describe alternatives you've considered

Acceptance Criteria

1. GIVEN [a precondition] AND [another precondition] WHEN [test step] AND [test step] THEN [verification step] AND [verification step]

Additional context

[lmsurpre]

Even if we don't implement this, maybe we should introduce a config option to throw an error if someone specifies a securityContext that we don't support?

[lmsurpre]

Decision: implement as suggested above and make that the default (secure by default)

introduce a config option to throw an error if someone specifies a securityContext that we don't support

[lmsurpre]

notes on introducing a new config parameter:

• constant in FHIRConfiguration
• use FHIRConfigHelper to read the value (with optional default value)
• document in FHIRServerUserGuide section 5.1 (3 tables)

fhirServer/security/validateSecurityContext with values true or false?

[lmsurpre]

I confirmed that, with fhir-smart installed in userlib, creating or updating a Binary resource that has a valued Binary.securityContext element now results in an 403 with an operation outcome like

{
	"resourceType": "OperationOutcome",
	"id": "c0-a8-0-97-073eb700-2542-4e05-a30d-918ccfc2f74b",
	"issue": [
		{
			"severity": "fatal",
			"code": "forbidden",
			"details": {
				"text": "securityContext is not supported for resource type Binary"
			}
		}
	]
}

One interesting case that I hit while testing is that, due to our "skippable updates" feature where no-op updates (where incoming resource matches the existing resource) get skipped by default, I wasn't seeing the interceptor complain about the Binary.securityContext field on an update. After adding X-FHIR-FORCE-UPDATE, I saw the expected behavior.

[lmsurpre]

Additionally, I confirmed that by setting validateBinarySecurityContext in fhir-server-config.json this error goes away and becomes just a warning in the server logs.

Personally I think the message detail could be a bit better by making it clear that Binary.securityContext is the problematic element (and maybe by mentioning the value that was requested for this element). But we can improve that if/when we get to #4036

[lmsurpre]

I tried creating a Binary resource that has a SecurityContext element which is a "logical reference" (Reference.identifier) instead of a "literal reference" (Reference.value) and, to my surprise, even with validateBinarySecurityContext = true we are allowing this resource to be created.

Since we don't honor the intent of this field for any reference type (logical or literal or absolute or what-have-you), I think we want to provide the warning/error behavior for any securityContext element (i.e. whenever securityContext is not null)

[lmsurpre]

Prasanna added the fix for the above comment. Looks good now.