From 672e8fa17417bf14dd6142bed48d3b4bf2e6c155 Mon Sep 17 00:00:00 2001 From: Ben White Date: Mon, 18 Mar 2024 17:39:51 +0100 Subject: [PATCH 1/2] Fixed issues with sharing config updates to api --- posthog/api/test/test_personal_api_keys.py | 11 +++++++++++ posthog/permissions.py | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/posthog/api/test/test_personal_api_keys.py b/posthog/api/test/test_personal_api_keys.py index afd7b924605a0..d3d377c8fee79 100644 --- a/posthog/api/test/test_personal_api_keys.py +++ b/posthog/api/test/test_personal_api_keys.py @@ -3,6 +3,7 @@ from rest_framework import status from posthog.jwt import PosthogJwtAudience, encode_jwt +from posthog.models.insight import Insight from posthog.models.organization import Organization from posthog.models.personal_api_key import PersonalAPIKey, hash_key_value from posthog.models.team.team import Team @@ -422,6 +423,16 @@ def test_allows_overriding_write_scopes(self): ) assert response.status_code == status.HTTP_200_OK + def test_works_with_routes_missing_action(self): + self.key.scopes = ["sharing_configuration:write"] + self.key.save() + insight = Insight.objects.create(team=self.team, name="XYZ", created_by=self.user) + + response = self.client.patch( + f"/api/projects/{self.team.id}/insights/{insight.id}/sharing?personal_api_key={self.value}" + ) + assert response.status_code == status.HTTP_200_OK + class TestPersonalAPIKeysWithOrganizationScopeAPIAuthentication(PersonalAPIKeysBaseTest): def setUp(self): diff --git a/posthog/permissions.py b/posthog/permissions.py index 4d42f165020bf..0d61b015d7f84 100644 --- a/posthog/permissions.py +++ b/posthog/permissions.py @@ -268,11 +268,19 @@ class APIScopePermission(BasePermission): """ - write_actions: list[str] = ["create", "update", "partial_update", "destroy"] + write_actions: list[str] = ["create", "update", "partial_update", "patch", "destroy"] read_actions: list[str] = ["list", "retrieve"] scope_object_read_actions: list[str] = [] scope_object_write_actions: list[str] = [] + def _get_action(self, request, view) -> str: + # TRICKY: DRF doesn't have an action for non-detail level "patch" calls which we use sometimes + + if not view.action: + if request.method == "PATCH" and not view.detail: + return "patch" + return view.action + def has_permission(self, request, view) -> bool: # NOTE: We do this first to error out quickly if the view is missing the required attribute # Helps devs remember to add it. @@ -341,12 +349,13 @@ def get_required_scopes(self, request, view) -> list[str]: if scope_object == "INTERNAL": raise PermissionDenied(f"This action does not support Personal API Key access") + action = self._get_action(request, view) read_actions = getattr(view, "scope_object_read_actions", self.read_actions) write_actions = getattr(view, "scope_object_write_actions", self.write_actions) - if view.action in write_actions: + if action in write_actions: return [f"{scope_object}:write"] - elif view.action in read_actions or request.method == "OPTIONS": + elif action in read_actions or request.method == "OPTIONS": return [f"{scope_object}:read"] # If we get here this typically means an action was called without a required scope From 1d3889f53705dc2722cf384a2812bd2343ece42c Mon Sep 17 00:00:00 2001 From: Ben White Date: Mon, 18 Mar 2024 17:44:25 +0100 Subject: [PATCH 2/2] fix test --- posthog/api/test/test_personal_api_keys.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/posthog/api/test/test_personal_api_keys.py b/posthog/api/test/test_personal_api_keys.py index d3d377c8fee79..1fe9c3af847e9 100644 --- a/posthog/api/test/test_personal_api_keys.py +++ b/posthog/api/test/test_personal_api_keys.py @@ -424,10 +424,17 @@ def test_allows_overriding_write_scopes(self): assert response.status_code == status.HTTP_200_OK def test_works_with_routes_missing_action(self): - self.key.scopes = ["sharing_configuration:write"] - self.key.save() insight = Insight.objects.create(team=self.team, name="XYZ", created_by=self.user) + self.key.scopes = ["sharing_configuration:read"] + self.key.save() + response = self.client.patch( + f"/api/projects/{self.team.id}/insights/{insight.id}/sharing?personal_api_key={self.value}" + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + self.key.scopes = ["sharing_configuration:write"] + self.key.save() response = self.client.patch( f"/api/projects/{self.team.id}/insights/{insight.id}/sharing?personal_api_key={self.value}" )