From a4d5b15955da974351f6c5f4b4da032d91d6e6ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CA=88=E1=B5=83=E1=B5=A2?= Date: Mon, 29 Jun 2026 09:46:25 -0700 Subject: [PATCH] =?UTF-8?q?fix(slack):=20support=20org-scoped=20tokens=20o?= =?UTF-8?q?n=20Enterprise=20Grid=20via=20SLACK=5FTE=E2=80=A6=20(#41473)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../configuration/alerts-reports.mdx | 15 ++++ superset/config.py | 6 ++ superset/utils/slack.py | 56 ++++++++++++-- tests/unit_tests/utils/slack_test.py | 73 ++++++++++++++++++- 4 files changed, 142 insertions(+), 8 deletions(-) diff --git a/docs/admin_docs/configuration/alerts-reports.mdx b/docs/admin_docs/configuration/alerts-reports.mdx index 6d863508e48..b7327bc9fc6 100644 --- a/docs/admin_docs/configuration/alerts-reports.mdx +++ b/docs/admin_docs/configuration/alerts-reports.mdx @@ -81,6 +81,21 @@ SLACK_CACHE_TIMEOUT = int(timedelta(days=2).total_seconds()) SLACK_API_RATE_LIMIT_RETRY_COUNT = 5 ``` +#### Slack Enterprise Grid (org-scoped tokens) + +On a Slack Enterprise Grid org, an org-scoped token spans multiple workspaces, so +workspace-scoped methods such as `conversations.list` require a `team_id` to +indicate which workspace to target. Set `SLACK_TEAM_ID` to your workspace (team) +ID so Superset can list channels and deliver reports: + +```python +# The workspace (team) ID to target, e.g. "T01234567" +SLACK_TEAM_ID = "T01234567" +``` + +This defaults to `None` and only needs to be set when using an org-scoped token; +it is accepted but ignored for standard workspace-level tokens. + ### Webhook integration Superset can send alert and report notifications to any HTTP endpoint — useful for chat platforms, incident management tools, or custom automation. diff --git a/superset/config.py b/superset/config.py index ac6d39c1ace..3b9160c95a2 100644 --- a/superset/config.py +++ b/superset/config.py @@ -2192,6 +2192,12 @@ EMAIL_REPORTS_CTA = "Explore in Superset" # Slack API token for the superset reports, either string or callable SLACK_API_TOKEN: Callable[[], str] | str | None = None SLACK_PROXY = None +# Slack workspace (team) ID, either a string or a callable. Required when using +# an org-scoped token on an Enterprise Grid org so that workspace-scoped methods +# (e.g. conversations.list) know which workspace to target. It is accepted but +# ignored for workspace-level tokens, so leaving it as None preserves the default +# single-workspace behavior. +SLACK_TEAM_ID: Callable[[], str] | str | None = None SLACK_CACHE_TIMEOUT = int(timedelta(days=1).total_seconds()) # Maximum number of retries when Slack API returns rate limit errors diff --git a/superset/utils/slack.py b/superset/utils/slack.py index 85437d384bd..51fbd497171 100644 --- a/superset/utils/slack.py +++ b/superset/utils/slack.py @@ -17,7 +17,7 @@ import logging -from typing import Callable, Optional +from typing import Any, Callable, Optional from flask import current_app as app from slack_sdk import WebClient @@ -63,22 +63,63 @@ def get_slack_client() -> WebClient: return client -@cache_util.memoized_func( - key="slack_conversations_list", - cache=cache_manager.cache, -) -def get_channels() -> list[SlackChannelSchema]: +def get_team_id() -> Optional[str]: + """ + Return the Slack workspace (team) ID to target, or None. + + On an Enterprise Grid org, an org-scoped token spans multiple workspaces, so + workspace-scoped methods such as ``conversations.list`` need a ``team_id`` to + indicate which workspace to act on. The value is read from the ``SLACK_TEAM_ID`` + config var, which may be a string or a callable (mirroring ``SLACK_API_TOKEN``); + when it is unset (the default for workspace-level tokens) None is returned and + no ``team_id`` is sent, preserving the prior behavior. + """ + team_id = app.config.get("SLACK_TEAM_ID") + if callable(team_id): + team_id = team_id() + return team_id or None + + +def get_channels( + team_id: Optional[str] = None, **kwargs: Any +) -> list[SlackChannelSchema]: """ Retrieves a list of all conversations accessible by the bot from the Slack API, and caches results (to avoid rate limits). The Slack API does not provide search so to apply a search use get_channels_with_search instead. + + :param team_id: workspace (team) ID to target, required for org-scoped tokens + on an Enterprise Grid org. Defaults to the configured ``SLACK_TEAM_ID`` + (via get_team_id) when not given. When set it also keys the cache so that + distinct workspaces never share cached channel lists; when unset, the + legacy cache key is used so that upgrading does not invalidate existing + caches. + :param kwargs: forwarded to the memoized fetch (``force``, ``cache_timeout``, + ``cache``). """ + if team_id is None: + team_id = get_team_id() + cache_key = "slack_conversations_list" + if team_id: + cache_key = f"{cache_key}_{team_id}" + return _get_channels(cache_key, team_id=team_id, **kwargs) + + +@cache_util.memoized_func( + key="{cache_key}", + cache=cache_manager.cache, +) +def _get_channels( + cache_key: str, team_id: Optional[str] = None +) -> list[SlackChannelSchema]: client = get_slack_client() channel_schema = SlackChannelSchema() channels: list[SlackChannelSchema] = [] extra_params = {"types": ",".join(SlackChannelTypes)} + if team_id: + extra_params["team_id"] = team_id cursor = None page_count = 0 @@ -188,7 +229,8 @@ def should_use_v2_api() -> bool: return False try: client = get_slack_client() - client.conversations_list() + team_id = get_team_id() + client.conversations_list(**({"team_id": team_id} if team_id else {})) logger.info("Slack API v2 is available") return True except SlackApiError: diff --git a/tests/unit_tests/utils/slack_test.py b/tests/unit_tests/utils/slack_test.py index 024d6cf96ee..ca3edc3f207 100644 --- a/tests/unit_tests/utils/slack_test.py +++ b/tests/unit_tests/utils/slack_test.py @@ -17,7 +17,11 @@ import pytest -from superset.utils.slack import get_channels_with_search, SlackChannelTypes +from superset.utils.slack import ( + get_channels_with_search, + should_use_v2_api, + SlackChannelTypes, +) class MockResponse: @@ -191,6 +195,73 @@ The server responded with: missing scope: channels:read""" result = get_channels_with_search(types=types) assert {channel["id"] for channel in result} == expected_channel_ids + def test_passes_team_id_to_conversations_list_when_configured(self, mocker): + # When SLACK_TEAM_ID is set (org-scoped token on an Enterprise Grid org), + # it must be forwarded to conversations.list so Slack knows which + # workspace to target. + mock_data = { + "channels": [{"name": "general", "id": "C12345"}], + "response_metadata": {"next_cursor": None}, + } + mock_client = mocker.Mock() + mock_client.conversations_list.return_value = MockResponse(mock_data) + mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client) + mocker.patch.dict( + "superset.utils.slack.app.config", {"SLACK_TEAM_ID": "T123456"} + ) + + get_channels_with_search(force=True) + + assert mock_client.conversations_list.call_args.kwargs["team_id"] == "T123456" + + def test_resolves_callable_team_id(self, mocker): + # SLACK_TEAM_ID may be a callable (e.g. to fetch from a secrets store), + # mirroring SLACK_API_TOKEN; it is resolved before being forwarded. + mock_data = { + "channels": [{"name": "general", "id": "C12345"}], + "response_metadata": {"next_cursor": None}, + } + mock_client = mocker.Mock() + mock_client.conversations_list.return_value = MockResponse(mock_data) + mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client) + mocker.patch.dict( + "superset.utils.slack.app.config", + {"SLACK_TEAM_ID": lambda: "T999999"}, + ) + + get_channels_with_search(force=True) + + assert mock_client.conversations_list.call_args.kwargs["team_id"] == "T999999" + + def test_omits_team_id_from_conversations_list_when_not_configured(self, mocker): + # Default behavior (workspace-level token): no team_id is sent. + mock_data = { + "channels": [{"name": "general", "id": "C12345"}], + "response_metadata": {"next_cursor": None}, + } + mock_client = mocker.Mock() + mock_client.conversations_list.return_value = MockResponse(mock_data) + mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client) + mocker.patch.dict("superset.utils.slack.app.config", {"SLACK_TEAM_ID": None}) + + get_channels_with_search(force=True) + + assert "team_id" not in mock_client.conversations_list.call_args.kwargs + + def test_should_use_v2_api_passes_team_id_when_configured(self, mocker): + mock_client = mocker.Mock() + mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client) + mocker.patch( + "superset.utils.slack.feature_flag_manager.is_feature_enabled", + return_value=True, + ) + mocker.patch.dict( + "superset.utils.slack.app.config", {"SLACK_TEAM_ID": "T123456"} + ) + + assert should_use_v2_api() is True + assert mock_client.conversations_list.call_args.kwargs["team_id"] == "T123456" + def test_handle_pagination_multiple_pages(self, mocker): mock_data_page1 = { "channels": [{"name": "general", "id": "C12345"}],