fhir-data-pipes icon indicating copy to clipboard operation
fhir-data-pipes copied to clipboard

Affected Issue(s): Extend OAuth 2.0 support

Open muhammad-levi opened this issue 1 year ago • 4 comments
trafficstars

Description of what I changed

Resolves https://github.com/google/fhir-data-pipes/issues/1193, https://github.com/sid-indonesia/it-team/issues/333

What this commit has achieved:

  1. Added support for Body Authentication on Client Credentials flow
  2. TODO implement OAuth 2.0 for sink FHIR Server.

E2E test

TESTED:

Please replace this with a description of how you tested your PR beyond the automated e2e/unit tests.

Checklist: I completed these to help reviewers :)

  • [ ] I have read and will follow the review process.

  • [ ] I am familiar with Google Style Guides for the language I have coded in.

    No? Please take some time and review Java and Python style guides.

  • [ ] My IDE is configured to follow the Google code styles.

    No? Unsure? -> configure your IDE.

  • [ ] I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)

  • [ ] I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.

  • [ ] All new and existing tests passed.

  • [ ] My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

muhammad-levi avatar Sep 13 '24 09:09 muhammad-levi

Thanks @muhammad-levi for adding body-authentication option to the client-credentials flow. I think the general approach you are taking is fine. You should be able to catch/fix the compilation failure issue (I think you need to update FetchUtilTest.java). Please feel free to remove the Draft tag and assign this to me for review, once it is ready.

bashir2 avatar Sep 19 '24 17:09 bashir2

BTW, I cannot see https://github.com/sid-indonesia/it-team/issues/333; it would be nice to file an issue in this repo and reference it in your PR.

bashir2 avatar Sep 19 '24 18:09 bashir2

@bashir2 Okay, thank you. I was planning to add the OAuth 2.0 support to the sink FHIR Server as well, but not doing it yet as other higher priority tasks arise.

BTW, I cannot see https://github.com/sid-indonesia/it-team/issues/333; it would be nice to file an issue in this repo and reference it in your PR.

Alright, I will file an issue in this repo.

muhammad-levi avatar Sep 20 '24 02:09 muhammad-levi

@bashir2 Okay, thank you. I was planning to add the OAuth 2.0 support to the sink FHIR Server as well, but not doing it yet as other higher priority tasks arise.

If you can test this in your local environment (with the custom FHIR server you have) that should be enough for now (plus proper unit-tests). We don't have e2e test coverage for other authentication methods either. We should add them at some point but I don't want to hold this PR on that, if you need this feature now. Adding OAuth support to the test sink FHIR server is probably not trivial as we need other components too (e.g., token issuer).

bashir2 avatar Sep 20 '24 18:09 bashir2