Skip to content

Commit

Permalink
Merge pull request #254 from smart-on-fhir/mikix/loosen-up-on-auth
Browse files Browse the repository at this point in the history
fix: don't be so strict when connecting to a FHIR server
  • Loading branch information
mikix authored Jul 27, 2023
2 parents adb6291 + c1cdd44 commit cf310fc
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 16 deletions.
21 changes: 11 additions & 10 deletions cumulus_etl/fhir/fhir_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,17 @@ async def _get_token_endpoint(self, session: httpx.AsyncClient) -> str:
timeout=300, # five minutes
)
response.raise_for_status()

# Validate that the server can talk the client-confidential-asymmetric protocol with us.
# Some servers (like Cerner) don't advertise their support with the 'client-confidential-asymmetric'
# capability keyword, so let's not bother checking for it. But we can confirm that the pieces are there.
config = response.json()
if "private_key_jwt" not in config.get("token_endpoint_auth_methods_supported", []) or not config.get(
"token_endpoint"
):

# We used to validate some other pieces of this response (like support for the 'client-confidential-asymmetric'
# capability keyword or 'private_key_jwt' in the 'token_endpoint_auth_methods_supported' field).
# But servers rarely advertise correctly (No one seems to use the asymmetric capability keyword and Veradigm
# doesn't fill out the endpoint auth methods field with all methods it supports).
# So :shrug: we'll just assume things are fine and error out later if they aren't fine.
# The only thing we _need_ is the token endpoint.
if not config.get("token_endpoint"):
errors.fatal(
f"Server {self._server_root} does not support the client-confidential-asymmetric protocol",
f"Server {self._server_root} does not expose an OAuth token endpoint",
errors.FHIR_AUTH_FAILED,
)

Expand All @@ -140,10 +141,10 @@ def _make_signed_jwt(self) -> str:
"""
# Find a usable singing JWK from JWKS
for key in self._jwks.get("keys", []):
if key.get("alg") in ["ES384", "RS384"] and "sign" in key.get("key_ops", []) and key.get("kid"):
if "sign" in key.get("key_ops", []) and key.get("kid") and key.get("alg"):
break
else: # no valid private JWK found
raise errors.FatalError("No private ES384 or RS384 key found in the provided JWKS file.")
raise errors.FatalError("No valid private key found in the provided JWKS file.")

# Now generate a signed JWT based off the given JWK
header = {
Expand Down
10 changes: 4 additions & 6 deletions tests/fhir/test_fhir_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,17 @@ async def test_get_with_overriden_header(self):

@ddt.data(
{}, # no keys
{"keys": [{"alg": "RS384"}]}, # no key op
{"keys": [{"alg": "RS384", "key_ops": ["verify"], "kid": "a"}]}, # bad key op
{"keys": [{"alg": "RS128", "key_ops": ["sign"]}], "kid": "a"}, # bad algo
{"keys": [{"key_ops": ["sign"], "kid": "a"}]}, # no alg
{"keys": [{"alg": "RS384", "kid": "a"}]}, # no key op
{"keys": [{"alg": "RS384", "key_ops": ["sign"]}]}, # no kid
{"keys": [{"alg": "RS384", "key_ops": ["verify"], "kid": "a"}]}, # bad key op
)
async def test_jwks_without_suitable_key(self, bad_jwks):
with self.assertRaisesRegex(errors.FatalError, "No private ES384 or RS384 key found"):
with self.assertRaisesRegex(errors.FatalError, "No valid private key found"):
async with fhir.FhirClient(self.server_url, [], smart_client_id=self.client_id, smart_jwks=bad_jwks):
pass

@ddt.data(
{"token_endpoint_auth_methods_supported": None},
{"token_endpoint_auth_methods_supported": ["nope"]},
{"token_endpoint": None},
{"token_endpoint": ""},
)
Expand Down

0 comments on commit cf310fc

Please sign in to comment.