Skip to content

feat: add superadmin role and permission-based admin system#13

Merged
kemalcalak merged 22 commits into
mainfrom
admin-rbac
Jun 9, 2026
Merged

feat: add superadmin role and permission-based admin system#13
kemalcalak merged 22 commits into
mainfrom
admin-rbac

Conversation

@kemalcalak

Copy link
Copy Markdown
Owner

No description provided.

@kemalcalak kemalcalak self-assigned this Jun 9, 2026
@kemalcalak

Copy link
Copy Markdown
Owner Author

/review — 6 bulgu

1. İki ayrı commit ile çift-root superadmin riski

Dosya: app/services/admin/admin_service.py#L457-L458
Kategori: bug · Önem: blocker

confirm_root_transfer_service root bayrağını iki ayrı update_user çağrısıyla değiştiriyor; update_user her çağrıda kendi session.commit()'ini yapar. İlk commit (target → is_root_superadmin=True) başarılı olup süreç çökerse ikinci commit hiç çalışmaz ve sistemde kalıcı olarak iki root superadmin kalır. root_superadmin_exists() yalnızca en az bir root'un varlığını kontrol eder, duplikasyonu düzeltmez; DB'de UNIQUE kısıt da yoktur.

await update_user(session, target, {"is_root_superadmin": True}) / await update_user(session, current_user, {"is_root_superadmin": False})

Öneri: İki flag değişikliğini tek session.execute(update(User).where(User.id.in_([target.id, current_user.id])).values(...)) + tek commit ile atomik hale getir.


2. OTP doğrulama endpoint'inde rate-limit dekoratörü eksik

Dosya: app/api/routes/admin/admins.py#L80-L98
Kategori: bug · Önem: blocker

POST /admin/admins/transfer-root/confirm 6 haneli OTP (10^6 olasılık) kabul ediyor ancak rate-limit dekoratörü yok. 180 saniyelik TTL penceresi içinde kısıtlama olmadan brute-force denenebilir ve root yetkisi ele geçirilebilir. Aynı eksiklik PR #12'de (close_ticket) "past-comment" olarak işaretlenip @rate_limit_authenticated ile kapatılmıştı; çok daha kritik bu endpoint'te konvansiyon yeniden ihlal ediliyor. request: Request zaten imzada mevcut.

REVIEW.md §5.8 — "Hassas endpoint (/auth/login, /auth/forgot-password, ...) decorator'sız."

Öneri: @audit_unexpected_failure'ın üstüne @rate_limit_strict(...) ekle.


3. OTP başlatma endpoint'inde rate-limit dekoratörü eksik

Dosya: app/api/routes/admin/admins.py#L57-L77
Kategori: bug · Önem: major

POST /admin/admins/transfer-root de rate-limit dekoratörü taşımıyor. Sınırsız çağrı Redis'teki bekleyen OTP'yi her seferinde yeni kodla ezer ve root'un e-posta adresine spam OTP gönderir. request: Request imzada mevcut.

REVIEW.md §3.7 — "rate_limit_strict (3/min — şifre reset/hesap silme gibi)."

Öneri: @rate_limit_strict(...) (veya @rate_limit_authenticated(...)) ekle.


4. delete_admin_service'te son aktif admin koruması eksik

Dosya: app/services/admin/admin_service.py#L313-L351
Kategori: review-md-compliance · Önem: major

delete_admin_service, son aktif plain admin silinirken is_last_active_admin kontrolü yapmıyor. Aynı invariant user_service.py'daki delete_user_admin_service içinde uygulanıyor (ADMIN_CANNOT_DELETE_LAST_ADMIN sabiti bunu tescil ediyor). Bu, /admin/users ve /admin/admins yüzeyleri arasında tutarsızlık yaratıyor.

REVIEW.md §3.18 — "Rol/katman invariant'ları serviste ... son superadmin korunur (is_last_active_superadmin)."

Öneri: target plain admin ise silmeden önce if await is_last_active_admin(session, target.id): raise HTTPException(400, ErrorMessages.ADMIN_CANNOT_DELETE_LAST_ADMIN) ekle.


5. require_permission / require_permissions iç fonksiyonları docstring'siz

Dosya: app/api/deps.py#L201-L248
Kategori: code-comment · Önem: major

require_permission ve require_permissions factory'lerinin döndürdüğü iç async def _require(...) fonksiyonlarının ikisi de docstring içermiyor. CLAUDE.md hard rule tüm fonksiyonlar için minimum bir satır İngilizce docstring zorunlu kılıyor (closure'lar dahil).

CLAUDE.md — "All functions must have a docstring — minimum one line, always in English"

Öneri: Her _require'a tek satırlık İngilizce docstring ekle (örn. """Enforce the captured permission for the resolved admin.""").


6. _drop_null_permissions serializer metodu docstring'siz

Dosya: app/schemas/user.py#L122-L128
Kategori: code-comment · Önem: major

UserMe sınıfına eklenen @model_serializer metodu _drop_null_permissions docstring içermiyor; CLAUDE.md hard rule ihlali.

CLAUDE.md — "All functions must have a docstring — minimum one line, always in English"

Öneri: Metoda bir satırlık docstring ekle (örn. """Omit the permissions key entirely when it is None.""").


🤖 Generated with Claude Code

@kemalcalak

Copy link
Copy Markdown
Owner Author

/review bulguları çözüldü ✅

6dd713a ile review'daki 6 bulgunun tamamı giderildi:

# Bulgu Önem Çözüm
1 İki ayrı commit ile çift-root superadmin riski blocker transfer_root_superadmin repo fonksiyonu — iki UPDATE tek transaction + tek commit ile atomik hale getirildi
2 OTP doğrulama endpoint'inde rate-limit yok blocker POST /transfer-root/confirm@rate_limit_strict("5/minute")
3 OTP başlatma endpoint'inde rate-limit yok major POST /transfer-root@rate_limit_strict("3/minute")
4 delete_admin_service son aktif admin koruması eksik major is_last_active_admin guard + ADMIN_CANNOT_DELETE_LAST_ADMIN eklendi
5 require_permission/require_permissions iç fonksiyonları docstring'siz major Her iki _require closure'a İngilizce docstring eklendi
6 _drop_null_permissions serializer metodu docstring'siz major Docstring eklendi

Ek sertleştirme: OTP doğrulaması artık _MAX_ATTEMPTS=3 deneme sayacıyla brute-force'a karşı korunuyor (TTL korunarak), verify_root_transfer_otp ile.

Doğrulama:

  • ✅ 36 test geçti (test_admins + test_root_transfer + test_rbac)
  • ✅ Ruff temiz
  • ✅ Dekoratör sırası repo konvansiyonuyla tutarlı (@router.post@rate_limit_*@audit_unexpected_failure)

🤖 Generated with Claude Code

@kemalcalak kemalcalak merged commit 9dea0ec into main Jun 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant