From 8331d9a2c158ad25a2977f83434801b12c2f2377 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Thu, 22 Sep 2022 21:09:11 +0530 Subject: [PATCH] add dry permissions for facility cover_image (#1003) * add dry permissions for facility cover_image * fix read_cover_image_url returning incomplete url * refactor facility cover image endpoint --- care/facility/api/serializers/facility.py | 39 +++++++++------- care/facility/api/viewsets/facility.py | 44 +++++++++++-------- care/facility/models/facility.py | 4 +- .../models/mixins/permissions/facility.py | 30 ++++++++++--- 4 files changed, 74 insertions(+), 43 deletions(-) diff --git a/care/facility/api/serializers/facility.py b/care/facility/api/serializers/facility.py index ed026424f5..8368dc3514 100644 --- a/care/facility/api/serializers/facility.py +++ b/care/facility/api/serializers/facility.py @@ -1,17 +1,18 @@ -from distutils import extension import boto3 - -from django.contrib.auth import get_user_model from django.conf import settings +from django.contrib.auth import get_user_model from rest_framework import serializers -from care.facility.api.serializers.facility_capacity import FacilityCapacitySerializer from care.facility.models import FACILITY_TYPES, Facility, FacilityLocalGovtBody from care.facility.models.facility import FEATURE_CHOICES -from care.facility.models.patient_base import reverse_choices -from care.users.api.serializers.lsg import DistrictSerializer, LocalBodySerializer, StateSerializer, WardSerializer -from config.serializers import ChoiceField +from care.users.api.serializers.lsg import ( + DistrictSerializer, + LocalBodySerializer, + StateSerializer, + WardSerializer, +) from care.utils.csp import config as cs_provider +from config.serializers import ChoiceField User = get_user_model() @@ -47,7 +48,10 @@ class FacilityBasicInfoSerializer(serializers.ModelSerializer): features = serializers.MultipleChoiceField(choices=FEATURE_CHOICES) def get_facility_type(self, facility): - return {"id": facility.facility_type, "name": facility.get_facility_type_display()} + return { + "id": facility.facility_type, + "name": facility.get_facility_type_display(), + } class Meta: model = Facility @@ -75,7 +79,7 @@ class FacilitySerializer(FacilityBasicInfoSerializer): # "latitude": 49.8782482189424, # "longitude": 24.452545489 # } - read_cover_image_url = serializers.CharField(read_only=True) + read_cover_image_url = serializers.URLField(read_only=True) # location = PointField(required=False) features = serializers.MultipleChoiceField(choices=FEATURE_CHOICES) @@ -121,25 +125,26 @@ def create(self, validated_data): class FacilityImageUploadSerializer(serializers.ModelSerializer): cover_image = serializers.ImageField(required=True, write_only=True) + read_cover_image_url = serializers.URLField(read_only=True) class Meta: model = Facility - fields = ("cover_image",) + # Check DRYpermissions before updating + fields = ("cover_image", "read_cover_image_url") def save(self, **kwargs): facility = self.instance image = self.validated_data["cover_image"] - image_extension = image.name.split(".")[-1] + image_extension = image.name.rsplit(".", 1)[-1] s3 = boto3.client( - "s3", - **cs_provider.get_client_config(cs_provider.BucketType.FACILITY.value) + "s3", **cs_provider.get_client_config(cs_provider.BucketType.FACILITY.value) ) - upload_response = s3.put_object( + image_location = f"cover_images/{facility.external_id}_cover.{image_extension}" + s3.put_object( Bucket=settings.FACILITY_S3_BUCKET, - Key=f"cover_images/{facility.external_id}_cover.{image_extension}", + Key=image_location, Body=image.file, ) - print(upload_response['ResponseMetadata']['HTTPHeaders']['location']) - facility.cover_image_url = f"cover_images/{facility.external_id}_cover.{image_extension}" + facility.cover_image_url = image_location facility.save() return facility diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index 4376d97fd6..ebf0ee46ab 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -5,6 +5,7 @@ from rest_framework import filters as drf_filters from rest_framework import mixins, status, viewsets from rest_framework.decorators import action +from rest_framework.parsers import MultiPartParser from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -12,14 +13,13 @@ FacilityBasicInfoSerializer, FacilityImageUploadSerializer, FacilitySerializer, - FacilityImageUploadSerializer ) from care.facility.models import ( Facility, FacilityCapacity, FacilityPatientStatsHistory, HospitalDoctors, - PatientRegistration + PatientRegistration, ) from care.users.models import User @@ -85,9 +85,21 @@ class FacilityViewSet( FACILITY_DOCTORS_CSV_KEY = "doctors" FACILITY_TRIAGE_CSV_KEY = "triage" + def initialize_request(self, request, *args, **kwargs): + self.action = self.action_map.get(request.method.lower()) + return super().initialize_request(request, *args, **kwargs) + + def get_parsers(self): + if self.action == "cover_image": + return [MultiPartParser()] + return super().get_parsers() + def get_serializer_class(self): if self.request.query_params.get("all") == "true": return FacilityBasicInfoSerializer + if self.action == "cover_image": + # Check DRYpermissions before updating + return FacilityImageUploadSerializer else: return FacilitySerializer @@ -129,24 +141,20 @@ def list(self, request, *args, **kwargs): return super(FacilityViewSet, self).list(request, *args, **kwargs) - @action(methods=["POST", "DELETE"], detail=True, permission_classes=[IsAuthenticated, DRYPermissions]) + @action(methods=["POST"], detail=True) def cover_image(self, request, external_id): facility = self.get_object() - if not facility: - return Response({"facility": "does not exist"}, status=status.HTTP_404_NOT_FOUND) - - if request.method == "POST": - serialized_data = FacilityImageUploadSerializer(facility, data=request.data) - if serialized_data.is_valid(): - serialized_data.save() - return Response(serialized_data.data) - - return Response(serialized_data.errors, status=status.HTTP_400_BAD_REQUEST) - - if request.method == "DELETE": - facility.cover_image_url = None - facility.save() - return Response(status=status.HTTP_204_NO_CONTENT) + serializer = FacilityImageUploadSerializer(facility, data=request.data) + serializer.is_valid(raise_exception=True) + serializer.save() + return Response(serializer.data) + + @cover_image.mapping.delete + def cover_image_delete(self, *args, **kwargs): + facility = self.get_object() + facility.cover_image_url = None + facility.save() + return Response(status=status.HTTP_204_NO_CONTENT) class AllFacilityViewSet( diff --git a/care/facility/models/facility.py b/care/facility/models/facility.py index cdc7dc96e2..3538fbe6a2 100644 --- a/care/facility/models/facility.py +++ b/care/facility/models/facility.py @@ -168,7 +168,9 @@ class Meta: verbose_name_plural = "Facilities" def read_cover_image_url(self): - return settings.FACILITY_S3_STATIC_PREFIX + (self.cover_image_url or "") + if self.cover_image_url: + return settings.FACILITY_S3_STATIC_PREFIX + self.cover_image_url + return None def __str__(self): return f"{self.name}" diff --git a/care/facility/models/mixins/permissions/facility.py b/care/facility/models/mixins/permissions/facility.py index 0ab1a3fe8b..190d1eb508 100644 --- a/care/facility/models/mixins/permissions/facility.py +++ b/care/facility/models/mixins/permissions/facility.py @@ -9,23 +9,27 @@ def has_bulk_upsert_permission(request): @staticmethod def has_write_permission(request): - from care.users.models import State, District, LocalBody + from care.users.models import District, LocalBody, State + try: state = State.objects.get(id=request.data["state"]) - district = District.objects.get(id=request.data["district"]) + district = District.objects.get(id=request.data["district"]) local_body = LocalBody.objects.get(id=request.data["local_body"]) return ( - request.user.is_superuser or ( + request.user.is_superuser + or ( request.user.user_type <= User.TYPE_VALUE_MAP["LocalBodyAdmin"] and state == request.user.state and district == request.user.district and local_body == request.user.local_body - ) or ( + ) + or ( request.user.user_type > User.TYPE_VALUE_MAP["LocalBodyAdmin"] and request.user.user_type <= User.TYPE_VALUE_MAP["DistrictAdmin"] and state == request.user.state and district == request.user.district - ) or ( + ) + or ( request.user.user_type > User.TYPE_VALUE_MAP["DistrictAdmin"] and request.user.user_type <= User.TYPE_VALUE_MAP["StateAdmin"] and state == request.user.state @@ -34,6 +38,11 @@ def has_write_permission(request): except Exception: return False + @staticmethod + def has_cover_image_permission(request): + # Returning true here as the permission is validated at object level for this action + return True + def has_object_read_permission(self, request): return ( (request.user.is_superuser) @@ -62,11 +71,16 @@ def has_object_write_permission(self, request): return self.has_object_read_permission(request) def has_object_update_permission(self, request): - return super().has_object_update_permission(request) or self.has_object_write_permission(request) + return super().has_object_update_permission( + request + ) or self.has_object_write_permission(request) def has_object_destroy_permission(self, request): return self.has_object_read_permission(request) + def has_object_cover_image_permission(self, request): + return self.has_object_update_permission(request) + class FacilityRelatedPermissionMixin(BasePermissionMixin): @staticmethod @@ -82,7 +96,9 @@ def has_write_permission(request): facility = False try: - facility = Facility.objects.get(external_id=request.parser_context["kwargs"]["facility_external_id"]) + facility = Facility.objects.get( + external_id=request.parser_context["kwargs"]["facility_external_id"] + ) except Facility.DoesNotExist: return False return (request.user.is_superuser or request.user.verified) and (