BB2-4233: Ensure _id param is actually parsed for search Patient call… (#1425)

* BB2-4233: Ensure _id param is actually parsed for search Patient calls, throwing a 404 if it does not match current session. Remove _has:Coverage from swagger docs

* Remove prints

* Address PR feedback: use parse_qs, remove parse_string function/tests

* Add additional test case that previously caused an erroneous 404

Author: JamesDemeryNava

apps/fhir/bluebutton/tests/fhir_resources/fhir_meta_v1.json

@@ -128,11 +128,6 @@
                             "type": "string",
                             "documentation": "The offset used for result pagination"
                         },
-                        {
-                            "name": "_has:Coverage",
-                            "type": "token",
-                            "documentation": "Part D coverage type"
-                        },
                         {
                             "name": "cursor",
                             "type": "string",

apps/fhir/bluebutton/tests/fhir_resources/fhir_meta_v2.json

@@ -136,11 +136,6 @@
                             "type": "string",
                             "documentation": "The offset used for result pagination"
                         },
-                        {
-                            "name": "_has:Coverage",
-                            "type": "token",
-                            "documentation": "Part D coverage type"
-                        },
                         {
                             "name": "cursor",
                             "type": "string",

apps/fhir/bluebutton/tests/test_read_and_search.py

@@ -26,7 +26,7 @@
 def get_expected_read_request(version: int):
     return {
         'method': 'GET',
-        'url': f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/{FHIR_ID_V2}/?_format=json',
+        'url': f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/{FHIR_ID_V2}/?_format=json&_id={FHIR_ID_V2}',
         'headers': {
             # 'User-Agent': 'python-requests/2.20.0',
             'Accept-Encoding': 'gzip, deflate',

apps/fhir/bluebutton/tests/test_utils.py

@@ -18,7 +18,7 @@
     crosswalk_patient_id,
     get_resourcerouter,
     build_oauth_resource,
-    valid_caller_for_patient_read,
+    valid_patient_read_or_search_call,
 )
 
 ENCODED = settings.ENCODING
@@ -70,16 +70,48 @@ def test_notNone(self):
         response = notNone(listing, "number")
         self.assertEqual(response, listing)
 
-    def test_valid_caller_for_patient_read(self):
-        result = valid_caller_for_patient_read('PatientId:-20140000008326', '-20140000008326')
+    def test_valid_patient_read_or_search_call_valid_read_calls(self):
+        result = valid_patient_read_or_search_call('PatientId:-20140000008329', '-20140000008329', '')
         assert result is True
 
-        result = valid_caller_for_patient_read('PatientId:-20140000008326', '-20140000008329')
+        result = valid_patient_read_or_search_call('PatientId:-99140000008329', '-99140000008329', '')
+        assert result is True
+
+    def test_valid_patient_read_or_search_call_invalid_read_calls(self):
+        result = valid_patient_read_or_search_call('PatientId:-20140000008329', '-99140000008329', '')
+        assert result is False
+
+        result = valid_patient_read_or_search_call('PatientId:-99140000008329', '-20140000008329', '')
         assert result is False
 
-        # call with no colon in beneficiary_id to make sure
-        invalid_call = valid_caller_for_patient_read('PatientId-20140000008326', '-20140000008329')
-        assert invalid_call is False
+    def test_valid_patient_read_or_search_call_valid_search_calls(self):
+        result = valid_patient_read_or_search_call('PatientId:-20140000008329', None, '_id=-20140000008329')
+        assert result is True
+
+        result = valid_patient_read_or_search_call(
+            'PatientId:-99140000008329',
+            None,
+            '_lastUpdated=lt2024-06-15&startIndex=0&cursor=0&_id=-99140000008329'
+        )
+        assert result is True
+
+        result = valid_patient_read_or_search_call(
+            'PatientId:-99140000008329',
+            None,
+            '_id=-99140000008329&_lastUpdated=lt2024-06-15&startIndex=0&cursor=0'
+        )
+        assert result is True
+
+    def test_valid_patient_read_or_search_call_invalid_search_calls(self):
+        result = valid_patient_read_or_search_call('PatientId:-20140000008329', None, '_id=-99140000008329')
+        assert result is False
+
+        result = valid_patient_read_or_search_call(
+            'PatientId:-99140000008329',
+            None,
+            '_lastUpdated=lt2024-06-15&startIndex=0&cursor=0&_id=-20140000008329'
+        )
+        assert result is False
 
 
 class BlueButtonUtilSupportedResourceTypeControlTestCase(TestCase):

apps/fhir/bluebutton/utils.py

@@ -10,6 +10,8 @@
 from collections import OrderedDict
 from datetime import datetime
 from pytz import timezone
+from typing import Optional
+from urllib.parse import parse_qs
 
 from django.conf import settings
 from django.contrib import messages
@@ -745,18 +747,31 @@ def get_patient_by_mbi_hash(mbi_hash, request):
     return response.json()
 
 
-def valid_caller_for_patient_read(beneficiary_id: str, patient_id: str) -> bool:
-    """When making a read patient call, we only want to ping BFD if the patient_id
-    being passed matches the fhir_id (currently v2) associated with the current session
+def valid_patient_read_or_search_call(beneficiary_id: str, resource_id: Optional[str], query_param: str) -> bool:
+    """Determine if a read or search Patient call is valid, based on what was passed for the resource_id (read call)
+    or the query_parameter (search call)
 
     Args:
-        beneficiary_id (str): beneficiary_id that is associated with the current session/
-        Should have format 'patientId:00000000000000 coming in
-        patient_id (str): patient_id that is will be passed to BFD
+        beneficiary_id (str): This comes from the BlueButton-OriginalQuery attribute of the headers for the API call.
+        Has the format, patientId:{{patientId}}, where patientId comes from the bluebutton_crosswalk table
+        resource_id (Optional[str]): This will only be populated for read calls, and it is the id being passed to BFD
+        query_param (str): String for the query parameter being passed for search calls. For read calls this is a blank string
 
     Returns:
-        bool: If the patient_id matches the beneficiary_id, then return True, else False
+        bool: Whether or not the call is valid
     """
-    parts = beneficiary_id.split(':', 1)
-    beneficiary_comparison_id = parts[1] if len(parts) == 2 else None
-    return beneficiary_comparison_id == patient_id
+    bene_split = beneficiary_id.split(':', 1)
+    beneficiary_id = bene_split[1] if len(bene_split) > 1 else None
+    # Handles the case where it is a read call, but what is passed does not match the beneficiary_id
+    # which is constructed using the patient id for the current session in generate_info_headers.
+    if resource_id and beneficiary_id and resource_id != beneficiary_id:
+        return False
+
+    # Handles the case where it is a search call, but what is passed does not match the beneficiary_id
+    # so a 404 Not found will be thrown before reaching out to BFD
+    query_dict = parse_qs(query_param)
+    passed_identifier = query_dict.get('_id', [None])
+    if passed_identifier[0] and passed_identifier[0] != beneficiary_id:
+        return False
+
+    return True

apps/fhir/bluebutton/views/generic.py

@@ -33,7 +33,7 @@
 from ..utils import (build_fhir_response,
                      FhirServerVerify,
                      get_resourcerouter,
-                     valid_caller_for_patient_read)
+                     valid_patient_read_or_search_call)
 
 logger = logging.getLogger(bb2logging.HHS_SERVER_LOGNAME_FMT.format(__name__))
 
@@ -151,16 +151,26 @@ def fetch_data(self, request, resource_type, *args, **kwargs):
 
         prepped = s.prepare_request(req)
 
-        resource_id = kwargs.get('resource_id')
-        beneficiary_id = prepped.headers.get('BlueButton-BeneficiaryId')
+        if resource_type == 'Patient':
+            query_param = prepped.headers.get('BlueButton-OriginalQuery')
+            resource_id = kwargs.get('resource_id')
+            beneficiary_id = prepped.headers.get('BlueButton-BeneficiaryId')
 
-        if resource_type == 'Patient' and resource_id and beneficiary_id:
-            # If it is a patient read request, confirm it is valid for the current user
-            # If not, throw a 404 before pinging BFD
-            if not valid_caller_for_patient_read(beneficiary_id, resource_id):
+            # For patient read and search calls, we need to ensure that what is being passed, either in
+            # query parameters for search calls, or in the resource_id for read calls, is valid for the
+            # current session (matching the beneficiary_id). If not, raise a 404 Not found before calling BFD.
+            if not valid_patient_read_or_search_call(beneficiary_id, resource_id, query_param):
                 error = NotFound('Not found.')
                 raise error
 
+            # Handle the case where it is a patient search call, but neither _id or identifier were passed
+            if '_id' not in get_parameters.keys() and 'identifier' not in get_parameters.keys():
+                # depending on if 4166 is merged first, this will need to be updated
+                get_parameters['_id'] = request.crosswalk.fhir_id(2)
+                # Reset the request parameters and the prepped request after adding the missing, but required, _id param
+                req.params = get_parameters
+                prepped = s.prepare_request(req)
+
         match self.version:
             case Versions.V1:
                 api_ver_str = 'v1'

apps/fhir/bluebutton/views/search.py

@@ -83,6 +83,11 @@ def build_url(self, resource_router, resource_type, *args, **kwargs):
 class SearchViewPatient(SearchView):
     # Class used for Patient resource search view
     required_scopes = ['patient/Patient.read', 'patient/Patient.rs', 'patient/Patient.s']
+    QUERY_SCHEMA = {
+        **SearchView.QUERY_SCHEMA,
+        '_id': str,
+        'identifier': str
+    }
 
     def __init__(self, version=1):
         super().__init__(version)
@@ -91,8 +96,6 @@ def __init__(self, version=1):
     def build_parameters(self, request, *args, **kwargs):
         return {
             '_format': 'application/json+fhir',
-            # BB2-4166-TODO : this needs to use self.version to determine fhir_id
-            '_id': request.crosswalk.fhir_id(2)
         }