IHE / IT-Infrastructure

Online repository for information assets supporting the profiles (implementation specifications) in the IHE IT Infrastructure Technical Framework.
Creative Commons Attribution 4.0 International
33 stars 13 forks source link

Limited role in scope parameter #104

Closed Martin-Rosner closed 3 years ago

Martin-Rosner commented 4 years ago

The role in the scope parameter is now limited to user and patient. The system role from the FHIR bulk API should be added here to support service-to-service authorization.

msmock commented 4 years ago

It was not intended to limit it. I thought that the use of RECOMMENDED and SHOULD in section "3.71.6.3 Scope Parameter" allows to use values according to your needs. I just wanted to make a simple proposal for possible values which are obvious to the reader. Actually in Switzerland we use different values. But I can modify it, if it is not clear.

Do you have a proposal for a text improvement which makes things clear?

joostreuzel commented 4 years ago

For the text the SHOULD can become a MAY in my view. SHOULD I interpret as "must unless", which is still quite strong. MAY is more of a suggestion...

In our current work we are using the transaction identifiers as access scopes. For example IHE QEDm introduces transaction PCC-44 and a number of options. We model these as scopes such as: PCC-44 and PCC-44.OBSERVATION. This may be an interesting approach for IHE in general as it is base-standard agnostic and could apply to e.g. DICOM-Web as well.

JohnMoehrke commented 4 years ago

For the text the SHOULD can become a MAY in my view. SHOULD I interpret as "must unless", which is still quite strong. MAY is more of a suggestion...

In our current work we are using the transaction identifiers as access scopes. For example IHE QEDm introduces transaction PCC-44 and a number of options. We model these as scopes such as: PCC-44 and PCC-44.OBSERVATION. This may be an interesting approach for IHE in general as it is base-standard agnostic and could apply to e.g. DICOM-Web as well.

@joostreuzel can you provide more detail on how you are using the transaction numbers? I would like to have scope definitions be in the content specific profile. So QEDm would define the scopes to use when grouped with IUA. IUA would only define general purpose scopes that are useful on all RESTful. We would like to add some of these scope definitions as CP to these other profiles.

This is different than the original issue which was specific to user and patient.

JohnMoehrke commented 4 years ago

as discussed on the call today. The proposed use of user and patient do conflict with SMART. We should avoid conflicts when possible. We discussed that we could define these uses, but have different role words like "subject" (the user is the patient subject of the data), and "practitioner" (the user is a practitioner). I think in both cases we should also be clear which Patient and which Practitioner. It might be left open to "look it up by username as an .identifier in Patient or Practitioner. Which would be a fine clarification in my view.

msmock commented 4 years ago

As decided yesterday I will rework this part to be neutral (see comments on #109).

joostreuzel commented 4 years ago

As indicated above we simply use the transaction identifiers as scope values. e.g. ITI103 for the new introspect endpoint, PCC44 for the transaction defined in QEDm. In case there are options to be understood, we postfix them to the transaction with a dot as seperator. For example, QEDm has an Observation option. This is translated to the PCC44.OBSERVATION scope.

In other words, a client invoking a resource server with an access token that contains the PCC44 scope claim is allowed to call all API methods and paths defined as part of the QEDm PCC-44 transaction. Both resource server and client will know what such a scope implies as they have both gone through a conformance test...

Alternatively, each profile referring to IUA profile could define its set of scopes, but there is a risk of conflict. Two APIs from different profiles may define the same scopes. As the transaction ids are globally defined we thought that would be a safer way of working.

msmock commented 4 years ago

Please be aware that not all scopes do depend on the ressource server or the transactions. In switzerland scopes are used to convey other claims like breaking the glass access, user roles, a.o. Thus we agreed to neutralize the scope requirements. This task is assigned to me and I plan to do it to the due date.

Am 05.10.2020 um 17:54 schrieb joostreuzel notifications@github.com:

 As indicated above we simply use the transaction identifiers as scope values. e.g. ITI103 for the new introspect endpoint, PCC44 for the transaction defined in QEDm. In case there are options to be understood, we postfix them to the transaction with a dot as seperator. For example, QEDm has an Observation option. This is translated to the PCC44.OBSERVATION scope.

In other words, a client invoking a resource server with an access token that contains the PCC44 scope claim is allowed to call all API methods and paths defined as part of the QEDm PCC-44 transaction. Both resource server and client will know what such a scope implies as they have both gone through a conformance test...

Alternatively, each profile referring to IUA profile could define its set of scopes, but there is a risk of conflict. Two APIs from different profiles may define the same scopes. As the transaction ids are globally defined we thought that would be a safer way of working.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

JohnMoehrke commented 4 years ago

It seems to me this is very clearly indicating that there is a need to define scopes somewhere inside the IHE profiles. These specifications would not forbid extensions, for realm specific use.

Note, I think break glass could be something that IUA itself should define. Do we think that this could be defined? Might be useful to define this inside of a named option, so that it would be clear that the scope use was needed.

I am not against using the transaction numbers as @joostreuzel explains. However would like to use that mostly where the transaction is highly constrained, mostly transactions. Where the transaction is simply FHIR rest query (e.g. QEDm), it seems to me using the SMART scope model is reasonable.

msmock commented 4 years ago

I do not agree. This would be a too restricted view on the role of the scopes in OAuth.

As I wrote in the introduction the aim of OAuth is to authorize applications to access certain data on behalf of the user. Thus a user (e.g., healthcare professional, patient, …) shall be able to limit the application access to specific data, not on specific transactions.

As an example the user might want his medication mobile application on the tablet to access medication related documents only, but no other documents. Thus the user must be able to restrict the scope to the medication data retrieved by MHD, but not on the transaction (which is by the way already targeted with the audience restriction).

Think of your mobile: When installing apps you are typically asked if you want to allow the application to access your contacts, calendar or other data. It does not ask you to allow transactions using the transaction names or numbers, which are typically not known to the user.

This is analogous to Google or other services use OAuth: They restrict the scope granted to third party applications by data type, and not by transaction. The confusion might be caused by the fact, that most services transactions are one to one to the data type. But we don’t have that in IHE in general.

If we don’t want to use the scopes as suggested by Heart (which work as described above), we should do as we agreed upon in the meeting and leave the scopes open to be defined in the projects or national extensions (like for our Swiss EPR).

I’m on holiday next week and plan to add explaining text on scopes afterwards.

Please note, that I will not be able to join the next IUA meeting.

On 5 Oct 2020, at 23:06, John Moehrke notifications@github.com wrote:

It seems to me this is very clearly indicating that there is a need to define scopes somewhere inside the IHE profiles. These specifications would not forbid extensions, for realm specific use.

Note, I think break glass could be something that IUA itself should define. Do we think that this could be defined? Might be useful to define this inside of a named option, so that it would be clear that the scope use was needed.

I am not against using the transaction numbers as @joostreuzel https://github.com/joostreuzel explains. However would like to use that mostly where the transaction is highly constrained, mostly transactions. Where the transaction is simply FHIR rest query (e.g. QEDm), it seems to me using the SMART scope model is reasonable.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/IHE/IT-Infrastructure/issues/104#issuecomment-703888643, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEC6MUI2GPEX3LHBXVEYFDSJIYL7ANCNFSM4RZR76RQ.

joostreuzel commented 4 years ago

I do see your point about the transactions to be too course grain for some use-cases. Moreover, features like break-the-glass and VIP access cover yet another range of features that can be controlled through scopes.

All-in-all, I'm in favor of not specifying the scopes as part of the IUA profile. Leaving it up to individual implementations is also not a good idea. The middle-ground in my view would be to specify these scopes as part of the different IHE profiles that cover HTTP-based transactions. For example, QEDm and PIXm and PRIM should refer to IUA for authorization and could define the scopes required to access the different resources and operations. This also allows these profiles to choose scopes that match the underlying base standard. For FHIR-based APIs that may be the HEART scopes, but what to use for DICOM-Web, or for the message based transactions as used in PRIM?

That said:

msmock commented 4 years ago

Sorry, maybe I was too short on this.

The scope coding names are not important. That’s true. But the user expects scopes representing the content of the data to be able to authorize the application client to access the data. Taking MHD for example, the data content is typically coded in the document metadata (document class, document type, facility, …) and the values of the metadata are taken from value sets, chosen by the project. Thus I don’t see, how the profiles can specify the scopes, only the project can do that.

On 6 Oct 2020, at 21:58, joostreuzel notifications@github.com wrote:

I do see your point about the transactions to be too course grain for some use-cases. Moreover, features like break-the-glass and VIP access cover yet another range of features that can be controlled through scopes.

All-in-all, I'm in favor of not specifying the scopes as part of the IUA profile. Leaving it up to individual implementations is also not a good idea. The middle-ground in my view would be to specify these scopes as part of the different IHE profiles that cover HTTP-based transactions. For example, QEDm and PIXm and PRIM should refer to IUA for authorization and could define the scopes required to access the different resources and operations. This also allows these profiles to choose scopes that match the underlying base standard. For FHIR-based APIs that may be the HEART scopes, but what to use for DICOM-Web, or for the message based transactions as used in PRIM?

That said:

The audience restriction is too course grain to be compared with a transaction. Audience points to the resource server, which may implement multiple transactions. Moreover, I do not get your point about how such scopes are presented to the user. Scope names are not intended to rendered to the user directly. There is an (application specific) mapping between what user sees and agrees to to the underlying data structures and access scopes. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/IHE/IT-Infrastructure/issues/104#issuecomment-704519582, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEC6MWVSRXJKYAUUPY4LKTSJNZHHANCNFSM4RZR76RQ.

joostreuzel commented 4 years ago

I see your point. So we have discussed three options (with increasing level of detail):

  1. define scopes as transaction names
  2. define scopes as part of transaction definition
  3. define scopes as part of projects

Question to me is which of these three levels do we want to support? I can imagine that we would define the base scopes as part of the transactions, allowing projects to add additional scopes where necessary. The transactions could even define how the projects can refine the scopes.

We could do also all three, where level 2 (transaction specific scopes) is only applicable if a corresponding level 1 (transaction id as scope) is present. Similarly, project specific scopes would become relevant only if a corresponding level 2 (transaction specific scopes) is present.

As an example: scope: PCC44 patient/Observation:read LOINC:39156-5 would allow reading a BMI Observation through transaction PCC44. PCC44 refers to the transaction. The HEART scope usage would be defined by the PCC44 transaction. The LOINC code is a project specific refinement. Omitting PCC44 would render the HEART and LOINC-based scope useless.

The worry I have is that if we would leave the scopes open, this could break interoperability...

JohnMoehrke commented 4 years ago

I would really like us to come to some conclusion.

  1. I don;'t thin that IUA can define any scope values, except possibly break-glass if we think we can identify that one. And is that a scope or something else.
  2. I think that scopes can be defined at the 'other' profile transaction level. For example QEDm could define scopes for use with QEDm. MHD could define scopes for MHD. These scopes would be in the "grouping behavior". Meaning they become SHALL 'when grouped'.
  3. We can not and should not forbid projects or realms from further refinement of scopes. But I don't think we should leave it ONLY to projects.

For (2) I think that we have choices to discuss. I recognize that we are currently working on IUA, but I really want us to have these discussions so as to set a pattern. I don't think that using profile names in scopes is appropriate, I am not against a project from doing this, nor would I be against this use. I just don't think it is an appropriate pattern for us to recommend. I don't think it is a good recommendation as profiles are written around an interoperability need, not around a security or privacy need.

I do think that we should seriously consider the SMART scopes, and use them where we don't find them broken. I really would like to understand strongly a reason to not use them. The IHE process is very clear that we first must pick from available standards, we are only to create new standards when there is no standard that fits. SMART is a standard, it might seem it is a USA focused standard, but it meets the definition of a standard. Thus we must consider it, it is required that we consider it by the governance of the IHE process.

This said, there will be times when SMART does not fit. This is especially true as IUA is more focused on organization-to-organization flows, where as SMART is more focused on end-user to organization flow. I don't think that SMART should be considered prefect or always fit for use-case. However their focus on classic REST scope patterns is logical. What I do find very broken with SMART is that they rely on the shared context, where for patient scope I want things the 'patient/id' explicit in the scope too.

This all said.; I think we MUST work up what QEDm and MHD should say about grouping with IUA, and their declaration of scope.

joostreuzel commented 4 years ago

The SMART scopes are a good choice for profiles/transactions that are based on FHIR. However, I do think IUA should be independent of FHIR as it may used in combination with any HTTP based protocol such as DICOM-Web. As such I think that these scopes should be defined on a profile level. For example, QEDm, being FHIR-based, should indeed consider the SMART scopes.

Do I feel correctly that we are moving to a consensus of defining scopes on a per-profile level and allowing projects to add scopes as they see fit? The IUA could do a forward reference (as part of the role grouping) to those profiles indicating that scope usage is to be defined there...

JohnMoehrke commented 4 years ago

The SMART scopes are a good choice for profiles/transactions that are based on FHIR. However, I do think IUA should be independent of FHIR as it may used in combination with any HTTP based protocol such as DICOM-Web. As such I think that these scopes should be defined on a profile level. For example, QEDm, being FHIR-based, should indeed consider the SMART scopes.

What have I said that leads you to concern about this? I think I have been very clear that I agree with you... right?

Do I feel correctly that we are moving to a consensus of defining scopes on a per-profile level and allowing projects to add scopes as they see fit? The IUA could do a forward reference (as part of the role grouping) to those profiles indicating that scope usage is to be defined there...

IUA does not need to do a forward reference.

However the "supplement", which is the markdown we are working on CAN be the vehicle that writes these MHD and QEDm specific text into the MHD profile and QEDm profile. I am sensing that there is not good understanding of the distinction between profile and supplement. The fact that the IUA supplement includes MHD and QEDm changes does not mean that those changes are part of the IUA profile. a 'supplement' is effectively a big change-proposal.

msmock commented 4 years ago

I‘m not with you, but can accept any scope related text in IUA.b, as long it is not mandatory and can be extended with useful scope values in the swiss EPR. Neither transacton/profile related scopes, nor the smart on fhir scopes will work on long term in the Swiss EPR. Thus if we are not forced to use those scopes and are free to define Swiss specific scopes, it is OK for me. And as a footnote: I expect that the FHIR community will soon realize, that the SMART on FHIR scopes will not work, if it comes to a broader practical use.

Am 07.10.2020 um 16:36 schrieb John Moehrke notifications@github.com:

 The SMART scopes are a good choice for profiles/transactions that are based on FHIR. However, I do think IUA should be independent of FHIR as it may used in combination with any HTTP based protocol such as DICOM-Web. As such I think that these scopes should be defined on a profile level. For example, QEDm, being FHIR-based, should indeed consider the SMART scopes.

What have I said that leads you to concern about this? I think I have been very clear that I agree with you... right?

Do I feel correctly that we are moving to a consensus of defining scopes on a per-profile level and allowing projects to add scopes as they see fit? The IUA could do a forward reference (as part of the role grouping) to those profiles indicating that scope usage is to be defined there...

IUA does not need to do a forward reference.

However the "supplement", which is the markdown we are working on CAN be the vehicle that writes these MHD and QEDm specific text into the MHD profile and QEDm profile. I am sensing that there is not good understanding of the distinction between profile and supplement. The fact that the IUA supplement includes MHD and QEDm changes does not mean that those changes are part of the IUA profile. a 'supplement' is effectively a big change-proposal.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

JohnMoehrke commented 4 years ago

@msmock I think I understand you do NOT want any specification of scope in IUA. I agree

is that what you are "not with me" on?

or you not agreeing with @joostreuzel ? If so, what are you disagreeing with there... as far as I can tell @joostreuzel also does not want any scopes defined in IUA... so not sure what you are disagreeing with there

so I think all three of us agree that IUA should not include scope definitions.

I would be happy to move the QEDm and MHD discussion to another issue so that we can close this issue.

msmock commented 4 years ago

Ok, fine. I misunderstood the posts. Sorry for causing confusion.

Am 07.10.2020 um 20:11 schrieb John Moehrke notifications@github.com:

 @msmock I think I understand you do NOT want any specification of scope in IUA. I agree

is that what you are "not with me" on?

or you not agreeing with @joostreuzel ? If so, what are you disagreeing with there... as far as I can tell @joostreuzel also does not want any scopes defined in IUA... so not sure what you are disagreeing with there

so I think all three of us agree that IUA should not include scope definitions.

I would be happy to move the QEDm and MHD discussion to another issue so that we can close this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

joostreuzel commented 4 years ago

However the "supplement", which is the markdown we are working on CAN be the vehicle that writes these MHD and QEDm specific text into the MHD profile and QEDm profile. I am sensing that there is not good understanding of the distinction between profile and supplement. The fact that the IUA supplement includes MHD and QEDm changes does not mean that those changes are part of the IUA profile. a 'supplement' is effectively a big change-proposal.

My mistake. I know about profiles, but was unaware supplements could span these. As such, indeed, I think we all agree.

JohnMoehrke commented 3 years ago

Resolved as scopes are not defined in IUA, but rather are defined in the 'other' profile grouped with IUA. Such as MHD which the update to MHD is included in this supplement.