Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@
(default `"redis"`). Parameterizes the lock-token prefix for observability
and interop.

### Upstream parity

- **Teams: `TeamsAuthCertificate` config shape** (Issue #58). Ports the
upstream `TeamsAuthCertificate` interface (`adapter-teams/src/types.ts:3-10`)
as a Python dataclass with `certificate_private_key`, `certificate_thumbprint`,
and `x5c` fields. `TeamsAdapterConfig(certificate=...)` is accepted and
re-exported from `chat_sdk.adapters.teams` so consumers can code against the
shape ahead of MS Teams SDK support. Passing a non-`None` value still throws
at adapter startup — the error message is now verbatim with
`adapter-teams/src/config.ts:13-18` (`"Certificate-based authentication is
not yet supported by the Teams SDK adapter. Use appPassword (client secret)
or federated (workload identity) authentication instead."`). Not a functional
implementation; upstream does not implement cert auth either.

### Test hygiene

- Sweep remaining `time.sleep` → `await asyncio.sleep` in async tests
Expand Down
11 changes: 10 additions & 1 deletion src/chat_sdk/adapters/teams/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
"""Teams adapter for chat-sdk."""

from chat_sdk.adapters.teams.adapter import TeamsAdapter, create_teams_adapter
from chat_sdk.adapters.teams.types import (
TeamsAdapterConfig,
TeamsAuthCertificate,
)

__all__ = ["TeamsAdapter", "create_teams_adapter"]
__all__ = [
"TeamsAdapter",
"TeamsAdapterConfig",
"TeamsAuthCertificate",
"create_teams_adapter",
]
8 changes: 5 additions & 3 deletions src/chat_sdk/adapters/teams/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,13 @@ def __init__(self, config: TeamsAdapterConfig | None = None) -> None:
self._app_password = config.app_password or os.environ.get("TEAMS_APP_PASSWORD", "")
self._app_tenant_id = config.app_tenant_id or os.environ.get("TEAMS_APP_TENANT_ID", "")

if config.certificate:
if config.certificate is not None:
# Exact parity with upstream adapter-teams/src/config.ts:13-18.
# ``appPassword`` is referenced in camelCase to match upstream text.
raise ValidationError(
"teams",
"Certificate-based authentication is not yet supported. "
"Use app_password (client secret) or federated (workload identity) authentication instead.",
"Certificate-based authentication is not yet supported by the Teams SDK adapter. "
"Use appPassword (client secret) or federated (workload identity) authentication instead.",
)

if not self._app_id:
Expand Down
21 changes: 16 additions & 5 deletions src/chat_sdk/adapters/teams/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,24 @@
# =============================================================================


class TeamsAuthCertificate(TypedDict, total=False):
"""Certificate-based authentication config (not yet supported)."""
@dataclass
class TeamsAuthCertificate:
Comment on lines +19 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The conversion of TeamsAuthCertificate from a TypedDict to a dataclass introduces an inconsistency with other authentication configuration shapes in this file, such as TeamsAuthFederated (line 39), which remains a TypedDict. Additionally, this is a breaking change for any existing code that passes a dictionary literal to the certificate field of TeamsAdapterConfig. To maintain a consistent API and support dictionary literals (which is common for configuration shapes ported from TypeScript interfaces), consider keeping TeamsAuthCertificate as a TypedDict. If you retain the TypedDict structure, ensure it is defined with total=False if it will be used for mode detection when narrowing from a Mapping[str, Any], as per the project's typing standards. If a dataclass is preferred, TeamsAuthFederated should also be converted for consistency.

References
  1. When using cast to narrow a Mapping[str, Any] into a TypedDict, ensure that the TypedDict is a superset (total=False) that admits all possible keys, especially when performing duck-typing with .get() for mode detection.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted — the TypedDict vs dataclass inconsistency with TeamsAuthFederated is deliberately deferred as bikeshed. No consumer usage to unify around, and TeamsAuthFederated isn't re-exported from __init__.py. Tracking as a potential follow-up; not blocking this PR.

"""Certificate-based authentication config.

.. deprecated::
Certificate auth is not yet supported by the Teams SDK. Setting
``certificate`` on :class:`TeamsAdapterConfig` raises at adapter
startup. Ported for shape parity with upstream
``adapter-teams/src/types.ts`` so consumers can code against the
config shape ahead of MS Teams SDK support.
"""

# PEM-encoded certificate private key
certificate_private_key: str
# Hex-encoded certificate thumbprint (optional when x5c is provided)
certificate_thumbprint: str
certificate_thumbprint: str | None = None
# Public certificate for subject-name validation (optional)
x5c: str
x5c: str | None = None


class TeamsAuthFederated(TypedDict, total=False):
Expand Down Expand Up @@ -53,7 +62,9 @@ class TeamsAdapterConfig:
app_tenant_id: str | None = None
# Microsoft App Type.
app_type: str | None = None # "MultiTenant" | "SingleTenant"
# Certificate auth (not yet supported by the Teams SDK).
# Deprecated: certificate auth is not yet supported by the Teams SDK.
# Passing a non-None value raises at adapter startup — kept for shape
# parity with upstream adapter-teams/src/types.ts.
certificate: TeamsAuthCertificate | None = None
# Federated (workload identity) authentication.
federated: TeamsAuthFederated | None = None
Expand Down
22 changes: 4 additions & 18 deletions tests/test_teams_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@
TeamsAdapter,
_validate_service_url,
)
from chat_sdk.adapters.teams.types import TeamsAdapterConfig, TeamsThreadId
from chat_sdk.adapters.teams.types import (
TeamsAdapterConfig,
TeamsThreadId,
)
from chat_sdk.shared.errors import (
AuthenticationError,
NetworkError,
Expand Down Expand Up @@ -1478,23 +1481,6 @@ def routed_post(url, **kwargs):
assert len(result.messages) >= 0


# ---------------------------------------------------------------------------
# Certificate config raises
# ---------------------------------------------------------------------------


class TestCertificateConfig:
def test_certificate_raises_validation_error(self):
with pytest.raises(ValidationError, match="Certificate"):
TeamsAdapter(
TeamsAdapterConfig(
app_id="test",
app_password="test",
certificate={"thumbprint": "abc", "private_key": "key"},
)
)


# ---------------------------------------------------------------------------
# _extract_attachments_from_graph_message
# ---------------------------------------------------------------------------
Expand Down
43 changes: 41 additions & 2 deletions tests/test_teams_extended.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
TeamsAdapter,
_handle_teams_error,
)
from chat_sdk.adapters.teams.types import TeamsAdapterConfig, TeamsThreadId
from chat_sdk.adapters.teams.types import (
TeamsAdapterConfig,
TeamsAuthCertificate,
TeamsThreadId,
)
from chat_sdk.shared.errors import (
AdapterPermissionError,
AdapterRateLimitError,
Expand Down Expand Up @@ -542,10 +546,45 @@ def test_raises_validation_error(self):
TeamsAdapterConfig(
app_id="app",
app_password="pass",
certificate={"certificate_private_key": "key", "certificate_thumbprint": "thumb"},
certificate=TeamsAuthCertificate(
certificate_private_key="key",
certificate_thumbprint="thumb",
),
)
)

def test_raises_with_exact_upstream_message(self):
"""Startup throw message matches upstream adapter-teams/src/config.ts:13-18 verbatim.

Upstream references ``appPassword`` (camelCase TS field name); we preserve
that in the error text so consumers tailing upstream logs see identical
output. Protects against well-meaning rewording to ``app_password``.
"""
expected = (
"Certificate-based authentication is not yet supported by the Teams SDK adapter. "
"Use appPassword (client secret) or federated (workload identity) authentication instead."
)
with pytest.raises(ValidationError) as exc_info:
TeamsAdapter(
TeamsAdapterConfig(
certificate=TeamsAuthCertificate(certificate_private_key="key"),
)
)
assert expected in str(exc_info.value)

def test_minimal_certificate_only_requires_private_key(self):
"""``certificate_thumbprint`` and ``x5c`` are optional per upstream types.ts:7-9.

A ``TeamsAuthCertificate`` constructed with only ``certificate_private_key``
must still trigger the startup throw (i.e. the adapter checks presence, not
shape).
"""
cert = TeamsAuthCertificate(certificate_private_key="pem-key")
assert cert.certificate_thumbprint is None
assert cert.x5c is None
with pytest.raises(ValidationError, match="Certificate-based"):
TeamsAdapter(TeamsAdapterConfig(certificate=cert))


# ---------------------------------------------------------------------------
# Stream via post+edit
Expand Down
Loading