Refactor Bulk Data Auth specs to support imported SMART tests#739
Refactor Bulk Data Auth specs to support imported SMART tests#739abhinandan2012 wants to merge 6 commits intoonc-healthit:mainfrom
Conversation
|
@karlnaden After the latest commit (requirements coverage update), GitHub reset the workflow approval. Whenever convenient, could you please approve the workflow so CI can run? Thanks! |
karlnaden
left a comment
karlnaden
left a comment
There was a problem hiding this comment.
One piece of this that we need to be careful about the is the jwks set. Currently, g10 has its own jwks set that is hosts for these bulk auth tests, with a configuration key in the environment to use a different one.
SMART has its own jwks set, and a different configuration key to override it. It just so happens that the default jwks sets in the two repositories are the same, so everything is fine as long as neither default is changed and no updates are made to one but not the other. That seems like a risky situation to leave ourselves in, so I think we need to come up with some kind of solution.
One solution that probably would work would be to list the g10 jwks url in the jwks component of the bulk_smart_auth_info input. It is locked so it can't be changed and that would force the SMART logic to use the g10 configured jwks instead of its configured one.
spec/onc_certification_g10_test_kit/bulk_data_authorization_spec.rb
Outdated
Show resolved
Hide resolved
karlnaden
left a comment
karlnaden
left a comment
There was a problem hiding this comment.
Default URL wasn't correct. Please also provide instructions for how to test the case where the G10_BULK_DATA_JWKS environment variable is set to a different file that contains a different jwks than the current one in the g10 and SMART test kits. Otherwise, we won't be sure that the change to use the default jwks setting worked.
| inputs: { smart_auth_info: { name: :bulk_smart_auth_info } }, | ||
| outputs: { | ||
| smart_auth_info: { name: :bulk_smart_auth_info }, | ||
| authentication_response: { name: :authentication_response } |
There was a problem hiding this comment.
This line not needed since you are not renaming the input. The previous line is. Same issue in the next test. Remove both.
| }, | ||
| { | ||
| name: :jwks, | ||
| default: "#{Inferno::Application['base_url']}/custom/g10_certification/jwks", |
There was a problem hiding this comment.
wrong url
| default: "#{Inferno::Application['base_url']}/custom/g10_certification/jwks", | |
| default: "#{Inferno::Application['base_url']}/custom//g10_certification/.well-known/jwks.json", |
2 participants
Summary
This PR removes duplicated SMART Backend Services authorization tests from the (g)(10) Bulk Data Authorization group and imports the canonical SMART tests instead.
Background
The (g)(10) test kit previously reimplemented SMART Backend Services authorization logic, which caused spec drift and required fixes such as #721.
Changes
smart_auth_infoinputs/outputs to existingbulk_smart_auth_infoauthentication_responseinput directly, matching SMART test semanticsBenefits
Testing