Skip to content

Commit

Permalink
Resolve /decide endpoint errors due to invalid token (#4772)
Browse files Browse the repository at this point in the history
* update token cleaning code

* add test
  • Loading branch information
neilkakkar authored Jun 16, 2021
1 parent c0106b1 commit 4080c5a
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 35 deletions.
11 changes: 5 additions & 6 deletions posthog/api/capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from sentry_sdk import capture_exception
from statshog.defaults.django import statsd

from posthog.api.utils import clean_token, get_token
from posthog.api.utils import get_token
from posthog.celery import app as celery_app
from posthog.constants import ENVIRONMENT_TEST
from posthog.ee import is_clickhouse_enabled
Expand Down Expand Up @@ -134,7 +134,7 @@ def get_event(request):

sent_at = _get_sent_at(data, request)

token = get_token(data, request)
token, is_test_environment = get_token(data, request)

if not token:
return cors_response(
Expand All @@ -148,9 +148,6 @@ def get_event(request):
),
)

token, is_test_environment = clean_token(token)
assert token is not None

team = Team.objects.get_team_from_token(token)

if team is None:
Expand Down Expand Up @@ -254,7 +251,9 @@ def get_event(request):
capture_internal(event, distinct_id, ip, site_url, now, sent_at, team.pk)

timer.stop()
statsd.incr(f"posthog_cloud_raw_endpoint_success", tags={"endpoint": "capture",})
statsd.incr(
f"posthog_cloud_raw_endpoint_success", tags={"endpoint": "capture",},
)
return cors_response(request, JsonResponse({"status": 1}))


Expand Down
10 changes: 6 additions & 4 deletions posthog/api/decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from sentry_sdk import capture_exception
from statshog.defaults.django import statsd

from posthog.api.utils import clean_token, get_token
from posthog.api.utils import get_token
from posthog.exceptions import RequestParsingError, generate_exception_response
from posthog.models import Team, User
from posthog.models.feature_flag import get_active_feature_flags
Expand Down Expand Up @@ -89,8 +89,8 @@ def get_decide(request: HttpRequest):
request,
generate_exception_response("decide", f"Malformed request data: {error}", code="malformed_data"),
)
token = get_token(data, request)
token, _ = clean_token(token)

token, _ = get_token(data, request)
team = Team.objects.get_team_from_token(token)
if team is None and token:
project_id = _get_project_id(data, request)
Expand Down Expand Up @@ -124,5 +124,7 @@ def get_decide(request: HttpRequest):
response["featureFlags"] = get_active_feature_flags(team, data["distinct_id"])
if team.session_recording_opt_in and (on_permitted_domain(team, request) or len(team.app_urls) == 0):
response["sessionRecording"] = {"endpoint": "/s/"}
statsd.incr(f"posthog_cloud_raw_endpoint_success", tags={"endpoint": "decide",})
statsd.incr(
f"posthog_cloud_raw_endpoint_success", tags={"endpoint": "decide",},
)
return cors_response(request, JsonResponse(response))
5 changes: 2 additions & 3 deletions posthog/api/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from rest_framework_extensions.routers import ExtendedDefaultRouter
from rest_framework_extensions.settings import extensions_api_settings

from posthog.api.utils import clean_token, get_token
from posthog.api.utils import get_token
from posthog.models.organization import Organization
from posthog.models.team import Team
from posthog.utils import load_data_from_request
Expand Down Expand Up @@ -121,10 +121,9 @@ def get_serializer_context(self) -> Dict[str, Any]:

def _get_team_from_request(self) -> Optional["Team"]:
team_found = None
token = get_token(None, self.request)
token, _ = get_token(None, self.request)

if token:
token, _ = clean_token(token)
team = Team.objects.get_team_from_token(token)
if team:
team_found = team
Expand Down
13 changes: 13 additions & 0 deletions posthog/api/test/test_decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,19 @@ def test_personal_api_key_without_project_id(self):
},
)

def test_missing_token(self):
key = PersonalAPIKey(label="X", user=self.user)
key.save()
Person.objects.create(team=self.team, distinct_ids=["example_id"])
FeatureFlag.objects.create(
team=self.team, rollout_percentage=100, name="Test", key="test", created_by=self.user,
)
response = self._post_decide({"distinct_id": "example_id", "api_key": None, "project_id": self.team.id})
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_json = response.json()
self.assertEqual(response_json["featureFlags"], [])
self.assertFalse(response_json["sessionRecording"])

def test_invalid_payload_on_decide_endpoint(self):

invalid_payloads = [base64.b64encode("1-1".encode("utf-8")).decode("utf-8"), "1==1", "{distinct_id-1}"]
Expand Down
50 changes: 28 additions & 22 deletions posthog/api/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Optional
from typing import Optional, Tuple

from rest_framework import request

Expand Down Expand Up @@ -32,29 +32,35 @@ def format_next_url(request: request.Request, offset: int, page_size: int):
return next_url


def get_token(data, request) -> Optional[str]:
def get_token(data, request) -> Tuple[Optional[str], bool]:
token = None
if request.method == "GET":
if request.GET.get("token"):
return request.GET.get("token") # token passed as query param
if request.GET.get("api_key"):
return request.GET.get("api_key") # api_key passed as query param
if request.POST.get("api_key"):
return request.POST["api_key"]
if request.POST.get("token"):
return request.POST["token"]
if data:
if isinstance(data, list):
data = data[0] # Mixpanel Swift SDK
if isinstance(data, dict):
if data.get("$token"):
return data["$token"] # JS identify call
if data.get("token"):
return data["token"] # JS reloadFeatures call
if data.get("api_key"):
return data["api_key"] # server-side libraries like posthog-python and posthog-ruby
if data.get("properties") and data["properties"].get("token"):
return data["properties"]["token"] # JS capture call
return None
token = request.GET.get("token") # token passed as query param
elif request.GET.get("api_key"):
token = request.GET.get("api_key") # api_key passed as query param

if not token:
if request.POST.get("api_key"):
token = request.POST["api_key"]
elif request.POST.get("token"):
token = request.POST["token"]
elif data:
if isinstance(data, list):
data = data[0] # Mixpanel Swift SDK
if isinstance(data, dict):
if data.get("$token"):
token = data["$token"] # JS identify call
elif data.get("token"):
token = data["token"] # JS reloadFeatures call
elif data.get("api_key"):
token = data["api_key"] # server-side libraries like posthog-python and posthog-ruby
elif data.get("properties") and data["properties"].get("token"):
token = data["properties"]["token"] # JS capture call

if token:
return clean_token(token)
return None, False


# Support test_[apiKey] for users with multiple environments
Expand Down

0 comments on commit 4080c5a

Please sign in to comment.