Ревью кода (#8) #34
@@ -10,6 +10,7 @@ from sqlmodel import Session
|
||||
|
||||
from app.auth.provider import ExternalIdentity
|
||||
from app.core import security
|
||||
from app.core.security import client_ip
|
||||
from app.core.errors import ForbiddenError
|
||||
from app.models import User
|
||||
from app.services import audit_service, user_service
|
||||
@@ -29,7 +30,7 @@ def establish_session(
|
||||
entity_type="user",
|
||||
entity_id=user.id,
|
||||
payload={"provider": provider},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
|
||||
@@ -6,7 +6,7 @@ from datetime import datetime, timedelta, timezone
|
||||
|
||||
import bcrypt
|
||||
import jwt
|
||||
from fastapi import Response
|
||||
from fastapi import Request, Response
|
||||
|
||||
from app.core.config import settings
|
||||
|
||||
@@ -118,3 +118,11 @@ def clear_user_session(response: Response) -> None:
|
||||
|
||||
def clear_admin_session(response: Response) -> None:
|
||||
response.delete_cookie(ADMIN_COOKIE, path=_ADMIN_PATH, domain=settings.cookie_domain_value)
|
||||
|
||||
|
||||
def client_ip(request: Request) -> str | None:
|
||||
"""IP клиента для журнала аудита.
|
||||
|
||||
Одна точка на всё приложение: за VPS-привратником адрес придётся брать из
|
||||
X-Forwarded-For, и менять это в двух десятках роутеров — не вариант."""
|
||||
return request.client.host if request.client else None
|
||||
|
||||
@@ -414,9 +414,3 @@ class AuditLog(SQLModel, table=True):
|
||||
created_at: datetime = Field(default_factory=utcnow, nullable=False)
|
||||
|
||||
|
||||
# Заготовка под будущие вложения (НЕ в v1-миграции, добавится отдельно):
|
||||
# class Attachment(SQLModel, table=True):
|
||||
# id, match_id (FK CASCADE), participant_id (FK NULL SET NULL),
|
||||
# uploaded_by (FK), kind ('photo'|'video'), storage_path, mime_type,
|
||||
# size_bytes, created_at
|
||||
# Файлы — на томе /data/uploads; в БД только метаданные и относительный путь.
|
||||
|
||||
@@ -7,12 +7,14 @@ from sqlmodel import Session
|
||||
|
||||
from app.auth.deps import get_current_admin
|
||||
from app.core import security
|
||||
from app.core.errors import NotFoundError, ValidationError
|
||||
from app.core.security import client_ip
|
||||
from app.core.errors import NotFoundError
|
||||
from app.core.timeutil import iso_utc
|
||||
from app.db.session import get_session
|
||||
from app.models import User
|
||||
from app.routers.matches import attachment_read, build_match_read
|
||||
from app.schemas import api as s
|
||||
from app.services.match_service import ParticipantInput
|
||||
from app.services import (
|
||||
achievement_service,
|
||||
admin_service,
|
||||
@@ -25,8 +27,6 @@ from app.services import (
|
||||
)
|
||||
|
||||
_ACHIEVEMENT_ICON_MAX_BYTES = 2 * 1024 * 1024 # 2 МБ
|
||||
_ATTACHMENT_MAX_BYTES = 10 * 1024 * 1024 # 10 МБ
|
||||
from app.services.match_service import ParticipantInput
|
||||
|
||||
router = APIRouter(prefix="/admin", tags=["admin"])
|
||||
|
||||
@@ -48,7 +48,7 @@ def admin_login(
|
||||
action="login",
|
||||
entity_type="admin",
|
||||
entity_id=admin.id,
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -104,7 +104,7 @@ def update_user(
|
||||
entity_type="user",
|
||||
entity_id=user_id,
|
||||
payload=body.model_dump(exclude_none=True),
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return s.AdminUserRead(
|
||||
@@ -148,7 +148,7 @@ def delete_group(
|
||||
admin_service.delete_group(session, group_id)
|
||||
audit_service.record(
|
||||
session, actor_id=admin.id, action="delete", entity_type="group", entity_id=group_id,
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return s.OkResponse()
|
||||
@@ -239,7 +239,7 @@ def update_match(
|
||||
action="update",
|
||||
entity_type="match",
|
||||
entity_id=match.id,
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
notify.match_changed(session, match)
|
||||
@@ -277,7 +277,7 @@ def rename_faction(
|
||||
entity_type="faction",
|
||||
entity_id=faction_id,
|
||||
payload={"name_ru": f.name_ru},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return s.FactionRead(
|
||||
@@ -296,7 +296,7 @@ def delete_match(
|
||||
admin_service.delete_match(session, match_id)
|
||||
audit_service.record(
|
||||
session, actor_id=admin.id, action="delete", entity_type="match", entity_id=match_id,
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
notify.match_removed(session, match_id, group_id)
|
||||
@@ -326,12 +326,9 @@ def admin_add_attachment(
|
||||
admin: User = Depends(get_current_admin),
|
||||
) -> s.AttachmentRead:
|
||||
match = match_service.get_match(session, match_id)
|
||||
content = file.file.read(_ATTACHMENT_MAX_BYTES + 1)
|
||||
if len(content) > _ATTACHMENT_MAX_BYTES:
|
||||
raise ValidationError("Файл слишком большой (макс. 10 МБ).")
|
||||
ext = user_service.sniff_image_ext(content)
|
||||
if ext is None:
|
||||
raise ValidationError("Поддерживаются только изображения PNG, JPEG или WebP.")
|
||||
content, ext = user_service.read_capped_image(
|
||||
file, attachment_service.MAX_ATTACHMENT_BYTES, "Файл слишком большой (макс. 10 МБ)."
|
||||
)
|
||||
att = attachment_service.add_photo(
|
||||
session, match, admin, content, ext, user_service.avatar_media_type(ext)
|
||||
)
|
||||
@@ -389,7 +386,7 @@ def create_achievement(
|
||||
audit_service.record(
|
||||
session, actor_id=admin.id, action="create", entity_type="achievement",
|
||||
payload={"slug": ach["slug"], "name": ach["name"]},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return ach
|
||||
@@ -408,7 +405,7 @@ def update_achievement(
|
||||
)
|
||||
audit_service.record(
|
||||
session, actor_id=admin.id, action="update", entity_type="achievement",
|
||||
payload={"slug": slug}, ip=request.client.host if request.client else None,
|
||||
payload={"slug": slug}, ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return ach
|
||||
@@ -420,10 +417,9 @@ def upload_achievement_icon(
|
||||
file: UploadFile = File(...),
|
||||
_admin: User = Depends(get_current_admin),
|
||||
) -> dict:
|
||||
content = file.file.read(_ACHIEVEMENT_ICON_MAX_BYTES + 1)
|
||||
if len(content) > _ACHIEVEMENT_ICON_MAX_BYTES:
|
||||
raise ValidationError("Файл слишком большой (макс. 2 МБ).")
|
||||
ext = achievement_service.validate_icon(content)
|
||||
content, ext = user_service.read_capped_image(
|
||||
file, _ACHIEVEMENT_ICON_MAX_BYTES, "Файл слишком большой (макс. 2 МБ)."
|
||||
)
|
||||
return achievement_service.set_icon(slug, content, ext)
|
||||
|
||||
|
||||
@@ -437,7 +433,7 @@ def delete_achievement(
|
||||
achievement_service.delete(slug)
|
||||
audit_service.record(
|
||||
session, actor_id=admin.id, action="delete", entity_type="achievement",
|
||||
payload={"slug": slug}, ip=request.client.host if request.client else None,
|
||||
payload={"slug": slug}, ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return s.OkResponse()
|
||||
|
||||
@@ -18,6 +18,7 @@ from fastapi import APIRouter, Depends, Request
|
||||
from sqlmodel import Session, select
|
||||
|
||||
from app.auth.deps import get_current_admin
|
||||
from app.core.security import client_ip
|
||||
from app.core.errors import NotFoundError, ValidationError
|
||||
from app.db.session import get_session
|
||||
from app.models import Group, Match, MatchParticipant, User
|
||||
@@ -63,7 +64,7 @@ def delete_user_hard(
|
||||
entity_type="user",
|
||||
entity_id=user_id,
|
||||
payload={"hard": True, "nickname": nickname},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return s.OkResponse()
|
||||
|
||||
@@ -5,6 +5,7 @@ from fastapi import APIRouter, Depends, Query, Request
|
||||
from sqlmodel import Session
|
||||
|
||||
from app.auth.deps import get_current_user
|
||||
from app.core.security import client_ip
|
||||
from app.core.timeutil import iso_utc
|
||||
from app.db.session import get_session
|
||||
from app.models import User
|
||||
@@ -61,7 +62,7 @@ def create_group(
|
||||
entity_type="group",
|
||||
entity_id=group.id,
|
||||
payload={"name": group.name},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -157,7 +158,7 @@ def invite_member(
|
||||
entity_type="group_invitation",
|
||||
entity_id=group_id,
|
||||
payload={"invited_user_id": invited.id, "nickname": invited.nickname},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
|
||||
@@ -6,7 +6,8 @@ from fastapi.responses import FileResponse
|
||||
from sqlmodel import Session
|
||||
|
||||
from app.auth.deps import get_current_user
|
||||
from app.core.errors import ConflictError, NoGroupError, NotFoundError, ValidationError
|
||||
from app.core.security import client_ip
|
||||
from app.core.errors import ConflictError, NoGroupError, NotFoundError
|
||||
from app.core.timeutil import iso_utc
|
||||
from app.db.session import get_session
|
||||
from app.models import Match, MatchAttachment, User
|
||||
@@ -24,7 +25,6 @@ from app.services.match_service import FinishInput, ParticipantInput, RosterInpu
|
||||
|
||||
router = APIRouter(prefix="/matches", tags=["matches"])
|
||||
|
||||
_ATTACHMENT_MAX_BYTES = 10 * 1024 * 1024 # 10 МБ
|
||||
|
||||
|
||||
def attachment_read(att: MatchAttachment, base: str) -> s.AttachmentRead:
|
||||
@@ -119,7 +119,7 @@ def start_match(
|
||||
entity_type="match",
|
||||
entity_id=match.id,
|
||||
payload={"group_id": match.group_id, "player_count": match.player_count, "status": "in_progress"},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -165,7 +165,7 @@ def finish_match(
|
||||
entity_type="match",
|
||||
entity_id=match.id,
|
||||
payload={"event": "finish", "win_reason": match.win_reason, "duration_minutes": match.duration_minutes},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -225,7 +225,7 @@ def update_match(
|
||||
action="update",
|
||||
entity_type="match",
|
||||
entity_id=match.id,
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -250,12 +250,9 @@ def add_attachment(
|
||||
) -> s.AttachmentRead:
|
||||
match = match_service.get_match(session, match_id)
|
||||
_assert_can_attach(session, match, user)
|
||||
content = file.file.read(_ATTACHMENT_MAX_BYTES + 1)
|
||||
if len(content) > _ATTACHMENT_MAX_BYTES:
|
||||
raise ValidationError("Файл слишком большой (макс. 10 МБ).")
|
||||
ext = user_service.sniff_image_ext(content)
|
||||
if ext is None:
|
||||
raise ValidationError("Поддерживаются только изображения PNG, JPEG или WebP.")
|
||||
content, ext = user_service.read_capped_image(
|
||||
file, attachment_service.MAX_ATTACHMENT_BYTES, "Файл слишком большой (макс. 10 МБ)."
|
||||
)
|
||||
att = attachment_service.add_photo(
|
||||
session, match, user, content, ext, user_service.avatar_media_type(ext)
|
||||
)
|
||||
@@ -315,7 +312,7 @@ def delete_match(
|
||||
entity_type="match",
|
||||
entity_id=match_id_val,
|
||||
payload={"group_id": group_id_val},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
|
||||
@@ -6,7 +6,8 @@ from fastapi.responses import FileResponse
|
||||
from sqlmodel import Session
|
||||
|
||||
from app.auth.deps import get_current_user
|
||||
from app.core.errors import NotFoundError, ValidationError
|
||||
from app.core.security import client_ip
|
||||
from app.core.errors import NotFoundError
|
||||
from app.db.session import get_session
|
||||
from app.models import User
|
||||
from app.schemas import api as s
|
||||
@@ -61,7 +62,7 @@ def update_me(
|
||||
entity_type="user",
|
||||
entity_id=user.id,
|
||||
payload={"nickname": user.nickname},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
user_agent=request.headers.get("user-agent"),
|
||||
)
|
||||
session.commit()
|
||||
@@ -108,7 +109,7 @@ def update_my_profile(
|
||||
entity_type="user",
|
||||
entity_id=user.id,
|
||||
payload={key: True for key in changed},
|
||||
ip=request.client.host if request.client else None,
|
||||
ip=client_ip(request),
|
||||
)
|
||||
session.commit()
|
||||
return build_me(session, user)
|
||||
@@ -120,12 +121,9 @@ def upload_my_avatar(
|
||||
session: Session = Depends(get_session),
|
||||
user: User = Depends(get_current_user),
|
||||
) -> s.MeRead:
|
||||
content = file.file.read(_AVATAR_MAX_BYTES + 1)
|
||||
if len(content) > _AVATAR_MAX_BYTES:
|
||||
raise ValidationError("Файл слишком большой (макс. 2 МБ).")
|
||||
ext = user_service.sniff_image_ext(content)
|
||||
if ext is None:
|
||||
raise ValidationError("Поддерживаются только изображения PNG, JPEG или WebP.")
|
||||
content, ext = user_service.read_capped_image(
|
||||
file, _AVATAR_MAX_BYTES, "Файл слишком большой (макс. 2 МБ)."
|
||||
)
|
||||
user_service.set_avatar(session, user, content, ext)
|
||||
return build_me(session, user)
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ from app.core.errors import ConflictError, NotFoundError
|
||||
from app.models import Match, MatchAttachment, User
|
||||
|
||||
MAX_ATTACHMENTS = 10
|
||||
MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024 # 10 МБ
|
||||
_SUBDIR = "matches"
|
||||
|
||||
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Пользователи: создание из внешней личности, ник, активная группа, профиль."""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
@@ -12,7 +11,7 @@ from app.auth.provider import ExternalIdentity
|
||||
from app.core.config import settings
|
||||
from app.core.errors import NicknameTakenError, NotFoundError, ValidationError
|
||||
from app.core.timeutil import utcnow
|
||||
from app.models import AuthIdentity, Faction, GroupMember, User
|
||||
from app.models import AuthIdentity, Faction, User
|
||||
|
||||
_NICK_RE = re.compile(r"^[\w .\-]{2,64}$", re.UNICODE)
|
||||
_BIO_MAX = 500
|
||||
@@ -135,7 +134,7 @@ def register_from_identity(
|
||||
|
||||
def update_nickname(session: Session, user: User, new_nickname: str) -> User:
|
||||
new_nickname = (new_nickname or "").strip()
|
||||
if not _NICK_RE.match(new_nickname):
|
||||
if not nickname_format_ok(new_nickname):
|
||||
raise ValidationError("Ник: 2–64 символа, буквы/цифры/пробел/.-_")
|
||||
if not nickname_available(session, new_nickname, exclude_user_id=user.id):
|
||||
raise NicknameTakenError()
|
||||
@@ -148,12 +147,9 @@ def update_nickname(session: Session, user: User, new_nickname: str) -> User:
|
||||
|
||||
def set_active_group(session: Session, user: User, group_id: int | None) -> User:
|
||||
if group_id is not None:
|
||||
member = session.exec(
|
||||
select(GroupMember).where(
|
||||
GroupMember.group_id == group_id, GroupMember.user_id == user.id
|
||||
)
|
||||
).first()
|
||||
if member is None:
|
||||
from app.services import group_service # избегаем цикла импорта
|
||||
|
||||
if group_service.get_membership(session, group_id, user.id) is None:
|
||||
raise ValidationError("Нельзя сделать активной группу, в которой вы не состоите.")
|
||||
user.active_group_id = group_id
|
||||
session.add(user)
|
||||
@@ -225,6 +221,21 @@ def update_favorite_faction(session: Session, user: User, faction_id: int | None
|
||||
return user
|
||||
|
||||
|
||||
def read_capped_image(file, max_bytes: int, limit_message: str) -> tuple[bytes, str]:
|
||||
"""Прочитать загруженный файл с ограничением размера и убедиться, что это картинка.
|
||||
|
||||
Читаем на байт больше лимита: так превышение видно, не загружая файл целиком.
|
||||
Один хелпер на все загрузки (аватар, фото партии, иконка ачивки) — иначе
|
||||
правка лимита или списка форматов расходится по четырём роутерам."""
|
||||
content = file.file.read(max_bytes + 1)
|
||||
if len(content) > max_bytes:
|
||||
raise ValidationError(limit_message)
|
||||
ext = sniff_image_ext(content)
|
||||
if ext is None:
|
||||
raise ValidationError("Поддерживаются только изображения PNG, JPEG или WebP.")
|
||||
return content, ext
|
||||
|
||||
|
||||
def sniff_image_ext(content: bytes) -> str | None:
|
||||
"""Расширение по магическим байтам (PNG/JPEG/WebP), без Pillow. None — не картинка."""
|
||||
if content.startswith(b"\x89PNG\r\n\x1a\n"):
|
||||
|
||||
Reference in New Issue
Block a user