From 8697b9e30bebe98d75fac3b615172962c70b52ef Mon Sep 17 00:00:00 2001 From: Nikolay Tatarinov Date: Wed, 18 Feb 2026 09:24:51 +0300 Subject: [PATCH] Refactor duty authentication and event type handling - Introduced a new function `get_authenticated_username` to centralize Mini App authentication logic, improving code readability and maintainability. - Updated the duty fetching logic to map unknown event types to "duty" for consistent API responses. - Enhanced the `get_duties` function to include duties starting on the last day of the specified date range. - Improved session management in the database layer to ensure rollback on exceptions. - Added tests to validate the new authentication flow and event type handling. --- api/app.py | 81 ++++++++++++----------------- db/repository.py | 9 ++-- db/schemas.py | 5 +- db/session.py | 5 +- handlers/group_duty_pin.py | 3 +- tests/test_app.py | 28 ++++++++++ tests/test_repository_duty_range.py | 14 +++++ webapp/app.js | 5 +- 8 files changed, 94 insertions(+), 56 deletions(-) diff --git a/api/app.py b/api/app.py index dfbcdbc..ba1cbd3 100644 --- a/api/app.py +++ b/api/app.py @@ -46,7 +46,11 @@ def _fetch_duties_response(from_date: str, to_date: str) -> list[DutyWithUser]: start_at=duty.start_at, end_at=duty.end_at, full_name=full_name, - event_type=duty.event_type, + event_type=( + duty.event_type + if duty.event_type in ("duty", "unavailable", "vacation") + else "duty" + ), ) for duty, full_name in rows ] @@ -87,6 +91,33 @@ def _is_private_client(client_host: str | None) -> bool: return False +def get_authenticated_username( + request: Request, + x_telegram_init_data: str | None, +) -> str: + """Validate Mini App auth. Returns username (or "" when bypass allowed); raises HTTPException 403 otherwise.""" + init_data = (x_telegram_init_data or "").strip() + if not init_data: + client_host = request.client.host if request.client else None + if _is_private_client(client_host) or config.MINI_APP_SKIP_AUTH: + if config.MINI_APP_SKIP_AUTH: + log.warning("allowing without initData (MINI_APP_SKIP_AUTH is set)") + return "" + log.warning("no X-Telegram-Init-Data header (client=%s)", client_host) + raise HTTPException(status_code=403, detail="Откройте календарь из Telegram") + max_age = config.INIT_DATA_MAX_AGE_SECONDS or None + username, auth_reason = validate_init_data_with_reason( + init_data, config.BOT_TOKEN, max_age_seconds=max_age + ) + if username is None: + log.warning("initData validation failed: %s", auth_reason) + raise HTTPException(status_code=403, detail=_auth_error_detail(auth_reason)) + if not config.can_access_miniapp(username): + log.warning("username not in allowlist: %s", username) + raise HTTPException(status_code=403, detail="Доступ запрещён") + return username + + app = FastAPI(title="Duty Teller API") app.add_middleware( CORSMiddleware, @@ -110,54 +141,10 @@ def list_duties( request.client.host if request.client else "?", bool((x_telegram_init_data or "").strip()), ) - init_data = (x_telegram_init_data or "").strip() - if not init_data: - client_host = request.client.host if request.client else None - if _is_private_client(client_host) or config.MINI_APP_SKIP_AUTH: - if config.MINI_APP_SKIP_AUTH: - log.warning( - "duties: allowing without initData (MINI_APP_SKIP_AUTH is set)" - ) - return _fetch_duties_response(from_date, to_date) - log.warning("duties: no X-Telegram-Init-Data header (client=%s)", client_host) - raise HTTPException(status_code=403, detail="Откройте календарь из Telegram") - max_age = config.INIT_DATA_MAX_AGE_SECONDS or None - username, auth_reason = validate_init_data_with_reason( - init_data, config.BOT_TOKEN, max_age_seconds=max_age - ) - if username is None: - log.warning("duties: initData validation failed: %s", auth_reason) - raise HTTPException(status_code=403, detail=_auth_error_detail(auth_reason)) - if not config.can_access_miniapp(username): - log.warning("duties: username not in allowlist") - raise HTTPException(status_code=403, detail="Доступ запрещён") + get_authenticated_username(request, x_telegram_init_data) return _fetch_duties_response(from_date, to_date) -def _require_same_auth( - request: Request, - x_telegram_init_data: str | None, -) -> None: - """Raise HTTPException 403 if not allowed (same logic as list_duties).""" - init_data = (x_telegram_init_data or "").strip() - if not init_data: - client_host = request.client.host if request.client else None - if _is_private_client(client_host) or config.MINI_APP_SKIP_AUTH: - return - log.warning("calendar-events: no X-Telegram-Init-Data header (client=%s)", client_host) - raise HTTPException(status_code=403, detail="Откройте календарь из Telegram") - max_age = config.INIT_DATA_MAX_AGE_SECONDS or None - username, auth_reason = validate_init_data_with_reason( - init_data, config.BOT_TOKEN, max_age_seconds=max_age - ) - if username is None: - log.warning("calendar-events: initData validation failed: %s", auth_reason) - raise HTTPException(status_code=403, detail=_auth_error_detail(auth_reason)) - if not config.can_access_miniapp(username): - log.warning("calendar-events: username not in allowlist") - raise HTTPException(status_code=403, detail="Доступ запрещён") - - @app.get("/api/calendar-events", response_model=list[CalendarEvent]) def list_calendar_events( request: Request, @@ -166,7 +153,7 @@ def list_calendar_events( x_telegram_init_data: str | None = Header(None, alias="X-Telegram-Init-Data"), ) -> list[CalendarEvent]: _validate_duty_dates(from_date, to_date) - _require_same_auth(request, x_telegram_init_data) + get_authenticated_username(request, x_telegram_init_data) url = config.EXTERNAL_CALENDAR_ICS_URL if not url: return [] diff --git a/db/repository.py b/db/repository.py index 46b67f3..5de4e06 100644 --- a/db/repository.py +++ b/db/repository.py @@ -85,14 +85,15 @@ def get_duties( ) -> list[tuple[Duty, str]]: """Return list of (Duty, full_name) overlapping the given date range. - from_date/to_date are YYYY-MM-DD (first/last day of month in client's local calendar). - Duty.start_at and end_at are stored in UTC (ISO 8601 with Z); lexicographic comparison - with date strings yields correct overlap. + from_date/to_date are YYYY-MM-DD (inclusive). Duty.start_at and end_at are stored + in UTC (ISO 8601 with Z). Use to_date_next so duties starting on to_date are included + (start_at like 2025-01-31T09:00:00Z is > "2025-01-31" lexicographically). """ + to_date_next = (datetime.fromisoformat(to_date + "T00:00:00") + timedelta(days=1)).strftime("%Y-%m-%d") q = ( session.query(Duty, User.full_name) .join(User, Duty.user_id == User.id) - .filter(Duty.start_at <= to_date, Duty.end_at >= from_date) + .filter(Duty.start_at < to_date_next, Duty.end_at >= from_date) ) return list(q.all()) diff --git a/db/schemas.py b/db/schemas.py index 56a3e0d..b8ea6f1 100644 --- a/db/schemas.py +++ b/db/schemas.py @@ -40,7 +40,10 @@ class DutyInDb(DutyBase): class DutyWithUser(DutyInDb): - """Duty with full_name and event_type for calendar display.""" + """Duty with full_name and event_type for calendar display. + + event_type: only these values are returned; unknown DB values are mapped to "duty" in the API. + """ full_name: str event_type: Literal["duty", "unavailable", "vacation"] = "duty" diff --git a/db/session.py b/db/session.py index 251ae3c..5954a32 100644 --- a/db/session.py +++ b/db/session.py @@ -19,10 +19,13 @@ _SessionLocal = None @contextmanager def session_scope(database_url: str) -> Generator[Session, None, None]: - """Context manager: yields a session and closes it on exit.""" + """Context manager: yields a session, rolls back on exception, closes on exit.""" session = get_session(database_url) try: yield session + except Exception: + session.rollback() + raise finally: session.close() diff --git a/handlers/group_duty_pin.py b/handlers/group_duty_pin.py index d3cf5cc..c8b1951 100644 --- a/handlers/group_duty_pin.py +++ b/handlers/group_duty_pin.py @@ -118,7 +118,8 @@ async def _schedule_next_update( for job in job_queue.get_jobs_by_name(name): job.schedule_removal() if when_utc is not None: - delay = when_utc - datetime.utcnow() + now_utc = datetime.now(timezone.utc).replace(tzinfo=None) + delay = when_utc - now_utc if delay.total_seconds() < 1: delay = 1 job_queue.run_once( diff --git a/tests/test_app.py b/tests/test_app.py index 3614200..6fd15b7 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -131,3 +131,31 @@ def test_duties_e2e_auth_real_validation(client, monkeypatch): assert r.status_code == 200 assert r.json() == [] mock_fetch.assert_called_once_with("2025-01-01", "2025-01-31") + + +@patch("api.app.config.MINI_APP_SKIP_AUTH", True) +def test_duties_200_with_unknown_event_type_mapped_to_duty(client): + """When DB returns duty with event_type not in (duty, unavailable, vacation), API returns 200 with event_type='duty'.""" + from types import SimpleNamespace + + fake_duty = SimpleNamespace( + id=1, + user_id=10, + start_at="2025-01-15T09:00:00Z", + end_at="2025-01-15T18:00:00Z", + event_type="unknown", + ) + + def fake_get_duties(session, from_date, to_date): + return [(fake_duty, "User A")] + + with patch("api.app.get_duties", side_effect=fake_get_duties): + r = client.get( + "/api/duties", + params={"from": "2025-01-01", "to": "2025-01-31"}, + ) + assert r.status_code == 200 + data = r.json() + assert len(data) == 1 + assert data[0]["event_type"] == "duty" + assert data[0]["full_name"] == "User A" diff --git a/tests/test_repository_duty_range.py b/tests/test_repository_duty_range.py index 8daef8e..9d18fcb 100644 --- a/tests/test_repository_duty_range.py +++ b/tests/test_repository_duty_range.py @@ -91,3 +91,17 @@ def test_delete_duties_in_range_other_user_unchanged(session, user_a): remaining = get_duties(session, "2026-02-01", "2026-02-28") assert len(remaining) == 1 assert remaining[0][1] == "User B" + + +def test_get_duties_includes_duty_starting_on_last_day_of_range(session, user_a): + """Duty starting on to_date (e.g. 2026-01-31T09:00:00Z) must be included when to_date is 2026-01-31.""" + insert_duty( + session, + user_a.id, + "2026-01-31T09:00:00Z", + "2026-02-01T09:00:00Z", + ) + rows = get_duties(session, "2026-01-01", "2026-01-31") + assert len(rows) == 1 + assert rows[0][0].start_at == "2026-01-31T09:00:00Z" + assert rows[0][1] == "User A" diff --git a/webapp/app.js b/webapp/app.js index fc37ef4..b4924b5 100644 --- a/webapp/app.js +++ b/webapp/app.js @@ -369,10 +369,11 @@ var EVENT_TYPE_LABELS = { duty: "Дежурство", unavailable: "Недоступен", vacation: "Отпуск" }; /** Format UTC date from ISO string as DD.MM for display. */ + /** Format date as DD.MM in user's local timezone (for duty card labels). */ function formatDateKey(isoDateStr) { const d = new Date(isoDateStr); - const day = String(d.getUTCDate()).padStart(2, "0"); - const month = String(d.getUTCMonth() + 1).padStart(2, "0"); + const day = String(d.getDate()).padStart(2, "0"); + const month = String(d.getMonth() + 1).padStart(2, "0"); return day + "." + month; }