From ff77be50670566cd1f0d292f540c99425b77897c Mon Sep 17 00:00:00 2001 From: Ben White Date: Mon, 16 Dec 2024 14:19:23 +0100 Subject: [PATCH] fix: Remoteconfig session replay domains check (#26924) --- posthog/api/decide.py | 14 +++++++++---- posthog/api/test/test_decide.py | 37 +++++++++++++++++++++++++++++---- posthog/models/remote_config.py | 3 ++- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/posthog/api/decide.py b/posthog/api/decide.py index bd2e363dfa435..98b331de0472d 100644 --- a/posthog/api/decide.py +++ b/posthog/api/decide.py @@ -53,11 +53,17 @@ def get_base_config(token: str, team: Team, request: HttpRequest, skip_db: bool ) if use_remote_config: - response = RemoteConfig.get_config_via_token(token) + response = RemoteConfig.get_config_via_token(token, request=request) - if _session_recording_domain_not_allowed(team, request): - # Fallback for sessionRecording domain check - new endpoint will be used differently - response["sessionRecording"] = False + # Add in a bunch of backwards compatibility stuff + response["isAuthenticated"] = False + response["toolbarParams"] = {} + response["config"] = {"enable_collect_everything": True} + response["surveys"] = True if len(response["surveys"]) > 0 else False + + # Remove some stuff that is specific to the new RemoteConfig + del response["hasFeatureFlags"] + del response["token"] return response diff --git a/posthog/api/test/test_decide.py b/posthog/api/test/test_decide.py index 1060bc89ce916..bbf74ee1ecb72 100644 --- a/posthog/api/test/test_decide.py +++ b/posthog/api/test/test_decide.py @@ -5,6 +5,7 @@ from typing import Optional from unittest.mock import patch, Mock +from inline_snapshot import snapshot import pytest from django.conf import settings from django.core.cache import cache @@ -3590,10 +3591,38 @@ class TestDecideRemoteConfig(TestDecide): use_remote_config = True def test_definitely_loads_via_remote_config(self, *args): - response = self._post_decide(api_version=3) - # NOTE: Using these as a sanity check as they are subtly different in format - assert response.json()["surveys"] == [] - assert response.json()["hasFeatureFlags"] is False + # NOTE: This is a sanity check test that we aren't just using the old decide logic + + with patch.object( + RemoteConfig, "get_config_via_token", wraps=RemoteConfig.get_config_via_token + ) as wrapped_get_config_via_token: + response = self._post_decide(api_version=3) + wrapped_get_config_via_token.assert_called_once() + + # NOTE: If this changes it indicates something is wrong as we should keep this exact format + # for backwards compatibility + assert response.json() == snapshot( + { + "supportedCompression": ["gzip", "gzip-js"], + "captureDeadClicks": False, + "capturePerformance": {"network_timing": True, "web_vitals": False, "web_vitals_allowed_metrics": None}, + "autocapture_opt_out": False, + "autocaptureExceptions": False, + "analytics": {"endpoint": "/i/v0/e/"}, + "elementsChainAsString": True, + "sessionRecording": False, + "heatmaps": False, + "surveys": False, + "defaultIdentifiedOnly": True, + "siteApps": [], + "isAuthenticated": False, + "toolbarParams": {}, + "config": {"enable_collect_everything": True}, + "featureFlags": {}, + "errorsWhileComputingFlags": False, + "featureFlagPayloads": {}, + } + ) class TestDatabaseCheckForDecide(BaseTest, QueryMatchingTest): diff --git a/posthog/models/remote_config.py b/posthog/models/remote_config.py index 3f127a710c3e4..08eaa81fc8a41 100644 --- a/posthog/models/remote_config.py +++ b/posthog/models/remote_config.py @@ -74,7 +74,8 @@ def sanitize_config_for_public_cdn(config: dict, request: Optional[HttpRequest] if "domains" in config["sessionRecording"]: domains = config["sessionRecording"].pop("domains") - if request: + # Empty list of domains means always permitted + if request and domains: if not on_permitted_recording_domain(domains, request=request): config["sessionRecording"] = False