Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FI-3531 Add optional community parameter input #20

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

alisawallace
Copy link
Collaborator

Summary

This PR adds an optional input for the community query to the .well-known endpoint in the Discovery step. Server support for this is optional per the IG, but it provides testers extra convenience by allowing them to specify which certs the server should use for the signed_metadata element.

Testing Guidance

Unit tests were not updated, since they are using stubbed requests. However, you can verify the changes work as intended in the web app against a reference server:

  1. Run tests from Group 1.1 or 2.1 Discovery Tests (either flow group)
  2. To test proper inclusion of the community parameter, run tests with the following inputs:
    • FHIR Server Base URL: https://identity-matching.fast.hl7.org/fhir
    • UDAP Community Parameter: udap://stage.healthtogo.me/
  3. Run again without the community input to verify it's properly omitted

All tests but the last should pass in both cases and the proper URL should be displayed under the Requests section of test 1 UDAP Well-Known configuration is available.

Copy link
Contributor

@Jammjammjamm Jammjammjamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-01-08 at 10 30 35 AM

This functionality seems fine, but this checkbox doesn't appear to actually be locked. Should it be a checkbox rather than a radio button?

@alisawallace
Copy link
Collaborator Author

Screenshot 2025-01-08 at 10 30 35 AM

This functionality seems fine, but this checkbox doesn't appear to actually be locked. Should it be a checkbox rather than a radio button?

I had noticed that earlier and meant to ask about it on the Slack channel - it's not clear to me if this is a UI issue or not, because the input is definitely locked in the configuration. And do you mean should it be a radio button instead of checkbox (because it currently is a checkbox)? If so, then no, because the server metadata can support one or both flow types; if the Discovery requirements were assessed independently (rather than bundled as the first step of the full OAuth flow as we do here), then technically the IG doesn't indicate which grant type(s) need to be supported, it could be either. If neither checkbox is selected, then the test will just verify that at least one grant type is supported. I added the checkbox and then locked it to enforce that the grant types advertised align with what will be used in the subsequent steps when all the tests are bundled like this. Hope that makes sense.

@alisawallace alisawallace merged commit cbe7cd1 into main Jan 8, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants