fix(auth): warn on insecure token and use composite credentials on TLS channels

zeevdr · claude · zeevdr · commit b16666079757 · 2026-05-20T17:02:18.000+03:00
When insecure=True and a token is provided, emit a UserWarning at client construction time so callers know the token will travel in cleartext. When TLS is active (insecure=False or credentials is set), embed the token via grpc.composite_channel_credentials + metadata_call_credentials so it is protected by the TLS layer rather than sent as a raw authorization header. Document the secure-default expectation in the token and insecure docstring parameters of both ConfigClient and AsyncConfigClient. Co-Authored-By: Claude <noreply@anthropic.com> Closes #53
diff --git a/sdk/src/opendecree/_channel.py b/sdk/src/opendecree/_channel.py
@@ -14,29 +14,69 @@
 ]
 
 
+def _token_call_credentials(token: str) -> grpc.CallCredentials:
+    """Return gRPC call credentials that inject a Bearer token."""
+
+    def _callback(context: object, callback: object) -> None:  # type: ignore[type-arg]
+        assert callable(callback)
+        callback([("authorization", f"Bearer {token}")], None)
+
+    return grpc.metadata_call_credentials(_callback)
+
+
 def create_channel(
     target: str,
     *,
     insecure: bool = True,
     credentials: grpc.ChannelCredentials | None = None,
+    token: str | None = None,
 ) -> grpc.Channel:
-    """Create a gRPC channel with sensible defaults."""
-    if credentials is not None:
-        return grpc.secure_channel(target, credentials, options=_DEFAULT_OPTIONS)
-    if insecure:
-        return grpc.insecure_channel(target, options=_DEFAULT_OPTIONS)
-    return grpc.secure_channel(target, grpc.ssl_channel_credentials(), options=_DEFAULT_OPTIONS)
+    """Create a gRPC channel with sensible defaults.
+
+    When *token* is provided and TLS is active (``insecure=False`` or
+    *credentials* is given), the token is embedded via
+    ``composite_channel_credentials`` so it is protected by the TLS layer.
+    On an insecure channel the token is sent as a raw header — callers should
+    warn the user before allowing this.
+    """
+    channel_creds: grpc.ChannelCredentials | None = credentials
+    if channel_creds is None and not insecure:
+        channel_creds = grpc.ssl_channel_credentials()
+
+    if channel_creds is not None:
+        if token:
+            channel_creds = grpc.composite_channel_credentials(
+                channel_creds, _token_call_credentials(token)
+            )
+        return grpc.secure_channel(target, channel_creds, options=_DEFAULT_OPTIONS)
+
+    return grpc.insecure_channel(target, options=_DEFAULT_OPTIONS)
 
 
 def create_aio_channel(
     target: str,
     *,
     insecure: bool = True,
     credentials: grpc.ChannelCredentials | None = None,
+    token: str | None = None,
 ) -> grpc.aio.Channel:
-    """Create an async gRPC channel with sensible defaults."""
-    if credentials is not None:
-        return grpc.aio.secure_channel(target, credentials, options=_DEFAULT_OPTIONS)
-    if insecure:
-        return grpc.aio.insecure_channel(target, options=_DEFAULT_OPTIONS)
-    return grpc.aio.secure_channel(target, grpc.ssl_channel_credentials(), options=_DEFAULT_OPTIONS)
+    """Create an async gRPC channel with sensible defaults.
+
+    When *token* is provided and TLS is active (``insecure=False`` or
+    *credentials* is given), the token is embedded via
+    ``composite_channel_credentials`` so it is protected by the TLS layer.
+    On an insecure channel the token is sent as a raw header — callers should
+    warn the user before allowing this.
+    """
+    channel_creds: grpc.ChannelCredentials | None = credentials
+    if channel_creds is None and not insecure:
+        channel_creds = grpc.ssl_channel_credentials()
+
+    if channel_creds is not None:
+        if token:
+            channel_creds = grpc.composite_channel_credentials(
+                channel_creds, _token_call_credentials(token)
+            )
+        return grpc.aio.secure_channel(target, channel_creds, options=_DEFAULT_OPTIONS)
+
+    return grpc.aio.insecure_channel(target, options=_DEFAULT_OPTIONS)
diff --git a/sdk/src/opendecree/async_client.py b/sdk/src/opendecree/async_client.py
@@ -7,6 +7,7 @@
 
 from __future__ import annotations
 
+import warnings
 from datetime import timedelta
 from typing import TYPE_CHECKING, overload
 
@@ -61,7 +62,12 @@ def __init__(
             role: Role for ``x-role`` metadata header. Defaults to ``"superadmin"``.
             tenant_id: Default tenant for ``x-tenant-id`` metadata header.
             token: Bearer token. When set, metadata headers are not sent.
+                On a TLS channel the token is embedded via
+                ``composite_channel_credentials`` and protected by the TLS
+                layer. On an insecure channel it travels in cleartext and a
+                ``UserWarning`` is raised — prefer TLS in production.
             insecure: Use plaintext (no TLS). Defaults to True for local dev.
+                Do not combine with *token* in production.
             credentials: TLS channel credentials. Overrides *insecure*.
             timeout: Default per-RPC timeout in seconds. Defaults to 10.
             retry: Retry configuration. Defaults to ``RetryConfig()``.
@@ -70,12 +76,28 @@ def __init__(
         self._timeout = timeout
         self._retry = retry if retry is not None else RetryConfig()
 
+        tls_active = credentials is not None or not insecure
+        if token and not tls_active:
+            warnings.warn(
+                "Bearer token sent over insecure channel without TLS — "
+                "the token will be transmitted in cleartext. "
+                "Set insecure=False or provide credentials in production.",
+                UserWarning,
+                stacklevel=2,
+            )
+
+        # On a TLS channel, embed the token via composite_channel_credentials
+        # (protected by TLS). On an insecure channel, fall back to raw header.
         # grpc.aio doesn't support intercept_channel, so we inject metadata
-        # directly on each call via self._metadata().
+        # directly on each call via self._auth_metadata.
+        channel_token = token if tls_active else None
+        metadata_token = token if not tls_active else None
         self._auth_metadata = _build_metadata(
-            subject=subject, role=role, tenant_id=tenant_id, token=token
+            subject=subject, role=role, tenant_id=tenant_id, token=metadata_token
+        )
+        self._channel = create_aio_channel(
+            target, insecure=insecure, credentials=credentials, token=channel_token
         )
-        self._channel = create_aio_channel(target, insecure=insecure, credentials=credentials)
 
         cs_pb2, cs_grpc = ensure_stubs()
         self._stub = cs_grpc.ConfigServiceStub(self._channel)
diff --git a/sdk/src/opendecree/client.py b/sdk/src/opendecree/client.py
@@ -65,7 +65,12 @@ def __init__(
             role: Role for ``x-role`` metadata header. Defaults to ``"superadmin"``.
             tenant_id: Default tenant for ``x-tenant-id`` metadata header.
             token: Bearer token. When set, metadata headers are not sent.
+                On a TLS channel the token is embedded via
+                ``composite_channel_credentials`` and protected by the TLS
+                layer. On an insecure channel it travels in cleartext and a
+                ``UserWarning`` is raised — prefer TLS in production.
             insecure: Use plaintext (no TLS). Defaults to True for local dev.
+                Do not combine with *token* in production.
             credentials: TLS channel credentials. Overrides *insecure*.
             timeout: Default per-RPC timeout in seconds. Defaults to 10.
             retry: Retry configuration. Defaults to ``RetryConfig()``.
@@ -74,12 +79,30 @@ def __init__(
         self._timeout = timeout
         self._retry = retry if retry is not None else RetryConfig()
 
-        metadata = _build_metadata(subject=subject, role=role, tenant_id=tenant_id, token=token)
+        tls_active = credentials is not None or not insecure
+        if token and not tls_active:
+            warnings.warn(
+                "Bearer token sent over insecure channel without TLS — "
+                "the token will be transmitted in cleartext. "
+                "Set insecure=False or provide credentials in production.",
+                UserWarning,
+                stacklevel=2,
+            )
+
+        # On a TLS channel, embed the token via composite_channel_credentials
+        # (protected by TLS). On an insecure channel, fall back to raw header.
+        channel_token = token if tls_active else None
+        metadata_token = token if not tls_active else None
+        metadata = _build_metadata(
+            subject=subject, role=role, tenant_id=tenant_id, token=metadata_token
+        )
         interceptors: list[grpc.UnaryUnaryClientInterceptor] = []
         if metadata:
             interceptors.append(AuthInterceptor(metadata))
 
-        channel = create_channel(target, insecure=insecure, credentials=credentials)
+        channel = create_channel(
+            target, insecure=insecure, credentials=credentials, token=channel_token
+        )
         if interceptors:
             self._channel = grpc.intercept_channel(channel, *interceptors)
         else:
diff --git a/sdk/tests/test_channel.py b/sdk/tests/test_channel.py
@@ -34,6 +34,35 @@ def test_secure_default(self):
             mock_sec.assert_called_once()
             assert ch is mock_sec.return_value
 
+    def test_tls_with_token_uses_composite_credentials(self):
+        creds = MagicMock(spec=grpc.ChannelCredentials)
+        composite = MagicMock(spec=grpc.ChannelCredentials)
+        call_creds = MagicMock(spec=grpc.CallCredentials)
+        with (
+            patch("opendecree._channel.grpc.secure_channel") as mock_sec,
+            patch(
+                "opendecree._channel.grpc.metadata_call_credentials",
+                return_value=call_creds,
+            ),
+            patch(
+                "opendecree._channel.grpc.composite_channel_credentials",
+                return_value=composite,
+            ) as mock_comp,
+        ):
+            mock_sec.return_value = MagicMock()
+            create_channel("host:443", credentials=creds, token="tok")
+            mock_comp.assert_called_once_with(creds, call_creds)
+            args = mock_sec.call_args[0]
+            assert args[1] is composite
+
+    def test_insecure_with_token_does_not_use_composite(self):
+        with patch("opendecree._channel.grpc.insecure_channel") as mock_insecure:
+            with patch("opendecree._channel.grpc.composite_channel_credentials") as mock_comp:
+                mock_insecure.return_value = MagicMock()
+                create_channel("localhost:9090", insecure=True, token="tok")
+                mock_insecure.assert_called_once()
+                mock_comp.assert_not_called()
+
 
 class TestCreateAioChannel:
     def test_insecure(self):
@@ -61,3 +90,32 @@ def test_secure_default(self):
             mock_ssl.assert_called_once()
             mock_sec.assert_called_once()
             assert ch is mock_sec.return_value
+
+    def test_tls_with_token_uses_composite_credentials(self):
+        creds = MagicMock(spec=grpc.ChannelCredentials)
+        composite = MagicMock(spec=grpc.ChannelCredentials)
+        call_creds = MagicMock(spec=grpc.CallCredentials)
+        with (
+            patch("opendecree._channel.grpc.aio.secure_channel") as mock_sec,
+            patch(
+                "opendecree._channel.grpc.metadata_call_credentials",
+                return_value=call_creds,
+            ),
+            patch(
+                "opendecree._channel.grpc.composite_channel_credentials",
+                return_value=composite,
+            ) as mock_comp,
+        ):
+            mock_sec.return_value = MagicMock()
+            create_aio_channel("host:443", credentials=creds, token="tok")
+            mock_comp.assert_called_once_with(creds, call_creds)
+            args = mock_sec.call_args[0]
+            assert args[1] is composite
+
+    def test_insecure_with_token_does_not_use_composite(self):
+        with patch("opendecree._channel.grpc.aio.insecure_channel") as mock_insecure:
+            with patch("opendecree._channel.grpc.composite_channel_credentials") as mock_comp:
+                mock_insecure.return_value = MagicMock()
+                create_aio_channel("localhost:9090", insecure=True, token="tok")
+                mock_insecure.assert_called_once()
+                mock_comp.assert_not_called()
diff --git a/sdk/tests/test_client.py b/sdk/tests/test_client.py
@@ -1,5 +1,6 @@
 """Tests for the sync ConfigClient."""
 
+import warnings
 from unittest.mock import MagicMock, patch
 
 import grpc
@@ -11,6 +12,55 @@
 from tests.conftest import FakeRpcError
 
 
+def _make_patched_client(**kwargs):
+    """Create a ConfigClient with mocked channel and interceptors."""
+    with patch("opendecree.client.create_channel") as mock_ch:
+        mock_channel = MagicMock()
+        mock_ch.return_value = mock_channel
+        with patch("opendecree.client.grpc.intercept_channel") as mock_intercept:
+            mock_intercept.return_value = mock_channel
+            return opendecree.ConfigClient("localhost:9090", **kwargs)
+
+
+class TestConfigClientTokenWarning:
+    def test_warns_on_insecure_with_token(self):
+        with pytest.warns(UserWarning, match="insecure channel"):
+            _make_patched_client(token="secret", insecure=True)
+
+    def test_no_warning_on_tls_with_token(self):
+        creds = MagicMock(spec=grpc.ChannelCredentials)
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", UserWarning)
+            _make_patched_client(token="secret", credentials=creds)
+
+    def test_no_warning_on_insecure_without_token(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", UserWarning)
+            _make_patched_client(insecure=True)
+
+    def test_tls_token_not_in_metadata(self):
+        creds = MagicMock(spec=grpc.ChannelCredentials)
+        with patch("opendecree.client.create_channel") as mock_ch:
+            mock_ch.return_value = MagicMock()
+            with patch("opendecree.client.grpc.intercept_channel"):
+                client = opendecree.ConfigClient(
+                    "localhost:9090", token="secret", credentials=creds
+                )
+        # No interceptor metadata should contain the raw authorization header.
+        assert client._raw_channel is mock_ch.return_value
+        # Token routed to channel factory, not raw header interceptor.
+        assert mock_ch.call_args.kwargs.get("token") == "secret"
+
+    def test_insecure_token_in_metadata_not_channel(self):
+        with pytest.warns(UserWarning):
+            with patch("opendecree.client.create_channel") as mock_ch:
+                mock_ch.return_value = MagicMock()
+                with patch("opendecree.client.grpc.intercept_channel"):
+                    opendecree.ConfigClient("localhost:9090", token="secret", insecure=True)
+        # Token should NOT be forwarded to channel factory on insecure path.
+        assert mock_ch.call_args.kwargs.get("token") is None
+
+
 class TestConfigClientImport:
     """Test that the client is importable and has the expected API."""
 
