Stream: cds hooks/github
Topic: docs / Issue #205 Specify that an implementation specific...
Github Notifications (May 13 2018 at 10:10):
isaacvetter opened Issue #205
During the HL7 FHIR Connectathon in Koeln, we had a CDS Hooks breakout session and talked through the tenant / billing_code discussion that originated with this SMART issue: https://github.com/smart-on-fhir/smart-on-fhir.github.io/issues/133
We talked through this issue for CDS Hooks and started to get consensus that an implementation specific key/value pair in the JWT body is a reasonable method to suggest in the specification.
Github Notifications (May 13 2018 at 10:32):
grahamegrieve commented on Issue #205
I like it in the JWT, and that works for CDS-hooks, but I worry that putting in the JWT is too late for some applications - they'll want to know this before auth not after. However it will be hard to be secure if it's ut in the launch URL
Github Notifications (May 13 2018 at 10:37):
jmandel commented on Issue #205
The JWT is available right up front -- a CDS Service can validate it first, then apply logic... or can look inside it before validating. I'm not sure how this affects your assessment @grahamegrieve
Github Notifications (May 13 2018 at 11:27):
grahamegrieve commented on Issue #205
it is in the CDS-hooks context, but not in the context of a smart app launched from the EHR
Github Notifications (May 13 2018 at 11:33):
jmandel commented on Issue #205
That's a SMART issue though. For non-sensitive information in SMART, we
could add another ehr-launch-time query param to convey this information.
We could even migrate from the current "iss" and "launch" parameters to a
signed JWT. But that's a separate discussion.On Sun, May 13, 2018, 13:27 Grahame Grieve <notifications@github.com> wrote:
it is in the CDS-hooks context, but not in the context of a smart app
launched from the EHR—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<https://github.com/cds-hooks/docs/issues/205#issuecomment-388619827>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AATHAX1X3SEpKbmJa7FQDiwAhNwVzoQvks5tyBiGgaJpZM4T8vWc>
.
Github Notifications (May 13 2018 at 12:14):
olbrich commented on Issue #205
To a large extent this problem can be resolved by allowing implementers to include additional fields in the service request that would be specified by a service provider in their implementation guide.
Github Notifications (May 13 2018 at 12:51):
nschwertner commented on Issue #205
The proposed approach to use extend the JWT to be the carrier of vendor/site/etc claims might open new possibilities beyond our current approach of:
a) Inferring the vendor/site from the the FHIR Base/Service URI at launch time
b) Injecting custom parameters in the SMART launch URI or otherwise using unique launch URIs per siteWhile (b) would work for a SMART app, it it not that trivial to implement in a CDS Hooks service scenario without turning it into a hack. The nice part of JWT is that it would work equally well for a SMART app and a CDS Hooks service with the added benefit of signature-based authenticity validation of the claims.
I wonder though if there is anything simpler to implement than a JWT to the same effect. For example, the CDS service invocation (the payload of the POST req) already contains context that is trusted, then why not extend that context with other useful claims?
Github Notifications (May 13 2018 at 14:18):
kpshek commented on Issue #205
I wonder though if there is anything simpler to implement than a JWT to the same effect. For example, the CDS service invocation (the payload of the POST req) already contains context that is trusted, then why not extend that context with other useful claims?
The reason why I think placing this information in the JWT is most appropriate because it seems to be related to the EHR authentication process as well as information already in the JWT (namely, the issuer represented by the
iss
).Here is a concrete proposal for a new private claim in the JWT payload:
Field Optionality Type Value tenant REQUIRED string An opaque string identifying the healthcare organization that is invoking the CDS Hooks request. Per rfc7519, regarding the claims:
All the names are short because a core goal of JWTs is for the representation to be compact.
Additionally, since we are defining a private claim here, also from rfc7519:
Unlike Public Claim Names, Private Claim Names are subject to collision and should be used with caution.
I am proposing a private claim (as opposed to public) as I don't see a need register a new public claim in the IANA "JSON Web Token Claims" registry. I am not concerned with claim name collisions since we are defining what fields we use in the JWT and there are no public claims with the proposed name today.
This new JWT payload claim satisfies the immediate need expressed in the original issue description.
For the needs expressed by @olbrich, we could add this additional statement to our specification:
"An EHR and CDS Service may negotiate additional custom private claims in the JWT payload to communicate additional data. These custom private claims should be named with a prefix of "
x-
" (extension) to avoid name collisions with possible future CDS Hooks claims."Thoughts?
Github Notifications (May 13 2018 at 14:33):
olbrich commented on Issue #205
@kpshek Perhaps
entity
, it's less tied to a particular use case.
Github Notifications (May 13 2018 at 14:36):
kpshek commented on Issue #205
Perhaps entity, it's less tied to a particular use case.
I wouldn't be opposed to using
entity
as the claim name.
Github Notifications (May 13 2018 at 14:45):
nschwertner commented on Issue #205
entity
or eveninstance
sound abstract enough to me. What is the rationale behind making this a "required" claim?
Github Notifications (May 13 2018 at 14:57):
kpshek commented on Issue #205
What is the rationale behind making this a "required" claim?
My thinking was that every EHR is going to have some identifier (already in use or can create a new identifier) that represents this concept.
I know we're focused on CDS Hooks here, but in my experience with production SMART on FHIR apps, nearly all of them need this value (which is why the large EHR vendors provide this value as part of the SMART app launch). This reality was another reason why I am proposing this field is required.
Github Notifications (May 13 2018 at 14:57):
kpshek labeled Issue #205
Github Notifications (May 15 2018 at 14:25):
jec commented on Issue #205
In one implementation, I called mine
rqe
for "requesting entity."
Github Notifications (May 29 2018 at 21:10):
isaacvetter commented on Issue #205
How would
entity
be different fromsub
in the JWT? ... and why?
Github Notifications (May 30 2018 at 18:38):
kpshek commented on Issue #205
How would entity be different from sub in the JWT? ... and why?
I wouldn't be opposed to using the
sub
field for this. rfc 7519 definessub
as:4.1.2. "sub" (Subject) Claim
The "sub" (subject) claim identifies the principal that is the
subject of the JWT. The claims in a JWT are normally statements
about the subject. The subject value MUST either be scoped to be
locally unique in the context of the issuer or be globally unique.
The processing of this claim is generally application specific. The
"sub" value is a case-sensitive string containing a StringOrURI
value. Use of this claim is OPTIONAL.We could argue that the subject of our JWT is that entity/organization. I was hoping that whatever field name we came up with, we could use this same name as a new launch context parameter in SMART. I'd be fine using
sub
here in the context of a JWT as we can explain that but a field namedsub
seems a bit odder in the SMART on FHIR launch context parameter use case.With all of that being said, I've been beaten down on this particular topic for a long time so I'm fine with whatever name we decide to use --
sub
or something equally as generic (likeentity
). :stuck_out_tongue_closed_eyes:
Github Notifications (Jun 13 2018 at 15:53):
kpshek milestoned Issue #205
Github Notifications (Jun 13 2018 at 15:53):
kpshek unlabeled Issue #205
Github Notifications (Jun 13 2018 at 20:49):
kpshek commented on Issue #205
## :telephone_receiver: CDS Working Group Block Vote (6-13-2018)
Meeting notes: http://wiki.hl7.org/index.php?title=File:2018-06-13_CDS_WG_Call_Minutes.docx
@kpshek moved the following disposition, seconded by @isaacvetter.
We will add the following new private claim to the JWT:
Field Optionality Type Value tenant OPTIONAL string An opaque string identifying the healthcare organization that is invoking the CDS Hooks request. :+1: For: 10
:expressionless: Abstain: 0
:-1: Against: 0:tada: The motion passed! :tada:
Github Notifications (Aug 01 2018 at 04:38):
kpshek labeled Issue #205
Github Notifications (Nov 07 2018 at 16:56):
kpshek closed Issue #205
Last updated: Apr 12 2022 at 19:14 UTC