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

fix: Remoteconfig session replay domains check #26924

Merged
merged 2 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading