diff --git a/posthog/api/test/test_personal_api_keys.py b/posthog/api/test/test_personal_api_keys.py index afd7b924605a0..1fe9c3af847e9 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,23 @@ def test_allows_overriding_write_scopes(self): ) assert response.status_code == status.HTTP_200_OK + def test_works_with_routes_missing_action(self): + 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}" + ) + 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