diff --git a/gateway/authz_mixin.py b/gateway/authz_mixin.py index bcefb4eecb4..bff02153a3a 100644 --- a/gateway/authz_mixin.py +++ b/gateway/authz_mixin.py @@ -31,6 +31,28 @@ from gateway.whatsapp_identity import ( class GatewayAuthorizationMixin: """User/chat authorization methods for ``GatewayRunner``.""" + def _adapter_authorization_is_upstream(self, platform: Optional[Platform]) -> bool: + """Whether the adapter for *platform* delegates authz to a trusted upstream. + + Mirrors ``BasePlatformAdapter.authorization_is_upstream``. The relay + adapter sets this True: the Team Gateway connector authenticates the + gateway's WS and resolves owner-only author bindings before delivering, + so an inbound relay event is already authorized as this instance's bound + user. Unlike ``_adapter_enforces_own_access_policy`` (a LOCAL config + policy the gateway mirrors only when it's an allowlist), this is an + UPSTREAM decision the gateway honors directly. Defaults to ``False`` when + the adapter is unknown or doesn't expose the flag. + """ + if not platform: + return False + adapters = getattr(self, "adapters", None) + if not adapters: + return False + adapter = adapters.get(platform) + if adapter is None: + return False + return bool(getattr(adapter, "authorization_is_upstream", False)) + def _adapter_enforces_own_access_policy(self, platform: Optional[Platform]) -> bool: """Whether the adapter for *platform* gates access at intake itself. @@ -193,6 +215,21 @@ class GatewayAuthorizationMixin: if source.platform in {Platform.HOMEASSISTANT, Platform.WEBHOOK}: return True + # Relay (and any adapter whose authorization is enforced by a trusted + # authenticated upstream): the Team Gateway connector authenticates this + # gateway's WS with a per-instance secret and resolves owner-only author + # bindings BEFORE delivering, so an inbound relay event was already + # authorized as this instance's bound user (the author id is the one the + # connector observed, never gateway-asserted). There is no local + # RELAY_ALLOWED_USERS env allowlist to consult, and default-denying for + # its absence is the bug this branch fixes. This is delegation to a + # trusted upstream, NOT a fail-open: it fires only for an adapter that + # explicitly sets authorization_is_upstream=True; every direct + # network-exposed adapter leaves it False and the env-allowlist + # default-deny below still applies unchanged. + if self._adapter_authorization_is_upstream(source.platform): + return True + user_id = source.user_id # Telegram (and similar) authorize entire group/forum/channel chats diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 0637ca75519..9971399c3d9 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -2247,6 +2247,38 @@ class BasePlatformAdapter(ABC): """ return False + @property + def authorization_is_upstream(self) -> bool: + """Whether inbound on this adapter was already authorized UPSTREAM. + + Distinct from ``enforces_own_access_policy``: that flag describes an + adapter that enforces a LOCAL, config-driven access surface + (``dm_policy: allowlist`` / ``allow_from``) the gateway can mirror. This + flag describes an adapter whose authorization is performed by a TRUSTED + UPSTREAM over an authenticated transport — there is no local policy to + consult, and the env allowlist (``{PLATFORM}_ALLOWED_USERS``) does not + apply because the sender identity isn't a platform account the operator + configures here. + + The relay adapter is the sole user: it fronts the Team Gateway + connector over a per-instance-authenticated WebSocket, and the connector + performs owner-only author-binding resolution BEFORE delivering — a + message only reaches this gateway because the connector resolved it to + THIS instance's bound user (``user_instance_binding``). The author id is + read off the event the connector observed, never gateway-asserted. So an + inbound relay event carries an authorization decision already made by a + trusted, authenticated upstream; default-denying it (no env allowlist ⇒ + deny) is incorrect. + + This is NOT a fail-open: it is authorization DELEGATED to a trusted + upstream that authenticated the transport (the relay WS secret) and + enforced owner-only binding, as opposed to authorization being ABSENT. + It only takes effect for an adapter that explicitly overrides this to + ``True``; every network-exposed direct adapter leaves it ``False`` and + the env-allowlist default-deny continues to apply unchanged. + """ + return False + def supports_draft_streaming( self, chat_type: Optional[str] = None, diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index ef8e0cdfb63..e4f9d84c15c 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -75,6 +75,20 @@ class RelayAdapter(BasePlatformAdapter): self._revocation_monitor: Optional[asyncio.Task[None]] = None # ── capability surface (from descriptor) ───────────────────────────── + @property + def authorization_is_upstream(self) -> bool: + """Relay authorization is enforced by the connector, not locally. + + The connector authenticates this gateway's WS (per-instance secret) and + performs owner-only author-binding resolution before delivering, so any + inbound relay event was already authorized as THIS instance's bound user + (``user_instance_binding``, keyed on the connector-observed author id). + The instance therefore must not default-deny relay users for lack of a + local ``RELAY_ALLOWED_USERS`` env allowlist. See + ``BasePlatformAdapter.authorization_is_upstream``. + """ + return True + @property def message_len_fn(self) -> Callable[[str], int]: return _LEN_FNS.get(self.descriptor.len_unit, len) diff --git a/tests/gateway/test_relay_upstream_authz.py b/tests/gateway/test_relay_upstream_authz.py new file mode 100644 index 00000000000..bee1b60a6df --- /dev/null +++ b/tests/gateway/test_relay_upstream_authz.py @@ -0,0 +1,146 @@ +"""Tests for relay upstream-enforced authorization at the gateway layer. + +Background: the relay adapter fronts the Team Gateway connector over a +per-instance-authenticated WebSocket. The connector performs owner-only +author-binding resolution BEFORE delivering an inbound event — a message only +reaches this gateway because the connector resolved it to THIS instance's bound +user (``user_instance_binding``, keyed on the connector-observed author id, +never a gateway claim). So a relay inbound is already authorized by a trusted, +authenticated upstream. + +Before this fix, ``_is_user_authorized`` had no notion of upstream +authorization: ``Platform.RELAY`` matched no ``*_ALLOWED_USERS`` allowlist and +isn't in the HA/WEBHOOK always-authorized set, so every relay user hit the +default-deny ("No user allowlists configured. All unauthorized users will be +denied.") and the agent never saw the message. This was the live staging bug: +the message routed correctly through the connector to the instance, then the +instance's authz layer dropped it as ``Unauthorized user``. + +The fix adds a generic ``BasePlatformAdapter.authorization_is_upstream`` +capability (default ``False``) that the relay adapter overrides to ``True``, +plus a dedicated trusted branch in ``_is_user_authorized``. It is delegation to +a trusted upstream, NOT a fail-open: it fires only for an adapter that +explicitly declares the flag; every direct network-exposed adapter leaves it +``False`` and the env-allowlist default-deny is unchanged. +""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import Platform +from gateway.session import SessionSource + + +def _clear_auth_env(monkeypatch) -> None: + for key in ( + "DISCORD_ALLOWED_USERS", + "GATEWAY_ALLOWED_USERS", + "GATEWAY_ALLOW_ALL_USERS", + "DISCORD_ALLOW_ALL_USERS", + ): + monkeypatch.delenv(key, raising=False) + + +def _make_runner(*, platform: Platform, authorization_is_upstream: bool): + """Build a bare GatewayRunner with one adapter for *platform*. + + ``authorization_is_upstream`` controls whether that adapter declares the + upstream-authz capability. + """ + from gateway.run import GatewayRunner + + runner = object.__new__(GatewayRunner) + adapter = SimpleNamespace( + send=AsyncMock(), + authorization_is_upstream=authorization_is_upstream, + enforces_own_access_policy=False, + ) + runner.adapters = {platform: adapter} + runner.pairing_store = MagicMock() + runner.pairing_store.is_approved.return_value = False + runner.pairing_store._is_rate_limited.return_value = False + return runner, adapter + + +def _relay_source(**kw) -> SessionSource: + base = dict( + platform=Platform.RELAY, + user_id="428014785045725184", + chat_id="1400724139874058314", + user_name="definitely_not_cthulhu", + chat_type="group", + ) + base.update(kw) + return SessionSource(**base) + + +# --------------------------------------------------------------------------- +# Capability contract +# --------------------------------------------------------------------------- + + +def test_base_adapter_defaults_to_not_upstream_authorized(): + """The base property is False — direct adapters keep env default-deny.""" + from gateway.platforms.base import BasePlatformAdapter + + assert BasePlatformAdapter.authorization_is_upstream.fget(object()) is False + + +def test_relay_adapter_declares_upstream_authz(): + """The relay adapter overrides the capability to True (static capability).""" + from gateway.relay.adapter import RelayAdapter + + # Property reflects a static capability, independent of instance config. + assert RelayAdapter.authorization_is_upstream.fget(object()) is True + + +# --------------------------------------------------------------------------- +# Authorization behavior +# --------------------------------------------------------------------------- + + +def test_relay_user_authorized_with_no_env_allowlist(monkeypatch): + """A relay user is authorized even with NO env allowlist configured. + + This is the staging-bug regression guard: the connector already authorized + the author via owner-only binding, so the instance must not default-deny. + """ + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.RELAY, authorization_is_upstream=True) + assert runner._is_user_authorized(_relay_source()) is True + + +def test_relay_dm_authorized_with_no_env_allowlist(monkeypatch): + """The /link DM path is also authorized (DMs are upstream-bound too).""" + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.RELAY, authorization_is_upstream=True) + assert runner._is_user_authorized(_relay_source(chat_type="dm")) is True + + +def test_non_upstream_adapter_still_default_denies(monkeypatch): + """A direct adapter that does NOT declare the flag still default-denies. + + Guards against the fix becoming a blanket fail-open: an adapter with + authorization_is_upstream=False and no env allowlist must remain denied. + """ + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.DISCORD, authorization_is_upstream=False) + src = SessionSource( + platform=Platform.DISCORD, + user_id="123", + chat_id="456", + user_name="someone", + chat_type="dm", + ) + assert runner._is_user_authorized(src) is False + + +def test_upstream_authz_helper_false_for_unknown_platform(monkeypatch): + """The helper returns False when there's no adapter for the platform.""" + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.RELAY, authorization_is_upstream=True) + # A platform with no registered adapter must not be treated as upstream-authz. + assert runner._adapter_authorization_is_upstream(Platform.DISCORD) is False + assert runner._adapter_authorization_is_upstream(None) is False