Security review fixes: admin permissions, guest sessions, payment edge cases, assistant leaks
- Admin: refund/cancel/mark-resolved/Bazaar-link actions need change permission; question and curriculum import need add/delete; content browser needs view_question; guest device ids are shown to superusers only. - Accounts: registering a guest's phone revokes the guest's earlier sessions; self-deletion keeps Redemption, UserOffer and the phone's OTP rows; the public demo guest cannot be deleted (DEMO_GUEST_DEVICE_IDS); display-name contact filter covers any separator and social handles. - Payment: callback without Authority is 400; a second Bazaar purchase on a paid order is 409; orders snapshot credits/hearts/days at creation (payment.0011). - Assistant: a broken diagram no longer makes a streamed answer free; a late off-topic marker refuses the answer (not stored, not cached); a disconnect saves the partial answer; only KNOWN_COLLECTIONS are searched. - Infra: secret-key file is created atomically (workers no longer split keys); no default INTERNAL_SERVICE_KEY; Kavenegar key and Bazaar token kept out of error logs; Swagger/ReDoc pinned; schema throttled; docker compose bound to 127.0.0.1; non-ASCII digit ids are 400; ticket text capped at 5000. - docs/security-before-production.md: remaining open items 24-31 and the guard tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -49,8 +49,27 @@ class SendSMSForm(forms.Form):
|
||||
return cleaned_data
|
||||
|
||||
|
||||
class DeviceIdForSuperuserOnly:
|
||||
"""
|
||||
شناسهی دستگاه برای مهمان تنها رمزِ ورود است (`POST /api/auth/session`)؛ پس فقط ابرکاربر
|
||||
آن را در فهرست، جستجو و فرم میبیند، نه کارمندی با اجازهی دیدن (گروهِ Supervisor).
|
||||
"""
|
||||
|
||||
def get_list_display(self, request):
|
||||
cols = super().get_list_display(request)
|
||||
return cols if request.user.is_superuser else tuple(c for c in cols if c != 'device_id')
|
||||
|
||||
def get_search_fields(self, request):
|
||||
cols = super().get_search_fields(request)
|
||||
return cols if request.user.is_superuser else tuple(c for c in cols if c != 'device_id')
|
||||
|
||||
def get_exclude(self, request, obj=None):
|
||||
exclude = tuple(super().get_exclude(request, obj) or ())
|
||||
return exclude if request.user.is_superuser else exclude + ('device_id',)
|
||||
|
||||
|
||||
@admin.register(User)
|
||||
class UserAdmin(admin.ModelAdmin):
|
||||
class UserAdmin(DeviceIdForSuperuserOnly, admin.ModelAdmin):
|
||||
list_display = ('id', 'public_id', 'name', 'phone', 'device_id', 'is_guest', 'grade_id', 'created_at')
|
||||
search_fields = ('name', 'phone', 'device_id')
|
||||
|
||||
@@ -165,7 +184,7 @@ class ProfileAdmin(admin.ModelAdmin):
|
||||
|
||||
|
||||
@admin.register(Session)
|
||||
class SessionAdmin(admin.ModelAdmin):
|
||||
class SessionAdmin(DeviceIdForSuperuserOnly, admin.ModelAdmin):
|
||||
list_display = ('id', 'user', 'device_id', 'expires_at', 'revoked')
|
||||
list_filter = ('revoked',)
|
||||
|
||||
|
||||
@@ -68,15 +68,20 @@ def plan(user_ids):
|
||||
return counts
|
||||
|
||||
|
||||
def delete_users(user_ids, keep=()):
|
||||
"""keep: برچسبِ مدلهایی که نمیمانند پاک شوند (مثلاً 'payment.Order' برای حذفِ حسابِ خودِ کاربر)."""
|
||||
def delete_users(user_ids, keep=(), delete_otp_codes=True):
|
||||
"""
|
||||
keep: برچسبِ مدلهایی که نباید پاک شوند (مثلاً 'payment.Order' برای حذفِ حسابِ خودِ کاربر).
|
||||
delete_otp_codes: این دستور (کارِ ادمین) کدهای شماره را پاک میکند تا آن شماره از صفر شروع کند؛
|
||||
حذفِ حساب به دستِ خودِ کاربر این کار را نمیکند تا سقفِ پیامکِ شماره دور زده نشود.
|
||||
"""
|
||||
phones = list(User.objects.filter(pk__in=user_ids).exclude(phone=None).values_list('phone', flat=True))
|
||||
with transaction.atomic():
|
||||
for model, column in user_scoped_models():
|
||||
if model._meta.label in keep:
|
||||
continue
|
||||
model.objects.filter(**{f'{column}__in': user_ids}).delete()
|
||||
OTPCode.objects.filter(phone__in=phones).delete()
|
||||
if delete_otp_codes:
|
||||
OTPCode.objects.filter(phone__in=phones).delete()
|
||||
User.objects.filter(pk__in=user_ids).delete()
|
||||
|
||||
|
||||
|
||||
@@ -5,16 +5,23 @@ from rest_framework import serializers
|
||||
from .phone import normalize_phone
|
||||
|
||||
# لینک، آیدیِ شبکهی اجتماعی، و هر رشتهی ۵ رقمی یا بیشتر (شمارهی تلفن با
|
||||
# فاصله/خطتیره هم) — ارقامِ فارسی و عربی هم.
|
||||
# فاصله/خطتیره هم) — ارقامِ فارسی و عربی هم. این نام در رتبهبندی به بچههای دیگر نشان داده میشود.
|
||||
_CONTACT = re.compile(
|
||||
r'(https?://\S+|www\.\S+|\S+\.(?:com|ir|org|net|me)\S*|@\w+'
|
||||
r'|(?:[0-9۰-۹٠-٩][\s\-.]?){5,})',
|
||||
r'|(?:[0-9۰-۹٠-٩][\s\-._/\\|,،]*){5,})',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# «ig: name»، «insta name»، «آیدی name» و مانندش: کلیدواژه و هر چه بعدش آمده حذف میشود.
|
||||
_HANDLE = re.compile(
|
||||
r'(?:\b(?:ig|insta(?:gram)?|tele(?:gram)?|tg|id|whats ?app|rubika|eitaa|bale)\b'
|
||||
r'|اینستا\S*|(?:تلگرام|تل|آیدی|ایدی|واتساپ|روبیکا|ایتا)(?!\w)|بله\s*:)\s*[::]?.*',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def clean_display_name(value: str) -> str:
|
||||
return ' '.join(_CONTACT.sub(' ', value or '').split())[:30]
|
||||
value = _HANDLE.sub(' ', value or '')
|
||||
return ' '.join(_CONTACT.sub(' ', value).split())[:30]
|
||||
|
||||
|
||||
class IranMobileField(serializers.CharField):
|
||||
|
||||
@@ -134,8 +134,10 @@ def _kavenegar_post(*, phone: str, method: str, payload: dict) -> None:
|
||||
'phone_suffix=%s method=%s', _phone_suffix(phone), method,
|
||||
)
|
||||
raise SMSDeliveryUncertain('پاسخِ سرویسِ پیامک دیر رسید.')
|
||||
except requests.RequestException:
|
||||
logger.exception('SMS request to provider failed: phone_suffix=%s', _phone_suffix(phone))
|
||||
except requests.RequestException as exc:
|
||||
# فقط نوعِ خطا: متنِ خطای requests آدرسِ کامل را دارد و کلیدِ API بخشی از همان آدرس است.
|
||||
logger.error('SMS request to provider failed: phone_suffix=%s method=%s error=%s',
|
||||
_phone_suffix(phone), method, type(exc).__name__)
|
||||
raise SMSDeliveryError('ارسال پیامک با خطا مواجه شد.')
|
||||
|
||||
if response.status_code != 200:
|
||||
|
||||
179
accounts/tests_security_review.py
Normal file
179
accounts/tests_security_review.py
Normal file
@@ -0,0 +1,179 @@
|
||||
"""
|
||||
حفاظهای بازبینیِ امنیتیِ پیش از تحویل (هویت و حساب). هر تست رفتاری را نگه میدارد که پیشتر غلط بود.
|
||||
"""
|
||||
import logging
|
||||
import socket
|
||||
from unittest import mock
|
||||
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.models import Group
|
||||
from django.core.cache import cache
|
||||
from django.core.management import call_command
|
||||
from django.test import TestCase, override_settings
|
||||
from django.utils import timezone
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from accounts.authentication import issue_access_token
|
||||
from accounts.models import OTPCode, User
|
||||
from config.log_formatters import JSONLogFormatter
|
||||
|
||||
|
||||
def _bearer(token):
|
||||
c = APIClient()
|
||||
c.credentials(HTTP_AUTHORIZATION=f'Bearer {token}')
|
||||
return c
|
||||
|
||||
|
||||
def _otp_row(phone, code='482913'):
|
||||
OTPCode.objects.create(phone=phone, code=code, expires_at=timezone.now() + timezone.timedelta(minutes=5))
|
||||
return code
|
||||
|
||||
|
||||
def _guest(device):
|
||||
return APIClient().post('/api/auth/session', {'type': 'guest', 'deviceId': device}, format='json').json()
|
||||
|
||||
|
||||
class GuestUpgradeRevokesEarlierSessionsTests(TestCase):
|
||||
"""کسی که پیش از ثبتِ شماره با شناسهی دستگاه نشست گرفته بود، بعد از آن به حسابِ شمارهدار نمیرسید."""
|
||||
|
||||
def setUp(self):
|
||||
cache.clear()
|
||||
|
||||
def test_pre_upgrade_tokens_stop_working(self):
|
||||
attacker = _guest('shared-device')
|
||||
victim = _guest('shared-device')
|
||||
login = _bearer(victim['token']).post('/api/auth/login',
|
||||
{'phone': '09121230001', 'otp': _otp_row('09121230001')},
|
||||
format='json')
|
||||
self.assertEqual(login.status_code, 200)
|
||||
self.assertEqual(login.json()['userId'], victim['userId'])
|
||||
|
||||
refresh = APIClient().post('/api/auth/refresh', {'refreshToken': attacker['refreshToken']}, format='json')
|
||||
self.assertEqual(refresh.status_code, 401)
|
||||
# توکنِ دسترسیِ پیش از ثبت هم (اگر همان ثانیه صادر نشده باشد) رد میشود؛ نشستِ تازه کار میکند.
|
||||
self.assertEqual(_bearer(login.json()['token']).get('/api/user/profile').status_code, 200)
|
||||
fresh = APIClient().post('/api/auth/refresh', {'refreshToken': login.json()['refreshToken']}, format='json')
|
||||
self.assertEqual(fresh.status_code, 200)
|
||||
|
||||
|
||||
class AccountDeletionKeepsLimitsTests(TestCase):
|
||||
def setUp(self):
|
||||
cache.clear()
|
||||
from discount.models import DiscountCode
|
||||
from subscription.models import Plan
|
||||
self.plan = Plan.objects.create(title='pack20', price=45000, kind='pack',
|
||||
question_credits=20, hearts_days=7, duration_days=0)
|
||||
DiscountCode.objects.create(code='FREE1', type='percent', value=100, max_uses=1)
|
||||
|
||||
def _guest_redeems(self, device):
|
||||
s = _guest(device)
|
||||
c = _bearer(s['token'])
|
||||
p = c.post('/api/subscription/purchase', {'planId': str(self.plan.id)}, format='json')
|
||||
return c, c.post('/api/discount/redeem', {'code': 'FREE1', 'orderId': p.json()['orderId']}, format='json')
|
||||
|
||||
def test_single_use_code_stays_used_after_delete(self):
|
||||
c1, first = self._guest_redeems('g-1')
|
||||
self.assertEqual(first.status_code, 200)
|
||||
self.assertEqual(c1.post('/api/user/delete', {'confirm': 'حذف'}, format='json').status_code, 200)
|
||||
_, again = self._guest_redeems('g-2')
|
||||
self.assertEqual(again.status_code, 400)
|
||||
|
||||
@override_settings(DEBUG=False, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
|
||||
def test_per_phone_sms_cap_survives_delete(self):
|
||||
phone = '09121230009'
|
||||
user = User.objects.create(phone=phone, is_guest=False)
|
||||
for i in range(10):
|
||||
OTPCode.objects.create(phone=phone, code=f'{i:06d}', is_used=True,
|
||||
expires_at=timezone.now() - timezone.timedelta(minutes=i + 1))
|
||||
with mock.patch('accounts.views.send_otp_sms'):
|
||||
OTPCode.objects.create(phone=phone, code='777777',
|
||||
expires_at=timezone.now() + timezone.timedelta(minutes=5))
|
||||
d = _bearer(issue_access_token(user.id)).post('/api/user/delete', {'otp': '777777'}, format='json')
|
||||
self.assertEqual(d.status_code, 200)
|
||||
cache.clear()
|
||||
r = APIClient().post('/api/auth/otp/request', {'phone': phone}, format='json')
|
||||
self.assertEqual(r.status_code, 429)
|
||||
|
||||
def test_public_demo_guest_cannot_be_deleted(self):
|
||||
demo = User.objects.create(device_id='frontend-test-device', is_guest=True, name='demo')
|
||||
s = _guest('frontend-test-device')
|
||||
self.assertEqual(int(s['userId']), demo.id)
|
||||
r = _bearer(s['token']).post('/api/user/delete', {'confirm': 'حذف'}, format='json')
|
||||
self.assertEqual(r.status_code, 403)
|
||||
self.assertTrue(User.objects.filter(pk=demo.id).exists())
|
||||
|
||||
|
||||
class DisplayNameContactFilterTests(TestCase):
|
||||
def test_contacts_are_removed_and_names_kept(self):
|
||||
from accounts.serializers import clean_display_name as clean
|
||||
|
||||
for sample in ('0912/123/4567', '۰۹۱۲/۱۲۳/۴۵۶۷', '0912 _123_ 4567', 'ig: ali_kid_2014', 'آیدی: kid2014'):
|
||||
self.assertEqual(clean(sample), '', sample)
|
||||
for name in ('علی رضایی', 'آیدین', 'Sara 2014', 'ایتالیا', 'تلما'):
|
||||
self.assertEqual(clean(name), name)
|
||||
|
||||
|
||||
class _Capture(logging.Handler):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.lines = []
|
||||
self.setFormatter(JSONLogFormatter())
|
||||
|
||||
def emit(self, record):
|
||||
self.lines.append(self.format(record))
|
||||
|
||||
|
||||
FAKE_KEY = '4E6F74415265616C4B6176656E656761724B6579414243444546'
|
||||
|
||||
|
||||
@override_settings(KAVENEGAR_API_KEY=FAKE_KEY, KAVENEGAR_OTP_TEMPLATE='SMSVerify')
|
||||
class KavenegarKeyNotLoggedTests(TestCase):
|
||||
"""کلیدِ کاوهنگار بخشی از آدرس است و متنِ خطای requests آدرس را دارد."""
|
||||
|
||||
def test_connection_error_log_has_no_key(self):
|
||||
from accounts import services
|
||||
|
||||
cap = _Capture()
|
||||
log = logging.getLogger('accounts.services')
|
||||
log.addHandler(cap)
|
||||
|
||||
def no_dns(*args, **kwargs):
|
||||
raise socket.gaierror(socket.EAI_NONAME, 'network disabled in tests')
|
||||
|
||||
try:
|
||||
with mock.patch('socket.getaddrinfo', no_dns), self.assertRaises(services.SMSDeliveryError):
|
||||
services.send_verify_lookup(phone='09120000000', template='SMSVerify', token='123456')
|
||||
finally:
|
||||
log.removeHandler(cap)
|
||||
out = '\n'.join(cap.lines)
|
||||
self.assertIn('SMS request to provider failed', out)
|
||||
self.assertNotIn(FAKE_KEY, out)
|
||||
|
||||
|
||||
class SupervisorDoesNotSeeDeviceIdsTests(TestCase):
|
||||
"""شناسهی دستگاه برای مهمان رمزِ ورود است؛ کارمندِ فقطخواندنی آن را نمیبیند."""
|
||||
|
||||
def setUp(self):
|
||||
call_command('setup_supervisor_group', verbosity=0)
|
||||
self.guest = User.objects.create(device_id='secret-device-123', is_guest=True)
|
||||
from accounts.models import Session
|
||||
Session.objects.create(user=self.guest, refresh_token='h' * 64, device_id='secret-device-123',
|
||||
expires_at=timezone.now() + timezone.timedelta(days=1))
|
||||
staff = get_user_model().objects.create_user('sup', 'sup@example.com', 'pw-for-tests-only', is_staff=True)
|
||||
staff.groups.add(Group.objects.get(name='Supervisor'))
|
||||
# Supervisor نشستها را نمیبیند؛ اینجا اجازهی دیدن داده میشود تا ستونِ آن صفحه هم سنجیده شود.
|
||||
from django.contrib.auth.models import Permission
|
||||
staff.user_permissions.add(Permission.objects.get(codename='view_session', content_type__app_label='accounts'))
|
||||
self.client.force_login(staff)
|
||||
|
||||
def test_hidden_in_list_search_and_form(self):
|
||||
listing = self.client.get('/admin/accounts/user/')
|
||||
self.assertEqual(listing.status_code, 200)
|
||||
self.assertNotContains(listing, 'secret-device-123')
|
||||
# متنِ جستجو در خودِ صفحه تکرار میشود؛ نتیجه نباید همان مهمان باشد.
|
||||
search = self.client.get('/admin/accounts/user/', {'q': 'secret-device-123'})
|
||||
self.assertNotContains(search, f'/admin/accounts/user/{self.guest.pk}/change/')
|
||||
form = self.client.get(f'/admin/accounts/user/{self.guest.pk}/change/')
|
||||
self.assertNotContains(form, 'secret-device-123')
|
||||
sessions = self.client.get('/admin/accounts/session/')
|
||||
self.assertNotContains(sessions, 'secret-device-123')
|
||||
@@ -362,7 +362,11 @@ class LoginView(APIView):
|
||||
# جزیرهها و گفتگوها همه با همین user_id ذخیره شدهاند. پیشتر یک
|
||||
# کاربرِ تازه و خالی ساخته میشد و پیشرفتِ مهمان جا میماند.
|
||||
guest.phone = phone
|
||||
guest.save(update_fields=['phone'])
|
||||
# هر نشستی که پیش از این با شناسهی دستگاه گرفته شده بود (گوشیِ مشترک، کسی که
|
||||
# شناسه را دیده) نباید به حسابِ شمارهدار برسد؛ فقط نشستِ همین ورود میماند.
|
||||
guest.tokens_valid_after = timezone.now()
|
||||
guest.save(update_fields=['phone', 'tokens_valid_after'])
|
||||
Session.objects.filter(user=guest, revoked=False).update(revoked=True)
|
||||
user, created = guest, True
|
||||
elif user is None:
|
||||
user, created = User.objects.get_or_create(
|
||||
@@ -426,7 +430,10 @@ DELETE_CONFIRM_WORD = 'حذف'
|
||||
# سوابقِ پرداخت سندِ مالیاند و با حذفِ حساب نمیروند (تراکنشها با سفارش میمانند).
|
||||
# Conversion هم میماند: ردیفِ «کاربرِ معرفیشده» سابقهی پورسانتِ *معرف* است، نه دادهی این
|
||||
# کاربر؛ تا ۲۰۲۶-۱۰-۰۳ با حذفِ حسابِ معرفیشده پورسانتِ معرف هم پاک میشد.
|
||||
KEPT_ON_ACCOUNT_DELETE = ('payment.Order', 'statuslog.StatusChangeLog', 'affiliate.Conversion')
|
||||
# Redemption و UserOffer هم میمانند: سقفِ مصرفِ کدِ تخفیف ردیفهای Redemption را میشمارد، پس پاک
|
||||
# شدنشان یک کدِ یکبارمصرف را با «بساز، بخر، حذف کن» دوباره آزاد میکرد.
|
||||
KEPT_ON_ACCOUNT_DELETE = ('payment.Order', 'statuslog.StatusChangeLog', 'affiliate.Conversion',
|
||||
'discount.Redemption', 'discount.UserOffer')
|
||||
|
||||
|
||||
class DeleteAccountView(APIView):
|
||||
@@ -439,7 +446,7 @@ class DeleteAccountView(APIView):
|
||||
بتواند حساب را برای همیشه پاک کند.
|
||||
- مهمان شماره ندارد؛ باید کلمهی «حذف» را تایپ کرده باشد. حسابش فقط به همین دستگاه وصل است.
|
||||
- داده در همهی اپها پاک میشود (delete_users)، جز سفارشها و تراکنشهایشان که سندِ مالیاند.
|
||||
- حسابهای دمو (TEST_LOGIN_PHONES) حذف نمیشوند؛ مشترکاند و برای ارائه پُرشدهاند.
|
||||
- حسابهای دمو (TEST_LOGIN_PHONES و مهمانهای DEMO_GUEST_DEVICE_IDS) حذف نمیشوند؛ مشترکاند و برای ارائه پُرشدهاند.
|
||||
- بعد از حذف، توکنهای همین کاربر کار نمیکنند چون کاربری با آن شناسه نمانده.
|
||||
"""
|
||||
|
||||
@@ -460,7 +467,8 @@ class DeleteAccountView(APIView):
|
||||
v = serializer.validated_data
|
||||
user = request.user
|
||||
|
||||
if user.phone and user.phone in settings.TEST_LOGIN_PHONES:
|
||||
if (user.phone and user.phone in settings.TEST_LOGIN_PHONES) or (
|
||||
user.is_guest and user.device_id in settings.DEMO_GUEST_DEVICE_IDS):
|
||||
return Response({'detail': 'حسابِ نمایشی را نمیشود حذف کرد.'}, status=status.HTTP_403_FORBIDDEN)
|
||||
if user.phone:
|
||||
if not v.get('otp'):
|
||||
@@ -474,7 +482,8 @@ class DeleteAccountView(APIView):
|
||||
status=status.HTTP_400_BAD_REQUEST)
|
||||
|
||||
logger.info('Account deleted by its owner: user_id=%s guest=%s', user.id, user.is_guest)
|
||||
delete_users([user.id], keep=KEPT_ON_ACCOUNT_DELETE)
|
||||
# کدهای پیامکیِ شماره میمانند: سقفِ روزانهی پیامکِ هر شماره از همین ردیفها شمرده میشود.
|
||||
delete_users([user.id], keep=KEPT_ON_ACCOUNT_DELETE, delete_otp_codes=False)
|
||||
return Response(DeleteAccountResponseSerializer({'success': True}).data)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user