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

feat: personal api key api access for @current #26519

Merged
merged 12 commits into from
Nov 29, 2024
40 changes: 37 additions & 3 deletions posthog/api/personal_api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import uuid

from rest_framework import response, serializers, viewsets
from rest_framework.permissions import IsAuthenticated
from rest_framework.permissions import IsAuthenticated, BasePermission

from posthog.auth import PersonalAPIKeyAuthentication, SessionAuthentication
from posthog.models import PersonalAPIKey, User
from posthog.models.personal_api_key import hash_key_value, mask_key_value
from posthog.models.scopes import API_SCOPE_ACTIONS, API_SCOPE_OBJECTS
Expand All @@ -12,7 +13,6 @@
from posthog.permissions import TimeSensitiveActionPermission
from posthog.user_permissions import UserPermissions


MAX_API_KEYS_PER_USER = 10 # Same as in personalAPIKeysLogic.tsx


Expand Down Expand Up @@ -112,14 +112,48 @@ def create(self, validated_data: dict, **kwargs) -> PersonalAPIKey:
return personal_api_key


class PersonalApiKeySelfAccessPermission(BasePermission):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zlwaterfield I opted for a dedicated permission rather than muddying the other permissions. I prefer this as its easier to reason on these edge-casey things local to where the edge case is.

Also avoided bringing in the common "TeamAndOrg" mixin

"""
Personal API Keys can only access their own key and only for retrieval
"""

message = "This action does not support Personal API Key access"

def has_permission(self, request, view) -> bool:
# This permission check only applies to the personal api key
if not isinstance(request.successful_authenticator, PersonalAPIKeyAuthentication):
return True

# TRICKY: Legacy API keys have no scopes and are allowed to do anything, even if the view is unsupported.
if view.action != "retrieve":
return False

return True

def has_object_permission(self, request, view, item: PersonalAPIKey) -> bool:
if not isinstance(request.successful_authenticator, PersonalAPIKeyAuthentication):
return True

return request.successful_authenticator.personal_api_key == item


class PersonalAPIKeyViewSet(viewsets.ModelViewSet):
lookup_field = "id"
serializer_class = PersonalAPIKeySerializer
permission_classes = [IsAuthenticated, TimeSensitiveActionPermission]
permission_classes = [IsAuthenticated, TimeSensitiveActionPermission, PersonalApiKeySelfAccessPermission]
authentication_classes = [PersonalAPIKeyAuthentication, SessionAuthentication]
queryset = PersonalAPIKey.objects.none()

def get_queryset(self):
return PersonalAPIKey.objects.filter(user_id=cast(User, self.request.user).id).order_by("-created_at")

def get_object(self) -> PersonalAPIKey:
lookup_value = self.kwargs[self.lookup_field]
if lookup_value == "@current":
return self.request.successful_authenticator.personal_api_key

return super().get_object()

def list(self, request, *args, **kwargs):
queryset = self.filter_queryset(self.get_queryset())
serializer = self.get_serializer(queryset, many=True)
Expand Down
69 changes: 67 additions & 2 deletions posthog/api/test/test_personal_api_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from posthog.models.utils import generate_random_token_personal
from posthog.schema import EventsQuery
from posthog.test.base import APIBaseTest
from posthog.api.personal_api_key import PersonalAPIKeySerializer


class TestPersonalAPIKeysAPI(APIBaseTest):
Expand Down Expand Up @@ -362,7 +363,7 @@ def test_cannot_create_other_keys(self):
HTTP_AUTHORIZATION=f"Bearer {self.value}",
)

assert response.status_code == status.HTTP_401_UNAUTHORIZED, response.json()
assert response.status_code == status.HTTP_403_FORBIDDEN, response.json()

def test_cannot_edit_self(self):
response = self.client.post(
Expand All @@ -371,7 +372,7 @@ def test_cannot_edit_self(self):
HTTP_AUTHORIZATION=f"Bearer {self.value}",
)

assert response.status_code == status.HTTP_401_UNAUTHORIZED, response.json()
assert response.status_code == status.HTTP_403_FORBIDDEN, response.json()


# NOTE: These tests use feature flags as an example of a scope, but the actual feature flag functionality is not relevant
Expand Down Expand Up @@ -541,3 +542,67 @@ def test_allows_user_me_read_access(self):
# (e.g. in our Zapier integration), hence it's exempt from org/team scoping
response = self._do_request(f"/api/users/@me/")
assert response.status_code == status.HTTP_200_OK, response.json()


class TestPersonalAPIKeyAPIAccess(APIBaseTest):
def setUp(self):
super().setUp()

# Create a mock request context
class MockRequest:
def __init__(self, user):
self.user = user

# Create the key using the serializer
serializer = PersonalAPIKeySerializer(
data={"label": "Test key", "scopes": ["*"], "scoped_organizations": [], "scoped_teams": []},
context={"request": MockRequest(self.user)},
)
serializer.is_valid(raise_exception=True)
self.personal_api_key = serializer.save()
self.api_key_value = self.personal_api_key._value # This will contain the raw key value

def _get_auth_headers(self, key: str):
return {"HTTP_AUTHORIZATION": f"Bearer {key}"}

def test_list_personal_api_keys_with_bearer_auth(self):
# Should not be allowed to list with API key
response = self.client.get(f"/api/personal_api_keys/", **self._get_auth_headers(self.api_key_value))
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json()["detail"], "This action does not support Personal API Key access")

def test_retrieve_personal_api_key_with_bearer_auth(self):
# Should be allowed to get current key
response = self.client.get(f"/api/personal_api_keys/@current/", **self._get_auth_headers(self.api_key_value))
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["label"], "Test key")

# Should not be allowed to get by ID
response = self.client.get(
f"/api/personal_api_keys/{self.personal_api_key.id}/", **self._get_auth_headers(self.api_key_value)
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["label"], "Test key")

def test_create_personal_api_key_with_bearer_auth(self):
response = self.client.post(
f"/api/personal_api_keys/", {"label": "New key"}, **self._get_auth_headers(self.api_key_value)
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json()["detail"], "This action does not support Personal API Key access")

def test_update_personal_api_key_with_bearer_auth(self):
response = self.client.patch(
f"/api/personal_api_keys/@current/", {"label": "Updated key"}, **self._get_auth_headers(self.api_key_value)
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json()["detail"], "This action does not support Personal API Key access")

def test_delete_personal_api_key_with_bearer_auth(self):
response = self.client.delete(f"/api/personal_api_keys/@current/", **self._get_auth_headers(self.api_key_value))
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json()["detail"], "This action does not support Personal API Key access")

def test_invalid_bearer_token(self):
response = self.client.get(f"/api/personal_api_keys/@current/", **self._get_auth_headers("invalid_key"))
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
5 changes: 4 additions & 1 deletion posthog/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ def extract_organization(object: Model, view: ViewSet) -> Organization:
try:
return object.project.organization # type: ignore
except AttributeError:
pass
try:
return object.user.organization # type: ignore
except AttributeError:
pass
raise ValueError("Object not compatible with organization-based permissions!")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -640,12 +640,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '446'
AND "ee_accesscontrol"."resource_id" = '422'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '446'
AND "ee_accesscontrol"."resource_id" = '422'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -1688,12 +1688,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -2441,12 +2441,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -3129,12 +3129,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -3881,12 +3881,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -4597,12 +4597,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -5395,12 +5395,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -5659,12 +5659,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -6091,12 +6091,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -6556,12 +6556,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -7248,12 +7248,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down Expand Up @@ -7997,12 +7997,12 @@
LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id")
WHERE (("ee_accesscontrol"."organization_member_id" IS NULL
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("posthog_organizationmembership"."user_id" = 99999
AND "ee_accesscontrol"."resource" = 'project'
AND "ee_accesscontrol"."resource_id" = '453'
AND "ee_accesscontrol"."resource_id" = '429'
AND "ee_accesscontrol"."role_id" IS NULL
AND "ee_accesscontrol"."team_id" = 99999)
OR ("ee_accesscontrol"."organization_member_id" IS NULL
Expand Down