Codebase hardening: 102 fixes across 35+ files
Deep audit identified 106 findings; 102 fixed, 4 deferred. Covers 8 areas: - Settings & deploy: env-gated DEBUG/SECRET_KEY, HTTPS headers, gunicorn, celery worker - Auth (registered_user): password write_only, request.data fixes, transaction safety, proper HTTP status codes - Workout app: IDOR protection, get_object_or_404, prefetch_related N+1 fixes, transaction.atomic - Video/scripts: path traversal sanitization, HLS trigger guard, auth on cache wipe - Models (exercise/equipment/muscle/superset): null-safe __str__, stable IDs, prefetch support - Generator views: helper for registered_user lookup, logger.exception, bulk_update, transaction wrapping - Generator core (rules/selector/generator): push-pull ratio, type affinity normalization, modality checks, side-pair exact match, word-boundary regex, equipment cache clearing - Generator services (plan_builder/analyzer/normalizer): transaction.atomic, muscle cache, bulk_update, glutes classification fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Trey t
190
workout/views.py
190
workout/views.py
@@ -3,17 +3,18 @@ from .serializers import *
|
||||
|
||||
from django.shortcuts import render
|
||||
from rest_framework.decorators import api_view
|
||||
from rest_framework.decorators import api_view
|
||||
from rest_framework.response import Response
|
||||
from rest_framework import status
|
||||
from django.contrib.auth.models import User
|
||||
from django.contrib.auth import authenticate
|
||||
from rest_framework.authentication import TokenAuthentication
|
||||
from rest_framework.permissions import IsAuthenticated
|
||||
from rest_framework.permissions import IsAuthenticated, IsAdminUser
|
||||
from rest_framework.decorators import authentication_classes
|
||||
from rest_framework.decorators import permission_classes
|
||||
from django.shortcuts import get_object_or_404
|
||||
from datetime import datetime, timedelta
|
||||
from django.utils import timezone
|
||||
from datetime import timedelta
|
||||
from django.db import transaction
|
||||
|
||||
from django.core.cache import cache
|
||||
from .tasks import add_from_files_tasks
|
||||
@@ -22,14 +23,25 @@ from .tasks import add_from_files_tasks
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def all_workouts(request):
|
||||
if 'all_workouts' in cache:
|
||||
data = cache.get('all_workouts')
|
||||
# Fix #13: IDOR - filter workouts by the authenticated user
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
|
||||
cache_name = 'all_workouts_user_' + str(registered_user.pk)
|
||||
if cache_name in cache:
|
||||
data = cache.get(cache_name)
|
||||
return Response(data=data, status=status.HTTP_200_OK)
|
||||
|
||||
users = Workout.objects.all()
|
||||
serializer = WorkoutSerializer(users, many=True)
|
||||
|
||||
# Fix #16: N+1 - add prefetch_related for exercises, muscles, and equipment
|
||||
workouts = Workout.objects.filter(
|
||||
registered_user=registered_user
|
||||
).prefetch_related(
|
||||
'superset_set__supersetexercise_set__exercise',
|
||||
'superset_set__supersetexercise_set__exercise__muscles',
|
||||
'superset_set__supersetexercise_set__exercise__equipment_required_list',
|
||||
)
|
||||
serializer = WorkoutSerializer(workouts, many=True)
|
||||
data = serializer.data
|
||||
cache.set('all_workouts', data, timeout=None)
|
||||
cache.set(cache_name, data, timeout=None)
|
||||
return Response(data=data, status=status.HTTP_200_OK)
|
||||
|
||||
|
||||
@@ -37,12 +49,27 @@ def all_workouts(request):
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def workout_details(request, workout_id):
|
||||
cache_name = "wk"+str(workout_id)
|
||||
# Fix #14: IDOR - verify the workout belongs to the requesting user
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
|
||||
# Include user in cache key to prevent IDOR via cached data
|
||||
cache_name = "wk" + str(workout_id) + "_user_" + str(registered_user.pk)
|
||||
if cache_name in cache:
|
||||
data = cache.get(cache_name)
|
||||
return Response(data=data, status=status.HTTP_200_OK)
|
||||
|
||||
workout = Workout.objects.get(pk=workout_id)
|
||||
|
||||
# Fix #1: get_object_or_404 instead of Workout.objects.get
|
||||
# Fix #14: also filter by registered_user for ownership check
|
||||
# Fix #16: N+1 - add prefetch_related for exercises, muscles, and equipment
|
||||
workout = get_object_or_404(
|
||||
Workout.objects.prefetch_related(
|
||||
'superset_set__supersetexercise_set__exercise',
|
||||
'superset_set__supersetexercise_set__exercise__muscles',
|
||||
'superset_set__supersetexercise_set__exercise__equipment_required_list',
|
||||
),
|
||||
pk=workout_id,
|
||||
registered_user=registered_user
|
||||
)
|
||||
serializer = WorkoutDetailSerializer(workout, many=False)
|
||||
data = serializer.data
|
||||
cache.set(cache_name, data, timeout=300)
|
||||
@@ -52,29 +79,32 @@ def workout_details(request, workout_id):
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def complete_workout(request):
|
||||
registered_user = RegisteredUser.objects.get(user=request.user)
|
||||
# Fix #1: get_object_or_404
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
serializer = CompleteWorkoutSerializer(data=request.data, context = {"registered_user":registered_user.pk})
|
||||
if serializer.is_valid():
|
||||
serializer.save()
|
||||
return Response(serializer.data, status=status.HTTP_201_CREATED)
|
||||
return Response(serializer.errors, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
# Fix #2: validation errors return 400 not 500
|
||||
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
|
||||
|
||||
@api_view(['GET'])
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def workouts_completed_by_logged_in_user(request):
|
||||
registered_user = RegisteredUser.objects.get(user=request.user)
|
||||
# Fix #1: get_object_or_404
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
workouts = CompletedWorkout.objects.filter(registered_user=registered_user)
|
||||
serializer = GetCompleteWorkoutSerializer(workouts, many=True)
|
||||
return Response(serializer.data, status=status.HTTP_201_CREATED)
|
||||
# Fix #3: GET returns 200 not 201
|
||||
return Response(serializer.data, status=status.HTTP_200_OK)
|
||||
|
||||
@api_view(['POST'])
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def add_workout(request):
|
||||
registered_user = RegisteredUser.objects.get(user=request.user)
|
||||
if registered_user is None:
|
||||
return Response(status=status.HTTP_400_BAD_REQUEST)
|
||||
# Fix #1: get_object_or_404
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
|
||||
# exercise_data = dict(request.POST)["exercise_data"]
|
||||
exercise_data = request.data["supersets"]
|
||||
@@ -87,67 +117,75 @@ def add_workout(request):
|
||||
serializer = POSTCompleteWorkoutSerializer(data=request.data)
|
||||
|
||||
if serializer.is_valid():
|
||||
workout = serializer.save(registered_user=registered_user)
|
||||
workout.save()
|
||||
# Fix #10: wrap creation logic in transaction.atomic()
|
||||
with transaction.atomic():
|
||||
workout = serializer.save(registered_user=registered_user)
|
||||
# Fix #18: removed redundant save() right after create()
|
||||
|
||||
workout_total_time = 0
|
||||
for superset in exercise_data:
|
||||
name = superset["name"]
|
||||
rounds = superset["rounds"]
|
||||
exercises = superset["exercises"]
|
||||
superset_order = superset["order"]
|
||||
|
||||
superset = Superset.objects.create(
|
||||
workout=workout,
|
||||
name=name,
|
||||
rounds=rounds,
|
||||
order=superset_order
|
||||
)
|
||||
superset.save()
|
||||
|
||||
superset_total_time = 0
|
||||
for exercise in exercises:
|
||||
exercise_id = exercise["id"]
|
||||
exercise_obj = Exercise.objects.get(pk=exercise_id)
|
||||
order = exercise["order"]
|
||||
workout_total_time = 0
|
||||
for superset in exercise_data:
|
||||
name = superset["name"]
|
||||
rounds = superset["rounds"]
|
||||
exercises = superset["exercises"]
|
||||
superset_order = superset["order"]
|
||||
|
||||
supersetExercise = SupersetExercise.objects.create(
|
||||
superset=superset,
|
||||
exercise=exercise_obj,
|
||||
order=order
|
||||
superset = Superset.objects.create(
|
||||
workout=workout,
|
||||
name=name,
|
||||
rounds=rounds,
|
||||
order=superset_order
|
||||
)
|
||||
|
||||
if "weight" in exercise:
|
||||
supersetExercise.weight = exercise["weight"]
|
||||
if "reps" in exercise:
|
||||
supersetExercise.reps = exercise["reps"]
|
||||
superset_total_time += exercise["reps"] * exercise_obj.estimated_rep_duration
|
||||
if "duration" in exercise:
|
||||
supersetExercise.duration = exercise["duration"]
|
||||
superset_total_time += exercise["duration"]
|
||||
# Fix #18: removed redundant save() right after create()
|
||||
|
||||
supersetExercise.save()
|
||||
superset_total_time = 0
|
||||
for exercise in exercises:
|
||||
exercise_id = exercise["id"]
|
||||
# Fix #1: get_object_or_404
|
||||
exercise_obj = get_object_or_404(Exercise, pk=exercise_id)
|
||||
order = exercise["order"]
|
||||
|
||||
superset.estimated_time = superset_total_time
|
||||
superset.save()
|
||||
supersetExercise = SupersetExercise.objects.create(
|
||||
superset=superset,
|
||||
exercise=exercise_obj,
|
||||
order=order
|
||||
)
|
||||
|
||||
workout_total_time += (superset_total_time * rounds)
|
||||
if "weight" in exercise:
|
||||
supersetExercise.weight = exercise["weight"]
|
||||
if "reps" in exercise:
|
||||
supersetExercise.reps = exercise["reps"]
|
||||
# Fix #4: None multiplication risk
|
||||
superset_total_time += exercise["reps"] * (exercise_obj.estimated_rep_duration or 3.0)
|
||||
if "duration" in exercise:
|
||||
supersetExercise.duration = exercise["duration"]
|
||||
superset_total_time += exercise["duration"]
|
||||
|
||||
superset_order += 1
|
||||
workout.estimated_time = workout_total_time
|
||||
workout.save()
|
||||
|
||||
cache.delete('all_workouts')
|
||||
supersetExercise.save()
|
||||
|
||||
superset.estimated_time = superset_total_time
|
||||
superset.save()
|
||||
|
||||
workout_total_time += (superset_total_time * rounds)
|
||||
|
||||
superset_order += 1
|
||||
workout.estimated_time = workout_total_time
|
||||
workout.save()
|
||||
|
||||
# Fix #19: invalidate per-user cache key (matches all_workouts view)
|
||||
cache.delete('all_workouts_user_' + str(registered_user.pk))
|
||||
|
||||
return Response(serializer.data, status=status.HTTP_201_CREATED)
|
||||
return Response(serializer.errors, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
# Fix #2: validation errors return 400 not 500
|
||||
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
|
||||
|
||||
@api_view(['GET'])
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def workouts_planned_by_logged_in_user(request):
|
||||
registered_user = RegisteredUser.objects.get(user=request.user)
|
||||
workouts = PlannedWorkout.objects.filter(registered_user=registered_user, on_date__gte=datetime.now()- timedelta(days=1))
|
||||
# Fix #1: get_object_or_404
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
# Fix #12: timezone.now() instead of datetime.now()
|
||||
workouts = PlannedWorkout.objects.filter(registered_user=registered_user, on_date__gte=timezone.now()- timedelta(days=1))
|
||||
serializer = PlannedWorkoutSerializer(workouts, many=True)
|
||||
return Response(serializer.data, status=status.HTTP_200_OK)
|
||||
|
||||
@@ -155,19 +193,23 @@ def workouts_planned_by_logged_in_user(request):
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
def plan_workout(request):
|
||||
registered_user = RegisteredUser.objects.get(user=request.user)
|
||||
serializer = POSTPlannedWorkoutSerializer(data=request.data,
|
||||
# Fix #1: get_object_or_404
|
||||
registered_user = get_object_or_404(RegisteredUser, user=request.user)
|
||||
serializer = POSTPlannedWorkoutSerializer(data=request.data,
|
||||
context = {"registered_user":registered_user.pk})
|
||||
|
||||
|
||||
if serializer.is_valid():
|
||||
serializer.save()
|
||||
return Response(serializer.data, status=status.HTTP_201_CREATED)
|
||||
return Response(serializer.errors, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
# Fix #2: validation errors return 400 not 500
|
||||
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
|
||||
|
||||
@api_view(['GET'])
|
||||
# Fix #15: This GET endpoint triggers data mutation (importing from files).
|
||||
# Changed to POST. This should be admin-only.
|
||||
@api_view(['POST'])
|
||||
@authentication_classes([TokenAuthentication])
|
||||
@permission_classes([IsAuthenticated])
|
||||
@permission_classes([IsAdminUser])
|
||||
def add_from_files(request):
|
||||
add_from_files_tasks.delay()
|
||||
cache.delete('all_workouts')
|
||||
return Response(status=status.HTTP_200_OK)
|
||||
# Cache invalidation is handled in the task after import completes
|
||||
return Response(status=status.HTTP_200_OK)
|
||||
|
||||
Reference in New Issue
Block a user