Skip to content

Commit 0edf083

Browse files
DominikB2014claude
andcommitted
fix(slack): Require linked identity for explore unfurl
Explore URL unfurls were bypassing the identity check that Discover unfurls enforce. Unlinked Slack users could trigger unfurls without a linked Sentry account. Add the same identity validation so users are prompted to link their account before explore charts are unfurled. Refs DAIN-1438 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 20749fb commit 0edf083

File tree

4 files changed

+140
-3
lines changed

4 files changed

+140
-3
lines changed

src/sentry/integrations/slack/requests/event.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ def has_discover_links(links: list[str]) -> bool:
1414
return any(match_link(link)[0] == LinkType.DISCOVER for link in links)
1515

1616

17+
def has_explore_links(links: list[str]) -> bool:
18+
return any(match_link(link)[0] == LinkType.EXPLORE for link in links)
19+
20+
1721
def is_event_challenge(data: Mapping[str, Any]) -> bool:
1822
return data.get("type", "") == "url_verification"
1923

@@ -79,7 +83,8 @@ def validate_integration(self) -> None:
7983
super().validate_integration()
8084

8185
if (self.text in COMMANDS) or (
82-
self.type == "link_shared" and has_discover_links(self.links)
86+
self.type == "link_shared"
87+
and (has_discover_links(self.links) or has_explore_links(self.links))
8388
):
8489
self._validate_identity()
8590

src/sentry/integrations/slack/unfurl/explore.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ def _unfurl_explore(
158158

159159
group_bys = params.getlist("groupBy")
160160

161-
# Determine display mode based on whether groupBy is present
161+
# Only the first yAxis is charted; multiple charts per unfurl not yet supported
162162
if group_bys:
163163
y_axis = y_axes[0]
164164
aggregate_fn = y_axis.split("(")[0]

src/sentry/integrations/slack/webhooks/event.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ def _get_unfurlable_links(
197197

198198
if (
199199
organization
200-
and link_type == LinkType.DISCOVER
200+
and link_type in (LinkType.DISCOVER, LinkType.EXPLORE)
201201
and not slack_request.has_identity
202202
and features.has(
203203
"organizations:discover-basic", organization, actor=request.user
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import re
2+
from unittest.mock import Mock, patch
3+
4+
import orjson
5+
import pytest
6+
from slack_sdk.web import SlackResponse
7+
8+
from sentry.integrations.slack.unfurl.types import Handler, LinkType, make_type_coercer
9+
from sentry.silo.base import SiloMode
10+
from sentry.testutils.silo import assume_test_silo_mode
11+
from sentry.users.models.identity import Identity, IdentityStatus
12+
13+
from . import LINK_SHARED_EVENT, BaseEventTest
14+
15+
16+
class ExploreLinkSharedEvent(BaseEventTest):
17+
@pytest.fixture(autouse=True)
18+
def mock_chat_postEphemeral(self):
19+
with patch(
20+
"slack_sdk.web.client.WebClient.chat_postEphemeral",
21+
return_value=SlackResponse(
22+
client=None,
23+
http_verb="POST",
24+
api_url="https://slack.com/api/chat.postEphemeral",
25+
req_args={},
26+
data={"ok": True},
27+
headers={},
28+
status_code=200,
29+
),
30+
) as self.mock_post:
31+
yield
32+
33+
@pytest.fixture(autouse=True)
34+
def mock_chat_unfurl(self):
35+
with patch(
36+
"slack_sdk.web.client.WebClient.chat_unfurl",
37+
return_value=SlackResponse(
38+
client=None,
39+
http_verb="POST",
40+
api_url="https://slack.com/api/chat.unfurl",
41+
req_args={},
42+
data={"ok": True},
43+
headers={},
44+
status_code=200,
45+
),
46+
) as self.mock_unfurl:
47+
yield
48+
49+
@patch(
50+
"sentry.integrations.slack.webhooks.event.match_link",
51+
side_effect=[
52+
(LinkType.EXPLORE, {"arg1": "value1"}),
53+
(LinkType.EXPLORE, {"arg1", "value2"}),
54+
(LinkType.EXPLORE, {"arg1": "value1"}),
55+
],
56+
)
57+
@patch("sentry.integrations.slack.requests.event.has_explore_links", return_value=True)
58+
@patch(
59+
"sentry.integrations.slack.webhooks.event.link_handlers",
60+
{
61+
LinkType.EXPLORE: Handler(
62+
matcher=[re.compile(r"test")],
63+
arg_mapper=make_type_coercer({}),
64+
fn=Mock(return_value={"link1": "unfurl", "link2": "unfurl"}),
65+
)
66+
},
67+
)
68+
def share_explore_links_sdk(self, mock_match_link, mock_):
69+
resp = self.post_webhook(event_data=orjson.loads(LINK_SHARED_EVENT))
70+
assert resp.status_code == 200, resp.content
71+
return self.mock_unfurl.call_args[1]
72+
73+
@patch(
74+
"sentry.integrations.slack.webhooks.event.match_link",
75+
side_effect=[
76+
(LinkType.EXPLORE, {"arg1": "value1"}),
77+
(LinkType.EXPLORE, {"arg1", "value2"}),
78+
(LinkType.EXPLORE, {"arg1": "value1"}),
79+
],
80+
)
81+
@patch("sentry.integrations.slack.requests.event.has_explore_links", return_value=True)
82+
@patch(
83+
"sentry.integrations.slack.webhooks.event.link_handlers",
84+
{
85+
LinkType.EXPLORE: Handler(
86+
matcher=[re.compile(r"test")],
87+
arg_mapper=make_type_coercer({}),
88+
fn=Mock(return_value={"link1": "unfurl", "link2": "unfurl"}),
89+
)
90+
},
91+
)
92+
def share_explore_links_ephemeral_sdk(self, mock_match_link, mock_):
93+
resp = self.post_webhook(event_data=orjson.loads(LINK_SHARED_EVENT))
94+
assert resp.status_code == 200, resp.content
95+
return self.mock_post.call_args[1]
96+
97+
def test_share_explore_links_unlinked_user(self) -> None:
98+
with assume_test_silo_mode(SiloMode.CONTROL):
99+
self.create_identity_provider(type="slack", external_id="TXXXXXXX1")
100+
with self.feature("organizations:discover-basic"):
101+
data = self.share_explore_links_ephemeral_sdk()
102+
103+
blocks = orjson.loads(data["blocks"])
104+
105+
assert blocks[0]["type"] == "section"
106+
assert (
107+
blocks[0]["text"]["text"]
108+
== "Link your Slack identity to Sentry to unfurl Discover charts."
109+
)
110+
111+
assert blocks[1]["type"] == "actions"
112+
assert len(blocks[1]["elements"]) == 2
113+
assert [button["text"]["text"] for button in blocks[1]["elements"]] == ["Link", "Cancel"]
114+
115+
def test_share_explore_links_linked_user(self) -> None:
116+
with assume_test_silo_mode(SiloMode.CONTROL):
117+
idp = self.create_identity_provider(type="slack", external_id="TXXXXXXX1")
118+
Identity.objects.create(
119+
external_id="Uxxxxxxx",
120+
idp=idp,
121+
user=self.user,
122+
status=IdentityStatus.VALID,
123+
scopes=[],
124+
)
125+
data = self.share_explore_links_sdk()
126+
127+
unfurls = data["unfurls"]
128+
129+
# We only have two unfurls since one link was duplicated
130+
assert len(unfurls) == 2
131+
assert unfurls["link1"] == "unfurl"
132+
assert unfurls["link2"] == "unfurl"

0 commit comments

Comments
 (0)