From 9bd9b816c823d2fd9e6b322541b1d5d1f0679e23 Mon Sep 17 00:00:00 2001 From: Troy Biesterfeld Date: Fri, 9 Apr 2021 17:00:35 -0500 Subject: [PATCH] Issue #1961 - Optionally reject references without resource type Signed-off-by: Troy Biesterfeld --- docs/src/pages/guides/FHIRServerUsersGuide.md | 4 +- .../json/spec/claimresponse-example.json | 2 +- ...ligibilityresponse-example-benefits-2.json | 2 +- .../coverageeligibilityresponse-example.json | 2 +- .../json/spec/enrollmentresponse-example.json | 2 +- .../spec/paymentreconciliation-example.json | 4 +- .../xml/spec/claimresponse-example.xml | 2 +- ...eligibilityresponse-example-benefits-2.xml | 2 +- .../coverageeligibilityresponse-example.xml | 2 +- .../xml/spec/enrollmentresponse-example.xml | 2 +- .../spec/paymentreconciliation-example.xml | 4 +- .../fhir/model/util/ValidationSupport.java | 84 +++++----- .../model/test/CheckReferenceTypeTest.java | 150 ++++++++++++++++-- .../ibm/fhir/server/test/ReferenceTest.java | 49 ++++++ 14 files changed, 246 insertions(+), 65 deletions(-) create mode 100644 fhir-server-test/src/test/java/com/ibm/fhir/server/test/ReferenceTest.java diff --git a/docs/src/pages/guides/FHIRServerUsersGuide.md b/docs/src/pages/guides/FHIRServerUsersGuide.md index 19ddda25b5a..31aedda0c5c 100644 --- a/docs/src/pages/guides/FHIRServerUsersGuide.md +++ b/docs/src/pages/guides/FHIRServerUsersGuide.md @@ -3,7 +3,7 @@ layout: post title: IBM FHIR Server User's Guide description: IBM FHIR Server User's Guide Copyright: years 2017, 2021 -lastupdated: "2021-03-10" +lastupdated: "2021-04-09" permalink: /FHIRServerUsersGuide/ --- @@ -2592,7 +2592,7 @@ For more information about topics related to configuring a FHIR server, see the - 5 - An external reference is a reference to a resource which is meaningful outside a particular request bundle. The value typically includes the resource type and the resource identifier, and could be an absolute or relative URL. Examples: `https://fhirserver1:9443/fhir-server/api/v4/Patient/12345`, `Patient/12345`, etc. [↩](#a5) + An external reference is a reference to a resource which is meaningful outside a particular request bundle. The value must include the resource type and the resource identifier, and could be an absolute or relative URL. Examples: `https://fhirserver1:9443/fhir-server/api/v4/Patient/12345`, `Patient/12345`, etc. [↩](#a5) - 6 diff --git a/fhir-examples/src/main/resources/json/spec/claimresponse-example.json b/fhir-examples/src/main/resources/json/spec/claimresponse-example.json index be2e9e70e7a..ba57364a064 100644 --- a/fhir-examples/src/main/resources/json/spec/claimresponse-example.json +++ b/fhir-examples/src/main/resources/json/spec/claimresponse-example.json @@ -43,7 +43,7 @@ "reference": "Organization/1" }, "request": { - "reference": "http://www.BenefitsInc.com/fhir/oralhealthclaim/15476332402" + "reference": "http://www.BenefitsInc.com/fhir/Claim/15476332402" }, "outcome": "complete", "disposition": "Claim settled as per contract.", diff --git a/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example-benefits-2.json b/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example-benefits-2.json index cbebdc60521..f87521c8f5f 100644 --- a/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example-benefits-2.json +++ b/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example-benefits-2.json @@ -107,7 +107,7 @@ } }, "request": { - "reference": "http://www.BenefitsInc.com/fhir/coverageeligibilityrequest/225476332405" + "reference": "http://www.BenefitsInc.com/fhir/CoverageEligibilityRequest/225476332405" }, "outcome": "complete", "disposition": "Policy is currently in-force.", diff --git a/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example.json b/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example.json index e8cf8529f61..45a78b17533 100644 --- a/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example.json +++ b/fhir-examples/src/main/resources/json/spec/coverageeligibilityresponse-example.json @@ -20,7 +20,7 @@ }, "created": "2014-08-16", "request": { - "reference": "http://www.BenefitsInc.com/fhir/coverageeligibilityrequest/225476332402" + "reference": "http://www.BenefitsInc.com/fhir/CoverageEligibilityRequest/225476332402" }, "outcome": "complete", "disposition": "Policy is currently in-force.", diff --git a/fhir-examples/src/main/resources/json/spec/enrollmentresponse-example.json b/fhir-examples/src/main/resources/json/spec/enrollmentresponse-example.json index d1e7338eb5f..b943bf19b89 100644 --- a/fhir-examples/src/main/resources/json/spec/enrollmentresponse-example.json +++ b/fhir-examples/src/main/resources/json/spec/enrollmentresponse-example.json @@ -13,7 +13,7 @@ ], "status": "active", "request": { - "reference": "http://www.BenefitsInc.com/fhir/eligibility/225476332402" + "reference": "http://www.BenefitsInc.com/fhir/EnrollmentRequest/225476332402" }, "outcome": "complete", "disposition": "Dependant added to policy.", diff --git a/fhir-examples/src/main/resources/json/spec/paymentreconciliation-example.json b/fhir-examples/src/main/resources/json/spec/paymentreconciliation-example.json index 1dd7e7d334f..8e7849cbffd 100644 --- a/fhir-examples/src/main/resources/json/spec/paymentreconciliation-example.json +++ b/fhir-examples/src/main/resources/json/spec/paymentreconciliation-example.json @@ -21,7 +21,7 @@ "reference": "Organization/2" }, "request": { - "reference": "http://www.BenefitsInc.com/fhir/eligibility/225476332402" + "reference": "http://www.BenefitsInc.com/fhir/Task/225476332402" }, "requestor": { "reference": "Organization/1" @@ -89,7 +89,7 @@ ] }, "request": { - "reference": "http://www.BenefitsInc.com/fhir/oralhealthclaim/225476332699" + "reference": "http://www.BenefitsInc.com/fhir/Claim/225476332699" }, "date": "2014-08-12", "amount": { diff --git a/fhir-examples/src/main/resources/xml/spec/claimresponse-example.xml b/fhir-examples/src/main/resources/xml/spec/claimresponse-example.xml index 4d92569b025..6baa72d9bf7 100644 --- a/fhir-examples/src/main/resources/xml/spec/claimresponse-example.xml +++ b/fhir-examples/src/main/resources/xml/spec/claimresponse-example.xml @@ -55,7 +55,7 @@ - + diff --git a/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example-benefits-2.xml b/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example-benefits-2.xml index 3b53c228256..6e9c4f196fb 100644 --- a/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example-benefits-2.xml +++ b/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example-benefits-2.xml @@ -112,7 +112,7 @@ - + diff --git a/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example.xml b/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example.xml index 561d64b82dd..00d18181b12 100644 --- a/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example.xml +++ b/fhir-examples/src/main/resources/xml/spec/coverageeligibilityresponse-example.xml @@ -29,7 +29,7 @@ - + diff --git a/fhir-examples/src/main/resources/xml/spec/enrollmentresponse-example.xml b/fhir-examples/src/main/resources/xml/spec/enrollmentresponse-example.xml index 260afb5c586..c5ae05a7dfe 100644 --- a/fhir-examples/src/main/resources/xml/spec/enrollmentresponse-example.xml +++ b/fhir-examples/src/main/resources/xml/spec/enrollmentresponse-example.xml @@ -21,7 +21,7 @@ - + diff --git a/fhir-examples/src/main/resources/xml/spec/paymentreconciliation-example.xml b/fhir-examples/src/main/resources/xml/spec/paymentreconciliation-example.xml index c6519685448..cbeac752d78 100644 --- a/fhir-examples/src/main/resources/xml/spec/paymentreconciliation-example.xml +++ b/fhir-examples/src/main/resources/xml/spec/paymentreconciliation-example.xml @@ -32,7 +32,7 @@ - + @@ -104,7 +104,7 @@ - + diff --git a/fhir-model/src/main/java/com/ibm/fhir/model/util/ValidationSupport.java b/fhir-model/src/main/java/com/ibm/fhir/model/util/ValidationSupport.java index bde79a71baf..3f2e45d7bf1 100644 --- a/fhir-model/src/main/java/com/ibm/fhir/model/util/ValidationSupport.java +++ b/fhir-model/src/main/java/com/ibm/fhir/model/util/ValidationSupport.java @@ -1,5 +1,5 @@ /* - * (C) Copyright IBM Corp. 2019, 2020 + * (C) Copyright IBM Corp. 2019, 2021 * * SPDX-License-Identifier: Apache-2.0 */ @@ -55,6 +55,7 @@ public final class ValidationSupport { private static final int RESOURCE_TYPE_GROUP = 4; private static final int MIN_STRING_LENGTH = 1; private static final int MAX_STRING_LENGTH = 1048576; // 1024 * 1024 = 1MB + private static final String LOCAL_REF_PREFIX = "urn:"; private static final String FHIR_XHTML_XSD = "fhir-xhtml.xsd"; private static final String FHIR_XML_XSD = "xml.xsd"; private static final String FHIR_XMLDSIG_CORE_SCHEMA_XSD = "xmldsig-core-schema.xsd"; @@ -672,54 +673,57 @@ public static void checkReferenceType(Element choiceElement, String elementName, } /** + * Checks that the reference contains valid resource type values. + * @param reference the reference + * @param elementName the element name + * @param referenceTypes the valid resource types for the reference * @throws IllegalStateException if the resource type found in the reference value does not match the specified Reference.type value * or is not one of the allowed reference types for that element */ public static void checkReferenceType(Reference reference, String elementName, String... referenceTypes) { - boolean checkReferenceTypes = FHIRModelConfig.getCheckReferenceTypes(); - if (reference != null && checkReferenceTypes) { - String referenceType = getReferenceType(reference); - if (referenceType != null && !ModelSupport.isResourceType(referenceType)) { - throw new IllegalStateException( - String.format("Resource type found in Reference.type: '%s' for element: '%s' must be a valid resource type name", - referenceType, elementName)); - } - List referenceTypeList = Arrays.asList(referenceTypes); - - // If there is an explicit Reference.type, ensure its an allowed type - if (referenceType != null && !referenceTypeList.contains(referenceType)) { - throw new IllegalStateException( - String.format("Resource type found in Reference.type: '%s' for element: '%s' must be one of: %s", - referenceType, elementName, referenceTypeList.toString())); - } - - String referenceReference = getReferenceReference(reference); - String resourceType = null; - - if (referenceReference != null && !referenceReference.startsWith("#")) { - Matcher matcher = REFERENCE_PATTERN.matcher(referenceReference); - if (matcher.matches()) { - resourceType = matcher.group(RESOURCE_TYPE_GROUP); - // If there is an explicit Reference.type, check that the resourceType pattern matches it - if (referenceType != null && !resourceType.equals(referenceType)) { - throw new IllegalStateException( - String.format("Resource type found in reference value: '%s' for element: '%s' does not match Reference.type: %s", - referenceReference, elementName, referenceType)); - } - } + if (reference == null || !FHIRModelConfig.getCheckReferenceTypes()) { + return; + } + + String resourceType = null; + String referenceReference = getReferenceReference(reference); + List referenceTypeList = Arrays.asList(referenceTypes); + + if (referenceReference != null && !referenceReference.startsWith("#") && !referenceReference.startsWith(LOCAL_REF_PREFIX)) { + Matcher matcher = REFERENCE_PATTERN.matcher(referenceReference); + if (matcher.matches()) { + resourceType = matcher.group(RESOURCE_TYPE_GROUP); } + // resourceType is required in the reference value if (resourceType == null) { - resourceType = referenceType; + throw new IllegalStateException(String.format("Resource type not found in reference value: '%s' for element: '%s'", referenceReference, elementName)); } - // If we've successfully inferred a type, check that its an allowed value - if (resourceType != null) { - if (!referenceTypeList.contains(resourceType)) { - throw new IllegalStateException( - String.format("Resource type found in reference value: '%s' for element: '%s' must be one of: %s", - referenceReference, elementName, referenceTypeList.toString())); - } + if (!ModelSupport.isResourceType(resourceType)) { + throw new IllegalStateException(String.format("Resource type found in reference value: '%s' for element: '%s' must be a valid resource type name", referenceReference, elementName)); + } + + // If there is a resourceType in the reference value, check that it's an allowed value + if (!referenceTypeList.contains(resourceType)) { + throw new IllegalStateException(String.format("Resource type found in reference value: '%s' for element: '%s' must be one of: %s", referenceReference, elementName, referenceTypeList.toString())); + } + } + + String referenceType = getReferenceType(reference); + if (referenceType != null) { + if (!ModelSupport.isResourceType(referenceType)) { + throw new IllegalStateException(String.format("Resource type found in Reference.type: '%s' for element: '%s' must be a valid resource type name", referenceType, elementName)); + } + + // If there is an explicit Reference.type, ensure it's an allowed type + if (!referenceTypeList.contains(referenceType)) { + throw new IllegalStateException(String.format("Resource type found in Reference.type: '%s' for element: '%s' must be one of: %s", referenceType, elementName, referenceTypeList.toString())); + } + + // If there is an explicit Reference.type, check that the resourceType pattern matches it + if (resourceType != null && !resourceType.equals(referenceType)) { + throw new IllegalStateException(String.format("Resource type found in reference value: '%s' for element: '%s' does not match Reference.type: %s", referenceReference, elementName, referenceType)); } } } diff --git a/fhir-model/src/test/java/com/ibm/fhir/model/test/CheckReferenceTypeTest.java b/fhir-model/src/test/java/com/ibm/fhir/model/test/CheckReferenceTypeTest.java index 30d248e655f..27bbd6c7115 100644 --- a/fhir-model/src/test/java/com/ibm/fhir/model/test/CheckReferenceTypeTest.java +++ b/fhir-model/src/test/java/com/ibm/fhir/model/test/CheckReferenceTypeTest.java @@ -1,5 +1,5 @@ /* - * (C) Copyright IBM Corp. 2020 + * (C) Copyright IBM Corp. 2020, 2021 * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,6 +7,7 @@ package com.ibm.fhir.model.test; import static com.ibm.fhir.model.type.String.string; +import static com.ibm.fhir.model.type.Uri.uri; import static org.testng.Assert.fail; import java.io.Reader; @@ -24,11 +25,22 @@ public class CheckReferenceTypeTest { @Test public void testCheckReferenceType() throws Exception { boolean originalSetting = FHIRModelConfig.getCheckReferenceTypes(); - - FHIRModelConfig.setCheckReferenceTypes(true); + try (Reader reader = ExamplesUtil.resourceReader("json/ibm/minimal/Observation-1.json")) { + FHIRModelConfig.setCheckReferenceTypes(true); Observation observation = FHIRParser.parser(Format.JSON).parse(reader); - + + // valid + try { + observation.toBuilder() + .subject(Reference.builder() + .type(uri("Patient")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + // valid try { observation.toBuilder() @@ -39,7 +51,30 @@ public void testCheckReferenceType() throws Exception { } catch (IllegalStateException e) { fail(); } - + + // valid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("Patient/1234")) + .type(uri("Patient")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + + // valid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("urn:uuid:7113a0bb-d9e0-49df-9855-887409388c69")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + // invalid try { observation.toBuilder() @@ -50,10 +85,56 @@ public void testCheckReferenceType() throws Exception { fail(); } catch (IllegalStateException e) { } - + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("1234")) + .build()) + .build(); + fail(); + } catch (IllegalStateException e) { + } + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("Patient/1234")) + .type(uri("Condition")) + .build()) + .build(); + fail(); + } catch (IllegalStateException e) { + } + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("Patient/1234")) + .type(uri("Group")) + .build()) + .build(); + fail(); + } catch (IllegalStateException e) { + } + // turn off reference type checking FHIRModelConfig.setCheckReferenceTypes(false); - + + // valid + try { + observation.toBuilder() + .subject(Reference.builder() + .type(uri("Patient")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + // valid try { observation.toBuilder() @@ -64,7 +145,18 @@ public void testCheckReferenceType() throws Exception { } catch (IllegalStateException e) { fail(); } - + + // valid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("urn:uuid:7113a0bb-d9e0-49df-9855-887409388c69")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + // invalid try { observation.toBuilder() @@ -75,9 +167,45 @@ public void testCheckReferenceType() throws Exception { } catch (IllegalStateException e) { fail(); } + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("1234")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("Patient/1234")) + .type(uri("Condition")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + + // invalid + try { + observation.toBuilder() + .subject(Reference.builder() + .reference(string("Patient/1234")) + .type(uri("Group")) + .build()) + .build(); + } catch (IllegalStateException e) { + fail(); + } + + } finally { + // Restore the original config setting to be as unobtrusive as possible + FHIRModelConfig.setCheckReferenceTypes(originalSetting); } - - // Restore the original config setting to be as unobtrusive as possible - FHIRModelConfig.setCheckReferenceTypes(originalSetting); } } diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ReferenceTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ReferenceTest.java new file mode 100644 index 00000000000..a529c7830f9 --- /dev/null +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ReferenceTest.java @@ -0,0 +1,49 @@ +/* + * (C) Copyright IBM Corp. 2021 + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.ibm.fhir.server.test; + +import static com.ibm.fhir.model.type.String.string; + +import javax.ws.rs.client.Entity; +import javax.ws.rs.client.WebTarget; +import javax.ws.rs.core.Response; + +import org.testng.annotations.Test; + +import com.ibm.fhir.core.FHIRMediaType; +import com.ibm.fhir.model.config.FHIRModelConfig; +import com.ibm.fhir.model.resource.Patient; +import com.ibm.fhir.model.test.TestUtil; +import com.ibm.fhir.model.type.Reference; + +/** + * This class contains tests for reference checking. + */ +public class ReferenceTest extends FHIRServerTestBase { + + @Test(groups = { "server-reference" }) + public void testReferenceWithNoResourceType() throws Exception { + WebTarget target = getWebTarget(); + Patient patient = TestUtil.readLocalResource("Patient_JohnDoe.json"); + + boolean originalSetting = FHIRModelConfig.getCheckReferenceTypes(); + try { + // Temporarily set setCheckReferenceTypes to false so reference type checking is skipped + FHIRModelConfig.setCheckReferenceTypes(false); + patient = patient.toBuilder() + .managingOrganization(Reference.builder().reference(string("1234")).build()) + .build(); + } finally { + FHIRModelConfig.setCheckReferenceTypes(originalSetting); + } + + // Request should fail since the server will perform the reference type checking + Entity entity = Entity.entity(patient, FHIRMediaType.APPLICATION_FHIR_JSON); + Response response = target.path("Patient").request().post(entity, Response.class); + assertResponse(response, Response.Status.BAD_REQUEST.getStatusCode()); + } +}