Internal
Public Access
Fix Critical Security Issue: Authorization bypass on shared tasks
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.5
parent
deabe6ab3d
commit
c50aaa2d81
@@ -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
|
||||||
@@ -197,6 +197,16 @@ class TaskShareSerializer(serializers.ModelSerializer):
|
|||||||
raise serializers.ValidationError("Must share either a task or a tag.")
|
raise serializers.ValidationError("Must share either a task or a tag.")
|
||||||
if attrs.get('task') and attrs.get('tag'):
|
if attrs.get('task') and attrs.get('tag'):
|
||||||
raise serializers.ValidationError("Cannot share both a task and a 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
|
return attrs
|
||||||
|
|
||||||
def create(self, validated_data):
|
def create(self, validated_data):
|
||||||
|
|||||||
+3
-2
@@ -22,6 +22,7 @@ from .serializers import (
|
|||||||
TimeEntrySerializer,
|
TimeEntrySerializer,
|
||||||
TaskShareSerializer,
|
TaskShareSerializer,
|
||||||
)
|
)
|
||||||
|
from .permissions import IsOwnerOrReadOnlyIfShared
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -66,7 +67,7 @@ class TaskDetailAPIView(generics.RetrieveUpdateDestroyAPIView):
|
|||||||
"""API endpoint for task details."""
|
"""API endpoint for task details."""
|
||||||
|
|
||||||
serializer_class = TaskSerializer
|
serializer_class = TaskSerializer
|
||||||
permission_classes = [permissions.IsAuthenticated]
|
permission_classes = [permissions.IsAuthenticated, IsOwnerOrReadOnlyIfShared]
|
||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
user = self.request.user
|
user = self.request.user
|
||||||
@@ -94,7 +95,7 @@ class TagDetailAPIView(generics.RetrieveUpdateDestroyAPIView):
|
|||||||
"""API endpoint for tag details."""
|
"""API endpoint for tag details."""
|
||||||
|
|
||||||
serializer_class = TagSerializer
|
serializer_class = TagSerializer
|
||||||
permission_classes = [permissions.IsAuthenticated]
|
permission_classes = [permissions.IsAuthenticated, IsOwnerOrReadOnlyIfShared]
|
||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
user = self.request.user
|
user = self.request.user
|
||||||
|
|||||||
Reference in New Issue
Block a user