Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use CustomURLValidator in custom_button #1398

Merged
merged 20 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
24b89af
use CustomURLValidator in custom_button
hoptical Feb 23, 2023
e03cfeb
Merge branch 'dev' into custom_url_validator
joeyorlando Feb 24, 2023
4e17c18
Merge branch 'dev' into custom_url_validator
hoptical Mar 2, 2023
841e8ba
change urlvalidator class name and use customvalidator for DANGEROUS_…
hoptical Mar 2, 2023
d73b498
Merge branch 'dev' into custom_url_validator
joeyorlando Mar 6, 2023
44531a3
add test_urlvalidator_without_tld as unit test
hoptical Mar 20, 2023
d87e819
Merge branch 'custom_url_validator' of github.com:hoptical/oncall int…
hoptical Mar 20, 2023
82fd45a
Merge branch 'dev' into custom_url_validator
joeyorlando Mar 20, 2023
cbd6fb4
use more efficient overriding for URLValidatorWithoutTLD
hoptical Mar 20, 2023
2987abb
add api test for URLValidatorWithoutTLD
hoptical Mar 20, 2023
34e52e5
Merge branch 'grafana:dev' into custom_url_validator
hoptical Mar 20, 2023
4d44060
Merge branch 'dev' of github.com:hoptical/oncall into custom_url_vali…
hoptical Mar 20, 2023
5f434b7
Merge branch 'custom_url_validator' of github.com:hoptical/oncall int…
hoptical Mar 20, 2023
789d2fa
update changelog for URLValidatorWithoutTLD
hoptical Mar 20, 2023
dfd7d66
Update CHANGELOG.md
Konstantinov-Innokentii Mar 21, 2023
8fe4d56
Lint
Konstantinov-Innokentii Mar 21, 2023
231e883
Merge branch 'dev' into custom_url_validator
joeyorlando Mar 23, 2023
6f74906
update changelog
joeyorlando Mar 23, 2023
bae5cc3
remove extra unit test introduced from
joeyorlando Mar 23, 2023
1512009
Merge branch 'dev' into custom_url_validator
joeyorlando Mar 23, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Modified `check_escalation_finished_task` celery task to use read-only databases for its query, if one is defined +
make the validation logic stricter + ping a configurable heartbeat on successful completion of this task ([1266](https://github.com/grafana/oncall/pull/1266))

- Add `URLValidatorWithoutTLD` for domains without TLDs like container hostnames. This will let them to be added as outgoing webhooks if `DANGEROUS_WEBHOOKS_ENABLED` is set to true ([1398](https://github.com/grafana/oncall/pull/1398)).
### Changed

- Updated wording throughout plugin to use 'Alert Group' instead of 'Incident' ([1565](https://github.com/grafana/oncall/pull/1565),
Expand Down
9 changes: 7 additions & 2 deletions engine/apps/api/serializers/custom_button.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from collections import defaultdict

from django.core.validators import URLValidator, ValidationError
from django.core.validators import ValidationError, URLValidator
from rest_framework import serializers
from rest_framework.validators import UniqueTogetherValidator

Expand All @@ -9,6 +9,8 @@
from common.api_helpers.utils import CurrentOrganizationDefault, CurrentTeamDefault
from common.jinja_templater import apply_jinja_template
from common.jinja_templater.apply_jinja_template import JinjaTemplateError, JinjaTemplateWarning
from common.api_helpers.utils import URLValidatorWithoutTLD
from apps.base.utils import live_settings


class CustomButtonSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -41,7 +43,10 @@ class Meta:
def validate_webhook(self, webhook):
if webhook:
try:
URLValidator()(webhook)
if live_settings.DANGEROUS_WEBHOOKS_ENABLED:
URLValidatorWithoutTLD()(webhook)
else:
URLValidator()(webhook)
except ValidationError:
raise serializers.ValidationError("Webhook is incorrect")
return webhook
Expand Down
35 changes: 35 additions & 0 deletions engine/apps/api/tests/test_custom_button.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from apps.api.permissions import LegacyAccessControlRole

TEST_URL = "https://amixr.io"
URL_WITH_TLD = "http://www.google.com"
URL_WITHOUT_TLD = "http://container:8080"


@pytest.fixture()
Expand Down Expand Up @@ -477,3 +479,36 @@ def test_custom_button_from_other_team_without_flag(

response = client.get(url, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_403_FORBIDDEN


@pytest.mark.django_db
@pytest.mark.parametrize(
"dangerous_webhooks,webhook_url,expected_status",
[
(True, URL_WITH_TLD, status.HTTP_201_CREATED),
(True, URL_WITHOUT_TLD, status.HTTP_201_CREATED),
(False, URL_WITH_TLD, status.HTTP_201_CREATED),
(False, URL_WITHOUT_TLD, status.HTTP_400_BAD_REQUEST),
],
)
def test_url_without_tld_custom_button(
custom_button_internal_api_setup,
make_user_auth_headers,
settings,
dangerous_webhooks,
webhook_url,
expected_status,
):
settings.DANGEROUS_WEBHOOKS_ENABLED = dangerous_webhooks

user, token, _ = custom_button_internal_api_setup
client = APIClient()
url = reverse("api-internal:custom_button-list")

data = {
"name": "amixr_button",
"webhook": webhook_url,
"team": None,
}
response = client.post(url, data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == expected_status
31 changes: 31 additions & 0 deletions engine/common/api_helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
from urllib.parse import urljoin

import requests
import re
from django.core.validators import URLValidator
from django.utils.regex_helper import _lazy_re_compile
from django.conf import settings
from django.utils import dateparse, timezone
from icalendar import Calendar
Expand Down Expand Up @@ -45,6 +48,34 @@ def __repr__(self):
return "%s()" % self.__class__.__name__


class URLValidatorWithoutTLD(URLValidator):
"""
Overrides Django URLValidator Regex. It removes the tld part because
most of the time, containers don't have any TLD in their urls and such outgoing webhooks
can't be registered.
"""

host_re = (
"("
+ URLValidator.hostname_re
+ URLValidator.domain_re
+ URLValidator.tld_re
+ "|"
+ URLValidator.hostname_re
+ "|localhost)"
)

regex = _lazy_re_compile(
r"^(?:[a-z0-9.+-]*)://" # scheme is validated separately
r"(?:[^\s:@/]+(?::[^\s:@/]*)?@)?" # user:pass authentication
r"(?:" + URLValidator.ipv4_re + "|" + URLValidator.ipv6_re + "|" + host_re + ")"
r"(?::[0-9]{1,5})?" # port
r"(?:[/?#][^\s]*)?" # resource path
r"\Z",
re.IGNORECASE,
)


class CurrentUserDefault:
"""
Utility class to get the current user right from the serializer field.
Expand Down
19 changes: 19 additions & 0 deletions engine/common/tests/test_urlvalidator_without_tld.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from common.api_helpers.utils import URLValidatorWithoutTLD
import pytest
from django.core.validators import ValidationError

valid_urls = ["https://www.google.com", "https://www.google", "http://conatainer1"]
invalid_urls = ["https:/www.google.com", "htt://www.google.com/"]


@pytest.mark.parametrize("url", valid_urls)
def test_urlvalidator_without_tld_valid_urls(url):
# Test valid URLs
URLValidatorWithoutTLD()(url)


@pytest.mark.parametrize("url", invalid_urls)
def test_urlvalidator_without_tld_invalid_urls(url):
# Test an invalid URL
with pytest.raises(ValidationError):
URLValidatorWithoutTLD()(url)