From 859d8ed9ac7c5026db09714a26c85c1458abb038 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Tue, 29 Mar 2022 15:29:57 +0200 Subject: [PATCH] fix(toolbar): only redirect to hosts in app_urls (#9268) * fix(toolbar): only redirect to hosts in app_urls * fix mypy and regex --- posthog/api/decide.py | 27 +++++++++++++++++---------- posthog/test/test_urls.py | 9 +++++++++ posthog/urls.py | 12 +++++++++--- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/posthog/api/decide.py b/posthog/api/decide.py index 3d0aa0c4e09fb..42d3953eaee00 100644 --- a/posthog/api/decide.py +++ b/posthog/api/decide.py @@ -20,23 +20,30 @@ def on_permitted_domain(team: Team, request: HttpRequest) -> bool: + origin = parse_domain(request.headers.get("Origin")) + referer = parse_domain(request.headers.get("Referer")) + return hostname_in_app_urls(team, origin) or hostname_in_app_urls(team, referer) + + +def hostname_in_app_urls(team: Team, hostname: Optional[str]) -> bool: + if not hostname: + return False + permitted_domains = ["127.0.0.1", "localhost"] for url in team.app_urls: - hostname = parse_domain(url) - if hostname: - permitted_domains.append(hostname) + host = parse_domain(url) + if host: + permitted_domains.append(host) - origin = parse_domain(request.headers.get("Origin")) - referer = parse_domain(request.headers.get("Referer")) for permitted_domain in permitted_domains: if "*" in permitted_domain: - pattern = "^{}$".format(permitted_domain.replace(".", "\\.").replace("*", "(.*)")) - if (origin and re.search(pattern, origin)) or (referer and re.search(pattern, referer)): - return True - else: - if permitted_domain == origin or permitted_domain == referer: + pattern = "^{}$".format(re.escape(permitted_domain).replace("\\*", "(.*)")) + if re.search(pattern, hostname): return True + elif permitted_domain == hostname: + return True + return False diff --git a/posthog/test/test_urls.py b/posthog/test/test_urls.py index d214621e3759a..4328b0a50ca08 100644 --- a/posthog/test/test_urls.py +++ b/posthog/test/test_urls.py @@ -56,6 +56,15 @@ def test_unauthenticated_routes_get_loaded_on_the_frontend(self): self.assertEqual(response.status_code, status.HTTP_200_OK) def test_authorize_and_redirect_domain(self): + self.team.app_urls = ["https://domain.com", "https://not.com"] + self.team.save() + + response = self.client.get( + "/authorize_and_redirect/?redirect=https://not-permitted.com", HTTP_REFERER="https://not-permitted.com" + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertTrue("Can only redirect to a permitted domain." in str(response.content)) + response = self.client.get( "/authorize_and_redirect/?redirect=https://domain.com", HTTP_REFERER="https://not.com" ) diff --git a/posthog/urls.py b/posthog/urls.py index 731a3204729cc..1a04fb9a6cc00 100644 --- a/posthog/urls.py +++ b/posthog/urls.py @@ -1,8 +1,8 @@ -from typing import Any, Callable, List, Optional +from typing import Any, Callable, List, Optional, cast from urllib.parse import urlparse from django.conf import settings -from django.http import HttpResponse +from django.http import HttpRequest, HttpResponse from django.urls import URLPattern, include, path, re_path from django.views.decorators import csrf from django.views.decorators.csrf import csrf_exempt @@ -21,7 +21,9 @@ signup, user, ) +from posthog.api.decide import hostname_in_app_urls from posthog.demo import demo +from posthog.models import User from .utils import render_template from .views import health, login_required, preflight_check, robots_txt, security_txt, sso_login, stats @@ -50,15 +52,19 @@ def home(request, *args, **kwargs): return render_template("index.html", request) -def authorize_and_redirect(request): +def authorize_and_redirect(request: HttpRequest) -> HttpResponse: if not request.GET.get("redirect"): return HttpResponse("You need to pass a url to ?redirect=", status=401) if not request.META.get("HTTP_REFERER"): return HttpResponse('You need to make a request that includes the "Referer" header.', status=400) + current_team = cast(User, request.user).team referer_url = urlparse(request.META["HTTP_REFERER"]) redirect_url = urlparse(request.GET["redirect"]) + if not current_team or not hostname_in_app_urls(current_team, redirect_url.hostname): + return HttpResponse(f"Can only redirect to a permitted domain.", status=400) + if referer_url.hostname != redirect_url.hostname: return HttpResponse(f"Can only redirect to the same domain as the referer: {referer_url.hostname}", status=400)