onc-healthit / onc-certification-g10-test-kit

ONC Certification (g)(10) Standardized API Tests
Apache License 2.0
34 stars 11 forks source link

SMART Launch Refresh Token Bug? #522

Closed joshuahiltunen closed 3 weeks ago

joshuahiltunen commented 3 months ago

Test 1.6.05 indicates "OAuth token exchange response body contains patient context and patient resource can be retrieved." I'm testing an issue where we're not producing patient context in the refresh token exchange, however Inferno indicated everything was all good. If you look at the request (and response) associated with that test, you see that the token omits the "patient" property. Is this correct? The examples in the STU documentation that the test links to also show the "patient" coming back in refresh token exchanges.

Session: https://inferno.healthit.gov/suites/g10_certification/iySRv3EHCy7#1

image

Jammjammjamm commented 3 months ago

This title is misleading. According to the spec:

if the app was launched from within a patient context, parameters to communicate the context values MAY BE included.

So, I believe the test behavior is correct in passing even though the patient context parameters are not present, and accessing the patient resource is done here to verify that the new access token works.

We will update the test's title to match the behavior.

joshuahiltunen commented 3 months ago

I think that's incorrect. This section of the Inferno test suite is the 'Standalone Patient App' section, and you fail in the preceding test 1.4.06 if the "patient" doesn't come back. You're using the refresh token from the earlier request, but you get different information. In other words, you've already established that you have a patient context associated with the token information you have. But you're judging the exchange of refresh token for access token without acknowledging that.

I'd agree if you were changing the scopes that you were asking for in the refresh token => access token exchange. But you're not. The test is still trying to obtain 'launch/patient', which it was successfully able to obtain in the earlier request in the collection.

image

Jammjammjamm commented 3 months ago

In 1.4.06 it is necessary for the patient context to be sent with the initial access token request.

You're using the refresh token from the earlier request, but you get different information. In other words, you've already established that you have a patient context associated with the token information you have. But you're judging the exchange of refresh token for access token without acknowledging that.

I don't understand what you're getting at here.

I'd agree if you were changing the scopes that you were asking for in the refresh token => access token exchange. But you're not. The test is still trying to obtain 'launch/patient', which it was successfully able to obtain in the earlier request in the collection.

Regardless of whether launch/patient is being requested, the patient context doesn't change, and it does not need to be sent when refreshing the token.