sadpandajoe commented on code in PR #45508: URL: https://github.com/apache/superset/pull/45508#discussion_r4373127845
########## superset/integrations/teams/adapter.py: ########## @@ -0,0 +1,545 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Microsoft Teams SDK boundary for Flask callbacks and root channel delivery.""" + +from __future__ import annotations + +import asyncio +import math +from collections.abc import AsyncIterator, Callable +from contextlib import AbstractAsyncContextManager, asynccontextmanager +from dataclasses import dataclass +from datetime import datetime, timezone +from email.utils import parsedate_to_datetime +from time import monotonic +from typing import Any, Literal, Protocol +from uuid import UUID + +import httpx +import requests +from microsoft_teams.api import ( + ApiClient, + Attachment, + ClientCredentials, + InstalledActivity, + InvokeResponse, + JsonWebToken, + MessageActivityInput, + UninstalledActivity, +) +from microsoft_teams.api.auth.cloud_environment import PUBLIC +from microsoft_teams.api.clients.conversation import CreateConversationParams +from microsoft_teams.apps import HttpServer, HttpServerAdapter +from microsoft_teams.apps.events import ActivityEvent +from microsoft_teams.apps.http import HttpRequest, HttpResponse, HttpRouteHandler +from microsoft_teams.common import Client, ClientOptions +from msal import ConfidentialClientApplication +from pydantic import ValidationError + +from superset.integrations.teams.config import TeamsConfig, TeamsDestination + +EVENTS_PATH = "/api/v1/teams/events/" + + +@dataclass(frozen=True) +class TeamsInstallationEvent: + """Authenticated, deployment-scoped installation data, with no SDK objects.""" + + team_id: str + service_url: str + active: bool + event_at: datetime + + +@dataclass(frozen=True) +class TeamsEventResult: + """Callback outcome; only an authenticated event may reach persistence.""" + + status: int + event: TeamsInstallationEvent | None = None + + +class _FlaskServerAdapter(HttpServerAdapter): + """Capture the SDK route for one invocation of Flask's existing POST route. + + Flask registers its route once at startup. Each request creates a fresh SDK + authentication server; its handler runs to completion in an operation-scoped + event loop. No SDK App, outbound clients, tasks, or separate server are created. + """ + + def __init__(self) -> None: + """Hold only the handler for this request.""" + self.handler: HttpRouteHandler | None = None + + def register_route( + self, method: Literal["POST"], path: str, handler: HttpRouteHandler + ) -> None: + """Accept only the independently authenticated Teams callback.""" + if method != "POST" or path != EVENTS_PATH or self.handler is not None: + raise ValueError("Unexpected Teams SDK route registration") + self.handler = handler + + async def start(self, port: int) -> None: + """Prevent the SDK from starting its own HTTP server.""" + raise RuntimeError("Flask manages the Teams callback server") + + async def stop(self) -> None: + """Release the operation's handler; Flask owns the HTTP server.""" + self.handler = None + + +def _same_uuid(value: object, expected: str) -> bool: + """Compare concrete UUIDs without accepting tenant aliases.""" + try: + return isinstance(value, str) and UUID(value) == UUID(expected) + except ValueError: + return False + + +def _allowed_token(config: TeamsConfig, event: ActivityEvent) -> bool: + """Restrict the SDK's broader auth support to expiring bot service tokens.""" + # Only Bot Framework service tokens bind serviceurl to a signed claim. + # Agent 365/Entra activities and bot-to-bot callers are outside this integration. + token = event.token + return ( + isinstance(token, JsonWebToken) + and token.issuer == PUBLIC.token_issuer + and token.expiration is not None + and (not token.tenant_id or _same_uuid(token.tenant_id, config.tenant_id)) + ) + + +def _installation_event( + config: TeamsConfig, event: ActivityEvent, body: dict[str, Any] +) -> TeamsEventResult: + """Apply deployment restrictions only after the SDK has authenticated the token.""" + if not _allowed_token(config, event): + return TeamsEventResult(403) + token = event.token + actions = { + "add": "add", + "add-upgrade": "add", + "remove": "remove", + "remove-upgrade": "remove", + } + action = body.get("action") + if ( + body.get("type") != "installationUpdate" + or not isinstance(action, str) + or action not in actions + ): + return TeamsEventResult(400) + action = actions[action] + # Require the wire-format fields used by SDK authentication. Do not allow + # model defaults or snake_case aliases to bypass missing channel/service data. + if body.get("channelId") != "msteams" or not isinstance( + body.get("serviceUrl"), str + ): + return TeamsEventResult(400) + service_url = body["serviceUrl"] + if not service_url or token.service_url.rstrip("/") != service_url.rstrip("/"): + return TeamsEventResult(403) + try: + model = InstalledActivity if action == "add" else UninstalledActivity + activity = model.model_validate({**body, "action": action}) + except ValidationError: + return TeamsEventResult(400) + if activity.recipient.id not in { + config.application_id, + f"28:{config.application_id}", + }: + return TeamsEventResult(403) + data = activity.channel_data + if ( + data is None + or data.tenant is None + or not _same_uuid(data.tenant.id, config.tenant_id) + or activity.conversation.conversation_type != "channel" + or ( + activity.conversation.tenant_id is not None + and not _same_uuid(activity.conversation.tenant_id, config.tenant_id) + ) + or data.team is None + or not data.team.id + or data.team.team_type not in (None, "standard") + or (data.channel is not None and data.channel.type not in (None, "standard")) + ): + return TeamsEventResult(403) + approved_teams = {destination.team_id for destination in config.destinations} + if data.team.id not in approved_teams: + return TeamsEventResult(403) + event_at = activity.timestamp + if ( + body.get("attachments") + or event_at is None + or event_at.tzinfo is None + or event_at.utcoffset() is None + ): + return TeamsEventResult(400) + return TeamsEventResult( + 200, + TeamsInstallationEvent(data.team.id, service_url, action == "add", event_at), + ) + + +def process_teams_event( + config: TeamsConfig, body: dict[str, Any], headers: dict[str, str] +) -> TeamsEventResult: + """Authenticate through the SDK's server adapter without starting a server. + + The SDK's HttpServer uses PyJWKClient, whose network responses close within + its scoped fetch. It creates no persistent HTTP clients. Keep construction, + authentication, and adapter cleanup within the same event loop; asyncio.run + closes that loop on success and failure. SDK response bodies are never exposed. + """ + + async def process() -> TeamsEventResult: + """Run one SDK invocation, returning only sanitized application data.""" + adapter = _FlaskServerAdapter() + result = TeamsEventResult(500) + + async def on_request(event: ActivityEvent) -> InvokeResponse[None]: + """Receive activities only after SDK authentication succeeds.""" + nonlocal result + result = _installation_event(config, event, body) + return InvokeResponse(status=result.status) + + try: + server = HttpServer(adapter, messaging_endpoint=EVENTS_PATH) + server.on_request = on_request + server.initialize( + credentials=ClientCredentials( + client_id=config.application_id, + client_secret=config.credential, + tenant_id=config.tenant_id, + ), + dangerously_allow_unauthenticated_requests=False, + ) + if adapter.handler is None: + return TeamsEventResult(500) + response: HttpResponse = await adapter.handler( + HttpRequest(body=body, headers=headers) + ) + if response["status"] != 200: + return TeamsEventResult(response["status"]) + return result + finally: + await adapter.stop() + + return asyncio.run(process()) + + +# Count every send, including the single 401 replay, against the same retry bound. +_DELIVERY_ATTEMPTS = 4 +_DELIVERY_TIMEOUT = 60.0 +_DELIVERY_REQUEST_TIMEOUT = 10.0 + + +class TeamsDeliveryError(Exception): + """Sanitized delivery outcome for mapping to existing notification exceptions.""" + + def __init__( + self, category: Literal["authorization", "transient", "parameter"] + ) -> None: + """Expose a stable message without response bodies, SQL, tokens, or URLs.""" + self.category = category + messages = { + "authorization": "Teams bot authorization failed", + "transient": "Teams alert delivery temporarily unavailable", + "parameter": "Teams alert delivery was rejected", + } + super().__init__(messages[category]) + + +class TeamsTransportError(Exception): + """An HTTP failure carrying only the status and Microsoft's retry metadata.""" + + def __init__( + self, + status: int | None = None, + headers: httpx.Headers + | requests.structures.CaseInsensitiveDict[str] + | None = None, + ) -> None: + """Retain only retry headers; never retain a response body or request token.""" + super().__init__("Teams transport request failed") + self.status = status + self.retry_after = headers.get("Retry-After") if headers is not None else None + self.retry_after_ms = ( + headers.get("x-ms-retry-after-ms") if headers is not None else None + ) + + +class TeamsTransport(Protocol): + """Replaceable operation-scoped transport; retries live outside SDK calls.""" + + async def post_root( + self, destination: TeamsDestination, card: dict[str, Any] + ) -> None: + """Create a new channel root post containing the card.""" + ... + + async def refresh_auth(self) -> None: + """Invalidate cached authentication before a single 401 replay.""" + ... + + +def _remaining(deadline: float) -> float: + """Bound each request by its timeout and remaining delivery budget.""" + remaining = deadline - monotonic() + if remaining <= 0: + raise TeamsTransportError() + return min(_DELIVERY_REQUEST_TIMEOUT, remaining) + + +class _AuthHttpClient: + """MSAL HTTP interface backed by a caller-owned, timeout-bounded session.""" + + def __init__(self, session: requests.Session, deadline: float) -> None: + """Keep OAuth traffic within the same operation as the channel post.""" + self.session = session + self.deadline = deadline + + def get(self, url: str, **kwargs: Any) -> requests.Response: + """Fetch MSAL metadata without leaving a response open.""" + return self._request("GET", url, **kwargs) + + def post(self, url: str, **kwargs: Any) -> requests.Response: + """Acquire tokens without leaving a response open.""" + return self._request("POST", url, **kwargs) + + def _request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + """Use normal serialization, explicit timeouts, and sanitized retry errors.""" + kwargs["timeout"] = _remaining(self.deadline) + response = self.session.request(method, url, **kwargs) + try: + _remaining(self.deadline) + # requests eagerly reads responses; MSAL can still read cached JSON + # after close. Closing here also covers retryable OAuth errors. + if response.status_code in (408, 429) or response.status_code >= 500: + raise TeamsTransportError(response.status_code, response.headers) + return response + finally: + response.close() + + +class _ChannelConversationParams(CreateConversationParams): + """Include the connector's group flag, absent from the SDK base parameter model.""" + + is_group: Literal[True] = True + + +class _SDKTeamsTransport: + """Use the SDK create-conversation API with shared, explicitly owned resources.""" + + def __init__( + self, + config: TeamsConfig, + service_url: str, + http: Client, + auth_http: _AuthHttpClient, + deadline: float, + ) -> None: + """Prepare the SDK without constructing a full App or starting a server.""" + self.config = config + self.http = http + self.auth_http = auth_http + self.deadline = deadline + self.auth: ConfidentialClientApplication | None = None + self.api = ApiClient(service_url, http) + + def _authenticate(self) -> None: + """Acquire a bot token without exposing identity-provider errors.""" + if self.http.token: + return + if self.auth is None: + self.auth = ConfidentialClientApplication( + self.config.application_id, + client_credential=self.config.credential, + authority=f"{PUBLIC.login_endpoint}/{self.config.tenant_id}", + http_client=self.auth_http, + instance_discovery=False, + ) + result = self.auth.acquire_token_for_client([PUBLIC.bot_scope]) + token = result.get("access_token") + if not isinstance(token, str) or not token: + status = ( + 503 + if result.get("error") in {"temporarily_unavailable", "server_error"} + else 401 + ) + raise TeamsTransportError(status) + self.http.token = token + + async def refresh_auth(self) -> None: + """Invalidate MSAL app tokens so the next request acquires a fresh token.""" + if self.auth is not None: + self.auth.remove_tokens_for_client() + self.http.token = None + + async def post_root( + self, destination: TeamsDestination, card: dict[str, Any] + ) -> None: + """Create a new conversation with an initial card, never reply to a thread.""" + try: + self._authenticate() + self.http.http.timeout = httpx.Timeout(_remaining(self.deadline)) + await self.api.conversations.create( + _ChannelConversationParams( + tenant_id=self.config.tenant_id, + channel_data={ + "tenant": {"id": self.config.tenant_id}, + "team": {"id": destination.team_id}, + "channel": {"id": destination.channel_id}, + }, + activity=MessageActivityInput( + attachments=[ + Attachment( + content_type="application/vnd.microsoft.card.adaptive", + content=card, + ) + ] + ), + ) + ) + except httpx.HTTPStatusError as ex: + error = TeamsTransportError(ex.response.status_code, ex.response.headers) + await ex.response.aclose() + raise error from None + except (httpx.RequestError, requests.RequestException): Review Comment: Every `httpx.RequestError` becomes a status-less `TeamsTransportError`, and `_deliver_with_retries` treats that as retryable. That includes `ReadTimeout`/`ReadError`/`RemoteProtocolError`, which fire after the conversation-create POST was already sent. That call has no idempotency key, so if Teams accepts the post but responds slower than the 10s request timeout, the loop reposts and the channel can get up to four identical alert cards (`delivery_test.py` asserts four posts for status `None`). The Slack write path only retries errors that cannot duplicate an accepted write (`superset/utils/slack.py`, `is_retryable_slack_transport_error`). Should only pre-send failures (e.g. connect errors) be retried here, with read/protocol errors raised as a non-retried delivery failure? ########## tests/unit_tests/integrations/teams/teams_api_test.py: ########## @@ -0,0 +1,832 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Exercise the real Teams SDK authentication boundary and Flask persistence.""" + +from __future__ import annotations + +import asyncio +import importlib +import importlib.util +from collections.abc import Callable +from copy import deepcopy +from datetime import datetime, timedelta, timezone +from io import BytesIO +from types import SimpleNamespace +from typing import Any +from unittest.mock import patch + +import jwt +import pytest +from cryptography.hazmat.primitives.asymmetric import rsa +from flask import current_app, Flask, Response +from flask.testing import FlaskClient +from sqlalchemy.engine import Engine +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset.integrations.teams import api as teams_api +from superset.integrations.teams.config import parse_teams_config +from superset.integrations.teams.models import TeamsInstallation +from superset.integrations.teams.store import TeamsInstallationStore +from superset.utils import json + +if importlib.util.find_spec("microsoft_teams") is None: + pytest.skip("Optional Teams SDK is not installed", allow_module_level=True) + +teams_adapter = importlib.import_module("superset.integrations.teams.adapter") +PATH = "/api/v1/teams/events/" +TENANT = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" +APPLICATION = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb" +OTHER = "cccccccc-cccc-4ccc-8ccc-cccccccccccc" +SERVICE_URL = "https://smba.trafficmanager.net/amer/"; + + [email protected](scope="module") +def signing_key() -> rsa.RSAPrivateKey: + """Sign real SDK-validated tokens without contacting Microsoft's JWKS service.""" + return rsa.generate_private_key(public_exponent=65537, key_size=2048) + + [email protected](autouse=True) +def trusted_signing_key( + monkeypatch: pytest.MonkeyPatch, signing_key: rsa.RSAPrivateKey +) -> None: + """Replace only key discovery; the SDK verifies signature and claims itself.""" + monkeypatch.setattr( + jwt.PyJWKClient, + "get_signing_key_from_jwt", + lambda self, token: SimpleNamespace(key=signing_key.public_key()), + ) + + [email protected] +def auth_headers(signing_key: rsa.RSAPrivateKey) -> Callable[..., dict[str, str]]: + """Build connector JWTs with optional invalid claims or signing key.""" + + def sign( + claims: dict[str, Any] | None = None, + key: rsa.RSAPrivateKey | None = None, + ) -> dict[str, str]: + """Sign a token so failures exercise the SDK's real validator.""" + now = datetime.now(timezone.utc) + payload = { + "aud": APPLICATION, + "iss": "https://api.botframework.com";, + "iat": int(now.timestamp()), + "nbf": int(now.timestamp()), + "exp": int((now + timedelta(hours=1)).timestamp()), + "serviceurl": SERVICE_URL, + } + payload.update(claims or {}) + return { + "Authorization": "Bearer " + + jwt.encode(payload, key or signing_key, algorithm="RS256") + } + + return sign + + [email protected] +def installation() -> dict[str, Any]: + """An installationUpdate event from a standard team channel.""" + return { + "type": "installationUpdate", + "action": "add", + "id": "event-1", + "timestamp": "2026-10-01T12:00:00Z", + "serviceUrl": SERVICE_URL, + "channelId": "msteams", + "from": {"id": "29:installer"}, + "recipient": {"id": f"28:{APPLICATION}"}, + "conversation": { + "id": "channel-1", + "conversationType": "channel", + "tenantId": TENANT, + }, + "channelData": { + "tenant": {"id": TENANT}, + "team": {"id": "team-1"}, + "channel": {"id": "channel-1", "type": "standard"}, + }, + } + + [email protected] +def teams_client( + teams_config: dict[str, Any], + teams_engine: Engine, + monkeypatch: pytest.MonkeyPatch, +) -> FlaskClient: + """Mount the callback in Flask with real CSRF enforcement and SQLAlchemy.""" + app = Flask(__name__) + app.config.update( + TESTING=True, + SECRET_KEY="test-callback-key", # noqa: S106 + WTF_CSRF_ENABLED=True, + FEATURE_FLAGS={"ALERT_REPORT_TEAMS": True}, + ALERT_REPORTS_TEAMS=teams_config, + ) + monkeypatch.setattr(teams_api, "db", SimpleNamespace(engine=teams_engine)) + monkeypatch.setattr( + teams_api.feature_flag_manager, + "is_feature_enabled", + lambda name: current_app.config["FEATURE_FLAGS"].get(name, False), + ) + teams_api.csrf.init_app(app) + teams_api.register_teams_api(app) + + @app.post("/csrf-control") + def csrf_control() -> Response: + """An unrelated route must remain CSRF protected.""" + return Response(status=200) + + return app.test_client() + + +def test_callback_persists_only_approved_metadata( + teams_client: FlaskClient, + teams_engine: Engine, + teams_config: dict[str, Any], + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], +) -> None: + before = deepcopy(teams_config) + response = teams_client.post(PATH, json=installation, headers=auth_headers()) + assert response.status_code == 200 + assert response.data == b"" + with Session(teams_engine) as session: + persisted = session.query(TeamsInstallation).one() + assert persisted.service_url == SERVICE_URL + assert persisted.active + assert str(persisted.tenant_id) == TENANT + assert str(persisted.application_id) == APPLICATION + assert persisted.team_id == "team-1" + assert teams_config == before + assert b"deployment-secret" not in response.data + + [email protected]( + "authentication", + [ + "missing", + "malformed", + "bad-signature", + "wrong-audience", + "expired", + "wrong-issuer", + "missing-serviceurl", + "mismatched-serviceurl", + ], +) +def test_invalid_authentication_never_opens_database_session( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], + authentication: str, +) -> None: + if authentication == "missing": + headers = {} + elif authentication == "malformed": + headers = {"Authorization": "Bearer not-a-jwt"} + elif authentication == "bad-signature": + headers = auth_headers( + key=rsa.generate_private_key(public_exponent=65537, key_size=2048) + ) + else: + claims = { + "wrong-audience": {"aud": OTHER}, + "expired": {"exp": 1}, + "wrong-issuer": {"iss": "https://untrusted.example"}, + "missing-serviceurl": {"serviceurl": None}, + "mismatched-serviceurl": { + "serviceurl": "https://smba.trafficmanager.net/other/"; + }, + } + headers = auth_headers(claims[authentication]) + with patch.object(teams_api, "Session") as session: + response = teams_client.post(PATH, json=installation, headers=headers) + assert response.status_code == 401 + assert response.data == b"" + session.assert_not_called() + + [email protected]( + "mismatch", + [ + "tenant", + "conversation-tenant", + "token-tenant", + "recipient", + "team", + "personal", + "groupChat", + "private", + "shared", + "private-team", + "shared-team", + ], +) +def test_authenticated_events_must_match_deployment_scope( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], + mismatch: str, +) -> None: + claims = {} + if mismatch == "tenant": + installation["channelData"]["tenant"]["id"] = OTHER + elif mismatch == "conversation-tenant": + installation["conversation"]["tenantId"] = OTHER + elif mismatch == "token-tenant": + claims = {"tid": OTHER} + elif mismatch == "recipient": + installation["recipient"]["id"] = f"28:{OTHER}" + elif mismatch == "team": + installation["channelData"]["team"]["id"] = "unapproved-team" + elif mismatch in {"personal", "groupChat"}: + installation["conversation"]["conversationType"] = mismatch + elif mismatch in {"private", "shared"}: + installation["channelData"]["channel"]["type"] = mismatch + else: + installation["channelData"]["team"]["teamType"] = ( + "privateChannel" if mismatch == "private-team" else "sharedChannel" + ) + with patch.object(teams_api, "Session") as session: + response = teams_client.post( + PATH, json=installation, headers=auth_headers(claims) + ) + assert response.status_code == 403 + session.assert_not_called() + + +def test_entra_tokens_cannot_bypass_signed_service_url_binding( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], +) -> None: + # The SDK also supports Agent 365 tokens; this notification bot accepts only + # connector tokens whose signed serviceurl claim binds the installation URL. + headers = auth_headers( + { + "iss": f"https://login.microsoftonline.com/{TENANT}/v2.0";, + "tid": TENANT, + "azp": "5a807f24-c9de-44ee-a3a7-329e88a00ffc", + "serviceurl": None, + } + ) + with patch.object(teams_api, "Session") as session: + response = teams_client.post(PATH, json=installation, headers=headers) + assert response.status_code == 403 + session.assert_not_called() + + +def test_tokens_without_expiration_are_rejected( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], +) -> None: + with patch.object(teams_api, "Session") as session: + response = teams_client.post( + PATH, json=installation, headers=auth_headers({"exp": None}) + ) + assert response.status_code in (401, 403) + session.assert_not_called() + + [email protected]("add_action", ["add", "add-upgrade"]) [email protected]("remove_action", ["remove", "remove-upgrade"]) +def test_repeated_events_removal_and_reinstallation( + teams_client: FlaskClient, + teams_engine: Engine, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], + add_action: str, + remove_action: str, +) -> None: + installation["action"] = add_action + headers = auth_headers() + for _ in range(2): + assert ( + teams_client.post(PATH, json=installation, headers=headers).status_code + == 200 + ) + with Session(teams_engine) as session: + original = session.query(TeamsInstallation).one() + identity, created_on = original.id, original.created_on + installation["action"] = remove_action + for _ in range(2): + assert ( + teams_client.post(PATH, json=installation, headers=headers).status_code + == 200 + ) + with Session(teams_engine) as session: + removed = session.query(TeamsInstallation).one() + assert removed.id == identity + assert not removed.active + installation["action"] = add_action + installation["timestamp"] = "2026-10-01T12:01:00Z" + refreshed = "https://smba.trafficmanager.net/emea/"; + installation["serviceUrl"] = refreshed + for _ in range(2): + response = teams_client.post( + PATH, json=installation, headers=auth_headers({"serviceurl": refreshed}) + ) + assert response.status_code == 200 + with Session(teams_engine) as session: + reinstalled = session.query(TeamsInstallation).one() + assert reinstalled.id == identity + assert reinstalled.created_on == created_on + assert reinstalled.active + assert reinstalled.service_url == refreshed + + +def test_removal_retains_unknown_installation_tombstone( + teams_client: FlaskClient, + teams_engine: Engine, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], +) -> None: + installation["action"] = "remove" + assert ( + teams_client.post(PATH, json=installation, headers=auth_headers()).status_code + == 200 + ) + with Session(teams_engine) as session: + assert not session.query(TeamsInstallation).one().active + + [email protected]( + "change", + [ + "type", + "action", + "malformed-action", + "missing-service", + "missing-channel", + "missing-recipient", + "attachment", + ], +) +def test_unsupported_or_incomplete_events_do_not_persist( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], + change: str, +) -> None: + if change == "type": + installation["type"] = "message" + elif change == "action": + installation["action"] = "unknown" + elif change == "malformed-action": + installation["action"] = [] + elif change == "missing-service": + installation.pop("serviceUrl") + elif change == "missing-channel": + installation.pop("channelId") + elif change == "missing-recipient": + installation.pop("recipient") + else: + installation["attachments"] = [{"contentType": "image/png"}] + with patch.object(teams_api, "Session") as session: + response = teams_client.post(PATH, json=installation, headers=auth_headers()) + assert response.status_code == 400 + session.assert_not_called() + + +def test_session_cookie_does_not_authenticate_callback( + teams_client: FlaskClient, installation: dict[str, Any] +) -> None: + with teams_client.session_transaction() as browser_session: + browser_session["_user_id"] = "1" + browser_session["_fresh"] = True + with patch.object(teams_api, "Session") as session: + response = teams_client.post(PATH, json=installation) + assert response.status_code == 401 + session.assert_not_called() + + +def test_csrf_exemption_is_only_for_callback(teams_client: FlaskClient) -> None: + # Without a CSRF exemption this would be 400 before SDK authentication. + assert teams_client.post(PATH, json={}).status_code == 401 + assert teams_client.post("/csrf-control").status_code == 400 + assert teams_client.get(PATH).status_code == 405 + + [email protected]( + "mode", ["disabled", "unconfigured", "invalid-config", "missing-sdk"] +) +def test_callback_fails_closed_without_sdk_or_configuration( + teams_client: FlaskClient, + installation: dict[str, Any], + auth_headers: Callable[..., dict[str, str]], + mode: str, +) -> None: + expected = 503 + if mode == "disabled": + teams_client.application.config["FEATURE_FLAGS"]["ALERT_REPORT_TEAMS"] = False + expected = 404 + elif mode == "unconfigured": + teams_client.application.config["ALERT_REPORTS_TEAMS"] = {} + elif mode == "invalid-config": + teams_client.application.config["ALERT_REPORTS_TEAMS"] = { + "credential": "deployment-secret" + } + with patch.dict("sys.modules", {"superset.integrations.teams.adapter": None}): Review Comment: The `sys.modules` patch applies to every parametrized mode, including `unconfigured`, so that case returns 503 from the SDK import failure even without the `config is None` guard in `teams_events`. If that guard were dropped, this test would still pass, while a deployment with the SDK installed and `ALERT_REPORTS_TEAMS = {}` would call `process_teams_event(None, ...)` and fail. Could the import-failure patch be limited to `missing-sdk`, so `unconfigured` runs with the SDK present and asserts 503 without invoking event processing? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
