Skip to content

Commit

Permalink
Fix channel filter endpoint (#3192)
Browse files Browse the repository at this point in the history
# What this PR does
Add filtering term length check for channel filter endpoints

## Which issue(s) this PR fixes

Closes grafana/oncall-private#2114

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
  • Loading branch information
Ferril authored Oct 25, 2023
1 parent 9991c3c commit a3aeb0e
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Fixed

- Add filtering term length check for channel filter endpoints @Ferril ([#3192](https://github.com/grafana/oncall/pull/3192))

## v1.3.46 (2023-10-23)

### Added
Expand Down
4 changes: 3 additions & 1 deletion engine/apps/alerts/models/channel_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ class ChannelFilter(OrderedModel):
notification_backends = models.JSONField(null=True, default=None)

created_at = models.DateTimeField(auto_now_add=True)
filtering_term = models.CharField(max_length=1024, null=True, default=None)

FILTERING_TERM_MAX_LENGTH = 1024
filtering_term = models.CharField(max_length=FILTERING_TERM_MAX_LENGTH, null=True, default=None)

FILTERING_TERM_TYPE_REGEX = 0
FILTERING_TERM_TYPE_JINJA2 = 1
Expand Down
7 changes: 6 additions & 1 deletion engine/apps/api/serializers/channel_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ class Meta:
def validate(self, data):
filtering_term = data.get("filtering_term")
filtering_term_type = data.get("filtering_term_type")
if filtering_term is not None:
if len(filtering_term) > ChannelFilter.FILTERING_TERM_MAX_LENGTH:
raise serializers.ValidationError(
f"Expression is too long. Maximum length: {ChannelFilter.FILTERING_TERM_MAX_LENGTH} characters, "
f"current length: {len(filtering_term)}"
)
if filtering_term_type == ChannelFilter.FILTERING_TERM_TYPE_JINJA2:
try:
valid_jinja_template_for_serializer_method_field({"route_template": filtering_term})
Expand Down Expand Up @@ -141,7 +147,6 @@ def get_filtering_term_as_jinja2(self, obj):
class ChannelFilterCreateSerializer(ChannelFilterSerializer):
alert_receive_channel = OrganizationFilteredPrimaryKeyRelatedField(queryset=AlertReceiveChannel.objects)
slack_channel = serializers.CharField(allow_null=True, required=False, source="slack_channel_id")
filtering_term = serializers.CharField(required=False, allow_null=True, allow_blank=True)

class Meta:
model = ChannelFilter
Expand Down
38 changes: 38 additions & 0 deletions engine/apps/api/tests/test_channel_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,3 +552,41 @@ def test_channel_filter_convert_from_regex_to_jinja2(
assert jinja2_channel_filter.filtering_term == final_filtering_term
# Check if the same alert is matched to the channel filter (route) new jinja2
assert bool(jinja2_channel_filter.is_satisfying(payload)) is True


@pytest.mark.django_db
def test_channel_filter_long_filtering_term(
make_organization_and_user_with_plugin_token,
make_alert_receive_channel,
make_escalation_chain,
make_channel_filter,
make_user_auth_headers,
):
organization, user, token = make_organization_and_user_with_plugin_token()
alert_receive_channel = make_alert_receive_channel(organization)
make_escalation_chain(organization)
make_channel_filter(alert_receive_channel, is_default=True)
client = APIClient()
long_filtering_term = "a" * (ChannelFilter.FILTERING_TERM_MAX_LENGTH + 1)

url = reverse("api-internal:channel_filter-list")
data_for_creation = {
"alert_receive_channel": alert_receive_channel.public_primary_key,
"filtering_term": long_filtering_term,
}

response = client.post(url, data=data_for_creation, format="json", **make_user_auth_headers(user, token))

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "Expression is too long" in response.json()["non_field_errors"][0]

channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False)
url = reverse("api-internal:channel_filter-detail", kwargs={"pk": channel_filter.public_primary_key})
data_for_update = {
"filtering_term": long_filtering_term,
}

response = client.put(url, data=data_for_update, format="json", **make_user_auth_headers(user, token))

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "Expression is too long" in response.json()["non_field_errors"][0]
10 changes: 8 additions & 2 deletions engine/apps/public_api/serializers/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,14 @@ def create(self, validated_data):
return super().create(validated_data)

def validate(self, data):
filtering_term = data.get("routing_regex")
filtering_term_type = data.get("routing_type")
filtering_term = data.get("filtering_term")
filtering_term_type = data.get("filtering_term_type")
if filtering_term is not None:
if len(filtering_term) > ChannelFilter.FILTERING_TERM_MAX_LENGTH:
raise serializers.ValidationError(
f"Expression is too long. Maximum length: {ChannelFilter.FILTERING_TERM_MAX_LENGTH} characters, "
f"current length: {len(filtering_term)}"
)
if filtering_term_type == ChannelFilter.FILTERING_TERM_TYPE_JINJA2:
try:
valid_jinja_template_for_serializer_method_field({"route_template": filtering_term})
Expand Down
33 changes: 33 additions & 0 deletions engine/apps/public_api/tests/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,36 @@ def test_update_route_with_manual_ordering(

response = client.put(url, format="json", HTTP_AUTHORIZATION=token, data=data_to_update)
assert response.status_code == status.HTTP_400_BAD_REQUEST


@pytest.mark.django_db
def test_routes_long_filtering_term(
route_public_api_setup,
make_channel_filter,
):
organization, _, token, alert_receive_channel, escalation_chain, _ = route_public_api_setup
client = APIClient()
long_filtering_term = "a" * (ChannelFilter.FILTERING_TERM_MAX_LENGTH + 1)

url = reverse("api-public:routes-list")
data_for_creation = {
"integration_id": alert_receive_channel.public_primary_key,
"escalation_chain_id": escalation_chain.public_primary_key,
"routing_regex": long_filtering_term,
}

response = client.post(url, data=data_for_creation, format="json", HTTP_AUTHORIZATION=token)

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "Expression is too long" in response.json()["non_field_errors"][0]

channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False)
url = reverse("api-public:routes-detail", kwargs={"pk": channel_filter.public_primary_key})
data_for_update = {
"routing_regex": long_filtering_term,
}

response = client.put(url, data=data_for_update, format="json", HTTP_AUTHORIZATION=token)

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "Expression is too long" in response.json()["non_field_errors"][0]

0 comments on commit a3aeb0e

Please sign in to comment.