Dev

[ngovinh2k2]

What?

Why?

How?

Testing?

• Functional Testing
• Security
• Performance
• Error Handling
• Code Quality
• Documentation
• Database
• Deployment
• Final Review

Anything Else?

[gemini-code-assist]

Code Review

This pull request introduces several optimizations and features, including custom email template integration via a console service client, optimized space queries using Django subqueries, and the addition of Django Silk for profiling. It also implements cleanup mechanisms for deleted user avatars and space logos. The review feedback highlights a critical Broken Object Level Authorization (BOLA) vulnerability in the space users endpoint, a logic bug where default_display is incorrectly set to False during space creation, performance bottlenecks from synchronous S3 deletions in signal receivers, and transaction safety issues when dispatching Celery tasks before database commits.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Security Vulnerability: Broken Object Level Authorization (BOLA) / IDOR

The get method in GetSpaceUsersAPIView retrieves and returns all user IDs associated with a space using only the space_slug from the URL parameters:

space = get_object_or_404(Space, slug_name=space_slug)

There is no validation to ensure that the requesting user (identified by the X-User-ID header) actually belongs to or has permission to access the requested space. This allows any authenticated user to query the membership of any space in the system simply by guessing or obtaining its slug.

Recommendation

Verify the user's membership in the requested space before returning the user list. You can leverage the existing get_spaces_queryset_for_user helper function:

    def get(self, request, *args, **kwargs):
        space_slug = kwargs.get("space_slug")
        if not space_slug:
            return Response(
                {"error": "Space slug is required"},
                status=status.HTTP_400_BAD_REQUEST,
            )

        user_id = request.headers.get("X-User-ID", None)
        if not user_id:
            return Response(
                {"error": "User ID is required"},
                status=status.HTTP_401_UNAUTHORIZED,
            )

        # Ensure the user has access to the space
        spaces_qs = get_spaces_queryset_for_user(Space.objects.all(), user_id)
        space = get_object_or_404(spaces_qs, slug_name=space_slug)

        user_ids = (
            OrganizationUser.objects.filter(
                space_role_user__space_role__space_id=space.id
            )
            .values_list("id", flat=True)
            .distinct()
        )

        return Response(
            {
                "user_ids": list(user_ids),
                "total_users": len(user_ids),
            },
            status=status.HTTP_200_OK,
        )

[gemini-code-assist]

Correctness Bug: default_display Will Always Be False

When a new space is created, serializer.save() triggers the post_save signal handle_new_space synchronously. Inside that signal, a SpaceRoleUser membership is automatically created for the user.

Because of this, checking SpaceRoleUser.objects.filter(organization_user_id=organization_user.id).exists() immediately after serializer.save() will always return True, which sets instance.default_display to False even if this was the user's very first space (which should have been marked as default).

To fix this, query the database specifically for the membership of this user in this space and check if it is marked as default.

Suggested change

	instance.default_display = not SpaceRoleUser.objects.filter(
	organization_user_id=organization_user.id
	).exists()
	instance.default_display = SpaceRoleUser.objects.filter(
	organization_user_id=organization_user.id,
	space_role__space=instance,
	is_default=True,
	).exists()

[gemini-code-assist]

Performance Issue: Synchronous Network I/O in Signal Receiver

Calling delete_file synchronously inside a Django signal receiver blocks the request-response cycle. If S3 is slow or down, this will cause the user deletion request to hang or fail.

Since there is already a Celery task delete_upload_file configured for this purpose, you should offload this deletion to Celery using send_task.

Suggested change

	delete_file(
	settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	f"uploads/{instance.avatar}",
	)
	from common.celery import constants
	from common.celery.task_senders import send_task
	send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{instance.avatar}",
	},
	)

[gemini-code-assist]

Performance Issue: Synchronous Network I/O in Signal Receiver

Calling delete_file synchronously inside a Django signal receiver blocks the request-response cycle. If S3 is slow or down, this will cause the space deletion request to hang or fail.

Since there is already a Celery task delete_upload_file configured for this purpose, you should offload this deletion to Celery using send_task.

Suggested change

	delete_file(
	settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	f"uploads/{instance.logo}",
	)
	from common.celery import constants
	from common.celery.task_senders import send_task
	send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{instance.logo}",
	},
	)

[gemini-code-assist]

Transaction Safety: Sending Celery Tasks Inside Transactions

Sending a Celery task immediately during a database update can lead to race conditions or data inconsistency. If the database transaction is rolled back after the task is sent, the file will be deleted from S3, but the database will still reference the old avatar.

To prevent this, use transaction.on_commit to ensure the task is only sent after the transaction successfully commits.

Suggested change

	if old_avatar and old_avatar != new_avatar:
	send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{old_avatar}",
	},
	)
	if old_avatar and old_avatar != new_avatar:
	from django.db import transaction
	transaction.on_commit(
	lambda: send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{old_avatar}",
	},
	)
	)

[gemini-code-assist]

Transaction Safety: Sending Celery Tasks Inside Transactions

Sending a Celery task immediately during a database update can lead to race conditions or data inconsistency. If the database transaction is rolled back after the task is sent, the file will be deleted from S3, but the database will still reference the old logo.

To prevent this, use transaction.on_commit to ensure the task is only sent after the transaction successfully commits.

Suggested change

	if old_logo and old_logo != new_logo:
	send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{old_logo}",
	},
	)
	if old_logo and old_logo != new_logo:
	from django.db import transaction
	transaction.on_commit(
	lambda: send_task(
	name=constants.AUTH_SERVICE_DELETE_UPLOAD_FILE,
	message={
	"bucket_name": settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
	"link_file": f"uploads/{old_logo}",
	},
	)
	)