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

fix: Sharing configuration api scopes #20983

Merged
merged 3 commits into from
Mar 19, 2024
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
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
Loading