FHIR Chat · docs / PR #167 Refine authorization details around how th... · cds hooks/committers

Stream: cds hooks/committers

Topic: docs / PR #167 Refine authorization details around how th...


view this post on Zulip Github Notifications (Feb 15 2018 at 19:55):

kpshek opened PR #167
from refine-authz-details to master

This pull request removes mention of the specifics of how an EHR is to obtain an access token on behalf of the CDS Service. Instead, the documentation just lays out what the access code needs to honor.

Additionally, this pull request changes the conformance verbiage that states the access token MUST be restricted to the same permissions as the current user. Previously, this was SHOULD but after discussion with @bakerdb, @jmandel, @isaacvetter, @brettmarquard, and myself, we felt MUST was more appropriate.

This will resolve #159.

view this post on Zulip Github Notifications (Feb 15 2018 at 19:55):

kpshek review_requested PR #167

view this post on Zulip Github Notifications (Feb 15 2018 at 19:55):

kpshek commented on PR #167

should smart be uppercase (SMART) everywhere?

Yes and this pull request ensures that all references to SMART are uppercase now.

view this post on Zulip Github Notifications (Feb 15 2018 at 20:13):

kpshek synchronized PR #167

view this post on Zulip Github Notifications (Feb 16 2018 at 04:13):

kpshek commented on PR #167

Given that the fhirServer parameter (in HTTP Request) is relevant only if a fhirAuthorization is included in the request, I suggest making fhirServer a field within the fhirAuthorization parameter. This will enable us to make fhirServer REQUIRED within fhirAuthorization, thereby eliminating the need for the (current) explanation that "If fhirAuthorization is provided, this [OPTIONAL] field is REQUIRED."

@bakerdb - I had this same thought too and we discussed this on one of our previous Argonaut security calls. I had the action item to take this to the broader CDS Hooks community for feedback which uncovered a use case where it is valuable to continue keeping these two fields separated.

There is a use case in which the FHIR server is not accessible (externally) or does not offer access via the access token (in which case fhirAuthorization is omitted) but the data is available via prefetch; however, the fhirServer parameter is still useful in this scenario as the CDS Service knows where the data came from. Due to this use case (which several developers have in their production environments), keeping our fields as-is is appropriate for everyone.

view this post on Zulip Github Notifications (Feb 19 2018 at 19:31):

bakerdb commented on PR #167

Thank you, @kpshek. I knew that we had discussed this and that you were to present it to the CDS Hooks community. But I was unaware of the resolution. I don't fully understand why it's necessary for the CDS Service to know where EHR-prefetched data came from, but if existing production environments require the fhirServer parameter, then I see no compelling reason to move it so long as its association with fhirAuthorization is preserved.

view this post on Zulip Github Notifications (Feb 19 2018 at 21:14):

kpshek commented on PR #167

Great, thanks @bakerdb. Glad we have this cleared up. :smile:

view this post on Zulip Github Notifications (Feb 19 2018 at 21:14):

kpshek closed PR #167


Last updated: Apr 12 2022 at 19:14 UTC