Skip to content

Commit ca38577

Browse files
kconsandrewshie-sentry
authored andcommitted
feat(aci): Reject attempts to set Action type to non-permitted integration actions (#91317)
Add a new `is_action_permitted` method for checking organization permissions to use action types. Extend the action validator to require any action type being chosen to be permitted.
1 parent 8c2dbad commit ca38577

File tree

6 files changed

+151
-5
lines changed

6 files changed

+151
-5
lines changed

src/sentry/workflow_engine/endpoints/validators/base/action.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from sentry.api.serializers.rest_framework import CamelSnakeSerializer
66
from sentry.workflow_engine.endpoints.validators.utils import validate_json_schema
77
from sentry.workflow_engine.models import Action
8+
from sentry.workflow_engine.processors.action import is_action_permitted
89
from sentry.workflow_engine.registry import action_handler_registry
910
from sentry.workflow_engine.types import ActionHandler
1011

@@ -30,12 +31,32 @@ def validate_config(self, value) -> ActionConfig:
3031
config_schema = self._get_action_handler().config_schema
3132
return validate_json_schema(value, config_schema)
3233

34+
def validate_type(self, value) -> Any:
35+
try:
36+
action_type = Action.Type(value)
37+
except ValueError:
38+
raise serializers.ValidationError(f"Invalid action type: {value}")
39+
self._check_action_type(action_type)
40+
return value
41+
42+
def _check_action_type(self, action_type: Action.Type) -> None:
43+
organization = self.context.get("organization")
44+
if not organization:
45+
# ¯\_(ツ)_/¯
46+
# TODO(kylec): Require organization to be in the context.
47+
return
48+
if not is_action_permitted(action_type, organization):
49+
raise serializers.ValidationError(
50+
f"Organization does not allow this action type: {action_type}"
51+
)
52+
3353
def create(self, validated_value: dict[str, Any]) -> Action:
34-
"""
35-
TODO @saponifi3d -- add any org checks for creating actions here
36-
"""
54+
""" """
55+
self._check_action_type(Action.Type(validated_value["type"]))
3756
return Action.objects.create(**validated_value)
3857

3958
def update(self, instance: Action, validated_value: dict[str, Any]) -> Action:
59+
if instance.type != validated_value["type"]:
60+
self._check_action_type(Action.Type(validated_value["type"]))
4061
instance.update(**validated_value)
4162
return instance

src/sentry/workflow_engine/endpoints/validators/base/workflow.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ def validate_action_filters(self, value: ListInputData) -> ListInputData:
5757
BaseDataConditionGroupValidator(data=condition_group).is_valid(raise_exception=True)
5858

5959
for action in actions:
60-
BaseActionValidator(data=action).is_valid(raise_exception=True)
60+
BaseActionValidator(data=action, context=self.context).is_valid(
61+
raise_exception=True
62+
)
6163

6264
return value
6365

src/sentry/workflow_engine/models/action.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,18 @@ class Type(StrEnum):
5757
PLUGIN = "plugin"
5858
WEBHOOK = "webhook"
5959

60+
def is_integration(self) -> bool:
61+
"""
62+
Returns True if the action is an integration action.
63+
For those, the value should correspond to the integration key.
64+
"""
65+
return self not in [
66+
Action.Type.EMAIL,
67+
Action.Type.SENTRY_APP,
68+
Action.Type.PLUGIN,
69+
Action.Type.WEBHOOK,
70+
]
71+
6072
# The type field is used to denote the type of action we want to trigger
6173
type = models.TextField(choices=[(t.value, t.value) for t in Type])
6274
data = models.JSONField(default=dict)

src/sentry/workflow_engine/processors/action.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@
77
from django.db.models.functions import Cast, Coalesce
88
from django.utils import timezone
99

10+
from sentry import features
1011
from sentry.constants import ObjectStatus
1112
from sentry.db.models.manager.base_query_set import BaseQuerySet
13+
from sentry.exceptions import NotRegistered
14+
from sentry.integrations.base import IntegrationFeatures
15+
from sentry.integrations.manager import default_manager as integrations_manager
1216
from sentry.integrations.services.integration import RpcIntegration, integration_service
1317
from sentry.integrations.types import IntegrationProviderSlug
1418
from sentry.models.group import Group
@@ -188,3 +192,45 @@ def get_integration_services(organization_id: int) -> dict[int, list[tuple[int,
188192
)
189193

190194
return services
195+
196+
197+
def _get_integration_features(action_type: Action.Type) -> frozenset[IntegrationFeatures]:
198+
"""
199+
Get the IntegrationFeatures for an integration-based action type.
200+
"""
201+
assert action_type.is_integration()
202+
integration_key = action_type.value # action types should be match integration keys.
203+
try:
204+
integration = integrations_manager.get(integration_key)
205+
except NotRegistered:
206+
raise ValueError(f"No integration found for action type: {action_type}")
207+
return integration.features
208+
209+
210+
# The features that are relevent to Action behaviors;
211+
# if the organization doesn't have access to all of the features an integration
212+
# requires that are in this list, the action should not be permitted.
213+
_ACTION_RELEVANT_INTEGRATION_FEATURES = {
214+
IntegrationFeatures.ISSUE_BASIC,
215+
IntegrationFeatures.ISSUE_SYNC,
216+
IntegrationFeatures.TICKET_RULES,
217+
IntegrationFeatures.ALERT_RULE,
218+
IntegrationFeatures.ENTERPRISE_ALERT_RULE,
219+
IntegrationFeatures.ENTERPRISE_INCIDENT_MANAGEMENT,
220+
IntegrationFeatures.INCIDENT_MANAGEMENT,
221+
}
222+
223+
224+
def is_action_permitted(action_type: Action.Type, organization: Organization) -> bool:
225+
"""
226+
Check if an action type is permitted for an organization.
227+
"""
228+
if not action_type.is_integration():
229+
return True
230+
integration_features = _get_integration_features(action_type)
231+
required_org_features = integration_features.intersection(_ACTION_RELEVANT_INTEGRATION_FEATURES)
232+
feature_names = [
233+
f"organizations:integrations-{integration_feature}"
234+
for integration_feature in required_org_features
235+
]
236+
return all(features.has(feature_name, organization) for feature_name in feature_names)

tests/sentry/workflow_engine/endpoints/validators/test_base_action.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
from unittest import TestCase, mock
1+
from unittest import mock
22

3+
from sentry.testutils.cases import TestCase
34
from sentry.workflow_engine.endpoints.validators.base import BaseActionValidator
45
from sentry.workflow_engine.models import Action
56
from tests.sentry.workflow_engine.test_base import MockActionHandler
@@ -84,3 +85,24 @@ def test_validate_data__invalid(self, mock_action_handler_get):
8485

8586
result = validator.is_valid()
8687
assert result is False
88+
89+
def test_validate_type__action_gated(self, mock_action_handler_get):
90+
organization = self.create_organization()
91+
92+
def make_validator():
93+
return BaseActionValidator(
94+
context={"organization": organization},
95+
data={
96+
**self.valid_data,
97+
"type": Action.Type.SLACK,
98+
},
99+
)
100+
101+
validator = make_validator()
102+
with self.feature({"organizations:integrations-alert-rule": False}):
103+
assert not validator.is_valid()
104+
105+
# Exact same one, but new, because validation is cached.
106+
validator2 = make_validator()
107+
with self.feature("organizations:integrations-alert-rule"):
108+
assert validator2.is_valid()

tests/sentry/workflow_engine/processors/test_action.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
from datetime import timedelta
2+
from unittest.mock import patch
23

34
from django.utils import timezone
45

6+
from sentry.integrations.base import IntegrationFeatures
57
from sentry.testutils.helpers.datetime import freeze_time
68
from sentry.workflow_engine.models import Action, DataConditionGroup, WorkflowFireHistory
79
from sentry.workflow_engine.models.action_group_status import ActionGroupStatus
810
from sentry.workflow_engine.processors.action import (
911
create_workflow_fire_histories,
1012
filter_recently_fired_workflow_actions,
13+
is_action_permitted,
1114
)
1215
from sentry.workflow_engine.types import WorkflowEventData
1316
from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
@@ -96,3 +99,43 @@ def test_create_workflow_fire_histories(self):
9699
).count()
97100
== 1
98101
)
102+
103+
104+
class TestIsActionPermitted(BaseWorkflowTest):
105+
@patch("sentry.workflow_engine.processors.action._get_integration_features")
106+
def test_basic(self, mock_get_features):
107+
org = self.create_organization()
108+
109+
# Test non-integration actions (should always be permitted)
110+
assert is_action_permitted(Action.Type.EMAIL, org)
111+
assert is_action_permitted(Action.Type.SENTRY_APP, org)
112+
113+
# Single rule.
114+
mock_get_features.return_value = {IntegrationFeatures.ALERT_RULE}
115+
with self.feature({"organizations:integrations-alert-rule": False}):
116+
assert not is_action_permitted(Action.Type.SLACK, org)
117+
118+
with self.feature("organizations:integrations-alert-rule"):
119+
assert is_action_permitted(Action.Type.SLACK, org)
120+
121+
# Multiple required features.
122+
mock_get_features.return_value = {
123+
IntegrationFeatures.ALERT_RULE,
124+
IntegrationFeatures.ISSUE_BASIC,
125+
}
126+
with self.feature(
127+
{
128+
"organizations:integrations-alert-rule": True,
129+
"organizations:integrations-issue-basic": False,
130+
}
131+
):
132+
assert not is_action_permitted(Action.Type.JIRA, org)
133+
134+
# Both need to be enabled for permission.
135+
with self.feature(
136+
{
137+
"organizations:integrations-alert-rule": True,
138+
"organizations:integrations-issue-basic": True,
139+
}
140+
):
141+
assert is_action_permitted(Action.Type.JIRA, org)

0 commit comments

Comments
 (0)