Skip to content
Open
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
6 changes: 5 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ POSTGRES_DB=todo_postgres
POSTGRES_HOST=postgres
POSTGRES_PASSWORD=todo_password
POSTGRES_PORT=5432
POSTGRES_USER=todo_user
POSTGRES_USER=todo_user

CLOUDINARY_CLOUD_NAME="your_cloud_name"
CLOUDINARY_API_KEY="123456789012345"
CLOUDINARY_API_SECRET="your_api_secret"
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ testcontainers[mongodb]==4.10.0
drf-spectacular==0.28.0
debugpy==1.8.14
psycopg2-binary==2.9.9
cloudinary==1.44.1
32 changes: 32 additions & 0 deletions todo/repositories/user_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,38 @@ def create_or_update(cls, user_data: dict) -> UserModel:
raise
raise APIException(RepositoryErrors.USER_CREATE_UPDATE_FAILED.format(str(e)))

@classmethod
def update_picture_by_id(cls, user_id: str, picture_url: str) -> UserModel:
collection = cls._get_collection()
now = datetime.now(timezone.utc)
object_id = PyObjectId(user_id)
Comment thread
AnujChhikara marked this conversation as resolved.

result = collection.find_one_and_update(
{"_id": object_id},
{"$set": {"picture": picture_url, "updated_at": now}},
return_document=ReturnDocument.AFTER,
)

if not result:
raise UserNotFoundException()

user_model = UserModel(**result)

dual_write_service = EnhancedDualWriteService()
dual_write_success = dual_write_service.update_document(
collection_name="users",
mongo_id=user_id,
data={"picture": picture_url, "updated_at": now},
)

if not dual_write_success:
import logging

logger = logging.getLogger(__name__)
logger.warning(f"Failed to sync picture update for user {user_id} to Postgres")

return user_model

@classmethod
def search_users(cls, query: str, page: int = 1, limit: int = 10) -> tuple[List[UserModel], int]:
"""
Expand Down
18 changes: 18 additions & 0 deletions todo/serializers/update_profile_serializer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from rest_framework import serializers


ALLOWED_IMAGE_TYPES = ["image/jpeg", "image/png", "image/webp"]
MAX_FILE_SIZE = 5 * 1024 * 1024


class UpdateProfileSerializer(serializers.Serializer):
picture = serializers.FileField(required=True)

def validate_picture(self, value):
if value.content_type not in ALLOWED_IMAGE_TYPES:
raise serializers.ValidationError(f"Invalid file type. Allowed: {', '.join(ALLOWED_IMAGE_TYPES)}")

if value.size > MAX_FILE_SIZE:
raise serializers.ValidationError("File size must be under 5MB")

return value
60 changes: 60 additions & 0 deletions todo/services/cloudinary_service.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import io
import os

from cloudinary import uploader
import cloudinary

from todo.exceptions.auth_exceptions import APIException


class CloudinaryService:
@staticmethod
def _require_config() -> tuple[str, str, str]:
cloud_name = os.getenv("CLOUDINARY_CLOUD_NAME")
api_key = os.getenv("CLOUDINARY_API_KEY")
api_secret = os.getenv("CLOUDINARY_API_SECRET")

if not cloud_name or not api_key or not api_secret:
raise APIException(
"Cloudinary is not configured. Set CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET."
)

return str(cloud_name), str(api_key), str(api_secret)
Comment on lines +4 to +22
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

cloudinary python SDK configure credentials cloudinary.config vs CLOUDINARY_URL

💡 Result:

In the Cloudinary Python SDK, credentials can be configured in two primary ways: using the CLOUDINARY_URL environment variable or explicitly via cloudinary.config. The CLOUDINARY_URL environment variable is the recommended and most convenient method, especially for security. It follows the format cloudinary://api_key:api_secret@cloud_name and can include additional query parameters for other config options (e.g., ?secure=true). The SDK automatically detects and parses this variable upon import/initialization if present, populating the config with cloud_name, api_key, api_secret, and other params. No explicit config call is needed for basic setup. Use tools like python-dotenv to load it from a .env file. Example: # .env CLOUDINARY_URL=cloudinary://your_api_key:your_api_secret@your_cloud_name # In code from dotenv import load_dotenv load_dotenv import cloudinary # Credentials are auto-loaded; verify with: print(cloudinary.config.cloud_name) Alternatively, use cloudinary.config to set credentials programmatically, either directly or from other env vars. This overrides or supplements the auto-loaded config. Example: import cloudinary import os cloudinary.config( cloud_name="your_cloud_name", api_key=os.getenv("CLOUDINARY_API_KEY"), api_secret=os.getenv("CLOUDINARY_API_SECRET"), secure=True ) Both methods support all config params (e.g., secure, private_cdn). CLOUDINARY_URL is preferred for production as it centralizes secrets in env vars. Call config before importing uploader/api modules for full functionality. The global config is a singleton updated by both approaches.

Citations:


🏁 Script executed:

cat -n todo/services/cloudinary_service.py

Repository: RealDevSquad/todo-backend

Length of output: 1764


🏁 Script executed:

rg "cloudinary\.config" --type py

Repository: RealDevSquad/todo-backend

Length of output: 51


🏁 Script executed:

rg "CLOUDINARY_URL" --type py

Repository: RealDevSquad/todo-backend

Length of output: 51


Critical: Cloudinary is not configured — uploader.upload will fail in production

_require_config() reads the environment variables and returns them, but the return value is discarded and cloudinary.config() is never called. The Cloudinary SDK requires either a CLOUDINARY_URL environment variable (which is auto-parsed) or an explicit cloudinary.config(cloud_name=..., api_key=..., api_secret=...) call; with three separate CLOUDINARY_* variables and no config call, uploader.upload will raise an authentication error at runtime. The cloudinary import is also dead code until configuration is applied.

At line 32, _require_config() is called but its return value is ignored. The credentials are read but never used.

Proposed fix: Call cloudinary.config() with the environment variables before uploader.upload() is called:

Example fix
     `@staticmethod`
-    def _require_config() -> tuple[str, str, str]:
+    def _require_config() -> None:
         cloud_name = os.getenv("CLOUDINARY_CLOUD_NAME")
         api_key = os.getenv("CLOUDINARY_API_KEY")
         api_secret = os.getenv("CLOUDINARY_API_SECRET")

         if not cloud_name or not api_key or not api_secret:
             raise APIException(
                 "Cloudinary is not configured. Set CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET."
             )
-
-        return str(cloud_name), str(api_key), str(api_secret)
+        cloudinary.config(
+            cloud_name=cloud_name,
+            api_key=api_key,
+            api_secret=api_secret,
+            secure=True,
+        )

Alternatively, move the configuration call to application startup to avoid reconfiguring on every upload.

🧰 Tools
🪛 GitHub Actions: Tests

[error] 5-5: ruff check reported unused import: cloudinary imported but unused (F401). Remove unused import: cloudinary.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@todo/services/cloudinary_service.py` around lines 4 - 22, The
_require_config() in CloudinaryService reads CLOUDINARY_CLOUD_NAME,
CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET but never applies them, so
uploader.upload will fail; fix by calling cloudinary.config(cloud_name=...,
api_key=..., api_secret=...) with the values returned from _require_config()
before any upload (or move that config call to application startup). Update
CloudinaryService (where _require_config() is invoked) to use its return tuple
to call cloudinary.config(...) prior to calling uploader.upload(), ensuring
credentials are actually applied; keep the cloudinary import and remove any
unused env reads if you centralize config at app startup.

✅ Confirmed as addressed by @AnujChhikara

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

check now

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat -n todo/services/cloudinary_service.py

Length of output: 2016


@AnujChhikara, the fix looks good! The core issue is resolved:

  • The new _configure() method (Lines 24–31) correctly unpacks the tuple returned by _require_config() and passes the credentials to cloudinary.config(...).
  • upload_image now calls cls._configure() (Line 41) before invoking uploader.upload(), ensuring credentials are applied at runtime.
  • The cloudinary import is no longer dead code.

One optional improvement: consider adding secure=True to the cloudinary.config() call to ensure all asset URLs use HTTPS by default:

cloudinary.config(
    cloud_name=cloud_name,
    api_key=api_key,
    api_secret=api_secret,
    secure=True,  # ensures secure_url is always HTTPS
)

But that's a non-critical enhancement — the authentication issue is fully addressed. ✅


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@AnujChhikara can you try adding secure=True as well in config?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


@staticmethod
def _configure() -> None:
cloud_name, api_key, api_secret = CloudinaryService._require_config()
cloudinary.config(
cloud_name=cloud_name,
api_key=api_key,
api_secret=api_secret,
secure=True,
)

@classmethod
def upload_image(
cls,
*,
file_data: bytes,
user_id: str,
image_name: str,
) -> str:
cls._configure()

if not image_name.strip():
raise APIException("imageName must be a non-empty string")

upload_folder = f"todo/users/{user_id}"
public_id = image_name.strip()

file_obj = io.BytesIO(file_data)

result = uploader.upload(
file_obj,
public_id=public_id,
folder=upload_folder,
overwrite=True,
resource_type="image",
)

return result["secure_url"]
42 changes: 42 additions & 0 deletions todo/tests/integration/test_user_profile_api.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
from http import HTTPStatus
from django.core.files.uploadedfile import SimpleUploadedFile
from django.urls import reverse
from todo.tests.integration.base_mongo_test import AuthenticatedMongoTestCase
from unittest.mock import patch


class UserProfileAPIIntegrationTest(AuthenticatedMongoTestCase):
def setUp(self):
super().setUp()
self.url = reverse("users")
self.profile_url = reverse("user_profile")

def test_user_profile_true_requires_auth(self):
client = self.client.__class__()
Expand All @@ -19,3 +22,42 @@ def test_user_profile_true_returns_user_info(self):
data = response.json()["data"]
self.assertEqual(data["id"], str(self.user_id))
self.assertEqual(data["email"], self.user_data["email"])

def test_update_profile_picture_requires_auth(self):
client = self.client.__class__()
response = client.patch(
self.profile_url,
data={"picture": SimpleUploadedFile("test.jpg", b"fake", "image/jpeg")},
format="multipart",
)
self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED)

def test_update_profile_picture_persists_picture_url(self):
new_picture = "https://res.cloudinary.com/test_cloud/image/upload/v1/todo/users/abc/profile/picture.png"

with patch("todo.services.cloudinary_service.CloudinaryService.upload_image", return_value=new_picture):
response = self.client.patch(
self.profile_url,
data={"picture": SimpleUploadedFile("test.jpg", b"fake_image_data", "image/jpeg")},
format="multipart",
)

self.assertEqual(response.status_code, HTTPStatus.OK)

profile_response = self.client.get(self.url + "?profile=true")
self.assertEqual(profile_response.status_code, HTTPStatus.OK)
profile_data = profile_response.json()["data"]
self.assertEqual(profile_data["picture"], new_picture)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def test_update_profile_picture_invalid_file_type(self):
response = self.client.patch(
self.profile_url,
data={"picture": SimpleUploadedFile("test.txt", b"fake", "text/plain")},
format="multipart",
)
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
self.assertIn("Invalid file type", response.json().get("message", ""))

def test_update_profile_picture_missing_file(self):
response = self.client.patch(self.profile_url, data={}, format="multipart")
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
3 changes: 2 additions & 1 deletion todo/urls.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from django.urls import path
from todo.views.task import TaskListView, TaskDetailView, TaskUpdateView
from todo.views.health import HealthView
from todo.views.user import UsersView
from todo.views.user import UsersView, UserDetailView
from todo.views.auth import GoogleLoginView, GoogleCallbackView, LogoutView
from todo.views.role import RoleListView, RoleDetailView
from todo.views.user_role import UserRoleListView, TeamUserRoleListView, TeamUserRoleDetailView, TeamUserRoleDeleteView
Expand Down Expand Up @@ -57,6 +57,7 @@
path("auth/google/callback", GoogleCallbackView.as_view(), name="google_callback"),
path("auth/logout", LogoutView.as_view(), name="google_logout"),
path("users", UsersView.as_view(), name="users"),
path("users/profile", UserDetailView.as_view(), name="user_profile"),
path("users/<str:user_id>/roles", UserRoleListView.as_view(), name="user_roles"),
path("team-invite-codes/generate", GenerateTeamCreationInviteCodeView.as_view(), name="generate_team_invite_code"),
path("team-invite-codes/verify", VerifyTeamCreationInviteCodeView.as_view(), name="verify_team_invite_code"),
Expand Down
69 changes: 69 additions & 0 deletions todo/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
from rest_framework.request import Request
from todo.constants.messages import ApiErrors
from todo.services.user_service import UserService
from todo.repositories.user_repository import UserRepository
from todo.services.cloudinary_service import CloudinaryService
from todo.exceptions.auth_exceptions import APIException
from rest_framework import status
from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse
from drf_spectacular.types import OpenApiTypes
from todo.dto.user_dto import UserSearchResponseDTO, UsersDTO
from todo.dto.responses.error_response import ApiErrorResponse
from drf_spectacular.utils import OpenApiExample, inline_serializer
from rest_framework import serializers
from todo.serializers.update_profile_serializer import UpdateProfileSerializer
import logging


class UsersView(APIView):
Expand Down Expand Up @@ -117,3 +124,65 @@ def get(self, request: Request):
},
status=status.HTTP_200_OK,
)


class UserDetailView(APIView):
@extend_schema(
operation_id="update_user_profile",
summary="Update current user profile picture",
description="Updates the profile picture of the currently authenticated user.",
tags=["users"],
request=inline_serializer(
name="UserProfilePictureRequest",
fields={
"picture": serializers.FileField(),
},
),
responses={
200: OpenApiResponse(description="Profile picture updated"),
400: OpenApiResponse(description="Bad request"),
401: OpenApiResponse(description="Unauthorized"),
500: OpenApiResponse(description="Internal server error"),
},
examples=[
OpenApiExample(
name="Update profile image",
value={"picture": "<binary file>"},
request_only=True,
)
],
)
def patch(self, request: Request):
serializer = UpdateProfileSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

picture_file = serializer.validated_data["picture"]

try:
profile_url = CloudinaryService.upload_image(
file_data=picture_file.read(),
user_id=request.user_id,
image_name="profile-picture",
)
except APIException:
return Response(
{"message": "Image upload configuration error"},
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
except Exception as e:
logger = logging.getLogger(__name__)
logger.error(f"Failed to upload image for user {request.user_id}: {str(e)}")
return Response(
{"message": "Failed to upload image"},
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

user = UserRepository.update_picture_by_id(request.user_id, profile_url)
userData = user.model_dump(mode="json", exclude_none=True)
userResponse = {
"id": userData["id"],
"email": userData["email_id"],
"name": userData.get("name"),
"picture": userData.get("picture"),
}
return Response({"message": "User updated successfully", "data": userResponse}, status=200)
Loading