Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow read only users from performing writes to asset #2252

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion care/facility/api/viewsets/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class AssetViewSet(
lookup_field = "external_id"
filter_backends = (filters.DjangoFilterBackend, drf_filters.SearchFilter)
search_fields = ["name", "serial_number", "qr_code_id"]
permission_classes = [IsAuthenticated]
permission_classes = (IsAuthenticated, DRYPermissions)
filterset_class = AssetFilter

def get_queryset(self):
Expand Down
20 changes: 20 additions & 0 deletions care/facility/models/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,26 @@ def delete(self, *args, **kwargs):
AssetBed.objects.filter(asset=self).update(deleted=True)
super().delete(*args, **kwargs)

@staticmethod
def has_write_permission(request):
if request.user.asset or request.user.user_type in User.READ_ONLY_TYPES:
return False
return (
request.user.is_superuser
or request.user.verified
and request.user.user_type >= User.TYPE_VALUE_MAP["Staff"]
)

def has_object_write_permission(self, request):
return self.has_write_permission(request)

@staticmethod
def has_read_permission(request):
return request.user.is_superuser or request.user.verified

def has_object_read_permission(self, request):
return self.has_read_permission(request)

def __str__(self):
return self.name

Expand Down
16 changes: 16 additions & 0 deletions care/facility/tests/test_asset_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from rest_framework.test import APITestCase

from care.facility.models import Asset, Bed
from care.users.models import User
from care.utils.assetintegration.asset_classes import AssetClasses
from care.utils.tests.test_utils import TestUtils

Expand All @@ -17,6 +18,11 @@ def setUpTestData(cls) -> None:
cls.facility = cls.create_facility(cls.super_user, cls.district, cls.local_body)
cls.asset_location = cls.create_asset_location(cls.facility)
cls.user = cls.create_user("staff", cls.district, home_facility=cls.facility)
cls.state_admin_ro = cls.create_user(
"stateadmin-ro",
cls.district,
user_type=User.TYPE_VALUE_MAP["StateReadOnlyAdmin"],
)
cls.patient = cls.create_patient(
cls.district, cls.facility, local_body=cls.local_body
)
Expand All @@ -38,6 +44,16 @@ def test_create_asset(self):
response = self.client.post("/api/v1/asset/", sample_data)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)

def test_create_asset_read_only(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rithviknishad Shouldn't we have tests for all ReadOnly Users?

Copy link
Member Author

@rithviknishad rithviknishad Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gigincg this would require creating user objects for each of the user type in the test's setup.

Checking with one of the read-only user type seems to be fine right?

cc: @sainak

sample_data = {
"name": "Test Asset",
"asset_type": 50,
"location": self.asset_location.external_id,
}
self.client.force_authenticate(self.state_admin_ro)
response = self.client.post("/api/v1/asset/", sample_data)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_create_asset_with_warranty_past(self):
sample_data = {
"name": "Test Asset",
Expand Down