diff --git a/ee/api/ee_event_definition.py b/ee/api/ee_event_definition.py index e83b293b8caaa..325a845aaa804 100644 --- 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 index aa190bbd7c72d..308e7461942ca 100644 --- 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 index 31e76a168f43c..5849481d2f4c2 100644 --- 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 index 8cd97356fd826..360828f98bb67 100644 --- 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: 'system@x.com', }, 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 index e5a6d241847c7..f63aae1cb7233 100644 --- 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 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 index 56feba7f32e78..ec3953b0693e2 100644 --- 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 index 6616c8beaddab..5ee9e6ddc6b64 100644 --- 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 index e4f5dd8104f8c..f5fd304c75d85 100644 --- 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 index cf3623bf066de..abbd123ff929f 100644 --- 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 index 9990038255635..2eee340ef6e3b 100644 --- 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,6 +117,7 @@ def log_insight_activity( + *, activity: str, insight: Insight, insight_id: int, @@ -122,6 +125,7 @@ def log_insight_activity( 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 index d0f63e08220fa..718a0e36eab27 100644 --- 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 index d4148c2b4ad73..91646e9d23920 100644 --- 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", diff --git a/posthog/api/plugin.py b/posthog/api/plugin.py index 889d6d3e0d8d6..a64c233b2daa5 100644 --- a/posthog/api/plugin.py +++ b/posthog/api/plugin.py @@ -51,6 +51,7 @@ from posthog.redis import get_client from posthog.utils import format_query_params_absolute_url + # Keep this in sync with: frontend/scenes/plugins/utils.ts SECRET_FIELD_VALUE = "**************** POSTHOG SECRET FIELD ****************" @@ -60,11 +61,11 @@ def _update_plugin_attachments(request: request.Request, plugin_config: PluginCo for key, file in request.FILES.items(): match = re.match(r"^add_attachment\[([^]]+)\]$", key) if match: - _update_plugin_attachment(plugin_config, match.group(1), file, user) + _update_plugin_attachment(request, plugin_config, match.group(1), file, user) for key, _file in request.POST.items(): match = re.match(r"^remove_attachment\[([^]]+)\]$", key) if match: - _update_plugin_attachment(plugin_config, match.group(1), None, user) + _update_plugin_attachment(request, plugin_config, match.group(1), None, user) def get_plugin_config_changes(old_config: Dict[str, Any], new_config: Dict[str, Any], secret_fields=[]) -> List[Change]: @@ -82,13 +83,16 @@ def get_plugin_config_changes(old_config: Dict[str, Any], new_config: Dict[str, return config_changes -def log_enabled_change_activity(new_plugin_config: PluginConfig, old_enabled: bool, user: User, changes=[]): +def log_enabled_change_activity( + new_plugin_config: PluginConfig, old_enabled: bool, user: User, was_impersonated: bool, changes=[] +): if old_enabled != new_plugin_config.enabled: log_activity( organization_id=new_plugin_config.team.organization.id, # Users in an org but not yet in a team can technically manage plugins via the API team_id=new_plugin_config.team.id, user=user, + was_impersonated=was_impersonated, item_id=new_plugin_config.id, scope="PluginConfig", activity="enabled" if not old_enabled else "disabled", @@ -102,6 +106,7 @@ def log_config_update_activity( secret_fields: Set[str], old_enabled: bool, user: User, + was_impersonated: bool, ): config_changes = get_plugin_config_changes( old_config=old_config, @@ -115,16 +120,21 @@ def log_config_update_activity( # Users in an org but not yet in a team can technically manage plugins via the API team_id=new_plugin_config.team.id, user=user, + was_impersonated=was_impersonated, item_id=new_plugin_config.id, scope="PluginConfig", activity="config_updated", detail=Detail(name=new_plugin_config.plugin.name, changes=config_changes), ) - log_enabled_change_activity(new_plugin_config=new_plugin_config, old_enabled=old_enabled, user=user) + log_enabled_change_activity( + new_plugin_config=new_plugin_config, old_enabled=old_enabled, user=user, was_impersonated=was_impersonated + ) -def _update_plugin_attachment(plugin_config: PluginConfig, key: str, file: Optional[UploadedFile], user: User): +def _update_plugin_attachment( + request: request.Request, plugin_config: PluginConfig, key: str, file: Optional[UploadedFile], user: User +): try: plugin_attachment = PluginAttachment.objects.get(team=plugin_config.team, plugin_config=plugin_config, key=key) if file: @@ -170,6 +180,7 @@ def _update_plugin_attachment(plugin_config: PluginConfig, key: str, file: Optio organization_id=plugin_config.team.organization.id, team_id=plugin_config.team.id, user=user, + was_impersonated=is_impersonated_session(request), item_id=plugin_config.id, scope="PluginConfig", activity=activity, @@ -487,6 +498,7 @@ def destroy(self, request: request.Request, *args, **kwargs) -> Response: # Users in an org but not yet in a team can technically manage plugins via the API team_id=user.team.id if user.team else 0, # type: ignore user=user, # type: ignore + was_impersonated=is_impersonated_session(self.request), item_id=instance_id, scope="Plugin", activity="uninstalled", @@ -505,6 +517,7 @@ def perform_create(self, serializer): # Users in an org but not yet in a team can technically manage plugins via the API team_id=user.team.id if user.team else 0, user=user, + was_impersonated=is_impersonated_session(self.request), item_id=serializer.instance.id, scope="Plugin", activity="installed", @@ -657,6 +670,7 @@ def create(self, validated_data: Dict, *args: Any, **kwargs: Any) -> PluginConfi secret_fields=_get_secret_fields_for_plugin(plugin_config.plugin), ), user=self.context["request"].user, + was_impersonated=is_impersonated_session(self.context["request"]), ) _update_plugin_attachments(self.context["request"], plugin_config) @@ -694,6 +708,7 @@ def update( # type: ignore old_enabled=old_enabled, secret_fields=secret_fields, user=self.context["request"].user, + was_impersonated=is_impersonated_session(self.context["request"]), ) _update_plugin_attachments(self.context["request"], plugin_config) @@ -754,6 +769,7 @@ def rearrange(self, request: request.Request, **kwargs): # Users in an org but not yet in a team can technically manage plugins via the API team_id=self.team.id, user=request.user, # type: ignore + was_impersonated=is_impersonated_session(self.request), item_id=plugin_config.id, scope="Plugin", # use the type plugin so we can also provide unified history activity="order_changed", @@ -820,6 +836,7 @@ def job(self, request: request.Request, **kwargs): # Users in an org but not yet in a team can technically manage plugins via the API team_id=self.team.pk, user=request.user, # type: ignore + was_impersonated=is_impersonated_session(self.request), item_id=plugin_config_id, scope="PluginConfig", # use the type plugin so we can also provide unified history activity="job_triggered", diff --git a/posthog/api/property_definition.py b/posthog/api/property_definition.py index b298d8b630377..8cde3479894b3 100644 --- a/posthog/api/property_definition.py +++ b/posthog/api/property_definition.py @@ -31,6 +31,7 @@ OrganizationMemberPermissions, TeamMemberAccessPermission, ) +from loginas.utils import is_impersonated_session class SeenTogetherQuerySerializer(serializers.Serializer): @@ -625,6 +626,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=cast(User, request.user), + was_impersonated=is_impersonated_session(self.request), item_id=instance_id, scope="PropertyDefinition", activity="deleted", diff --git a/posthog/api/sharing.py b/posthog/api/sharing.py index 302511a0db98f..1259b4418d7de 100644 --- a/posthog/api/sharing.py +++ b/posthog/api/sharing.py @@ -33,6 +33,7 @@ from posthog.session_recordings.session_recording_api import SessionRecordingSerializer from posthog.user_permissions import UserPermissions from posthog.utils import render_template +from loginas.utils import is_impersonated_session def shared_url_as_png(url: str = "") -> str: @@ -181,6 +182,7 @@ def patch(self, request: Request, *args: Any, **kwargs: Any) -> response.Respons organization_id=None, team_id=self.team_id, user=cast(User, self.request.user), + was_impersonated=is_impersonated_session(self.request), item_id=instance.insight.pk, scope="Insight", activity="sharing " + ("enabled" if serializer.data.get("enabled") else "disabled"), diff --git a/posthog/api/test/notebooks/__snapshots__/test_notebook.ambr b/posthog/api/test/notebooks/__snapshots__/test_notebook.ambr index ff161a2839f81..550b197d0f5ca 100644 --- a/posthog/api/test/notebooks/__snapshots__/test_notebook.ambr +++ b/posthog/api/test/notebooks/__snapshots__/test_notebook.ambr @@ -290,6 +290,7 @@ "posthog_activitylog"."team_id", "posthog_activitylog"."organization_id", "posthog_activitylog"."user_id", + "posthog_activitylog"."was_impersonated", "posthog_activitylog"."is_system", "posthog_activitylog"."activity", "posthog_activitylog"."item_id", diff --git a/posthog/api/test/test_app_metrics.py b/posthog/api/test/test_app_metrics.py index 7d3f2a4aa9bf0..32ae14f01edc6 100644 --- a/posthog/api/test/test_app_metrics.py +++ b/posthog/api/test/test_app_metrics.py @@ -233,11 +233,12 @@ def test_error_details(self): def _create_activity_log(self, **kwargs): log_activity( - **{ + **{ # Using dict form so that kwargs can override these defaults "organization_id": self.team.organization.id, "team_id": self.team.pk, "user": self.user, "item_id": self.plugin_config.id, + "was_impersonated": False, "scope": "PluginConfig", **kwargs, } diff --git a/posthog/management/commands/test_migrations_are_safe.py b/posthog/management/commands/test_migrations_are_safe.py index 0dbf2534c8702..566533fd9fe69 100644 --- a/posthog/management/commands/test_migrations_are_safe.py +++ b/posthog/management/commands/test_migrations_are_safe.py @@ -40,6 +40,7 @@ def validate_migration_sql(sql) -> bool: if ( re.findall(r"(? None: + if was_impersonated and user is None: + logger.warn( + "activity_log.failed_to_write_to_activity_log", + team=team_id, + organization_id=organization_id, + scope=scope, + activity=activity, + exception=ValueError("Cannot log impersonated activity without a user"), + ) + return try: if activity == "updated" and (detail.changes is None or len(detail.changes) == 0) and not force_save: logger.warn( @@ -344,6 +357,7 @@ def log_activity( organization_id=organization_id, team_id=team_id, user=user, + was_impersonated=was_impersonated, is_system=user is None, item_id=str(item_id), scope=scope, @@ -359,6 +373,10 @@ def log_activity( activity=activity, exception=e, ) + if settings.TEST: + # Re-raise in tests, so that we can catch failures in test suites - but keep quiet in production, + # as we currently don't treat activity logs as critical + raise e @dataclasses.dataclass(frozen=True) diff --git a/posthog/models/comment.py b/posthog/models/comment.py index fdd3f63237883..a1fff705c0632 100644 --- a/posthog/models/comment.py +++ b/posthog/models/comment.py @@ -55,6 +55,7 @@ def log_comment_activity(sender, instance: Comment, created: bool, **kwargs): organization_id=None, team_id=instance.team_id, user=instance.created_by, + was_impersonated=None, # TODO - Find way to determine if the user was impersonated item_id=item_id, scope=scope, activity="commented", diff --git a/posthog/queries/app_metrics/test/__snapshots__/test_historical_exports.ambr b/posthog/queries/app_metrics/test/__snapshots__/test_historical_exports.ambr index 2069be3bc686c..4f90b5aa41d7e 100644 --- a/posthog/queries/app_metrics/test/__snapshots__/test_historical_exports.ambr +++ b/posthog/queries/app_metrics/test/__snapshots__/test_historical_exports.ambr @@ -52,6 +52,7 @@ "posthog_activitylog"."team_id", "posthog_activitylog"."organization_id", "posthog_activitylog"."user_id", + "posthog_activitylog"."was_impersonated", "posthog_activitylog"."is_system", "posthog_activitylog"."activity", "posthog_activitylog"."item_id", @@ -109,6 +110,7 @@ "posthog_activitylog"."team_id", "posthog_activitylog"."organization_id", "posthog_activitylog"."user_id", + "posthog_activitylog"."was_impersonated", "posthog_activitylog"."is_system", "posthog_activitylog"."activity", "posthog_activitylog"."item_id", @@ -164,6 +166,7 @@ "posthog_activitylog"."team_id", "posthog_activitylog"."organization_id", "posthog_activitylog"."user_id", + "posthog_activitylog"."was_impersonated", "posthog_activitylog"."is_system", "posthog_activitylog"."activity", "posthog_activitylog"."item_id", @@ -219,6 +222,7 @@ "posthog_activitylog"."team_id", "posthog_activitylog"."organization_id", "posthog_activitylog"."user_id", + "posthog_activitylog"."was_impersonated", "posthog_activitylog"."is_system", "posthog_activitylog"."activity", "posthog_activitylog"."item_id", diff --git a/posthog/queries/app_metrics/test/test_historical_exports.py b/posthog/queries/app_metrics/test/test_historical_exports.py index 6bed36981931c..4962548950fbd 100644 --- a/posthog/queries/app_metrics/test/test_historical_exports.py +++ b/posthog/queries/app_metrics/test/test_historical_exports.py @@ -345,12 +345,13 @@ def test_historical_export_metrics(self): def _create_activity_log(self, **kwargs): log_activity( - **{ + **{ # Using dict form so that kwargs can override these defaults "organization_id": self.team.organization.id, "team_id": self.team.pk, "user": self.user, "item_id": self.plugin_config.pk, "scope": "PluginConfig", + "was_impersonated": False, **kwargs, } ) diff --git a/posthog/settings/web.py b/posthog/settings/web.py index 36309548bb4a4..08c7f00769619 100644 --- a/posthog/settings/web.py +++ b/posthog/settings/web.py @@ -142,6 +142,7 @@ "django.template.context_processors.request", "django.contrib.auth.context_processors.auth", "django.contrib.messages.context_processors.messages", + "loginas.context_processors.impersonated_session_status", ] }, } diff --git a/posthog/templates/head.html b/posthog/templates/head.html index ed0d359faa014..f0ac06dea5a1a 100644 --- a/posthog/templates/head.html +++ b/posthog/templates/head.html @@ -22,7 +22,7 @@ window.JS_POSTHOG_HOST = {{js_posthog_host | safe}}; window.JS_POSTHOG_SELF_CAPTURE = {{self_capture | yesno:"true,false" }}; window.POSTHOG_USER_IDENTITY_WITH_FLAGS = JSON.parse("{{ posthog_bootstrap | escapejs }}") - window.IMPERSONATED_SESSION = {{impersonated_session | yesno:"true,false"}}; + window.IMPERSONATED_SESSION = {{is_impersonated_session | yesno:"true,false"}}; window.POSTHOG_JS_UUID_VERSION = "{{posthog_js_uuid_version}}"; {% endif %} diff --git a/posthog/test/activity_logging/test_activity_logging.py b/posthog/test/activity_logging/test_activity_logging.py index 9e2030779f1a9..8bfe40c7034fa 100644 --- a/posthog/test/activity_logging/test_activity_logging.py +++ b/posthog/test/activity_logging/test_activity_logging.py @@ -25,6 +25,7 @@ def test_can_save_a_model_changed_activity_log(self) -> None: organization_id=self.organization.id, team_id=self.team.id, user=self.user, + was_impersonated=False, item_id=6, scope="FeatureFlag", activity="updated", @@ -45,6 +46,7 @@ def test_can_save_a_log_that_has_no_model_changes(self) -> None: organization_id=self.organization.id, team_id=self.team.id, user=self.user, + was_impersonated=False, item_id=None, scope="dinglehopper", activity="added_to_clink_expander", @@ -53,20 +55,21 @@ def test_can_save_a_log_that_has_no_model_changes(self) -> None: log: ActivityLog = ActivityLog.objects.latest("id") self.assertEqual(log.activity, "added_to_clink_expander") - def test_does_not_save_an_updated_activity_that_has_no_changes(self) -> None: + def test_does_not_save_impersonated_activity_without_user(self) -> None: log_activity( organization_id=self.organization.id, team_id=self.team.id, - user=self.user, + user=None, + was_impersonated=True, item_id=None, scope="dinglehopper", - activity="updated", + activity="added_to_clink_expander", detail=Detail(), ) with pytest.raises(ActivityLog.DoesNotExist): ActivityLog.objects.latest("id") - def test_can_not_save_if_there_is_neither_a_team_id_nor_an_organisation_id(self) -> None: + def test_does_not_save_if_there_is_neither_a_team_id_nor_an_organisation_id(self) -> None: # even when there are logs with team id or org id saved ActivityLog.objects.create(team_id=3) ActivityLog.objects.create(organization_id=UUIDT()) @@ -81,20 +84,22 @@ def test_can_not_save_if_there_is_neither_a_team_id_nor_an_organisation_id(self) def test_does_not_throw_if_cannot_log_activity(self) -> None: with self.assertLogs(level="WARN") as log: - try: - log_activity( - organization_id=UUIDT(), - team_id=1, - # will cause logging to raise exception because user is unsaved - # avoids needing to mock anything to force the exception - user=User(first_name="testy", email="test@example.com"), - item_id="12345", - scope="testing throwing exceptions on create", - activity="does not explode", - detail=Detail(), - ) - except Exception as e: - raise pytest.fail(f"Should not have raised exception: {e}") + with self.settings(TEST=False): # Enable production-level silencing + try: + log_activity( + organization_id=UUIDT(), + team_id=1, + # will cause logging to raise exception because user is unsaved + # avoids needing to mock anything to force the exception + user=User(first_name="testy", email="test@example.com"), + was_impersonated=False, + item_id="12345", + scope="testing throwing exceptions on create", + activity="does not explode", + detail=Detail(), + ) + except Exception as e: + raise pytest.fail(f"Should not have raised exception: {e}") logged_warning = log.records[0].__dict__ self.assertEqual(logged_warning["levelname"], "WARNING") diff --git a/posthog/utils.py b/posthog/utils.py index ecefc4ea1f6e0..1689eef3f6cc9 100644 --- a/posthog/utils.py +++ b/posthog/utils.py @@ -309,12 +309,10 @@ def render_template( If team_for_public_context is provided, this means this is a public page such as a shared dashboard. """ - from loginas.utils import is_impersonated_session template = get_template(template_name) context["opt_out_capture"] = settings.OPT_OUT_CAPTURE - context["impersonated_session"] = is_impersonated_session(request) context["self_capture"] = settings.SELF_CAPTURE if sentry_dsn := os.environ.get("SENTRY_DSN"):