From a2d76cac5adb5e011bb2df0ff1afb8a9f643720b Mon Sep 17 00:00:00 2001 From: jsuar Date: Sun, 19 Jan 2025 15:09:04 -0500 Subject: [PATCH] Fix Slack file upload (#135818) * pgrade Slack integration to use AsyncWebClient and support files_upload_v2 - Replaced deprecated WebClient with AsyncWebClient throughout the integration. - Removed the unsupported `run_async` parameter. - Added a helper function to resolve channel names to channel IDs. - Updated `_async_send_local_file_message` and `_async_send_remote_file_message` to handle Slack's new API requirements, including per-channel uploads. - Updated dependency from slackclient==2.5.0 to slack-sdk>=3.0.0. - Improved error handling and logging for channel resolution and file uploads. * Fix test to use AsyncWebClient for Slack authentication flow * Fix Slack authentication URL by removing the www subdomain * Refactor Slack file upload functionality and add utility for file uploads --- homeassistant/components/slack/__init__.py | 7 +- homeassistant/components/slack/config_flow.py | 6 +- homeassistant/components/slack/entity.py | 11 +- homeassistant/components/slack/manifest.json | 2 +- homeassistant/components/slack/notify.py | 126 ++++++++++++------ homeassistant/components/slack/sensor.py | 4 +- homeassistant/components/slack/utils.py | 62 +++++++++ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/slack/__init__.py | 2 +- tests/components/slack/test_config_flow.py | 2 +- 11 files changed, 166 insertions(+), 60 deletions(-) create mode 100644 homeassistant/components/slack/utils.py diff --git a/homeassistant/components/slack/__init__.py b/homeassistant/components/slack/__init__.py index 6fce38e4774..aa67739016d 100644 --- a/homeassistant/components/slack/__init__.py +++ b/homeassistant/components/slack/__init__.py @@ -5,8 +5,8 @@ from __future__ import annotations import logging from aiohttp.client_exceptions import ClientError -from slack import WebClient from slack.errors import SlackApiError +from slack_sdk.web.async_client import AsyncWebClient from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_API_KEY, Platform @@ -40,7 +40,9 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Slack from a config entry.""" session = aiohttp_client.async_get_clientsession(hass) - slack = WebClient(token=entry.data[CONF_API_KEY], run_async=True, session=session) + slack = AsyncWebClient( + token=entry.data[CONF_API_KEY], session=session + ) # No run_async try: res = await slack.auth_test() @@ -49,6 +51,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: _LOGGER.error("Invalid API key") return False raise ConfigEntryNotReady("Error while setting up integration") from ex + data = { DATA_CLIENT: slack, ATTR_URL: res[ATTR_URL], diff --git a/homeassistant/components/slack/config_flow.py b/homeassistant/components/slack/config_flow.py index 7f6d7288606..fcdc2e8b362 100644 --- a/homeassistant/components/slack/config_flow.py +++ b/homeassistant/components/slack/config_flow.py @@ -4,8 +4,8 @@ from __future__ import annotations import logging -from slack import WebClient from slack.errors import SlackApiError +from slack_sdk.web.async_client import AsyncSlackResponse, AsyncWebClient import voluptuous as vol from homeassistant.config_entries import ConfigFlow, ConfigFlowResult @@ -57,10 +57,10 @@ class SlackFlowHandler(ConfigFlow, domain=DOMAIN): async def _async_try_connect( self, token: str - ) -> tuple[str, None] | tuple[None, dict[str, str]]: + ) -> tuple[str, None] | tuple[None, AsyncSlackResponse]: """Try connecting to Slack.""" session = aiohttp_client.async_get_clientsession(self.hass) - client = WebClient(token=token, run_async=True, session=session) + client = AsyncWebClient(token=token, session=session) # No run_async try: info = await client.auth_test() diff --git a/homeassistant/components/slack/entity.py b/homeassistant/components/slack/entity.py index 7147186ee9b..30218360054 100644 --- a/homeassistant/components/slack/entity.py +++ b/homeassistant/components/slack/entity.py @@ -2,7 +2,7 @@ from __future__ import annotations -from slack import WebClient +from slack_sdk.web.async_client import AsyncWebClient from homeassistant.config_entries import ConfigEntry from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo @@ -14,21 +14,18 @@ from .const import ATTR_URL, ATTR_USER_ID, DATA_CLIENT, DEFAULT_NAME, DOMAIN class SlackEntity(Entity): """Representation of a Slack entity.""" - _attr_attribution = "Data provided by Slack" - _attr_has_entity_name = True - def __init__( self, - data: dict[str, str | WebClient], + data: dict[str, AsyncWebClient], description: EntityDescription, entry: ConfigEntry, ) -> None: """Initialize a Slack entity.""" - self._client = data[DATA_CLIENT] + self._client: AsyncWebClient = data[DATA_CLIENT] self.entity_description = description self._attr_unique_id = f"{data[ATTR_USER_ID]}_{description.key}" self._attr_device_info = DeviceInfo( - configuration_url=data[ATTR_URL], + configuration_url=str(data[ATTR_URL]), entry_type=DeviceEntryType.SERVICE, identifiers={(DOMAIN, entry.entry_id)}, manufacturer=DEFAULT_NAME, diff --git a/homeassistant/components/slack/manifest.json b/homeassistant/components/slack/manifest.json index 1b35db6f061..3b2322283fe 100644 --- a/homeassistant/components/slack/manifest.json +++ b/homeassistant/components/slack/manifest.json @@ -7,5 +7,5 @@ "integration_type": "service", "iot_class": "cloud_push", "loggers": ["slack"], - "requirements": ["slackclient==2.5.0"] + "requirements": ["slack_sdk==3.33.4"] } diff --git a/homeassistant/components/slack/notify.py b/homeassistant/components/slack/notify.py index 28f9dd203ff..16dd212301a 100644 --- a/homeassistant/components/slack/notify.py +++ b/homeassistant/components/slack/notify.py @@ -5,13 +5,13 @@ from __future__ import annotations import asyncio import logging import os -from typing import Any, TypedDict +from typing import Any, TypedDict, cast from urllib.parse import urlparse -from aiohttp import BasicAuth, FormData +from aiohttp import BasicAuth from aiohttp.client_exceptions import ClientError -from slack import WebClient from slack.errors import SlackApiError +from slack_sdk.web.async_client import AsyncWebClient import voluptuous as vol from homeassistant.components.notify import ( @@ -38,6 +38,7 @@ from .const import ( DATA_CLIENT, SLACK_DATA, ) +from .utils import upload_file_to_slack _LOGGER = logging.getLogger(__name__) @@ -136,7 +137,7 @@ class SlackNotificationService(BaseNotificationService): def __init__( self, hass: HomeAssistant, - client: WebClient, + client: AsyncWebClient, config: dict[str, str], ) -> None: """Initialize.""" @@ -160,17 +161,23 @@ class SlackNotificationService(BaseNotificationService): parsed_url = urlparse(path) filename = os.path.basename(parsed_url.path) - try: - await self._client.files_upload( - channels=",".join(targets), - file=path, - filename=filename, - initial_comment=message, - title=title or filename, - thread_ts=thread_ts or "", - ) - except (SlackApiError, ClientError) as err: - _LOGGER.error("Error while uploading file-based message: %r", err) + channel_ids = [await self._async_get_channel_id(target) for target in targets] + channel_ids = [cid for cid in channel_ids if cid] # Remove None values + + if not channel_ids: + _LOGGER.error("No valid channel IDs resolved for targets: %s", targets) + return + + await upload_file_to_slack( + client=self._client, + channel_ids=channel_ids, + file_content=None, + file_path=path, + filename=filename, + title=title, + message=message, + thread_ts=thread_ts, + ) async def _async_send_remote_file_message( self, @@ -183,12 +190,7 @@ class SlackNotificationService(BaseNotificationService): username: str | None = None, password: str | None = None, ) -> None: - """Upload a remote file (with message) to Slack. - - Note that we bypass the python-slackclient WebClient and use aiohttp directly, - as the former would require us to download the entire remote file into memory - first before uploading it to Slack. - """ + """Upload a remote file (with message) to Slack.""" if not self._hass.config.is_allowed_external_url(url): _LOGGER.error("URL is not allowed: %s", url) return @@ -196,36 +198,35 @@ class SlackNotificationService(BaseNotificationService): filename = _async_get_filename_from_url(url) session = aiohttp_client.async_get_clientsession(self._hass) + # Fetch the remote file kwargs: AuthDictT = {} - if username and password is not None: + if username and password: kwargs = {"auth": BasicAuth(username, password=password)} - resp = await session.request("get", url, **kwargs) - try: - resp.raise_for_status() + async with session.get(url, **kwargs) as resp: + resp.raise_for_status() + file_content = await resp.read() except ClientError as err: _LOGGER.error("Error while retrieving %s: %r", url, err) return - form_data: FormDataT = { - "channels": ",".join(targets), - "filename": filename, - "initial_comment": message, - "title": title or filename, - "token": self._client.token, - } + channel_ids = [await self._async_get_channel_id(target) for target in targets] + channel_ids = [cid for cid in channel_ids if cid] # Remove None values - if thread_ts: - form_data["thread_ts"] = thread_ts + if not channel_ids: + _LOGGER.error("No valid channel IDs resolved for targets: %s", targets) + return - data = FormData(form_data, charset="utf-8") - data.add_field("file", resp.content, filename=filename) - - try: - await session.post("https://slack.com/api/files.upload", data=data) - except ClientError as err: - _LOGGER.error("Error while uploading file message: %r", err) + await upload_file_to_slack( + client=self._client, + channel_ids=channel_ids, + file_content=file_content, + filename=filename, + title=title, + message=message, + thread_ts=thread_ts, + ) async def _async_send_text_only_message( self, @@ -327,3 +328,46 @@ class SlackNotificationService(BaseNotificationService): title, thread_ts=data.get(ATTR_THREAD_TS), ) + + async def _async_get_channel_id(self, channel_name: str) -> str | None: + """Get the Slack channel ID from the channel name. + + This method retrieves the channel ID for a given Slack channel name by + querying the Slack API. It handles both public and private channels. + Including this so users can provide channel names instead of IDs. + + Args: + channel_name (str): The name of the Slack channel. + + Returns: + str | None: The ID of the Slack channel if found, otherwise None. + + Raises: + SlackApiError: If there is an error while communicating with the Slack API. + + """ + try: + # Remove # if present + channel_name = channel_name.lstrip("#") + + # Get channel list + # Multiple types is not working. Tested here: https://api.slack.com/methods/conversations.list/test + # response = await self._client.conversations_list(types="public_channel,private_channel") + # + # Workaround for the types parameter not working + channels = [] + for channel_type in ("public_channel", "private_channel"): + response = await self._client.conversations_list(types=channel_type) + channels.extend(response["channels"]) + + # Find channel ID + for channel in channels: + if channel["name"] == channel_name: + return cast(str, channel["id"]) + + _LOGGER.error("Channel %s not found", channel_name) + + except SlackApiError as err: + _LOGGER.error("Error getting channel ID: %r", err) + + return None diff --git a/homeassistant/components/slack/sensor.py b/homeassistant/components/slack/sensor.py index 9e3beaadd8b..d53555ba82a 100644 --- a/homeassistant/components/slack/sensor.py +++ b/homeassistant/components/slack/sensor.py @@ -2,7 +2,7 @@ from __future__ import annotations -from slack import WebClient +from slack_sdk.web.async_client import AsyncWebClient from homeassistant.components.sensor import ( SensorDeviceClass, @@ -43,7 +43,7 @@ async def async_setup_entry( class SlackSensorEntity(SlackEntity, SensorEntity): """Representation of a Slack sensor.""" - _client: WebClient + _client: AsyncWebClient async def async_update(self) -> None: """Get the latest status.""" diff --git a/homeassistant/components/slack/utils.py b/homeassistant/components/slack/utils.py new file mode 100644 index 00000000000..7619d7d265f --- /dev/null +++ b/homeassistant/components/slack/utils.py @@ -0,0 +1,62 @@ +"""Utils for the Slack integration.""" + +import logging + +import aiofiles +from slack_sdk.errors import SlackApiError +from slack_sdk.web.async_client import AsyncWebClient + +_LOGGER = logging.getLogger(__name__) + + +async def upload_file_to_slack( + client: AsyncWebClient, + channel_ids: list[str | None], + file_content: bytes | str | None, + filename: str, + title: str | None, + message: str, + thread_ts: str | None, + file_path: str | None = None, # Allow passing a file path +) -> None: + """Upload a file to Slack for the specified channel IDs. + + Args: + client (AsyncWebClient): The Slack WebClient instance. + channel_ids (list[str | None]): List of channel IDs to upload the file to. + file_content (Union[bytes, str, None]): Content of the file (local or remote). If None, file_path is used. + filename (str): The file's name. + title (str | None): Title of the file in Slack. + message (str): Initial comment to accompany the file. + thread_ts (str | None): Thread timestamp for threading messages. + file_path (str | None): Path to the local file to be read if file_content is None. + + Raises: + SlackApiError: If the Slack API call fails. + OSError: If there is an error reading the file. + + """ + if file_content is None and file_path: + # Read file asynchronously if file_content is not provided + try: + async with aiofiles.open(file_path, "rb") as file: + file_content = await file.read() + except OSError as os_err: + _LOGGER.error("Error reading file %s: %r", file_path, os_err) + return + + for channel_id in channel_ids: + try: + await client.files_upload_v2( + channel=channel_id, + file=file_content, + filename=filename, + title=title or filename, + initial_comment=message, + thread_ts=thread_ts or "", + ) + _LOGGER.info("Successfully uploaded file to channel %s", channel_id) + except SlackApiError as err: + _LOGGER.error( + "Error while uploading file to channel %s: %r", channel_id, err + ) diff --git a/requirements_all.txt b/requirements_all.txt index cb2a03b0fbf..e830aecc277 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2720,7 +2720,7 @@ sisyphus-control==3.1.4 skyboxremote==0.0.6 # homeassistant.components.slack -slackclient==2.5.0 +slack_sdk==3.33.4 # homeassistant.components.xmpp slixmpp==1.8.5 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 047d7db7821..e31db056291 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -2190,7 +2190,7 @@ simplisafe-python==2024.01.0 skyboxremote==0.0.6 # homeassistant.components.slack -slackclient==2.5.0 +slack_sdk==3.33.4 # homeassistant.components.smart_meter_texas smart-meter-texas==0.5.5 diff --git a/tests/components/slack/__init__.py b/tests/components/slack/__init__.py index acb52a11a6c..507e96294ff 100644 --- a/tests/components/slack/__init__.py +++ b/tests/components/slack/__init__.py @@ -12,7 +12,7 @@ from homeassistant.core import HomeAssistant from tests.common import MockConfigEntry, load_fixture from tests.test_util.aiohttp import AiohttpClientMocker -AUTH_URL = "https://www.slack.com/api/auth.test" +AUTH_URL = "https://slack.com/api/auth.test" TOKEN = "abc123" TEAM_NAME = "Test Team" diff --git a/tests/components/slack/test_config_flow.py b/tests/components/slack/test_config_flow.py index 565b5ec2149..6d0953da5e9 100644 --- a/tests/components/slack/test_config_flow.py +++ b/tests/components/slack/test_config_flow.py @@ -81,7 +81,7 @@ async def test_flow_user_cannot_connect( async def test_flow_user_unknown_error(hass: HomeAssistant) -> None: """Test user initialized flow with unreachable server.""" with patch( - "homeassistant.components.slack.config_flow.WebClient.auth_test" + "homeassistant.components.slack.config_flow.AsyncWebClient.auth_test" ) as mock: mock.side_effect = Exception result = await hass.config_entries.flow.async_init(