Skip to content

Commit

Permalink
fix: Remoteconfig session replay domains check (#26924)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Dec 16, 2024
1 parent 78959b2 commit ff77be5
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 9 deletions.
14 changes: 10 additions & 4 deletions posthog/api/decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
37 changes: 33 additions & 4 deletions posthog/api/test/test_decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion posthog/models/remote_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit ff77be5

Please sign in to comment.