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

Resolve /decide endpoint errors due to invalid token #4772

Merged
merged 7 commits into from
Jun 16, 2021
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
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