Skip to content

Commit

Permalink
Merge branch 'master' into feat/notice-cohort
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Nov 29, 2024
2 parents 82ce9f1 + b43825f commit e1f29f3
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 7 deletions.
19 changes: 18 additions & 1 deletion frontend/src/scenes/data-warehouse/external/forms/SourceForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,24 @@ export function SourceFormComponent({ sourceConfig, showPrefix = true, jobInputs
</Group>
{showPrefix && (
<LemonField name="prefix" label="Table Prefix (optional)">
<LemonInput className="ph-ignore-input" data-attr="prefix" placeholder="internal_" />
{({ value, onChange }) => (
<>
<LemonInput
className="ph-ignore-input"
data-attr="prefix"
placeholder="internal_"
value={value}
onChange={onChange}
/>
<p>
Example table name:{' '}
<strong>
{value}
{sourceConfig.name.toLowerCase()}_table_name
</strong>
</p>
</>
)}
</LemonField>
)}
</div>
Expand Down
37 changes: 34 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,45 @@ def create(self, validated_data: dict, **kwargs) -> PersonalAPIKey:
return personal_api_key


class PersonalApiKeySelfAccessPermission(BasePermission):
"""
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

return view.action == "retrieve"

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":
authenticator = cast(PersonalAPIKeyAuthentication, self.request.successful_authenticator)
return 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

0 comments on commit e1f29f3

Please sign in to comment.