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

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',
@@ -660,6 +660,8 @@ def _read_request(self, version: int = 1):
 
         @all_requests
         def catchall(url, req):
+            print("EXPECTED: ", expected_request['url'])
+            print("RETURNED: ", unquote(req.url))
             self.assertEqual(expected_request['url'], unquote(req.url))
             self.assertEqual(expected_request['method'], req.method)
             self.assertDictContainsSubset(expected_request['headers'], req.headers)

apps/fhir/bluebutton/tests/test_utils.py

@@ -18,7 +18,8 @@
     crosswalk_patient_id,
     get_resourcerouter,
     build_oauth_resource,
-    valid_caller_for_patient_read,
+    parse_string,
+    valid_patient_read_or_search_call,
 )
 
 ENCODED = settings.ENCODING
@@ -70,16 +71,51 @@ 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_parse_string(self):
+        result = parse_string('PatientId:-20140000008326', ':')
+        assert result == '-20140000008326'
+
+        result = parse_string('?_lastUpdated=lt2024-06-15&startIndex=0&cursor=0&_id=-99000010256546', '_id=')
+        assert result == '-99000010256546'
+
+        result = parse_string('?_lastUpdated=lt2024-06-15&startIndex=0&cursor=0&_id=-99000010256546', '_id==')
+        assert result is None
+
+    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_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
+
+    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_caller_for_patient_read('PatientId:-20140000008326', '-20140000008329')
+        result = valid_patient_read_or_search_call(
+            'PatientId:-99140000008329',
+            None,
+            '_lastUpdated=lt2024-06-15&startIndex=0&cursor=0&_id=-99140000008329'
+        )
+        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
 
-        # 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
+        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,7 @@
 from collections import OrderedDict
 from datetime import datetime
 from pytz import timezone
+from typing import Optional
 
 from django.conf import settings
 from django.contrib import messages
@@ -745,18 +746,42 @@ 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 parse_string(string_to_parse: str, split_char: str) -> str:
+    """_summary_
+    Args:
+        string_to_parse (str): _description_
+        split_char (str): _description_
+    Returns:
+        str: _description_
+    """
+    parts = string_to_parse.split(split_char, 1)
+    parsed_string = parts[1] if len(parts) > 1 else None
+    return parsed_string
+
+
+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
+    beneficiary_id = parse_string(beneficiary_id, ':')
+    # 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 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
+    patient_id = parse_string(query_param, '_id=')
+    if patient_id and patient_id != 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)
         }