fix: add Epic vendor quirk to pass Epic-Client-ID #249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikix
commented
Jul 18, 2023
cumulus_etl/fhir/fhir_auth.py
Outdated
There was a problem hiding this comment.
There is nothing new or interesting in this file - I just broke out some of the auth abstraction and helpers into here, because fhir_client.py was starting to feel too big.
Comment on lines
+16
to
+19
| class ServerType(enum.Enum): | ||
| UNKNOWN = enum.auto() | ||
| CERNER = enum.auto() | ||
| EPIC = enum.auto() |
| file=sys.stderr, | ||
| ) | ||
| raise SystemExit(errors.BASIC_CREDENTIALS_MISSING) | ||
| async def _read_capabilities(self) -> None: |
Comment on lines
+193
to
+197
| # Epic wants to see the Epic-Client-ID header, especially for non-OAuth flows. | ||
| # (but I've heard reports of also wanting it in OAuth flows too) | ||
| # See https://fhir.epic.com/Documentation?docId=oauth2§ion=NonOauth_Epic-Client-ID-Header | ||
| if self._server_type == ServerType.EPIC and self._client_id: | ||
| headers["Epic-Client-ID"] = self._client_id |
mikix
force-pushed
the
mikix/epic-client-id-header
branch
from
57820a0
to
1a72e77
Compare
mikix
changed the title
dogversioning
approved these changes
Jul 19, 2023
Comment on lines
+171
to
+172
| # Assume utf8 is acceptable -- we should in theory also run these through Unicode normalization, in case they | ||
| # have interesting Unicode characters. But we can always add that in the future. |
There was a problem hiding this comment.
i cannot believe you're not just assuming that someone is putting emoji into their username!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've tested that this sends the header to the Epic sandbox and doesn't do it for the Cerner sandbox. But I have never experienced the case where Epic requires this header (and to get a user/password to test the non-OAuth flow that does require it would require asking that of the Epic security team). So I'm just going to assume this is great. We can hear from our partners on Epic about its efficacy. Seems harmless and likely to work (based on reports of them patching this in basically).
Checklist
docs/) needs to be updated