Skip to content

Commit

Permalink
fix(discord): Verify User Bot Installation Permission Take 2 (#76586)
Browse files Browse the repository at this point in the history
#76360 broke prod, so trying
again.

this time, we are just going to check if the user is actually part of
the guild. to call this method, we needed to ask for guild info in the
oauth scope.
  • Loading branch information
iamrajjoshi authored Aug 28, 2024
1 parent e4ff0e6 commit c2a7bf7
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 39 deletions.
17 changes: 17 additions & 0 deletions src/sentry/integrations/discord/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from sentry.integrations.client import ApiClient
from sentry.integrations.discord.message_builder.base.base import DiscordMessageBuilder
from sentry.integrations.discord.utils.consts import DISCORD_ERROR_CODES, DISCORD_USER_ERRORS
from sentry.shared_integrations.exceptions import ApiError

# to avoid a circular import
from sentry.utils import metrics
Expand Down Expand Up @@ -102,6 +103,22 @@ def get_user_id(self, access_token: str):
user_id = response["id"] # type: ignore[index]
return user_id

def check_user_bot_installation_permission(self, access_token: str, guild_id: str) -> bool:
headers = {"Authorization": f"Bearer {access_token}"}

# We only want information about guild_id and check the user's permission in the guild, but we can't currently do that
# https://github.com/discord/discord-api-docs/discussions/6846
# TODO(iamrajjoshi): Eventually, we should use `/users/@me/guilds/{guild.id}/member`
# Instead, we check if the user in a member of the guild

try:
self.get(f"/users/@me/guilds/{guild_id}/member", headers=headers)
except ApiError as e:
if e.code == 404:
return False

return True

def leave_guild(self, guild_id: str) -> None:
"""
Leave the given guild_id, if the bot is currently a member.
Expand Down
28 changes: 11 additions & 17 deletions src/sentry/integrations/discord/integration.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

from collections.abc import Mapping, Sequence
from enum import Enum
from typing import Any
from urllib.parse import urlencode

Expand All @@ -17,6 +16,7 @@
IntegrationProvider,
)
from sentry.integrations.discord.client import DiscordClient
from sentry.integrations.discord.types import DiscordPermissions
from sentry.integrations.models.integration import Integration
from sentry.organizations.services.organization.model import RpcOrganizationSummary
from sentry.pipeline.views.base import PipelineView
Expand Down Expand Up @@ -111,19 +111,6 @@ 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 @@ -132,7 +119,8 @@ class DiscordIntegrationProvider(IntegrationProvider):
features = frozenset([IntegrationFeatures.CHAT_UNFURL, IntegrationFeatures.ALERT_RULE])

# https://discord.com/developers/docs/topics/oauth2#shared-resources-oauth2-scopes
oauth_scopes = frozenset(["applications.commands", "bot", "identify"])
oauth_scopes = frozenset(["applications.commands", "bot", "identify", "guilds.members.read"])
access_token = ""

bot_permissions = (
DiscordPermissions.VIEW_CHANNEL.value
Expand Down Expand Up @@ -175,6 +163,12 @@ def build_integration(self, state: Mapping[str, object]) -> Mapping[str, object]
auth_code = str(state.get("code"))
if auth_code:
discord_user_id = self._get_discord_user_id(auth_code, url)
if options.get(
"discord.validate-user"
) and not self.client.check_user_bot_installation_permission(
access_token=self.access_token, guild_id=guild_id
):
raise IntegrationError("User does not have permissions to install bot.")
else:
raise IntegrationError("Missing code from state.")

Expand Down Expand Up @@ -238,13 +232,13 @@ def _get_discord_user_id(self, auth_code: str, url: str) -> str:
"""
try:
access_token = self.client.get_access_token(auth_code, url)
self.access_token = self.client.get_access_token(auth_code, url)
except ApiError:
raise IntegrationError("Failed to get Discord access token from API.")
except KeyError:
raise IntegrationError("Failed to get Discord access token from key.")
try:
user_id = self.client.get_user_id(access_token)
user_id = self.client.get_user_id(self.access_token)
except ApiError:
raise IntegrationError("Failed to get Discord user ID from API.")
except KeyError:
Expand Down
17 changes: 17 additions & 0 deletions src/sentry/integrations/discord/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from enum import Enum


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

MANAGE_GUILD = 1 << 5
ADMINISTRATOR = 1 << 3
3 changes: 3 additions & 0 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@
register("slack.verification-token", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)
register("slack.signing-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)

# Discord
register("discord.validate-user", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE)

# Codecov Integration
register("codecov.client-secret", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK)

Expand Down
103 changes: 81 additions & 22 deletions tests/sentry/integrations/discord/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def setUp(self):
options.set("discord.public-key", self.public_key)
options.set("discord.bot-token", self.bot_token)
options.set("discord.client-secret", self.client_secret)
options.set("discord.validate-user", True)

@mock.patch("sentry.integrations.discord.client.DiscordClient.set_application_command")
def assert_setup_flow(
Expand Down Expand Up @@ -72,6 +73,12 @@ def assert_setup_flow(
json=[] if command_response_empty else COMMANDS,
)

responses.add(
responses.GET,
url=f"{DiscordClient.base_url}/users/@me/guilds/{guild_id}/member",
json={},
)

if command_response_empty:
for command in COMMANDS:
responses.add(
Expand Down Expand Up @@ -160,6 +167,13 @@ def assert_setup_flow_from_discord(
"access_token": "access_token",
},
)

responses.add(
responses.GET,
url=f"{DiscordClient.base_url}/users/@me/guilds/{guild_id}/member",
json={},
)

responses.add(
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": "user_1234"}
)
Expand Down Expand Up @@ -237,19 +251,22 @@ def test_multiple_integrations(self):


class DiscordIntegrationTest(DiscordSetupTestCase):
def setUp(self):
super().setUp()
self.user_id = "user1234"
self.guild_id = "12345"
self.guild_name = "guild_name"

@responses.activate
def test_get_guild_name(self):
provider = self.provider()
user_id = "user1234"
guild_id = "guild_id"
guild_name = "guild_name"
responses.add(
responses.GET,
url=f"{DiscordClient.base_url}{GUILD_URL.format(guild_id=guild_id)}",
url=f"{DiscordClient.base_url}{GUILD_URL.format(guild_id=self.guild_id)}",
match=[header_matcher({"Authorization": f"Bot {self.bot_token}"})],
json={
"id": guild_id,
"name": guild_name,
"id": self.guild_id,
"name": self.guild_name,
},
)
responses.add(
Expand All @@ -262,21 +279,26 @@ def test_get_guild_name(self):
responses.add(
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": "user_1234"}
)
result = provider.build_integration({"guild_id": "guild_id", "code": user_id})
assert result["name"] == guild_name

responses.add(
responses.GET,
url=f"{DiscordClient.base_url}/users/@me/guilds/{self.guild_id}/member",
json={},
)

result = provider.build_integration({"guild_id": self.guild_id, "code": self.user_id})
assert result["name"] == self.guild_name

@responses.activate
def test_build_integration_no_code_in_state(self):
provider = self.provider()
guild_id = "guild_id"
guild_name = "guild_name"
responses.add(
responses.GET,
url=f"{DiscordClient.base_url}{GUILD_URL.format(guild_id=guild_id)}",
url=f"{DiscordClient.base_url}{GUILD_URL.format(guild_id=self.guild_id)}",
match=[header_matcher({"Authorization": f"Bot {self.bot_token}"})],
json={
"id": guild_id,
"name": guild_name,
"id": self.guild_id,
"name": self.guild_name,
},
)
with pytest.raises(IntegrationError):
Expand All @@ -285,9 +307,40 @@ def test_build_integration_no_code_in_state(self):
@responses.activate
def test_get_guild_name_failure(self):
provider = self.provider()
user_id = "user1234"
guild_id = "guild_id"
responses.add(responses.GET, "https://discord.com/api/v10/guilds/guild_name", status=500)

responses.add(responses.GET, "https://discord.com/api/v10/guilds/guild_name", status=500),
responses.add(
responses.POST,
url="https://discord.com/api/v10/oauth2/token",
json={
"access_token": "access_token",
},
)
responses.add(
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": self.user_id}
)
responses.add(
responses.GET,
url=f"{DiscordClient.base_url}/users/@me/guilds/{self.guild_id}/member",
json={},
)

result = provider.build_integration({"guild_id": self.guild_id, "code": self.user_id})
assert result["name"] == self.guild_id

@responses.activate
def test_get_user_insufficient_permission(self):
provider = self.provider()

responses.add(
responses.GET,
url=f"{DiscordClient.base_url}{GUILD_URL.format(guild_id=self.guild_id)}",
match=[header_matcher({"Authorization": f"Bot {self.bot_token}"})],
json={
"id": self.guild_id,
"name": self.guild_name,
},
)
responses.add(
responses.POST,
url="https://discord.com/api/v10/oauth2/token",
Expand All @@ -296,15 +349,21 @@ def test_get_guild_name_failure(self):
},
)
responses.add(
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": user_id}
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": self.user_id}
)
responses.add(
responses.GET,
url=f"{DiscordClient.base_url}/users/@me/guilds/{self.guild_id}/member",
json={"code": 10004, "message": "Unknown guild"},
status=404,
)
result = provider.build_integration({"guild_id": guild_id, "code": user_id})
assert result["name"] == guild_id

with pytest.raises(IntegrationError):
provider.build_integration({"guild_id": self.guild_id, "code": self.user_id})

@responses.activate
def test_get_discord_user_id(self):
provider = self.provider()
user_id = "user1234"

responses.add(
responses.POST,
Expand All @@ -314,12 +373,12 @@ def test_get_discord_user_id(self):
},
)
responses.add(
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": user_id}
responses.GET, url=f"{DiscordClient.base_url}/users/@me", json={"id": self.user_id}
)

result = provider._get_discord_user_id("auth_code", "1")

assert result == user_id
assert result == self.user_id

@responses.activate
def test_get_discord_user_id_oauth_failure(self):
Expand Down

0 comments on commit c2a7bf7

Please sign in to comment.