Skip to content

refactor(auth): remove redundant authentication checks #270

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 5, 2025
Merged

refactor(auth): remove redundant authentication checks #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 10 additions & 28 deletions todo/views/task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
from rest_framework.response import Response
from rest_framework import status
from rest_framework.request import Request
from rest_framework.exceptions import AuthenticationFailed, ValidationError
from rest_framework.exceptions import ValidationError
from django.conf import settings
from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse
from drf_spectacular.types import OpenApiTypes
from todo.middlewares.jwt_auth import get_current_user_info
from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer
from todo.serializers.create_task_serializer import CreateTaskSerializer
from todo.serializers.update_task_serializer import UpdateTaskSerializer
Expand Down Expand Up @@ -78,22 +77,18 @@ def get(self, request: Request):
query = GetTaskQueryParamsSerializer(data=request.query_params)
query.is_valid(raise_exception=True)
if query.validated_data["profile"]:
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)
status_filter = query.validated_data.get("status", "").upper()
response = TaskService.get_tasks_for_user(
user_id=user["user_id"],
user_id=request.user_id,
page=query.validated_data["page"],
limit=query.validated_data["limit"],
status_filter=status_filter,
)
return Response(data=response.model_dump(mode="json"), status=status.HTTP_200_OK)

user = get_current_user_info(request)
if query.validated_data["profile"]:
response = TaskService.get_tasks_for_user(
user_id=user["user_id"],
user_id=request.user_id,
page=query.validated_data["page"],
limit=query.validated_data["limit"],
)
Expand All @@ -109,7 +104,7 @@ def get(self, request: Request):
limit=query.validated_data["limit"],
sort_by=query.validated_data["sort_by"],
order=query.validated_data.get("order"),
user_id=user["user_id"],
user_id=request.user_id,
team_id=team_id,
status_filter=status_filter,
)
Expand Down Expand Up @@ -137,15 +132,13 @@ def post(self, request: Request):
Returns:
Response: HTTP response with created task data or error details
"""
user = get_current_user_info(request)

serializer = CreateTaskSerializer(data=request.data)

if not serializer.is_valid():
return self._handle_validation_errors(serializer.errors)

try:
dto = CreateTaskDTO(**serializer.validated_data, createdBy=user["user_id"])
dto = CreateTaskDTO(**serializer.validated_data, createdBy=request.user_id)
response: CreateTaskResponse = TaskService.create_task(dto)

return Response(data=response.model_dump(mode="json"), status=status.HTTP_201_CREATED)
Expand Down Expand Up @@ -245,9 +238,8 @@ def get(self, request: Request, task_id: str):
},
)
def delete(self, request: Request, task_id: str):
user = get_current_user_info(request)
task_id = ObjectId(task_id)
TaskService.delete_task(task_id, user["user_id"])
TaskService.delete_task(task_id, request.user_id)
return Response(status=status.HTTP_204_NO_CONTENT)

@extend_schema(
Expand Down Expand Up @@ -283,16 +275,14 @@ def patch(self, request: Request, task_id: str):
Can also be used to defer a task by using ?action=defer query parameter.
"""
action = request.query_params.get("action", "update")
user = get_current_user_info(request)

if action == "defer":
serializer = DeferTaskSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

updated_task_dto = TaskService.defer_task(
task_id=task_id,
deferred_till=serializer.validated_data["deferredTill"],
user_id=user["user_id"],
user_id=request.user_id,
)
elif action == "update":
serializer = UpdateTaskSerializer(data=request.data, partial=True)
Expand All @@ -302,7 +292,7 @@ def patch(self, request: Request, task_id: str):
updated_task_dto = TaskService.update_task(
task_id=task_id,
validated_data=serializer.validated_data,
user_id=user["user_id"],
user_id=request.user_id,
)
else:
raise ValidationError({"action": ValidationErrors.UNSUPPORTED_ACTION.format(action)})
Expand Down Expand Up @@ -338,10 +328,6 @@ def patch(self, request: Request, task_id: str):
Update both task details and assignee information in a single request.
Similar to task creation but for updates.
"""
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)

serializer = UpdateTaskSerializer(data=request.data, partial=True)

if not serializer.is_valid():
Expand All @@ -350,7 +336,7 @@ def patch(self, request: Request, task_id: str):
try:
# Update the task using the service with validated data
updated_task_dto = TaskService.update_task_with_assignee_from_dict(
task_id=task_id, validated_data=serializer.validated_data, user_id=user["user_id"]
task_id=task_id, validated_data=serializer.validated_data, user_id=request.user_id
)

return Response(data=updated_task_dto.model_dump(mode="json"), status=status.HTTP_200_OK)
Expand Down Expand Up @@ -429,10 +415,6 @@ class AssignTaskToUserView(APIView):
},
)
def patch(self, request: Request, task_id: str):
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)

serializer = AssignTaskToUserSerializer(data=request.data)
if not serializer.is_valid():
return Response(data={"errors": serializer.errors}, status=status.HTTP_400_BAD_REQUEST)
Expand All @@ -441,7 +423,7 @@ def patch(self, request: Request, task_id: str):
dto = CreateTaskAssignmentDTO(
task_id=task_id, assignee_id=serializer.validated_data["assignee_id"], user_type="user"
)
response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, user["user_id"])
response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, request.user_id)
return Response(data=response.model_dump(mode="json"), status=status.HTTP_200_OK)
except Exception as e:
error_response = ApiErrorResponse(
Expand Down
26 changes: 6 additions & 20 deletions todo/views/task_assignment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@
from rest_framework.response import Response
from rest_framework import status
from rest_framework.request import Request
from rest_framework.exceptions import AuthenticationFailed
from django.conf import settings
from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse
from drf_spectacular.types import OpenApiTypes
from rest_framework import serializers

from todo.middlewares.jwt_auth import get_current_user_info
from todo.serializers.create_task_assignment_serializer import CreateTaskAssignmentSerializer
from todo.services.task_assignment_service import TaskAssignmentService
from todo.dto.task_assignment_dto import CreateTaskAssignmentDTO
Expand Down Expand Up @@ -53,17 +51,13 @@ def post(self, request: Request):
Returns:
Response: HTTP response with created assignment data or error details
"""
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)

serializer = CreateTaskAssignmentSerializer(data=request.data)
if not serializer.is_valid():
return Response(data={"errors": serializer.errors}, status=status.HTTP_400_BAD_REQUEST)

try:
dto = CreateTaskAssignmentDTO(**serializer.validated_data)
response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, user["user_id"])
response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, request.user_id)

return Response(data=response.model_dump(mode="json"), status=status.HTTP_201_CREATED)

Expand Down Expand Up @@ -177,10 +171,6 @@ def patch(self, request: Request, task_id: str):
Set or update the executor for a team-assigned task. Only the SPOC can perform this action.
For user assignments, this endpoint is not applicable.
"""
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)

executor_id = request.data.get("executor_id")
if not executor_id:
return Response({"error": "executor_id is required"}, status=status.HTTP_400_BAD_REQUEST)
Expand All @@ -204,7 +194,7 @@ def patch(self, request: Request, task_id: str):
)

# Only SPOC can update executor
if not TeamRepository.is_user_spoc(str(assignment.assignee_id), user["user_id"]):
if not TeamRepository.is_user_spoc(str(assignment.assignee_id), request.user_id):
return Response(
{"error": "Only the SPOC can update executor for this team task."}, status=status.HTTP_403_FORBIDDEN
)
Expand All @@ -224,14 +214,14 @@ def patch(self, request: Request, task_id: str):
# Update executor_id
try:
updated_assignment = TaskAssignmentRepository.update_assignment(
task_id, executor_id, "user", user["user_id"]
task_id, executor_id, "user", request.user_id
)
if not updated_assignment:
# Get more details about why it failed
import traceback

print(
f"DEBUG: update_executor failed for task_id={task_id}, executor_id={executor_id}, user_id={user['user_id']}"
f"DEBUG: update_executor failed for task_id={task_id}, executor_id={executor_id}, user_id={request.user_id}"
)
print(f"DEBUG: assignment details: {assignment}")
return Response(
Expand All @@ -257,7 +247,7 @@ def patch(self, request: Request, task_id: str):
team_id=assignment.assignee_id,
previous_executor_id=previous_executor_id,
new_executor_id=executor_id,
spoc_id=user["user_id"],
spoc_id=request.user_id,
action="reassign_executor",
)
AuditLogRepository.create(audit_log)
Expand Down Expand Up @@ -295,12 +285,8 @@ def delete(self, request: Request, task_id: str):
Returns:
Response: HTTP response with success or error details
"""
user = get_current_user_info(request)
if not user:
raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED)

try:
success = TaskAssignmentService.delete_task_assignment(task_id, user["user_id"])
success = TaskAssignmentService.delete_task_assignment(task_id, request.user_id)
if not success:
error_response = ApiErrorResponse(
statusCode=404,
Expand Down