Skip to content

Commit

Permalink
Validate also the schema
Browse files Browse the repository at this point in the history
  • Loading branch information
amureki committed Jul 27, 2023
1 parent bba3d11 commit a741aec
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
13 changes: 11 additions & 2 deletions emark/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,17 @@ def get(self, request, *args, **kwargs):
# The redirect_to URL is user-provided, so it might be malicious
# or malformed. We use Django's URL validation to ensure that it
# is safe to redirect to.
host = urlparse(redirect_to).netloc
if not host or not validate_host(host, settings.ALLOWED_HOSTS):
parsed_url = urlparse(redirect_to)
allowed_hosts = settings.ALLOWED_HOSTS
if settings.DEBUG:
allowed_hosts = settings.ALLOWED_HOSTS + [".localhost", "127.0.0.1", "[::1]"]
if any(
[
not parsed_url.netloc
or not validate_host(parsed_url.netloc, allowed_hosts),
request.scheme != parsed_url.scheme,
]
):
return http.HttpResponseBadRequest("Missing url or malformed parameter")

models.Click.objects.create_for_request(
Expand Down
26 changes: 25 additions & 1 deletion tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,19 @@ def test_get__unsafe_redirect_url(self, client, live_server):
assert response.status_code == 400

@pytest.mark.django_db
def test_get__subdomain_redirect_url(self, client, live_server):
def test_get__different_schema_redirect_url(self, client, live_server):
msg = baker.make("emark.Send")
redirect_url = "https://sub.testserver/?utm_source=foo"

url = reverse("emark:email-click", kwargs={"pk": msg.pk})

url = f"{url}?{urlencode({'url': redirect_url})}"
response = client.get(url)
assert response.status_code == 400

@pytest.mark.django_db
def test_get__subdomain_redirect_url(self, client, live_server, settings):
settings.ALLOWED_HOSTS = ["testserver", ".testserver"]
msg = baker.make("emark.Send")
redirect_url = "http://sub.testserver/?utm_source=foo"

Expand All @@ -62,6 +74,18 @@ def test_get__subdomain_redirect_url(self, client, live_server):
response = client.get(url)
assert response.status_code == 302

@pytest.mark.django_db
def test_get__subdomain_debug(self, client, live_server, settings):
settings.DEBUG = True
msg = baker.make("emark.Send")
redirect_url = "http://sub.localhost/?utm_source=foo"

url = reverse("emark:email-click", kwargs={"pk": msg.pk})

url = f"{url}?{urlencode({'url': redirect_url})}"
response = client.get(url)
assert response.status_code == 302

@pytest.mark.django_db
def test_get__no_email(self, client):
response = client.get(reverse("emark:email-click", kwargs={"pk": uuid.uuid4()}))
Expand Down
5 changes: 1 addition & 4 deletions tests/testapp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = [
"testserver",
".testserver",
]
ALLOWED_HOSTS = []


# Application definition
Expand Down

0 comments on commit a741aec

Please sign in to comment.