Refactored a few more endpoints to use consolidated auth

Author: dlzhry2nhs

…nd/src/authorisation/ApiOperationCode.py …/src/authorisation/api_operation_code.pybackend/src/authorisation/ApiOperationCode.py renamed to backend/src/authorisation/api_operation_code.py backend/src/authorisation/ApiOperationCode.py renamed to backend/src/authorisation/api_operation_code.py

@@ -1,6 +1,6 @@
 import json
 
-from authorisation.ApiOperationCode import ApiOperationCode
+from authorisation.api_operation_code import ApiOperationCode
 from clients import redis_client, logger
 from constants import SUPPLIER_PERMISSIONS_HASH_KEY
 
@@ -27,19 +27,19 @@ def _expand_permissions(permissions: list[str]) -> dict[str, list[ApiOperationCo
 
         return expanded_permissions
 
-    def _get_supplier_permissions(self, supplier_name: str) -> dict[str, list[ApiOperationCode]]:
-        raw_permissions_data = self._cache_client.hget(SUPPLIER_PERMISSIONS_HASH_KEY, supplier_name)
+    def _get_supplier_permissions(self, supplier_system: str) -> dict[str, list[ApiOperationCode]]:
+        raw_permissions_data = self._cache_client.hget(SUPPLIER_PERMISSIONS_HASH_KEY, supplier_system)
         permissions_data = json.loads(raw_permissions_data) if raw_permissions_data else []
 
         return self._expand_permissions(permissions_data)
 
     def authorise(
         self,
-        supplier_name: str,
+        supplier_system: str,
         requested_operation: ApiOperationCode,
         vaccination_types: set[str]
     ) -> bool:
-        supplier_permissions = self._get_supplier_permissions(supplier_name)
+        supplier_permissions = self._get_supplier_permissions(supplier_system)
 
         logger.info(
             f"operation: {requested_operation}, supplier_permissions: {supplier_permissions}, "

backend/src/authorisation/Authoriser.py backend/src/authorisation/authoriser.pybackend/src/authorisation/Authoriser.py renamed to backend/src/authorisation/authoriser.py

@@ -125,7 +125,7 @@ def get_immunization_by_id(self, aws_event) -> dict:
             return self.create_response(403, unauthorized.to_operation_outcome())
 
         try:
-            if resource := self.fhir_service.get_immunization_by_id(imms_id, imms_vax_type_perms):
+            if resource := self.fhir_service.get_immunization_by_id(imms_id, supplier_system):
                 version = str()
                 if isinstance(resource, Immunization):
                     resp = resource
@@ -157,19 +157,19 @@ def create_immunization(self, aws_event):
             return self.create_response(403, unauthorized.to_operation_outcome())
 
         # Call the common method and unpack the results
+        # TODO - can remove this and the block above. Only need supplier system
         response, imms_vax_type_perms, supplier_system = self.check_vaccine_type_permissions(
             aws_event
         )
         if response:
             return response
 
         try:
-            imms = json.loads(aws_event["body"], parse_float=Decimal)
+            immunisation = json.loads(aws_event["body"], parse_float=Decimal)
         except json.decoder.JSONDecodeError as e:
             return self._create_bad_request(f"Request's body contains malformed JSON: {e}")
         try:
-            resource = self.fhir_service.create_immunization(
-                imms, imms_vax_type_perms, supplier_system)
+            resource = self.fhir_service.create_immunization(immunisation, supplier_system)
             if "diagnostics" in resource:
                 exp_error = create_operation_outcome(
                     resource_id=str(uuid.uuid4()),
@@ -366,10 +366,9 @@ def delete_immunization(self, aws_event):
         except UnauthorizedError as unauthorized:
             return self.create_response(403, unauthorized.to_operation_outcome())
 
-        # Validate the imms id - start
+        # Validate the imms id
         if id_error := self._validate_id(imms_id):
             return FhirController.create_response(400, json.dumps(id_error))
-        # Validate the imms id - end
 
         # Call the common method and unpack the results
         response, imms_vax_type_perms, supplier_system = self.check_vaccine_type_permissions(

backend/src/fhir_controller.py

@@ -104,7 +104,7 @@ def get_immunization_by_identifier(self, identifier_pk: str) -> tuple[Optional[d
         else:
             return None, None
 
-    def get_immunization_by_id(self, imms_id: str, imms_vax_type_perms: str) -> Optional[dict]:
+    def get_immunization_by_id(self, imms_id: str) -> Optional[dict]:
         response = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)})
         item = response.get("Item")
 
@@ -113,12 +113,6 @@ def get_immunization_by_id(self, imms_id: str, imms_vax_type_perms: str) -> Opti
         if item.get("DeletedAt") and item["DeletedAt"] != "reinstated":
             return None
 
-        # Get vaccine type + validate permissions
-        vaccine_type = self._vaccine_type(item["PatientSK"])
-        if not validate_permissions(imms_vax_type_perms, ApiOperationCode.READ, [vaccine_type]):
-            raise UnauthorizedVaxError()
-
-        # Build response
         return {
             "Resource": json.loads(item["Resource"]),
             "Version": item["Version"]
@@ -157,14 +151,11 @@ def get_immunization_by_id_all(self, imms_id: str, imms: dict) -> Optional[dict]
         else:
             return None
 
-    def create_immunization(
-        self, immunization: dict, patient: any, imms_vax_type_perms, supplier_system
-    ) -> dict:
+    def create_immunization(self, immunization: dict, patient: any, supplier_system: str) -> dict:
         new_id = str(uuid.uuid4())
         immunization["id"] = new_id
         attr = RecordAttributes(immunization, patient)
-        if not validate_permissions(imms_vax_type_perms,ApiOperationCode.CREATE, [attr.vaccine_type]):
-            raise UnauthorizedVaxError()
+
         query_response = _query_identifier(self.table, "IdentifierGSI", "IdentifierPK", attr.identifier)
 
         if query_response is not None:

backend/src/fhir_repository.py

@@ -12,17 +12,17 @@
     BundleEntrySearch,
 )
 from fhir.resources.R4B.immunization import Immunization
-from poetry.console.commands import self
 from pydantic import ValidationError
 
 import parameter_parser
-from authorisation.ApiOperationCode import ApiOperationCode
-from authorisation.Authoriser import Authoriser
+from authorisation.api_operation_code import ApiOperationCode
+from authorisation.authoriser import Authoriser
 from fhir_repository import ImmunizationRepository
 from models.errors import InvalidPatientId, CustomValidationError, UnauthorizedVaxError
 from models.fhir_immunization import ImmunizationValidator
 from models.utils.generic_utils import nhs_number_mod11_check, get_occurrence_datetime, create_diagnostics, form_json, get_contained_patient
 from models.errors import MandatoryError
+from models.utils.validation_utils import get_vaccine_type
 from timer import timed
 from filter import Filter
 
@@ -80,16 +80,20 @@ def get_immunization_by_identifier(
         imms_resp['resource'] = filtered_resource
         return form_json(imms_resp, element, identifier, base_url)
 
-    def get_immunization_by_id(self, imms_id: str, imms_vax_type_perms: list[str]) -> Optional[dict]:
+    def get_immunization_by_id(self, imms_id: str, supplier_system: str) -> Optional[dict]:
         """
         Get an Immunization by its ID. Return None if it is not found. If the patient doesn't have an NHS number,
         return the Immunization.
         """
-        if not (imms_resp := self.immunization_repo.get_immunization_by_id(imms_id, imms_vax_type_perms)):
+        if not (imms_resp := self.immunization_repo.get_immunization_by_id(imms_id)):
             return None
 
         # Returns the Immunisation full resource with no obfuscation
         resource = imms_resp.get("Resource", {})
+        vaccination_type = get_vaccine_type(resource)
+
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.READ, {vaccination_type}):
+            raise UnauthorizedVaxError()
 
         return {
             "Version": imms_resp.get("Version", ""),
@@ -109,10 +113,7 @@ def get_immunization_by_id_all(self, imms_id: str, imms: dict) -> Optional[dict]
         imms_resp = self.immunization_repo.get_immunization_by_id_all(imms_id, imms)
         return imms_resp
 
-    def create_immunization(
-        self, immunization: dict, imms_vax_type_perms, supplier_system
-    ) -> Immunization:
-
+    def create_immunization(self, immunization: dict, supplier_system: str) -> dict | Immunization:
         if immunization.get("id") is not None:
             raise CustomValidationError("id field must not be present for CREATE operation")
 
@@ -121,14 +122,17 @@ def create_immunization(
         except (ValidationError, ValueError, MandatoryError) as error:
             raise CustomValidationError(message=str(error)) from error
         patient = self._validate_patient(immunization)
+
         if "diagnostics" in patient:
             return patient
 
-        imms = self.immunization_repo.create_immunization(
-            immunization, patient, imms_vax_type_perms, supplier_system
-        )
+        vaccination_type = get_vaccine_type(immunization)
 
-        return Immunization.parse_obj(imms)
+        if not self.authoriser.authorise(supplier_system, ApiOperationCode.CREATE, {vaccination_type}):
+            raise UnauthorizedVaxError()
+
+        immunisation = self.immunization_repo.create_immunization(immunization, patient, supplier_system)
+        return Immunization.parse_obj(immunisation)
 
     def update_immunization(
         self,
@@ -200,7 +204,7 @@ def update_reinstated_immunization(
 
         return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
 
-    def delete_immunization(self, imms_id, imms_vax_type_perms, supplier_system) -> Immunization:
+    def delete_immunization(self, imms_id: str, imms_vax_type_perms, supplier_system: str) -> Immunization:
         """
         Delete an Immunization if it exits and return the ID back if successful.
         Exception will be raised if resource didn't exit. Multiple calls to this method won't change

backend/src/fhir_service.py

@@ -5,13 +5,11 @@
 import unittest
 import uuid
 
-from unittest.mock import patch
 from fhir.resources.R4B.bundle import Bundle
 from fhir.resources.R4B.immunization import Immunization
 from unittest.mock import create_autospec, ANY, patch, Mock
 from urllib.parse import urlencode
 import urllib.parse
-from moto import mock_aws
 from authorization import Authorization
 from fhir_controller import FhirController
 from fhir_repository import ImmunizationRepository
@@ -22,15 +20,12 @@
     InvalidPatientId,
     CustomValidationError,
     ParameterException,
-    InconsistentIdError,
     UnauthorizedVaxError,
-    UnauthorizedError,
     IdentifierDuplicationError,
 )
 from tests.utils.immunization_utils import create_covid_19_immunization
 from parameter_parser import patient_identifier_system, process_search_params
 from tests.utils.generic_utils import load_json_data
-from tests.utils.values_for_tests import ValidValues
 
 class TestFhirControllerBase(unittest.TestCase):
     """Base class for all tests to set up common fixtures"""
@@ -211,7 +206,7 @@ def test_get_imms_by_identifer_patient_identifier_and_element_present(self, mock
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -234,7 +229,30 @@ def test_get_imms_by_identifer_both_body_and_query_params_present(self, mock_get
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
+
+        self.assertEqual(response["statusCode"], 400)
+        body = json.loads(response["body"])
+        self.assertEqual(body["resourceType"], "OperationOutcome")
+
+    @patch("fhir_controller.get_supplier_permissions")
+    def test_get_imms_by_identifer_imms_identifier_and_element_not_present(self, mock_get_supplier_permissions):
+        """it should return Immunization Id if it exists"""
+        # Given
+        mock_get_supplier_permissions.return_value = ["COVID19.CRUDS"]
+        self.service.get_immunization_by_identifier.return_value = {"id": "test", "Version": 1}
+        lambda_event = {
+            "headers": {"SupplierSystem": "test"},
+            "queryStringParameters": {
+                "-immunization.target": "test",
+                "immunization.identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184",
+            },
+            "body": None,
+        }
+        # When
+        response = self.controller.get_immunization_by_identifier(lambda_event)
+        # Then
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -258,7 +276,7 @@ def test_get_imms_by_identifer_both_identifier_present(self, mock_get_supplier_p
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -512,7 +530,7 @@ def test_get_imms_by_identifer_patient_identifier_and_element_present(self, mock
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -532,7 +550,7 @@ def test_get_imms_by_identifer_imms_identifier_and_element_not_present(self,mock
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -616,7 +634,7 @@ def test_get_imms_by_identifer_both_identifier_present(self, mock_get_supplier_p
         # When
         response = self.controller.get_immunization_by_identifier(lambda_event)
         # Then
-        self.service.get_immunization_by_identifier.assert_not_called
+        self.service.get_immunization_by_identifier.assert_not_called()
 
         self.assertEqual(response["statusCode"], 400)
         body = json.loads(response["body"])
@@ -757,7 +775,7 @@ def test_get_imms_by_id(self, mock_permissions):
         response = self.controller.get_immunization_by_id(lambda_event)
         # Then
         mock_permissions.assert_called_once_with("test")
-        self.service.get_immunization_by_id.assert_called_once_with(imms_id, ["COVID19.CRUDS"])
+        self.service.get_immunization_by_id.assert_called_once_with(imms_id, "test")
 
         self.assertEqual(response["statusCode"], 200)
         body = json.loads(response["body"])
@@ -780,6 +798,7 @@ def test_get_imms_by_id_unauthorised_vax_error(self,mock_permissions):
         # Then
         mock_permissions.assert_called_once_with("test")
         self.assertEqual(response["statusCode"], 403)
+
     @patch("fhir_controller.get_supplier_permissions")
     def test_get_imms_by_id_no_vax_permission(self, mock_permissions):
         """it should return Immunization Id if it exists"""
@@ -814,7 +833,7 @@ def test_not_found(self,mock_permissions):
 
         # Then
         mock_permissions.assert_called_once_with("test")
-        self.service.get_immunization_by_id.assert_called_once_with(imms_id, ["COVID19.CRUDS"])
+        self.service.get_immunization_by_id.assert_called_once_with(imms_id, "test")
 
         self.assertEqual(response["statusCode"], 404)
         body = json.loads(response["body"])
@@ -860,7 +879,7 @@ def test_create_immunization(self,mock_get_permissions):
 
         imms_obj = json.loads(aws_event["body"])
         mock_get_permissions.assert_called_once_with("Test")
-        self.service.create_immunization.assert_called_once_with(imms_obj, ["COVID19.CRUDS", "FLU.CRUDS"], "Test")
+        self.service.create_immunization.assert_called_once_with(imms_obj, "Test")
         self.assertEqual(response["statusCode"], 201)
         self.assertTrue("body" not in response)
         self.assertTrue(response["headers"]["Location"].endswith(f"Immunization/{imms_id}"))
@@ -1271,7 +1290,7 @@ def test_update_record_exists(self, mock_get_supplier_permissions):
         response = self.controller.get_immunization_by_id(lambda_event)
 
         # Then
-        self.service.get_immunization_by_id.assert_called_once_with(imms_id, ["COVID19.CRUDS"])
+        self.service.get_immunization_by_id.assert_called_once_with(imms_id, "Test")
 
         self.assertEqual(response["statusCode"], 404)
         body = json.loads(response["body"])