From 927f2883bd6a05e4cc8e2bf2c089198878369bf5 Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Fri, 7 Jun 2024 12:21:22 +0530 Subject: [PATCH 01/17] new profile pic feature --- .env.example | 1 + aws/backend.json | 8 +++++ aws/celery.json | 24 +++++++++++++++ care/users/api/serializers/user.py | 30 +++++++++++++++++++ care/users/api/viewsets/users.py | 26 ++++++++++++++++ .../0017_user_profile_picture_url.py | 18 +++++++++++ care/users/models.py | 8 +++++ care/users/tests/test_api.py | 1 + care/utils/csp/config.py | 12 ++++++++ config/settings/base.py | 13 ++++++++ data/dummy/users.json | 24 +++++++++++++++ docker/.local.env | 1 + docker/.prebuilt.env | 1 + docker/awslocal/bucket-setup.sh | 1 + 14 files changed, 168 insertions(+) create mode 100644 care/users/migrations/0017_user_profile_picture_url.py diff --git a/.env.example b/.env.example index 091bea02fd..4dec231b05 100644 --- a/.env.example +++ b/.env.example @@ -18,3 +18,4 @@ BUCKET_ENDPOINT=http://localhost:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket +USER_S3_BUCKET=user-bucket diff --git a/aws/backend.json b/aws/backend.json index 3d419d67c7..4b7757febd 100644 --- a/aws/backend.json +++ b/aws/backend.json @@ -81,6 +81,14 @@ "name": "FACILITY_S3_STATIC_PREFIX", "value": "https://egov-s3-facility-10bedicu.s3.ap-south-1.amazonaws.com/egov-s3-facility-10bedicu" }, + { + "name": "USER_S3_BUCKET", + "value": "egov-s3-user-10bedicu" + }, + { + "name": "USER_S3_BUCKET_ENDPOINT", + "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" + }, { "name": "FILE_UPLOAD_BUCKET", "value": "egov-s3-patient-data-10bedicu" diff --git a/aws/celery.json b/aws/celery.json index 524a548144..4b05779ccf 100644 --- a/aws/celery.json +++ b/aws/celery.json @@ -76,6 +76,14 @@ "name": "FILE_UPLOAD_BUCKET_ENDPOINT", "value": "https://egov-s3-patient-data-10bedicu.s3.amazonaws.com" }, + { + "name": "USER_S3_BUCKET", + "value": "egov-s3-user-10bedicu" + }, + { + "name": "USER_S3_BUCKET_ENDPOINT", + "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" + }, { "name": "MAINTENANCE_MODE", "value": "0" @@ -355,6 +363,14 @@ "name": "FILE_UPLOAD_BUCKET_ENDPOINT", "value": "https://egov-s3-patient-data-10bedicu.s3.amazonaws.com" }, + { + "name": "USER_S3_BUCKET", + "value": "egov-s3-user-10bedicu" + }, + { + "name": "USER_S3_BUCKET_ENDPOINT", + "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" + }, { "name": "MAINTENANCE_MODE", "value": "0" @@ -493,6 +509,14 @@ "valueFrom": "/care/backend/FACILITY_S3_SECRET", "name": "FACILITY_S3_SECRET" }, + { + "valueFrom": "/care/backend/USER_S3_KEY", + "name": "USER_S3_KEY" + }, + { + "valueFrom": "/care/backend/USER_S3_SECRET", + "name": "USER_S3_SECRET" + }, { "valueFrom": "/care/backend/VAPID_PUBLIC_KEY", "name": "VAPID_PUBLIC_KEY" diff --git a/care/users/api/serializers/user.py b/care/users/api/serializers/user.py index 9e267ac98e..987ab281c3 100644 --- a/care/users/api/serializers/user.py +++ b/care/users/api/serializers/user.py @@ -1,3 +1,4 @@ +import boto3 from django.contrib.auth.hashers import make_password from django.db import transaction from django.utils.timezone import now @@ -15,6 +16,7 @@ from care.utils.queryset.facility import get_home_facility_queryset from care.utils.serializer.external_id_field import ExternalIdSerializerField from config.serializers import ChoiceField +from care.utils.csp.config import BucketType, get_client_config class SignUpSerializer(serializers.ModelSerializer): @@ -279,6 +281,7 @@ class UserSerializer(SignUpSerializer): source="home_facility", read_only=True, ) + read_profile_picture_url = serializers.URLField(read_only=True) home_facility = ExternalIdSerializerField(queryset=Facility.objects.all()) @@ -316,6 +319,7 @@ class Meta: "pf_endpoint", "pf_p256dh", "pf_auth", + "read_profile_picture_url", ) read_only_fields = ( "is_superuser", @@ -409,6 +413,7 @@ class UserListSerializer(serializers.ModelSerializer): read_only=True, ) home_facility = ExternalIdSerializerField(queryset=Facility.objects.all()) + read_profile_picture_url = serializers.URLField(read_only=True) class Meta: model = User @@ -431,4 +436,29 @@ class Meta: "home_facility_object", "home_facility", "video_connect_link", + "read_profile_picture_url", ) + +class UserImageUploadSerializer(serializers.ModelSerializer): + profile_picture_url = serializers.ImageField(required=True, write_only=True) + read_profile_picture_url = serializers.CharField(read_only=True) + + class Meta: + model = User + fields = ("profile_picture_url", "read_profile_picture_url") + + def save(self, **kwargs): + user = self.instance + image = self.validated_data["profile_picture_url"] + image_extension = image.name.split(".")[-1] + config, bucket_name = get_client_config(BucketType.USER, True) + s3 = boto3.client("s3", **config) + image_location = f"{user.external_id}/profile.{image_extension}" + s3.put_object( + Bucket=bucket_name, + Key=image_location, + Body=image.file, + ) + user.profile_picture_url = image_location + user.save() + return user diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index f2152d3762..02551dde6c 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -12,6 +12,7 @@ from rest_framework.response import Response from rest_framework.serializers import ValidationError from rest_framework.viewsets import GenericViewSet +from rest_framework.parsers import MultiPartParser from care.facility.api.serializers.facility import FacilityBasicInfoSerializer from care.facility.models.facility import Facility, FacilityUser @@ -19,6 +20,7 @@ UserCreateSerializer, UserListSerializer, UserSerializer, + UserImageUploadSerializer, ) from care.users.models import User from care.utils.cache.cache_allowed_facilities import get_accessible_facilities @@ -150,6 +152,11 @@ def get_queryset(self): ) return self.queryset.filter(query) + def get_parsers(self): + if self.request.method == "POST" and self.request.path.endswith("profile_picture"): + return [MultiPartParser()] + return super().get_parsers() + def get_serializer_class(self): if self.action == "list": return UserListSerializer @@ -157,6 +164,8 @@ def get_serializer_class(self): return UserCreateSerializer # elif self.action == "create": # return SignUpSerializer + elif self.action == "profile_picture": + return UserImageUploadSerializer else: return UserSerializer @@ -359,3 +368,20 @@ def check_availability(self, request, username): if User.check_username_exists(username): return Response(status=status.HTTP_409_CONFLICT) return Response(status=status.HTTP_200_OK) + + @extend_schema(tags=["users"]) + @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) + def profile_picture(self, request, username): + user = self.get_object() + serializer = UserImageUploadSerializer(user, data=request.data) + serializer.is_valid(raise_exception=True) + serializer.save() + return Response(status=status.HTTP_200_OK) + + @extend_schema(tags=["users"]) + @profile_picture.mapping.delete + def profile_picture_delete(self, *args, **kwargs): + user = self.get_object() + user.profile_picture_url = None + user.save() + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/care/users/migrations/0017_user_profile_picture_url.py b/care/users/migrations/0017_user_profile_picture_url.py new file mode 100644 index 0000000000..77771b05e0 --- /dev/null +++ b/care/users/migrations/0017_user_profile_picture_url.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-06-07 06:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0016_upgrade_user_skills"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="profile_picture_url", + field=models.CharField(blank=True, default=None, max_length=500, null=True), + ), + ] diff --git a/care/users/models.py b/care/users/models.py index c06ae3e43c..347451b72b 100644 --- a/care/users/models.py +++ b/care/users/models.py @@ -247,6 +247,9 @@ class User(AbstractUser): gender = models.IntegerField(choices=GENDER_CHOICES, blank=False) date_of_birth = models.DateField(null=True, blank=True) + profile_picture_url = models.CharField( + blank=True, null=True, default=None, max_length=500 + ) skills = models.ManyToManyField("Skill", through=UserSkill) home_facility = models.ForeignKey( "facility.Facility", on_delete=models.PROTECT, null=True, blank=True @@ -311,6 +314,11 @@ class User(AbstractUser): CSV_MAKE_PRETTY = {"user_type": (lambda x: User.REVERSE_TYPE_MAP[x])} + def read_profile_picture_url(self): + if self.profile_picture_url: + return f"{settings.USER_S3_BUCKET_EXTERNAL_ENDPOINT}/{settings.USER_S3_BUCKET}/{self.profile_picture_url}" + return None + @staticmethod def has_read_permission(request): return True diff --git a/care/users/tests/test_api.py b/care/users/tests/test_api.py index 90ca8b45e4..b038932403 100644 --- a/care/users/tests/test_api.py +++ b/care/users/tests/test_api.py @@ -45,6 +45,7 @@ def get_detail_representation(self, obj=None) -> dict: "doctor_qualification": obj.doctor_qualification, "weekly_working_hours": obj.weekly_working_hours, "video_connect_link": obj.video_connect_link, + "profile_picture_url": obj.profile_picture_url, **self.get_local_body_district_state_representation(obj), } diff --git a/care/utils/csp/config.py b/care/utils/csp/config.py index dbd145b536..30bf4daf23 100644 --- a/care/utils/csp/config.py +++ b/care/utils/csp/config.py @@ -24,6 +24,7 @@ class CSProvider(enum.Enum): class BucketType(enum.Enum): PATIENT = "PATIENT" FACILITY = "FACILITY" + USER = "USER" def get_facility_bucket_config(external) -> tuple[ClientConfig, BucketName]: @@ -47,10 +48,21 @@ def get_patient_bucket_config(external) -> tuple[ClientConfig, BucketName]: else settings.FILE_UPLOAD_BUCKET_ENDPOINT, }, settings.FILE_UPLOAD_BUCKET +def get_user_bucket_config(external) -> tuple[ClientConfig, BucketName]: + return { + "region_name": settings.USER_S3_REGION, + "aws_access_key_id": settings.USER_S3_KEY, + "aws_secret_access_key": settings.USER_S3_SECRET, + "endpoint_url": settings.USER_S3_BUCKET_EXTERNAL_ENDPOINT + if external + else settings.USER_S3_BUCKET_ENDPOINT, + }, settings.USER_S3_BUCKET def get_client_config(bucket_type: BucketType, external=False): if bucket_type == BucketType.FACILITY: return get_facility_bucket_config(external=external) elif bucket_type == BucketType.PATIENT: return get_patient_bucket_config(external=external) + elif bucket_type == BucketType.USER: + return get_user_bucket_config(external=external) raise ValueError("Invalid Bucket Type") diff --git a/config/settings/base.py b/config/settings/base.py index 03e764e8b3..f06fd4c631 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -571,6 +571,19 @@ else FACILITY_S3_BUCKET_ENDPOINT, ) +USER_S3_BUCKET = env("USER_S3_BUCKET", default="") +USER_S3_REGION = env("USER_S3_REGION_CODE", default=BUCKET_REGION) +USER_S3_KEY = env("USER_S3_KEY", default=BUCKET_KEY) +USER_S3_SECRET = env("USER_S3_SECRET", default=BUCKET_SECRET) +USER_S3_BUCKET_ENDPOINT = env("USER_S3_BUCKET_ENDPOINT", default=BUCKET_ENDPOINT) +USER_S3_BUCKET_EXTERNAL_ENDPOINT = env( + "USER_S3_BUCKET_EXTERNAL_ENDPOINT", + default= BUCKET_EXTERNAL_ENDPOINT + if BUCKET_ENDPOINT + else USER_S3_BUCKET_ENDPOINT, +) + + # for setting the shifting mode PEACETIME_MODE = env.bool("PEACETIME_MODE", default=True) diff --git a/data/dummy/users.json b/data/dummy/users.json index 5f20d2f3b6..fc55f68670 100644 --- a/data/dummy/users.json +++ b/data/dummy/users.json @@ -31,6 +31,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -67,6 +68,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -103,6 +105,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -139,6 +142,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -175,6 +179,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -211,6 +216,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -247,6 +253,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -283,6 +290,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -319,6 +327,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -355,6 +364,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -391,6 +401,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -427,6 +438,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -463,6 +475,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -499,6 +512,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -535,6 +549,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -571,6 +586,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -607,6 +623,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -643,6 +660,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -679,6 +697,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -715,6 +734,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -751,6 +771,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -787,6 +808,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -823,6 +845,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } @@ -859,6 +882,7 @@ "pf_p256dh": null, "pf_auth": null, "asset": null, + "profile_picture_url": null, "groups": [], "user_permissions": [] } diff --git a/docker/.local.env b/docker/.local.env index ccfaef5fba..c39aa57060 100644 --- a/docker/.local.env +++ b/docker/.local.env @@ -16,3 +16,4 @@ BUCKET_ENDPOINT=http://localstack:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket +USER_S3_BUCKET=user-bucket diff --git a/docker/.prebuilt.env b/docker/.prebuilt.env index 02267439a7..1a4ebb55eb 100644 --- a/docker/.prebuilt.env +++ b/docker/.prebuilt.env @@ -16,6 +16,7 @@ BUCKET_ENDPOINT=http://localstack:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket +USER_S3_BUCKET=user-bucket SNS_ACCESS_KEY=123 SNS_SECRET_KEY=123 diff --git a/docker/awslocal/bucket-setup.sh b/docker/awslocal/bucket-setup.sh index 05025d1ed8..219cd34091 100755 --- a/docker/awslocal/bucket-setup.sh +++ b/docker/awslocal/bucket-setup.sh @@ -3,4 +3,5 @@ set -x awslocal s3 mb s3://patient-bucket awslocal s3 mb s3://facility-bucket +awslocal s3 mb s3://user-bucket set +x From c30735bdb3369b35df1f161007962edc4d3f065c Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Fri, 7 Jun 2024 12:52:27 +0530 Subject: [PATCH 02/17] bug fix --- care/users/api/serializers/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/care/users/api/serializers/user.py b/care/users/api/serializers/user.py index 987ab281c3..422e47767a 100644 --- a/care/users/api/serializers/user.py +++ b/care/users/api/serializers/user.py @@ -451,7 +451,7 @@ def save(self, **kwargs): user = self.instance image = self.validated_data["profile_picture_url"] image_extension = image.name.split(".")[-1] - config, bucket_name = get_client_config(BucketType.USER, True) + config, bucket_name = get_client_config(BucketType.USER) s3 = boto3.client("s3", **config) image_location = f"{user.external_id}/profile.{image_extension}" s3.put_object( From fc4c573c26cca46475b0a46e7834faade5f7d7ee Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Tue, 9 Jul 2024 15:29:15 +0530 Subject: [PATCH 03/17] removes new bucket --- .env.example | 1 - aws/backend.json | 8 -------- aws/celery.json | 24 ------------------------ care/users/api/serializers/user.py | 4 ++-- care/users/models.py | 2 +- care/utils/csp/config.py | 13 ------------- config/settings/base.py | 13 ------------- docker/.local.env | 1 - docker/.prebuilt.env | 1 - docker/awslocal/bucket-setup.sh | 1 - 10 files changed, 3 insertions(+), 65 deletions(-) diff --git a/.env.example b/.env.example index 4dec231b05..091bea02fd 100644 --- a/.env.example +++ b/.env.example @@ -18,4 +18,3 @@ BUCKET_ENDPOINT=http://localhost:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket -USER_S3_BUCKET=user-bucket diff --git a/aws/backend.json b/aws/backend.json index 4b7757febd..3d419d67c7 100644 --- a/aws/backend.json +++ b/aws/backend.json @@ -81,14 +81,6 @@ "name": "FACILITY_S3_STATIC_PREFIX", "value": "https://egov-s3-facility-10bedicu.s3.ap-south-1.amazonaws.com/egov-s3-facility-10bedicu" }, - { - "name": "USER_S3_BUCKET", - "value": "egov-s3-user-10bedicu" - }, - { - "name": "USER_S3_BUCKET_ENDPOINT", - "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" - }, { "name": "FILE_UPLOAD_BUCKET", "value": "egov-s3-patient-data-10bedicu" diff --git a/aws/celery.json b/aws/celery.json index 4b05779ccf..524a548144 100644 --- a/aws/celery.json +++ b/aws/celery.json @@ -76,14 +76,6 @@ "name": "FILE_UPLOAD_BUCKET_ENDPOINT", "value": "https://egov-s3-patient-data-10bedicu.s3.amazonaws.com" }, - { - "name": "USER_S3_BUCKET", - "value": "egov-s3-user-10bedicu" - }, - { - "name": "USER_S3_BUCKET_ENDPOINT", - "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" - }, { "name": "MAINTENANCE_MODE", "value": "0" @@ -363,14 +355,6 @@ "name": "FILE_UPLOAD_BUCKET_ENDPOINT", "value": "https://egov-s3-patient-data-10bedicu.s3.amazonaws.com" }, - { - "name": "USER_S3_BUCKET", - "value": "egov-s3-user-10bedicu" - }, - { - "name": "USER_S3_BUCKET_ENDPOINT", - "value": "https://egov-s3-user-10bedicu.s3.amazonaws.com" - }, { "name": "MAINTENANCE_MODE", "value": "0" @@ -509,14 +493,6 @@ "valueFrom": "/care/backend/FACILITY_S3_SECRET", "name": "FACILITY_S3_SECRET" }, - { - "valueFrom": "/care/backend/USER_S3_KEY", - "name": "USER_S3_KEY" - }, - { - "valueFrom": "/care/backend/USER_S3_SECRET", - "name": "USER_S3_SECRET" - }, { "valueFrom": "/care/backend/VAPID_PUBLIC_KEY", "name": "VAPID_PUBLIC_KEY" diff --git a/care/users/api/serializers/user.py b/care/users/api/serializers/user.py index 422e47767a..2d643a56ca 100644 --- a/care/users/api/serializers/user.py +++ b/care/users/api/serializers/user.py @@ -451,9 +451,9 @@ def save(self, **kwargs): user = self.instance image = self.validated_data["profile_picture_url"] image_extension = image.name.split(".")[-1] - config, bucket_name = get_client_config(BucketType.USER) + config, bucket_name = get_client_config(BucketType.FACILITY) s3 = boto3.client("s3", **config) - image_location = f"{user.external_id}/profile.{image_extension}" + image_location = f"avatars/{user.external_id}.{image_extension}" s3.put_object( Bucket=bucket_name, Key=image_location, diff --git a/care/users/models.py b/care/users/models.py index 347451b72b..3038ff08a8 100644 --- a/care/users/models.py +++ b/care/users/models.py @@ -316,7 +316,7 @@ class User(AbstractUser): def read_profile_picture_url(self): if self.profile_picture_url: - return f"{settings.USER_S3_BUCKET_EXTERNAL_ENDPOINT}/{settings.USER_S3_BUCKET}/{self.profile_picture_url}" + return f"{settings.FACILITY_S3_BUCKET_EXTERNAL_ENDPOINT}/{settings.FACILITY_S3_BUCKET}/{self.profile_picture_url}" return None @staticmethod diff --git a/care/utils/csp/config.py b/care/utils/csp/config.py index 30bf4daf23..d55ffce439 100644 --- a/care/utils/csp/config.py +++ b/care/utils/csp/config.py @@ -24,7 +24,6 @@ class CSProvider(enum.Enum): class BucketType(enum.Enum): PATIENT = "PATIENT" FACILITY = "FACILITY" - USER = "USER" def get_facility_bucket_config(external) -> tuple[ClientConfig, BucketName]: @@ -48,21 +47,9 @@ def get_patient_bucket_config(external) -> tuple[ClientConfig, BucketName]: else settings.FILE_UPLOAD_BUCKET_ENDPOINT, }, settings.FILE_UPLOAD_BUCKET -def get_user_bucket_config(external) -> tuple[ClientConfig, BucketName]: - return { - "region_name": settings.USER_S3_REGION, - "aws_access_key_id": settings.USER_S3_KEY, - "aws_secret_access_key": settings.USER_S3_SECRET, - "endpoint_url": settings.USER_S3_BUCKET_EXTERNAL_ENDPOINT - if external - else settings.USER_S3_BUCKET_ENDPOINT, - }, settings.USER_S3_BUCKET - def get_client_config(bucket_type: BucketType, external=False): if bucket_type == BucketType.FACILITY: return get_facility_bucket_config(external=external) elif bucket_type == BucketType.PATIENT: return get_patient_bucket_config(external=external) - elif bucket_type == BucketType.USER: - return get_user_bucket_config(external=external) raise ValueError("Invalid Bucket Type") diff --git a/config/settings/base.py b/config/settings/base.py index d4f35b36fd..c9430e24f8 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -583,19 +583,6 @@ ), ) -USER_S3_BUCKET = env("USER_S3_BUCKET", default="") -USER_S3_REGION = env("USER_S3_REGION_CODE", default=BUCKET_REGION) -USER_S3_KEY = env("USER_S3_KEY", default=BUCKET_KEY) -USER_S3_SECRET = env("USER_S3_SECRET", default=BUCKET_SECRET) -USER_S3_BUCKET_ENDPOINT = env("USER_S3_BUCKET_ENDPOINT", default=BUCKET_ENDPOINT) -USER_S3_BUCKET_EXTERNAL_ENDPOINT = env( - "USER_S3_BUCKET_EXTERNAL_ENDPOINT", - default= BUCKET_EXTERNAL_ENDPOINT - if BUCKET_ENDPOINT - else USER_S3_BUCKET_ENDPOINT, -) - - # for setting the shifting mode PEACETIME_MODE = env.bool("PEACETIME_MODE", default=True) diff --git a/docker/.local.env b/docker/.local.env index c39aa57060..ccfaef5fba 100644 --- a/docker/.local.env +++ b/docker/.local.env @@ -16,4 +16,3 @@ BUCKET_ENDPOINT=http://localstack:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket -USER_S3_BUCKET=user-bucket diff --git a/docker/.prebuilt.env b/docker/.prebuilt.env index 1a4ebb55eb..02267439a7 100644 --- a/docker/.prebuilt.env +++ b/docker/.prebuilt.env @@ -16,7 +16,6 @@ BUCKET_ENDPOINT=http://localstack:4566 BUCKET_EXTERNAL_ENDPOINT=http://localhost:4566 FILE_UPLOAD_BUCKET=patient-bucket FACILITY_S3_BUCKET=facility-bucket -USER_S3_BUCKET=user-bucket SNS_ACCESS_KEY=123 SNS_SECRET_KEY=123 diff --git a/docker/awslocal/bucket-setup.sh b/docker/awslocal/bucket-setup.sh index 219cd34091..05025d1ed8 100755 --- a/docker/awslocal/bucket-setup.sh +++ b/docker/awslocal/bucket-setup.sh @@ -3,5 +3,4 @@ set -x awslocal s3 mb s3://patient-bucket awslocal s3 mb s3://facility-bucket -awslocal s3 mb s3://user-bucket set +x From 4fb991088685e820c6f231204a9131e372d7576e Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Tue, 9 Jul 2024 15:34:29 +0530 Subject: [PATCH 04/17] fix bug --- care/users/api/viewsets/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index b1473e7722..a6bf2126d7 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -154,7 +154,7 @@ def get_queryset(self): return self.queryset.filter(query) def get_parsers(self): - if self.request.method == "POST" and self.request.path.endswith("profile_picture"): + if self.request and self.request.method == "POST" and self.request.path.endswith("profile_picture"): return [MultiPartParser()] return super().get_parsers() From 1f408c6e4588986507803ac47f0e87ec5f00cb7d Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Tue, 9 Jul 2024 16:24:30 +0530 Subject: [PATCH 05/17] checks for write permissions --- care/users/api/viewsets/users.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index a6bf2126d7..09e56674eb 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -380,6 +380,18 @@ def check_availability(self, request, username): @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) def profile_picture(self, request, username): user = self.get_object() + allowed_user_types = [ + User.TYPE_VALUE_MAP["WardAdmin"], + User.TYPE_VALUE_MAP["LocalBodyAdmin"], + User.TYPE_VALUE_MAP["DistrictAdmin"], + User.TYPE_VALUE_MAP["StateAdmin"], + ] + has_write_permission = request.user.is_superuser or request.user.id == user.id or ( + request.user.user_type in allowed_user_types + and self.facility.has_object_write_permission(request) + ) + if not has_write_permission: + return Response(status=status.HTTP_403_FORBIDDEN) serializer = UserImageUploadSerializer(user, data=request.data) serializer.is_valid(raise_exception=True) serializer.save() From 0057a6a9bf2fcce928e06fc921279fe83a50e57b Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Fri, 16 Aug 2024 03:50:53 +0530 Subject: [PATCH 06/17] lint fix --- care/users/api/serializers/user.py | 3 ++- care/users/api/viewsets/users.py | 20 ++++++++++++++------ care/utils/csp/config.py | 1 + 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/care/users/api/serializers/user.py b/care/users/api/serializers/user.py index 2d643a56ca..ce91ca12fe 100644 --- a/care/users/api/serializers/user.py +++ b/care/users/api/serializers/user.py @@ -13,10 +13,10 @@ ) from care.users.api.serializers.skill import UserSkillSerializer from care.users.models import GENDER_CHOICES, User +from care.utils.csp.config import BucketType, get_client_config from care.utils.queryset.facility import get_home_facility_queryset from care.utils.serializer.external_id_field import ExternalIdSerializerField from config.serializers import ChoiceField -from care.utils.csp.config import BucketType, get_client_config class SignUpSerializer(serializers.ModelSerializer): @@ -439,6 +439,7 @@ class Meta: "read_profile_picture_url", ) + class UserImageUploadSerializer(serializers.ModelSerializer): profile_picture_url = serializers.ImageField(required=True, write_only=True) read_profile_picture_url = serializers.CharField(read_only=True) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index ad68d71485..0fa39e25ab 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -12,19 +12,19 @@ from rest_framework import mixins, status from rest_framework.decorators import action from rest_framework.generics import get_object_or_404 +from rest_framework.parsers import MultiPartParser from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.serializers import ValidationError from rest_framework.viewsets import GenericViewSet -from rest_framework.parsers import MultiPartParser from care.facility.api.serializers.facility import FacilityBasicInfoSerializer from care.facility.models.facility import Facility, FacilityUser from care.users.api.serializers.user import ( UserCreateSerializer, + UserImageUploadSerializer, UserListSerializer, UserSerializer, - UserImageUploadSerializer, ) from care.users.models import User from care.utils.cache.cache_allowed_facilities import get_accessible_facilities @@ -166,7 +166,11 @@ def get_queryset(self): return self.queryset.filter(query) def get_parsers(self): - if self.request and self.request.method == "POST" and self.request.path.endswith("profile_picture"): + if ( + self.request + and self.request.method == "POST" + and self.request.path.endswith("profile_picture") + ): return [MultiPartParser()] return super().get_parsers() @@ -403,9 +407,13 @@ def profile_picture(self, request, username): User.TYPE_VALUE_MAP["DistrictAdmin"], User.TYPE_VALUE_MAP["StateAdmin"], ] - has_write_permission = request.user.is_superuser or request.user.id == user.id or ( - request.user.user_type in allowed_user_types - and self.facility.has_object_write_permission(request) + has_write_permission = ( + request.user.is_superuser + or request.user.id == user.id + or ( + request.user.user_type in allowed_user_types + and self.facility.has_object_write_permission(request) + ) ) if not has_write_permission: return Response(status=status.HTTP_403_FORBIDDEN) diff --git a/care/utils/csp/config.py b/care/utils/csp/config.py index a55b58b3b9..edff720dc9 100644 --- a/care/utils/csp/config.py +++ b/care/utils/csp/config.py @@ -53,6 +53,7 @@ def get_patient_bucket_config(external) -> tuple[ClientConfig, BucketName]: ), }, settings.FILE_UPLOAD_BUCKET + def get_client_config(bucket_type: BucketType, external=False): if bucket_type == BucketType.FACILITY: return get_facility_bucket_config(external=external) From 39bee6c87b2716e059a5b54824887039a77441e0 Mon Sep 17 00:00:00 2001 From: Uday Sagar Date: Sun, 18 Aug 2024 19:16:54 +0530 Subject: [PATCH 07/17] adds permission checks --- care/users/api/viewsets/users.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index 0fa39e25ab..9cba57a686 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -397,25 +397,27 @@ def check_availability(self, request, username): return Response(status=status.HTTP_409_CONFLICT) return Response(status=status.HTTP_200_OK) - @extend_schema(tags=["users"]) - @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) - def profile_picture(self, request, username): - user = self.get_object() + def has_profile_image_write_permission(self, user, request): allowed_user_types = [ User.TYPE_VALUE_MAP["WardAdmin"], User.TYPE_VALUE_MAP["LocalBodyAdmin"], User.TYPE_VALUE_MAP["DistrictAdmin"], User.TYPE_VALUE_MAP["StateAdmin"], ] - has_write_permission = ( - request.user.is_superuser - or request.user.id == user.id + return ( + user.is_superuser + or (user.id == request.user.id) or ( - request.user.user_type in allowed_user_types + user.user_type in allowed_user_types and self.facility.has_object_write_permission(request) ) ) - if not has_write_permission: + + @extend_schema(tags=["users"]) + @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) + def profile_picture(self, request, username): + user = self.get_object() + if not self.has_profile_image_write_permission(user, request): return Response(status=status.HTTP_403_FORBIDDEN) serializer = UserImageUploadSerializer(user, data=request.data) serializer.is_valid(raise_exception=True) @@ -426,6 +428,8 @@ def profile_picture(self, request, username): @profile_picture.mapping.delete def profile_picture_delete(self, *args, **kwargs): user = self.get_object() + if not self.has_profile_image_write_permission(user, self.request): + return Response(status=status.HTTP_403_FORBIDDEN) user.profile_picture_url = None user.save() return Response(status=status.HTTP_204_NO_CONTENT) From 9e83dc00484c6167ec3e8ab70adf1c074b23ae33 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:21:44 +0530 Subject: [PATCH 08/17] refactor cover image upload logic --- care/facility/api/serializers/facility.py | 34 +++++------------- care/users/api/serializers/user.py | 37 ++++++++++--------- care/utils/file_uploads/__init__.py | 0 care/utils/file_uploads/cover_image.py | 43 +++++++++++++++++++++++ 4 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 care/utils/file_uploads/__init__.py create mode 100644 care/utils/file_uploads/cover_image.py diff --git a/care/facility/api/serializers/facility.py b/care/facility/api/serializers/facility.py index 1e50f33271..4d97a79c03 100644 --- a/care/facility/api/serializers/facility.py +++ b/care/facility/api/serializers/facility.py @@ -1,7 +1,3 @@ -import uuid - -import boto3 -from django.conf import settings from django.contrib.auth import get_user_model from rest_framework import serializers @@ -15,7 +11,7 @@ StateSerializer, WardSerializer, ) -from care.utils.csp.config import BucketType, get_client_config +from care.utils.file_uploads.cover_image import upload_cover_image from care.utils.models.validators import ( cover_image_validator, custom_image_extension_validator, @@ -192,25 +188,13 @@ class Meta: fields = ("cover_image", "read_cover_image_url") def save(self, **kwargs): - facility = self.instance + facility: Facility = self.instance image = self.validated_data["cover_image"] - - config, bucket_name = get_client_config(BucketType.FACILITY) - s3 = boto3.client("s3", **config) - - if facility.cover_image_url: - s3.delete_object(Bucket=bucket_name, Key=facility.cover_image_url) - - image_extension = image.name.rsplit(".", 1)[-1] - image_location = f"cover_images/{facility.external_id}_{str(uuid.uuid4())[0:8]}.{image_extension}" - boto_params = { - "Bucket": bucket_name, - "Key": image_location, - "Body": image.file, - } - if settings.BUCKET_HAS_FINE_ACL: - boto_params["ACL"] = "public-read" - s3.put_object(**boto_params) - facility.cover_image_url = image_location - facility.save() + facility.cover_image_url = upload_cover_image( + image, + str(facility.external_id), + "cover_images", + facility.cover_image_url, + ) + facility.save(update_fields=["cover_image_url"]) return facility diff --git a/care/users/api/serializers/user.py b/care/users/api/serializers/user.py index 508279a677..edb72c38b4 100644 --- a/care/users/api/serializers/user.py +++ b/care/users/api/serializers/user.py @@ -1,4 +1,3 @@ -import boto3 from django.contrib.auth.hashers import make_password from django.db import transaction from django.utils.timezone import now @@ -13,7 +12,11 @@ ) from care.users.api.serializers.skill import UserSkillSerializer from care.users.models import GENDER_CHOICES, User -from care.utils.csp.config import BucketType, get_client_config +from care.utils.file_uploads.cover_image import upload_cover_image +from care.utils.models.validators import ( + cover_image_validator, + custom_image_extension_validator, +) from care.utils.queryset.facility import get_home_facility_queryset from care.utils.serializer.external_id_field import ExternalIdSerializerField from config.serializers import ChoiceField @@ -447,25 +450,25 @@ class Meta: class UserImageUploadSerializer(serializers.ModelSerializer): - profile_picture_url = serializers.ImageField(required=True, write_only=True) - read_profile_picture_url = serializers.CharField(read_only=True) + profile_picture = serializers.ImageField( + required=True, + write_only=True, + validators=[custom_image_extension_validator, cover_image_validator], + ) + read_profile_picture_url = serializers.URLField(read_only=True) class Meta: model = User - fields = ("profile_picture_url", "read_profile_picture_url") + fields = ("profile_picture", "read_profile_picture_url") def save(self, **kwargs): - user = self.instance - image = self.validated_data["profile_picture_url"] - image_extension = image.name.split(".")[-1] - config, bucket_name = get_client_config(BucketType.FACILITY) - s3 = boto3.client("s3", **config) - image_location = f"avatars/{user.external_id}.{image_extension}" - s3.put_object( - Bucket=bucket_name, - Key=image_location, - Body=image.file, + user: User = self.instance + image = self.validated_data["profile_picture"] + user.profile_picture_url = upload_cover_image( + image, + str(user.external_id), + "avatars", + user.profile_picture_url, ) - user.profile_picture_url = image_location - user.save() + user.save(update_fields=["profile_picture_url"]) return user diff --git a/care/utils/file_uploads/__init__.py b/care/utils/file_uploads/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/care/utils/file_uploads/cover_image.py b/care/utils/file_uploads/cover_image.py new file mode 100644 index 0000000000..bb67a8aad8 --- /dev/null +++ b/care/utils/file_uploads/cover_image.py @@ -0,0 +1,43 @@ +import logging +import uuid +from typing import Literal + +import boto3 +from django.conf import settings +from django.core.files.uploadedfile import UploadedFile + +from care.utils.csp.config import BucketType, get_client_config + +logger = logging.getLogger(__name__) + + +def upload_cover_image( + image: UploadedFile, + object_external_id: str, + folder: Literal["cover_images", "avatars"], + old_key: str = None, +) -> str: + config, bucket_name = get_client_config(BucketType.FACILITY) + s3 = boto3.client("s3", **config) + + if old_key: + try: + s3.delete_object(Bucket=bucket_name, Key=f"{folder}/{old_key}") + except Exception: + logger.warning(f"Failed to delete old cover image {old_key}") + + image_extension = image.name.rsplit(".", 1)[-1] + image_key = ( + f"{folder}/{object_external_id}_{str(uuid.uuid4())[0:8]}.{image_extension}" + ) + + boto_params = { + "Bucket": bucket_name, + "Key": image_key, + "Body": image.file, + } + if settings.BUCKET_HAS_FINE_ACL: + boto_params["ACL"] = "public-read" + s3.put_object(**boto_params) + + return image_key From d6f4bafbdc5f9cfc1499a744dc7bd14eaa348b9d Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:22:00 +0530 Subject: [PATCH 09/17] update perms and cleanup --- care/users/api/viewsets/users.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index 769803c0ad..2ce4df28fc 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -178,7 +178,7 @@ def get_parsers(self): return [MultiPartParser()] return super().get_parsers() - def get_object(self): + def get_object(self) -> User: try: return super().get_object() except Http404: @@ -402,20 +402,7 @@ def check_availability(self, request, username): return Response(status=status.HTTP_200_OK) def has_profile_image_write_permission(self, user, request): - allowed_user_types = [ - User.TYPE_VALUE_MAP["WardAdmin"], - User.TYPE_VALUE_MAP["LocalBodyAdmin"], - User.TYPE_VALUE_MAP["DistrictAdmin"], - User.TYPE_VALUE_MAP["StateAdmin"], - ] - return ( - user.is_superuser - or (user.id == request.user.id) - or ( - user.user_type in allowed_user_types - and self.facility.has_object_write_permission(request) - ) - ) + return user.is_superuser or (user.id == request.user.id) @extend_schema(tags=["users"]) @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) @@ -423,7 +410,7 @@ def profile_picture(self, request, username): user = self.get_object() if not self.has_profile_image_write_permission(user, request): return Response(status=status.HTTP_403_FORBIDDEN) - serializer = UserImageUploadSerializer(user, data=request.data) + serializer = self.get_serializer(user, data=request.data) serializer.is_valid(raise_exception=True) serializer.save() return Response(status=status.HTTP_200_OK) From 8ef37b885612984e8a1b708fba8b273989191142 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:22:12 +0530 Subject: [PATCH 10/17] update migrations --- ...rofile_picture_url.py => 0018_user_profile_picture_url.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename care/users/migrations/{0017_user_profile_picture_url.py => 0018_user_profile_picture_url.py} (77%) diff --git a/care/users/migrations/0017_user_profile_picture_url.py b/care/users/migrations/0018_user_profile_picture_url.py similarity index 77% rename from care/users/migrations/0017_user_profile_picture_url.py rename to care/users/migrations/0018_user_profile_picture_url.py index 77771b05e0..fea3ac507c 100644 --- a/care/users/migrations/0017_user_profile_picture_url.py +++ b/care/users/migrations/0018_user_profile_picture_url.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-07 06:16 +# Generated by Django 5.1.1 on 2024-09-21 09:50 from django.db import migrations, models @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ("users", "0016_upgrade_user_skills"), + ("users", "0017_userflag"), ] operations = [ From 2191cdb4222b1ba594ab1402abf14670c9de588d Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:27:29 +0530 Subject: [PATCH 11/17] add tests --- care/users/api/viewsets/users.py | 12 +++--- care/users/tests/test_api.py | 68 ++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index 2ce4df28fc..b0864da440 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -401,14 +401,14 @@ def check_availability(self, request, username): return Response(status=status.HTTP_409_CONFLICT) return Response(status=status.HTTP_200_OK) - def has_profile_image_write_permission(self, user, request): - return user.is_superuser or (user.id == request.user.id) + def has_profile_image_write_permission(self, request, user): + return request.user.is_superuser or (user.id == request.user.id) @extend_schema(tags=["users"]) @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) - def profile_picture(self, request, username): + def profile_picture(self, request, *args, **kwargs): user = self.get_object() - if not self.has_profile_image_write_permission(user, request): + if not self.has_profile_image_write_permission(request, user): return Response(status=status.HTTP_403_FORBIDDEN) serializer = self.get_serializer(user, data=request.data) serializer.is_valid(raise_exception=True) @@ -417,9 +417,9 @@ def profile_picture(self, request, username): @extend_schema(tags=["users"]) @profile_picture.mapping.delete - def profile_picture_delete(self, *args, **kwargs): + def profile_picture_delete(self, request, *args, **kwargs): user = self.get_object() - if not self.has_profile_image_write_permission(user, self.request): + if not self.has_profile_image_write_permission(request, user): return Response(status=status.HTTP_403_FORBIDDEN) user.profile_picture_url = None user.save() diff --git a/care/users/tests/test_api.py b/care/users/tests/test_api.py index 8fa270a13a..afcfc02af1 100644 --- a/care/users/tests/test_api.py +++ b/care/users/tests/test_api.py @@ -1,6 +1,9 @@ +import io from datetime import date, timedelta +from django.core.files.uploadedfile import SimpleUploadedFile from django.utils import timezone +from PIL import Image from rest_framework import status from rest_framework.test import APITestCase @@ -293,3 +296,68 @@ def test_home_facility_filter(self): self.assertIn( self.user_5.username, {r["username"] for r in res_data_json["results"]} ) + + +class TestUserProfilePicture(TestUtils, APITestCase): + @classmethod + def setUpTestData(cls) -> None: + cls.state = cls.create_state() + cls.district = cls.create_district(cls.state) + cls.super_user = cls.create_super_user("su", cls.district) + cls.user = cls.create_user("staff1", cls.district) + + def get_base_url(self) -> str: + return f"/api/v1/users/{self.user.username}/profile_picture/" + + def get_payload(self) -> dict: + image = Image.new("RGB", (400, 400)) + file = io.BytesIO() + image.save(file, format="JPEG") + test_file = SimpleUploadedFile("test.jpg", file.getvalue(), "image/jpeg") + test_file.size = 2000 + return {"profile_picture": test_file} + + def test_user_can_upload_profile_picture(self): + image = Image.new("RGB", (400, 400)) + file = io.BytesIO() + image.save(file, format="JPEG") + test_file = SimpleUploadedFile("test.jpg", file.getvalue(), "image/jpeg") + test_file.size = 2000 + response = self.client.post( + self.get_base_url(), self.get_payload(), format="multipart" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsNotNone( + User.objects.get(username=self.user.username).profile_picture_url + ) + + def test_user_can_delete_profile_picture(self): + self.user.profile_picture_url = "image.jpg" + self.user.save(update_fields=["profile_picture_url"]) + + response = self.client.delete(self.get_base_url()) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertIsNone( + User.objects.get(username=self.user.username).profile_picture_url + ) + + def test_superuser_can_upload_profile_picture(self): + self.client.force_authenticate(self.super_user) + response = self.client.post( + self.get_base_url(), self.get_payload(), format="multipart" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsNotNone( + User.objects.get(username=self.user.username).profile_picture_url + ) + + def test_superuser_can_delete_profile_picture(self): + self.user.profile_picture_url = "image.jpg" + self.user.save(update_fields=["profile_picture_url"]) + + self.client.force_authenticate(self.super_user) + response = self.client.delete(self.get_base_url()) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertIsNone( + User.objects.get(username=self.user.username).profile_picture_url + ) From 69de28471cd615c3a54eb1ed2fa590b5ad1c35e6 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:35:50 +0530 Subject: [PATCH 12/17] cleanup parsers --- care/facility/api/viewsets/facility.py | 9 +++------ care/users/api/viewsets/users.py | 13 +++---------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index 93b7e4bd2e..c9ee338aa2 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -1,11 +1,12 @@ from django.conf import settings +from django.utils.decorators import method_decorator from django_filters import rest_framework as filters from djqscsv import render_to_csv_response from drf_spectacular.utils import extend_schema, extend_schema_view from dry_rest_permissions.generics import DRYPermissionFiltersBase, DRYPermissions from rest_framework import filters as drf_filters from rest_framework import mixins, status, viewsets -from rest_framework.decorators import action +from rest_framework.decorators import action, parser_classes from rest_framework.parsers import MultiPartParser from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -96,11 +97,6 @@ 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 @@ -151,6 +147,7 @@ def list(self, request, *args, **kwargs): return super(FacilityViewSet, self).list(request, *args, **kwargs) @extend_schema(tags=["facility"]) + @method_decorator(parser_classes([MultiPartParser])) @action(methods=["POST"], detail=True) def cover_image(self, request, external_id): facility = self.get_object() diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index b0864da440..74eaad85fc 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -4,13 +4,14 @@ from django.db.models import F, Q, Subquery from django.http import Http404 from django.utils import timezone +from django.utils.decorators import method_decorator from django_filters import rest_framework as filters from drf_spectacular.utils import extend_schema from dry_rest_permissions.generics import DRYPermissions from rest_framework import filters as drf_filters from rest_framework import filters as rest_framework_filters from rest_framework import mixins, status -from rest_framework.decorators import action +from rest_framework.decorators import action, parser_classes from rest_framework.generics import get_object_or_404 from rest_framework.parsers import MultiPartParser from rest_framework.permissions import IsAuthenticated @@ -169,15 +170,6 @@ def get_queryset(self): ) return self.queryset.filter(query) - def get_parsers(self): - if ( - self.request - and self.request.method == "POST" - and self.request.path.endswith("profile_picture") - ): - return [MultiPartParser()] - return super().get_parsers() - def get_object(self) -> User: try: return super().get_object() @@ -405,6 +397,7 @@ def has_profile_image_write_permission(self, request, user): return request.user.is_superuser or (user.id == request.user.id) @extend_schema(tags=["users"]) + @method_decorator(parser_classes([MultiPartParser])) @action(detail=True, methods=["POST"], permission_classes=[IsAuthenticated]) def profile_picture(self, request, *args, **kwargs): user = self.get_object() From ec87341b2f9f653734377d4f5e4ca0d7d0def3c8 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:38:44 +0530 Subject: [PATCH 13/17] lint fix --- care/users/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/care/users/models.py b/care/users/models.py index 27fff199f0..2a701a090e 100644 --- a/care/users/models.py +++ b/care/users/models.py @@ -350,7 +350,6 @@ class User(AbstractUser): CSV_MAKE_PRETTY = {"user_type": (lambda x: User.REVERSE_TYPE_MAP[x])} - def read_profile_picture_url(self): if self.profile_picture_url: return f"{settings.FACILITY_S3_BUCKET_EXTERNAL_ENDPOINT}/{settings.FACILITY_S3_BUCKET}/{self.profile_picture_url}" From db5926c5bcb4df59c153658ed5e8411ca7bdb1e5 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 16:46:02 +0530 Subject: [PATCH 14/17] optimize unique key generation --- care/utils/file_uploads/cover_image.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/care/utils/file_uploads/cover_image.py b/care/utils/file_uploads/cover_image.py index bb67a8aad8..39c8c600fa 100644 --- a/care/utils/file_uploads/cover_image.py +++ b/care/utils/file_uploads/cover_image.py @@ -1,5 +1,5 @@ import logging -import uuid +import secrets from typing import Literal import boto3 @@ -28,7 +28,7 @@ def upload_cover_image( image_extension = image.name.rsplit(".", 1)[-1] image_key = ( - f"{folder}/{object_external_id}_{str(uuid.uuid4())[0:8]}.{image_extension}" + f"{folder}/{object_external_id}_{secrets.token_hex(8)}.{image_extension}" ) boto_params = { From 237258ee38c69217590172ee5ab76cdff8f4e99b Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sat, 21 Sep 2024 22:30:33 +0530 Subject: [PATCH 15/17] delete images from s3 with delete operation --- care/facility/api/viewsets/facility.py | 2 ++ care/users/api/viewsets/users.py | 2 ++ care/utils/file_uploads/cover_image.py | 10 ++++++++++ 3 files changed, 14 insertions(+) diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index c9ee338aa2..174ab612fc 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -24,6 +24,7 @@ ) from care.facility.models.facility import FacilityUser from care.users.models import User +from care.utils.file_uploads.cover_image import delete_cover_image class FacilityFilter(filters.FilterSet): @@ -160,6 +161,7 @@ def cover_image(self, request, external_id): @cover_image.mapping.delete def cover_image_delete(self, *args, **kwargs): facility = self.get_object() + delete_cover_image(facility.cover_image_url, "cover_images") facility.cover_image_url = None facility.save() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/care/users/api/viewsets/users.py b/care/users/api/viewsets/users.py index 74eaad85fc..9628c4a685 100644 --- a/care/users/api/viewsets/users.py +++ b/care/users/api/viewsets/users.py @@ -29,6 +29,7 @@ ) from care.users.models import User from care.utils.cache.cache_allowed_facilities import get_accessible_facilities +from care.utils.file_uploads.cover_image import delete_cover_image def remove_facility_user_cache(user_id): @@ -414,6 +415,7 @@ def profile_picture_delete(self, request, *args, **kwargs): user = self.get_object() if not self.has_profile_image_write_permission(request, user): return Response(status=status.HTTP_403_FORBIDDEN) + delete_cover_image(user.profile_picture_url, "avatars") user.profile_picture_url = None user.save() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/care/utils/file_uploads/cover_image.py b/care/utils/file_uploads/cover_image.py index 39c8c600fa..5924610c33 100644 --- a/care/utils/file_uploads/cover_image.py +++ b/care/utils/file_uploads/cover_image.py @@ -11,6 +11,16 @@ logger = logging.getLogger(__name__) +def delete_cover_image(image_key: str, folder: Literal["cover_images", "avatars"]): + config, bucket_name = get_client_config(BucketType.FACILITY) + s3 = boto3.client("s3", **config) + + try: + s3.delete_object(Bucket=bucket_name, Key=f"{folder}/{image_key}") + except Exception: + logger.warning(f"Failed to delete cover image {image_key}") + + def upload_cover_image( image: UploadedFile, object_external_id: str, From 8a8b533425ce1d7065a6cce79021c675617cc04e Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sun, 22 Sep 2024 00:11:09 +0530 Subject: [PATCH 16/17] fix tests --- care/users/tests/test_api.py | 2 +- care/utils/tests/test_utils.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/care/users/tests/test_api.py b/care/users/tests/test_api.py index afcfc02af1..096699deba 100644 --- a/care/users/tests/test_api.py +++ b/care/users/tests/test_api.py @@ -49,7 +49,7 @@ def get_detail_representation(self, obj=None) -> dict: "doctor_qualification": obj.doctor_qualification, "weekly_working_hours": obj.weekly_working_hours, "video_connect_link": obj.video_connect_link, - "profile_picture_url": obj.profile_picture_url, + "read_profile_picture_url": obj.profile_picture_url, "user_flags": [], **self.get_local_body_district_state_representation(obj), } diff --git a/care/utils/tests/test_utils.py b/care/utils/tests/test_utils.py index 3b340ff5b0..efdfb748fa 100644 --- a/care/utils/tests/test_utils.py +++ b/care/utils/tests/test_utils.py @@ -107,6 +107,8 @@ class TestUtils: Base class for tests, handles most of the test setup and tools for setting up data """ + maxDiff = None + def setUp(self) -> None: self.client.force_login(self.user) From 777bffcf21a1d64e50de72ffdbcbb9db01a9e890 Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Sun, 22 Sep 2024 23:28:42 +0530 Subject: [PATCH 17/17] fix --- care/utils/file_uploads/cover_image.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/care/utils/file_uploads/cover_image.py b/care/utils/file_uploads/cover_image.py index 5924610c33..55f1183782 100644 --- a/care/utils/file_uploads/cover_image.py +++ b/care/utils/file_uploads/cover_image.py @@ -16,7 +16,7 @@ def delete_cover_image(image_key: str, folder: Literal["cover_images", "avatars" s3 = boto3.client("s3", **config) try: - s3.delete_object(Bucket=bucket_name, Key=f"{folder}/{image_key}") + s3.delete_object(Bucket=bucket_name, Key=image_key) except Exception: logger.warning(f"Failed to delete cover image {image_key}") @@ -32,7 +32,7 @@ def upload_cover_image( if old_key: try: - s3.delete_object(Bucket=bucket_name, Key=f"{folder}/{old_key}") + s3.delete_object(Bucket=bucket_name, Key=old_key) except Exception: logger.warning(f"Failed to delete old cover image {old_key}")