feat(activity-log): Log if activity was completed by PostHog staff us…

…ing impersonation (#19657) * Move "impersonated_session" to a context processor * Add field `was_impersonated` to `ActivityLog` * Fix `cannot_be_both_system_and_staff` constraint * Fix `log_insight_activity` * Update activityLogMocks.ts * Update plugin.py * Update test_migrations_are_safe.py * Fix log_activity helpers * Update query snapshots * Update UI snapshots for `chromium` (2) * Remove constraint * Fix `ExportedAssetSerializer` * Update query snapshots * Update UI snapshots for `chromium` (1) * Add integrity check * Fail activity logging loudly in tests --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
PostHog · Jan 11, 2024 · bfa3b80 · bfa3b80
1 parent 6f82631
commit bfa3b80
Show file tree

Hide file tree

Showing 27 changed files with 147 additions and 45 deletions.
diff --git a/ee/api/ee_event_definition.py b/ee/api/ee_event_definition.py
@@ -10,6 +10,8 @@
     Detail,
 )
 
+from loginas.utils import is_impersonated_session
+
 
 class EnterpriseEventDefinitionSerializer(TaggedItemSerializerMixin, serializers.ModelSerializer):
     updated_by = UserBasicSerializer(read_only=True)
@@ -98,6 +100,7 @@ def update(self, event_definition: EnterpriseEventDefinition, validated_data):
             item_id=str(event_definition.id),
             scope="EventDefinition",
             activity="changed",
+            was_impersonated=is_impersonated_session(self.context["request"]),
             detail=Detail(name=str(event_definition.name), changes=changes),
         )
 

diff --git a/ee/api/ee_property_definition.py b/ee/api/ee_property_definition.py
@@ -9,6 +9,7 @@
     log_activity,
     Detail,
 )
+from loginas.utils import is_impersonated_session
 
 
 class EnterprisePropertyDefinitionSerializer(TaggedItemSerializerMixin, serializers.ModelSerializer):
@@ -77,6 +78,7 @@ def update(self, property_definition: EnterprisePropertyDefinition, validated_da
             organization_id=None,
             team_id=self.context["team_id"],
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(self.context["request"]),
             item_id=str(property_definition.id),
             scope="PropertyDefinition",
             activity="changed",

diff --git a/ee/session_recordings/session_recording_playlist.py b/ee/session_recordings/session_recording_playlist.py
@@ -40,6 +40,7 @@
 )
 from posthog.session_recordings.session_recording_api import list_recordings_response
 from posthog.utils import relative_date_parse
+from loginas.utils import is_impersonated_session
 
 logger = structlog.get_logger(__name__)
 
@@ -52,6 +53,7 @@ def log_playlist_activity(
     organization_id: UUIDT,
     team_id: int,
     user: User,
+    was_impersonated: bool,
     changes: Optional[List[Change]] = None,
 ) -> None:
     """
@@ -66,6 +68,7 @@ def log_playlist_activity(
             organization_id=organization_id,
             team_id=team_id,
             user=user,
+            was_impersonated=was_impersonated,
             item_id=playlist_id,
             scope="SessionRecordingPlaylist",
             activity=activity,
@@ -125,6 +128,7 @@ def create(self, validated_data: Dict, *args, **kwargs) -> SessionRecordingPlayl
             organization_id=self.context["request"].user.current_organization_id,
             team_id=team.id,
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(self.context["request"]),
         )
 
         return playlist
@@ -150,6 +154,7 @@ def update(self, instance: SessionRecordingPlaylist, validated_data: Dict, **kwa
             organization_id=self.context["request"].user.current_organization_id,
             team_id=self.context["team_id"],
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(self.context["request"]),
             changes=changes,
         )
 

diff --git a/frontend/src/lib/components/ActivityLog/__mocks__/activityLogMocks.ts b/frontend/src/lib/components/ActivityLog/__mocks__/activityLogMocks.ts
@@ -791,8 +791,7 @@ export const insightsActivityResponseJson: ActivityLogItem[] = [
     {
         user: {
             first_name: 'System',
-            email: null,
-            is_system: true,
+            email: '[email protected]',
         },
         activity: 'exported for opengraph image',
         scope: ActivityScope.INSIGHT,

diff --git a/frontend/src/lib/components/ActivityLog/humanizeActivity.tsx b/frontend/src/lib/components/ActivityLog/humanizeActivity.tsx
@@ -2,7 +2,7 @@ import { dayjs } from 'lib/dayjs'
 import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown'
 import { fullName } from 'lib/utils'
 
-import { ActivityScope, InsightShortId, PersonType } from '~/types'
+import { ActivityScope, InsightShortId, PersonType, UserBasicType } from '~/types'
 
 export interface ActivityChange {
     type: ActivityScope
@@ -34,21 +34,19 @@ export interface ActivityLogDetail {
     type?: string
 }
 
-export interface ActivityUser {
-    email: string | null
-    first_name: string
-    is_system?: boolean
-}
-
 export type ActivityLogItem = {
-    user?: ActivityUser
+    user?: Pick<UserBasicType, 'email' | 'first_name' | 'last_name'>
     activity: string
     created_at: string
     scope: ActivityScope
     item_id?: string
     detail: ActivityLogDetail
-    unread?: boolean // when used as a notification
-    is_system?: boolean // when auto-created e.g. an exported image when sharing an insight
+    /** Present if the log is used as a notification. Whether the notification is unread. */
+    unread?: boolean
+    /** Whether the activity was initiated by a PostHog staff member impersonating a user. */
+    is_staff?: boolean
+    /** Whether the activity was initiated by the PostHog backend. Example: an exported image when sharing an insight. */
+    is_system?: boolean
 }
 
 // the description of a single activity log is a sentence describing one or more changes that makes up the entry

diff --git a/latest_migrations.manifest b/latest_migrations.manifest
@@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name
 ee: 0015_add_verified_properties
 otp_static: 0002_throttling
 otp_totp: 0002_auto_20190420_0723
-posthog: 0383_externaldatasource_cascade
+posthog: 0384_activity_log_was_impersonated
 sessions: 0001_initial
 social_django: 0010_uid_db_index
 two_factor: 0007_auto_20201201_1019
diff --git a/posthog/api/event_definition.py b/posthog/api/event_definition.py
@@ -28,6 +28,7 @@
     TeamMemberAccessPermission,
 )
 from posthog.settings import EE_AVAILABLE
+from loginas.utils import is_impersonated_session
 
 # If EE is enabled, we use ee.api.ee_event_definition.EnterpriseEventDefinitionSerializer
 
@@ -187,6 +188,7 @@ def destroy(self, request: request.Request, *args: Any, **kwargs: Any) -> respon
             organization_id=cast(UUIDT, self.organization_id),
             team_id=self.team_id,
             user=user,
+            was_impersonated=is_impersonated_session(request),
             item_id=instance_id,
             scope="EventDefinition",
             activity="deleted",

diff --git a/posthog/api/exports.py b/posthog/api/exports.py
@@ -24,6 +24,7 @@
     TeamMemberAccessPermission,
 )
 from posthog.tasks import exporter
+from loginas.utils import is_impersonated_session
 
 logger = structlog.get_logger(__name__)
 
@@ -99,6 +100,9 @@ def _create_asset(
                     organization_id=insight.team.organization.id,
                     team_id=self.context["team_id"],
                     user=user,
+                    was_impersonated=is_impersonated_session(self.context["request"])
+                    if "request" in self.context
+                    else False,
                     item_id=insight_id,  # Type: ignore
                     scope="Insight",
                     activity="exported" if reason is None else f"exported for {reason}",

diff --git a/posthog/api/feature_flag.py b/posthog/api/feature_flag.py
@@ -55,6 +55,7 @@
     TeamMemberAccessPermission,
 )
 from posthog.rate_limit import BurstRateThrottle
+from loginas.utils import is_impersonated_session
 
 DATABASE_FOR_LOCAL_EVALUATION = (
     "default"
@@ -683,6 +684,7 @@ def perform_create(self, serializer):
             organization_id=self.organization.id,
             team_id=self.team_id,
             user=serializer.context["request"].user,
+            was_impersonated=is_impersonated_session(serializer.context["request"]),
             item_id=serializer.instance.id,
             scope="FeatureFlag",
             activity="created",
@@ -705,6 +707,7 @@ def perform_update(self, serializer):
             organization_id=self.organization.id,
             team_id=self.team_id,
             user=serializer.context["request"].user,
+            was_impersonated=is_impersonated_session(serializer.context["request"]),
             item_id=instance_id,
             scope="FeatureFlag",
             activity="updated",

diff --git a/posthog/api/insight.py b/posthog/api/insight.py
@@ -104,6 +104,8 @@
     relative_date_parse,
     str_to_bool,
 )
+from loginas.utils import is_impersonated_session
+
 
 logger = structlog.get_logger(__name__)
 
@@ -115,13 +117,15 @@
 
 
 def log_insight_activity(
+    *,
     activity: str,
     insight: Insight,
     insight_id: int,
     insight_short_id: str,
     organization_id: UUIDT,
     team_id: int,
     user: User,
+    was_impersonated: bool,
     changes: Optional[List[Change]] = None,
 ) -> None:
     """
@@ -136,6 +140,7 @@ def log_insight_activity(
             organization_id=organization_id,
             team_id=team_id,
             user=user,
+            was_impersonated=was_impersonated,
             item_id=insight_id,
             scope="Insight",
             activity=activity,
@@ -343,6 +348,7 @@ def create(self, validated_data: Dict, *args: Any, **kwargs: Any) -> Insight:
             organization_id=self.context["request"].user.current_organization_id,
             team_id=team_id,
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(self.context["request"]),
         )
 
         return insight
@@ -409,6 +415,7 @@ def _log_insight_update(self, before_update, dashboards_before_change, updated_i
                 organization_id=self.context["request"].user.current_organization_id,
                 team_id=self.context["team_id"],
                 user=self.context["request"].user,
+                was_impersonated=is_impersonated_session(self.context["request"]),
                 changes=changes,
             )
 

diff --git a/posthog/api/notebook.py b/posthog/api/notebook.py
@@ -40,6 +40,7 @@
 )
 from posthog.settings import DEBUG
 from posthog.utils import relative_date_parse
+from loginas.utils import is_impersonated_session
 
 logger = structlog.get_logger(__name__)
 
@@ -62,6 +63,7 @@ def log_notebook_activity(
     organization_id: UUIDT,
     team_id: int,
     user: User,
+    was_impersonated: bool,
     changes: Optional[List[Change]] = None,
 ) -> None:
     short_id = str(notebook.short_id)
@@ -70,6 +72,7 @@ def log_notebook_activity(
         organization_id=organization_id,
         team_id=team_id,
         user=user,
+        was_impersonated=was_impersonated,
         item_id=notebook.short_id,
         scope="Notebook",
         activity=activity,
@@ -139,6 +142,7 @@ def create(self, validated_data: Dict, *args, **kwargs) -> Notebook:
             organization_id=self.context["request"].user.current_organization_id,
             team_id=team.id,
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(request),
         )
 
         return notebook
@@ -173,6 +177,7 @@ def update(self, instance: Notebook, validated_data: Dict, **kwargs) -> Notebook
             organization_id=self.context["request"].user.current_organization_id,
             team_id=self.context["team_id"],
             user=self.context["request"].user,
+            was_impersonated=is_impersonated_session(self.context["request"]),
             changes=changes,
         )
 

diff --git a/posthog/api/person.py b/posthog/api/person.py
@@ -95,6 +95,7 @@
 )
 from prometheus_client import Counter
 from posthog.metrics import LABEL_TEAM_ID
+from loginas.utils import is_impersonated_session
 
 DEFAULT_PAGE_LIMIT = 100
 # Sync with .../lib/constants.tsx and .../ingestion/hooks.ts
@@ -406,6 +407,7 @@ def destroy(self, request: request.Request, pk=None, **kwargs):
                 organization_id=self.organization.id,
                 team_id=self.team_id,
                 user=cast(User, request.user),
+                was_impersonated=is_impersonated_session(request),
                 item_id=person_id,
                 scope="Person",
                 activity="deleted",
@@ -482,6 +484,7 @@ def split(self, request: request.Request, pk=None, **kwargs) -> response.Respons
             organization_id=self.organization.id,
             team_id=self.team.id,
             user=request.user,
+            was_impersonated=is_impersonated_session(request),
             item_id=person.id,
             scope="Person",
             activity="split_person",
@@ -573,6 +576,7 @@ def delete_property(self, request: request.Request, pk=None, **kwargs) -> respon
             organization_id=self.organization.id,
             team_id=self.team.id,
             user=request.user,
+            was_impersonated=is_impersonated_session(request),
             item_id=person.id,
             scope="Person",
             activity="delete_property",
@@ -675,6 +679,7 @@ def _set_properties(self, properties, user):
             organization_id=self.organization.id,
             team_id=self.team.id,
             user=user,
+            was_impersonated=is_impersonated_session(self.request),
             item_id=instance.pk,
             scope="Person",
             activity="updated",