FI-3220 Add scope and aud parameters as optional inputs for authorization code redirect test #16
Conversation
| ), | ||
| optional: true | ||
|
|
||
| input :udap_authorization_code_request_aud, |
There was a problem hiding this comment.
Is there a reason not to just use a checkbox input or configuration option for this?
There was a problem hiding this comment.
Yes, I originally attempted a checkbox but then there would have been two inputs for it: the checkbox input to use/not use the aud param and then the udap_fhir_base_url (originally input in the Discovery group) to get the actual value needed for the request. I thought it may be kind of confusing when running from the authorization and authentication group level (group 1.3), to have an input for the aud param value and then also a checkbox to opt to use it even though the value is already present, as opposed to "if you want to use this parameter, provide an input value for it; if it's empty the parameter won't be used," which felt simpler.
That said, I didn't really love any options I thought of for this input, and am open to cleaner suggestions.
There was a problem hiding this comment.
I don't think this should be a distinct input from the base url. So I think I would vote for a checkbox plus base url input.
There was a problem hiding this comment.
I updated the tests to use the checkbox and base url inputs.
Summary
Currently, the authorization code redirect test has no means of providing a
scopeparameter to the authorization endpoint. However, RFC 6749 Section 4.1.1, as referenced in the HL7 UDAP STU 1.0 IG, states that inclusion of ascopeparameter is optional, and testers should therefore be able to provide it as an optional input.This PR adds an optional
scopeinput for the authorization code redirect test. It also adds an optional input for theaudparameter, which is not mentioned in the HL7 UDAP IG or RFC 6749 but is required for SMART App Launch. These changes are effectively doing the same thing as the authorization redirect test created in the SMART-UDAP Harmonization Test Kit.My justification for including the
audparameter here, despite it not being required by RFC 6749 or the HL7 UDAP IG, is that it would reduce duplicated code across test kits. If this PR is merged, then the SMART-UDAP test kit can just import this updated test from the baseline UDAP test kit instead of having its own redirect test. It would also make the redirect test more reusable in other UDAP test contexts where an implementation is likely to support both SMART and UDAP.If we really don't want to allow testers to add an
audparameter in this test kit, we can always configure that input to be locked at the group level.Testing Guidance
Unit tests were updated to account for changes. We don't have a reference implementation to run against, but inputs can be seen in the web UI.