From c50aaa2d81f92a762aaf1136d27693c385faecd8 Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Mon, 22 Dec 2025 22:38:43 -0700 Subject: [PATCH] Fix Critical Security Issue: Authorization bypass on shared tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added comprehensive authorization checks to prevent users from: 1. Sharing tasks/tags they don't own 2. Modifying tasks/tags that are only shared with them (read-only access) Changes: tasks/serializers.py (TaskShareSerializer): - Added validation in validate() method to verify task/tag ownership - Users can only share their own tasks and tags - Prevents malicious users from sharing other people's resources tasks/permissions.py (NEW): - Created IsOwnerOrReadOnlyIfShared permission class - Owners: full access (read, update, delete) - Shared users: read-only access - Prevents privilege escalation via shared access tasks/views.py: - Applied IsOwnerOrReadOnlyIfShared to TaskDetailAPIView - Applied IsOwnerOrReadOnlyIfShared to TagDetailAPIView - Enforces object-level permissions on all update/delete operations Security impact: - Prevents users from modifying or deleting resources they don't own - Prevents users from sharing resources they don't own - Maintains proper separation between owner and shared access levels 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- tasks/permissions.py | 28 ++++++++++++++++++++++++++++ tasks/serializers.py | 10 ++++++++++ tasks/views.py | 5 +++-- 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 tasks/permissions.py diff --git a/tasks/permissions.py b/tasks/permissions.py new file mode 100644 index 0000000..ccbce29 --- /dev/null +++ b/tasks/permissions.py @@ -0,0 +1,28 @@ +""" +Custom permissions for task-related views. +""" + +from rest_framework import permissions + + +class IsOwnerOrReadOnlyIfShared(permissions.BasePermission): + """ + Permission that allows: + - Owners: full access (read, update, delete) + - Shared users: read-only access + + SECURITY: Prevents users with shared access from modifying tasks/tags they don't own. + """ + + def has_object_permission(self, request, view, obj): + # Read permissions are allowed for shared tasks/tags + if request.method in permissions.SAFE_METHODS: + # Allow if user owns the object OR if it's shared with them + return ( + obj.user == request.user or + hasattr(obj, 'shares') and obj.shares.filter(shared_with=request.user).exists() or + hasattr(obj, 'task') and obj.task.shares.filter(shared_with=request.user).exists() + ) + + # Write permissions (PUT, PATCH, DELETE) only for owners + return obj.user == request.user diff --git a/tasks/serializers.py b/tasks/serializers.py index d123a2c..8710bd8 100644 --- a/tasks/serializers.py +++ b/tasks/serializers.py @@ -197,6 +197,16 @@ class TaskShareSerializer(serializers.ModelSerializer): raise serializers.ValidationError("Must share either a task or a tag.") if attrs.get('task') and attrs.get('tag'): raise serializers.ValidationError("Cannot share both a task and a tag.") + + # SECURITY: Verify that the task/tag belongs to the requesting user + user = self.context['request'].user + if attrs.get('task'): + if attrs['task'].user != user: + raise serializers.ValidationError("You can only share your own tasks.") + if attrs.get('tag'): + if attrs['tag'].user != user: + raise serializers.ValidationError("You can only share your own tags.") + return attrs def create(self, validated_data): diff --git a/tasks/views.py b/tasks/views.py index c752220..4aa3cfd 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -22,6 +22,7 @@ from .serializers import ( TimeEntrySerializer, TaskShareSerializer, ) +from .permissions import IsOwnerOrReadOnlyIfShared logger = logging.getLogger(__name__) @@ -66,7 +67,7 @@ class TaskDetailAPIView(generics.RetrieveUpdateDestroyAPIView): """API endpoint for task details.""" serializer_class = TaskSerializer - permission_classes = [permissions.IsAuthenticated] + permission_classes = [permissions.IsAuthenticated, IsOwnerOrReadOnlyIfShared] def get_queryset(self): user = self.request.user @@ -94,7 +95,7 @@ class TagDetailAPIView(generics.RetrieveUpdateDestroyAPIView): """API endpoint for tag details.""" serializer_class = TagSerializer - permission_classes = [permissions.IsAuthenticated] + permission_classes = [permissions.IsAuthenticated, IsOwnerOrReadOnlyIfShared] def get_queryset(self): user = self.request.user