From cc03c5fa3dca3f8ac14d1a40adbc7abb20eab99f Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 9 Sep 2026 05:35:16 +0300 Subject: [PATCH] =?UTF-8?q?:lock:=20fix(security):=20=D0=B3=D1=80=D0=B0?= =?UTF-8?q?=D0=BD=D0=B8=D1=86=D1=8B,=20=D0=BA=D0=BE=D1=82=D0=BE=D1=80?= =?UTF-8?q?=D1=8B=D0=B5=20=D0=BE=D0=B1=D1=85=D0=BE=D0=B4=D0=B8=D0=BB=D0=B8?= =?UTF-8?q?=D1=81=D1=8C=20=D1=87=D0=B5=D1=80=D0=B5=D0=B7=20=D1=81=D0=BE?= =?UTF-8?q?=D1=81=D0=B5=D0=B4=D0=BD=D0=B8=D0=B9=20=D1=8D=D0=BD=D0=B4=D0=BF?= =?UTF-8?q?=D0=BE=D0=B8=D0=BD=D1=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Разбор аудита, блоки B, C и D. Каждая правка — с тестом. Критичное: - 2FA снималась без пароля: POST /auth/profile/totp/start/ выключал уже включённую 2FA, минуя и текущий пароль, и требование организации, которые спрашивает соседний /disable/. Теперь 409, если 2FA включена. - Сброс пароля по письму оставлял чужие сессии живыми — то есть не помогал ровно в том случае, ради которого пароль и сбрасывают. Сессии завершаются, как при смене пароля из профиля. - HTTPS на домене установки не выпускался никогда: ask-эндпоинт шлюза знал только домены порталов. Теперь он признаёт и адрес самой установки. - WebSocket молча не работал на любой установке с TLS: браузер держит __Host-cookie, а Channels ищет сессию по обычному имени, и HTTP-middleware на хендшейк не выполняется. Добавлен chatballs.http.ws_middleware. Существенное: - Повтор входящего сообщения ронял весь цикл поллинга: IntegrityError ловился без точки сохранения внутри чужой транзакции. - Смена адреса в «Настройках» выбрасывала того, кто её делает. Прежний адрес остаётся принятым (identity.0033). - Редирект уводил скачивание во внутреннюю сеть: политика исходящих проверяла только исходный адрес. Проверка висит на каждом Location. - Портал помощи можно было повесить на адрес установки и подменить сотрудникам приложение своим Help Center. - Пароль прокси уходил в ответ API целиком; теперь маскируется, а маска при сохранении возвращает сохранённый пароль. - /api/v1/health/ready/ закрыт на публичной границе. Мелочи: адрес для A-записи портала считается от адреса установки, а не от 127.0.0.1; колонка EncryptedCharField вмещает шифротекст, а не открытое значение; WS-маршруты проверяют Origin; мёртвый require_organization_scope убран. Co-Authored-By: Claude Opus 5 --- AUDIT-TODO.md | 42 ++++-- apps/backend/chatballs/ai/agent_card_views.py | 1 - apps/backend/chatballs/api/permissions.py | 5 + apps/backend/chatballs/calls/routing.py | 6 +- .../backend/chatballs/conversations/ingest.py | 23 ++- .../chatballs/conversations/routing.py | 7 +- .../conversations/test_ingest_dedup.py | 53 +++++++ .../conversations/transports/base.py | 10 +- .../chatballs/http/test_ws_middleware.py | 122 ++++++++++++++++ apps/backend/chatballs/http/ws_middleware.py | 117 +++++++++++++++ .../identity/administration_views.py | 3 - .../chatballs/identity/auth/password_reset.py | 9 +- .../chatballs/identity/auth/profile.py | 16 ++- apps/backend/chatballs/identity/crypto.py | 28 +++- apps/backend/chatballs/identity/demo_views.py | 1 - .../chatballs/identity/instance_settings.py | 28 +++- .../chatballs/identity/instance_views.py | 6 +- .../0033_instance_previous_public_host.py | 23 +++ .../0034_encrypted_column_widths.py | 34 +++++ .../chatballs/identity/test_auth_hardening.py | 133 ++++++++++++++++++ .../chatballs/identity/test_crypto_columns.py | 52 +++++++ .../identity/test_instance_address.py | 121 ++++++++++++++++ .../migrations/0008_encrypted_column_width.py | 19 +++ apps/backend/chatballs/integrations/proxy.py | 45 +++++- .../chatballs/integrations/serializers.py | 47 ++++++- .../integrations/test_outbound_redirects.py | 75 ++++++++++ .../integrations/test_proxy_masking.py | 108 ++++++++++++++ apps/backend/chatballs/integrations/views.py | 22 ++- .../chatballs/support_portals/addressing.py | 17 +++ .../support_portals/domain_services.py | 7 +- .../support_portals/gateway_views.py | 21 ++- .../support_portals/host_boundary.py | 9 +- .../support_portals/public_address.py | 84 +++++++++++ .../chatballs/support_portals/serializers.py | 7 +- .../tests/test_public_address.py | 55 ++++++++ .../support_portals/tests/test_public_api.py | 34 +++++ .../0030_encrypted_column_widths.py | 28 ++++ .../chatballs_backend/settings_base.py | 26 ++-- .../features/integrations/IntegrationForm.tsx | 5 +- deploy/nginx/frontend.production.conf | 8 ++ 40 files changed, 1383 insertions(+), 74 deletions(-) create mode 100644 apps/backend/chatballs/conversations/test_ingest_dedup.py create mode 100644 apps/backend/chatballs/http/test_ws_middleware.py create mode 100644 apps/backend/chatballs/http/ws_middleware.py create mode 100644 apps/backend/chatballs/identity/migrations/0033_instance_previous_public_host.py create mode 100644 apps/backend/chatballs/identity/migrations/0034_encrypted_column_widths.py create mode 100644 apps/backend/chatballs/identity/test_auth_hardening.py create mode 100644 apps/backend/chatballs/identity/test_crypto_columns.py create mode 100644 apps/backend/chatballs/identity/test_instance_address.py create mode 100644 apps/backend/chatballs/integrations/migrations/0008_encrypted_column_width.py create mode 100644 apps/backend/chatballs/integrations/test_outbound_redirects.py create mode 100644 apps/backend/chatballs/integrations/test_proxy_masking.py create mode 100644 apps/backend/chatballs/support_portals/public_address.py create mode 100644 apps/backend/chatballs/support_portals/tests/test_public_address.py create mode 100644 apps/backend/chatballs/tenancy/migrations/0030_encrypted_column_widths.py diff --git a/AUDIT-TODO.md b/AUDIT-TODO.md index 5b36bcb..ad66a45 100644 --- a/AUDIT-TODO.md +++ b/AUDIT-TODO.md @@ -74,7 +74,7 @@ ## B. Безопасность — критично -- [ ] **B1. 2FA снимается без пароля.** +- [x] **B1. 2FA снимается без пароля.** `apps/backend/chatballs/identity/auth/profile.py:117` — `ProfileTotpStartView` (`POST /api/v1/auth/profile/totp/start/`, только `IsAuthenticated`) ставит `totp_enabled = False` и чистит секрет. @@ -86,8 +86,9 @@ что у `/disable/`. **Доказательство:** тест — при включённой 2FA `/start/` не меняет `totp_enabled`/`totp_secret` и не обходит `user_requires_totp`. + **Сделано:** `/start/` отвечает 409, если 2FA уже включена: перевыпуск секрета возможен только при выключенной. Тест `identity.test_auth_hardening`. -- [ ] **B2. Сброс пароля по письму не завершает чужие сессии.** +- [x] **B2. Сброс пароля по письму не завершает чужие сессии.** `apps/backend/chatballs/identity/auth/password_reset.py:96` — `set_password` и всё. Смена пароля в профиле сессии отзывает (`profile.py:107`), админский сброс тоже (`identity/employee_password.py:75`). То есть ровно тот сценарий, @@ -95,8 +96,9 @@ **Сделать:** после успешного сброса звать `revoke_user_sessions(user.id)`. **Доказательство:** тест — активная сессия до сброса становится недействительной после него. + **Сделано:** После сброса зовётся `revoke_user_sessions`; ответ отдаёт число завершённых сессий. Тест `identity.test_auth_hardening`. -- [ ] **B3. HTTPS на домене установки не включается никогда.** +- [x] **B3. HTTPS на домене установки не включается никогда.** `Caddyfile` выдаёт сертификаты только через `on_demand` + `ask`, а `apps/backend/chatballs/support_portals/gateway_views.py:16` авторизует **только домены порталов** из `support_portal_directory`. Проверено на живом @@ -111,8 +113,9 @@ (и, если нужно, платформенный домен) наравне с доменами порталов. **Доказательство:** тест на эндпоинт (204 для заданного в UI адреса) + ручная проверка выпуска сертификата на реальном домене. + **Сделано:** Ask-эндпоинт признаёт адрес установки (и предыдущий) наравне с доменами порталов. Тест `support_portals.tests.test_public_api`. -- [ ] **B4. WebSocket ломается под HTTPS.** +- [x] **B4. WebSocket ломается под HTTPS.** `TlsAwareCookieMiddleware` — HTTP-middleware, на WS-хендшейк не работает. Channels читает `settings.SESSION_COOKIE_NAME` = `chatballs_app_session`, а браузер под TLS держит только `__Host-chatballs-app-session` (обычное имя @@ -123,6 +126,7 @@ правило имён (`CHATBALLS_TLS_COOKIE_NAMES`) к WS-scope. **Доказательство:** тест WS-подключения со scope, где выставлен только `__Host-`-cookie и `scheme=wss`. + **Сделано:** Добавлен `chatballs.http.ws_middleware`: имена cookie приводятся к тем, по которым Channels ищет сессию, до `AuthMiddlewareStack`. Тест `http.test_ws_middleware`. - [x] **B5. Загрузка файлов упадёт на Linux-хосте.** Prod-образ работает под `USER hub` (`apps/backend/Dockerfile.production:35`), @@ -141,7 +145,7 @@ ## C. Безопасность и корректность — существенно -- [ ] **C1. Дубль входящего сообщения ломает транзакцию.** +- [x] **C1. Дубль входящего сообщения ломает транзакцию.** `apps/backend/chatballs/conversations/ingest.py:52` ловит `IntegrityError` от `InboxEvent.objects.create` **без вложенного `transaction.atomic()`**, а вызывается изнутри `tenant_atomic` (воркер: @@ -152,8 +156,9 @@ **Сделать:** обернуть create в `transaction.atomic()` (savepoint). **Доказательство:** тест — повторная доставка того же `external_id` внутри транзакции возвращает «уже обработано» и не ломает последующие запросы. + **Сделано:** Вставка в inbox идёт своей точкой сохранения. Тест `conversations.test_ingest_dedup`. -- [ ] **C2. Смена адреса в «Настройках» может залочить владельца.** +- [x] **C2. Смена адреса в «Настройках» может залочить владельца.** `identity/instance_views.py:63` пишет новый `public_host`, а `support_portals/host_boundary.py:35` принимает только его + `localhost/127.0.0.1/app.localhost`. Владелец, сидящий на `http://`, @@ -164,8 +169,9 @@ список адресов установки, а не одно поле). **Доказательство:** тест — после смены адреса запрос со старым Host всё ещё обслуживается. + **Сделано:** Добавлено поле `previous_public_host` (миграция identity.0033): прежний адрес остаётся принятым и шлюзом, и проверкой Host. Тест `identity.test_instance_address`. -- [ ] **C3. SSRF через редирект.** +- [x] **C3. SSRF через редирект.** `apps/backend/chatballs/integrations/outbound.py:54` проверяет адрес **до** запроса, а `build_opener` тянет штатный `HTTPRedirectHandler`. Ответ подставного провайдера отдаёт 302 на `http://169.254.169.254/…` — и хаб @@ -174,8 +180,9 @@ **Сделать:** свой `HTTPRedirectHandler`, прогоняющий `ensure_downloadable` на каждый `Location`. **Доказательство:** тест с локальным сервером, отдающим 302 на приватный адрес. + **Сделано:** Свой `HTTPRedirectHandler` прогоняет политику на каждый Location, включая проверку схемы до urllib. Тест `integrations.test_outbound_redirects`. -- [ ] **C4. Портал может «съесть» само приложение.** +- [x] **C4. Портал может «съесть» само приложение.** `support_portals/addressing.py:64` запрещает `custom_domain` только из `CHATBALLS_APP_PRIMARY_HOSTS` и не смотрит на `InstanceSettings.public_host`. Админ вешает портал на адрес установки → SPA пробует `/api/v1/help/` @@ -183,20 +190,23 @@ вместо приложения. Сотрудники теряют вход. **Сделать:** добавить адрес установки в запрещённые для `custom_domain`. **Доказательство:** тест валидации портала. + **Сделано:** Адрес установки (текущий и предыдущий) добавлен в запрещённые для `custom_domain`. Тест `identity.test_instance_address`. -- [ ] **C5. Пароль прокси уходит в API.** +- [x] **C5. Пароль прокси уходит в API.** `integrations/serializers.py:29` отдаёт `proxyUrl` целиком, а формат — `socks5://user:pass@host:port` (`integrations/proxy.py`). Секрет интеграции маскируется, credentials прокси — нет. **Сделать:** отдавать прокси без user:pass (как `hasSecret`/маска у секрета). **Доказательство:** тест payload'а интеграции. + **Сделано:** Пароль прокси маскируется в ответе; маска того же прокси при сохранении возвращает сохранённый пароль. Тест `integrations.test_proxy_masking`. -- [ ] **C6. `/api/v1/health/ready/` публичен.** +- [x] **C6. `/api/v1/health/ready/` публичен.** Доступен снаружи через Caddy → frontend → backend без авторизации и отдаёт состояние БД и Redis. **Сделать:** оставить снаружи только `live/`, `ready/` увести во внутренний контур (отдельный путь/сеть либо отказ на уровне frontend nginx). **Доказательство:** запрос снаружи — 404, изнутри сети — 200. + **Сделано:** `/api/v1/health/ready/` закрыт на публичной границе (frontend nginx); liveness остаётся открытым. --- @@ -208,16 +218,20 @@ комментарием. То же самое проверить для `CHATBALLS_APP_DOMAIN` после B3. **Сделано:** `CHATBALLS_ACME_EMAIL` и `CHATBALLS_APP_DOMAIN` убраны из окружения шлюза как неиспользуемые. Выпуск сертификата на домен установки остаётся в B3. -- [ ] **D2.** В коробке `CHATBALLS_HELP_PUBLIC_IPV4` схлопывается в `127.0.0.1` +- [x] **D2.** В коробке `CHATBALLS_HELP_PUBLIC_IPV4` схлопывается в `127.0.0.1` (`chatballs_backend/settings_base.py:258`) — инструкции по DNS для порталов будут указывать на loopback. -- [ ] **D3.** `EncryptedCharField(max_length=512)` хранит **шифротекст** в + **Сделано:** Значение считается в рантайме от адреса установки (`support_portals.public_address`); переменная окружения — переопределение, фейл-фаст убран. Тест `support_portals.tests.test_public_address`. +- [x] **D3.** `EncryptedCharField(max_length=512)` хранит **шифротекст** в varchar(512) (`identity/crypto.py`), а Fernet раздувает примерно в 1.4 раза плюс сотня символов: длинный SMTP-пароль обрежется или упадёт на записи. -- [ ] **D4.** WS-роутинг без `AllowedHostsOriginValidator` + **Сделано:** `max_length` описывает открытое значение, ширину колонки считает `ciphertext_length` (миграции identity.0034, integrations.0008, tenancy.0030). Тест `identity.test_crypto_columns`. +- [x] **D4.** WS-роутинг без `AllowedHostsOriginValidator` (`conversations/routing.py`) — сейчас спасает только `SameSite=Lax`. -- [ ] **D5.** `require_organization_scope = True` в `identity/demo_views.py:32` — + **Сделано:** Добавлен `SameOriginWebSocketMiddleware` на оба WS-маршрута. Тест `http.test_ws_middleware`. +- [x] **D5.** `require_organization_scope = True` в `identity/demo_views.py:32` — мёртвый атрибут, `HasCapability` его не читает. Убрать или начать читать. + **Сделано:** Мёртвый атрибут убран во всех восьми местах; в `HasCapability` записано, что область организации не отключается. --- diff --git a/apps/backend/chatballs/ai/agent_card_views.py b/apps/backend/chatballs/ai/agent_card_views.py index eaa0847..94a54f2 100644 --- a/apps/backend/chatballs/ai/agent_card_views.py +++ b/apps/backend/chatballs/ai/agent_card_views.py @@ -225,7 +225,6 @@ class AgentCardTestChatView(APIView): permission_classes = [HasCapability] # Исполняет агента, а не изменяет канал: остаётся на ai.manage (ADR-HUB-0037 §9). required_capability = "ai.manage" - require_organization_scope = True def post(self, request: Request, agent_id: int) -> Response: try: diff --git a/apps/backend/chatballs/api/permissions.py b/apps/backend/chatballs/api/permissions.py index 2a15275..75acab7 100644 --- a/apps/backend/chatballs/api/permissions.py +++ b/apps/backend/chatballs/api/permissions.py @@ -12,6 +12,11 @@ class HasCapability(BasePermission): Views declare ``required_capability`` or a method keyed ``required_capabilities`` mapping. Object/resource scope is still checked by the view after loading the canonical resource. + + Организационная область не объявляется вьюхой и не отключается: без + членства в организации проверка не проходит вообще. Раньше рядом стоял + атрибут ``require_organization_scope = True``, который никто не читал — он + выглядел как переключатель там, где переключателя нет. """ message = "Required capability is missing" diff --git a/apps/backend/chatballs/calls/routing.py b/apps/backend/chatballs/calls/routing.py index b560057..86432fd 100644 --- a/apps/backend/chatballs/calls/routing.py +++ b/apps/backend/chatballs/calls/routing.py @@ -1,7 +1,11 @@ from django.urls import path from chatballs.calls.consumers import CallSignalingConsumer +from chatballs.http.ws_middleware import SameOriginWebSocketMiddleware +# Сигналинг звонка аутентифицируется call access token, а не сессией, поэтому +# переименование cookie ему не нужно. Проверка Origin — нужна: страницу звонка +# открывает браузер, и чужой сайт не должен открывать сокет за него. websocket_urlpatterns = [ - path("ws/calls/", CallSignalingConsumer.as_asgi()), + path("ws/calls/", SameOriginWebSocketMiddleware(CallSignalingConsumer.as_asgi())), ] diff --git a/apps/backend/chatballs/conversations/ingest.py b/apps/backend/chatballs/conversations/ingest.py index 9d5d65b..6dae786 100644 --- a/apps/backend/chatballs/conversations/ingest.py +++ b/apps/backend/chatballs/conversations/ingest.py @@ -48,15 +48,24 @@ _ROLE = { def _already_processed(context: TenantContext, source: str, external_id: str, text: str) -> bool: + """Отметить сообщение обработанным; True — оно уже приходило. + + Вставка идёт своей точкой сохранения. Вызывают эту функцию изнутри + транзакции (воркер держит ``tenant_atomic`` на весь цикл поллинга), а + IntegrityError в Postgres обрывает транзакцию целиком: без savepoint + первый же повтор сообщения ронял не дедупликацию, а весь цикл — со всеми + остальными подключениями организации. + """ payload_hash = hashlib.sha256(text.encode("utf-8")).hexdigest()[:32] try: - InboxEvent.objects.create( - source=source, - external_event_id=external_id, - payload_hash=payload_hash, - ownership=EventOwnership.TENANT, - organization=context.organization, - ) + with transaction.atomic(): + InboxEvent.objects.create( + source=source, + external_event_id=external_id, + payload_hash=payload_hash, + ownership=EventOwnership.TENANT, + organization=context.organization, + ) return False except IntegrityError: return True diff --git a/apps/backend/chatballs/conversations/routing.py b/apps/backend/chatballs/conversations/routing.py index 02364bf..ffa4842 100644 --- a/apps/backend/chatballs/conversations/routing.py +++ b/apps/backend/chatballs/conversations/routing.py @@ -2,13 +2,18 @@ from channels.auth import AuthMiddlewareStack from django.urls import path from chatballs.conversations.consumers import ConversationEventsConsumer +from chatballs.http.ws_middleware import websocket_boundary # Оповещения о диалогах аутентифицируются сессией того же SPA — отдельного # токена, как у сигналинга звонков, здесь не нужно: сокет открывает тот же # браузер. Организация стоит в адресе, как и во всём HTTP-слое. +# +# websocket_boundary снаружи AuthMiddlewareStack: он приводит имена cookie к +# тем, по которым Channels ищет сессию (по TLS браузер держит __Host-…), и +# отбивает хендшейк с чужим Origin. websocket_urlpatterns = [ path( "ws/organizations//conversations/", - AuthMiddlewareStack(ConversationEventsConsumer.as_asgi()), + websocket_boundary(AuthMiddlewareStack(ConversationEventsConsumer.as_asgi())), ), ] diff --git a/apps/backend/chatballs/conversations/test_ingest_dedup.py b/apps/backend/chatballs/conversations/test_ingest_dedup.py new file mode 100644 index 0000000..1151f28 --- /dev/null +++ b/apps/backend/chatballs/conversations/test_ingest_dedup.py @@ -0,0 +1,53 @@ +"""Повторная доставка входящего не должна ломать транзакцию. + +Дедупликация ставит запись в inbox и ловит IntegrityError на повторе. Ловить +его без точки сохранения нельзя: Postgres обрывает транзакцию целиком, и +следующий же запрос падает с TransactionManagementError. А вызывают это +изнутри транзакции — воркер держит ``tenant_atomic`` на весь цикл поллинга, +так что первый повтор ронял не дедупликацию, а весь цикл организации. +""" + +from __future__ import annotations + +from django.db import transaction +from django.test import TestCase + +from chatballs.conversations.ingest import _already_processed +from chatballs.events.models import InboxEvent +from chatballs.identity.bootstrap import bootstrap_owner +from chatballs.tenancy.database import tenant_atomic +from chatballs.testing import system_tenant_context + + +class InboundDeduplicationTests(TestCase): + def setUp(self) -> None: + result = bootstrap_owner(email="ingest-owner@example.com", password="Owner-Password-2026!") + self.context = system_tenant_context(result.organization) + + def test_repeat_is_reported_without_breaking_the_transaction(self) -> None: + with tenant_atomic(self.context): + first = _already_processed(self.context, "telegram:1", "update-42", "привет") + second = _already_processed(self.context, "telegram:1", "update-42", "привет") + + self.assertFalse(first) + self.assertTrue(second) + + # Главное: транзакция жива и дальше в ней можно работать. Раньше + # именно здесь всё и разваливалось. + self.assertEqual( + InboxEvent.objects.filter(external_event_id="update-42").count(), 1 + ) + + def test_repeat_does_not_roll_back_work_done_earlier(self) -> None: + with transaction.atomic(): + with tenant_atomic(self.context): + _already_processed(self.context, "max:7", "update-1", "первое") + _already_processed(self.context, "max:7", "update-1", "первое") + _already_processed(self.context, "max:7", "update-2", "второе") + + self.assertEqual(InboxEvent.objects.filter(source="max:7").count(), 2) + + def test_different_sources_do_not_collide(self) -> None: + with tenant_atomic(self.context): + self.assertFalse(_already_processed(self.context, "telegram:1", "shared-id", "текст")) + self.assertFalse(_already_processed(self.context, "max:2", "shared-id", "текст")) diff --git a/apps/backend/chatballs/conversations/transports/base.py b/apps/backend/chatballs/conversations/transports/base.py index 9f58a6a..5e1cc9a 100644 --- a/apps/backend/chatballs/conversations/transports/base.py +++ b/apps/backend/chatballs/conversations/transports/base.py @@ -94,9 +94,15 @@ def download_bytes( ``allowed_host`` — хост из ``base_url`` подключения, который владелец назвал сам. """ - ensure_downloadable(url, allowed_host=allowed_host, via_proxy=bool(proxy_url)) + def guard(candidate: str) -> None: + ensure_downloadable(candidate, allowed_host=allowed_host, via_proxy=bool(proxy_url)) + + guard(url) request = urllib.request.Request(url) - with build_opener(proxy_url).open(request, timeout=settings.CHATBALLS_AI_REQUEST_TIMEOUT) as response: + # Та же проверка на каждый редирект: провайдер отдаёт адрес данными, и + # 302 увёл бы скачивание туда, куда исходный адрес не пустили. + opener = build_opener(proxy_url, validate_redirect=guard) + with opener.open(request, timeout=settings.CHATBALLS_AI_REQUEST_TIMEOUT) as response: data = response.read(max_bytes + 1) if len(data) > max_bytes: raise ValueError("Файл больше допустимого размера") diff --git a/apps/backend/chatballs/http/test_ws_middleware.py b/apps/backend/chatballs/http/test_ws_middleware.py new file mode 100644 index 0000000..2dc9c5e --- /dev/null +++ b/apps/backend/chatballs/http/test_ws_middleware.py @@ -0,0 +1,122 @@ +"""WebSocket-обвязка: имена cookie по TLS и проверка Origin. + +Установка с сертификатом отдаёт браузеру только ``__Host-``-имена, а Channels +ищет сессию строго по ``settings.SESSION_COOKIE_NAME``. Пока это не сходилось, +каждый сокет на https закрывался как неаутентифицированный — и живые +обновления диалогов молча не работали, притом что REST на той же странице +работал. +""" + +from __future__ import annotations + +import asyncio + +from django.test import SimpleTestCase, override_settings + +from chatballs.http.ws_middleware import ( + SameOriginWebSocketMiddleware, + TlsAwareCookieASGIMiddleware, +) + +PLAIN = "chatballs_app_session" +HARDENED = "__Host-chatballs-app-session" +COOKIE_NAMES = {PLAIN: HARDENED, "chatballs_app_csrftoken": "__Host-chatballs-app-csrf"} + + +class _Recorder: + """Внутреннее приложение: запоминает scope, до которого дошёл запрос.""" + + def __init__(self) -> None: + self.scope: dict | None = None + + async def __call__(self, scope, receive, send): + self.scope = scope + + +def _scope(*, scheme: str = "wss", cookie: str = "", origin: str = "", host: str = "app.example") -> dict: + headers = [(b"host", host.encode())] + if cookie: + headers.append((b"cookie", cookie.encode())) + if origin: + headers.append((b"origin", origin.encode())) + return {"type": "websocket", "scheme": scheme, "headers": headers} + + +def _cookies(scope: dict) -> dict[str, str]: + raw = next((value for key, value in scope["headers"] if key == b"cookie"), b"") + items = [part.strip() for part in raw.decode().split(";") if part.strip()] + return dict(item.split("=", 1) for item in items) + + +@override_settings(CHATBALLS_TLS_COOKIE_NAMES=COOKIE_NAMES) +class TlsAwareCookieASGIMiddlewareTests(SimpleTestCase): + def _run(self, scope: dict) -> dict: + inner = _Recorder() + asyncio.run(TlsAwareCookieASGIMiddleware(inner)(scope, None, None)) + assert inner.scope is not None + return inner.scope + + def test_hardened_cookie_is_delivered_under_the_plain_name(self) -> None: + scope = self._run(_scope(cookie=f"{HARDENED}=session-value")) + + self.assertEqual(_cookies(scope)[PLAIN], "session-value") + + def test_plain_cookie_alone_is_dropped_over_tls(self) -> None: + """По TLS обычное имя мы не выдаём — значит пришло оно не от нас.""" + scope = self._run(_scope(cookie=f"{PLAIN}=planted")) + + self.assertNotIn(PLAIN, _cookies(scope)) + + def test_hardened_cookie_wins_over_a_planted_plain_one(self) -> None: + scope = self._run(_scope(cookie=f"{PLAIN}=planted; {HARDENED}=real")) + + self.assertEqual(_cookies(scope)[PLAIN], "real") + + def test_plain_http_scope_is_untouched(self) -> None: + scope = self._run(_scope(scheme="ws", cookie=f"{PLAIN}=session-value")) + + self.assertEqual(_cookies(scope)[PLAIN], "session-value") + + def test_scope_without_cookies_passes_through(self) -> None: + scope = self._run(_scope()) + + self.assertEqual(_cookies(scope), {}) + + +class SameOriginWebSocketMiddlewareTests(SimpleTestCase): + def _run(self, scope: dict) -> tuple[dict | None, list[dict]]: + inner = _Recorder() + sent: list[dict] = [] + + async def send(message): + sent.append(message) + + asyncio.run(SameOriginWebSocketMiddleware(inner)(scope, None, send)) + return inner.scope, sent + + def test_same_origin_handshake_passes(self) -> None: + scope, sent = self._run(_scope(origin="https://app.example", host="app.example")) + + self.assertIsNotNone(scope) + self.assertEqual(sent, []) + + def test_foreign_origin_is_closed(self) -> None: + scope, sent = self._run(_scope(origin="https://evil.example", host="app.example")) + + self.assertIsNone(scope) + self.assertEqual(sent, [{"type": "websocket.close", "code": 4403}]) + + def test_origin_with_matching_port_passes(self) -> None: + scope, sent = self._run( + _scope(origin="http://localhost:5173", host="localhost:5173") + ) + + self.assertIsNotNone(scope) + self.assertEqual(sent, []) + + def test_client_without_origin_is_allowed(self) -> None: + """Origin шлёт браузер; клиенты без него — не то, от чего мы защищаемся.""" + scope, sent = self._run(_scope(host="app.example")) + + self.assertIsNotNone(scope) + self.assertEqual(sent, []) diff --git a/apps/backend/chatballs/http/ws_middleware.py b/apps/backend/chatballs/http/ws_middleware.py new file mode 100644 index 0000000..39281c6 --- /dev/null +++ b/apps/backend/chatballs/http/ws_middleware.py @@ -0,0 +1,117 @@ +"""ASGI-обвязка WebSocket: имена cookie по факту TLS и проверка Origin. + +HTTP-слой продукта переименовывает cookie по протоколу запроса +(``chatballs.http.middleware.TlsAwareCookieMiddleware``): по https браузер +держит ``__Host-…``, по http — обычное имя. Django-middleware на +WebSocket-хендшейк не выполняется, а Channels ищет cookie строго по +``settings.SESSION_COOKIE_NAME``. Из-за этого на установке с TLS сокет не +находил сессию вообще: браузер присылал только защищённое имя, и каждое +подключение закрывалось как неаутентифицированное — живые обновления диалогов +молча переставали работать. + +Здесь то же правило применяется к scope до ``AuthMiddlewareStack``. +""" + +from __future__ import annotations + +from collections.abc import Callable +from http.cookies import SimpleCookie +from urllib.parse import urlsplit + +from django.conf import settings + + +def _tls_cookie_pairs() -> tuple[tuple[str, str], ...]: + mapping = getattr(settings, "CHATBALLS_TLS_COOKIE_NAMES", {}) + return tuple((plain, hardened) for plain, hardened in mapping.items() if plain != hardened) + + +def _header(scope: dict, name: bytes) -> bytes: + for key, value in scope.get("headers") or (): + if key.lower() == name: + return value + return b"" + + +def _replace_header(scope: dict, name: bytes, value: bytes) -> None: + headers = [(key, item) for key, item in (scope.get("headers") or ()) if key.lower() != name] + if value: + headers.append((name, value)) + scope["headers"] = headers + + +class TlsAwareCookieASGIMiddleware: + """По wss отдаёт вглубь защищённые cookie под обычными именами.""" + + def __init__(self, inner: Callable) -> None: + self.inner = inner + + async def __call__(self, scope, receive, send): + if scope.get("type") == "websocket" and scope.get("scheme") in {"wss", "https"}: + self._rewrite(scope) + return await self.inner(scope, receive, send) + + @staticmethod + def _rewrite(scope: dict) -> None: + pairs = _tls_cookie_pairs() + if not pairs: + return + raw = _header(scope, b"cookie") + if not raw: + return + jar = SimpleCookie() + jar.load(raw.decode("latin-1")) + values = {key: morsel.value for key, morsel in jar.items()} + changed = False + for plain, hardened in pairs: + if hardened in values: + # Защищённое имя всегда сильнее обычного — то же правило, что и + # в HTTP-слое: cookie с префиксом __Host- браузер принимает + # только с самого хоста и только по TLS. + if values.get(plain) != values[hardened]: + values[plain] = values[hardened] + changed = True + elif plain in values: + # По TLS обычное имя мы не выдаём — значит пришло не от нас. + del values[plain] + changed = True + if not changed: + return + rebuilt = "; ".join(f"{key}={value}" for key, value in values.items()) + _replace_header(scope, b"cookie", rebuilt.encode("latin-1")) + + +class SameOriginWebSocketMiddleware: + """Отклоняет хендшейк, у которого Origin не совпадает с Host. + + Сокет открывает то же SPA, что и REST, поэтому Origin у него всегда наш. + Кросс-сайтовый запрос сейчас и так остаётся без cookie (``SameSite=Lax``), + но полагаться на один барьер не стоит: у cookie этот флаг настраиваемый. + Клиенты без браузера Origin не присылают — их не трогаем. + """ + + def __init__(self, inner: Callable) -> None: + self.inner = inner + + async def __call__(self, scope, receive, send): + if scope.get("type") == "websocket" and not self._allowed(scope): + await send({"type": "websocket.close", "code": 4403}) + return + return await self.inner(scope, receive, send) + + @staticmethod + def _allowed(scope: dict) -> bool: + origin = _header(scope, b"origin").decode("latin-1").strip() + if not origin: + return True + host = _header(scope, b"host").decode("latin-1").strip().lower() + if not host: + return False + origin_host = (urlsplit(origin).netloc or "").lower() + return origin_host == host + + +def websocket_boundary(inner: Callable) -> Callable: + """Обе проверки одним вызовом — порядок важен, Origin проверяется первым.""" + + return SameOriginWebSocketMiddleware(TlsAwareCookieASGIMiddleware(inner)) diff --git a/apps/backend/chatballs/identity/administration_views.py b/apps/backend/chatballs/identity/administration_views.py index 2524546..d169320 100644 --- a/apps/backend/chatballs/identity/administration_views.py +++ b/apps/backend/chatballs/identity/administration_views.py @@ -50,7 +50,6 @@ class OrganizationSettingsView(APIView): "GET": "settings.view", "PATCH": "settings.manage", } - require_organization_scope = True def get(self, request: Request) -> Response: return Response( @@ -93,7 +92,6 @@ class OrganizationLogoView(APIView): "POST": "settings.manage", "DELETE": "settings.manage", } - require_organization_scope = True def get_permissions(self): if self.request.method == "GET": @@ -173,7 +171,6 @@ class AuditListView(APIView): permission_classes = [HasCapability] required_capability = "audit.view" - require_organization_scope = True def get(self, request: Request) -> Response: organization_id = request.tenant_context.organization_id diff --git a/apps/backend/chatballs/identity/auth/password_reset.py b/apps/backend/chatballs/identity/auth/password_reset.py index 6e9d631..460c533 100644 --- a/apps/backend/chatballs/identity/auth/password_reset.py +++ b/apps/backend/chatballs/identity/auth/password_reset.py @@ -16,6 +16,7 @@ from chatballs.events.services import DomainEvent, enqueue_event from chatballs.identity.audit import record_audit_event from chatballs.identity.event_handlers import PASSWORD_RESET_REQUESTED from chatballs.identity.models import AuditResult, HumanUser +from chatballs.identity.sessions import revoke_user_sessions def _user_from_reset_link(uid: str, token: str) -> HumanUser | None: @@ -98,9 +99,15 @@ class PasswordResetConfirmView(APIView): user.must_change_password = False user.password_changed_at = timezone.now() user.save(update_fields=["password", "must_change_password", "password_changed_at"]) + # Пароль сбрасывают именно тогда, когда доступ к учётной записи мог + # оказаться у чужого. Оставить его сессии живыми — значит не сделать + # ничего: смена пароля из профиля и админский сброс их завершают, + # этот путь обязан вести себя так же. + revoked = revoke_user_sessions(user.id) record_audit_event( action="identity.password_reset_completed", actor=user, + payload={"revoked": revoked}, request=request, ) - return Response({"ok": True}) + return Response({"ok": True, "revoked": revoked}) diff --git a/apps/backend/chatballs/identity/auth/profile.py b/apps/backend/chatballs/identity/auth/profile.py index f064486..3510b07 100644 --- a/apps/backend/chatballs/identity/auth/profile.py +++ b/apps/backend/chatballs/identity/auth/profile.py @@ -115,12 +115,24 @@ class ProfilePasswordView(APIView): class ProfileTotpStartView(APIView): + """Начало настройки 2FA: выдать пользователю новый секрет. + + Выключать этим уже включённую 2FA нельзя. Иначе достаточно было бы + угнанной сессии: отключение (``ProfileTotpDisableView``) спрашивает + текущий пароль и не даёт обойти требование организации, а этот эндпоинт + молча делал ровно то же самое без единой проверки. + """ + permission_classes = [IsAuthenticated] def post(self, request: Request) -> Response: - request.user.totp_enabled = False + if request.user.totp_enabled: + return Response( + {"detail": "TOTP уже включена: сначала отключите её текущим паролем"}, + status=409, + ) request.user.totp_secret = "" - request.user.save(update_fields=["totp_enabled", "totp_secret"]) + request.user.save(update_fields=["totp_secret"]) record_audit_event( action="identity.profile_totp_setup_started", actor=request.user, diff --git a/apps/backend/chatballs/identity/crypto.py b/apps/backend/chatballs/identity/crypto.py index 7dcf078..c064b88 100644 --- a/apps/backend/chatballs/identity/crypto.py +++ b/apps/backend/chatballs/identity/crypto.py @@ -59,8 +59,34 @@ def decrypt_secret(token: str) -> str: return "" +def ciphertext_length(plaintext_chars: int) -> int: + """Сколько символов занимает Fernet-токен для строки такой длины. + + В колонке лежит не значение, а шифротекст: Fernet добавляет версию, + метку времени, IV и подпись, дополняет до блока AES и кодирует всё в + base64. Строка в 512 символов уже не помещается в varchar(512) — запись + падала бы на длинном пароле SMTP или ключе S3. Считаем по худшему случаю: + 4 байта на символ (UTF-8). + """ + payload = plaintext_chars * 4 + blocks = payload // 16 + 1 # PKCS#7 всегда добавляет хотя бы один байт + raw = 57 + 16 * blocks # 57 = версия + timestamp + IV + HMAC + return (raw + 2) // 3 * 4 # base64 без переносов + + class EncryptedCharField(models.CharField): - """CharField прозрачно шифрующий значение в БД (Fernet).""" + """CharField, прозрачно шифрующий значение в БД (Fernet). + + ``max_length`` описывает открытое значение — то, что вводит человек. Под + колонку берётся длина шифротекста: иначе ограничение поля и ограничение + столбца означают разное, и запись падает уже в базе. + """ + + def __init__(self, *args, **kwargs) -> None: + self.plaintext_max_length = kwargs.get("max_length") + if self.plaintext_max_length: + kwargs["max_length"] = ciphertext_length(self.plaintext_max_length) + super().__init__(*args, **kwargs) def from_db_value(self, value, expression, connection): # noqa: ANN001 if not value: diff --git a/apps/backend/chatballs/identity/demo_views.py b/apps/backend/chatballs/identity/demo_views.py index abf463b..bc913f9 100644 --- a/apps/backend/chatballs/identity/demo_views.py +++ b/apps/backend/chatballs/identity/demo_views.py @@ -30,7 +30,6 @@ class DemoDataView(APIView): "POST": "company.manage", "DELETE": "company.manage", } - require_organization_scope = True def get(self, request: Request) -> Response: return Response(service.demo_status(request.tenant_context.organization)) diff --git a/apps/backend/chatballs/identity/instance_settings.py b/apps/backend/chatballs/identity/instance_settings.py index ecd43b4..67f74ea 100644 --- a/apps/backend/chatballs/identity/instance_settings.py +++ b/apps/backend/chatballs/identity/instance_settings.py @@ -24,6 +24,12 @@ class InstanceSettings(models.Model): # Хост без схемы и порта: «crm.example.com» или «203.0.113.10». public_host = models.CharField(max_length=253, blank=True, default="") + # Предыдущий адрес: остаётся принятым, чтобы смена адреса не выбрасывала + # того, кто её делает. Владелец меняет адрес заранее — до того, как домен + # начал резолвиться и получил сертификат, — и сидит при этом на старом. + # Без этого он получал «Invalid host» через десять секунд после + # сохранения, а мастер уже закрыт: вернуться было бы неоткуда. + previous_public_host = models.CharField(max_length=253, blank=True, default="") # Схема, по которой установку открывают снаружи. Меняется вместе с # адресом, когда перед установкой появляется домен и сертификат. public_scheme = models.CharField(max_length=5, blank=True, default="") @@ -62,7 +68,7 @@ class InstanceSettings(models.Model): _CACHE_TTL_SECONDS = 10.0 _lock = threading.Lock() -_cached: tuple[float, str] | None = None +_cached: tuple[float, tuple[str, str]] | None = None def invalidate_cache() -> None: @@ -71,8 +77,8 @@ def invalidate_cache() -> None: _cached = None -def public_host() -> str: - """Адрес установки, запомненный мастером, или пустая строка.""" +def _hosts() -> tuple[str, str]: + """Текущий и предыдущий адрес установки (оба могут быть пустыми).""" global _cached now = time.monotonic() @@ -81,14 +87,26 @@ def public_host() -> str: return _cached[1] try: row = InstanceSettings.objects.filter(pk=InstanceSettings.SINGLETON_PK).first() - value = row.public_host if row is not None else "" + value = (row.public_host, row.previous_public_host) if row is not None else ("", "") except Exception: # таблицы ещё нет (первые миграции) - return "" + return ("", "") with _lock: _cached = (now, value) return value +def public_host() -> str: + """Адрес установки, запомненный мастером, или пустая строка.""" + + return _hosts()[0] + + +def accepted_hosts() -> tuple[str, ...]: + """Адреса, которые установка признаёт своими: текущий и предыдущий.""" + + return tuple(host for host in _hosts() if host) + + def remember_public_host(raw_host: str, scheme: str = "http") -> None: """Запомнить адрес, на котором прошли мастер, если он ещё не задан.""" diff --git a/apps/backend/chatballs/identity/instance_views.py b/apps/backend/chatballs/identity/instance_views.py index 4b263c2..b52ddd6 100644 --- a/apps/backend/chatballs/identity/instance_views.py +++ b/apps/backend/chatballs/identity/instance_views.py @@ -88,7 +88,11 @@ class InstanceAddressView(APIView): {"detail": next(iter(errors.values())), "errors": errors}, status=400 ) - fields = ["public_host", "public_scheme", "updated_at"] + fields = ["public_host", "public_scheme", "previous_public_host", "updated_at"] + if host != row.public_host: + # Прежний адрес остаётся принятым: владелец меняет адрес заранее, + # сидя на старом, и не должен выпасть из установки в тот же миг. + row.previous_public_host = row.public_host row.public_host = host row.public_scheme = scheme diff --git a/apps/backend/chatballs/identity/migrations/0033_instance_previous_public_host.py b/apps/backend/chatballs/identity/migrations/0033_instance_previous_public_host.py new file mode 100644 index 0000000..7fc44cb --- /dev/null +++ b/apps/backend/chatballs/identity/migrations/0033_instance_previous_public_host.py @@ -0,0 +1,23 @@ +"""Прежний адрес установки остаётся принятым после смены адреса. + +Владелец меняет адрес в «Настройках» заранее — до того, как новый домен начал +резолвиться и получил сертификат, — и сидит при этом на старом. Пока принятым +был только новый адрес, сохранение выбрасывало его из установки через десять +секунд (TTL кэша), а мастер первого запуска уже закрыт: вернуться было неоткуда. +""" + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("identity", "0032_remove_organization_tax_regime_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="instancesettings", + name="previous_public_host", + field=models.CharField(blank=True, default="", max_length=253), + ), + ] diff --git a/apps/backend/chatballs/identity/migrations/0034_encrypted_column_widths.py b/apps/backend/chatballs/identity/migrations/0034_encrypted_column_widths.py new file mode 100644 index 0000000..e364bf5 --- /dev/null +++ b/apps/backend/chatballs/identity/migrations/0034_encrypted_column_widths.py @@ -0,0 +1,34 @@ +"""Колонка вмещает шифротекст, а не открытое значение. + +``EncryptedCharField`` хранит Fernet-токен: версия, метка времени, IV, подпись, +дополнение до блока AES и base64 поверх всего. Значение в 512 символов +занимает почти 2.9 КБ, и в ``varchar(512)`` не помещалось — длинный пароль +SMTP или ключ S3 ронял запись уже в базе. ``max_length`` поля по-прежнему +описывает открытое значение; ширину столбца считает +``chatballs.identity.crypto.ciphertext_length``. +""" + +from django.db import migrations + +import chatballs.identity.crypto + + +class Migration(migrations.Migration): + dependencies = [ + ("identity", "0033_instance_previous_public_host"), + ] + + operations = [ + migrations.AlterField( + model_name="humanuser", + name="totp_secret", + field=chatballs.identity.crypto.EncryptedCharField(blank=True, max_length=1444), + ), + migrations.AlterField( + model_name="instancesettings", + name="email_password", + field=chatballs.identity.crypto.EncryptedCharField( + blank=True, default="", max_length=2828 + ), + ), + ] diff --git a/apps/backend/chatballs/identity/test_auth_hardening.py b/apps/backend/chatballs/identity/test_auth_hardening.py new file mode 100644 index 0000000..c0b2a86 --- /dev/null +++ b/apps/backend/chatballs/identity/test_auth_hardening.py @@ -0,0 +1,133 @@ +"""Границы, которые обходились через соседний эндпоинт. + +Два места вели себя не так, как обещает соседняя ручка того же экрана: + +- начало настройки 2FA молча выключало уже включённую, минуя и пароль, и + требование организации, — то есть было бесплатным способом снять 2FA для + того, у кого уже есть чужая сессия; +- сброс пароля по письму оставлял чужие сессии живыми, хотя пароль сбрасывают + как раз тогда, когда доступ мог оказаться у чужого. +""" + +from __future__ import annotations + +from django.contrib.auth.tokens import default_token_generator +from django.contrib.sessions.models import Session +from django.test import TestCase +from django.utils.encoding import force_bytes +from django.utils.http import urlsafe_base64_encode + +from chatballs.identity.auth.totp_utils import _generate_totp_secret +from chatballs.identity.bootstrap import bootstrap_owner +from chatballs.testing import TenantAPIClient + +PASSWORD = "Owner-Password-2026!" +NEW_PASSWORD = "Owner-Password-2027!" + + +class TotpSetupStartTests(TestCase): + def setUp(self) -> None: + self.result = bootstrap_owner(email="totp-owner@example.com", password=PASSWORD) + self.owner = self.result.owner + self.client = TenantAPIClient() + self.client.force_authenticate(self.owner) + + def test_start_issues_a_secret_while_totp_is_off(self) -> None: + self.owner.totp_secret = "OLDSECRET" + self.owner.save(update_fields=["totp_secret"]) + + response = self.client.post("/api/v1/auth/profile/totp/start/") + + self.assertEqual(response.status_code, 200, response.content) + self.owner.refresh_from_db() + self.assertEqual(self.owner.totp_secret, "") + self.assertFalse(self.owner.totp_enabled) + + def test_start_cannot_disable_enabled_totp(self) -> None: + secret = _generate_totp_secret() + self.owner.totp_secret = secret + self.owner.totp_enabled = True + self.owner.save(update_fields=["totp_secret", "totp_enabled"]) + + response = self.client.post("/api/v1/auth/profile/totp/start/") + + self.assertEqual(response.status_code, 409, response.content) + self.owner.refresh_from_db() + self.assertTrue(self.owner.totp_enabled) + self.assertEqual(self.owner.totp_secret, secret) + + def test_disable_still_requires_the_current_password(self) -> None: + self.owner.totp_secret = _generate_totp_secret() + self.owner.totp_enabled = True + self.owner.save(update_fields=["totp_secret", "totp_enabled"]) + + rejected = self.client.post( + "/api/v1/auth/profile/totp/disable/", + {"currentPassword": "wrong-password"}, + format="json", + ) + self.assertEqual(rejected.status_code, 400, rejected.content) + + accepted = self.client.post( + "/api/v1/auth/profile/totp/disable/", + {"currentPassword": PASSWORD}, + format="json", + ) + self.assertEqual(accepted.status_code, 200, accepted.content) + self.owner.refresh_from_db() + self.assertFalse(self.owner.totp_enabled) + + +class PasswordResetSessionTests(TestCase): + def setUp(self) -> None: + self.result = bootstrap_owner(email="reset-owner@example.com", password=PASSWORD) + self.owner = self.result.owner + + def _reset_link(self) -> tuple[str, str]: + # Токен считается от текущего состояния пользователя (в том числе + # last_login), поэтому берём его после всех входов. + self.owner.refresh_from_db() + return ( + urlsafe_base64_encode(force_bytes(self.owner.pk)), + default_token_generator.make_token(self.owner), + ) + + def _session_keys(self) -> set[str]: + keys = set() + for session in Session.objects.all(): + if str(session.get_decoded().get("_auth_user_id", "")) == str(self.owner.pk): + keys.add(session.session_key) + return keys + + def test_reset_revokes_existing_sessions(self) -> None: + stolen = TenantAPIClient() + self.assertTrue(stolen.login(email=self.owner.email, password=PASSWORD)) + self.assertTrue(self._session_keys()) + + uid, token = self._reset_link() + anonymous = TenantAPIClient() + response = anonymous.post( + "/api/v1/auth/password-reset/confirm/", + {"uid": uid, "token": token, "newPassword": NEW_PASSWORD}, + format="json", + ) + + self.assertEqual(response.status_code, 200, response.content) + self.assertGreaterEqual(response.json()["revoked"], 1) + self.assertEqual(self._session_keys(), set()) + + # Прежняя сессия больше не открывает приложение. + session = stolen.get("/api/v1/auth/session/") + self.assertFalse(session.json()["authenticated"]) + + def test_reset_still_sets_the_new_password(self) -> None: + uid, token = self._reset_link() + TenantAPIClient().post( + "/api/v1/auth/password-reset/confirm/", + {"uid": uid, "token": token, "newPassword": NEW_PASSWORD}, + format="json", + ) + + self.owner.refresh_from_db() + self.assertTrue(self.owner.check_password(NEW_PASSWORD)) + self.assertFalse(self.owner.must_change_password) diff --git a/apps/backend/chatballs/identity/test_crypto_columns.py b/apps/backend/chatballs/identity/test_crypto_columns.py new file mode 100644 index 0000000..87d332e --- /dev/null +++ b/apps/backend/chatballs/identity/test_crypto_columns.py @@ -0,0 +1,52 @@ +"""Шифротекст должен помещаться в колонку. + +``EncryptedCharField`` кладёт в базу Fernet-токен: версия, метка времени, IV, +подпись, дополнение до блока AES и base64 поверх всего. Значение в 512 +символов занимает почти 2.9 КБ — в varchar(512) оно не помещалось, и длинный +пароль SMTP или ключ S3 ронял запись уже в базе. +""" + +from __future__ import annotations + +from django.test import TestCase + +from chatballs.identity.crypto import ciphertext_length, decrypt_secret, encrypt_secret +from chatballs.identity.instance_settings import InstanceSettings +from chatballs.tenancy.storage_settings import StorageSettings + + +class CiphertextLengthTests(TestCase): + def test_estimate_covers_the_real_token(self) -> None: + for plaintext_chars in (1, 16, 255, 512, 1024): + value = "щ" * plaintext_chars # 2 байта на символ в UTF-8 + self.assertLessEqual( + len(encrypt_secret(value)), + ciphertext_length(plaintext_chars), + f"оценка мала для {plaintext_chars} символов", + ) + + def test_round_trip_survives(self) -> None: + value = "п" * 400 + self.assertEqual(decrypt_secret(encrypt_secret(value)), value) + + +class EncryptedColumnWidthTests(TestCase): + def test_long_smtp_password_is_stored(self) -> None: + password = "Пароль-" + "x" * 500 + row = InstanceSettings.load() + row.email_host = "smtp.example.test" + row.email_password = password + row.save(update_fields=["email_host", "email_password", "updated_at"]) + + row.refresh_from_db() + self.assertEqual(row.email_password, password) + + def test_long_s3_keys_are_stored(self) -> None: + secret = "S" * 500 + row = StorageSettings.load() + row.s3_access_key = "A" * 500 + row.s3_secret_key = secret + row.save(update_fields=["s3_access_key", "s3_secret_key"]) + + row.refresh_from_db() + self.assertEqual(row.s3_secret_key, secret) diff --git a/apps/backend/chatballs/identity/test_instance_address.py b/apps/backend/chatballs/identity/test_instance_address.py new file mode 100644 index 0000000..a02ca7c --- /dev/null +++ b/apps/backend/chatballs/identity/test_instance_address.py @@ -0,0 +1,121 @@ +"""Адрес установки: смена не должна выбрасывать того, кто её делает. + +Владелец меняет адрес заранее — до того, как новый домен начал резолвиться и +получил сертификат, — и сидит при этом на старом. Пока принятым был только +новый адрес, сохранение отвечало «Invalid host» через десять секунд (TTL +кэша), а мастер первого запуска уже закрыт: вернуться было неоткуда. + +Здесь же — запрет вешать портал помощи на адрес самой установки: SPA решает, +что рисовать, по ответу ``/api/v1/help/``, и такой портал подменял бы +сотрудникам приложение своим Help Center. +""" + +from __future__ import annotations + +from django.core.exceptions import ValidationError +from django.test import TestCase + +from chatballs.identity.bootstrap import bootstrap_owner +from chatballs.identity.instance_settings import ( + InstanceSettings, + accepted_hosts, + invalidate_cache, +) +from chatballs.support_portals.models import SupportPortal +from chatballs.testing import TenantAPIClient + +PASSWORD = "Owner-Password-2026!" + + +class InstanceAddressChangeTests(TestCase): + def setUp(self) -> None: + self.result = bootstrap_owner(email="address-owner@example.com", password=PASSWORD) + self.client = TenantAPIClient() + self.client.force_authenticate(self.result.owner) + row = InstanceSettings.load() + row.public_host = "203.0.113.10" + row.public_scheme = "http" + row.previous_public_host = "" + row.save(update_fields=["public_host", "public_scheme", "previous_public_host", "updated_at"]) + invalidate_cache() + self.addCleanup(invalidate_cache) + + def _patch(self, host: str, scheme: str = "https"): + return self.client.patch( + "/api/v1/company/administration/instance/", + {"publicHost": host, "publicScheme": scheme}, + format="json", + ) + + def test_previous_address_stays_accepted(self) -> None: + response = self._patch("crm.example.test") + + self.assertEqual(response.status_code, 200, response.content) + invalidate_cache() + self.assertEqual(set(accepted_hosts()), {"crm.example.test", "203.0.113.10"}) + + def test_old_address_still_answers_after_the_change(self) -> None: + self._patch("crm.example.test") + invalidate_cache() + + response = self.client.get("/api/v1/auth/session/", HTTP_HOST="203.0.113.10") + + self.assertEqual(response.status_code, 200, response.content) + + def test_stranger_host_is_still_rejected(self) -> None: + self._patch("crm.example.test") + invalidate_cache() + + response = self.client.get("/api/v1/auth/session/", HTTP_HOST="evil.example") + + self.assertEqual(response.status_code, 400) + + def test_only_one_previous_address_is_kept(self) -> None: + self._patch("first.example.test") + self._patch("second.example.test") + invalidate_cache() + + self.assertEqual(set(accepted_hosts()), {"second.example.test", "first.example.test"}) + + def test_saving_the_same_address_does_not_shift_history(self) -> None: + self._patch("crm.example.test") + self._patch("crm.example.test") + invalidate_cache() + + self.assertEqual(set(accepted_hosts()), {"crm.example.test", "203.0.113.10"}) + + +class PortalDomainCollisionTests(TestCase): + def setUp(self) -> None: + self.result = bootstrap_owner(email="portal-owner@example.com", password=PASSWORD) + row = InstanceSettings.load() + row.public_host = "crm.example.test" + row.previous_public_host = "203.0.113.10" + row.save(update_fields=["public_host", "previous_public_host", "updated_at"]) + invalidate_cache() + self.addCleanup(invalidate_cache) + + def _portal(self, custom_domain: str) -> SupportPortal: + return SupportPortal( + organization=self.result.organization, + slug="help", + name="Help", + custom_domain=custom_domain, + ) + + def test_installation_address_cannot_become_a_portal_domain(self) -> None: + with self.assertRaises(ValidationError) as error: + self._portal("crm.example.test").clean() + + self.assertIn("custom_domain", error.exception.message_dict) + + def test_previous_installation_address_is_also_refused(self) -> None: + with self.assertRaises(ValidationError): + self._portal("203.0.113.10").clean() + + def test_unrelated_domain_is_allowed(self) -> None: + portal = self._portal("help.example.test") + + portal.clean() # не должно бросать + + self.assertEqual(portal.custom_domain, "help.example.test") diff --git a/apps/backend/chatballs/integrations/migrations/0008_encrypted_column_width.py b/apps/backend/chatballs/integrations/migrations/0008_encrypted_column_width.py new file mode 100644 index 0000000..6d63900 --- /dev/null +++ b/apps/backend/chatballs/integrations/migrations/0008_encrypted_column_width.py @@ -0,0 +1,19 @@ +"""Секрет интеграции: колонка под шифротекст (см. identity.0034).""" + +from django.db import migrations + +import chatballs.identity.crypto + + +class Migration(migrations.Migration): + dependencies = [ + ("integrations", "0007_integration_feature_flags"), + ] + + operations = [ + migrations.AlterField( + model_name="integration", + name="secret", + field=chatballs.identity.crypto.EncryptedCharField(blank=True, max_length=5560), + ), + ] diff --git a/apps/backend/chatballs/integrations/proxy.py b/apps/backend/chatballs/integrations/proxy.py index 2fa8e95..214dc27 100644 --- a/apps/backend/chatballs/integrations/proxy.py +++ b/apps/backend/chatballs/integrations/proxy.py @@ -38,6 +38,40 @@ class _RefusedDataHandler(urllib.request.DataHandler): raise OutboundUrlRejected("Схема data: в исходящих запросах запрещена") +class _GuardedRedirectHandler(urllib.request.HTTPRedirectHandler): + """Проверяет каждый Location, а не только исходный адрес. + + Политика исходящих (``integrations.outbound``) проверяет адрес до запроса. + Но urllib сам ходит по редиректам, и ответ подставного провайдера мог + вернуть 302 на ``http://169.254.169.254/…`` — проверку прошёл один адрес, + а сходили по другому. + + Проверка стоит в ``http_error_302``, а не только в ``redirect_request``: + свою проверку схемы urllib делает раньше и отвечает на неё ``HTTPError``, + из-за чего запрет выглядел бы сетевой ошибкой, а не отказом политики. + """ + + def __init__(self, validate) -> None: + self._validate = validate + + def http_error_302(self, req, fp, code, msg, headers): # noqa: ANN001 + location = headers.get("location") or headers.get("uri") or "" + if location: + self._validate(urllib.parse.urljoin(req.full_url, location)) + return super().http_error_302(req, fp, code, msg, headers) + + # urllib связывает остальные коды с базовым методом на этапе создания + # класса, поэтому переопределения одного http_error_302 мало. + http_error_301 = http_error_302 + http_error_303 = http_error_302 + http_error_307 = http_error_302 + http_error_308 = http_error_302 + + def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: ANN001 + self._validate(newurl) + return super().redirect_request(req, fp, code, msg, headers, newurl) + + def _blocked_scheme_handlers() -> list[urllib.request.BaseHandler]: """Заглушки вместо file/ftp/data. @@ -50,9 +84,16 @@ def _blocked_scheme_handlers() -> list[urllib.request.BaseHandler]: return [_RefusedFileHandler(), _RefusedFTPHandler(), _RefusedDataHandler()] -def build_opener(proxy_url: str): - """urllib opener, проксирующий http/https/socks5 запросы. Пустой proxy_url → без прокси.""" +def build_opener(proxy_url: str, *, validate_redirect=None): + """urllib opener, проксирующий http/https/socks5 запросы. + + Пустой ``proxy_url`` → без прокси. ``validate_redirect`` — проверка адреса, + на который ответ просит перейти: её передают там, где сам адрес пришёл + данными от провайдера, а не из настроек подключения. + """ blocked = _blocked_scheme_handlers() + if validate_redirect is not None: + blocked.append(_GuardedRedirectHandler(validate_redirect)) if not proxy_url: return urllib.request.build_opener(*blocked) scheme = urllib.parse.urlparse(proxy_url).scheme.lower() diff --git a/apps/backend/chatballs/integrations/serializers.py b/apps/backend/chatballs/integrations/serializers.py index ef1cbd9..b5cb8ec 100644 --- a/apps/backend/chatballs/integrations/serializers.py +++ b/apps/backend/chatballs/integrations/serializers.py @@ -1,5 +1,50 @@ +from urllib.parse import urlsplit, urlunsplit + from chatballs.integrations.models import Integration +# Пароль прокси наружу не отдаётся: в списке подключений его видел бы каждый, +# у кого есть право смотреть интеграции, а сам адрес попадал бы в логи и +# историю браузера вместе с ним. Пустое поле при сохранении означает +# «оставить прежний» — ровно как у секрета интеграции. +PROXY_PASSWORD_MASK = "••••••••" + + +def mask_proxy_url(url: str) -> str: + """Адрес прокси без пароля: «socks5://user:••••••••@host:1080».""" + if not url: + return "" + parsed = urlsplit(url) + if not parsed.password: + return url + host = parsed.hostname or "" + if parsed.port: + host = f"{host}:{parsed.port}" + userinfo = f"{parsed.username or ''}:{PROXY_PASSWORD_MASK}" + return urlunsplit( + (parsed.scheme, f"{userinfo}@{host}", parsed.path, parsed.query, parsed.fragment) + ) + + +def restore_proxy_password(submitted: str, stored: str) -> str: + """Вернуть сохранённый пароль, если пришла маска того же прокси. + + Форма отправляет конфигурацию целиком, поэтому без этого замаскированное + значение сохранилось бы вместо настоящего пароля и прокси перестал бы + работать при первом же редактировании соседнего поля. + """ + if not submitted or PROXY_PASSWORD_MASK not in submitted or not stored: + return submitted + new, old = urlsplit(submitted), urlsplit(stored) + same_target = ( + new.scheme == old.scheme + and (new.hostname or "") == (old.hostname or "") + and new.port == old.port + and (new.username or "") == (old.username or "") + ) + if not same_target or not old.password: + return submitted + return stored + def secret_mask(secret: str) -> str: """Маска секрета для колонки «Секрет» (кадр N3): только публичный префикс @@ -26,7 +71,7 @@ def integration_payload(integration: Integration) -> dict[str, object]: "baseUrl": integration.config.get("base_url", ""), "defaultModel": integration.config.get("default_model", ""), "transcriptionModel": integration.config.get("transcription_model", ""), - "proxyUrl": integration.config.get("proxy_url", ""), + "proxyUrl": mask_proxy_url(str(integration.config.get("proxy_url", ""))), "botId": integration.config.get("bot_id", ""), "botUsername": integration.config.get("bot_username", ""), "botName": integration.config.get("bot_name", ""), diff --git a/apps/backend/chatballs/integrations/test_outbound_redirects.py b/apps/backend/chatballs/integrations/test_outbound_redirects.py new file mode 100644 index 0000000..39111f7 --- /dev/null +++ b/apps/backend/chatballs/integrations/test_outbound_redirects.py @@ -0,0 +1,75 @@ +"""Редирект не должен уводить скачивание туда, куда исходный адрес не пустили. + +Политика исходящих проверяет адрес до запроса, но urllib сам ходит по +редиректам: ответ подставного провайдера возвращал 302 на внутренний адрес, и +хаб шёл туда своими руками. Проверка теперь висит на каждом Location. +""" + +from __future__ import annotations + +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer + +from django.test import SimpleTestCase + +from chatballs.conversations.transports.base import download_bytes +from chatballs.integrations.outbound import OutboundUrlRejected + +PAYLOAD = b"file-content" + + +class _Handler(BaseHTTPRequestHandler): + redirect_to = "" + + def do_GET(self): # noqa: N802 + if self.path == "/file": + self.send_response(200) + self.send_header("Content-Length", str(len(PAYLOAD))) + self.end_headers() + self.wfile.write(PAYLOAD) + return + self.send_response(302) + self.send_header("Location", type(self).redirect_to) + self.end_headers() + + def log_message(self, *args): # тишина в выводе тестов + return + + +class OutboundRedirectTests(SimpleTestCase): + def setUp(self) -> None: + self.server = HTTPServer(("127.0.0.1", 0), _Handler) + self.host, self.port = self.server.server_address + thread = threading.Thread(target=self.server.serve_forever, daemon=True) + thread.start() + self.addCleanup(self.server.shutdown) + self.addCleanup(self.server.server_close) + # Хост из base_url подключения владелец назвал сам — он разрешён даже + # будучи внутренним. Именно так провайдер и живёт у self-hosted. + self.allowed_host = "127.0.0.1" + self.base = f"http://127.0.0.1:{self.port}" + + def test_direct_download_from_the_configured_host_works(self) -> None: + data = download_bytes(f"{self.base}/file", allowed_host=self.allowed_host) + + self.assertEqual(data, PAYLOAD) + + def test_redirect_inside_the_configured_host_is_followed(self) -> None: + _Handler.redirect_to = f"{self.base}/file" + + data = download_bytes(f"{self.base}/start", allowed_host=self.allowed_host) + + self.assertEqual(data, PAYLOAD) + + def test_redirect_to_a_foreign_internal_address_is_rejected(self) -> None: + # Классическая цель SSRF: метаданные облачной машины. + _Handler.redirect_to = "http://169.254.169.254/latest/meta-data/" + + with self.assertRaises(OutboundUrlRejected): + download_bytes(f"{self.base}/start", allowed_host=self.allowed_host) + + def test_redirect_to_a_forbidden_scheme_is_rejected(self) -> None: + _Handler.redirect_to = "file:///run/chatballs/secrets/secret_key" + + with self.assertRaises(OutboundUrlRejected): + download_bytes(f"{self.base}/start", allowed_host=self.allowed_host) diff --git a/apps/backend/chatballs/integrations/test_proxy_masking.py b/apps/backend/chatballs/integrations/test_proxy_masking.py new file mode 100644 index 0000000..0c4ee1f --- /dev/null +++ b/apps/backend/chatballs/integrations/test_proxy_masking.py @@ -0,0 +1,108 @@ +"""Пароль прокси не уходит наружу и не теряется при редактировании. + +Секрет интеграции в ответе маскируется, а адрес прокси отдавался целиком — +вместе с ``user:pass@``. Его видит каждый, у кого есть право смотреть +интеграции, и он же оседает в логах и истории браузера. + +Маскировать мало: форма отправляет конфигурацию целиком, поэтому замаскированное +значение сохранилось бы вместо настоящего пароля при правке соседнего поля. +""" + +from __future__ import annotations + +from django.test import TestCase + +from chatballs.identity.bootstrap import bootstrap_owner +from chatballs.integrations.models import IntegrationKind, IntegrationProvider +from chatballs.integrations.serializers import ( + PROXY_PASSWORD_MASK, + integration_payload, + mask_proxy_url, + restore_proxy_password, +) +from chatballs.integrations.services import IntegrationInput, create_integration, update_integration +from chatballs.testing import system_tenant_context + +PROXY = "socks5://proxy-user:s3cr3t-pass@proxy.example:1080" + + +class ProxyMaskTests(TestCase): + def test_password_is_replaced_and_the_rest_is_kept(self) -> None: + masked = mask_proxy_url(PROXY) + + self.assertNotIn("s3cr3t-pass", masked) + self.assertIn("proxy-user", masked) + self.assertIn("proxy.example:1080", masked) + self.assertIn(PROXY_PASSWORD_MASK, masked) + + def test_proxy_without_password_is_untouched(self) -> None: + self.assertEqual( + mask_proxy_url("http://proxy.example:3128"), "http://proxy.example:3128" + ) + + def test_empty_stays_empty(self) -> None: + self.assertEqual(mask_proxy_url(""), "") + + def test_mask_of_the_same_proxy_restores_the_stored_password(self) -> None: + self.assertEqual(restore_proxy_password(mask_proxy_url(PROXY), PROXY), PROXY) + + def test_mask_of_another_host_is_not_restored(self) -> None: + submitted = mask_proxy_url("socks5://proxy-user:other@elsewhere.example:1080") + + self.assertEqual(restore_proxy_password(submitted, PROXY), submitted) + + def test_new_password_replaces_the_stored_one(self) -> None: + submitted = "socks5://proxy-user:brand-new@proxy.example:1080" + + self.assertEqual(restore_proxy_password(submitted, PROXY), submitted) + + +class ProxyPayloadTests(TestCase): + def setUp(self) -> None: + result = bootstrap_owner(email="proxy-owner@example.com", password="Owner-Password-2026!") + self.context = system_tenant_context(result.organization) + self.integration = create_integration( + context=self.context, + data=IntegrationInput( + provider=IntegrationProvider.OPENROUTER, + name="LLM", + secret="sk-or-v1-secret-value", + config={"baseUrl": "https://openrouter.ai/api/v1", "proxyUrl": PROXY}, + ), + ) + + def test_payload_never_carries_the_proxy_password(self) -> None: + payload = integration_payload(self.integration) + + self.assertNotIn("s3cr3t-pass", str(payload)) + self.assertIn(PROXY_PASSWORD_MASK, payload["config"]["proxyUrl"]) + + def test_stored_value_keeps_the_real_password(self) -> None: + self.integration.refresh_from_db() + + self.assertEqual(self.integration.config["proxy_url"], PROXY) + + def test_editing_another_field_keeps_the_proxy_password(self) -> None: + """Форма возвращает то, что ей отдали, — включая маску.""" + from chatballs.integrations.views import _keep_proxy_password + + submitted = { + "baseUrl": "https://openrouter.ai/api/v1", + "proxyUrl": integration_payload(self.integration)["config"]["proxyUrl"], + "defaultModel": "openai/gpt-4o-mini", + } + restored = _keep_proxy_password(submitted, self.integration) + + updated = update_integration( + context=self.context, + integration=self.integration, + data=IntegrationInput( + provider=IntegrationProvider.OPENROUTER, + name="LLM", + secret=None, + config=restored, + ), + ) + + self.assertEqual(updated.config["proxy_url"], PROXY) + self.assertEqual(updated.kind, IntegrationKind.LLM_PROVIDER) diff --git a/apps/backend/chatballs/integrations/views.py b/apps/backend/chatballs/integrations/views.py index ff8992d..84bc8de 100644 --- a/apps/backend/chatballs/integrations/views.py +++ b/apps/backend/chatballs/integrations/views.py @@ -11,7 +11,7 @@ from chatballs.integrations.selectors import ( integration_for_context, integrations_for_context, ) -from chatballs.integrations.serializers import integration_payload +from chatballs.integrations.serializers import integration_payload, restore_proxy_password from chatballs.integrations.services import ( IntegrationInput, create_integration, @@ -23,6 +23,7 @@ from chatballs.integrations.services import ( def _input(body: dict[str, object], *, current: Integration | None = None) -> IntegrationInput: config = body.get("config", current.config if current else {}) + config = _keep_proxy_password(config, current) raw_channel = body.get("channelId", current.channel_id if current else None) channel_id = int(raw_channel) if isinstance(raw_channel, int) or (isinstance(raw_channel, str) and raw_channel.isdigit()) else None return IntegrationInput( @@ -39,6 +40,22 @@ def _input(body: dict[str, object], *, current: Integration | None = None) -> In ) +def _keep_proxy_password(config: object, current: Integration | None) -> object: + """Маска пароля прокси из ответа не должна затирать настоящий пароль.""" + if not isinstance(config, dict) or current is None: + return config + submitted = str(config.get("proxyUrl", config.get("proxy_url", "")) or "") + if not submitted: + return config + restored = restore_proxy_password(submitted, str(current.config.get("proxy_url", ""))) + if restored == submitted: + return config + config = dict(config) + config.pop("proxy_url", None) + config["proxyUrl"] = restored + return config + + def _validation_error(error: Exception) -> Response: if isinstance(error, ValidationError): if hasattr(error, "message_dict"): @@ -64,7 +81,6 @@ def _audit(request: Request, action: str, integration: Integration) -> None: class IntegrationListView(APIView): permission_classes = [HasCapability] required_capabilities = {"GET": "integrations.view", "POST": "integrations.manage"} - require_organization_scope = True def get(self, request: Request) -> Response: items = integrations_for_context(request.tenant_context) @@ -84,7 +100,6 @@ class IntegrationListView(APIView): class IntegrationDetailView(APIView): permission_classes = [HasCapability] required_capability = "integrations.manage" - require_organization_scope = True def _get(self, request: Request, integration_id: int) -> Integration: return integration_for_context( @@ -119,7 +134,6 @@ class IntegrationDetailView(APIView): class IntegrationTestView(APIView): permission_classes = [HasCapability] required_capability = "integrations.manage" - require_organization_scope = True def post(self, request: Request, integration_id: int) -> Response: try: diff --git a/apps/backend/chatballs/support_portals/addressing.py b/apps/backend/chatballs/support_portals/addressing.py index 1383e32..cee37aa 100644 --- a/apps/backend/chatballs/support_portals/addressing.py +++ b/apps/backend/chatballs/support_portals/addressing.py @@ -24,6 +24,16 @@ def validate_domain(value: str) -> str: return domain +def _installation_hosts() -> tuple[str, ...]: + """Адреса установки; пусто, если таблицы настроек ещё нет (ранние миграции).""" + from chatballs.identity.instance_settings import accepted_hosts + + try: + return accepted_hosts() + except Exception: + return () + + def hosted_domain(portal_key: str) -> str: base_domain = normalize_domain(settings.CHATBALLS_HELP_BASE_DOMAIN) return validate_domain(f"{portal_key}.{base_domain}") @@ -40,6 +50,13 @@ def clean_portal_domains(portal) -> None: for value in getattr(settings, "CHATBALLS_APP_PRIMARY_HOSTS", []) if value and not value.startswith(".") } + # Адрес самой установки — тоже адрес приложения, хотя в статическом + # списке его нет: он живёт в настройках. Портал, повешенный на него, + # подменял бы сотрудникам приложение своим Help Center: SPA пробует + # /api/v1/help/ и по ответу решает, что рисовать. + application_hosts |= { + normalize_domain(host) for host in _installation_hosts() if host + } if portal.custom_domain in application_hosts: raise ValidationError( {"custom_domain": "Домен внутреннего приложения использовать нельзя"} diff --git a/apps/backend/chatballs/support_portals/domain_services.py b/apps/backend/chatballs/support_portals/domain_services.py index ee9a246..cbd9291 100644 --- a/apps/backend/chatballs/support_portals/domain_services.py +++ b/apps/backend/chatballs/support_portals/domain_services.py @@ -2,10 +2,10 @@ from __future__ import annotations import dns.exception import dns.resolver -from django.conf import settings from django.core.exceptions import ValidationError from django.utils import timezone +from chatballs.support_portals.public_address import help_public_ipv4 from chatballs.support_portals.addressing import normalize_domain, validate_domain from chatballs.support_portals.models import SupportPortal from chatballs.support_portals.statuses import PortalStatus @@ -37,7 +37,8 @@ def verify_custom_domain(portal: SupportPortal) -> SupportPortal: # и установка принадлежат одному владельцу (README дизайн-базлайна, решение # 6). Остаётся техническая проверка «ведёт ли домен на этот сервер» — она # нужна, чтобы выписать сертификат. - if settings.CHATBALLS_HELP_PUBLIC_IPV4: + server_ipv4 = help_public_ipv4() + if server_ipv4: try: address_answers = dns.resolver.resolve(portal.custom_domain, "A") addresses = { @@ -53,7 +54,7 @@ def verify_custom_domain(portal: SupportPortal) -> SupportPortal: raise ValidationError( {"customDomain": "A-запись домена пока не найдена"} ) from error - if settings.CHATBALLS_HELP_PUBLIC_IPV4 not in addresses: + if server_ipv4 not in addresses: raise ValidationError( {"customDomain": "A-запись домена указывает не на сервер Chatballs"} ) diff --git a/apps/backend/chatballs/support_portals/gateway_views.py b/apps/backend/chatballs/support_portals/gateway_views.py index b9cc657..f262fb3 100644 --- a/apps/backend/chatballs/support_portals/gateway_views.py +++ b/apps/backend/chatballs/support_portals/gateway_views.py @@ -3,18 +3,35 @@ from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView +from chatballs.identity.instance_settings import accepted_hosts from chatballs.support_portals.addressing import normalize_domain from chatballs.tenancy.ingress import support_portal_route class HelpDomainAuthorizationView(APIView): - """Caddy on-demand TLS authorization for published portal hosts.""" + """Caddy on-demand TLS authorization. + + Шлюз спрашивает разрешение перед выпуском сертификата на каждый новый хост. + Разрешены два вида адресов, и оба человек задал сам в интерфейсе: + + - адрес самой установки (мастер первого запуска запомнил его, владелец + меняет в «Настройках») — без этого коробка навсегда оставалась бы на + http: свой домен ей выписать было нечем; + - домены опубликованных порталов помощи. + + Всё остальное — 404, иначе любой указавший на нас домен заставлял бы + установку ходить в ACME. + """ authentication_classes: list = [] permission_classes = [AllowAny] def get(self, request: Request) -> Response: domain = normalize_domain(str(request.query_params.get("domain", ""))) - if not domain or support_portal_route(domain) is None: + if not domain: + return Response(status=404) + if domain in {normalize_domain(item) for item in accepted_hosts()}: + return Response(status=204) + if support_portal_route(domain) is None: return Response(status=404) return Response(status=204) diff --git a/apps/backend/chatballs/support_portals/host_boundary.py b/apps/backend/chatballs/support_portals/host_boundary.py index ab10c56..0a0241a 100644 --- a/apps/backend/chatballs/support_portals/host_boundary.py +++ b/apps/backend/chatballs/support_portals/host_boundary.py @@ -39,12 +39,13 @@ class SupportPortalHostBoundaryMiddleware: or str(connections["default"].settings_dict["NAME"]).startswith("test_") ): return True - # Адрес, который человек ввёл в браузере на первом запуске: продукт - # запомнил его в настройках установки и признаёт своим. - from chatballs.identity.instance_settings import public_host + # Адреса, которые человек задал сам: тот, на котором прошли мастер, и + # предыдущий — чтобы смена адреса в «Настройках» не выбрасывала того, + # кто её делает, до того как новый домен вообще заработал. + from chatballs.identity.instance_settings import accepted_hosts try: - if host and host == public_host(): + if host and host in {normalize_domain(item) for item in accepted_hosts()}: return True except Exception: pass diff --git a/apps/backend/chatballs/support_portals/public_address.py b/apps/backend/chatballs/support_portals/public_address.py new file mode 100644 index 0000000..336afec --- /dev/null +++ b/apps/backend/chatballs/support_portals/public_address.py @@ -0,0 +1,84 @@ +"""Адрес, на который владелец направляет A-запись домена портала. + +Раньше это была переменная окружения с обязательной проверкой при старте, а у +коробки её задавать негде — поэтому значение по умолчанию схлопывалось в +``127.0.0.1``, и продукт печатал владельцу инструкцию «направьте домен на +loopback». Теперь источник тот же, что и у всего остального адресного: адрес +самой установки, который человек ввёл в мастере первого запуска и меняет в +«Настройках». + +Порядок: явная переменная окружения (контуры, которые ведут конфигурацию +сами) → адрес установки, если это IPv4 → его A-запись, если это домен. Пусто +— значит показывать нечего: до мастера адреса ещё нет. +""" + +from __future__ import annotations + +import threading +import time +from ipaddress import IPv4Address + +from django.conf import settings + +_CACHE_TTL_SECONDS = 60.0 +_lock = threading.Lock() +_cached: tuple[float, str] | None = None + + +def invalidate_cache() -> None: + global _cached + with _lock: + _cached = None + + +def _as_ipv4(value: str) -> str: + try: + return str(IPv4Address(value.strip())) + except ValueError: + return "" + + +def _resolve_a_record(hostname: str) -> str: + import dns.exception + import dns.resolver + + try: + answers = dns.resolver.resolve(hostname, "A") + except ( + dns.resolver.NoAnswer, + dns.resolver.NXDOMAIN, + dns.resolver.NoNameservers, + dns.exception.Timeout, + ): + return "" + for answer in answers: + address = _as_ipv4(getattr(answer, "address", str(answer).rstrip("."))) + if address: + return address + return "" + + +def help_public_ipv4() -> str: + """IPv4 установки для инструкции «направьте A-запись сюда»; пусто — нечего показать.""" + + configured = str(getattr(settings, "CHATBALLS_HELP_PUBLIC_IPV4", "") or "") + if configured: + return configured + + global _cached + now = time.monotonic() + with _lock: + if _cached is not None and now - _cached[0] < _CACHE_TTL_SECONDS: + return _cached[1] + + from chatballs.identity.instance_settings import public_host + + try: + host = public_host() + except Exception: # таблицы ещё нет (ранние миграции) + return "" + address = _as_ipv4(host) or (_resolve_a_record(host) if host else "") + + with _lock: + _cached = (now, address) + return address diff --git a/apps/backend/chatballs/support_portals/serializers.py b/apps/backend/chatballs/support_portals/serializers.py index b5e2e12..d9e2b01 100644 --- a/apps/backend/chatballs/support_portals/serializers.py +++ b/apps/backend/chatballs/support_portals/serializers.py @@ -1,8 +1,8 @@ -from django.conf import settings from chatballs.integrations.models import IntegrationProvider, IntegrationStatus from chatballs.support_portals.addressing import portal_public_url from chatballs.support_portals.content_markdown import normalize_file_links +from chatballs.support_portals.public_address import help_public_ipv4 from chatballs.support_portals.models import ( PortalArticle, PortalArticleFile, @@ -13,6 +13,7 @@ from chatballs.support_portals.models import ( def portal_payload(portal: SupportPortal, *, counts: dict | None = None) -> dict: + server_ipv4 = help_public_ipv4() if portal.custom_domain else "" public_url = portal_public_url( hosted=portal.hosted_domain, custom=portal.custom_domain, @@ -28,9 +29,9 @@ def portal_payload(portal: SupportPortal, *, counts: dict | None = None) -> dict { "name": portal.custom_domain, "type": "A", - "value": settings.CHATBALLS_HELP_PUBLIC_IPV4, + "value": server_ipv4, } - if portal.custom_domain and settings.CHATBALLS_HELP_PUBLIC_IPV4 + if portal.custom_domain and server_ipv4 else None ), "customDomainVerifiedAt": portal.custom_domain_verified_at, diff --git a/apps/backend/chatballs/support_portals/tests/test_public_address.py b/apps/backend/chatballs/support_portals/tests/test_public_address.py new file mode 100644 index 0000000..bb83277 --- /dev/null +++ b/apps/backend/chatballs/support_portals/tests/test_public_address.py @@ -0,0 +1,55 @@ +"""IPv4 для A-записи домена портала берётся из адреса установки. + +Раньше это была переменная окружения с обязательной проверкой при старте, а у +коробки её задавать негде — значение по умолчанию схлопывалось в 127.0.0.1, и +продукт печатал владельцу инструкцию «направьте домен на loopback». +""" + +from __future__ import annotations + +from django.test import TestCase, override_settings + +from chatballs.identity.instance_settings import InstanceSettings, invalidate_cache +from chatballs.support_portals import public_address + + +class HelpPublicIpv4Tests(TestCase): + def setUp(self) -> None: + public_address.invalidate_cache() + invalidate_cache() + self.addCleanup(public_address.invalidate_cache) + self.addCleanup(invalidate_cache) + + def _set_host(self, host: str) -> None: + row = InstanceSettings.load() + row.public_host = host + row.save(update_fields=["public_host", "updated_at"]) + invalidate_cache() + public_address.invalidate_cache() + + @override_settings(CHATBALLS_HELP_PUBLIC_IPV4="") + def test_ip_installation_address_is_used_as_is(self) -> None: + self._set_host("203.0.113.10") + + self.assertEqual(public_address.help_public_ipv4(), "203.0.113.10") + + @override_settings(CHATBALLS_HELP_PUBLIC_IPV4="") + def test_no_address_yet_means_nothing_to_show(self) -> None: + self._set_host("") + + self.assertEqual(public_address.help_public_ipv4(), "") + + @override_settings(CHATBALLS_HELP_PUBLIC_IPV4="") + def test_domain_installation_address_is_resolved(self) -> None: + self._set_host("crm.example.test") + original = public_address._resolve_a_record + public_address._resolve_a_record = lambda host: "198.51.100.7" + self.addCleanup(setattr, public_address, "_resolve_a_record", original) + + self.assertEqual(public_address.help_public_ipv4(), "198.51.100.7") + + @override_settings(CHATBALLS_HELP_PUBLIC_IPV4="192.0.2.5") + def test_explicit_setting_wins(self) -> None: + self._set_host("203.0.113.10") + + self.assertEqual(public_address.help_public_ipv4(), "192.0.2.5") diff --git a/apps/backend/chatballs/support_portals/tests/test_public_api.py b/apps/backend/chatballs/support_portals/tests/test_public_api.py index 92e4834..bb83ef7 100644 --- a/apps/backend/chatballs/support_portals/tests/test_public_api.py +++ b/apps/backend/chatballs/support_portals/tests/test_public_api.py @@ -3,6 +3,7 @@ from django.test import TestCase, override_settings from django.utils import timezone from chatballs.identity.bootstrap import bootstrap_owner +from chatballs.identity.instance_settings import InstanceSettings, invalidate_cache from chatballs.channels.models import Channel from chatballs.support_portals.models import PortalArticleFeedback, SupportPortal from chatballs.testing import TenantAPIClient @@ -205,6 +206,39 @@ class PublicSupportPortalTests(TestCase): sorted([attached_doc["name"], attached_image["name"]]), ) + @override_settings(ROOT_URLCONF="chatballs_backend.urls_platform") + def test_gateway_authorizes_the_installation_address(self) -> None: + """Свой домен установки шлюз обязан уметь закрыть сертификатом. + + Адрес коробка знает только от человека: мастер первого запуска + запомнил, на чём его открыли, владелец меняет это в «Настройках». + Пока ask-эндпоинт отвечал 404 на всё, кроме порталов, установка + оставалась на http навсегда — выписать сертификат было нечем. + """ + invalidate_cache() + unknown = self.client.get( + "/api/v1/gateway/help-domain/", {"domain": "crm.example.test"} + ) + self.assertEqual(unknown.status_code, 404, unknown.content) + + row = InstanceSettings.load() + row.public_host = "crm.example.test" + row.public_scheme = "https" + row.save(update_fields=["public_host", "public_scheme", "updated_at"]) + invalidate_cache() + + allowed = self.client.get( + "/api/v1/gateway/help-domain/", {"domain": "CRM.example.test." } + ) + stranger = self.client.get( + "/api/v1/gateway/help-domain/", {"domain": "someone-else.example"} + ) + empty = self.client.get("/api/v1/gateway/help-domain/") + + self.assertEqual(allowed.status_code, 204, allowed.content) + self.assertEqual(stranger.status_code, 404, stranger.content) + self.assertEqual(empty.status_code, 404, empty.content) + @override_settings(ROOT_URLCONF="chatballs_backend.urls_platform") def test_gateway_authorizes_only_published_portal_domains(self) -> None: custom_domain = "help.app.example" diff --git a/apps/backend/chatballs/tenancy/migrations/0030_encrypted_column_widths.py b/apps/backend/chatballs/tenancy/migrations/0030_encrypted_column_widths.py new file mode 100644 index 0000000..ea2f986 --- /dev/null +++ b/apps/backend/chatballs/tenancy/migrations/0030_encrypted_column_widths.py @@ -0,0 +1,28 @@ +"""Ключи S3: колонки под шифротекст (см. identity.0034).""" + +from django.db import migrations + +import chatballs.identity.crypto + + +class Migration(migrations.Migration): + dependencies = [ + ("tenancy", "0029_drop_product_support"), + ] + + operations = [ + migrations.AlterField( + model_name="storagesettings", + name="s3_access_key", + field=chatballs.identity.crypto.EncryptedCharField( + blank=True, default="", max_length=2828 + ), + ), + migrations.AlterField( + model_name="storagesettings", + name="s3_secret_key", + field=chatballs.identity.crypto.EncryptedCharField( + blank=True, default="", max_length=2828 + ), + ), + ] diff --git a/apps/backend/chatballs_backend/settings_base.py b/apps/backend/chatballs_backend/settings_base.py index 88e21f8..8e3629e 100644 --- a/apps/backend/chatballs_backend/settings_base.py +++ b/apps/backend/chatballs_backend/settings_base.py @@ -251,20 +251,18 @@ CHATBALLS_HELP_BASE_DOMAIN = os.environ.get( ).strip().lower().rstrip(".") CHATBALLS_HELP_PUBLIC_SCHEME = os.environ.get("CHATBALLS_HELP_PUBLIC_SCHEME", "https").strip().lower() CHATBALLS_HELP_PUBLIC_PORT = os.environ.get("CHATBALLS_HELP_PUBLIC_PORT", "").strip() -_default_help_public_ipv4 = os.environ.get( - "CHATBALLS_WEB_LISTENING_IP", - "", -).strip() -if _default_help_public_ipv4 in {"", "0.0.0.0", "::"}: - _default_help_public_ipv4 = "127.0.0.1" if CHATBALLS_HELP_BASE_DOMAIN == "localhost" else "" -CHATBALLS_HELP_PUBLIC_IPV4 = os.environ.get( - "CHATBALLS_HELP_PUBLIC_IPV4", - _default_help_public_ipv4, -).strip() -if not DEBUG and not TESTING and not CHATBALLS_HELP_PUBLIC_IPV4: - raise ImproperlyConfigured( - "CHATBALLS_HELP_PUBLIC_IPV4 or a non-wildcard CHATBALLS_WEB_LISTENING_IP is required" - ) +# Адрес, на который владелец направляет A-запись домена портала. Штатный +# источник — сам адрес установки (его знает только она сама, см. +# chatballs.support_portals.public_address); переменные ниже остаются +# переопределением для контуров, которые ведут конфигурацию сами. Пустое +# значение — не ошибка установки: до мастера первого запуска адреса просто +# ещё нет, а порталов с доменами тем более. +_configured_help_ipv4 = os.environ.get("CHATBALLS_HELP_PUBLIC_IPV4", "").strip() +if not _configured_help_ipv4: + _listening_ip = os.environ.get("CHATBALLS_WEB_LISTENING_IP", "").strip() + if _listening_ip not in {"", "0.0.0.0", "::"}: + _configured_help_ipv4 = _listening_ip +CHATBALLS_HELP_PUBLIC_IPV4 = _configured_help_ipv4 if CHATBALLS_HELP_PUBLIC_IPV4: try: IPv4Address(CHATBALLS_HELP_PUBLIC_IPV4) diff --git a/apps/internal-ui/src/features/integrations/IntegrationForm.tsx b/apps/internal-ui/src/features/integrations/IntegrationForm.tsx index 208183d..f1cb72d 100644 --- a/apps/internal-ui/src/features/integrations/IntegrationForm.tsx +++ b/apps/internal-ui/src/features/integrations/IntegrationForm.tsx @@ -152,7 +152,10 @@ export function IntegrationForm({ initial, kind, onClose, onSaved }: { initial: )} {!isWeb && !isEmail && !isDemo && ( - + <> + +
Пароль прокси наружу не отдаётся: вместо него точки. Оставьте их как есть — прежний пароль сохранится; чтобы сменить, впишите новый целиком
+ )} {meta.hasModel && ( diff --git a/deploy/nginx/frontend.production.conf b/deploy/nginx/frontend.production.conf index 6dbc5a7..5a37758 100644 --- a/deploy/nginx/frontend.production.conf +++ b/deploy/nginx/frontend.production.conf @@ -23,6 +23,14 @@ server { proxy_set_header X-Forwarded-Proto $http_x_forwarded_proto; } + # Готовность стека — внутренний сигнал: её спрашивают healthcheck'и и + # smoke изнутри сети. Снаружи она сообщала бы состояние базы и Redis + # любому желающему. Liveness (/health/live/) остаётся открытым: по нему + # балансировщик отличает живой контейнер от мёртвого. + location = /api/v1/health/ready/ { + return 404; + } + location /api/ { proxy_pass http://backend-app:8000/api/; proxy_http_version 1.1;