diff --git a/.coverage b/.coverage index 8644eac..87c081b 100644 Binary files a/.coverage and b/.coverage differ diff --git a/.env.example b/.env.example index cdff469..97da450 100644 --- a/.env.example +++ b/.env.example @@ -12,7 +12,8 @@ ADMIN_USERNAMES=admin1,admin2 # ALLOWED_PHONES= # ADMIN_PHONES=79001111111 -# Dev only: set to 1 to allow calendar without Telegram initData (insecure; do not use in production). +# Dev only: set to 1 to allow /api/duties and /api/calendar-events without Telegram initData. +# Insecure — never use in production. # MINI_APP_SKIP_AUTH=1 # Optional: URL of a public ICS calendar (e.g. holidays). Days from this calendar are highlighted on the duty grid; click "i" for summary. diff --git a/README.md b/README.md index 4c86862..fcb2224 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Ensure `.env` exists (e.g. `cp .env.example .env`) and contains `BOT_TOKEN`. The image is built from `Dockerfile`; on start, `entrypoint.sh` runs Alembic migrations then starts the app as user `botuser`. -**Production behind a reverse proxy:** When the app is behind nginx/Caddy etc., `request.client.host` is usually the proxy (e.g. 127.0.0.1). The "private IP" bypass (allowing requests without initData from localhost) then applies to the proxy, not the real client. Either ensure the Mini App always sends initData, or forward the real client IP (e.g. `X-Forwarded-For`) and use it for that check. See `api/app.py` `_is_private_client` for details. +**API auth:** Without valid `X-Telegram-Init-Data` header, `GET /api/duties` and `GET /api/calendar-events` return **403**. The only way to allow unauthenticated access is `MINI_APP_SKIP_AUTH=1` (dev only; do not use in production). When behind a reverse proxy, ensure the Mini App is opened from Telegram so initData is sent. ## API @@ -93,11 +93,11 @@ The HTTP server is FastAPI; the miniapp is served at `/app`. **Interactive API documentation (Swagger UI)** is available at **`/docs`**, e.g. `http://localhost:8080/docs` when running locally. -- **`GET /api/duties`** — List of duties (date params; auth via Telegram initData or, in dev, `MINI_APP_SKIP_AUTH` / private IP). +- **`GET /api/duties`** — List of duties (date params; auth via Telegram initData or, in dev only, `MINI_APP_SKIP_AUTH`). - **`GET /api/calendar-events`** — Calendar events (including external ICS when `EXTERNAL_CALENDAR_ICS_URL` is set). - **`GET /api/calendar/ical/{token}.ics`** — Personal ICS calendar (by secret token; no Telegram auth). -For production, initData validation is required; see the reverse-proxy paragraph above for proxy/headers. +Without initData the API returns 403; for local dev without Telegram use `MINI_APP_SKIP_AUTH=1` (insecure, dev only). ## Project layout diff --git a/docs/configuration.md b/docs/configuration.md index 489cfbd..7940dab 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,7 +12,7 @@ All configuration is read from the environment (e.g. `.env` via python-dotenv). | **ADMIN_USERNAMES** | comma-separated list | *(empty)* | Telegram usernames treated as **admin fallback** when the user has **no role in the DB**. If a user has a role in the DB, only that role applies. Example: `admin1,admin2`. | | **ALLOWED_PHONES** | comma-separated list | *(empty)* | **Not used for access.** Kept for reference only. | | **ADMIN_PHONES** | comma-separated list | *(empty)* | Phones treated as **admin fallback** when the user has **no role in the DB** (user sets phone via `/set_phone`). Comparison uses digits only. Example: `+7 999 123-45-67`. | -| **MINI_APP_SKIP_AUTH** | `1`, `true`, or `yes` | *(unset)* | If set, `/api/duties` is allowed without Telegram initData (dev only; insecure). | +| **MINI_APP_SKIP_AUTH** | `1`, `true`, or `yes` | *(unset)* | If set, `/api/duties` and `/api/calendar-events` are allowed without Telegram initData. **Dev only — never use in production.** | | **INIT_DATA_MAX_AGE_SECONDS** | integer | `0` | Reject Telegram initData older than this many seconds. `0` = disabled. Example: `86400` for 24 hours. | | **CORS_ORIGINS** | comma-separated list | `*` | Allowed origins for CORS. Leave unset or set to `*` for allow-all. Example: `https://your-domain.com`. | | **EXTERNAL_CALENDAR_ICS_URL** | string (URL) | *(empty)* | URL of a public ICS calendar (e.g. holidays). If set, those days are highlighted on the duty grid; users can tap "i" on a cell to see the event summary. Empty = no external calendar. | diff --git a/docs/runbook.md b/docs/runbook.md index 52d2b55..3fd5163 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -48,13 +48,13 @@ On container start, `entrypoint.sh` runs Alembic migrations then starts the app ### Miniapp "Open in browser" or direct link — access denied -- **Cause:** When users open the calendar via “Open in browser” or a direct URL, Telegram may not send `tgWebAppData` (initData). The API requires initData (or `MINI_APP_SKIP_AUTH` / private IP in dev). +- **Cause:** When users open the calendar via “Open in browser” or a direct URL, Telegram may not send `tgWebAppData` (initData). The API requires initData (or `MINI_APP_SKIP_AUTH` in dev). - **Action:** Users should open the calendar **via the bot’s menu button** (e.g. ⋮ → "Calendar") or a **Web App inline button** so Telegram sends user data. ### 403 "Open from Telegram" / no initData -- **Cause:** Request to `/api/duties` (or calendar) without valid `X-Telegram-Init-Data` header. In production, only private IP clients can be allowed without initData (see `_is_private_client` in `api/dependencies.py`); behind a reverse proxy, `request.client.host` is often the proxy (e.g. 127.0.0.1), so the “private IP” bypass may not apply to the real user. -- **Check:** Ensure the Mini App is opened from Telegram (menu or inline button). If behind a reverse proxy, see README “Production behind a reverse proxy” (forward real client IP or rely on initData). +- **Cause:** Request to `/api/duties` or `/api/calendar-events` without valid `X-Telegram-Init-Data` header. The API returns 403 without initData; the only bypass is `MINI_APP_SKIP_AUTH=1` (dev only, insecure). +- **Check:** Ensure the Mini App is opened from Telegram (menu or inline button). For local dev without Telegram, use `MINI_APP_SKIP_AUTH=1` in `.env`. ### Mini App URL — redirect and broken auth diff --git a/duty_teller/api/dependencies.py b/duty_teller/api/dependencies.py index 082d401..ee8cc89 100644 --- a/duty_teller/api/dependencies.py +++ b/duty_teller/api/dependencies.py @@ -118,34 +118,6 @@ def require_miniapp_username( return get_authenticated_username(request, x_telegram_init_data, session) -def _is_private_client(client_host: str | None) -> bool: - """Return True if client_host is localhost or RFC 1918 private IPv4. - - Used to allow /api/duties without initData when opened from local/private - network (e.g. dev). IPv4 only; IPv6 only 127/::1 checked. - - Args: - client_host: Client IP or hostname from request. - - Returns: - True if loopback or 10.x, 172.16–31.x, 192.168.x.x. - """ - if not client_host: - return False - if client_host in ("127.0.0.1", "::1"): - return True - # RFC 1918 private ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 - parts = client_host.split(".") - if len(parts) == 4: - try: - a, b, c, d = (int(x) for x in parts) - if (a == 10) or (a == 172 and 16 <= b <= 31) or (a == 192 and b == 168): - return True - except (ValueError, IndexError): - pass - return False - - def get_authenticated_username( request: Request, x_telegram_init_data: str | None, @@ -154,26 +126,25 @@ def get_authenticated_username( """Return identifier for miniapp auth (username or full_name or id:...); empty if skip-auth. Args: - request: FastAPI request (client host for private-IP bypass). + request: FastAPI request (for Accept-Language in error messages). x_telegram_init_data: Raw X-Telegram-Init-Data header value. session: DB session (for phone allowlist lookup). Returns: - Username, full_name, or "id:"; empty string if MINI_APP_SKIP_AUTH - or private IP and no initData. + Username, full_name, or "id:"; empty string only if MINI_APP_SKIP_AUTH. Raises: HTTPException: 403 if initData missing/invalid or user not in allowlist. """ if config.MINI_APP_SKIP_AUTH: - log.warning("allowing without any auth check (MINI_APP_SKIP_AUTH is set)") + log.warning( + "MINI_APP_SKIP_AUTH is set — no auth check (insecure, dev only); " + "do not use in production" + ) return "" 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): - return "" - log.warning("no X-Telegram-Init-Data header (client=%s)", client_host) + log.warning("no X-Telegram-Init-Data header") lang = _lang_from_accept_language(request.headers.get("Accept-Language")) raise HTTPException(status_code=403, detail=t(lang, "api.open_from_telegram")) max_age = config.INIT_DATA_MAX_AGE_SECONDS or None diff --git a/duty_teller/run.py b/duty_teller/run.py index ab31900..3966ec5 100644 --- a/duty_teller/run.py +++ b/duty_teller/run.py @@ -81,5 +81,9 @@ def main() -> None: ) t.start() + if config.MINI_APP_SKIP_AUTH: + logger.warning( + "MINI_APP_SKIP_AUTH is set — API auth disabled (insecure); use only for dev" + ) logger.info("Bot starting (polling)... HTTP API on port %s", config.HTTP_PORT) app.run_polling(allowed_updates=["message", "my_chat_member"]) diff --git a/tests/test_api_dependencies.py b/tests/test_api_dependencies.py index 784376d..b27cd64 100644 --- a/tests/test_api_dependencies.py +++ b/tests/test_api_dependencies.py @@ -71,29 +71,3 @@ class TestValidateDutyDates: assert exc_info.value.status_code == 400 assert exc_info.value.detail == "From after to message" mock_t.assert_called_with("ru", "dates.from_after_to") - - -class TestIsPrivateClient: - """Tests for _is_private_client.""" - - def test_loopback_true(self): - assert deps._is_private_client("127.0.0.1") is True - assert deps._is_private_client("::1") is True - - def test_rfc1918_private_true(self): - assert deps._is_private_client("10.0.0.1") is True - assert deps._is_private_client("192.168.1.1") is True - assert deps._is_private_client("172.16.0.1") is True - assert deps._is_private_client("172.31.255.255") is True - - def test_public_ip_false(self): - assert deps._is_private_client("8.8.8.8") is False - - def test_non_ip_false(self): - assert deps._is_private_client("example.com") is False - assert deps._is_private_client("") is False - assert deps._is_private_client(None) is False - - def test_172_non_private_octet_false(self): - assert deps._is_private_client("172.15.0.1") is False - assert deps._is_private_client("172.32.0.1") is False diff --git a/tests/test_app.py b/tests/test_app.py index b433abc..935562d 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -37,10 +37,9 @@ def test_duties_from_after_to(client): assert "from" in detail or "to" in detail or "after" in detail or "позже" in detail -@patch("duty_teller.api.dependencies._is_private_client") @patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False) -def test_duties_403_without_init_data_from_public_client(mock_private, client): - mock_private.return_value = False +def test_duties_403_without_init_data(client): + """Without X-Telegram-Init-Data and without MINI_APP_SKIP_AUTH → 403 (any client).""" r = client.get( "/api/duties", params={"from": "2025-01-01", "to": "2025-01-31"}, @@ -390,6 +389,16 @@ def test_calendar_ical_events_all_returns_all_event_types( # --- /api/calendar-events --- +@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False) +def test_calendar_events_403_without_init_data(client): + """Without X-Telegram-Init-Data and without MINI_APP_SKIP_AUTH → 403.""" + r = client.get( + "/api/calendar-events", + params={"from": "2025-01-01", "to": "2025-01-31"}, + ) + assert r.status_code == 403 + + @patch("duty_teller.api.app.config.EXTERNAL_CALENDAR_ICS_URL", "") @patch("duty_teller.api.app.config.MINI_APP_SKIP_AUTH", True) def test_calendar_events_empty_url_returns_empty_list(client):