From 28b852776042d45ebd5f2ecc8aa62adf3869cda2 Mon Sep 17 00:00:00 2001 From: Zareian Date: Mon, 5 Oct 2026 09:34:42 +0330 Subject: [PATCH] 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 --- accounts/admin.py | 48 +++++++++ accounts/jalali.py | 7 +- accounts/phone.py | 18 +++- accounts/serializers.py | 49 +++++++-- accounts/tests.py | 6 +- accounts/tests_security_review.py | 153 +++++++++++++++++++++++++++++ accounts/views.py | 12 ++- campaigns/admin.py | 11 +-- campaigns/tests_security_review.py | 30 ++++++ config/tests.py | 3 + content/serializers.py | 7 +- content/tests_security_review.py | 74 ++++++++++++++ content/views.py | 21 ++-- payment/admin.py | 6 ++ payment/tests_security_review.py | 18 ++++ 15 files changed, 422 insertions(+), 41 deletions(-) create mode 100644 campaigns/tests_security_review.py diff --git a/accounts/admin.py b/accounts/admin.py index a010794..791b5e2 100644 --- a/accounts/admin.py +++ b/accounts/admin.py @@ -67,6 +67,13 @@ class DeviceIdForSuperuserOnly: exclude = tuple(super().get_exclude(request, obj) or ()) return exclude if request.user.is_superuser else exclude + ('device_id',) + def lookup_allowed(self, lookup, value, request=None): + # جنگو هر فیلترِ querystring روی فیلدِ خودِ مدل را می‌پذیرد (?device_id__startswith=a)؛ + # بی‌این، با شمردنِ نتیجه شناسه حرف‌به‌حرف درمی‌آید. جنگو برای فیلترِ ردشده ۴۰۰ می‌دهد. + if lookup.split('__')[0] == 'device_id' and not (request and request.user.is_superuser): + return False + return super().lookup_allowed(lookup, value, request) + @admin.register(User) class UserAdmin(DeviceIdForSuperuserOnly, admin.ModelAdmin): @@ -90,6 +97,13 @@ class UserAdmin(DeviceIdForSuperuserOnly, admin.ModelAdmin): list_filter = ('is_guest',) actions = ['send_sms_to_selected'] + def get_readonly_fields(self, request, obj=None): + # شماره کلیدِ ورود است: کارمندی با change_user که شماره‌ی کاربری را به شماره‌ی خودش عوض + # کند، کد می‌گیرد و واردِ همان حساب می‌شود. is_guest هم همین‌طور ورود با شناسه‌ی دستگاه را + # روی حسابِ شماره‌دار باز می‌کند. هر دو فقط برای ابرکاربر قابلِ تغییرند. + fields = tuple(super().get_readonly_fields(request, obj)) + return fields if request.user.is_superuser else fields + ('phone', 'is_guest') + # permissions=['change']: بدونِ این، کاربرِ فقط-view (گروهِ Supervisor) # می‌تونست با POSTِ مستقیم این اکشن رو صدا بزنه بدونِ دیدنِ دکمه در UI؛ # این پارامتر باعث می‌شه ModelAdmin فقط برای change_user در دسترس بذارتش. @@ -250,3 +264,37 @@ def _admin_get_urls_with_supervisor(): admin.site.get_urls = _admin_get_urls_with_supervisor + + +# حساب‌های کارمندانِ پنل (auth.User/Group جنگو، جدا از accounts.User). در پیش‌فرضِ جنگو کارمندی با +# change_group همه‌ی اجازه‌ها را به گروهِ خودش می‌دهد و با change_user (auth) تیکِ ابرکاربر را برای +# خودش می‌زند. ساختن/تغییر/حذفِ این دو فقط برای ابرکاربر است؛ دیدن همان اجازه‌ی view را می‌خواهد. +from django.contrib.auth import get_user_model # noqa: E402 +from django.contrib.auth.admin import GroupAdmin as _DjangoGroupAdmin # noqa: E402 +from django.contrib.auth.admin import UserAdmin as _DjangoUserAdmin # noqa: E402 +from django.contrib.auth.models import Group # noqa: E402 + + +class _SuperuserWritesOnly: + def has_add_permission(self, request): + return request.user.is_superuser + + def has_change_permission(self, request, obj=None): + return request.user.is_superuser + + def has_delete_permission(self, request, obj=None): + return request.user.is_superuser + + +class StaffUserAdmin(_SuperuserWritesOnly, _DjangoUserAdmin): + pass + + +class StaffGroupAdmin(_SuperuserWritesOnly, _DjangoGroupAdmin): + pass + + +admin.site.unregister(get_user_model()) +admin.site.register(get_user_model(), StaffUserAdmin) +admin.site.unregister(Group) +admin.site.register(Group, StaffGroupAdmin) diff --git a/accounts/jalali.py b/accounts/jalali.py index fd519a5..ff7e056 100644 --- a/accounts/jalali.py +++ b/accounts/jalali.py @@ -81,10 +81,11 @@ def from_gregorian(d: date) -> tuple[int, int, int]: def parse(value: str) -> date: """«1385-04-12» (ارقامِ فارسی هم) → تاریخِ میلادی.""" - from accounts.phone import _DIGITS + from accounts.phone import ascii_digits - parts = str(value).translate(_DIGITS).strip().replace('/', '-').split('-') - if len(parts) != 3 or not all(p.isdigit() for p in parts): + parts = ascii_digits(str(value)).strip().replace('/', '-').split('-') + # isascii: «²» برای isdigit رقم است ولی int() آن را نمی‌پذیرد (پیامِ خطای انگلیسیِ پایتون). + if len(parts) != 3 or not all(p.isascii() and p.isdigit() for p in parts): raise ValueError('تاریخ باید به شکلِ سال-ماه-روز باشد، مثلاً 1385-04-12.') return to_gregorian(int(parts[0]), int(parts[1]), int(parts[2])) diff --git a/accounts/phone.py b/accounts/phone.py index 33cc0e7..38eadad 100644 --- a/accounts/phone.py +++ b/accounts/phone.py @@ -11,15 +11,27 @@ (کلیدهای TEST_LOGIN_PHONES)؛ فیلدِ serializer در accounts/serializers.py است. """ import re +import unicodedata -_DIGITS = str.maketrans('۰۱۲۳۴۵۶۷۸۹٠١٢٣٤٥٦٧٨٩', '01234567890123456789') _SEPARATORS = re.compile(r'[\s\-().\u200c\u200e\u200f]') -_MOBILE = re.compile(r'09\d{9}') +# [0-9] نه \d: در پایتون \d هر رقمِ یونیکد (تمام‌عرض، ریاضی، دوناگری…) را هم می‌گیرد. +_MOBILE = re.compile(r'09[0-9]{9}') + + +def ascii_digits(value: str) -> str: + """ + هر رقمِ دهدهیِ یونیکد (فارسی، عربی، تمام‌عرض «0»، ریاضی «𝟎»، …) → رقمِ لاتین. + + بدونِ این «09123456789» شماره‌ی جدایی می‌شد با سقفِ پیامک و حسابِ جدا. «²» و «①» رقمِ + دهدهی نیستند و دست نمی‌خورند، پس شماره‌ای که آن‌ها را دارد رد می‌شود. + """ + return ''.join(str(unicodedata.decimal(ch)) if not ch.isascii() and ch.isdecimal() else ch + for ch in value) def normalize_phone(value) -> str | None: """شکلِ یکتای ‎09XXXXXXXXX‎، یا None اگر موبایلِ ایران نباشد.""" - s = _SEPARATORS.sub('', str(value or '').translate(_DIGITS)) + s = _SEPARATORS.sub('', ascii_digits(str(value or ''))) if s.startswith('+98'): s = '0' + s[3:] elif s.startswith('0098'): diff --git a/accounts/serializers.py b/accounts/serializers.py index 3b34276..c1e3aa0 100644 --- a/accounts/serializers.py +++ b/accounts/serializers.py @@ -1,14 +1,17 @@ import re +import unicodedata from rest_framework import serializers -from .phone import normalize_phone +from .phone import ascii_digits, normalize_phone -# لینک، آیدیِ شبکه‌ی اجتماعی، و هر رشته‌ی ۵ رقمی یا بیشتر (شماره‌ی تلفن با -# فاصله/خط‌تیره هم) — ارقامِ فارسی و عربی هم. این نام در رتبه‌بندی به بچه‌های دیگر نشان داده می‌شود. +# لینک، آیدیِ شبکه‌ی اجتماعی، و هر رشته‌ی ۵ رقمی یا بیشتر با هر جداکننده‌ی غیرِ حرفی (فاصله، +# خط‌تیره، ستاره، ایموجی…). \d در پایتون هر رقمِ یونیکد است (فارسی، عربی، تمام‌عرض، ریاضی). +# این نام در رتبه‌بندی به بچه‌های دیگر نشان داده می‌شود. حرف بینِ رقم‌ها («0x9x1…») و عدد با +# حروف («صفر نه یک…») گرفته نمی‌شود. _CONTACT = re.compile( r'(https?://\S+|www\.\S+|\S+\.(?:com|ir|org|net|me)\S*|@\w+' - r'|(?:[0-9۰-۹٠-٩][\s\-._/\\|,،]*){5,})', + r'|(?:\d(?:[^\w@]|_)*){5,})', re.IGNORECASE, ) # «ig: name»، «insta name»، «آیدی name» و مانندش: کلیدواژه و هر چه بعدش آمده حذف می‌شود. @@ -20,7 +23,9 @@ _HANDLE = re.compile( def clean_display_name(value: str) -> str: - value = _HANDLE.sub(' ', value or '') + # NFKC: «①»، «²»، «𝟬» و «@» به رقم و @ِ معمولی برمی‌گردند تا الگوهای پایین آن‌ها را هم بگیرند؛ + # نامِ ذخیره‌شده همین شکلِ نرمال است. + value = _HANDLE.sub(' ', unicodedata.normalize('NFKC', value or '')) return ' '.join(_CONTACT.sub(' ', value).split())[:30] @@ -45,9 +50,30 @@ class GuestSessionRequestSerializer(serializers.Serializer): deviceId = serializers.CharField(max_length=64) +class OTPCodeField(serializers.CharField): + """ + کدِ ورود: ۴ تا ۶ رقم (کدِ پیامکی ۶ رقمی است، کدهای TEST_LOGIN_PHONES ۴ تا ۶). رقمِ فارسی و + دیگر رقم‌های یونیکد به لاتین برمی‌گردند؛ هر چیزِ دیگر ۴۰۰ است. بدونِ این، کدِ غیرِ ASCII برای + شماره‌ی تست به secrets.compare_digest می‌رسید و TypeError (۵۰۰) می‌داد. + """ + + def __init__(self, **kwargs): + kwargs.setdefault('max_length', 6) + super().__init__(**kwargs) + + def to_internal_value(self, data): + code = ascii_digits(super().to_internal_value(data)) + if not re.fullmatch(r'[0-9]{4,6}', code): + raise serializers.ValidationError('کد باید ۴ تا ۶ رقم باشد.') + return code + + class LoginRequestSerializer(serializers.Serializer): phone = IranMobileField() - otp = serializers.CharField(max_length=6) + otp = OTPCodeField() + # فرانتِ فعلی نمی‌فرستد؛ اگر بیاید روی Session.device_id می‌نشیند. null یا بیش از ۶۴ نویسه ۴۰۰ است + # (پیش‌تر null به NOT NULL می‌خورد و ۵۰۰ می‌داد). + deviceId = serializers.CharField(max_length=64, required=False, allow_blank=True) class RefreshRequestSerializer(serializers.Serializer): @@ -57,6 +83,15 @@ class RefreshRequestSerializer(serializers.Serializer): class SetGradeRequestSerializer(serializers.Serializer): gradeId = serializers.IntegerField() + def validate_gradeId(self, value): + # پایه‌ی ناموجود ۴۰۰ است، نه ذخیره‌ی بی‌صدا (نقشه‌ی خالی). عددِ بزرگ‌تر از ستونِ SQLite هم + # پیش‌تر هنگامِ ذخیره OverflowError (۵۰۰) می‌داد؛ exists() آن را بی‌خطا «نیست» می‌بیند. + from content.models import Grade + + if not Grade.objects.filter(pk=value).exists(): + raise serializers.ValidationError('این پایه وجود ندارد.') + return value + # همان بازه‌ای که پیش‌تر برای «سن» پذیرفته می‌شد. بزرگسال هم می‌آید (سینیور، # ۲۰۲۶-۱۰-۰۱): کسی که فقط یک مبحثِ ریاضی را می‌خواهد یاد بگیرد. @@ -182,7 +217,7 @@ class ProfileResponseSerializer(serializers.Serializer): class DeleteAccountRequestSerializer(serializers.Serializer): # کاربرِ شماره‌دار: کدِ پیامکیِ تازه (از POST /api/auth/otp/request با همان شماره). - otp = serializers.CharField(max_length=6, required=False, allow_blank=True) + otp = OTPCodeField(required=False, allow_blank=True) # مهمان شماره‌ای ندارد که کد بگیرد؛ باید همین کلمه را تایپ کرده باشد. confirm = serializers.CharField(max_length=10, required=False, allow_blank=True) diff --git a/accounts/tests.py b/accounts/tests.py index e8f4004..2ea3527 100644 --- a/accounts/tests.py +++ b/accounts/tests.py @@ -348,12 +348,12 @@ class LoginPhoneRateLimitTests(TestCase): for _ in range(3): response = self.client.post( - '/api/auth/login', {'phone': phone, 'otp': 'wrong'}, format='json', + '/api/auth/login', {'phone': phone, 'otp': '000000'}, format='json', ) self.assertEqual(response.status_code, 400) blocked_response = self.client.post( - '/api/auth/login', {'phone': phone, 'otp': 'wrong'}, format='json', + '/api/auth/login', {'phone': phone, 'otp': '000000'}, format='json', ) self.assertEqual(blocked_response.status_code, 429) @@ -1476,6 +1476,8 @@ class AuthUserRowCacheTests(TestCase): @override_settings(AUTH_USER_CACHE_TTL=30) def test_set_grade_still_works_with_cache_on(self): + from content.models import Grade + Grade.objects.create(pk=3, title='g3', icon='', color='') r = self.client.post( '/api/user/grade', {'gradeId': 3}, format='json', HTTP_AUTHORIZATION=f'Bearer {self.token}') diff --git a/accounts/tests_security_review.py b/accounts/tests_security_review.py index 6e82abb..035fbb9 100644 --- a/accounts/tests_security_review.py +++ b/accounts/tests_security_review.py @@ -177,3 +177,156 @@ class SupervisorDoesNotSeeDeviceIdsTests(TestCase): self.assertNotContains(form, 'secret-device-123') sessions = self.client.get('/admin/accounts/session/') self.assertNotContains(sessions, 'secret-device-123') + + +def _staff_with(*perms, username='staff1'): + from django.contrib.auth.models import Permission + staff = get_user_model().objects.create_user(username, f'{username}@example.com', 'pw-for-tests-only', is_staff=True) + for app_label, codename in perms: + staff.user_permissions.add(Permission.objects.get(content_type__app_label=app_label, codename=codename)) + return staff + + +class DeviceIdNotGuessableByAdminFilterTests(TestCase): + """فیلترِ querystringِ changelist (?device_id__startswith=…) شناسه‌ی دستگاه را برای غیرِ ابرکاربر لو نمی‌دهد.""" + + def setUp(self): + call_command('setup_supervisor_group', verbosity=0) + self.guest = User.objects.create(device_id='abc-secret-999', is_guest=True) + from accounts.models import Session + Session.objects.create(user=self.guest, refresh_token='h' * 64, device_id='abc-secret-999', + expires_at=timezone.now() + timezone.timedelta(days=1)) + staff = _staff_with(('accounts', 'view_session')) + staff.groups.add(Group.objects.get(name='Supervisor')) + self.client.force_login(staff) + + def test_filter_on_device_id_is_refused(self): + for lookup in ('device_id__startswith', 'device_id__regex', 'device_id'): + r = self.client.get('/admin/accounts/user/', {lookup: 'abc'}) + self.assertEqual(r.status_code, 400, lookup) + self.assertNotContains(r, f'/admin/accounts/user/{self.guest.pk}/change/', status_code=400) + r = self.client.get('/admin/accounts/session/', {'device_id__startswith': 'abc'}) + self.assertEqual(r.status_code, 400) + + def test_superuser_still_filters(self): + root = get_user_model().objects.create_superuser('root', 'root@example.com', 'pw-for-tests-only') + self.client.force_login(root) + r = self.client.get('/admin/accounts/user/', {'device_id__startswith': 'abc'}) + self.assertContains(r, f'/admin/accounts/user/{self.guest.pk}/change/') + + +class StaffCannotSwapLoginPhoneTests(TestCase): + """کارمندی با change_user شماره‌ی کاربر را عوض نمی‌کند؛ وگرنه با شماره‌ی خودش کد می‌گیرد و واردِ آن حساب می‌شود.""" + + def setUp(self): + cache.clear() + self.victim = User.objects.create(phone='09120000077', is_guest=False, name='victim') + self.client.force_login(_staff_with(('accounts', 'change_user'), ('accounts', 'view_user'))) + + def tearDown(self): + cache.clear() + + def test_phone_and_is_guest_are_read_only(self): + r = self.client.post(f'/admin/accounts/user/{self.victim.pk}/change/', { + 'phone': '09129999999', 'name': 'renamed', 'avatar': '', 'grade_id': '', 'is_guest': 'on'}) + self.assertEqual(r.status_code, 302) + self.victim.refresh_from_db() + self.assertEqual(self.victim.name, 'renamed') + self.assertEqual(self.victim.phone, '09120000077') + self.assertFalse(self.victim.is_guest) + login = APIClient().post('/api/auth/login', {'phone': '09129999999', 'otp': _otp_row('09129999999')}, + format='json') + self.assertNotEqual(login.json().get('userId'), str(self.victim.pk)) + + +class StaffAccountsAdminSuperuserOnlyTests(TestCase): + """auth.User/Group پنل: با change_group یا change_user (auth) کارمند خودش را ابرکاربر یا همه‌کاره نمی‌کند.""" + + def test_change_group_cannot_grant_all_permissions(self): + from django.contrib.auth.models import Permission + call_command('setup_supervisor_group', verbosity=0) + staff = _staff_with(('auth', 'change_group'), ('auth', 'view_group')) + group = Group.objects.get(name='Supervisor') + staff.groups.add(group) + self.client.force_login(staff) + r = self.client.post(f'/admin/auth/group/{group.pk}/change/', + {'name': 'Supervisor', 'permissions': list(Permission.objects.values_list('pk', flat=True))}) + self.assertEqual(r.status_code, 403) + staff = get_user_model().objects.get(pk=staff.pk) + self.assertFalse(staff.has_perm('accounts.add_otpcode')) + + def test_change_user_cannot_make_self_superuser(self): + staff = _staff_with(('auth', 'change_user'), ('auth', 'view_user')) + self.client.force_login(staff) + form = self.client.get(f'/admin/auth/user/{staff.pk}/change/') + self.assertEqual(form.status_code, 200) + self.assertNotContains(form, 'name="is_superuser"') + r = self.client.post(f'/admin/auth/user/{staff.pk}/change/', { + 'username': staff.username, 'is_active': 'on', 'is_staff': 'on', 'is_superuser': 'on', + 'last_login_0': '', 'last_login_1': '', 'date_joined_0': '2026-01-01', 'date_joined_1': '00:00:00'}) + self.assertEqual(r.status_code, 403) + self.assertFalse(get_user_model().objects.get(pk=staff.pk).is_superuser) + + +class UnicodeInputsAreNormalisedOrRejectedTests(TestCase): + """رقم‌های تمام‌عرض/ریاضی، کدِ غیرِ ASCII، پایه‌ی ناموجود و deviceIdِ null: ۴۰۰ یا نرمال‌شده، نه ۵۰۰ و نه حسابِ جدا.""" + + def setUp(self): + cache.clear() + self.addCleanup(cache.clear) + + def test_fullwidth_and_math_digits_normalise_to_same_phone(self): + from accounts.phone import normalize_phone + + for variant in ('09123456789', '𝟎𝟗𝟏𝟐𝟑𝟒𝟓𝟔𝟕𝟖𝟗', '۰۹۱۲۳۴۵۶۷۸۹', '+98 912 345 6789'): + self.assertEqual(normalize_phone(variant), '09123456789', variant) + self.assertIsNone(normalize_phone('0912345678²')) + + def test_non_ascii_otp_is_400_not_500(self): + phone = '09120000099' + with override_settings(TEST_LOGIN_PHONES={phone: '123456'}): + for code in ('²²²²', '١٢٣٤٥٦٧'[:6] + 'x', 'abcd'): + r = APIClient().post('/api/auth/login', {'phone': phone, 'otp': code}, format='json') + self.assertEqual(r.status_code, 400, code) + + def test_login_device_id_null_is_400(self): + phone = '09120000098' + code = _otp_row(phone) + r = APIClient().post('/api/auth/login', {'phone': phone, 'otp': code, 'deviceId': None}, format='json') + self.assertEqual(r.status_code, 400) + + def test_unknown_or_huge_grade_is_400(self): + user = User.objects.create(phone='09120000097', is_guest=False) + app = _bearer(issue_access_token(user.id)) + for gid in (99999, 10 ** 30): + self.assertEqual(app.post('/api/user/grade', {'gradeId': gid}, format='json').status_code, 400, gid) + + def test_birth_date_with_superscript_digit_is_400(self): + user = User.objects.create(phone='09120000096', is_guest=False) + r = _bearer(issue_access_token(user.id)).patch('/api/user/profile', {'birthDate': '13²5-04-12'}, format='json') + self.assertEqual(r.status_code, 400) + + def test_display_name_nfkc_contacts_removed(self): + from accounts.serializers import clean_display_name as clean + + for sample in ('09123456789', '𝟎𝟗𝟏𝟐𝟑𝟒𝟓𝟔𝟕𝟖𝟗', '0912*345*6789', '@kid2014'): + self.assertEqual(clean(sample), '', sample) + self.assertEqual(clean('علی رضایی'), 'علی رضایی') + + +class DemoGuestNotUpgradableTests(TestCase): + """مهمانِ نمایشیِ مشترک با ورودِ یک شماره به آن شماره تبدیل نمی‌شود؛ شماره‌ی تازه حسابِ تازه می‌گیرد.""" + + def test_demo_guest_keeps_its_identity(self): + cache.clear() + self.addCleanup(cache.clear) + s = _guest('frontend-test-device') + demo_id = int(s['userId']) + phone = '09120000095' + code = _otp_row(phone) + r = _bearer(s['token']).post('/api/auth/login', {'phone': phone, 'otp': code}, format='json') + self.assertEqual(r.status_code, 200) + self.assertNotEqual(int(r.json()['userId']), demo_id) + demo = User.objects.get(pk=demo_id) + self.assertIsNone(demo.phone) + self.assertTrue(demo.is_guest) diff --git a/accounts/views.py b/accounts/views.py index 9f7981d..ae6bc25 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -390,7 +390,7 @@ class LoginView(APIView): Session.objects.create( user=user, refresh_token=hash_refresh_token(refresh_token), - device_id=request.data.get('deviceId', ''), + device_id=serializer.validated_data.get('deviceId', ''), expires_at=timezone.now() + timezone.timedelta(days=settings.REFRESH_TOKEN_TTL_DAYS), ) access_token = issue_access_token(user.id) @@ -414,15 +414,19 @@ def _upgradable_guest(request, phone: str): """ مهمانی که با توکنِ خودش درخواستِ ورود داده و این شماره هنوز حسابی ندارد. - شماره‌های TEST_LOGIN_PHONES عمداً کنار گذاشته می‌شوند: حساب‌های دمو با - همین شماره‌ها ساخته شده‌اند و ورودِ آزمایشی نباید حسابِ مهمانِ دمو - (`frontend-test-device`) را به یک شماره تبدیل کند. + شماره‌های TEST_LOGIN_PHONES و مهمان‌های DEMO_GUEST_DEVICE_IDS عمداً کنار گذاشته می‌شوند: + هر دو حسابِ نمایشیِ مشترک‌اند و ورود نباید یکی را به دیگری یا به شماره‌ی یک نفر تبدیل کند. """ if phone in settings.TEST_LOGIN_PHONES: return None user = getattr(request, 'user', None) if user is None or not getattr(user, 'is_authenticated', False) or not user.is_guest: return None + # مهمانِ نمایشیِ عمومی (DEMO_GUEST_DEVICE_IDS) هم مالِ یک نفر نیست: بدونِ این، هر کس از + # نشستِ آن با شماره‌ی خودش وارد می‌شد کلِ حساب و داده‌ی دمو را به شماره‌ی خودش می‌برد و + # نشستِ بقیه را باطل می‌کرد. آن کاربر حسابِ تازه‌ی خودش را می‌گیرد. + if user.device_id in settings.DEMO_GUEST_DEVICE_IDS: + return None return User.objects.filter(pk=user.pk, is_guest=True, phone__isnull=True).first() diff --git a/campaigns/admin.py b/campaigns/admin.py index ac56ac7..d60d045 100644 --- a/campaigns/admin.py +++ b/campaigns/admin.py @@ -33,8 +33,8 @@ class MessageCampaignAdmin(admin.ModelAdmin): @admin.register(CampaignDelivery) class CampaignDeliveryAdmin(admin.ModelAdmin): """ - نمایشِ read-only: لیست و صفحه‌ی جزئیاتِ هر رکورد همیشه قابلِ - مشاهده‌ست (has_view_permission صریحاً True)، ولی هیچ راهی برای + نمایشِ read-only: لیست و صفحه‌ی جزئیات فقط با اجازه‌ی view_campaigndelivery + (رفتارِ پیش‌فرضِ جنگو)، و هیچ راهی برای افزودن، ویرایش یا حذفِ رکورد از ادمین وجود نداره تا وضعیتِ تحویل (که فقط باید توسطِ campaigns.services.execute_campaign آپدیت بشه) از ادمین دست‌کاری نشه. @@ -46,13 +46,6 @@ class CampaignDeliveryAdmin(admin.ModelAdmin): fields = ('campaign', 'user_id', 'channel', 'status', 'error_message', 'created_at') readonly_fields = fields - def has_view_permission(self, request, obj=None): - # صریحاً True: بدونِ override، fallbackِ پیش‌فرضِ Django به کاربرِ - # staffِ بدونِ پرمیشنِ view/change حتی اجازه‌ی دیدنِ این صفحه‌ی - # read-only رو هم نمی‌ده — یعنی مدل عملاً غیرقابلِ‌دسترس می‌شه، نه - # فقط view-only. - return True - def has_add_permission(self, request): return False diff --git a/campaigns/tests_security_review.py b/campaigns/tests_security_review.py new file mode 100644 index 0000000..7788134 --- /dev/null +++ b/campaigns/tests_security_review.py @@ -0,0 +1,30 @@ +""" +حفاظ‌های بازبینیِ امنیتی (کمپین‌ها در پنل). +""" +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission +from django.test import TestCase + +from .models import CampaignDelivery, MessageCampaign + + +class CampaignDeliveryNeedsViewPermissionTests(TestCase): + """لاگِ تحویلِ کمپین (user_id، متنِ خطای ارسال) فقط با view_campaigndelivery دیده می‌شود، نه برای هر کارمند.""" + + def setUp(self): + campaign = MessageCampaign.objects.create(title='t', body='b', channel='sms') + self.delivery = CampaignDelivery.objects.create(campaign=campaign, user_id=5, channel='sms', + status='failed', error_message='e') + self.staff = get_user_model().objects.create_user('st', 'st@example.com', 'pw-for-tests-only', is_staff=True) + self.client.force_login(self.staff) + + def test_staff_without_permission_gets_403(self): + self.assertEqual(self.client.get('/admin/campaigns/campaigndelivery/').status_code, 403) + self.assertEqual( + self.client.get(f'/admin/campaigns/campaigndelivery/{self.delivery.pk}/change/').status_code, 403) + + def test_view_permission_still_read_only(self): + self.staff.user_permissions.add( + Permission.objects.get(content_type__app_label='campaigns', codename='view_campaigndelivery')) + self.assertEqual(self.client.get('/admin/campaigns/campaigndelivery/').status_code, 200) + self.assertEqual(self.client.get('/admin/campaigns/campaigndelivery/add/').status_code, 403) diff --git a/config/tests.py b/config/tests.py index aeb3d02..2442626 100644 --- a/config/tests.py +++ b/config/tests.py @@ -174,6 +174,9 @@ class APIRequestLoggingMiddlewareTests(TestCase): user = User.objects.create(device_id='device-mw-log-1', is_guest=True, name='') token = issue_access_token(user.id) + from content.models import Grade + Grade.objects.create(pk=10, title='g10', icon='', color='') + with self.assertLogs('config.middleware', level='INFO') as captured: response = self.client.post( '/api/user/grade', diff --git a/content/serializers.py b/content/serializers.py index fa90400..4778e7a 100644 --- a/content/serializers.py +++ b/content/serializers.py @@ -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) diff --git a/content/tests_security_review.py b/content/tests_security_review.py index 44d0741..2b8154d 100644 --- a/content/tests_security_review.py +++ b/content/tests_security_review.py @@ -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) diff --git a/content/views.py b/content/views.py index 1164da3..410611e 100644 --- a/content/views.py +++ b/content/views.py @@ -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)) diff --git a/payment/admin.py b/payment/admin.py index fcfec5a..5de5feb 100644 --- a/payment/admin.py +++ b/payment/admin.py @@ -122,6 +122,12 @@ class BazaarCredentialAdmin(admin.ModelAdmin): def connected(self, obj): return bool(obj.refresh_token) + def lookup_allowed(self, lookup, value, request=None): + # فیلترِ querystring (?refresh_token__startswith=a) با شمردنِ نتیجه توکن را حرف‌به‌حرف لو می‌دهد. + if lookup.split('__')[0] in ('refresh_token', 'pending_state'): + return False + return super().lookup_allowed(lookup, value, request) + def changelist_view(self, request, extra_context=None): BazaarCredential.objects.get_or_create(pk=1) return super().changelist_view(request, extra_context) diff --git a/payment/tests_security_review.py b/payment/tests_security_review.py index 48ca4d9..602d816 100644 --- a/payment/tests_security_review.py +++ b/payment/tests_security_review.py @@ -227,3 +227,21 @@ class DiscountDuringPaymentRequestTests(_Base): self.assertEqual(order.amount, self.pack.price // 2) self.assertEqual(requested, [self.pack.price, self.pack.price // 2]) self.assertEqual(order.gateway_authority, 'A-2') + + +class BazaarTokenNotGuessableByAdminFilterTests(TestCase): + """فیلترِ querystring روی refresh_token/pending_state توکنِ بازار را حرف‌به‌حرف لو نمی‌دهد.""" + + def test_filter_is_refused(self): + from django.contrib.auth import get_user_model + from django.contrib.auth.models import Permission + + from .models import BazaarCredential + BazaarCredential.objects.update_or_create(pk=1, defaults={'refresh_token': 'TOPSECRET', 'pending_state': 'st8'}) + staff = get_user_model().objects.create_user('bz', 'bz@example.com', 'pw-for-tests-only', is_staff=True) + staff.user_permissions.add(Permission.objects.get(content_type__app_label='payment', codename='view_bazaarcredential')) + self.client.force_login(staff) + self.assertEqual(self.client.get('/admin/payment/bazaarcredential/').status_code, 200) + for lookup in ('refresh_token__startswith', 'pending_state__startswith', 'refresh_token__regex'): + r = self.client.get('/admin/payment/bazaarcredential/', {lookup: 'T'}) + self.assertEqual(r.status_code, 400, lookup)