diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 4b7957899e5d0c..3e3ced8eb0b1f9 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -1352,11 +1352,8 @@ def get_alert_rule_trigger_action_discord_channel_id( integration_id=integration.id, guild_name=integration.name, ) - except ValidationError: - raise InvalidTriggerActionError( - "Could not find channel %s. Channel may not exist, may be formatted incorrectly, or Sentry may not " - "have been granted permission to access it" % name - ) + except ValidationError as e: + raise InvalidTriggerActionError(e.message) except IntegrationError: raise InvalidTriggerActionError("Bad response from Discord channel lookup") except ApiTimeoutError: diff --git a/src/sentry/integrations/discord/integration.py b/src/sentry/integrations/discord/integration.py index 9f7c9d4710d710..e5514175724a5b 100644 --- a/src/sentry/integrations/discord/integration.py +++ b/src/sentry/integrations/discord/integration.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections.abc import Mapping, Sequence +from enum import Enum import requests from django.utils.translation import gettext_lazy as _ @@ -93,6 +94,19 @@ def uninstall(self) -> None: return +class DiscordPermissions(Enum): + # https://discord.com/developers/docs/topics/permissions#permissions + VIEW_CHANNEL = 1 << 10 + SEND_MESSAGES = 1 << 11 + SEND_TTS_MESSAGES = 1 << 12 + EMBED_LINKS = 1 << 14 + ATTACH_FILES = 1 << 15 + MANAGE_THREADS = 1 << 34 + CREATE_PUBLIC_THREADS = 1 << 35 + CREATE_PRIVATE_THREADS = 1 << 36 + SEND_MESSAGES_IN_THREADS = 1 << 38 + + class DiscordIntegrationProvider(IntegrationProvider): key = "discord" name = "Discord" @@ -104,9 +118,14 @@ class DiscordIntegrationProvider(IntegrationProvider): # https://discord.com/developers/docs/topics/oauth2#shared-resources-oauth2-scopes oauth_scopes = frozenset(["applications.commands", "bot", "identify"]) - # https://discord.com/developers/docs/topics/permissions#permissions - # Permissions value that can Send Messages (0x800), View Channel (0x400), and Embed Links (0x4000): - bot_permissions = 0x800 | 0x400 | 0x4000 + bot_permissions = ( + DiscordPermissions.VIEW_CHANNEL.value + | DiscordPermissions.SEND_MESSAGES.value + | DiscordPermissions.EMBED_LINKS.value + | DiscordPermissions.CREATE_PUBLIC_THREADS.value + | DiscordPermissions.CREATE_PRIVATE_THREADS.value + | DiscordPermissions.SEND_MESSAGES_IN_THREADS.value + ) setup_dialog_config = {"width": 600, "height": 900} diff --git a/src/sentry/integrations/discord/utils/channel.py b/src/sentry/integrations/discord/utils/channel.py index 72bc18cee4fe36..69f6b1a27108c0 100644 --- a/src/sentry/integrations/discord/utils/channel.py +++ b/src/sentry/integrations/discord/utils/channel.py @@ -1,5 +1,7 @@ from __future__ import annotations +from enum import Enum + from django.core.exceptions import ValidationError from requests.exceptions import Timeout @@ -10,6 +12,30 @@ from . import logger +class ChannelType(Enum): + # https://discord.com/developers/docs/resources/channel#channel-object-channel-types + GUILD_TEXT = 0 + DM = 1 + GUILD_VOICE = 2 + GROUP_DM = 3 + GUILD_CATEGORY = 4 + GUILD_ANNOUNCEMENT = 5 + ANNOUNCEMENT_THREAD = 10 + PUBLIC_THREAD = 11 + PRIVATE_THREAD = 12 + GUILD_STAGE_VOICE = 13 + GUILD_DIRECTORY = 14 + GUILD_FORUM = 15 + GUILD_MEDIA = 16 + + +SUPPORTED_CHANNEL_TYPES = { + ChannelType.GUILD_TEXT, + ChannelType.PUBLIC_THREAD, + ChannelType.PRIVATE_THREAD, +} + + def validate_channel_id( channel_id: str, guild_id: str, integration_id: int | None, guild_name: str | None ) -> None: @@ -80,6 +106,18 @@ def validate_channel_id( if not isinstance(result, dict): raise IntegrationError("Bad response from Discord channel lookup.") + if ChannelType(result["type"]) not in SUPPORTED_CHANNEL_TYPES: + # Forums are not supported + logger.info( + "rule.discord.wrong_channel_type", + extra={ + "channel_id": channel_id, + "guild_name": guild_name, + "channel_type": result["type"], + }, + ) + raise ValidationError("Discord channel type is not supported") + if result["guild_id"] != guild_id: # The channel exists and we have access to it, but it does not belong # to the specified guild! We'll use the same message as generic 404, diff --git a/tests/sentry/api/serializers/test_alert_rule_trigger_action.py b/tests/sentry/api/serializers/test_alert_rule_trigger_action.py index 59453ce33c8d47..160e5b5b99c234 100644 --- a/tests/sentry/api/serializers/test_alert_rule_trigger_action.py +++ b/tests/sentry/api/serializers/test_alert_rule_trigger_action.py @@ -4,6 +4,7 @@ from sentry.incidents.logic import create_alert_rule_trigger, create_alert_rule_trigger_action from sentry.incidents.models import AlertRuleTriggerAction from sentry.incidents.serializers import ACTION_TARGET_TYPE_TO_STRING +from sentry.integrations.discord.utils.channel import ChannelType from sentry.models.integrations.integration import Integration from sentry.testutils.cases import TestCase @@ -45,10 +46,7 @@ def test_discord(self): responses.add( method=responses.GET, url=f"{base_url}/channels/channel-id", - json={ - "guild_id": "guild_id", - "name": "guild_id", - }, + json={"guild_id": "guild_id", "name": "guild_id", "type": ChannelType.GUILD_TEXT.value}, ) alert_rule = self.create_alert_rule() @@ -84,6 +82,7 @@ def test_discord_channel_id_none(self): json={ "guild_id": "guild_id", "name": "guild_id", + "type": ChannelType.GUILD_TEXT.value, }, ) diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index 010fa093018e50..ace60ce1213bfd 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -67,6 +67,7 @@ IncidentType, TriggerStatus, ) +from sentry.integrations.discord.utils.channel import ChannelType from sentry.models.actor import ActorTuple, get_actor_for_user, get_actor_id_for_user from sentry.models.integrations.integration import Integration from sentry.models.integrations.organization_integration import OrganizationIntegration @@ -1443,6 +1444,7 @@ def test_discord(self): metadata = { "guild_id": guild_id, "name": "Server Name", + "type": ChannelType.GUILD_TEXT.value, } integration = Integration.objects.create( provider="discord", @@ -1479,6 +1481,7 @@ def test_discord_flag_off(self): metadata = { "guild_id": guild_id, "name": "Server Name", + "type": ChannelType.GUILD_TEXT.value, } integration = Integration.objects.create( provider="discord", @@ -1812,6 +1815,7 @@ def test_discord(self): metadata={ "guild_id": f"{guild_id}", "name": f"{guild_name}", + "type": ChannelType.GUILD_TEXT.value, }, ) @@ -1824,6 +1828,7 @@ def test_discord(self): json={ "guild_id": f"{guild_id}", "name": f"{guild_name}", + "type": ChannelType.GUILD_TEXT.value, }, ) @@ -1856,6 +1861,7 @@ def test_discord_invalid_channel_id(self): metadata={ "guild_id": f"{guild_id}", "name": f"{guild_name}", + "type": ChannelType.GUILD_TEXT.value, }, ) @@ -1888,6 +1894,7 @@ def test_discord_bad_response(self): metadata={ "guild_id": f"{guild_id}", "name": f"{guild_name}", + "type": ChannelType.GUILD_TEXT.value, }, ) @@ -1940,6 +1947,7 @@ def test_discord_timeout(self, mock_validate_channel_id): metadata={ "guild_id": f"{guild_id}", "name": f"{guild_name}", + "type": ChannelType.GUILD_TEXT.value, }, ) @@ -1965,6 +1973,88 @@ def test_discord_timeout(self, mock_validate_channel_id): integration_id=integration.id, ) + @responses.activate + def test_discord_channel_not_in_guild(self): + base_url: str = "https://discord.com/api/v10" + channel_id = "channel-id" + guild_id = "example-discord-server" + guild_name = "Server Name" + + integration = Integration.objects.create( + provider="discord", + name="Example Discord", + external_id=f"{guild_id}", + metadata={ + "guild_id": f"{guild_id}", + "name": f"{guild_name}", + "type": ChannelType.DM.value, + }, + ) + + integration.add_organization(self.organization, self.user) + type = AlertRuleTriggerAction.Type.DISCORD + target_type = AlertRuleTriggerAction.TargetType.SPECIFIC + responses.add( + method=responses.GET, + url=f"{base_url}/channels/{channel_id}", + json={ + "guild_id": "other-guild", + "name": f"{guild_name}", + "type": ChannelType.DM.value, + }, + ) + + with self.feature("organizations:integrations-discord-metric-alerts"): + with pytest.raises(InvalidTriggerActionError): + update_alert_rule_trigger_action( + self.action, + type, + target_type, + target_identifier=channel_id, + integration_id=integration.id, + ) + + @responses.activate + def test_discord_unsupported_type(self): + base_url: str = "https://discord.com/api/v10" + channel_id = "channel-id" + guild_id = "example-discord-server" + guild_name = "Server Name" + + integration = Integration.objects.create( + provider="discord", + name="Example Discord", + external_id=f"{guild_id}", + metadata={ + "guild_id": f"{guild_id}", + "name": f"{guild_name}", + "type": ChannelType.DM.value, + }, + ) + + integration.add_organization(self.organization, self.user) + type = AlertRuleTriggerAction.Type.DISCORD + target_type = AlertRuleTriggerAction.TargetType.SPECIFIC + responses.add( + method=responses.GET, + url=f"{base_url}/channels/{channel_id}", + json={ + "guild_id": f"{guild_id}", + "name": f"{guild_name}", + "type": ChannelType.DM.value, + }, + ) + + with self.feature("organizations:integrations-discord-metric-alerts"): + with pytest.raises(InvalidTriggerActionError): + update_alert_rule_trigger_action( + self.action, + type, + target_type, + target_identifier=channel_id, + integration_id=integration.id, + ) + class DeleteAlertRuleTriggerAction(BaseAlertRuleTriggerActionTest, TestCase): @cached_property diff --git a/tests/sentry/integrations/discord/test_utils.py b/tests/sentry/integrations/discord/test_utils.py index f26f90442a66f8..3543b50e3660ed 100644 --- a/tests/sentry/integrations/discord/test_utils.py +++ b/tests/sentry/integrations/discord/test_utils.py @@ -5,7 +5,7 @@ from requests.exceptions import Timeout from sentry.integrations.discord.utils.auth import verify_signature -from sentry.integrations.discord.utils.channel import validate_channel_id +from sentry.integrations.discord.utils.channel import ChannelType, validate_channel_id from sentry.shared_integrations.exceptions import ApiTimeoutError, IntegrationError from sentry.shared_integrations.exceptions.base import ApiError from sentry.testutils.cases import TestCase @@ -38,12 +38,13 @@ def test_verify_signature_invalid(self): class ValidateChannelTest(TestCase): guild_id = "guild-id" channel_id = "channel-id" + channel_type = 0 # text integration_id = 1234 guild_name = "server name" @mock.patch("sentry.integrations.discord.utils.channel.DiscordClient.get_channel") def test_happy_path(self, mock_get_channel): - mock_get_channel.return_value = {"guild_id": self.guild_id} + mock_get_channel.return_value = {"guild_id": self.guild_id, "type": self.channel_type} validate_channel_id(self.channel_id, self.guild_id, self.integration_id, self.guild_name) @mock.patch("sentry.integrations.discord.utils.channel.DiscordClient.get_channel") @@ -88,7 +89,7 @@ def test_bad_response(self, mock_get_channel): @mock.patch("sentry.integrations.discord.utils.channel.DiscordClient.get_channel") def test_not_guild_member(self, mock_get_channel): - mock_get_channel.return_value = {"guild_id": "not-my-guild"} + mock_get_channel.return_value = {"guild_id": "not-my-guild", "type": self.channel_type} with raises(ValidationError): validate_channel_id( self.channel_id, self.guild_id, self.integration_id, self.guild_name @@ -101,3 +102,11 @@ def test_timeout(self, mock_get_channel): validate_channel_id( self.channel_id, self.guild_id, self.integration_id, self.guild_name ) + + @mock.patch("sentry.integrations.discord.utils.channel.DiscordClient.get_channel") + def test_not_supported_type(self, mock_get_channel): + mock_get_channel.return_value = {"guild_id": self.guild_id, "type": ChannelType.DM.value} + with raises(ValidationError): + validate_channel_id( + self.channel_id, self.guild_id, self.integration_id, self.guild_name + )