Stream: bulk data
Topic: Expected scope negotiation behavior
Vladimir Ignatov (Sep 15 2021 at 16:53):
In Backend Services Authentication a client could request a system/Patient.* scope, but it is not explicitly described what the expected behavior of the servers should be in this case. The possible outcomes are:
- The request is rejected because it implies write access and a client should only have read access
- The scope is converted under the hood to
system/Patient.read - The scope is rejected/ignored and not granted (but you don't get an error)
- Other?
The reason for asking is that there is a potentially incorrect test in Inferno that asks for system/Patient.* and expects that to NOT result in an error.
Also note that many clients would request system/*.read or even system/*.*, expecting to be granted whatever specific scopes are available to them (for example system/*.read => system/Patient.read, system/Observation.read.
There are even some Bulk Data servers that requre clients to request sustem/*.*. That is not correct in my opinion, but that is the current state of things.
Lee Surprenant (Sep 15 2021 at 17:05):
To be clear, in Keycloak a client app can only request the scopes they are allowed to request. If they ask for a scope that they are not allowed to, the server will reject the request (number 1 above). It makes sense to me that some implementations would want to grant a tighter set of scopes instead (number 2 above), but I think that a server should be allowed to reject a request with overly broad scopes (like write access when only read access is allowed). Inferno Auth-14: Token endpoint supports wildcard action scopes does not allow that.
Josh Mandel (Sep 15 2021 at 17:50):
(1), (2), and (3) are all valid behaviors according to our current spec.
Vladimir Ignatov (Sep 15 2021 at 17:53):
OK, then the test should not expect system/Patient.* to be handled without an error.
valid behaviors according to our current spec
Where is that described?
Josh Mandel (Sep 15 2021 at 17:56):
As with any requested scope, the scopes ultimately granted by the authorization server may differ from the scopes requested by the client! When dealing with wildcard clinical scope requests, this is often true.
http://build.fhir.org/ig/HL7/smart-app-launch/scopes-and-launch-context.html
Josh Mandel (Sep 15 2021 at 17:56):
That's probably the most explicit statement.
Josh Mandel (Sep 15 2021 at 17:56):
We don't have specific guidance on rejection.
Yunwei Wang (Sep 16 2021 at 13:56):
@Vladimir Ignatov
test in Inferno that asks for system/Patient.* and expects that to NOT result in an error.
Can you clarify if this is in Inferno Program or Inferno Community?
Vladimir Ignatov (Sep 16 2021 at 14:05):
I think it was in community but @Lee Surprenant should confirm that. What is the difference though - aren't they using the same test suite?
I intended to make some commits/PRs for the few issues we discovered during the connectathon but I don't have the time to do it yet.
What is the right way to do this now? Are you using tests from https://github.com/smart-on-fhir/bdt or do you have your own fork?
Lee Surprenant (Sep 16 2021 at 14:06):
Yes, community edition
Vladimir Ignatov (Sep 16 2021 at 14:16):
For this issue we have 2 choices.
- Remove the test
- Modify it to request
system/*.readinstead. That would match the test name and description.
As far as I can recall, one of the CMS servers wasn't accepting system/*.read and was insisting for system/*.* instead. Such a server would fail if we choose option 2, but that is incorrect implementation anyway. What do you think?
Yunwei Wang (Sep 16 2021 at 14:21):
Inferno Program use its own testing module while Inferno Community wraps around bdt. In Inferno Program, we test system/*.read and make this scope configurable so the tester could change the scope before testing.
Vladimir Ignatov (Sep 16 2021 at 14:29):
To be clear, there is a default "scope" option in bdt. It is for the scope that should be used in any auth requests during the test flow. However, in this particular test the scope is intentionally hard-coded to system/Patient.*.
In other words, the scope option allows the tester to set the default scope to anything (like system/*.*) so that other Bulk Data tests are executed even if the authentication part is not using the standard system/*.read.
Lee Surprenant (Jan 11 2022 at 20:18):
Our implementation is still failing this inferno test. Is there anything we can do to help move this one forward? Maybe open an issue on the inferno-community and/or bdt repos?
Dan Gottlieb (Jan 11 2022 at 22:57):
Yup, if wildcard scope support isn't required in the spec, seems like there shouldn't be core tests around it. Can you open an issue at https://github.com/smart-on-fhir/bdt ?
Dan Gottlieb (Jan 11 2022 at 22:57):
Relatedly, are there tests around scope that would be helpful in dbt? For example, is it worth having implementations indicate the scopes they think they support for this client_id in the config and then testing those?
Lee Surprenant (Jan 12 2022 at 00:17):
vlad seems to have gotten to this one too (awesome!): https://github.com/smart-on-fhir/bdt/issues/6
Last updated: Apr 12 2022 at 19:14 UTC
