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.
This commit is contained in:
81
api/app.py
81
api/app.py
@@ -46,7 +46,11 @@ def _fetch_duties_response(from_date: str, to_date: str) -> list[DutyWithUser]:
|
|||||||
start_at=duty.start_at,
|
start_at=duty.start_at,
|
||||||
end_at=duty.end_at,
|
end_at=duty.end_at,
|
||||||
full_name=full_name,
|
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
|
for duty, full_name in rows
|
||||||
]
|
]
|
||||||
@@ -87,6 +91,33 @@ def _is_private_client(client_host: str | None) -> bool:
|
|||||||
return False
|
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 = FastAPI(title="Duty Teller API")
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
CORSMiddleware,
|
CORSMiddleware,
|
||||||
@@ -110,54 +141,10 @@ def list_duties(
|
|||||||
request.client.host if request.client else "?",
|
request.client.host if request.client else "?",
|
||||||
bool((x_telegram_init_data or "").strip()),
|
bool((x_telegram_init_data or "").strip()),
|
||||||
)
|
)
|
||||||
init_data = (x_telegram_init_data or "").strip()
|
get_authenticated_username(request, x_telegram_init_data)
|
||||||
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="Доступ запрещён")
|
|
||||||
return _fetch_duties_response(from_date, to_date)
|
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])
|
@app.get("/api/calendar-events", response_model=list[CalendarEvent])
|
||||||
def list_calendar_events(
|
def list_calendar_events(
|
||||||
request: Request,
|
request: Request,
|
||||||
@@ -166,7 +153,7 @@ def list_calendar_events(
|
|||||||
x_telegram_init_data: str | None = Header(None, alias="X-Telegram-Init-Data"),
|
x_telegram_init_data: str | None = Header(None, alias="X-Telegram-Init-Data"),
|
||||||
) -> list[CalendarEvent]:
|
) -> list[CalendarEvent]:
|
||||||
_validate_duty_dates(from_date, to_date)
|
_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
|
url = config.EXTERNAL_CALENDAR_ICS_URL
|
||||||
if not url:
|
if not url:
|
||||||
return []
|
return []
|
||||||
|
|||||||
@@ -85,14 +85,15 @@ def get_duties(
|
|||||||
) -> list[tuple[Duty, str]]:
|
) -> list[tuple[Duty, str]]:
|
||||||
"""Return list of (Duty, full_name) overlapping the given date range.
|
"""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).
|
from_date/to_date are YYYY-MM-DD (inclusive). Duty.start_at and end_at are stored
|
||||||
Duty.start_at and end_at are stored in UTC (ISO 8601 with Z); lexicographic comparison
|
in UTC (ISO 8601 with Z). Use to_date_next so duties starting on to_date are included
|
||||||
with date strings yields correct overlap.
|
(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 = (
|
q = (
|
||||||
session.query(Duty, User.full_name)
|
session.query(Duty, User.full_name)
|
||||||
.join(User, Duty.user_id == User.id)
|
.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())
|
return list(q.all())
|
||||||
|
|
||||||
|
|||||||
@@ -40,7 +40,10 @@ class DutyInDb(DutyBase):
|
|||||||
|
|
||||||
|
|
||||||
class DutyWithUser(DutyInDb):
|
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
|
full_name: str
|
||||||
event_type: Literal["duty", "unavailable", "vacation"] = "duty"
|
event_type: Literal["duty", "unavailable", "vacation"] = "duty"
|
||||||
|
|||||||
@@ -19,10 +19,13 @@ _SessionLocal = None
|
|||||||
|
|
||||||
@contextmanager
|
@contextmanager
|
||||||
def session_scope(database_url: str) -> Generator[Session, None, None]:
|
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)
|
session = get_session(database_url)
|
||||||
try:
|
try:
|
||||||
yield session
|
yield session
|
||||||
|
except Exception:
|
||||||
|
session.rollback()
|
||||||
|
raise
|
||||||
finally:
|
finally:
|
||||||
session.close()
|
session.close()
|
||||||
|
|
||||||
|
|||||||
@@ -118,7 +118,8 @@ async def _schedule_next_update(
|
|||||||
for job in job_queue.get_jobs_by_name(name):
|
for job in job_queue.get_jobs_by_name(name):
|
||||||
job.schedule_removal()
|
job.schedule_removal()
|
||||||
if when_utc is not None:
|
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:
|
if delay.total_seconds() < 1:
|
||||||
delay = 1
|
delay = 1
|
||||||
job_queue.run_once(
|
job_queue.run_once(
|
||||||
|
|||||||
@@ -131,3 +131,31 @@ def test_duties_e2e_auth_real_validation(client, monkeypatch):
|
|||||||
assert r.status_code == 200
|
assert r.status_code == 200
|
||||||
assert r.json() == []
|
assert r.json() == []
|
||||||
mock_fetch.assert_called_once_with("2025-01-01", "2025-01-31")
|
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"
|
||||||
|
|||||||
@@ -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")
|
remaining = get_duties(session, "2026-02-01", "2026-02-28")
|
||||||
assert len(remaining) == 1
|
assert len(remaining) == 1
|
||||||
assert remaining[0][1] == "User B"
|
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"
|
||||||
|
|||||||
@@ -369,10 +369,11 @@
|
|||||||
var EVENT_TYPE_LABELS = { duty: "Дежурство", unavailable: "Недоступен", vacation: "Отпуск" };
|
var EVENT_TYPE_LABELS = { duty: "Дежурство", unavailable: "Недоступен", vacation: "Отпуск" };
|
||||||
|
|
||||||
/** Format UTC date from ISO string as DD.MM for display. */
|
/** 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) {
|
function formatDateKey(isoDateStr) {
|
||||||
const d = new Date(isoDateStr);
|
const d = new Date(isoDateStr);
|
||||||
const day = String(d.getUTCDate()).padStart(2, "0");
|
const day = String(d.getDate()).padStart(2, "0");
|
||||||
const month = String(d.getUTCMonth() + 1).padStart(2, "0");
|
const month = String(d.getMonth() + 1).padStart(2, "0");
|
||||||
return day + "." + month;
|
return day + "." + month;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user