Security review: staff admin lockdown, unicode-safe login/phone/grade inputs, demo guest not upgradable, home cache not floodable
- accounts admin: phone/is_guest read-only for non-superusers, device_id filter lookups refused, auth.User/Group admin writable only by superusers - campaigns: delivery log needs view permission - phone/OTP/birth date: any Unicode digit normalised or rejected with 400; unknown gradeId is 400 - shared demo guest is never turned into a person's account by logging in - /api/home with unknown gradeId is no longer cached (LocMemCache flood reset throttle counters) - /api/progress/nodes: one query, huge unit is 400 - BazaarCredential token filter lookups refused Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,3 @@
|
||||
from drf_spectacular.utils import extend_schema_field
|
||||
from rest_framework import serializers
|
||||
|
||||
from .models import Grade, QuestionReport
|
||||
@@ -215,15 +214,11 @@ class NodeProgressSerializer(serializers.Serializer):
|
||||
choices=['not_started', 'in_progress', 'completed', 'abandoned'],
|
||||
)
|
||||
lessonsCompleted = serializers.IntegerField(source='lessons_completed_count')
|
||||
lessonsTotal = serializers.SerializerMethodField()
|
||||
lessonsTotal = serializers.IntegerField(source='lessons_total')
|
||||
startedAt = serializers.DateTimeField(source='started_at', allow_null=True)
|
||||
completedAt = serializers.DateTimeField(source='completed_at', allow_null=True)
|
||||
lastActivityAt = serializers.DateTimeField(source='last_activity_at', allow_null=True)
|
||||
|
||||
@extend_schema_field(serializers.IntegerField)
|
||||
def get_lessonsTotal(self, obj):
|
||||
return obj.node.lessons.count()
|
||||
|
||||
|
||||
class NodeProgressListResponseSerializer(serializers.Serializer):
|
||||
progress = NodeProgressSerializer(many=True)
|
||||
|
||||
@@ -78,3 +78,77 @@ class NonAsciiDigitsAre400Tests(TestCase):
|
||||
self.assertEqual(r.status_code, 400)
|
||||
ok = self.app.post('/api/support/bug-reports', {'title': 't', 'description': 'x' * 5000}, format='json')
|
||||
self.assertIn(ok.status_code, (200, 201))
|
||||
|
||||
|
||||
class JunkGradeIdsDoNotFlushCacheTests(TestCase):
|
||||
"""
|
||||
هر ?gradeId=ِ ناموجود در /api/home یک کلیدِ کش میساخت؛ ۴۰۰ عددِ ساختگی LocMemCache را پر و
|
||||
شمارندهی سقفِ ورودِ ادمین را پاک میکرد (یازدهمین تلاش دوباره ۲۰۰ میگرفت، نه ۴۲۹).
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
from unittest import mock
|
||||
|
||||
from rest_framework.throttling import ScopedRateThrottle
|
||||
|
||||
cache.clear()
|
||||
self.addCleanup(cache.clear)
|
||||
# فقط برای سرعتِ تست؛ با سقفِ واقعی (۱۲۰ در دقیقه) همین کار حدودِ سه دقیقه طول میکشید.
|
||||
patcher = mock.patch.dict(ScopedRateThrottle.THROTTLE_RATES, {'read': '100000/min'})
|
||||
patcher.start()
|
||||
self.addCleanup(patcher.stop)
|
||||
user = User.objects.create(phone='09120009911', is_guest=False)
|
||||
self.app = APIClient()
|
||||
self.app.credentials(HTTP_AUTHORIZATION=f'Bearer {issue_access_token(user.id)}')
|
||||
|
||||
def test_admin_login_limit_survives_junk_grade_ids(self):
|
||||
anon = APIClient()
|
||||
for _ in range(10):
|
||||
anon.post('/admin/login/', {'username': 'x', 'password': 'y'})
|
||||
self.assertEqual(anon.post('/admin/login/', {'username': 'x', 'password': 'y'}).status_code, 429)
|
||||
for n in range(1000, 1400):
|
||||
self.assertEqual(self.app.get('/api/home', {'gradeId': n}).status_code, 404)
|
||||
self.assertEqual(anon.post('/admin/login/', {'username': 'x', 'password': 'y'}).status_code, 429)
|
||||
|
||||
def test_unknown_grade_is_not_cached_and_new_grade_still_loads(self):
|
||||
self.app.get('/api/home', {'gradeId': 777})
|
||||
self.assertFalse(any('grade:777' in k for k in cache._cache)) # LocMemCache در تستها
|
||||
grade = Grade.objects.create(title='تازه', icon='', color='')
|
||||
self.assertEqual(self.app.get('/api/home', {'gradeId': grade.id}).status_code, 200)
|
||||
|
||||
|
||||
class ProgressNodesInputAndQueriesTests(TestCase):
|
||||
"""?unit= بزرگتر از INTEGERِ SQLite ۵۰۰ میداد؛ lessonsTotal برای هر ردیف یک کوئریِ جدا بود."""
|
||||
|
||||
def setUp(self):
|
||||
cache.clear()
|
||||
self.user = User.objects.create(phone='09120009912', is_guest=False)
|
||||
self.app = APIClient()
|
||||
self.app.credentials(HTTP_AUTHORIZATION=f'Bearer {issue_access_token(self.user.id)}')
|
||||
|
||||
def test_huge_unit_is_400(self):
|
||||
self.assertEqual(self.app.get('/api/progress/nodes', {'unit': '9' * 25}).status_code, 400)
|
||||
|
||||
def test_query_count_does_not_grow_with_rows(self):
|
||||
from django.db import connection
|
||||
from django.test.utils import CaptureQueriesContext
|
||||
|
||||
from content.models import NodeProgress
|
||||
|
||||
unit = Unit.objects.create(grade=Grade.objects.create(title='g', icon='', color=''), title='u', order=1)
|
||||
|
||||
def add_nodes(start, n):
|
||||
for i in range(start, start + n):
|
||||
node = Node.objects.create(unit=unit, title=f'n{i}', order=i)
|
||||
Lesson.objects.create(node=node, title='l1')
|
||||
Lesson.objects.create(node=node, title='l2')
|
||||
NodeProgress.objects.create(user_id=self.user.id, node=node)
|
||||
|
||||
add_nodes(0, 2)
|
||||
with CaptureQueriesContext(connection) as small:
|
||||
self.app.get('/api/progress/nodes')
|
||||
add_nodes(2, 8)
|
||||
with CaptureQueriesContext(connection) as big:
|
||||
r = self.app.get('/api/progress/nodes')
|
||||
self.assertEqual(len(big), len(small))
|
||||
self.assertEqual([row['lessonsTotal'] for row in r.json()['progress']], [2] * 10)
|
||||
|
||||
@@ -122,14 +122,16 @@ def _grade_tree(grade_id: int):
|
||||
"""اسکلتِ گریدِ [{id,title,nodes:[{id,title,lesson_ids}]}] — بدونِ status/دیتای کاربر.
|
||||
None یعنی گریدِ ناموجود؛ [] یعنی گریدِ موجودِ بدونِ واحد."""
|
||||
# tree2: هر node حالا `playable_ids` هم دارد؛ کلیدِ ساختارِ قبلی باطل شد.
|
||||
# پایهی ناموجود کش نمیشود: gradeId از query string میآید و هر عددِ تازه یک کلیدِ تازه
|
||||
# میساخت. LocMemCache (تا CACHE_URL نیست) بعد از ۳۰۰ کلید یکسومِ کهنهها را دور میریزد و
|
||||
# شمارندههای throttle (سقفِ ورودِ ادمین، OTP بر اساسِ IP) هم در همین کشاند؛ با چند صد
|
||||
# gradeIdِ ساختگی سقفِ ورودِ ادمین صفر میشد (بازتولید شد، tests_security_review).
|
||||
if grade_id not in {g['id'] for g in _grades_payload()['grades']}:
|
||||
return None
|
||||
key = f'content:tree2:grade:{grade_id}:v{curriculum_version()}'
|
||||
cached = cache.get(key)
|
||||
if cached is not None:
|
||||
return None if cached is False else cached
|
||||
|
||||
if not Grade.objects.filter(pk=grade_id).exists():
|
||||
cache.set(key, False, _CACHE_TTL)
|
||||
return None
|
||||
return cached
|
||||
|
||||
units = (
|
||||
Grade.objects.get(pk=grade_id)
|
||||
@@ -738,10 +740,15 @@ class NodeProgressListView(APIView):
|
||||
tags=['Curriculum (content)'],
|
||||
)
|
||||
def get(self, request):
|
||||
qs = NodeProgress.objects.filter(user_id=request.user.id).select_related('node')
|
||||
# lessons_total یکجا در همین کوئری؛ پیشتر سریالایزر برای هر ردیف یک COUNT جدا میزد.
|
||||
qs = (
|
||||
NodeProgress.objects.filter(user_id=request.user.id).select_related('node')
|
||||
.annotate(lessons_total=Count('node__lessons'))
|
||||
)
|
||||
unit_id = request.query_params.get('unit')
|
||||
if unit_id:
|
||||
if not (unit_id.isascii() and unit_id.isdigit()):
|
||||
# سقفِ طول: عددِ بزرگتر از INTEGERِ SQLite روی فیلترِ node__unit_id خطای ۵۰۰ میداد.
|
||||
if not (unit_id.isascii() and unit_id.isdigit() and len(unit_id) <= 9):
|
||||
raise ValidationError({'unit': 'شناسهی فصل باید عدد باشد.'})
|
||||
qs = qs.filter(node__unit_id=int(unit_id))
|
||||
|
||||
|
||||
Reference in New Issue
Block a user