From e6c2e5fe52386674b9a3775b799c37ca1a796f90 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 10:41:51 +0000 Subject: [PATCH 01/10] feat(messaging): configure push identity verification in the channel setup modals Adds an identity verification control (disabled / optional / required) to the Firebase and APNs setup modals, writing the push_identity_verification key the device registration endpoint already reads. Until now the mode could only be set by editing the integration config directly. Also lets the APNs signing key be uploaded as a .p8 file instead of only pasted, since that is the form Apple hands it out in. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- .../Channels/APNSSetup/APNSSetupModal.tsx | 34 +++++++--- .../Channels/APNSSetup/apnsSetupModalLogic.ts | 62 ++++++++++++++++++- .../Channels/FCMSetup/FCMSetupModal.tsx | 4 +- .../Channels/FCMSetup/fcmSetupModalLogic.ts | 28 ++++++++- .../PushIdentityVerificationField.tsx | 39 ++++++++++++ 5 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 products/workflows/frontend/Channels/PushIdentityVerificationField.tsx diff --git a/products/workflows/frontend/Channels/APNSSetup/APNSSetupModal.tsx b/products/workflows/frontend/Channels/APNSSetup/APNSSetupModal.tsx index c867068053ac..b462a37f98f3 100644 --- a/products/workflows/frontend/Channels/APNSSetup/APNSSetupModal.tsx +++ b/products/workflows/frontend/Channels/APNSSetup/APNSSetupModal.tsx @@ -4,12 +4,14 @@ import { Form } from 'kea-forms' import { LemonButton, LemonInput, LemonModal, LemonSegmentedButton, LemonTextArea, Link } from '@posthog/lemon-ui' import { LemonField } from 'lib/lemon-ui/LemonField' +import { LemonFileInput } from 'lib/lemon-ui/LemonFileInput' +import { PushIdentityVerificationField } from '../PushIdentityVerificationField' import { APNSSetupModalLogicProps, apnsSetupModalLogic } from './apnsSetupModalLogic' export const APNSSetupModal = (props: APNSSetupModalLogicProps): JSX.Element => { - const { isApnsIntegrationSubmitting } = useValues(apnsSetupModalLogic(props)) - const { submitApnsIntegration } = useActions(apnsSetupModalLogic(props)) + const { isApnsIntegrationSubmitting, apnsIntegration, signingKeyFileError } = useValues(apnsSetupModalLogic(props)) + const { submitApnsIntegration, setSigningKeyFiles } = useActions(apnsSetupModalLogic(props)) return ( {' '} under Certificates, Identifiers & Profiles > Keys.

- - + + + + + Upload .p8 file + + } /> - + {signingKeyFileError &&

{signingKeyFileError}

} + @@ -53,6 +72,7 @@ export const APNSSetupModal = (props: APNSSetupModalLogicProps): JSX.Element => fullWidth /> +
void @@ -20,12 +22,18 @@ export interface APNSFormType { teamId: string bundleId: string environment: 'production' | 'sandbox' + identityVerification: PushIdentityVerificationMode } +// Apple hands out the signing key as a .p8 file, so accepting an upload saves opening it in an +// editor to copy the PEM out. Both routes populate the same `signingKey` field. +const PEM_MARKER = 'PRIVATE KEY' + // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface apnsSetupModalLogicValues { apnsIntegration: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -36,6 +44,7 @@ export interface apnsSetupModalLogicValues { apnsIntegrationErrors: DeepPartialMap< { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -50,6 +59,7 @@ export interface apnsSetupModalLogicValues { apnsIntegrationValidationErrors: DeepPartialMap< { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -57,6 +67,7 @@ export interface apnsSetupModalLogicValues { }, ValidationErrorType > + signingKeyFileError: string | null isApnsIntegrationSubmitting: boolean isApnsIntegrationValid: boolean showApnsIntegrationErrors: boolean @@ -65,8 +76,15 @@ export interface apnsSetupModalLogicValues { // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface apnsSetupModalLogicActions { loadIntegrations: () => any // integrationsLogic + setSigningKeyFileError: (error: string | null) => { + error: string | null + } + setSigningKeyFiles: (files: File[]) => { + files: File[] + } resetApnsIntegration: (values?: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -74,6 +92,7 @@ export interface apnsSetupModalLogicActions { }) => { values?: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -93,6 +112,7 @@ export interface apnsSetupModalLogicActions { setApnsIntegrationValues: ( values: DeepPartial<{ bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -101,6 +121,7 @@ export interface apnsSetupModalLogicActions { ) => { values: DeepPartial<{ bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -119,6 +140,7 @@ export interface apnsSetupModalLogicActions { } submitApnsIntegrationRequest: (apnsIntegration: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -126,6 +148,7 @@ export interface apnsSetupModalLogicActions { }) => { apnsIntegration: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -134,6 +157,7 @@ export interface apnsSetupModalLogicActions { } submitApnsIntegrationSuccess: (apnsIntegration: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -141,6 +165,7 @@ export interface apnsSetupModalLogicActions { }) => { apnsIntegration: { bundleId: string + identityVerification: PushIdentityVerificationMode environment: 'production' keyId: string signingKey: string @@ -164,6 +189,39 @@ export const apnsSetupModalLogic = kea([ connect(() => ({ actions: [integrationsLogic, ['loadIntegrations']], })), + actions({ + setSigningKeyFiles: (files: File[]) => ({ files }), + setSigningKeyFileError: (error: string | null) => ({ error }), + }), + reducers({ + signingKeyFileError: [ + null as string | null, + { + setSigningKeyFileError: (_, { error }) => error, + setSigningKeyFiles: () => null, + }, + ], + }), + listeners(({ actions }) => ({ + setSigningKeyFiles: async ({ files }) => { + const file = files[0] + if (!file) { + return + } + try { + const contents = await file.text() + if (!contents.includes(PEM_MARKER)) { + actions.setSigningKeyFileError( + "That file doesn't look like a signing key. Upload the .p8 file you downloaded from Apple." + ) + return + } + actions.setApnsIntegrationValue('signingKey', contents.trim()) + } catch { + actions.setSigningKeyFileError("Couldn't read that file. Try again, or paste the key contents instead.") + } + }, + })), forms(({ props, actions, values }) => ({ apnsIntegration: { defaults: { @@ -172,6 +230,7 @@ export const apnsSetupModalLogic = kea([ teamId: '', bundleId: '', environment: 'production' as const, + identityVerification: PUSH_IDENTITY_VERIFICATION_DEFAULT, }, errors: ({ signingKey, keyId, teamId, bundleId }) => ({ signingKey: signingKey.trim() ? undefined : 'Signing key is required', @@ -189,6 +248,7 @@ export const apnsSetupModalLogic = kea([ team_id_apple: values.apnsIntegration.teamId, bundle_id: values.apnsIntegration.bundleId, environment: values.apnsIntegration.environment, + push_identity_verification: values.apnsIntegration.identityVerification, }, }) actions.loadIntegrations() diff --git a/products/workflows/frontend/Channels/FCMSetup/FCMSetupModal.tsx b/products/workflows/frontend/Channels/FCMSetup/FCMSetupModal.tsx index 9d6ad9ed87ed..2c792a7b9b76 100644 --- a/products/workflows/frontend/Channels/FCMSetup/FCMSetupModal.tsx +++ b/products/workflows/frontend/Channels/FCMSetup/FCMSetupModal.tsx @@ -5,10 +5,11 @@ import { LemonButton, LemonModal, LemonTextArea, Link } from '@posthog/lemon-ui' import { LemonField } from 'lib/lemon-ui/LemonField' +import { PushIdentityVerificationField } from '../PushIdentityVerificationField' import { FCMSetupModalLogicProps, fcmSetupModalLogic } from './fcmSetupModalLogic' export const FCMSetupModal = (props: FCMSetupModalLogicProps): JSX.Element => { - const { isFcmIntegrationSubmitting } = useValues(fcmSetupModalLogic(props)) + const { isFcmIntegrationSubmitting, fcmIntegration } = useValues(fcmSetupModalLogic(props)) const { submitFcmIntegration } = useActions(fcmSetupModalLogic(props)) return ( @@ -35,6 +36,7 @@ export const FCMSetupModal = (props: FCMSetupModalLogicProps): JSX.Element => { +
void @@ -16,18 +18,21 @@ export interface FCMSetupModalLogicProps { export interface FCMFormType { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode } // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface fcmSetupModalLogicValues { fcmIntegration: { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode } fcmIntegrationAllErrors: Record fcmIntegrationChanged: boolean fcmIntegrationErrors: DeepPartialMap< { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode }, ValidationErrorType > @@ -38,6 +43,7 @@ export interface fcmSetupModalLogicValues { fcmIntegrationValidationErrors: DeepPartialMap< { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode }, ValidationErrorType > @@ -49,9 +55,13 @@ export interface fcmSetupModalLogicValues { // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface fcmSetupModalLogicActions { loadIntegrations: () => any // integrationsLogic - resetFcmIntegration: (values?: { serviceAccountKey: string }) => { + resetFcmIntegration: (values?: { + serviceAccountKey: string + identityVerification: PushIdentityVerificationMode + }) => { values?: { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode } } setFcmIntegrationManualErrors: (errors: Record) => { @@ -67,10 +77,12 @@ export interface fcmSetupModalLogicActions { setFcmIntegrationValues: ( values: DeepPartial<{ serviceAccountKey: string + identityVerification: PushIdentityVerificationMode }> ) => { values: DeepPartial<{ serviceAccountKey: string + identityVerification: PushIdentityVerificationMode }> } submitFcmIntegration: () => { @@ -83,14 +95,22 @@ export interface fcmSetupModalLogicActions { error: Error errors: Record } - submitFcmIntegrationRequest: (fcmIntegration: { serviceAccountKey: string }) => { + submitFcmIntegrationRequest: (fcmIntegration: { + serviceAccountKey: string + identityVerification: PushIdentityVerificationMode + }) => { fcmIntegration: { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode } } - submitFcmIntegrationSuccess: (fcmIntegration: { serviceAccountKey: string }) => { + submitFcmIntegrationSuccess: (fcmIntegration: { + serviceAccountKey: string + identityVerification: PushIdentityVerificationMode + }) => { fcmIntegration: { serviceAccountKey: string + identityVerification: PushIdentityVerificationMode } } touchFcmIntegrationField: (key: string) => { @@ -114,6 +134,7 @@ export const fcmSetupModalLogic = kea([ fcmIntegration: { defaults: { serviceAccountKey: '', + identityVerification: PUSH_IDENTITY_VERIFICATION_DEFAULT, }, errors: ({ serviceAccountKey }) => ({ serviceAccountKey: serviceAccountKey.trim() ? undefined : 'Service account key is required', @@ -132,6 +153,7 @@ export const fcmSetupModalLogic = kea([ kind: 'firebase', config: { key_info: keyInfo, + push_identity_verification: values.fcmIntegration.identityVerification, }, }) actions.loadIntegrations() diff --git a/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx new file mode 100644 index 000000000000..863d90a1a62e --- /dev/null +++ b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx @@ -0,0 +1,39 @@ +import { LemonSegmentedButton } from '@posthog/lemon-ui' + +import { LemonField } from 'lib/lemon-ui/LemonField' + +export type PushIdentityVerificationMode = 'disabled' | 'optional' | 'required' + +export const PUSH_IDENTITY_VERIFICATION_DEFAULT: PushIdentityVerificationMode = 'disabled' + +const MODE_HELP: Record = { + disabled: 'Any client with your project API key can register a device for any user.', + optional: + 'Tokens are checked and recorded, but devices can still register without one. Use this to confirm your app sends valid tokens before you require them.', + required: + 'Devices without a valid token cannot register or unregister. Switch to this only once your app sends tokens, otherwise registration stops working.', +} + +/** + * Shared by the Firebase and APNs setup modals: both write the same + * `push_identity_verification` key, which the device registration endpoint reads. + */ +export function PushIdentityVerificationField({ mode }: { mode: PushIdentityVerificationMode }): JSX.Element { + return ( + + + + ) +} From 830ff53cca4f27fd37e5e1127b5c0de5189b746d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 10:57:42 +0000 Subject: [PATCH 02/10] fix(messaging): keep push identity verification across credential rotation The setup modals write push_identity_verification into the integration config, but the serializer only forwarded named provider fields, so the value was dropped before reaching the integration. Thread it through for both Firebase and APNs. Both integration_from_key helpers rebuild config from the credentials they are handed, so a credential rotation would also wipe the flag back to disabled and silently reopen device takeover. Carry the existing value forward unless the caller sets a new one, and reject unknown modes. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/api/integration.py | 8 +++- posthog/models/integration.py | 48 ++++++++++++++++++- .../test/test_apple_push_integration.py | 14 ++++++ .../models/test/test_firebase_integration.py | 16 ++++++- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/posthog/api/integration.py b/posthog/api/integration.py index 11e819855dfd..81b9f57fd873 100644 --- a/posthog/api/integration.py +++ b/posthog/api/integration.py @@ -458,7 +458,12 @@ def _build_integration(self, validated_data: Any) -> Any: key_info = config.get("key_info") if not key_info: raise ValidationError("Firebase service account key must be provided") - instance = FirebaseIntegration.integration_from_key(key_info, team_id, request.user) + instance = FirebaseIntegration.integration_from_key( + key_info, + team_id, + request.user, + push_identity_verification=(validated_data.get("config") or {}).get("push_identity_verification"), + ) return instance elif validated_data["kind"] == "email": @@ -670,6 +675,7 @@ def _build_integration(self, validated_data: Any) -> Any: team_id=team_id, created_by=request.user, environment=environment, + push_identity_verification=config.get("push_identity_verification"), ) return instance diff --git a/posthog/models/integration.py b/posthog/models/integration.py index bd0111d2fd16..bcb9adef3ac0 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -110,6 +110,40 @@ def _decode_jwt_payload(token: str) -> dict | None: # `config` key flagging a grant that only the legacy fallback credentials can refresh. CONFIG_LEGACY_OAUTH_CLIENT = "oauth_uses_legacy_client" +# `config` key holding the push device-registration identity verification policy, read by the +# push subscriptions endpoint. Owned by the customer, not by the provider credentials. +CONFIG_PUSH_IDENTITY_VERIFICATION = "push_identity_verification" +PUSH_IDENTITY_VERIFICATION_MODES = ("disabled", "optional", "required") + + +def preserved_push_config( + team_id: int, + kind: str, + integration_id: str, + push_identity_verification: str | None, +) -> dict: + """Config keys a push credential upsert must carry over rather than drop. + + Connecting a push integration is an upsert, and the provider helpers rebuild `config` from the + credentials they were handed. Anything they don't know about would be lost, so rotating a + Firebase key or APNs .p8 would silently reset an enabled identity verification policy back to + disabled, reopening the device takeover it exists to prevent. Carry the existing value forward + unless the caller explicitly sets a new one. + """ + if push_identity_verification is not None: + if push_identity_verification not in PUSH_IDENTITY_VERIFICATION_MODES: + raise ValidationError( + f"push_identity_verification must be one of: {', '.join(PUSH_IDENTITY_VERIFICATION_MODES)}" + ) + return {CONFIG_PUSH_IDENTITY_VERIFICATION: push_identity_verification} + + existing = ( + Integration.objects.filter(team_id=team_id, kind=kind, integration_id=integration_id).only("config").first() + ) + existing_mode = (existing.config or {}).get(CONFIG_PUSH_IDENTITY_VERIFICATION) if existing else None + return {CONFIG_PUSH_IDENTITY_VERIFICATION: existing_mode} if existing_mode else {} + + # Values for the counter's `reason` label, bucketed from the OAuth error response. REFRESH_FAILURE_REASON_INVALID_GRANT = "invalid_grant" REFRESH_FAILURE_REASON_INVALID_CLIENT = "invalid_client" @@ -2206,7 +2240,13 @@ def __init__(self, integration: Integration) -> None: self.integration = integration @classmethod - def integration_from_key(cls, key_info: dict, team_id: int, created_by: User | None = None) -> "Integration": + def integration_from_key( + cls, + key_info: dict, + team_id: int, + created_by: User | None = None, + push_identity_verification: str | None = None, + ) -> "Integration": scope = "https://www.googleapis.com/auth/firebase.messaging" try: @@ -2225,6 +2265,7 @@ def integration_from_key(cls, key_info: dict, team_id: int, created_by: User | N integration_id=project_id, defaults={ "config": { + **preserved_push_config(team_id, "firebase", project_id, push_identity_verification), "project_id": project_id, "expires_in": credentials.expiry.timestamp() - int(time.time()), "refreshed_at": int(time.time()), @@ -2316,6 +2357,7 @@ def integration_from_key( team_id: int, created_by: User | None = None, environment: str = "production", + push_identity_verification: str | None = None, ) -> "Integration": if not all([signing_key, key_id, team_id_apple, bundle_id]): raise ValidationError("All APNS fields are required: signing_key, key_id, team_id_apple, bundle_id") @@ -2323,12 +2365,14 @@ def integration_from_key( if environment not in ("production", "sandbox"): raise ValidationError("APNS environment must be 'production' or 'sandbox'") + integration_id = f"{team_id_apple}.{bundle_id}" integration, created = Integration.objects.update_or_create( team_id=team_id, kind="apns", - integration_id=f"{team_id_apple}.{bundle_id}", + integration_id=integration_id, defaults={ "config": { + **preserved_push_config(team_id, "apns", integration_id, push_identity_verification), "team_id": team_id_apple, "bundle_id": bundle_id, "key_id": key_id, diff --git a/posthog/models/test/test_apple_push_integration.py b/posthog/models/test/test_apple_push_integration.py index f7294cd458ae..1b86442f7475 100644 --- a/posthog/models/test/test_apple_push_integration.py +++ b/posthog/models/test/test_apple_push_integration.py @@ -12,6 +12,7 @@ def _create_apple_push_integration( key_id: str = "ABC123KEY", team_id_apple: str = "TEAM123", bundle_id: str = "com.example.app", + push_identity_verification: str | None = None, ) -> Integration: return ApplePushIntegration.integration_from_key( signing_key=signing_key, @@ -19,6 +20,7 @@ def _create_apple_push_integration( team_id_apple=team_id_apple, bundle_id=bundle_id, team_id=self.team.id, + push_identity_verification=push_identity_verification, ) def test_creates_integration(self): @@ -39,6 +41,18 @@ def test_upserts_on_same_team_and_bundle(self): second.refresh_from_db() assert second.config["key_id"] == "NEW_KEY_ID" + def test_reconnecting_preserves_identity_verification(self): + # Rotating the .p8 signing key is a routine action that re-upserts the integration. It must + # not silently reset the verification policy, which would reopen device takeover. + self._create_apple_push_integration(push_identity_verification="required") + reconnected = self._create_apple_push_integration(key_id="ROTATED_KEY") + + assert reconnected.config["push_identity_verification"] == "required" + + def test_rejects_an_unknown_identity_verification_mode(self): + with self.assertRaises(ValidationError): + self._create_apple_push_integration(push_identity_verification="enabled") + def test_separate_integrations_for_different_bundles(self): first = self._create_apple_push_integration(bundle_id="com.example.app1") second = self._create_apple_push_integration(bundle_id="com.example.app2") diff --git a/posthog/models/test/test_firebase_integration.py b/posthog/models/test/test_firebase_integration.py index 8bd3023fcdb1..c48477c8dc99 100644 --- a/posthog/models/test/test_firebase_integration.py +++ b/posthog/models/test/test_firebase_integration.py @@ -36,7 +36,9 @@ def _create_firebase_integration(self, mock_from_sa, mock_google_request, **over key_info = overrides.pop("key_info", FAKE_KEY_INFO) team_id = overrides.pop("team_id", self.team.id) created_by = overrides.pop("created_by", None) - return FirebaseIntegration.integration_from_key(key_info, team_id, created_by) + return FirebaseIntegration.integration_from_key( + key_info, team_id, created_by, push_identity_verification=overrides.pop("push_identity_verification", None) + ) def test_creates_integration(self): integration = self._create_firebase_integration() @@ -55,6 +57,18 @@ def test_upserts_on_same_project(self): assert first.id == second.id + def test_reconnecting_preserves_identity_verification(self): + # Rotating the service account key is a routine action that re-upserts the integration. It + # must not silently reset the verification policy, which would reopen device takeover. + self._create_firebase_integration(push_identity_verification="required") + reconnected = self._create_firebase_integration() + + assert reconnected.config["push_identity_verification"] == "required" + + def test_rejects_an_unknown_identity_verification_mode(self): + with self.assertRaises(ValidationError): + self._create_firebase_integration(push_identity_verification="enabled") + def test_separate_integrations_for_different_projects(self): first = self._create_firebase_integration() From c471f595a7c7794e0065b3e809d1f3f048c721e7 Mon Sep 17 00:00:00 2001 From: Daniel Marchuk Date: Tue, 28 Jul 2026 18:02:49 +0200 Subject: [PATCH 03/10] chore(messaging): reconcile kea typegen for push channel setup logics --- .../Channels/APNSSetup/apnsSetupModalLogic.ts | 36 +++++++++---------- .../Channels/FCMSetup/fcmSetupModalLogic.ts | 22 ++++++------ 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts index a49471393dfd..7e9b2854f9c7 100644 --- a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts @@ -33,8 +33,8 @@ const PEM_MARKER = 'PRIVATE KEY' export interface apnsSetupModalLogicValues { apnsIntegration: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string @@ -44,8 +44,8 @@ export interface apnsSetupModalLogicValues { apnsIntegrationErrors: DeepPartialMap< { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string @@ -59,41 +59,35 @@ export interface apnsSetupModalLogicValues { apnsIntegrationValidationErrors: DeepPartialMap< { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string }, ValidationErrorType > - signingKeyFileError: string | null isApnsIntegrationSubmitting: boolean isApnsIntegrationValid: boolean showApnsIntegrationErrors: boolean + signingKeyFileError: string | null } // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface apnsSetupModalLogicActions { loadIntegrations: () => any // integrationsLogic - setSigningKeyFileError: (error: string | null) => { - error: string | null - } - setSigningKeyFiles: (files: File[]) => { - files: File[] - } resetApnsIntegration: (values?: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string }) => { values?: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string @@ -112,8 +106,8 @@ export interface apnsSetupModalLogicActions { setApnsIntegrationValues: ( values: DeepPartial<{ bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string @@ -121,13 +115,19 @@ export interface apnsSetupModalLogicActions { ) => { values: DeepPartial<{ bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string }> } + setSigningKeyFileError: (error: string | null) => { + error: string | null + } + setSigningKeyFiles: (files: File[]) => { + files: File[] + } submitApnsIntegration: () => { value: boolean } @@ -140,16 +140,16 @@ export interface apnsSetupModalLogicActions { } submitApnsIntegrationRequest: (apnsIntegration: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string }) => { apnsIntegration: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string @@ -157,16 +157,16 @@ export interface apnsSetupModalLogicActions { } submitApnsIntegrationSuccess: (apnsIntegration: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string }) => { apnsIntegration: { bundleId: string - identityVerification: PushIdentityVerificationMode environment: 'production' + identityVerification: PushIdentityVerificationMode keyId: string signingKey: string teamId: string diff --git a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts index d3375ec1b4a2..d4fc8d1eb249 100644 --- a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts @@ -24,15 +24,15 @@ export interface FCMFormType { // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface fcmSetupModalLogicValues { fcmIntegration: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string } fcmIntegrationAllErrors: Record fcmIntegrationChanged: boolean fcmIntegrationErrors: DeepPartialMap< { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }, ValidationErrorType > @@ -42,8 +42,8 @@ export interface fcmSetupModalLogicValues { fcmIntegrationTouches: Record fcmIntegrationValidationErrors: DeepPartialMap< { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }, ValidationErrorType > @@ -56,12 +56,12 @@ export interface fcmSetupModalLogicValues { export interface fcmSetupModalLogicActions { loadIntegrations: () => any // integrationsLogic resetFcmIntegration: (values?: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }) => { values?: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string } } setFcmIntegrationManualErrors: (errors: Record) => { @@ -76,13 +76,13 @@ export interface fcmSetupModalLogicActions { } setFcmIntegrationValues: ( values: DeepPartial<{ - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }> ) => { values: DeepPartial<{ - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }> } submitFcmIntegration: () => { @@ -96,21 +96,21 @@ export interface fcmSetupModalLogicActions { errors: Record } submitFcmIntegrationRequest: (fcmIntegration: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }) => { fcmIntegration: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string } } submitFcmIntegrationSuccess: (fcmIntegration: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string }) => { fcmIntegration: { - serviceAccountKey: string identityVerification: PushIdentityVerificationMode + serviceAccountKey: string } } touchFcmIntegrationField: (key: string) => { From 54b96dcc1a89610855e1f1c14fba62e56f40e533 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 09:39:25 +0000 Subject: [PATCH 04/10] fix(messaging): close review findings on push identity verification setup Three issues raised in review, all reachable through the new setup UI: The setup forms defaulted the mode to disabled and always submitted it, so reconnecting an integration to rotate credentials sent an explicit disabled and bypassed the backend preservation this PR added. Seed the field from the integration's stored policy instead. Reading the existing policy and upserting it back are separate statements, so a concurrent write enabling verification could be overwritten by the stale read. Lock the row and hold it through the upsert. Enabling verification is a security policy change, so require project admin. A plain member could otherwise create a new APNs integration carrying another app's bundle_id under a different Apple team id, which creates rather than overwrites and so escaped the existing admin check, and set required on it. The push endpoint takes the strictest mode across integrations sharing a bundle_id, so that would reject the real app's registrations. Also cancel a stale .p8 read when a second file is picked, so the newer selection wins rather than whichever read finishes last. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/api/integration.py | 16 ++++ posthog/api/test/test_integration.py | 31 ++++++++ posthog/models/integration.py | 78 +++++++++++-------- .../Channels/APNSSetup/apnsSetupModalLogic.ts | 28 ++++--- .../Channels/FCMSetup/fcmSetupModalLogic.ts | 4 +- .../PushIdentityVerificationField.tsx | 14 ++++ 6 files changed, 124 insertions(+), 47 deletions(-) diff --git a/posthog/api/integration.py b/posthog/api/integration.py index 966622ce444b..043c0f0a0fa3 100644 --- a/posthog/api/integration.py +++ b/posthog/api/integration.py @@ -416,6 +416,22 @@ def create(self, validated_data: Any) -> Any: team_id = self.context["team_id"] kind = validated_data["kind"] + # Enabling push identity verification is a security policy change, not a credential upload, so + # it needs the same admin bar as editing an integration. Without this a plain member could add + # a *new* APNs integration carrying someone else's bundle_id under a different Apple team id — + # which sidesteps the overwrite check below, since it creates rather than overwrites — and set + # `required` on it. The push endpoint resolves the strictest mode across every integration + # matching a bundle_id, so that would start rejecting the real app's device registrations. + requested_verification = (validated_data.get("config") or {}).get("push_identity_verification") + if ( + requested_verification + and requested_verification != "disabled" + and not github_callback_state.has_team_management_access( + self.context["request"].user, self.context["get_team"]() + ) + ): + raise PermissionDenied("Changing push identity verification requires project admin access.") + # `create` is a POST with upsert semantics: each kind's helper does an `update_or_create` # keyed on (team, kind, integration_id), so re-submitting the same resource overwrites the # existing integration instead of adding a new one. Adding is allowed for any project diff --git a/posthog/api/test/test_integration.py b/posthog/api/test/test_integration.py index 8da0fc3fcc74..b6c1f3edfc1b 100644 --- a/posthog/api/test/test_integration.py +++ b/posthog/api/test/test_integration.py @@ -4967,6 +4967,37 @@ def test_member_cannot_delete_integration(self): assert response.status_code == status.HTTP_403_FORBIDDEN, response.content assert Integration.objects.filter(id=integration.id).exists() + def test_member_cannot_enable_push_identity_verification(self): + # Creating a *new* APNs integration sidesteps the overwrite check below, because a different + # Apple team id makes a different integration_id. But the push endpoint resolves the strictest + # verification mode across every integration sharing a bundle_id, so a member could otherwise + # set `required` on a lookalike and block the real app's device registrations. + Integration.objects.create( + team=self.team, + kind="apns", + integration_id="REALTEAM.com.example.app", + config={"bundle_id": "com.example.app", "team_id": "REALTEAM", "key_id": "KEY1"}, + sensitive_config={}, + ) + + response = self.client.post( + f"/api/environments/{self.team.pk}/integrations", + { + "kind": "apns", + "config": { + "signing_key": "-----BEGIN PRIVATE KEY-----\nfake\n-----END PRIVATE KEY-----", + "key_id": "KEY2", + "team_id_apple": "ATTACKER", + "bundle_id": "com.example.app", + "push_identity_verification": "required", + }, + }, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN, response.content + assert not Integration.objects.filter(team=self.team, integration_id="ATTACKER.com.example.app").exists() + def test_member_cannot_overwrite_existing_integration(self): # POST is an upsert (update_or_create keyed on team/kind/integration_id), so re-submitting the # same resource edits an existing integration. Members may add a new one, but overwriting an diff --git a/posthog/models/integration.py b/posthog/models/integration.py index 14162728d7c3..cc14217d0080 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -137,8 +137,14 @@ def preserved_push_config( ) return {CONFIG_PUSH_IDENTITY_VERIFICATION: push_identity_verification} + # Lock the row for the rest of the caller's transaction. The read and the upsert that follows it + # are separate statements, so without this a concurrent write that enables verification could land + # in between and then be overwritten by the stale value read here — silently disabling the policy. existing = ( - Integration.objects.filter(team_id=team_id, kind=kind, integration_id=integration_id).only("config").first() + Integration.objects.select_for_update() + .filter(team_id=team_id, kind=kind, integration_id=integration_id) + .only("config") + .first() ) existing_mode = (existing.config or {}).get(CONFIG_PUSH_IDENTITY_VERIFICATION) if existing else None return {CONFIG_PUSH_IDENTITY_VERIFICATION: existing_mode} if existing_mode else {} @@ -2259,24 +2265,26 @@ def integration_from_key( if not project_id: raise ValidationError("Service account key must contain a project_id") - integration, created = Integration.objects.update_or_create( - team_id=team_id, - kind="firebase", - integration_id=project_id, - defaults={ - "config": { - **preserved_push_config(team_id, "firebase", project_id, push_identity_verification), - "project_id": project_id, - "expires_in": credentials.expiry.timestamp() - int(time.time()), - "refreshed_at": int(time.time()), - }, - "sensitive_config": { - "key_info": key_info, - "access_token": credentials.token, + # Atomic so `preserved_push_config`'s row lock is held through the upsert that follows it. + with transaction.atomic(): + integration, created = Integration.objects.update_or_create( + team_id=team_id, + kind="firebase", + integration_id=project_id, + defaults={ + "config": { + **preserved_push_config(team_id, "firebase", project_id, push_identity_verification), + "project_id": project_id, + "expires_in": credentials.expiry.timestamp() - int(time.time()), + "refreshed_at": int(time.time()), + }, + "sensitive_config": { + "key_info": key_info, + "access_token": credentials.token, + }, + "created_by": created_by, }, - "created_by": created_by, - }, - ) + ) if integration.errors: integration.errors = "" @@ -2366,23 +2374,25 @@ def integration_from_key( raise ValidationError("APNS environment must be 'production' or 'sandbox'") integration_id = f"{team_id_apple}.{bundle_id}" - integration, created = Integration.objects.update_or_create( - team_id=team_id, - kind="apns", - integration_id=integration_id, - defaults={ - "config": { - **preserved_push_config(team_id, "apns", integration_id, push_identity_verification), - "team_id": team_id_apple, - "bundle_id": bundle_id, - "key_id": key_id, - "environment": environment, - }, - "sensitive_config": { - "signing_key": signing_key, + # Atomic so `preserved_push_config`'s row lock is held through the upsert that follows it. + with transaction.atomic(): + integration, created = Integration.objects.update_or_create( + team_id=team_id, + kind="apns", + integration_id=integration_id, + defaults={ + "config": { + **preserved_push_config(team_id, "apns", integration_id, push_identity_verification), + "team_id": team_id_apple, + "bundle_id": bundle_id, + "key_id": key_id, + "environment": environment, + }, + "sensitive_config": { + "signing_key": signing_key, + }, }, - }, - ) + ) if created and created_by is not None: integration.created_by = created_by diff --git a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts index 7e9b2854f9c7..bc44d19e9d7a 100644 --- a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts @@ -8,7 +8,7 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast' import { IntegrationType } from '~/types' -import { PUSH_IDENTITY_VERIFICATION_DEFAULT, PushIdentityVerificationMode } from '../PushIdentityVerificationField' +import { PushIdentityVerificationMode, resolvePushIdentityVerification } from '../PushIdentityVerificationField' export interface APNSSetupModalLogicProps { integration?: IntegrationType | null @@ -203,23 +203,29 @@ export const apnsSetupModalLogic = kea([ ], }), listeners(({ actions }) => ({ - setSigningKeyFiles: async ({ files }) => { + setSigningKeyFiles: async ({ files }, breakpoint) => { const file = files[0] if (!file) { return } + let contents: string try { - const contents = await file.text() - if (!contents.includes(PEM_MARKER)) { - actions.setSigningKeyFileError( - "That file doesn't look like a signing key. Upload the .p8 file you downloaded from Apple." - ) - return - } - actions.setApnsIntegrationValue('signingKey', contents.trim()) + contents = await file.text() } catch { actions.setSigningKeyFileError("Couldn't read that file. Try again, or paste the key contents instead.") + return + } + // Picking a second file while this read is in flight makes this result stale. Without the + // breakpoint whichever read finished last would win, so an earlier file could overwrite the + // key while the input shows the newer filename. + breakpoint() + if (!contents.includes(PEM_MARKER)) { + actions.setSigningKeyFileError( + "That file doesn't look like a signing key. Upload the .p8 file you downloaded from Apple." + ) + return } + actions.setApnsIntegrationValue('signingKey', contents.trim()) }, })), forms(({ props, actions, values }) => ({ @@ -230,7 +236,7 @@ export const apnsSetupModalLogic = kea([ teamId: '', bundleId: '', environment: 'production' as const, - identityVerification: PUSH_IDENTITY_VERIFICATION_DEFAULT, + identityVerification: resolvePushIdentityVerification(props.integration), }, errors: ({ signingKey, keyId, teamId, bundleId }) => ({ signingKey: signingKey.trim() ? undefined : 'Signing key is required', diff --git a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts index d4fc8d1eb249..bfe51cb9a3f4 100644 --- a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts @@ -8,7 +8,7 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast' import { IntegrationType } from '~/types' -import { PUSH_IDENTITY_VERIFICATION_DEFAULT, PushIdentityVerificationMode } from '../PushIdentityVerificationField' +import { PushIdentityVerificationMode, resolvePushIdentityVerification } from '../PushIdentityVerificationField' export interface FCMSetupModalLogicProps { integration?: IntegrationType | null @@ -134,7 +134,7 @@ export const fcmSetupModalLogic = kea([ fcmIntegration: { defaults: { serviceAccountKey: '', - identityVerification: PUSH_IDENTITY_VERIFICATION_DEFAULT, + identityVerification: resolvePushIdentityVerification(props.integration), }, errors: ({ serviceAccountKey }) => ({ serviceAccountKey: serviceAccountKey.trim() ? undefined : 'Service account key is required', diff --git a/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx index 863d90a1a62e..09332a181d9b 100644 --- a/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx +++ b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx @@ -2,10 +2,24 @@ import { LemonSegmentedButton } from '@posthog/lemon-ui' import { LemonField } from 'lib/lemon-ui/LemonField' +import { IntegrationType } from '~/types' + export type PushIdentityVerificationMode = 'disabled' | 'optional' | 'required' +export const PUSH_IDENTITY_VERIFICATION_MODES: PushIdentityVerificationMode[] = ['disabled', 'optional', 'required'] + export const PUSH_IDENTITY_VERIFICATION_DEFAULT: PushIdentityVerificationMode = 'disabled' +/** + * Seed the form from what the integration already has. Reconnecting to rotate credentials submits + * this field like any other, so defaulting to `disabled` would silently turn verification off for a + * channel that had it on — the backend can't tell that apart from someone deliberately disabling it. + */ +export function resolvePushIdentityVerification(integration?: IntegrationType | null): PushIdentityVerificationMode { + const stored = integration?.config?.push_identity_verification + return PUSH_IDENTITY_VERIFICATION_MODES.includes(stored) ? stored : PUSH_IDENTITY_VERIFICATION_DEFAULT +} + const MODE_HELP: Record = { disabled: 'Any client with your project API key can register a device for any user.', optional: From c76351c60295bca4ccf451b994860fa6358175d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 09:52:28 +0000 Subject: [PATCH 05/10] fix(messaging): drop unrecognized stored push verification modes A value read back from config was carried forward without checking it, so a corrupted or hand-edited mode would survive every credential rotation. Drop it instead: the push endpoint already treats an unknown mode as disabled, so this changes no enforcement behavior while letting the bad value clear itself. Chose dropping over raising so a corrupted integration can still be reconnected. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/models/integration.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/posthog/models/integration.py b/posthog/models/integration.py index cc14217d0080..2ea645cb9b3e 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -147,6 +147,11 @@ def preserved_push_config( .first() ) existing_mode = (existing.config or {}).get(CONFIG_PUSH_IDENTITY_VERIFICATION) if existing else None + # Drop a stored value we don't recognize rather than carrying it forward. The push endpoint already + # treats an unknown mode as disabled, so preserving it would keep dead data alive indefinitely, and + # raising here would leave a corrupted integration unable to rotate its credentials. + if existing_mode not in PUSH_IDENTITY_VERIFICATION_MODES: + return {} return {CONFIG_PUSH_IDENTITY_VERIFICATION: existing_mode} if existing_mode else {} From 819137a2f81f5c8a44ff312531b23a8c9d01137d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 09:54:08 +0000 Subject: [PATCH 06/10] fix(messaging): serialize first-time push integration setup select_for_update only locks a row that exists, so two concurrent first-time setups of the same integration could both read "no policy" and the later write would clobber a policy the earlier one had just set. Take an advisory transaction lock keyed on the integration's identity before the read, which covers the not-yet-created case. Keyed per integration rather than per team so it only serializes writers racing for the same one. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/models/integration.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/posthog/models/integration.py b/posthog/models/integration.py index 2ea645cb9b3e..1eafb6d35027 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -21,7 +21,7 @@ from django.conf import settings from django.core.cache import cache -from django.db import models, transaction +from django.db import connection, models, transaction from django.db.models import Q from django.dispatch import receiver from django.http import HttpRequest @@ -137,9 +137,14 @@ def preserved_push_config( ) return {CONFIG_PUSH_IDENTITY_VERIFICATION: push_identity_verification} - # Lock the row for the rest of the caller's transaction. The read and the upsert that follows it - # are separate statements, so without this a concurrent write that enables verification could land - # in between and then be overwritten by the stale value read here — silently disabling the policy. + # Serialize concurrent setup of this one integration for the rest of the caller's transaction. + # `select_for_update` alone only locks a row that already exists, so two first-time setups could + # both read "no policy" and the later write would clobber a policy the earlier one had just set. + # An advisory lock covers the not-yet-created case too, keyed on the integration's identity so it + # only serializes writers racing for the same integration. + with connection.cursor() as cursor: + cursor.execute("SELECT pg_advisory_xact_lock(%s, hashtext(%s))", [team_id, f"{kind}:{integration_id}"]) + existing = ( Integration.objects.select_for_update() .filter(team_id=team_id, kind=kind, integration_id=integration_id) From 4e642a8f22d2b24e988e34070c7c053188dfcbfa Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 10:16:55 +0000 Subject: [PATCH 07/10] test(messaging): cover the serializer path for push identity verification The existing tests call integration_from_key directly, so nothing exercised the serializer that threads push_identity_verification through to it. That layer is exactly where the value used to be dropped, leaving the setup UI's toggle inert, so assert an API-level create actually persists the mode. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/api/test/test_integration.py | 29 ++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/posthog/api/test/test_integration.py b/posthog/api/test/test_integration.py index b6c1f3edfc1b..e2e47ec4b8cc 100644 --- a/posthog/api/test/test_integration.py +++ b/posthog/api/test/test_integration.py @@ -4938,6 +4938,35 @@ def test_invalid_payload_is_rejected(self, _name, payload, mock_task, mock_repor mock_report.assert_not_called() +class TestPushIdentityVerificationAPI(APIBaseTest): + @patch("posthog.models.integration.GoogleRequest") + @patch("posthog.models.integration.service_account.Credentials.from_service_account_info") + def test_setting_the_mode_reaches_the_integration(self, mock_from_sa, _mock_google_request): + # The serializer builds each provider's arguments from named config fields, so a key it doesn't + # know about is dropped before it ever reaches the integration. That silently made the setup + # UI's toggle inert; this covers the plumbing rather than just the model helper underneath it. + credentials = MagicMock() + credentials.token = "access-token" + credentials.expiry.timestamp.return_value = time.time() + 3600 + mock_from_sa.return_value = credentials + + response = self.client.post( + f"/api/environments/{self.team.pk}/integrations", + { + "kind": "firebase", + "config": { + "key_info": {"type": "service_account", "project_id": "my-firebase-project"}, + "push_identity_verification": "required", + }, + }, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED, response.content + integration = Integration.objects.get(team=self.team, kind="firebase") + assert integration.config["push_identity_verification"] == "required" + + class TestIntegrationMembershipPermissions(APIBaseTest): def setUp(self): super().setUp() From 58576f7421f9423a21880fd19025766fb11d5c53 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 10:30:28 +0000 Subject: [PATCH 08/10] fix(messaging): grant admin in the push verification serializer test Setting a non-disabled policy requires project admin, but the base test user is a plain member, so the new happy-path test hit that gate and got a 403. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/api/test/test_integration.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/posthog/api/test/test_integration.py b/posthog/api/test/test_integration.py index e2e47ec4b8cc..97a39108b2cc 100644 --- a/posthog/api/test/test_integration.py +++ b/posthog/api/test/test_integration.py @@ -4939,6 +4939,12 @@ def test_invalid_payload_is_rejected(self, _name, payload, mock_task, mock_repor class TestPushIdentityVerificationAPI(APIBaseTest): + def setUp(self): + super().setUp() + # Setting the policy requires admin, and the base test user is a plain member. + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + @patch("posthog.models.integration.GoogleRequest") @patch("posthog.models.integration.service_account.Credentials.from_service_account_info") def test_setting_the_mode_reaches_the_integration(self, mock_from_sa, _mock_google_request): From 9f7cd305d2e82664cacfbccc98913fbb2d32a9f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 11:00:14 +0000 Subject: [PATCH 09/10] fix(workflows): take push config lock before explicit-mode return Every writer for an integration identity now participates in the advisory lock. Previously a request setting an explicit mode returned before acquiring it, so it could create the row between a preserving writer's read and write and have its policy dropped. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/models/integration.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/posthog/models/integration.py b/posthog/models/integration.py index 1eafb6d35027..065ba7c3dcc3 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -130,21 +130,24 @@ def preserved_push_config( disabled, reopening the device takeover it exists to prevent. Carry the existing value forward unless the caller explicitly sets a new one. """ - if push_identity_verification is not None: - if push_identity_verification not in PUSH_IDENTITY_VERIFICATION_MODES: - raise ValidationError( - f"push_identity_verification must be one of: {', '.join(PUSH_IDENTITY_VERIFICATION_MODES)}" - ) - return {CONFIG_PUSH_IDENTITY_VERIFICATION: push_identity_verification} + if push_identity_verification is not None and push_identity_verification not in PUSH_IDENTITY_VERIFICATION_MODES: + raise ValidationError( + f"push_identity_verification must be one of: {', '.join(PUSH_IDENTITY_VERIFICATION_MODES)}" + ) # Serialize concurrent setup of this one integration for the rest of the caller's transaction. # `select_for_update` alone only locks a row that already exists, so two first-time setups could # both read "no policy" and the later write would clobber a policy the earlier one had just set. # An advisory lock covers the not-yet-created case too, keyed on the integration's identity so it - # only serializes writers racing for the same integration. + # only serializes writers racing for the same integration. Every writer takes it, including one + # setting an explicit mode — otherwise it could slip its row in between a preserving writer's read + # and write, and have its policy dropped. with connection.cursor() as cursor: cursor.execute("SELECT pg_advisory_xact_lock(%s, hashtext(%s))", [team_id, f"{kind}:{integration_id}"]) + if push_identity_verification is not None: + return {CONFIG_PUSH_IDENTITY_VERIFICATION: push_identity_verification} + existing = ( Integration.objects.select_for_update() .filter(team_id=team_id, kind=kind, integration_id=integration_id) From 58dee5d53d254b44c04912cf489bc30c56726b7d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 11:10:14 +0000 Subject: [PATCH 10/10] fix(workflows): gate every explicit push verification value on admin The overwrite check reads existing integration ids before the write, so a member racing the first setup of a push integration is still classified as a create. Submitting an explicit `disabled` could therefore land over a policy an admin had just written, with no admin check. Any explicit value now requires admin. Omitting the key stays open to members and is what connecting a channel without touching the policy does, so the setup modals only send it when it differs from what is stored. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM --- posthog/api/integration.py | 20 ++++++++++--------- posthog/api/test/test_integration.py | 9 +++++++-- .../Channels/APNSSetup/apnsSetupModalLogic.ts | 11 ++++++++-- .../Channels/FCMSetup/fcmSetupModalLogic.ts | 11 ++++++++-- .../PushIdentityVerificationField.tsx | 12 +++++++++++ 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/posthog/api/integration.py b/posthog/api/integration.py index 043c0f0a0fa3..db7ff868cafd 100644 --- a/posthog/api/integration.py +++ b/posthog/api/integration.py @@ -416,19 +416,21 @@ def create(self, validated_data: Any) -> Any: team_id = self.context["team_id"] kind = validated_data["kind"] - # Enabling push identity verification is a security policy change, not a credential upload, so - # it needs the same admin bar as editing an integration. Without this a plain member could add - # a *new* APNs integration carrying someone else's bundle_id under a different Apple team id — + # Setting push identity verification is a security policy change, not a credential upload, so it + # needs the same admin bar as editing an integration. Without this a plain member could add a + # *new* APNs integration carrying someone else's bundle_id under a different Apple team id — # which sidesteps the overwrite check below, since it creates rather than overwrites — and set # `required` on it. The push endpoint resolves the strictest mode across every integration # matching a bundle_id, so that would start rejecting the real app's device registrations. + # + # `disabled` is gated too, not just the enabling modes. The overwrite check below reads the + # existing ids before the write, so a member racing the first setup of an integration would + # still be classified as a create and could land `disabled` over the policy an admin had just + # written. Omitting the key entirely stays open to members and is what connecting a channel + # without touching the policy does — that path preserves whatever is already stored. requested_verification = (validated_data.get("config") or {}).get("push_identity_verification") - if ( - requested_verification - and requested_verification != "disabled" - and not github_callback_state.has_team_management_access( - self.context["request"].user, self.context["get_team"]() - ) + if requested_verification and not github_callback_state.has_team_management_access( + self.context["request"].user, self.context["get_team"]() ): raise PermissionDenied("Changing push identity verification requires project admin access.") diff --git a/posthog/api/test/test_integration.py b/posthog/api/test/test_integration.py index 97a39108b2cc..69362c6a9320 100644 --- a/posthog/api/test/test_integration.py +++ b/posthog/api/test/test_integration.py @@ -5002,11 +5002,16 @@ def test_member_cannot_delete_integration(self): assert response.status_code == status.HTTP_403_FORBIDDEN, response.content assert Integration.objects.filter(id=integration.id).exists() - def test_member_cannot_enable_push_identity_verification(self): + @parameterized.expand(["required", "disabled"]) + def test_member_cannot_set_push_identity_verification(self, mode): # Creating a *new* APNs integration sidesteps the overwrite check below, because a different # Apple team id makes a different integration_id. But the push endpoint resolves the strictest # verification mode across every integration sharing a bundle_id, so a member could otherwise # set `required` on a lookalike and block the real app's device registrations. + # + # `disabled` is rejected too: the overwrite check reads existing ids before the write, so a + # member racing the first setup would look like a create and could land `disabled` over a + # policy an admin had just written. Integration.objects.create( team=self.team, kind="apns", @@ -5024,7 +5029,7 @@ def test_member_cannot_enable_push_identity_verification(self): "key_id": "KEY2", "team_id_apple": "ATTACKER", "bundle_id": "com.example.app", - "push_identity_verification": "required", + "push_identity_verification": mode, }, }, format="json", diff --git a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts index bc44d19e9d7a..8fdee7b4a907 100644 --- a/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/APNSSetup/apnsSetupModalLogic.ts @@ -8,7 +8,11 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast' import { IntegrationType } from '~/types' -import { PushIdentityVerificationMode, resolvePushIdentityVerification } from '../PushIdentityVerificationField' +import { + PushIdentityVerificationMode, + pushIdentityVerificationPayload, + resolvePushIdentityVerification, +} from '../PushIdentityVerificationField' export interface APNSSetupModalLogicProps { integration?: IntegrationType | null @@ -254,7 +258,10 @@ export const apnsSetupModalLogic = kea([ team_id_apple: values.apnsIntegration.teamId, bundle_id: values.apnsIntegration.bundleId, environment: values.apnsIntegration.environment, - push_identity_verification: values.apnsIntegration.identityVerification, + ...pushIdentityVerificationPayload( + values.apnsIntegration.identityVerification, + props.integration + ), }, }) actions.loadIntegrations() diff --git a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts index bfe51cb9a3f4..5d43d943bd74 100644 --- a/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts +++ b/products/workflows/frontend/Channels/FCMSetup/fcmSetupModalLogic.ts @@ -8,7 +8,11 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast' import { IntegrationType } from '~/types' -import { PushIdentityVerificationMode, resolvePushIdentityVerification } from '../PushIdentityVerificationField' +import { + PushIdentityVerificationMode, + pushIdentityVerificationPayload, + resolvePushIdentityVerification, +} from '../PushIdentityVerificationField' export interface FCMSetupModalLogicProps { integration?: IntegrationType | null @@ -153,7 +157,10 @@ export const fcmSetupModalLogic = kea([ kind: 'firebase', config: { key_info: keyInfo, - push_identity_verification: values.fcmIntegration.identityVerification, + ...pushIdentityVerificationPayload( + values.fcmIntegration.identityVerification, + props.integration + ), }, }) actions.loadIntegrations() diff --git a/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx index 09332a181d9b..573246f2fd6e 100644 --- a/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx +++ b/products/workflows/frontend/Channels/PushIdentityVerificationField.tsx @@ -20,6 +20,18 @@ export function resolvePushIdentityVerification(integration?: IntegrationType | return PUSH_IDENTITY_VERIFICATION_MODES.includes(stored) ? stored : PUSH_IDENTITY_VERIFICATION_DEFAULT } +/** + * Config fragment to merge into the create payload. Sending the key at all counts as changing the + * policy and requires project admin, so an unchanged field is omitted — that lets a member connect or + * reconnect a channel, and leaves the stored policy for the backend to carry forward. + */ +export function pushIdentityVerificationPayload( + mode: PushIdentityVerificationMode, + integration?: IntegrationType | null +): { push_identity_verification?: PushIdentityVerificationMode } { + return mode === resolvePushIdentityVerification(integration) ? {} : { push_identity_verification: mode } +} + const MODE_HELP: Record = { disabled: 'Any client with your project API key can register a device for any user.', optional: