Skip to content

Commit

Permalink
fix: Sharing configuration api scopes (#20983)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Mar 19, 2024
1 parent 50edb5b commit 0668b3a
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
18 changes: 18 additions & 0 deletions posthog/api/test/test_personal_api_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
15 changes: 12 additions & 3 deletions posthog/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0668b3a

Please sign in to comment.