Skip to content

Commit

Permalink
Julia/discord supported channels (#58644)
Browse files Browse the repository at this point in the history
Adding support for threads and giving specific error messages with
failure.

<img height="300" alt="Screenshot 2023-10-25 at 4 14 44 PM"
src="https://github.com/getsentry/sentry/assets/22582037/ad52ff1d-b26d-4a21-b291-2b73cdf3d1cf">

<img height="300" alt="Screenshot 2023-10-25 at 4 14 33 PM"
src="https://github.com/getsentry/sentry/assets/22582037/09a415a7-6b31-4a98-a8c9-e618f2bd8fc1">



![Screenshot 2023-10-24 at 9 32
00 AM](https://github.com/getsentry/sentry/assets/22582037/023eea6c-7b6b-411e-b64f-d6f690c1c2b4)


Avoids case where user can send a notification to a forum directly and
the notification tells user it sent, but it does not.

https://discord.com/channels/621778831602221064/804769304255266918/1164913320470515782

Closes #58536
  • Loading branch information
Julia Hoge authored Oct 26, 2023
1 parent 8672f25 commit acaf5e4
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 15 deletions.
7 changes: 2 additions & 5 deletions src/sentry/incidents/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 22 additions & 3 deletions src/sentry/integrations/discord/integration.py
Original file line number Diff line number Diff line change
@@ -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 _
Expand Down Expand Up @@ -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"
Expand All @@ -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}

Expand Down
38 changes: 38 additions & 0 deletions src/sentry/integrations/discord/utils/channel.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

from enum import Enum

from django.core.exceptions import ValidationError
from requests.exceptions import Timeout

Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -84,6 +82,7 @@ def test_discord_channel_id_none(self):
json={
"guild_id": "guild_id",
"name": "guild_id",
"type": ChannelType.GUILD_TEXT.value,
},
)

Expand Down
90 changes: 90 additions & 0 deletions tests/sentry/incidents/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1812,6 +1815,7 @@ def test_discord(self):
metadata={
"guild_id": f"{guild_id}",
"name": f"{guild_name}",
"type": ChannelType.GUILD_TEXT.value,
},
)

Expand All @@ -1824,6 +1828,7 @@ def test_discord(self):
json={
"guild_id": f"{guild_id}",
"name": f"{guild_name}",
"type": ChannelType.GUILD_TEXT.value,
},
)

Expand Down Expand Up @@ -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,
},
)

Expand Down Expand Up @@ -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,
},
)

Expand Down Expand Up @@ -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,
},
)

Expand All @@ -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
Expand Down
15 changes: 12 additions & 3 deletions tests/sentry/integrations/discord/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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
)

0 comments on commit acaf5e4

Please sign in to comment.