Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/sentry/integrations/messaging/linkage.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
from sentry.utils import metrics
from sentry.utils.signing import unsign
from sentry.web.client_config import get_client_config
from sentry.web.frontend.base import BaseView, cell_silo_view, control_silo_view
from sentry.web.frontend.base import BaseView, control_silo_view
from sentry.web.helpers import render_to_response

logger = logging.getLogger("sentry.integrations.messaging.linkage")
Expand Down Expand Up @@ -329,7 +329,6 @@ def persist_identity(
return None


@cell_silo_view
class TeamLinkageView(LinkageView, ABC):
@classmethod
def is_valid_role(cls, org_member: OrganizationMember) -> bool:
Expand Down
53 changes: 45 additions & 8 deletions src/sentry/integrations/slack/views/link_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@
from typing import Any

from django import forms
from django.http import HttpRequest
from django.http.response import HttpResponseBase
from django.utils.decorators import method_decorator
from django.views.decorators.cache import never_cache

from sentry import options
from sentry.api.utils import generate_locality_url
from sentry.integrations.messaging.linkage import LinkTeamView
from sentry.integrations.models.integration import Integration
from sentry.integrations.services.integration import RpcIntegration
from sentry.integrations.slack.views.linkage import SlackLinkageView
from sentry.models.team import Team
from sentry.silo.base import SiloMode
from sentry.web.frontend.base import cell_silo_view
from sentry.web.frontend.base import all_silo_view
from sentry.web.frontend.react_page import ReactMixin

from . import build_linking_url as base_build_linking_url

Expand All @@ -37,14 +43,24 @@ def build_team_linking_url(
channel_id=channel_id,
channel_name=channel_name,
response_url=response_url,
# TODO(cells): This is broken for a multi-cell locality as the router cannot identify
# the correct cell silo for routing. The endpoint should be moved to the control silo.
url_prefix=(
generate_locality_url() if SiloMode.get_current_mode() == SiloMode.CELL else None
),
url_prefix=_team_linking_url_prefix(),
)


def _team_linking_url_prefix() -> str | None:
# The React page finds each organization's cell itself, so it is served
# from the primary domain.
if options.get("slack.link-team-react-enabled"):
return None

# TODO(cells): This is broken for a multi-cell locality as the router cannot identify
# the correct cell silo for routing.
if SiloMode.get_current_mode() == SiloMode.CELL:
return generate_locality_url()

return None


class SelectTeamForm(forms.Form):
team = forms.ChoiceField(label="Team")

Expand All @@ -57,8 +73,29 @@ def __init__(self, teams: Sequence[Team], *args: Any, **kwargs: Any):
team_field.widget.choices = team_field.choices


@cell_silo_view
class SlackLinkTeamView(SlackLinkageView, LinkTeamView):
@all_silo_view
class SlackLinkTeamView(SlackLinkageView, LinkTeamView, ReactMixin):
"""
Django view for linking team to slack channel. Creates an entry on ExternalActor table.

With `slack.link-team-react-enabled`, GET serves the React page from the
control silo. Everything else still runs in the cell.
"""

@method_decorator(never_cache)
def handle(self, request: HttpRequest, signed_params: str) -> HttpResponseBase:
# The Slack request parser only sends GET requests for the React page
# to control, after checking the option itself.
if SiloMode.get_current_mode() == SiloMode.CONTROL:
if request.method != "GET":
return self.render_error_page(request, status=404, body_text="HTTP 404: Not found")
return self.handle_react(request)

if is_react_team_link_request(request):
return self.handle_react(request)

return super().handle(request, signed_params)


def is_react_team_link_request(request: HttpRequest) -> bool:
return request.method == "GET" and options.get("slack.link-team-react-enabled")
36 changes: 20 additions & 16 deletions src/sentry/integrations/slack/webhooks/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import options
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import cell_silo_endpoint
from sentry.identity.services.identity.service import identity_service
from sentry.identity.slack.provider import PREFERRED_ORGANIZATION_ID_KEY
from sentry.integrations.models.external_actor import ExternalActor
from sentry.integrations.services.integration import RpcIntegration
from sentry.integrations.slack.message_builder.disconnected import SlackDisconnectedMessageBuilder
from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError
from sentry.integrations.slack.requests.command import SlackCommandRequest
Expand All @@ -23,6 +25,7 @@
from sentry.models.organization import OrganizationStatus
from sentry.models.organizationmember import InviteStatus, OrganizationMember
from sentry.models.organizationmemberteam import OrganizationMemberTeam
from sentry.users.services.user import RpcUser

_logger = logging.getLogger("sentry.integration.slack.bot-commands")

Expand Down Expand Up @@ -69,6 +72,18 @@ def get_orgs_with_teams_linked_to_channel(
)


def has_link_team_role(integration: RpcIntegration, user: RpcUser) -> bool:
organization_memberships = OrganizationMember.objects.get_for_integration(integration, user)

# Batch check for team admin roles to avoid N+1 queries
team_admin_member_ids = get_team_admin_member_ids(organization_memberships)

return any(
is_valid_role(membership) or membership.id in team_admin_member_ids
for membership in organization_memberships
)


def get_team_admin_member_ids(org_members: Iterable[OrganizationMember]) -> set[int]:
return set(
OrganizationMemberTeam.objects.filter(
Expand Down Expand Up @@ -109,23 +124,12 @@ def link_team(self, slack_request: SlackDMRequest) -> Response:

integration = slack_request.integration
logger_params["integration_id"] = integration.id
organization_memberships = OrganizationMember.objects.get_for_integration(
integration, identity_user
)

# Batch check for team admin roles to avoid N+1 queries
team_admin_member_ids = get_team_admin_member_ids(organization_memberships)

has_valid_role = False
for organization_membership in organization_memberships:
if (
is_valid_role(organization_membership)
or organization_membership.id in team_admin_member_ids
):
has_valid_role = True
break

if not has_valid_role:
# This cell only sees its own memberships. The React page lists
# linkable teams from every cell, so leave the role check to it.
if not options.get("slack.link-team-react-enabled") and not has_link_team_role(
integration, identity_user
):
return self.reply(slack_request, INSUFFICIENT_ROLE_MESSAGE)

if not slack_request.user_id:
Expand Down
8 changes: 7 additions & 1 deletion src/sentry/middleware/integrations/parsers/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
from sentry.integrations.slack.sdk_client import SlackSdkClient
from sentry.integrations.slack.views import SALT
from sentry.integrations.slack.views.link_identity import SlackLinkIdentityView
from sentry.integrations.slack.views.link_team import SlackLinkTeamView
from sentry.integrations.slack.views.link_team import (
SlackLinkTeamView,
is_react_team_link_request,
)
from sentry.integrations.slack.views.unlink_identity import SlackUnlinkIdentityView
from sentry.integrations.slack.views.unlink_team import SlackUnlinkTeamView
from sentry.integrations.slack.webhooks.action import (
Expand Down Expand Up @@ -437,6 +440,9 @@ def _get_response(self) -> HttpResponseBase:
if self.view_class in self.control_classes:
return self.get_response_from_control_silo()

if self.view_class == SlackLinkTeamView and is_react_team_link_request(self.request):
return self.get_response_from_control_silo()

# Handle event interactions challenge request
data = None
try:
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,8 @@
register("slack.debug-channel", flags=FLAG_AUTOMATOR_MODIFIABLE)
# Log unfurl payloads for debugging
register("slack.log-unfurl-payload", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE)
# Send `/sentry link team` to the React team link page served from control
register("slack.link-team-react-enabled", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE)
# Frequency of slack nudge blocks on issue alerts (0.0 to 1.0, where 0.3 = 30%)
register(
"slack.nudge-frequency",
Expand Down
1 change: 1 addition & 0 deletions src/sentry/web/frontend/react_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"sentry-auth-organization",
"sentry-oauth-authorize",
"sentry-oauth-device",
"sentry-integration-slack-link-team",
]


Expand Down
101 changes: 100 additions & 1 deletion tests/sentry/integrations/slack/test_link_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
import pytest
from django.db.models import QuerySet
from django.http.response import HttpResponseBase
from django.urls import reverse
from rest_framework import status
from slack_sdk.web import SlackResponse

from sentry import options
from sentry.integrations.models.external_actor import ExternalActor
from sentry.integrations.slack.views import SALT
from sentry.integrations.slack.views.link_team import build_team_linking_url
from sentry.integrations.slack.views.unlink_team import build_team_unlinking_url
from sentry.integrations.types import ExternalProviders
Expand All @@ -19,7 +22,14 @@
from sentry.silo.base import SiloMode
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers import add_identity, install_slack, link_team
from sentry.testutils.silo import assume_test_silo_mode
from sentry.testutils.helpers.options import override_options
from sentry.testutils.silo import (
assume_test_silo_mode,
cell_silo_test,
control_silo_test,
create_test_cells,
)
from sentry.utils.signing import sign


class SlackIntegrationLinkTeamTestBase(TestCase):
Expand Down Expand Up @@ -127,6 +137,44 @@ def setUp(self) -> None:
organization=self.organization, name="Mariachi Band", members=[self.user]
)

def build_url_in_cell(self) -> str:
with (
assume_test_silo_mode(SiloMode.CELL),
patch(
"sentry.integrations.slack.views.link_team.generate_locality_url",
return_value="https://de.example.com",
),
):
return build_team_linking_url(
integration=self.integration,
slack_id=self.external_id,
channel_id=self.channel_id,
channel_name=self.channel_name,
response_url=self.response_url,
)

def test_linking_url_uses_the_cell_locality(self) -> None:
assert self.build_url_in_cell().startswith("https://de.example.com/extensions/slack/")

@override_options({"slack.link-team-react-enabled": True})
def test_react_linking_url_uses_the_primary_domain(self) -> None:
url_prefix = options.get("system.url-prefix")

assert self.build_url_in_cell().startswith(f"{url_prefix}/extensions/slack/")

@override_options({"slack.link-team-react-enabled": True})
def test_serves_the_react_page(self) -> None:
response = self.get_success_response()

self.assertTemplateUsed(response, "sentry/base-react.html")

@override_options({"slack.link-team-react-enabled": True})
def test_links_through_the_form_with_the_react_page_enabled(self) -> None:
response = self.get_success_response(data={"team": self.team.id})

self.assertTemplateUsed(response, "sentry/integrations/slack/post-linked-team.html")
assert len(self.get_linked_teams()) == 1

def test_link_team(self) -> None:
"""Test that we successfully link a team to a Slack channel"""
response = self.get_success_response()
Expand Down Expand Up @@ -207,6 +255,57 @@ def test_link_team_multiple_organizations(self) -> None:
assert len(external_actors) == 1


@control_silo_test
class SlackLinkTeamControlSiloTest(TestCase):
def setUp(self) -> None:
super().setUp()
self.login_as(self.user)
integration = install_slack(self.organization)
self.url = build_team_linking_url(
integration=integration,
slack_id="new-slack-id",
channel_id="my-channel_id",
channel_name="my-channel",
response_url="http://example.slack.com/response_url",
)

@override_options({"slack.link-team-react-enabled": True})
def test_serves_the_react_page(self) -> None:
response = self.client.get(self.url)

assert response.status_code == 200
self.assertTemplateUsed(response, "sentry/base-react.html")
assert "no-store" in response["Cache-Control"]

@override_options({"slack.link-team-react-enabled": True})
def test_sends_anonymous_users_to_log_in(self) -> None:
self.client.logout()

response = self.client.get(self.url)

assert response.status_code == 302
assert response["Location"].startswith("/auth/login/")


@cell_silo_test(cells=create_test_cells("us"))
class SlackLinkTeamCellLocalityTest(TestCase):
@override_options({"slack.link-team-react-enabled": True})
def test_sends_old_cell_locality_links_to_the_primary_domain(self) -> None:
self.login_as(self.user)
with assume_test_silo_mode(SiloMode.CONTROL):
integration = install_slack(self.organization)
path = reverse(
"sentry-integration-slack-link-team",
kwargs={"signed_params": sign(salt=SALT, integration_id=integration.id)},
)

with self.feature({"system:multi-region": True}):
response = self.client.get(path, HTTP_HOST="us.testserver")

assert response.status_code == 302
assert response["Location"] == f"http://testserver{path}"


class SlackIntegrationUnlinkTeamTestWithSdk(SlackIntegrationLinkTeamTestBase):
def setUp(self) -> None:
super().setUp()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from sentry.silo.base import SiloMode
from sentry.testutils.asserts import assert_slo_metric
from sentry.testutils.helpers import get_response_text, link_user
from sentry.testutils.helpers.options import override_options
from sentry.testutils.silo import assume_test_silo_mode
from tests.sentry.integrations.slack.webhooks.commands import SlackCommandsTest

Expand Down Expand Up @@ -118,6 +119,19 @@ def test_link_team_insufficient_role(self, mock_record: MagicMock) -> None:

assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)

@responses.activate
@override_options({"slack.link-team-react-enabled": True})
def test_link_team_leaves_the_role_check_to_the_react_page(self) -> None:
user2 = self.create_user()
self.create_member(
teams=[self.team], user=user2, role="member", organization=self.organization
)
self.login_as(user2)
link_user(user2, self.idp, slack_id=OTHER_SLACK_ID)

data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID)
assert "Link your Sentry team to this Slack channel!" in get_response_text(data)

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_link_team_as_team_admin(self, mock_record: MagicMock) -> None:
Expand Down
Loading
Loading