Merge branch 'aw/add-exception-class' of https://github.com/CMSgov/dpc-app into aw/add-exception-class

ashley-weaver · ashley-weaver · commit 4e1c3cbb194b · 2025-12-16T14:46:32.000-05:00
diff --git a/dpc-api/pom.xml b/dpc-api/pom.xml
@@ -244,7 +244,7 @@
         <dependency>
             <groupId>io.netty</groupId>
             <artifactId>netty-codec-http</artifactId>
-            <version>4.2.5.Final</version>
+            <version>4.2.8.Final</version>
         </dependency>
         <dependency>
             <groupId>io.swagger</groupId>
diff --git a/dpc-api/src/main/java/gov/cms/dpc/api/auth/macaroonauth/MacaroonsAuthenticator.java b/dpc-api/src/main/java/gov/cms/dpc/api/auth/macaroonauth/MacaroonsAuthenticator.java
@@ -3,11 +3,13 @@
 import ca.uhn.fhir.rest.client.api.IGenericClient;
 import gov.cms.dpc.api.auth.DPCAuthCredentials;
 import gov.cms.dpc.api.auth.OrganizationPrincipal;
+import gov.cms.dpc.api.auth.annotations.PathAuthorizer;
 import gov.cms.dpc.fhir.DPCIdentifierSystem;
 import gov.cms.dpc.fhir.DPCResourceType;
 import io.dropwizard.auth.Authenticator;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
+import jakarta.ws.rs.NotFoundException;
 import org.hl7.fhir.dstu3.model.Bundle;
 import org.hl7.fhir.dstu3.model.Organization;
 import org.slf4j.Logger;
@@ -33,39 +35,42 @@ public MacaroonsAuthenticator(@Named("attribution") IGenericClient client) {
     @Override
     public Optional<OrganizationPrincipal> authenticate(DPCAuthCredentials credentials) {
         logger.debug("Performing token authentication");
+        PathAuthorizer pathAuthorizer = credentials.getPathAuthorizer();
 
         // If we don't have a path authorizer, just return the principal
         final OrganizationPrincipal principal = new OrganizationPrincipal(credentials.getOrganization());
-        if (credentials.getPathAuthorizer() == null) {
+        if (pathAuthorizer == null) {
             logger.debug("No path authorizer is present, returning principal");
             return Optional.of(principal);
         }
 
         // If we're an organization, we just check the org ID against the path value and see if it matches
-        if (credentials.getPathAuthorizer().type() == DPCResourceType.Organization) {
+        if (pathAuthorizer.type() == DPCResourceType.Organization) {
             return validateOrganization(principal, credentials);
         }
 
         // Otherwise, try to lookup the matching resource
-        logger.debug("Looking up resource {} in path authorizer. With value: {}", credentials.getPathAuthorizer().type(), credentials.getPathAuthorizer().pathParam());
+        logger.debug("Looking up resource {} in path authorizer. With value: {}", pathAuthorizer.type(), pathAuthorizer.pathParam());
         Map<String, List<String>> searchParams = new HashMap<>();
         searchParams.put("_id", Collections.singletonList(credentials.getPathValue()));
         searchParams.put("organization", Collections.singletonList(credentials.getOrganization().getId()));
 
         // Special handling of Group resources, which use tags instead of resource properties.
-        if (credentials.getPathAuthorizer().type() == DPCResourceType.Group) {
+        if (pathAuthorizer.type() == DPCResourceType.Group) {
             searchParams.put("_tag", Collections.singletonList(String.format("%s|%s", DPCIdentifierSystem.DPC.getSystem(), credentials.getOrganization().getId())));
         }
         final Bundle bundle = this.client
                 .search()
-                .forResource(credentials.getPathAuthorizer().type().toString())
+                .forResource(pathAuthorizer.type().toString())
                 .whereMap(searchParams)
                 .returnBundle(Bundle.class)
                 .encodedJson()
                 .execute();
 
+        // We're using a path authorizer.  If the bundle is empty we want to force a 404 Not Found response instead of
+        // 401 Unauthorized that would be returned for a standard auth failure.
         if (bundle.getEntry().isEmpty()) {
-            return Optional.empty();
+            throw new NotFoundException(String.format("%s not found", pathAuthorizer.type().toString()));
         }
 
         return Optional.of(principal);
diff --git a/dpc-api/src/test/java/gov/cms/dpc/api/auth/macaroonauth/MacaroonsAuthenticatorUnitTest.java b/dpc-api/src/test/java/gov/cms/dpc/api/auth/macaroonauth/MacaroonsAuthenticatorUnitTest.java
@@ -7,14 +7,15 @@
 import gov.cms.dpc.api.auth.annotations.PathAuthorizer;
 import gov.cms.dpc.fhir.DPCIdentifierSystem;
 import gov.cms.dpc.fhir.DPCResourceType;
+import jakarta.ws.rs.NotFoundException;
 import org.hl7.fhir.dstu3.model.Bundle;
 import org.hl7.fhir.dstu3.model.Organization;
 import org.hl7.fhir.instance.model.api.IBaseBundle;
 import org.junit.jupiter.api.Test;
 
 import java.util.*;
 
-import static org.junit.Assert.assertSame;
+import static org.junit.Assert.*;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.*;
 
@@ -107,7 +108,9 @@ void test_authenticate_resource_not_authorized() {
 		when(whereMapQuery.returnBundle(Bundle.class).encodedJson()).thenReturn(bundleQuery);
 		when(bundleQuery.execute()).thenReturn(bundle);
 
-		assertTrue(macaroonsAuthenticator.authenticate(dpcAuthCredentials).isEmpty());
+		NotFoundException exception = assertThrows(NotFoundException.class,
+			() -> macaroonsAuthenticator.authenticate(dpcAuthCredentials));
+		assertEquals(String.format("%s not found", DPCResourceType.Patient.name()), exception.getMessage());
 	}
 
 	@Test
diff --git a/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/GroupResourceTest.java b/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/GroupResourceTest.java
@@ -4,8 +4,8 @@
 import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.client.api.IGenericClient;
 import ca.uhn.fhir.rest.gclient.ICreateTyped;
-import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
 import gov.cms.dpc.api.APITestHelpers;
 import gov.cms.dpc.api.AbstractSecureApplicationTest;
@@ -133,7 +133,7 @@ void testMissingProvenance() throws IOException {
     }
 
     @Test
-    public void testInvalidInputsWhenCreatingGroup() throws IOException {
+    void testInvalidInputsWhenCreatingGroup() throws IOException {
         final IParser parser = ctx.newJsonParser();
         IGenericClient client = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), ORGANIZATION_TOKEN, PUBLIC_KEY_ID, PRIVATE_KEY);
         APITestHelpers.setupPatientTest(client, parser);
@@ -253,7 +253,7 @@ void testCreateInvalidGroup() throws IOException, URISyntaxException {
     }
 
     @Test
-    public void testCreateGroupReturnsAppropriateHeaders() throws IOException {
+    void testCreateGroupReturnsAppropriateHeaders() throws IOException {
         final IParser parser = ctx.newJsonParser();
         IGenericClient client = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), ORGANIZATION_TOKEN, PUBLIC_KEY_ID, PRIVATE_KEY);
         APITestHelpers.setupPatientTest(client, parser);
@@ -340,7 +340,7 @@ public void testCreateGroupReturnsAppropriateHeaders() throws IOException {
     }
 
     @Test
-    public void testProvenanceHeaderAndGroupProviderMatch() {
+    void testProvenanceHeaderAndGroupProviderMatch() {
         IGenericClient client = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), ORGANIZATION_TOKEN, PUBLIC_KEY_ID, PRIVATE_KEY);
         Practitioner practitioner = APITestHelpers.createPractitionerResource(NPIUtil.generateNPI(), ORGANIZATION_ID);
 
@@ -401,7 +401,7 @@ public void testProvenanceHeaderAndGroupProviderMatch() {
     }
 
     @Test
-    public void testGroupCanOnlyBeRetrievedByOwner() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testGroupCanOnlyBeRetrievedByOwner() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -432,7 +432,7 @@ public void testGroupCanOnlyBeRetrievedByOwner() throws GeneralSecurityException
         assertEquals(orgBGroup.getId(), foundGroup.getId(), "Returned group ID should have been the same as requested ID");
 
 
-        assertThrows(AuthenticationException.class, () ->
+        assertThrows(ResourceNotFoundException.class, () ->
                 orgBClient.read()
                         .resource(Group.class)
                         .withId(orgAGroup.getId())
@@ -442,7 +442,7 @@ public void testGroupCanOnlyBeRetrievedByOwner() throws GeneralSecurityException
     }
 
     @Test
-    public void testOrgCanOnlyDeleteTheirOwnGroup() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testOrgCanOnlyDeleteTheirOwnGroup() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -455,7 +455,7 @@ public void testOrgCanOnlyDeleteTheirOwnGroup() throws GeneralSecurityException,
         Group orgBGroup = createAndSubmitGroup(orgBContext.getOrgId(), orgBPractitioner, orgBClient, Collections.emptyList());
 
         Provenance provenance = APITestHelpers.createProvenance(orgBContext.getOrgId(), orgBPractitioner.getId(), Collections.emptyList());
-        assertThrows(AuthenticationException.class, () ->
+        assertThrows(ResourceNotFoundException.class, () ->
                 orgBClient.delete()
                         .resource(orgAGroup)
                         .encodedJson()
@@ -472,7 +472,7 @@ public void testOrgCanOnlyDeleteTheirOwnGroup() throws GeneralSecurityException,
     }
 
     @Test
-    public void testOrgCanOnlyUpdateTheirOwnGroup() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testOrgCanOnlyUpdateTheirOwnGroup() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -485,7 +485,7 @@ public void testOrgCanOnlyUpdateTheirOwnGroup() throws GeneralSecurityException,
         Group orgBGroup = createAndSubmitGroup(orgBContext.getOrgId(), orgBPractitioner, orgBClient, Collections.emptyList());
 
         Provenance provenance = APITestHelpers.createProvenance(orgBContext.getOrgId(), orgBPractitioner.getId(), Collections.emptyList());
-        assertThrows(AuthenticationException.class, () ->
+        assertThrows(ResourceNotFoundException.class, () ->
                 orgBClient.update().resource(orgAGroup)
                         .encodedJson()
                         .withAdditionalHeader("X-Provenance", ctx.newJsonParser().encodeResourceToString(provenance))
@@ -501,7 +501,7 @@ public void testOrgCanOnlyUpdateTheirOwnGroup() throws GeneralSecurityException,
     }
 
     @Test
-    public void testOrgCanOnlyListTheirOwnGroups() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testOrgCanOnlyListTheirOwnGroups() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -525,7 +525,7 @@ public void testOrgCanOnlyListTheirOwnGroups() throws GeneralSecurityException,
 
 
     @Test
-    public void testOrgCanOnlyCreateGroupWithPatientsTheyManage() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testOrgCanOnlyCreateGroupWithPatientsTheyManage() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -553,7 +553,7 @@ public void testOrgCanOnlyCreateGroupWithPatientsTheyManage() throws GeneralSecu
     }
 
     @Test
-    public void testOrgCanOnlyUpdateGroupWithPatientsTheyManage() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
+    void testOrgCanOnlyUpdateGroupWithPatientsTheyManage() throws GeneralSecurityException, IOException, URISyntaxException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
@@ -582,7 +582,7 @@ public void testOrgCanOnlyUpdateGroupWithPatientsTheyManage() throws GeneralSecu
 
 
     @Test
-    public void testOrgCanOnlyAddPatientsTheyManageToGroup() throws IOException, URISyntaxException, GeneralSecurityException, ParseException {
+    void testOrgCanOnlyAddPatientsTheyManageToGroup() throws IOException, URISyntaxException, GeneralSecurityException, ParseException {
         final TestOrganizationContext orgAContext = registerAndSetupNewOrg();
         final TestOrganizationContext orgBContext = registerAndSetupNewOrg();
         final IGenericClient orgAClient = APIAuthHelpers.buildAuthenticatedClient(ctx, getBaseURL(), orgAContext.getClientToken(), UUID.fromString(orgAContext.getPublicKeyId()), orgAContext.getPrivateKey());
diff --git a/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/PatientResourceTest.java b/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/PatientResourceTest.java
@@ -10,10 +10,7 @@
 import ca.uhn.fhir.rest.gclient.IReadExecutable;
 import ca.uhn.fhir.rest.gclient.IUpdateExecutable;
 import ca.uhn.fhir.rest.param.StringParam;
-import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
-import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
-import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
-import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import ca.uhn.fhir.rest.server.exceptions.*;
 import com.codahale.metrics.MetricRegistry;
 import gov.cms.dpc.api.APITestHelpers;
 import gov.cms.dpc.api.AbstractSecureApplicationTest;
@@ -166,7 +163,7 @@ void ensurePatientsExist() throws IOException, URISyntaxException, GeneralSecuri
                 .withId(foundPatient.getId())
                 .encodedJson();
 
-        assertThrows(AuthenticationException.class, fetchRequest::execute, "Should not be authorized");
+        assertThrows(ResourceNotFoundException.class, fetchRequest::execute, "Should not be authorized");
 
         // Search, and find nothing
         final Bundle otherSpecificSearch = client
@@ -210,8 +207,7 @@ void testPatientRemoval() throws IOException, URISyntaxException, GeneralSecurit
                 .withId(patient.getId())
                 .encodedJson();
 
-        // TODO: DPC-433, this really should be NotFound, but we can't disambiguate between the two cases
-        assertThrows(AuthenticationException.class, fetchRequest::execute, "Should not have found the resource");
+        assertThrows(ResourceNotFoundException.class, fetchRequest::execute, "Should not have found the resource");
 
         // Search again
         final Bundle secondSearch = fetchPatients(client);
@@ -712,7 +708,7 @@ void testGetPatientByUUID() throws GeneralSecurityException, IOException, URISyn
 
         assertNotNull(APITestHelpers.getResourceById(orgAClient, Patient.class,orgAPatient.getId()), "Org should be able to retrieve their own patient.");
         assertNotNull(APITestHelpers.getResourceById(orgBClient,Patient.class, orgBPatient.getId()), "Org should be able to retrieve their own patient.");
-        assertThrows(AuthenticationException.class, () -> APITestHelpers.getResourceById(orgAClient,Patient.class, orgBPatient.getId()), "Expected auth error when retrieving another org's patient.");
+        assertThrows(ResourceNotFoundException.class, () -> APITestHelpers.getResourceById(orgAClient,Patient.class, orgBPatient.getId()), "Expected auth error when retrieving another org's patient.");
     }
 
     @Test
@@ -825,7 +821,7 @@ void testDeletePatient() throws GeneralSecurityException, IOException, URISyntax
         //Setup org B with a patient
         final Patient orgBPatient = (Patient) APITestHelpers.createResource(orgBClient, APITestHelpers.createPatientResource("4S41C00AA00", orgBContext.getOrgId())).getResource();
 
-        assertThrows(AuthenticationException.class, () -> APITestHelpers.deleteResourceById(orgAClient, DPCResourceType.Patient, orgBPatient.getIdElement().getIdPart()), "Expected auth error when deleting another org's patient.");
+        assertThrows(ResourceNotFoundException.class, () -> APITestHelpers.deleteResourceById(orgAClient, DPCResourceType.Patient, orgBPatient.getIdElement().getIdPart()), "Expected auth error when deleting another org's patient.");
         APITestHelpers.deleteResourceById(orgBClient, DPCResourceType.Patient, orgBPatient.getIdElement().getIdPart());
     }
 
@@ -842,17 +838,17 @@ void testPatientPathAuthorization() throws GeneralSecurityException, IOException
         //Test GET /Patient/{id}
         APITestHelpers.getResourceById(orgAClient,Patient.class,orgAPatient.getIdElement().getIdPart());
         APITestHelpers.getResourceById(orgBClient,Patient.class,orgBPatient.getIdElement().getIdPart());
-        assertThrows(AuthenticationException.class, () -> APITestHelpers.getResourceById(orgBClient,Patient.class,orgAPatient.getIdElement().getIdPart()), "Expected auth error when accessing another org's patient");
+        assertThrows(ResourceNotFoundException.class, () -> APITestHelpers.getResourceById(orgBClient,Patient.class,orgAPatient.getIdElement().getIdPart()), "Expected auth error when accessing another org's patient");
 
         //Test PUT /Patient/{id}
         APITestHelpers.updateResource(orgAClient,orgAPatient.getIdElement().getIdPart(),orgAPatient);
         APITestHelpers.updateResource(orgBClient,orgBPatient.getIdElement().getIdPart(),orgBPatient);
-        assertThrows(AuthenticationException.class, () -> APITestHelpers.updateResource(orgBClient,orgAPatient.getIdElement().getIdPart(),orgAPatient), "Expected auth error when updating another org's patient");
+        assertThrows(ResourceNotFoundException.class, () -> APITestHelpers.updateResource(orgBClient,orgAPatient.getIdElement().getIdPart(),orgAPatient), "Expected auth error when updating another org's patient");
 
         //Test PUT /Patient/{id}
         APITestHelpers.updateResource(orgAClient,orgAPatient.getIdElement().getIdPart(),orgAPatient);
         APITestHelpers.updateResource(orgBClient,orgBPatient.getIdElement().getIdPart(),orgBPatient);
-        assertThrows(AuthenticationException.class, () -> APITestHelpers.updateResource(orgBClient,orgAPatient.getIdElement().getIdPart(),orgAPatient), "Expected auth error when updating another org's patient");
+        assertThrows(ResourceNotFoundException.class, () -> APITestHelpers.updateResource(orgBClient,orgAPatient.getIdElement().getIdPart(),orgAPatient), "Expected auth error when updating another org's patient");
 
 
         //Test Get /Patient/{id}/$everything
@@ -867,7 +863,7 @@ void testPatientPathAuthorization() throws GeneralSecurityException, IOException
        assertEquals(64, result.getTotal(), "Should have 64 entries in Bundle");
 
         final String orgAPractitionerId = orgAPractitioner.getIdElement().getIdPart();
-       assertThrows(AuthenticationException.class, () ->
+       assertThrows(ResourceNotFoundException.class, () ->
                APITestHelpers.getPatientEverything(orgBClient, orgAPatient.getIdElement().getIdPart(), generateProvenance(orgAContext.getOrgId(), orgAPractitionerId))
        , "Expected auth error when export another org's patient's data");
     }
diff --git a/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/PractitionerResourceTest.java b/dpc-api/src/test/java/gov/cms/dpc/api/resources/v1/PractitionerResourceTest.java
diff --git a/dpc-bluebutton/pom.xml b/dpc-bluebutton/pom.xml
