From 3e14d104c0b0da0614f2b52739be536f7f936b6e Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Mon, 8 Jun 2026 17:23:55 +0530 Subject: [PATCH 01/11] fix:added queue token validation on authz retrive --- care/emr/api/viewsets/scheduling/token.py | 3 +++ care/emr/tests/test_token_api.py | 24 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/care/emr/api/viewsets/scheduling/token.py b/care/emr/api/viewsets/scheduling/token.py index d2c5e5f8e2..8107af9bb7 100644 --- a/care/emr/api/viewsets/scheduling/token.py +++ b/care/emr/api/viewsets/scheduling/token.py @@ -146,6 +146,9 @@ def authorize_destroy(self, instance): def authorize_retrieve(self, model_instance): _, queue = self.get_queue_obj() + if queue != model_instance.queue: + raise ValidationError("Token does not belong to the specified queue") + resource = queue.resource if not AuthorizationController.call( "can_list_token", diff --git a/care/emr/tests/test_token_api.py b/care/emr/tests/test_token_api.py index 66d4661fb4..d3f5045798 100644 --- a/care/emr/tests/test_token_api.py +++ b/care/emr/tests/test_token_api.py @@ -629,6 +629,30 @@ def test_retrieve_token_as_user_without_permission(self): "You do not have permission to read token", response.data["detail"] ) + def test_retrieve_token_with_invalid_queue_external_id(self): + """Test retrieving a token with invalid queue external id.""" + self.client.force_authenticate(user=self.superuser) + token = self.create_token( + patient=self.patient, + category=self.token_category, + queue=self.token_queue, + facility=self.facility, + status=TokenStatusOptions.CREATED, + ) + another_token_queue = self.create_queue( + facility=self.facility, + resource=self.schedule_resource, + date=timezone.now().date(), + ) + response = self.client.get( + self.generate_detail_url( + str(self.facility.external_id), + str(another_token_queue.external_id), + str(token.external_id), + ) + ) + self.assertEqual(response.status_code, 400) + # Tests for Token Listing def test_list_tokens_as_superuser(self): From bfaac38645a7a60442a094e83d87bf5b2430a029 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Tue, 9 Jun 2026 12:12:41 +0530 Subject: [PATCH 02/11] fix:added check for inventory item location on retrive api --- care/emr/api/viewsets/inventory/inventory_item.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/care/emr/api/viewsets/inventory/inventory_item.py b/care/emr/api/viewsets/inventory/inventory_item.py index c210ca14cf..21a98eae4d 100644 --- a/care/emr/api/viewsets/inventory/inventory_item.py +++ b/care/emr/api/viewsets/inventory/inventory_item.py @@ -46,6 +46,12 @@ def authorize_location_read(self, location): raise PermissionDenied("You do not have permission to read inventory items") def authorize_retrieve(self, model_instance): + location = self.get_location_obj() + if location.id != model_instance.location.id: + raise PermissionDenied( + "Inventory item does not belong to the specified location" + ) + self.authorize_location_read(model_instance.location) def get_queryset(self): From 8cb6d4e7b1863bff31f487e6de92cd3e360a1f7c Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Tue, 9 Jun 2026 15:18:07 +0530 Subject: [PATCH 03/11] fix:added validation check for bed with location on retrieve api --- care/emr/api/viewsets/location.py | 34 ++++++++++++++++++++++++----- care/emr/tests/test_location_api.py | 21 ++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/care/emr/api/viewsets/location.py b/care/emr/api/viewsets/location.py index a0e6993785..e17a0d9e26 100644 --- a/care/emr/api/viewsets/location.py +++ b/care/emr/api/viewsets/location.py @@ -303,6 +303,19 @@ def authorize_update(self, request_obj, model_instance): def authorize_destroy(self, instance): return self.authorize_create(instance) + def authorize_retrieve(self, model_instance): + location = self.get_location_obj() + facility = self.get_facility_obj() + if location.id != model_instance.location.id: + raise ValidationError("Bed does not belong to the specified location") + if not AuthorizationController.call( + "can_list_facility_location_obj", self.request.user, facility, location + ): + raise PermissionDenied("You do not have permission to given location") + return FacilityLocationEncounter.objects.filter(location=location).order_by( + "-created_date" + ) + def reset_encounter_location_association(self, location): """ Reset encounters to the right location. @@ -488,14 +501,23 @@ def _validate_data(self, instance, model_obj=None): # noqa PLR0912 def get_queryset(self): location = self.get_location_obj() facility = self.get_facility_obj() - if not AuthorizationController.call( - "can_list_facility_location_obj", self.request.user, facility, location - ): - raise PermissionDenied("You do not have permission to given location") - return FacilityLocationEncounter.objects.filter(location=location).order_by( - "-created_date" + queryset = ( + super() + .get_queryset() + .select_related("created_by", "updated_by") + .order_by("-modified_date") ) + if self.action == "list": + if not AuthorizationController.call( + "can_list_facility_location_obj", self.request.user, facility, location + ): + raise PermissionDenied("You do not have permission to given location") + return FacilityLocationEncounter.objects.filter(location=location).order_by( + "-created_date" + ) + return queryset + def close_related_location_from_encounter(instance): if instance.status in COMPLETED_CHOICES: diff --git a/care/emr/tests/test_location_api.py b/care/emr/tests/test_location_api.py index a3f8cb218f..e7720a677c 100644 --- a/care/emr/tests/test_location_api.py +++ b/care/emr/tests/test_location_api.py @@ -984,6 +984,27 @@ def test_retrieve_with_permissions(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["id"], facility_location_encounter["id"]) + def test_retrieve_facility_location_encounter_with_wrong_location(self): + another_location = self.create_facility_location() + facility_location_encounter = self.create_facility_location_encounter( + self.encounter + ) + self.client.force_authenticate(self.super_user) + url = reverse( + "association-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "location_external_id": another_location["id"], + "external_id": facility_location_encounter["id"], + }, + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.json()["errors"][0]["msg"], + "Bed does not belong to the specified location", + ) + # DELETE TESTS def test_delete_without_permission(self): facility_location_encounter = self.create_facility_location_encounter( From d5e56f2714b1c6b4f4c489db4645b26dc5b69724 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Wed, 10 Jun 2026 15:01:49 +0530 Subject: [PATCH 04/11] fix:added authz retrive func for viewsets --- care/emr/api/viewsets/device.py | 80 +++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/care/emr/api/viewsets/device.py b/care/emr/api/viewsets/device.py index 0caecad727..9725dc74e5 100644 --- a/care/emr/api/viewsets/device.py +++ b/care/emr/api/viewsets/device.py @@ -350,6 +350,17 @@ class DeviceLocationHistoryViewSet(EMRModelReadOnlyViewSet): def get_device(self): return get_object_or_404(Device, external_id=self.kwargs["device_external_id"]) + def authorize_retrieve(self, model_instance): + device = self.get_device() + if device.id != model_instance.device_id: + raise PermissionDenied("You do not have permission to access the device") + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied("You do not have permission to access the device") + def get_queryset(self): device = self.get_device() if not AuthorizationController.call( @@ -376,17 +387,29 @@ def get_queryset(self): Encounter history access is limited to everyone within the location or associated with the managing org """ device = self.get_device() - if not AuthorizationController.call( - "can_read_device", - self.request.user, - device, - ): - raise PermissionDenied("You do not have permission to access the device") - return ( - DeviceEncounterHistory.objects.filter(device=device) - .select_related("encounter", "encounter__patient", "encounter__facility") - .order_by("-end") + queryset = ( + super() + .get_queryset() + .select_related("created_by", "updated_by") + .order_by("-modified_date") ) + if self.action == "list": + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied( + "You do not have permission to access the device" + ) + return ( + DeviceEncounterHistory.objects.filter(device=device) + .select_related( + "encounter", "encounter__patient", "encounter__facility" + ) + .order_by("-end") + ) + return queryset class DeviceServiceHistoryViewSet( @@ -421,6 +444,17 @@ def authorize_create(self, instance): def authorize_update(self, request_obj, model_instance): self.authorize_create(model_instance) + def authorize_retrieve(self, model_instance): + device = self.get_device() + if device.id != model_instance.device_id: + raise PermissionDenied("You do not have permission to access the device") + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied("You do not have permission to access the device") + def perform_update(self, instance): if instance.edit_history and len(instance.edit_history) >= 50: # noqa PLR2004 raise ValidationError("Cannot Edit instance anymore") @@ -441,15 +475,25 @@ def get_queryset(self): Encounter history access is limited to everyone within the location or associated with the managing org """ device = self.get_device() - if not AuthorizationController.call( - "can_read_device", - self.request.user, - device, - ): - raise PermissionDenied("You do not have permission to access the device") - return DeviceServiceHistory.objects.filter(device=device).order_by( - "-serviced_on" + queryset = ( + super() + .get_queryset() + .select_related("created_by", "updated_by") + .order_by("-modified_date") ) + if self.action == "list": + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied( + "You do not have permission to access the device" + ) + return DeviceServiceHistory.objects.filter(device=device).order_by( + "-serviced_on" + ) + return queryset def disassociate_device_from_encounter(instance): From 49ef098c9cd437c4db24ef1a44a1ee48e7d42dd7 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Mon, 15 Jun 2026 12:54:19 +0530 Subject: [PATCH 05/11] fix:added authz retrive func for device related viewsets --- care/emr/api/viewsets/device.py | 50 +++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/care/emr/api/viewsets/device.py b/care/emr/api/viewsets/device.py index 9725dc74e5..4d1c5f7199 100644 --- a/care/emr/api/viewsets/device.py +++ b/care/emr/api/viewsets/device.py @@ -353,7 +353,9 @@ def get_device(self): def authorize_retrieve(self, model_instance): device = self.get_device() if device.id != model_instance.device_id: - raise PermissionDenied("You do not have permission to access the device") + raise ValidationError( + "device does not match with the device location history" + ) if not AuthorizationController.call( "can_read_device", self.request.user, @@ -363,16 +365,27 @@ def authorize_retrieve(self, model_instance): def get_queryset(self): device = self.get_device() - if not AuthorizationController.call( - "can_read_device", self.request.user, device - ): - raise PermissionDenied("You do not have permission to access the device") - - return ( - DeviceLocationHistory.objects.filter(device=device) - .select_related("location") - .order_by("-end") + queryset = ( + super() + .get_queryset() + .select_related("created_by", "updated_by") + .order_by("-modified_date") ) + if self.action == "list": + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied( + "You do not have permission to access the device" + ) + return ( + DeviceLocationHistory.objects.filter(device=device) + .select_related("location") + .order_by("-end") + ) + return queryset class DeviceEncounterHistoryViewSet(EMRModelReadOnlyViewSet): @@ -382,6 +395,19 @@ class DeviceEncounterHistoryViewSet(EMRModelReadOnlyViewSet): def get_device(self): return get_object_or_404(Device, external_id=self.kwargs["device_external_id"]) + def authorize_retrieve(self, model_instance): + device = self.get_device() + if device.id != model_instance.device_id: + raise ValidationError( + "device does not match with the device encounter history" + ) + if not AuthorizationController.call( + "can_read_device", + self.request.user, + device, + ): + raise PermissionDenied("You do not have permission to access the device") + def get_queryset(self): """ Encounter history access is limited to everyone within the location or associated with the managing org @@ -447,7 +473,9 @@ def authorize_update(self, request_obj, model_instance): def authorize_retrieve(self, model_instance): device = self.get_device() if device.id != model_instance.device_id: - raise PermissionDenied("You do not have permission to access the device") + raise ValidationError( + "device does not match with the device service history" + ) if not AuthorizationController.call( "can_read_device", self.request.user, From c5b793f6d1acb208fcc38d4da9a5884791c74a75 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Mon, 15 Jun 2026 12:57:53 +0530 Subject: [PATCH 06/11] added testcases --- care/emr/tests/test_device_api.py | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/care/emr/tests/test_device_api.py b/care/emr/tests/test_device_api.py index 8bd3a0c148..61a4f8345d 100644 --- a/care/emr/tests/test_device_api.py +++ b/care/emr/tests/test_device_api.py @@ -709,6 +709,25 @@ def test_list_device_with_location(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["count"], 1) + def test_retrive_device_location_history_with_mismatched_device(self): + history = self.associate_location_with_device(self.device, self.location) + other_device = self.create_device() + url = reverse( + "device_location_history-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "device_external_id": other_device["id"], + "external_id": history["id"], + }, + ) + self.add_permissions([DevicePermissions.can_list_devices.name]) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.json()["errors"][0]["msg"], + "device does not match with the device location history", + ) + class TestDeviceEncounterHistoryViewSet(DeviceBaseTest): def setUp(self): @@ -769,6 +788,30 @@ def test_retrieve_device_encounter_history(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["id"], history["id"]) + def test_retrive_device_encounter_history_with_mismatched_device(self): + history = self.associate_encounter_with_device(self.device, self.encounter) + other_device = self.create_device() + url = reverse( + "device_encounter_history-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "device_external_id": other_device["id"], + "external_id": history["id"], + }, + ) + self.add_permissions( + [ + DevicePermissions.can_list_devices.name, + EncounterPermissions.can_list_encounter.name, + ] + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.json()["errors"][0]["msg"], + "device does not match with the device encounter history", + ) + class TestDeviceServiceHistoryViewSet(DeviceBaseTest): def setUp(self): @@ -887,3 +930,22 @@ def test_delete_device_service_history(self): ) response = self.client.delete(url) self.assertEqual(response.status_code, 405) # delete doesn't exist + + def test_retrive_device_service_history_with_mismatched_device(self): + history = self.create_device_service_history(self.device) + other_device = self.create_device() + url = reverse( + "device_service_history-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "device_external_id": other_device["id"], + "external_id": history["id"], + }, + ) + self.add_permissions([DevicePermissions.can_list_devices.name]) + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.json()["errors"][0]["msg"], + "device does not match with the device service history", + ) From d71553127cac57b2dc9d97b0654cad74788d45e9 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Mon, 15 Jun 2026 13:09:00 +0530 Subject: [PATCH 07/11] cleanup --- care/emr/api/viewsets/location.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/care/emr/api/viewsets/location.py b/care/emr/api/viewsets/location.py index e17a0d9e26..186a6a2bec 100644 --- a/care/emr/api/viewsets/location.py +++ b/care/emr/api/viewsets/location.py @@ -312,9 +312,6 @@ def authorize_retrieve(self, model_instance): "can_list_facility_location_obj", self.request.user, facility, location ): raise PermissionDenied("You do not have permission to given location") - return FacilityLocationEncounter.objects.filter(location=location).order_by( - "-created_date" - ) def reset_encounter_location_association(self, location): """ From 70967ca949be6999d3f2f1fec979a842e57272d1 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Tue, 16 Jun 2026 17:06:11 +0530 Subject: [PATCH 08/11] fix :added authz check on retrive organization user and added tests --- care/emr/api/viewsets/organization.py | 31 ++++++++++++++++++++----- care/emr/tests/test_organization_api.py | 24 ++++++++++++++++--- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/care/emr/api/viewsets/organization.py b/care/emr/api/viewsets/organization.py index a8edfb227d..3a9455bd6b 100644 --- a/care/emr/api/viewsets/organization.py +++ b/care/emr/api/viewsets/organization.py @@ -449,15 +449,34 @@ def authorize_create(self, instance): ): raise PermissionDenied("User does not have permission for this action") - def get_queryset(self): - """ - Only users part of the organization can access its users - """ + def authorize_retrieve(self, model_instance): organization = self.get_organization_obj() + if model_instance.organization != organization: + raise ValidationError("User does not belong to the organization") if not AuthorizationController.call( "can_list_organization_users_obj", self.request.user, organization ): raise PermissionDenied( - "User does not have the required permissions to list users" + "User does not have the required permission to read user" ) - return OrganizationUser.objects.filter(organization=organization) + + def get_queryset(self): + """ + Only users part of the organization can access its users + """ + organization = self.get_organization_obj() + queryset = ( + super() + .get_queryset() + .select_related("created_by", "updated_by") + .order_by("-modified_date") + ) + if self.action == "list": + if not AuthorizationController.call( + "can_list_organization_users_obj", self.request.user, organization + ): + raise PermissionDenied( + "User does not have the required permission to list users" + ) + return OrganizationUser.objects.filter(organization=organization) + return queryset diff --git a/care/emr/tests/test_organization_api.py b/care/emr/tests/test_organization_api.py index 95841edfa8..e4a1381188 100644 --- a/care/emr/tests/test_organization_api.py +++ b/care/emr/tests/test_organization_api.py @@ -812,10 +812,28 @@ def test_get_users_in_organization_as_user_without_permission(self): self.assertEqual(response.status_code, 403) self.assertContains( response, - "User does not have the required permissions to list users", + "User does not have the required permission to read user", status_code=403, ) + def test_get_users_in_organization_with_invalid_organization(self): + """Test that getting users in an invalid organization returns a 400.""" + self.client.force_authenticate(user=self.super_user) + another_org = self.create_organization( + user=self.super_user, name="Another Organization", org_type="govt" + ) + org_user = self.attach_role_organization_user( + self.root_organization, self.user, self.administrator_role + ) + response = self.client.get( + self.get_detail_url(another_org.external_id, org_user.external_id) + ) + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.data["errors"][0]["msg"], + "User does not belong to the organization", + ) + # getting User List in Organization def test_list_users_in_organization_as_super_user(self): @@ -863,7 +881,7 @@ def test_list_users_in_organization_as_user_without_permission(self): self.assertEqual(response.status_code, 403) self.assertContains( response, - "User does not have the required permissions to list users", + "User does not have the required permission to list users", status_code=403, ) @@ -968,7 +986,7 @@ def test_update_user_in_organization_as_user_without_permission(self): self.assertEqual(response.status_code, 403) self.assertContains( response, - "User does not have the required permissions to list users", + "User does not have permission for this action", status_code=403, ) From facead84aad705273cc85e43638b7b3d57f46f5d Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Tue, 16 Jun 2026 19:00:01 +0530 Subject: [PATCH 09/11] fix:added authz retrive check and tests for notes --- care/emr/api/viewsets/notes.py | 27 ++++++++++++++++++--------- care/emr/tests/test_notes_api.py | 27 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/care/emr/api/viewsets/notes.py b/care/emr/api/viewsets/notes.py index 676bbebc30..35958d877e 100644 --- a/care/emr/api/viewsets/notes.py +++ b/care/emr/api/viewsets/notes.py @@ -120,10 +120,13 @@ def get_patient_obj(self): Patient, external_id=self.kwargs["patient_external_id"] ) - def perform_create(self, instance): - instance.thread = get_object_or_404( + def get_thread_obj(self): + return get_object_or_404( NoteThread, external_id=self.kwargs["thread_external_id"] ) + + def perform_create(self, instance): + instance.thread = self.get_thread_obj() if encounter_id := self.request.data.get("encounter"): encounter = get_object_or_404(Encounter, external_id=encounter_id) if encounter.patient != instance.thread.patient: @@ -138,9 +141,7 @@ def authorize_update(self, request_obj, model_instance): self.authorize_create({}) def authorize_create(self, instance): - thread = get_object_or_404( - NoteThread, external_id=self.kwargs["thread_external_id"] - ) + thread = self.get_thread_obj() if thread.encounter: allowed = AuthorizationController.call( "can_update_encounter_clinical_data", @@ -154,6 +155,11 @@ def authorize_create(self, instance): if not allowed: raise PermissionDenied("You do not have permission for this action") + def authorize_retrieve(self, model_instance): + thread = self.get_thread_obj() + if model_instance.thread != thread: + raise ValidationError("Message does not belong to the thread") + def get_queryset(self): if not AuthorizationController.call( "can_view_clinical_data", self.request.user, self.get_patient_obj() @@ -166,10 +172,13 @@ def get_queryset(self): raise PermissionDenied("Permission denied to user") else: raise PermissionDenied("Permission denied to user") - - return ( + thread = self.get_thread_obj() + queryset = ( super() .get_queryset() - .filter(thread__external_id=self.kwargs["thread_external_id"]) - .order_by("-created_date") + .select_related("created_by", "updated_by") + .order_by("-modified_date") ) + if self.action == "list": + return queryset.filter(thread=thread).order_by("-created_date") + return queryset diff --git a/care/emr/tests/test_notes_api.py b/care/emr/tests/test_notes_api.py index c547985acb..dcfe67d1d6 100644 --- a/care/emr/tests/test_notes_api.py +++ b/care/emr/tests/test_notes_api.py @@ -2,6 +2,11 @@ from model_bakery import baker from care.emr.models.notes import NoteMessage, NoteThread +from care.emr.signals.patient.facility_name_identifier import ( + FacilityPatientNameIdentifierConfig, +) +from care.emr.signals.patient.name_identifier import NameIdentifierConfig +from care.emr.signals.patient.phone_number_identifier import PhoneNumberIdentifierConfig from care.security.permissions.encounter import EncounterPermissions from care.security.permissions.patient import PatientPermissions from care.utils.tests.base import CareAPITestBase @@ -191,7 +196,11 @@ def test_update_thread_without_permission(self): class NoteMessageApiTestCase(CareAPITestBase): def setUp(self): super().setUp() + NameIdentifierConfig.CACHED_CONFIG = {} + PhoneNumberIdentifierConfig.CACHED_CONFIG = {} + FacilityPatientNameIdentifierConfig.CACHED_CONFIG = {} self.user = self.create_user() + self.superuser = self.create_super_user() self.facility = self.create_facility(user=self.user) self.facility_organization = self.create_facility_organization( facility=self.facility @@ -278,6 +287,24 @@ def test_list_notes_on_encounter(self): self.assertEqual(response.status_code, 200, response.data) self.assertContains(response, note.message, status_code=200) + def test_get_note_details_with_invalid_thread(self): + self.client.force_authenticate(user=self.superuser) + thread = self._create_thread() + note = self._create_note(thread) + url = reverse( + "note-detail", + kwargs={ + "patient_external_id": self.patient.external_id, + "thread_external_id": self._create_thread().external_id, + "external_id": note.external_id, + }, + ) + response = self.client.get(url, format="json") + self.assertEqual(response.status_code, 400, response.data) + self.assertContains( + response, "Message does not belong to the thread", status_code=400 + ) + def test_create_note_on_encounter_with_permission(self): role = self.create_role_with_permissions( permissions=[EncounterPermissions.can_write_encounter_clinical_data.name] From ad9076ae6ad967b05dd817d6289c04f120cac000 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Wed, 17 Jun 2026 13:30:06 +0530 Subject: [PATCH 10/11] cleanup --- care/emr/api/viewsets/device.py | 24 ++++++++--------- .../api/viewsets/inventory/inventory_item.py | 27 ++++++++++--------- care/emr/api/viewsets/location.py | 8 +++--- care/emr/api/viewsets/organization.py | 6 ++--- care/emr/api/viewsets/scheduling/token.py | 5 ++-- care/emr/tests/test_device_api.py | 6 ++--- care/emr/tests/test_token_api.py | 4 +++ 7 files changed, 41 insertions(+), 39 deletions(-) diff --git a/care/emr/api/viewsets/device.py b/care/emr/api/viewsets/device.py index 4d1c5f7199..356f1391b6 100644 --- a/care/emr/api/viewsets/device.py +++ b/care/emr/api/viewsets/device.py @@ -352,16 +352,16 @@ def get_device(self): def authorize_retrieve(self, model_instance): device = self.get_device() - if device.id != model_instance.device_id: - raise ValidationError( - "device does not match with the device location history" - ) if not AuthorizationController.call( "can_read_device", self.request.user, device, ): raise PermissionDenied("You do not have permission to access the device") + if device.id != model_instance.device_id: + raise ValidationError( + "device does not match with the device location history" + ) def get_queryset(self): device = self.get_device() @@ -397,16 +397,16 @@ def get_device(self): def authorize_retrieve(self, model_instance): device = self.get_device() - if device.id != model_instance.device_id: - raise ValidationError( - "device does not match with the device encounter history" - ) if not AuthorizationController.call( "can_read_device", self.request.user, device, ): raise PermissionDenied("You do not have permission to access the device") + if device.id != model_instance.device_id: + raise ValidationError( + "device does not match with the device encounter history" + ) def get_queryset(self): """ @@ -472,16 +472,16 @@ def authorize_update(self, request_obj, model_instance): def authorize_retrieve(self, model_instance): device = self.get_device() - if device.id != model_instance.device_id: - raise ValidationError( - "device does not match with the device service history" - ) if not AuthorizationController.call( "can_read_device", self.request.user, device, ): raise PermissionDenied("You do not have permission to access the device") + if device.id != model_instance.device_id: + raise ValidationError( + "device does not match with the device service history" + ) def perform_update(self, instance): if instance.edit_history and len(instance.edit_history) >= 50: # noqa PLR2004 diff --git a/care/emr/api/viewsets/inventory/inventory_item.py b/care/emr/api/viewsets/inventory/inventory_item.py index 21a98eae4d..5c2198c706 100644 --- a/care/emr/api/viewsets/inventory/inventory_item.py +++ b/care/emr/api/viewsets/inventory/inventory_item.py @@ -1,6 +1,6 @@ from django.db.models import Q from django_filters import rest_framework as filters -from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.filters import OrderingFilter from care.emr.api.viewsets.base import EMRBaseViewSet, EMRListMixin, EMRRetrieveMixin @@ -47,24 +47,25 @@ def authorize_location_read(self, location): def authorize_retrieve(self, model_instance): location = self.get_location_obj() + self.authorize_location_read(model_instance.location) if location.id != model_instance.location.id: - raise PermissionDenied( + raise ValidationError( "Inventory item does not belong to the specified location" ) - self.authorize_location_read(model_instance.location) - def get_queryset(self): queryset = super().get_queryset() location = self.get_location_obj() - self.authorize_location_read(location) - include_children = ( - self.request.GET.get("include_children", "false").lower() == "true" - ) - if include_children: - queryset = queryset.filter( - Q(location__parent_cache__overlap=[location.id]) | Q(location=location) + if self.action == "list": + self.authorize_location_read(location) + include_children = ( + self.request.GET.get("include_children", "false").lower() == "true" ) - else: - queryset = queryset.filter(location=location) + if include_children: + queryset = queryset.filter( + Q(location__parent_cache__overlap=[location.id]) + | Q(location=location) + ) + else: + queryset = queryset.filter(location=location) return queryset diff --git a/care/emr/api/viewsets/location.py b/care/emr/api/viewsets/location.py index 186a6a2bec..b00f761d67 100644 --- a/care/emr/api/viewsets/location.py +++ b/care/emr/api/viewsets/location.py @@ -306,12 +306,12 @@ def authorize_destroy(self, instance): def authorize_retrieve(self, model_instance): location = self.get_location_obj() facility = self.get_facility_obj() - if location.id != model_instance.location.id: - raise ValidationError("Bed does not belong to the specified location") if not AuthorizationController.call( "can_list_facility_location_obj", self.request.user, facility, location ): raise PermissionDenied("You do not have permission to given location") + if location.id != model_instance.location.id: + raise ValidationError("Bed does not belong to the specified location") def reset_encounter_location_association(self, location): """ @@ -510,9 +510,7 @@ def get_queryset(self): "can_list_facility_location_obj", self.request.user, facility, location ): raise PermissionDenied("You do not have permission to given location") - return FacilityLocationEncounter.objects.filter(location=location).order_by( - "-created_date" - ) + return queryset.objects.filter(location=location).order_by("-created_date") return queryset diff --git a/care/emr/api/viewsets/organization.py b/care/emr/api/viewsets/organization.py index 3a9455bd6b..f54ef52b8c 100644 --- a/care/emr/api/viewsets/organization.py +++ b/care/emr/api/viewsets/organization.py @@ -451,14 +451,14 @@ def authorize_create(self, instance): def authorize_retrieve(self, model_instance): organization = self.get_organization_obj() - if model_instance.organization != organization: - raise ValidationError("User does not belong to the organization") if not AuthorizationController.call( "can_list_organization_users_obj", self.request.user, organization ): raise PermissionDenied( "User does not have the required permission to read user" ) + if model_instance.organization != organization: + raise ValidationError("User does not belong to the organization") def get_queryset(self): """ @@ -478,5 +478,5 @@ def get_queryset(self): raise PermissionDenied( "User does not have the required permission to list users" ) - return OrganizationUser.objects.filter(organization=organization) + return queryset.filter(organization=organization) return queryset diff --git a/care/emr/api/viewsets/scheduling/token.py b/care/emr/api/viewsets/scheduling/token.py index 8107af9bb7..2d6cf9508e 100644 --- a/care/emr/api/viewsets/scheduling/token.py +++ b/care/emr/api/viewsets/scheduling/token.py @@ -146,9 +146,6 @@ def authorize_destroy(self, instance): def authorize_retrieve(self, model_instance): _, queue = self.get_queue_obj() - if queue != model_instance.queue: - raise ValidationError("Token does not belong to the specified queue") - resource = queue.resource if not AuthorizationController.call( "can_list_token", @@ -156,6 +153,8 @@ def authorize_retrieve(self, model_instance): self.request.user, ): raise PermissionDenied("You do not have permission to read token") + if queue != model_instance.queue: + raise ValidationError("Token does not belong to the specified queue") def get_queryset(self): _, queue = self.get_queue_obj() diff --git a/care/emr/tests/test_device_api.py b/care/emr/tests/test_device_api.py index 61a4f8345d..376d4ac0e9 100644 --- a/care/emr/tests/test_device_api.py +++ b/care/emr/tests/test_device_api.py @@ -709,7 +709,7 @@ def test_list_device_with_location(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["count"], 1) - def test_retrive_device_location_history_with_mismatched_device(self): + def test_retrieve_device_location_history_with_mismatched_device(self): history = self.associate_location_with_device(self.device, self.location) other_device = self.create_device() url = reverse( @@ -788,7 +788,7 @@ def test_retrieve_device_encounter_history(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["id"], history["id"]) - def test_retrive_device_encounter_history_with_mismatched_device(self): + def test_retrieve_device_encounter_history_with_mismatched_device(self): history = self.associate_encounter_with_device(self.device, self.encounter) other_device = self.create_device() url = reverse( @@ -931,7 +931,7 @@ def test_delete_device_service_history(self): response = self.client.delete(url) self.assertEqual(response.status_code, 405) # delete doesn't exist - def test_retrive_device_service_history_with_mismatched_device(self): + def test_retrieve_device_service_history_with_mismatched_device(self): history = self.create_device_service_history(self.device) other_device = self.create_device() url = reverse( diff --git a/care/emr/tests/test_token_api.py b/care/emr/tests/test_token_api.py index d3f5045798..c3811c2b12 100644 --- a/care/emr/tests/test_token_api.py +++ b/care/emr/tests/test_token_api.py @@ -652,6 +652,10 @@ def test_retrieve_token_with_invalid_queue_external_id(self): ) ) self.assertEqual(response.status_code, 400) + self.assertEqual( + response.data["errors"][0]["msg"], + "Token does not belong to the specified queue", + ) # Tests for Token Listing From 8c0d85959761758a264ef04505cfc9bb66a80371 Mon Sep 17 00:00:00 2001 From: nandkishorr Date: Wed, 17 Jun 2026 14:28:23 +0530 Subject: [PATCH 11/11] fix:queryset filtering --- care/emr/api/viewsets/location.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/care/emr/api/viewsets/location.py b/care/emr/api/viewsets/location.py index b00f761d67..8af12b3412 100644 --- a/care/emr/api/viewsets/location.py +++ b/care/emr/api/viewsets/location.py @@ -510,7 +510,7 @@ def get_queryset(self): "can_list_facility_location_obj", self.request.user, facility, location ): raise PermissionDenied("You do not have permission to given location") - return queryset.objects.filter(location=location).order_by("-created_date") + return queryset.filter(location=location).order_by("-created_date") return queryset