diff --git a/docs/internal/workflows-email-sending-tiers.md b/docs/internal/workflows-email-sending-tiers.md index 475506d892f2..fc9022631051 100644 --- a/docs/internal/workflows-email-sending-tiers.md +++ b/docs/internal/workflows-email-sending-tiers.md @@ -33,10 +33,12 @@ While the block exists, the endpoint returns 404 for the organization's projects Set `WORKFLOWS_SANDBOX_SENDER_DOMAIN`, `SES_SANDBOX_FROM_ADDRESS`, and `SES_SANDBOX_TENANT_NAME` in Django; any empty value leaves the endpoint unavailable and lets every team register any domain as before. The worker sends from the same `SES_SANDBOX_FROM_ADDRESS`, so set it once for Django and the worker. The address the UI shows is then the address the worker sends from. With the flag on, opening the Channels page creates the sender and shows it in its own row. -Workflow email steps offer it as a sender. It replaces any other selected sender and has no Reply-To or custom sender. Broadcasts and other email surfaces do not offer it. +Workflow email steps and template test sends offer it as a sender. In a step it replaces any other selected sender and has no Reply-To or custom sender. Broadcasts and other email surfaces do not offer it. Users cannot edit, verify, or remove the managed integration, make it the project default, use it for transactional email, or register their own integration on the shared domain. The managed integration is excluded from project SES tenant migration, identity cleanup, data suggestions, and the reputation page's domain breakdown. Configure the shared SES route, then enable `WORKFLOWS_SANDBOX_SENDER_ENABLED` on the worker and CDP API. Enable `workflows-sandbox-sender` last, and only for test projects until the worker reads the tenant pause and organization block in every region. +The sandbox sender is available only in workflow email steps and test sends. Broadcasts, destinations, sender rotation, custom From values, and Reply-To overrides are rejected. Unchanged stored sender settings remain editable. +Saves check the sandbox sender only while `workflows-sandbox-sender` is on for the project, so projects without the flag skip the integration lookup. ### Sending tiers diff --git a/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.ts b/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.ts index 1fb7da6542e0..124b6c0dc22b 100644 --- a/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.ts +++ b/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.ts @@ -1,6 +1,8 @@ import { router } from 'kea-router' import { expectLogic } from 'kea-test-utils' +import { LemonDialog } from '@posthog/lemon-ui' + import { FEATURE_FLAGS } from 'lib/constants' import { integrationsLogic } from 'lib/integrations/integrationsLogic' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' @@ -145,30 +147,67 @@ describe('emailTemplaterLogic', () => { expect(logic.values.sandboxSender).toBeNull() }) - it('picking the sandbox sender replaces the selection and clears the custom sender and Reply-To', async () => { - enableSandboxFlag() - const onChange = jest.fn() - await mountWithSenders({ - sandboxSenderAllowed: true, - liveChanges: true, - onChange, - value: { - ...DEFAULT_EMAIL_TEMPLATE, - from: { integrationId: 5, email: 'hello@acme.example.com', name: 'Acme' }, - replyTo: 'replies@acme.example.com', - }, - }) - - logic.actions.chooseSenders([5, 7]) - - expect(onChange).toHaveBeenLastCalledWith( - expect.objectContaining({ - from: { integrationId: 7, integrationIds: undefined, email: undefined, name: undefined }, - replyTo: '', + it.each([ + { + field: 'a custom address', + from: { integrationId: 5, email: 'hello@acme.example.com' }, + replyTo: '', + confirm: true, + }, + { field: 'a custom name', from: { integrationId: 5, name: 'Acme' }, replyTo: '', confirm: true }, + { field: 'a Reply-To', from: { integrationId: 5 }, replyTo: 'replies@acme.example.com', confirm: true }, + { field: 'a rotation', from: { integrationId: 5, integrationIds: [5, 6] }, replyTo: '', confirm: true }, + { field: 'one plain sender', from: { integrationId: 5 }, replyTo: '', confirm: false }, + ])( + 'picking the sandbox sender over $field replaces it after any confirmation', + async ({ from, replyTo, confirm }) => { + enableSandboxFlag() + useMocks({ + get: { + '/api/projects/:team_id/integrations/': { + results: [ + SANDBOX_SENDER, + OWN_SENDER, + { ...OWN_SENDER, id: 6, display_name: 'Second ' }, + ], + }, + }, }) - ) - expect(logic.values.isSandboxSenderSelected).toBe(true) - }) + const onChange = jest.fn() + const dialog = jest.spyOn(LemonDialog, 'open').mockImplementation(jest.fn()) + const props = { sandboxSenderAllowed: true, liveChanges: true, onChange } + await mountWithSenders({ ...props, value: { ...DEFAULT_EMAIL_TEMPLATE, from, replyTo } }) + onChange.mockClear() + + logic.actions.chooseSenders([...(from.integrationIds ?? [from.integrationId]), 7]) + + expect(dialog).toHaveBeenCalledTimes(confirm ? 1 : 0) + if (confirm) { + expect(onChange).not.toHaveBeenCalled() + if (from.integrationIds) { + expect(dialog.mock.calls[0][0].description).toContain( + 'This replaces Acme and Second ' + ) + } + emailTemplaterLogic( + makeProps({ + ...props, + value: { ...DEFAULT_EMAIL_TEMPLATE, from, replyTo, subject: 'Changed during confirmation' }, + }) + ) + ;(dialog.mock.calls[0][0].primaryButton?.onClick as () => void)() + } + expect(onChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + from: { integrationId: 7, integrationIds: undefined, email: undefined, name: undefined }, + replyTo: '', + subject: confirm ? 'Changed during confirmation' : DEFAULT_EMAIL_TEMPLATE.subject, + }) + ) + expect(logic.values.isSandboxSenderSelected).toBe(true) + dialog.mockRestore() + } + ) it('offers no Reply-To while the sandbox sender is selected', async () => { enableSandboxFlag() diff --git a/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsx b/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsx index 85559b8037ff..fd7119fb0523 100644 --- a/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsx +++ b/frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsx @@ -261,6 +261,22 @@ export interface EmailTemplaterLogicProps { sandboxSenderAllowed?: boolean } +function joinSenderNames(names: string[]): string { + return names.length > 1 ? `${names.slice(0, -1).join(', ')} and ${names[names.length - 1]}` : names[0] +} + +function sandboxSwitchDescription(replacedSenderNames: string[], hasCustomSenderDetails: boolean): string { + return [ + replacedSenderNames.length > 1 + ? `This replaces ${joinSenderNames(replacedSenderNames)} with the sandbox sender.` + : '', + hasCustomSenderDetails ? 'This removes your custom sender and Reply-To.' : '', + 'The sandbox sender sends from its own address.', + ] + .filter(Boolean) + .join(' ') +} + function autoRevealAdvancedFields( actions: { revealAdvancedField: (key: EmailMetaFieldKey) => void }, props: EmailTemplaterLogicProps @@ -295,6 +311,7 @@ export interface emailTemplaterLogicValues { emailTemplateTouched: boolean emailTemplateTouches: Record emailTemplateValidationErrors: DeepPartialMap + hasCustomSenderDetails: boolean hiddenAdvancedFields: EmailMetaField[] isEmailEditorReady: boolean isEmailTemplateSubmitting: boolean @@ -466,6 +483,7 @@ export interface emailTemplaterLogicMeta { emailTemplate: EmailTemplate ) => IntegrationType | null isSandboxSenderSelected: (sandboxSender: IntegrationType | null, emailTemplate: EmailTemplate) => boolean + hasCustomSenderDetails: (emailTemplate: EmailTemplate) => boolean supportedFields: (arg: EmailTemplaterType, isSandboxSenderSelected: boolean) => EmailMetaField[] visibleFields: ( supportedFields: EmailMetaField[], @@ -726,6 +744,13 @@ export const emailTemplaterLogic = kea([ (sandboxSender: IntegrationType | null, emailTemplate: EmailTemplate): boolean => !!sandboxSender && selectedSenderIds(senderFrom(emailTemplate)).includes(sandboxSender.id), ], + hasCustomSenderDetails: [ + (s) => [s.emailTemplate], + (emailTemplate: EmailTemplate): boolean => { + const from = senderFrom(emailTemplate) + return !!(from.email || from.name || emailTemplate?.replyTo) + }, + ], // The sandbox sender always replies from its own identity, so it offers no Reply-To. supportedFields: [ (s) => [(_, props: EmailTemplaterLogicProps) => props.type, s.isSandboxSenderSelected], @@ -754,8 +779,25 @@ export const emailTemplaterLogic = kea([ return } if (sandboxSenderId !== undefined && next.integrationId === sandboxSenderId) { - actions.setEmailTemplateValues({ from: next, replyTo: '' }) - actions.hideAdvancedField('replyTo') + const selectSandbox = (): void => { + actions.setEmailTemplateValues({ from: next, replyTo: '' }) + actions.hideAdvancedField('replyTo') + } + const replacedSenderIds = selectedSenderIds(senderFrom(values.emailTemplate)) + if (!values.hasCustomSenderDetails && replacedSenderIds.length < 2) { + selectSandbox() + return + } + const integrations = integrationsLogic.isMounted() ? (integrationsLogic.values.integrations ?? []) : [] + const replacedSenderNames = replacedSenderIds.map( + (id) => integrations.find((sender) => sender.id === id)?.display_name || `sender ${id}` + ) + LemonDialog.open({ + title: 'Switch to the sandbox sender?', + description: sandboxSwitchDescription(replacedSenderNames, values.hasCustomSenderDetails), + primaryButton: { children: 'Switch sender', onClick: selectSandbox }, + secondaryButton: { children: 'Cancel' }, + }) return } actions.setEmailTemplateValue('from', next) diff --git a/products/cdp/backend/api/hog_function.py b/products/cdp/backend/api/hog_function.py index 61adaa4f7162..4331ebc65ea1 100644 --- a/products/cdp/backend/api/hog_function.py +++ b/products/cdp/backend/api/hog_function.py @@ -197,6 +197,27 @@ def snapshot_hog_function_content(hog_function: HogFunction) -> dict: return snapshot +def _stored_email_values_by_key(instance: Optional[HogFunction]) -> dict[str, dict]: + """Stored email input values by input key, so a save that leaves one unchanged, such as disabling the function, keeps working. + + Only inputs the stored schema declares as email count. A dictionary input can hold the same shape, and + retyping it to email must not carry its value past the sandbox sender checks. + """ + if instance is None: + return {} + email_keys = { + str(schema.get("key")) + for schema in instance.inputs_schema or [] + if isinstance(schema, dict) and schema.get("type") in ("email", "native_email") + } + stored_inputs = instance.inputs if isinstance(instance.inputs, dict) else {} + return { + key: entry["value"] + for key, entry in stored_inputs.items() + if key in email_keys and isinstance(entry, dict) and isinstance(entry.get("value"), dict) + } + + def _named_warehouse_tables(entries: Any) -> list[Any]: """The warehouse tables a filters blob actually names, ignoring the picker's placeholder row. @@ -634,6 +655,7 @@ def to_internal_value(self, data): ): raise serializers.ValidationError({"filters": "Select the materialized view to trigger on."}) self.context["encrypted_inputs"] = instance.encrypted_inputs if instance else {} + self.context["existing_email_values_by_key"] = _stored_email_values_by_key(instance) template = None if data["template_id"]: diff --git a/products/cdp/backend/presentation/serializers.py b/products/cdp/backend/presentation/serializers.py index 78de717168e9..f1841ed412c5 100644 --- a/products/cdp/backend/presentation/serializers.py +++ b/products/cdp/backend/presentation/serializers.py @@ -8,4 +8,5 @@ InputsSchemaSerializer as InputsSchemaSerializer, InputsSerializer as InputsSerializer, require_viewer_access_to_added_actions as require_viewer_access_to_added_actions, + validate_sandbox_email_sender as validate_sandbox_email_sender, ) diff --git a/products/cdp/backend/validation.py b/products/cdp/backend/validation.py index 1dec2c491433..924df2f93252 100644 --- a/products/cdp/backend/validation.py +++ b/products/cdp/backend/validation.py @@ -5,6 +5,7 @@ from collections.abc import Iterable from typing import Any, Optional +from django.conf import settings from django.db import models from drf_spectacular.utils import extend_schema_field @@ -18,10 +19,11 @@ from posthog.hogql.parser import parse_program, parse_string_template from posthog.hogql.visitor import TraversingVisitor -from posthog.integrations import IntegrationManager -from posthog.models.integration import POSTHOG_CONNECT_KIND, WAREHOUSE_SOURCE_TYPE_CONFIG_KEY +from posthog.integrations import IntegrationManager, IntegrationRecord +from posthog.models.integration import POSTHOG_CONNECT_KIND, SANDBOX_EMAIL_PROVIDER, WAREHOUSE_SOURCE_TYPE_CONFIG_KEY from posthog.models.team.team import Team from posthog.models.user import User +from posthog.permissions import posthog_feature_flag_enabled from products.actions.backend.models.action import Action from products.cdp.backend.filters import ( @@ -93,6 +95,118 @@ def _sender_integration_ids(from_value: dict) -> set[int]: } +def _delivery_sender_ids(from_value: dict) -> set[int]: + rotation = from_value.get("integrationIds") + # Mirrors selectEmailSenderIntegrationId in the worker: a non-empty rotation replaces integrationId. + configured = rotation if isinstance(rotation, list) and rotation else [from_value.get("integrationId")] + return { + int(integration_id) + for integration_id in configured + if isinstance(integration_id, (int, float)) + and not isinstance(integration_id, bool) + and (not isinstance(integration_id, float) or integration_id.is_integer()) + } + + +def _sandbox_sender_configured() -> bool: + # Mirrors sandbox_sender_configured() in the workflows product, which imports this module. + return bool( + settings.WORKFLOWS_SANDBOX_SENDER_DOMAIN + and settings.SES_SANDBOX_FROM_ADDRESS + and settings.SES_SANDBOX_TENANT_NAME + ) + + +def _sandbox_email_integrations( + integration_ids: set[int], context: dict[str, Any] +) -> dict[int, IntegrationRecord | None]: + cache: dict[int, IntegrationRecord | None] = context.setdefault("sandbox_email_integration_cache", {}) + missing_ids = integration_ids - cache.keys() + if missing_ids: + integrations = IntegrationManager.email.credentials.get_many(context["get_team"]().id, missing_ids) + cache.update({integration_id: integrations.get(integration_id) for integration_id in missing_ids}) + return cache + + +def _sandbox_sender_enabled(team: Team, context: dict[str, Any]) -> bool: + flag_cache: dict[int, bool] = context.setdefault("sandbox_sender_enabled_cache", {}) + if team.id not in flag_cache: + try: + flag_cache[team.id] = posthog_feature_flag_enabled( + "workflows-sandbox-sender", str(team.uuid), organization_id=team.organization_id, team_id=team.id + ) + except Exception: + flag_cache[team.id] = False + return flag_cache[team.id] + + +def _stored_email_values(context: dict[str, Any]) -> list[object]: + # A workflow step passes its stored email values. A hog function passes them by input key, + # so only the value stored under the input being validated counts. + stored = list(context.get("existing_email_values") or []) + by_key = context.get("existing_email_values_by_key") + schema = context.get("schema") + if isinstance(by_key, dict) and isinstance(schema, dict) and str(schema.get("key")) in by_key: + stored.append(by_key[str(schema.get("key"))]) + return stored + + +def validate_sandbox_email_sender(email_value: object, context: dict[str, Any]) -> None: + # Every check returns before any query or flag evaluation while the sandbox settings are empty, + # and before any integration lookup while the project lacks the flag. + if not _sandbox_sender_configured(): + return + if not isinstance(email_value, dict) or not isinstance(email_value.get("from"), dict): + return + integration_ids = _delivery_sender_ids(email_value["from"]) + get_team = context.get("get_team") + if not integration_ids or get_team is None or not _sandbox_sender_enabled(get_team(), context): + return + # Bounds the lookup below. InputsItemSerializer applies the same limit to every save. + if len(integration_ids) > MAX_WORKFLOW_EMAIL_SENDERS: + raise serializers.ValidationError({"input": f"At most {MAX_WORKFLOW_EMAIL_SENDERS} email senders are allowed."}) + integrations = _sandbox_email_integrations(integration_ids, context) + if not any( + (integration := integrations[integration_id]) is not None + and (integration.config or {}).get("provider") == SANDBOX_EMAIL_PROVIDER + for integration_id in integration_ids + ): + return + if context.get("workflow_origin_product") == "broadcasts": + raise serializers.ValidationError( + {"input": "The sandbox sender cannot be used in broadcasts. Select a sender on your own verified domain."} + ) + for stored in _stored_email_values(context): + if ( + isinstance(stored, dict) + and email_value["from"] == stored.get("from") + and (email_value.get("replyTo") or "") == (stored.get("replyTo") or "") + ): + return + is_test_send = getattr(context.get("view"), "action", None) == "invocations" + if context.get("workflow_action_type") != "function_email" and not is_test_send: + raise serializers.ValidationError( + { + "input": "The sandbox sender can only be used in workflow email steps and test sends. " + "Select a sender on your own verified domain for destinations." + } + ) + if len(integration_ids) != 1: + raise serializers.ValidationError( + { + "input": "The sandbox sender must be the only sender. Remove the other senders " + "or select a sender on your own verified domain." + } + ) + if email_value["from"].get("email") or email_value["from"].get("name") or email_value.get("replyTo"): + raise serializers.ValidationError( + { + "input": "The sandbox sender uses a fixed From address and name and does not support Reply-To. " + "Remove these overrides or select a sender on your own verified domain." + } + ) + + def _validate_not_posthog_connection(integration_ids: list[int], context: dict) -> None: # A PostHog connection acts as the user who created it, so only that user may use it. A function # runs for the whole team and the runtime never resolves one, so reject it where the author sees why. @@ -885,6 +999,7 @@ def validate(self, attrs): elif item_type == "email" or item_type == "native_email": if not isinstance(value, dict): raise serializers.ValidationError({"input": f"Value must be an email object."}) + validate_sandbox_email_sender(value, self.context) # Report every missing key in one error: these objects are typically authored # programmatically, and a one-at-a-time raise forces a round trip per missing key. missing = [f"'{key_}'" for key_ in ("from", "to", "subject") if not value.get(key_)] diff --git a/products/workflows/backend/presentation/views/hog_flow.py b/products/workflows/backend/presentation/views/hog_flow.py index 5c9af73266e6..753305832de2 100644 --- a/products/workflows/backend/presentation/views/hog_flow.py +++ b/products/workflows/backend/presentation/views/hog_flow.py @@ -129,6 +129,7 @@ InputsSchemaSerializer, InputsSerializer, require_viewer_access_to_added_actions, + validate_sandbox_email_sender, ) from products.cohorts.backend.facade.api import get_cohort_ref from products.messaging.backend.facade.categories import MessageCategoryMissing, get_category @@ -1152,14 +1153,14 @@ def _has_exact_string_filter(filters: dict, key: str) -> bool: return False -def _existing_email_from_by_action(instance: WorkflowEditState) -> dict[str, list[dict]]: - """Stored email sender overrides keyed by action id, live and draft variants both kept. +def _existing_email_values_by_action(instance: WorkflowEditState) -> dict[str, list[tuple[str | None, dict]]]: + """Stored email inputs keyed by action id, live and draft variants both kept. Which variant a save should be compared against depends on how the request routes (a builder save targets the draft, a raw API write targets live), so validation grandfathers a value matching either - both are values already stored on the workflow. """ - result: dict[str, list[dict]] = {} + result: dict[str, list[tuple[str | None, dict]]] = {} draft = instance.draft if isinstance(instance.draft, dict) else {} for actions in (instance.actions, draft.get("actions")): for stored_action in actions or []: @@ -1168,8 +1169,8 @@ def _existing_email_from_by_action(instance: WorkflowEditState) -> dict[str, lis email_input = ((stored_action.get("config") or {}).get("inputs") or {}).get("email") value = email_input.get("value") if isinstance(email_input, dict) else None from_value = value.get("from") if isinstance(value, dict) else None - if isinstance(from_value, dict): - result.setdefault(stored_action["id"], []).append(from_value) + if isinstance(value, dict) and isinstance(from_value, dict): + result.setdefault(stored_action["id"], []).append((stored_action.get("type"), value)) return result @@ -2210,6 +2211,23 @@ def _subscribes_to(event_id: str) -> bool: inputs = data.get("config", {}).get("inputs", {}) if template_id == _FIXED_TEMPLATE_IDS["function_push"]: _fill_default_push_title(inputs, input_schema) + existing_emails = (self.context.get("existing_action_emails") or {}).get(data.get("id")) or [] + input_context = { + **self.context, + "workflow_action_type": data.get("type"), + "existing_email_values": [ + email + for action_type, email in existing_emails + if action_type == "function_email" and data.get("type") == "function_email" + ], + "sandbox_email_integration_cache": self.context.setdefault("_sandbox_email_integration_cache", {}), + "sandbox_sender_enabled_cache": self.context.setdefault("_sandbox_sender_enabled_cache", {}), + } + for schema in input_schema or []: + if schema.get("type") in {"email", "native_email"} and isinstance(inputs, dict): + email_input = inputs.get(schema["key"]) + if isinstance(email_input, dict): + validate_sandbox_email_sender(email_input.get("value"), input_context) function_config_serializer = HogFlowConfigFunctionInputsSerializer( data={ @@ -2217,6 +2235,7 @@ def _subscribes_to(event_id: str) -> bool: "inputs": inputs, }, context={ + **input_context, "function_type": template.type, "is_dwh_source": self.context.get("is_dwh_source", False), # The existing (decrypted) secret inputs for this action, so a resent @@ -2227,9 +2246,7 @@ def _subscribes_to(event_id: str) -> bool: # sender address is checked against the integration's verified domain at # save time, while an unchanged stored value is grandfathered. "get_team": self.context.get("get_team"), - "existing_email_from": (self.context.get("existing_action_email_from") or {}).get( - data.get("id") - ), + "existing_email_from": [email["from"] for _, email in existing_emails], # Request-scoped: a drip sequence's steps share senders, and the actions # list validates one action at a time (mirrors _message_template_cache). "email_integration_domain_cache": self.context.setdefault( @@ -3521,6 +3538,7 @@ def to_internal_value(self, data): # When used as a nested field (the `configuration` override on test invocations) DRF never # binds `self.instance`, so fall back to the flow passed in via context so recovery still works. instance = cast(Optional[WorkflowEditState], self.instance) or self.context.get("instance") + self.context["workflow_origin_product"] = instance.origin_product if instance else data.get("origin_product") # Who a "Create AI task" step runs as: the existing creator for an update, or the # requesting user for a brand-new flow (matches the `created_by` a create() actually @@ -3644,7 +3662,7 @@ def to_internal_value(self, data): # Stored sender overrides, keyed by action id, so child action validation only holds # newly written custom sender addresses to the verified-domain rule. Draft wins over # live for the same reason as secrets: it is the value the client last saw. - self.context["existing_action_email_from"] = _existing_email_from_by_action(instance) + self.context["existing_action_emails"] = _existing_email_values_by_action(instance) # Live "Run scout" notes, so only a new or changed note needs skill editor access. self.context["live_run_scout_notes"] = _live_run_scout_notes_by_action(instance) diff --git a/products/workflows/backend/services/broadcast_conversion.py b/products/workflows/backend/services/broadcast_conversion.py index 63d72ca384eb..ce83851eca38 100644 --- a/products/workflows/backend/services/broadcast_conversion.py +++ b/products/workflows/backend/services/broadcast_conversion.py @@ -6,12 +6,16 @@ from django.db import models, transaction from django.utils import timezone +from posthog.integrations import IntegrationManager +from posthog.models.integration import SANDBOX_EMAIL_PROVIDER + from products.access_control.backend.facade.user_access_control import UserAccessControl from products.workflows.backend.facade.contracts import WorkflowActor, WorkflowSummary from products.workflows.backend.models.hog_flow.hog_flow import HogFlow from products.workflows.backend.services.hog_flow_activity import log_workflow_activity from products.workflows.backend.services.hog_flow_reads import BROADCAST_TRIGGER_TYPE, annotate_broadcast_shape from products.workflows.backend.services.hog_flow_writes import field_values +from products.workflows.backend.services.sandbox_sender import sandbox_sender_enabled_for_team BROADCAST_CONVERSION_CANDIDATE_LIMIT: Final = 100 @@ -37,9 +41,51 @@ def is_convertible_to_broadcast(flow: HogFlow) -> bool: config = trigger.get("config") or {} if config.get("type") != BROADCAST_TRIGGER_TYPE or (config.get("filters") or {}).get("audience_type") == "accounts": return False + if _sender_ids(_email_sender(email)) & _sandbox_sender_ids(flow.team_id): + return False return _joins_in_order(flow, trigger.get("id"), email.get("id"), exit_step.get("id")) +def _email_sender(email_step: dict) -> object: + """The step's stored `from` value. Drafts save leniently, so any level can hold another type.""" + value: object = email_step + for key in ("config", "inputs", "email", "value", "from"): + value = value.get(key) if isinstance(value, dict) else None + return value + + +def _sender_ids(sender: object) -> set[str]: + """The integrations a `from` value sends from, as the worker picks them: a non-empty rotation replaces `integrationId`. + + Ids are compared as strings, since stored ids can be ints, floats or strings. + """ + if not isinstance(sender, dict): + return set() + rotation = sender.get("integrationIds") + ids = rotation if isinstance(rotation, list) and rotation else [sender.get("integrationId")] + return {_integration_id_key(integration_id) for integration_id in ids if integration_id is not None} + + +def _integration_id_key(integration_id: object) -> str: + if isinstance(integration_id, float) and integration_id.is_integer(): + return str(int(integration_id)) + return str(integration_id) + + +def _sandbox_sender_ids(team_id: int) -> set[str]: + """Broadcasts can't send through the sandbox sender, so a workflow that uses it stays a workflow.""" + try: + if not sandbox_sender_enabled_for_team(team_id): + return set() + except Exception: + # Without a flag answer, look the senders up, so a sandbox workflow is never offered. + pass + return { + str(sender.id) + for sender in IntegrationManager.email.credentials.list(team_id, where={"provider": SANDBOX_EMAIL_PROVIDER}) + } + + def _joins_in_order(flow: HogFlow, trigger_id: object, email_id: object, exit_id: object) -> bool: """Whether the only edges run trigger to email and email to exit.""" paths = {(edge.get("from"), edge.get("to")) for edge in flow.edges or [] if isinstance(edge, dict)} @@ -60,6 +106,15 @@ def _step_count(step_type: str) -> models.Func: return models.Func(steps, function="jsonb_array_length", output_field=models.IntegerField()) +def _email_sender_value() -> models.Func: + return models.Func( + models.F("actions"), + _json_path('$[*] ? (@.type == "function_email").config.inputs.email.value.from'), + function="jsonb_path_query_first", + output_field=models.JSONField(), + ) + + def _step_id(step_type: str) -> models.Func: return models.Func( models.F("actions"), @@ -111,6 +166,9 @@ def list_broadcast_conversion_candidates( .only("id", "team_id", "name", "status", "edges", "created_by") .order_by("-updated_at", "id") ) + sandbox_sender_ids = _sandbox_sender_ids(team_id) + if sandbox_sender_ids: + queryset = queryset.annotate(_email_sender=_email_sender_value()) found: list[HogFlow] = [] offset = 0 while len(found) < limit: @@ -118,7 +176,12 @@ def list_broadcast_conversion_candidates( if not page: break offset += len(page) - page = [flow for flow in page if _joins_in_order(flow, flow._trigger_id, flow._email_id, flow._exit_id)] + page = [ + flow + for flow in page + if _joins_in_order(flow, flow._trigger_id, flow._email_id, flow._exit_id) + and not (sandbox_sender_ids and _sender_ids(flow._email_sender) & sandbox_sender_ids) + ] if user_access_control is not None: # Converting needs editor access, so a workflow the caller can only view is not offered. user_access_control.preload_object_access_controls(cast("list[models.Model]", page)) diff --git a/products/workflows/backend/services/sandbox_sender.py b/products/workflows/backend/services/sandbox_sender.py index 45aa7861ee9d..1b1dde5a5fdd 100644 --- a/products/workflows/backend/services/sandbox_sender.py +++ b/products/workflows/backend/services/sandbox_sender.py @@ -54,6 +54,14 @@ def _sandbox_sender_enabled(team: Team) -> bool: ) +def sandbox_sender_enabled_for_team(team_id: int) -> bool: + """Whether the project has the sandbox sender flag. Runs no query while the deployment has no sandbox sender.""" + if not sandbox_sender_configured(): + return False + team = Team.objects.only("id", "uuid", "organization_id").get(id=team_id) + return _sandbox_sender_enabled(team) + + def _organization_blocked(team: Team) -> bool: return SandboxSenderOrganizationBlock.objects.filter(organization_id=team.organization_id).exists() diff --git a/products/workflows/backend/tests/api/test_broadcast_conversion.py b/products/workflows/backend/tests/api/test_broadcast_conversion.py index 54498d98385f..60f1fad6272d 100644 --- a/products/workflows/backend/tests/api/test_broadcast_conversion.py +++ b/products/workflows/backend/tests/api/test_broadcast_conversion.py @@ -1,12 +1,18 @@ +from collections.abc import Callable from typing import Any from posthog.test.base import APIBaseTest from unittest.mock import patch +from django.db import connection +from django.test import override_settings +from django.test.utils import CaptureQueriesContext + from parameterized import parameterized from rest_framework import status from posthog.models.activity_logging.activity_log import ActivityLog +from posthog.models.integration import SANDBOX_EMAIL_INTEGRATION_ID, SANDBOX_EMAIL_PROVIDER, Integration from products.workflows.backend.models import HogFlow @@ -14,6 +20,11 @@ EMAIL = {"id": "email_node", "name": "Email", "type": "function_email", "config": {"inputs": {}}} EXIT = {"id": "exit_node", "name": "Exit", "type": "exit", "config": {}} DELAY = {"id": "delay_node", "name": "Delay", "type": "delay", "config": {"delay_duration": "1h"}} +SANDBOX_SETTINGS = { + "WORKFLOWS_SANDBOX_SENDER_DOMAIN": "sandbox.example.com", + "SES_SANDBOX_FROM_ADDRESS": "hello@sandbox.example.com", + "SES_SANDBOX_TENANT_NAME": "posthog-sandbox", +} EDGES = [ {"from": "trigger_node", "to": "email_node", "type": "continue"}, {"from": "email_node", "to": "exit_node", "type": "continue"}, @@ -68,6 +79,11 @@ def _convert(self, flow: HogFlow) -> Any: ), ("extra step", {"actions": [TRIGGER, EMAIL, DELAY, EXIT]}, False), ("edge that skips the email", {"edges": [*EDGES, {"from": "trigger_node", "to": "exit_node"}]}, False), + ( + "email step without a sender object", + {"actions": [TRIGGER, {**EMAIL, "config": {"inputs": {"email": {"value": "draft"}}}}, EXIT]}, + True, + ), ] ) def test_only_workflows_the_broadcast_editor_can_own_are_offered_and_converted( @@ -91,6 +107,74 @@ def test_only_workflows_the_broadcast_editor_can_own_are_offered_and_converted( assert response.status_code == status.HTTP_400_BAD_REQUEST, response.json() assert flow.origin_product == overrides.get("origin_product") + @parameterized.expand( + [ + ("sandbox sender", {"integrationId": "sandbox"}, int, False), + ("sandbox sender stored as a whole-number float", {"integrationId": "sandbox"}, float, False), + ("sandbox sender stored as a string", {"integrationId": "sandbox"}, str, False), + ("sandbox sender in a rotation", {"integrationIds": ["own", "sandbox"]}, int, False), + ("own sender", {"integrationId": "own"}, int, True), + ( + "own rotation with a stale sandbox fallback", + {"integrationId": "sandbox", "integrationIds": ["own"]}, + int, + True, + ), + ] + ) + @override_settings(**SANDBOX_SETTINGS) + @patch("posthoganalytics.feature_enabled", return_value=True) + def test_a_workflow_sending_through_the_sandbox_sender_does_not_become_a_broadcast( + self, + _name: str, + sender: dict[str, Any], + stored_as: Callable[[int], Any], + convertible: bool, + _flag: Any, + ) -> None: + integration_ids = { + "sandbox": Integration.objects.create( + team=self.team, + kind="email", + integration_id=SANDBOX_EMAIL_INTEGRATION_ID, + config={"provider": SANDBOX_EMAIL_PROVIDER, "verified": True}, + ).id, + "own": Integration.objects.create( + team=self.team, + kind="email", + integration_id="sender@example.com", + config={"provider": "ses", "domain": "example.com", "verified": True}, + ).id, + } + sender_from = { + key: [stored_as(integration_ids[name]) for name in value] + if isinstance(value, list) + else stored_as(integration_ids[value]) + for key, value in sender.items() + } + email = {**EMAIL, "config": {"inputs": {"email": {"value": {"from": sender_from}}}}} + flow = self._flow(actions=[TRIGGER, email, EXIT]) + + assert (str(flow.id) in self._candidate_ids()) is convertible + response = self._convert(flow) + assert (response.status_code == status.HTTP_200_OK) is convertible, response.json() + + @parameterized.expand([("no sandbox settings", {}, True), ("flag off", SANDBOX_SETTINGS, False)]) + def test_conversion_skips_the_sandbox_lookup_while_the_sandbox_sender_is_off( + self, _name: str, sandbox_settings: dict[str, str], flag: bool + ) -> None: + flow = self._flow() + + with ( + override_settings(**sandbox_settings), + patch("posthoganalytics.feature_enabled", return_value=flag), + CaptureQueriesContext(connection) as queries, + ): + assert str(flow.id) in self._candidate_ids() + assert self._convert(flow).status_code == status.HTTP_200_OK + + assert not [query for query in queries.captured_queries if '"posthog_integration"' in query["sql"]] + def test_candidates_stop_at_the_cap_and_say_more_exist(self) -> None: newest = self._flow(name="Newest") self._flow(name="Broken", edges=[{"from": "trigger_node", "to": "exit_node"}]) diff --git a/products/workflows/backend/tests/api/test_sandbox_sender_validation.py b/products/workflows/backend/tests/api/test_sandbox_sender_validation.py new file mode 100644 index 000000000000..a87bfd403a50 --- /dev/null +++ b/products/workflows/backend/tests/api/test_sandbox_sender_validation.py @@ -0,0 +1,420 @@ +from copy import deepcopy +from types import SimpleNamespace +from typing import Any + +from posthog.test.base import APIBaseTest +from unittest.mock import patch + +from django.db import connection +from django.test import override_settings +from django.test.utils import CaptureQueriesContext + +from parameterized import parameterized + +from posthog.models.integration import Integration + +from products.cdp.backend.api.hog_function import HogFunctionInvocationSerializer +from products.cdp.backend.models.hog_function_template import HogFunctionTemplate +from products.cdp.backend.models.hog_functions.hog_function import HogFunction +from products.workflows.backend.facade.api import ensure_sandbox_email_sender +from products.workflows.backend.facade.workflows import get_workflow_edit_state +from products.workflows.backend.models.hog_flow import HogFlow +from products.workflows.backend.presentation.views.hog_flow import HogFlowInvocationSerializer + + +@override_settings( + WORKFLOWS_SANDBOX_SENDER_DOMAIN="sandbox.example.com", + SES_SANDBOX_FROM_ADDRESS="hello@sandbox.example.com", + SES_SANDBOX_TENANT_NAME="posthog-sandbox", +) +class TestSandboxSenderValidation(APIBaseTest): + def setUp(self) -> None: + super().setUp() + flag = patch("posthoganalytics.feature_enabled", return_value=True) + self.flag_enabled = flag.start() + self.addCleanup(flag.stop) + capture = patch("posthoganalytics.capture") + capture.start() + self.addCleanup(capture.stop) + self.sender = Integration.objects.get( + team_id=self.team.id, id=ensure_sandbox_email_sender(self.team.id).integration_id + ) + self.email_schema = [{"key": "email", "type": "native_email", "required": True, "templating": "liquid"}] + HogFunctionTemplate.objects.create( + template_id="template-email", + name="Email", + type="destination", + status="hidden", + code="return sendEmail(inputs.email);", + inputs_schema=self.email_schema, + ) + + def _workflow(self) -> dict[str, Any]: + return { + "name": "Sandbox workflow", + "actions": [ + { + "id": "trigger", + "type": "trigger", + "name": "Trigger", + "config": { + "type": "event", + "filters": {"events": [{"id": "$pageview", "type": "events", "order": 0}]}, + }, + }, + { + "id": "email", + "type": "function_email", + "name": "Send email", + "config": {"inputs": {"email": {"value": self._email()}}}, + }, + {"id": "exit", "type": "exit", "name": "Exit", "config": {}}, + ], + "edges": [ + {"from": "trigger", "to": "email", "type": "continue"}, + {"from": "email", "to": "exit", "type": "continue"}, + ], + } + + def _email(self) -> dict[str, Any]: + return { + "from": {"integrationId": self.sender.id}, + "to": "member@example.com", + "subject": "Hello", + "html": "

Hello

", + } + + def _function(self) -> dict[str, Any]: + return { + "name": "Sandbox destination", + "type": "destination", + "hog": "return 1;", + "inputs_schema": self.email_schema, + "inputs": {"email": {"value": self._email()}}, + } + + @parameterized.expand([("integer",), ("float",), ("malformed_rotation",)]) + def test_broadcast_cannot_select_the_sandbox_sender(self, sender_shape: str) -> None: + workflow = self._workflow() + workflow["origin_product"] = "broadcasts" + sender = workflow["actions"][1]["config"]["inputs"]["email"]["value"]["from"] + if sender_shape == "float": + sender["integrationId"] = float(self.sender.id) + elif sender_shape == "malformed_rotation": + sender["integrationIds"] = self.sender.id + + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + + assert response.status_code == 400, response.json() + assert "sandbox sender" in response.json()["detail"].lower() + assert "broadcast" in response.json()["detail"].lower() + + def test_destination_cannot_select_the_sandbox_sender(self) -> None: + response = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + self._function(), + ) + + assert response.status_code == 400, response.json() + assert "sandbox sender" in response.json()["detail"].lower() + assert "workflow email steps and test sends" in response.json()["detail"].lower() + + @parameterized.expand([("disabled", {"enabled": False}), ("deleted", {"deleted": True})]) + def test_stored_destination_with_the_sandbox_sender_can_still_be(self, _name: str, change: dict) -> None: + function = HogFunction.objects.create( + team=self.team, + name="Stored destination", + type="destination", + hog="return 1;", + inputs_schema=self.email_schema, + inputs={"email": {"value": self._email()}}, + enabled=True, + ) + + response = self.client.patch(f"/api/projects/{self.team.id}/hog_functions/{function.id}/", change) + + assert response.status_code == 200, response.json() + + @parameterized.expand( + [ + ("dictionary_input_retyped_as_email", "dictionary", "email"), + ("email_value_moved_to_another_input", "native_email", "backup"), + ] + ) + def test_stored_sandbox_value_is_not_grandfathered_into_a_different_email_input( + self, _name: str, stored_type: str, new_key: str + ) -> None: + function = HogFunction.objects.create( + team=self.team, + name="Stored destination", + type="destination", + hog="return 1;", + inputs_schema=[{"key": "email", "type": stored_type}], + inputs={"email": {"value": self._email()}}, + ) + + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_functions/{function.id}/", + { + "inputs_schema": [{**self.email_schema[0], "key": new_key}], + "inputs": {new_key: {"value": self._email()}}, + }, + ) + + assert response.status_code == 400, response.json() + assert "workflow email steps and test sends" in response.json()["detail"].lower() + + def test_destination_with_an_own_rotation_ignores_a_stale_sandbox_fallback(self) -> None: + own_sender = Integration.objects.create( + team=self.team, + kind="email", + config={"provider": "ses", "domain": "example.com", "email": "sender@example.com", "verified": True}, + ) + function = self._function() + function["inputs"]["email"]["value"]["from"] = { + "integrationId": self.sender.id, + "integrationIds": [own_sender.id], + } + + response = self.client.post(f"/api/projects/{self.team.id}/hog_functions/", function) + + assert response.status_code == 201, response.json() + + @parameterized.expand([("created",), ("changed_from_an_email_step",)]) + def test_generic_workflow_step_cannot_select_the_sandbox_sender_with_a_legacy_email_schema( + self, origin: str + ) -> None: + template = HogFunctionTemplate.objects.get(template_id="template-email") + template.inputs_schema = [{**self.email_schema[0], "type": "email"}] + template.save() + workflow = self._workflow() + created_id = None + if origin == "changed_from_an_email_step": + created = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + assert created.status_code == 201, created.json() + created_id = created.json()["id"] + workflow["actions"][1]["type"] = "function" + workflow["actions"][1]["config"]["template_id"] = "template-email" + + if created_id is None: + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + else: + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_flows/{created_id}", {"actions": workflow["actions"]} + ) + + assert response.status_code == 400, response.json() + assert "workflow email steps and test sends" in response.json()["detail"].lower() + + def test_workflow_can_select_the_sandbox_sender_alone(self) -> None: + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", self._workflow()) + + assert response.status_code == 201, response.json() + assert response.json()["actions"][1]["config"]["inputs"]["email"]["value"]["from"] == { + "integrationId": self.sender.id + } + + @parameterized.expand([("exit",), ("function",)]) + def test_inert_email_data_is_not_an_existing_sandbox_sender(self, action_type: str) -> None: + workflow = self._workflow() + action = workflow["actions"][1] + action["type"] = action_type + action["config"]["template_id"] = "missing-template" + email = action["config"]["inputs"]["email"]["value"] + email["from"]["name"] = "New sender" + email["replyTo"] = "reply@example.com" + created = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + assert created.status_code == 201, created.json() + action["type"] = "function_email" + action["config"].pop("template_id") + + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_flows/{created.json()['id']}", + {"actions": workflow["actions"]}, + ) + + assert response.status_code == 400, response.json() + assert "fixed From address and name" in response.json()["detail"] + + def test_sandbox_sender_cannot_rotate_with_own_senders(self) -> None: + own_sender = Integration.objects.create( + team=self.team, + kind="email", + config={"provider": "ses", "domain": "example.com", "email": "sender@example.com", "verified": True}, + ) + workflow = self._workflow() + workflow["actions"][1]["config"]["inputs"]["email"]["value"]["from"] = { + "integrationIds": [self.sender.id, own_sender.id] + } + + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + + assert response.status_code == 400, response.json() + assert "sandbox sender must be the only sender" in response.json()["detail"].lower() + + def test_oversized_sender_rotation_is_rejected_before_looking_up_integrations(self) -> None: + workflow = self._workflow() + workflow["actions"][1]["config"]["inputs"]["email"]["value"]["from"]["integrationIds"] = list(range(100)) + + with CaptureQueriesContext(connection) as queries: + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + + assert response.status_code == 400, response.json() + assert "At most 10 email senders" in response.json()["detail"] + assert not [query for query in queries if 'FROM "posthog_integration"' in query["sql"]] + + @parameterized.expand( + [ + ("custom From address", "email", "custom@sandbox.example.com"), + ("templated From address", "email", "{{ person.properties.email }}"), + ("custom From name", "name", "Custom sender"), + ("Reply-To", "replyTo", "reply@example.com"), + ] + ) + def test_sandbox_sender_cannot_use_sender_overrides(self, _name: str, field: str, value: str) -> None: + workflow = self._workflow() + email = workflow["actions"][1]["config"]["inputs"]["email"]["value"] + if field == "replyTo": + email[field] = value + else: + email["from"][field] = value + + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow) + + assert response.status_code == 400, response.json() + assert "fixed From address and name" in response.json()["detail"] + assert "Reply-To" in response.json()["detail"] + + @parameterized.expand( + [ + ("hog_flows", "flag off"), + ("hog_functions/", "flag off"), + ("hog_flows", "no sandbox settings"), + ("hog_functions/", "no sandbox settings"), + ] + ) + def test_saves_skip_the_sandbox_checks_while_the_sandbox_sender_is_off(self, endpoint: str, off: str) -> None: + self.flag_enabled.reset_mock() + self.flag_enabled.return_value = off != "flag off" + payload = self._workflow() if endpoint == "hog_flows" else self._function() + + with ( + override_settings(SES_SANDBOX_TENANT_NAME="" if off == "no sandbox settings" else "posthog-sandbox"), + CaptureQueriesContext(connection) as queries, + ): + response = self.client.post(f"/api/projects/{self.team.id}/{endpoint}", payload) + + assert response.status_code == 201, response.json() + assert not [query for query in queries.captured_queries if '"posthog_integration"' in query["sql"]] + sandbox_flag_checks = [ + call for call in self.flag_enabled.call_args_list if call.args[:1] == ("workflows-sandbox-sender",) + ] + assert bool(sandbox_flag_checks) is (off == "flag off") + + def test_test_send_configuration_can_select_the_sandbox_sender(self) -> None: + serializer = HogFunctionInvocationSerializer( + data={"configuration": self._function()}, + context={"get_team": lambda: self.team, "view": SimpleNamespace(action="invocations")}, + ) + + assert serializer.is_valid(), serializer.errors + assert serializer.validated_data["configuration"]["inputs"]["email"]["value"]["from"] == { + "integrationId": self.sender.id + } + + @parameterized.expand([("missing",), ("cleared",)]) + def test_broadcast_test_cannot_clear_its_origin(self, origin: str) -> None: + workflow = self._workflow() + stored = HogFlow.objects.create( + team=self.team, + name="Broadcast", + origin_product="broadcasts", + actions=workflow["actions"], + edges=workflow["edges"], + ) + if origin == "cleared": + workflow["origin_product"] = None + serializer = HogFlowInvocationSerializer( + data={"configuration": workflow}, + context={ + "get_team": lambda: self.team, + "view": SimpleNamespace(action="invocations"), + "instance": get_workflow_edit_state( + team_id=self.team.id, workflow_id=stored.id, user_access_control=None, required_level=None + ), + }, + ) + + assert not serializer.is_valid(), serializer.validated_data + assert "broadcast" in str(serializer.errors).lower() + + @parameterized.expand([("live",), ("draft",)]) + def test_unchanged_legacy_sender_survives_unrelated_edits(self, variant: str) -> None: + live = self._workflow() + live_email = live["actions"][1]["config"]["inputs"]["email"]["value"] + live_email["from"].update({"email": "legacy@example.com", "name": "Legacy sender"}) + live_email["replyTo"] = "legacy-reply@example.com" + draft = deepcopy(live) + draft_email = draft["actions"][1]["config"]["inputs"]["email"]["value"] + draft_email["from"]["name"] = "Draft legacy sender" + draft_email["replyTo"] = "draft-reply@example.com" + stored = HogFlow.objects.create( + team=self.team, + name="Legacy workflow", + actions=live["actions"], + edges=live["edges"], + status=HogFlow.State.ACTIVE, + draft=draft, + ) + actions = deepcopy(live["actions"] if variant == "live" else draft["actions"]) + actions[1]["config"]["inputs"]["email"]["value"]["subject"] = "Updated legacy subject" + + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_flows/{stored.id}", + {"name": "Renamed legacy workflow", "actions": actions}, + ) + + assert response.status_code == 200, response.json() + + @parameterized.expand([("email",), ("name",), ("replyTo",)]) + def test_legacy_sender_overrides_cannot_be_changed(self, field: str) -> None: + workflow = self._workflow() + email = workflow["actions"][1]["config"]["inputs"]["email"]["value"] + email["from"].update({"email": "legacy@example.com", "name": "Legacy sender"}) + email["replyTo"] = "legacy-reply@example.com" + stored = HogFlow.objects.create( + team=self.team, + name="Legacy workflow", + actions=workflow["actions"], + edges=workflow["edges"], + status=HogFlow.State.ACTIVE, + ) + if field == "replyTo": + email[field] = "changed@example.com" + else: + email["from"][field] = "changed@example.com" if field == "email" else "Changed sender" + + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_flows/{stored.id}", + {"actions": workflow["actions"]}, + ) + + assert response.status_code == 400, response.json() + assert "fixed From address and name" in response.json()["detail"] + + def test_malformed_sender_rotation_returns_a_validation_error(self) -> None: + workflow = self._workflow() + workflow["actions"][1]["config"]["inputs"]["email"]["value"]["from"]["integrationIds"] = self.sender.id + + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow, HTTP_X_POSTHOG_CLIENT="mcp") + + assert response.status_code == 400, response.json() + assert "list of Integration IDs" in response.json()["detail"] + + def test_null_email_inputs_return_a_validation_error(self) -> None: + workflow = self._workflow() + workflow["actions"][1]["config"]["inputs"] = None + + response = self.client.post(f"/api/projects/{self.team.id}/hog_flows", workflow, HTTP_X_POSTHOG_CLIENT="mcp") + + assert response.status_code == 400, response.json() diff --git a/products/workflows/frontend/Channels/emailSendingMessages.ts b/products/workflows/frontend/Channels/emailSendingMessages.ts index 1573829036cf..52fb836d018e 100644 --- a/products/workflows/frontend/Channels/emailSendingMessages.ts +++ b/products/workflows/frontend/Channels/emailSendingMessages.ts @@ -4,3 +4,6 @@ export const EMAIL_SENDING_SUSPENDED_MESSAGE = export const UNVERIFIED_SENDER_MESSAGE = "Verify the sender's domain before sending" export const NO_EMAIL_SENDER_MESSAGE = 'Set up an email sender before sending' + +export const SANDBOX_ONLY_SENDER_MESSAGE = + 'The sandbox sender only delivers to verified members of your organization. Set up your own email sender to reach this audience.' diff --git a/products/workflows/frontend/MessageAudience/messageAudienceWarnings.test.ts b/products/workflows/frontend/MessageAudience/messageAudienceWarnings.test.ts index 5ac223a19584..0f47fcb62c0e 100644 --- a/products/workflows/frontend/MessageAudience/messageAudienceWarnings.test.ts +++ b/products/workflows/frontend/MessageAudience/messageAudienceWarnings.test.ts @@ -1,3 +1,11 @@ +import { router } from 'kea-router' + +import { LemonDialog } from '@posthog/lemon-ui' + +import { FEATURE_FLAGS } from 'lib/constants' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' +import { urls } from 'scenes/urls' + import { initKeaTests } from '~/test/init' import { PropertyFilterType, PropertyOperator } from '~/types' @@ -6,7 +14,7 @@ import { integrationsList } from 'products/integrations/frontend/generated/api' import { hogFlowsEmailSendingSuspensionRetrieve } from 'products/workflows/frontend/generated/api' import { MessageAudience, MessageAudienceDestination } from './messageAudience' -import { loadMessageAudienceWarnings } from './messageAudienceWarnings' +import { loadMessageAudienceWarnings, openMessageAudience } from './messageAudienceWarnings' jest.mock('products/cohorts/frontend/generated/api', () => ({ cohortsRetrieve: jest.fn() })) jest.mock('products/integrations/frontend/generated/api', () => ({ integrationsList: jest.fn() })) @@ -17,13 +25,14 @@ const LIST_MATCHING = '"Signups" is still matching people. You can launch when i const verified = { kind: 'email', config: { verified: true } } const unverified = { kind: 'email', config: { verified: false } } +const sandbox = { kind: 'email', config: { provider: 'sandbox', verified: true } } const matchingList = { name: 'Signups', is_static: true, is_calculating: true, last_calculation: null } const audience: MessageAudience = { source: 'cohort', properties: [{ key: 'id', type: PropertyFilterType.Cohort, value: 7, operator: PropertyOperator.In }], } -describe('loadMessageAudienceWarnings', () => { +describe('messageAudienceWarnings', () => { beforeEach(() => { initKeaTests() }) @@ -33,6 +42,7 @@ describe('loadMessageAudienceWarnings', () => { destination: MessageAudienceDestination suspended?: boolean senders?: object[] | Error + sandboxFlag?: boolean morePages?: boolean cohort?: object | Error expected: string[] @@ -51,6 +61,33 @@ describe('loadMessageAudienceWarnings', () => { senders: [unverified], expected: ["Verify the sender's domain before sending"], }, + ...(['broadcast', 'workflow'] as const).map((destination) => ({ + case: `only the sandbox sender (${destination})`, + destination, + senders: [sandbox], + expected: [ + 'The sandbox sender only delivers to verified members of your organization. Set up your own email sender to reach this audience.', + ], + })), + { + case: 'only a sandbox sender kept after the flag was turned off', + destination: 'workflow', + senders: [sandbox], + sandboxFlag: false, + expected: [], + }, + { + case: 'an unverified own sender beside the sandbox sender', + destination: 'broadcast', + senders: [sandbox, unverified], + expected: ["Verify the sender's domain before sending"], + }, + { + case: 'a verified own sender beside the sandbox sender', + destination: 'broadcast', + senders: [sandbox, verified], + expected: [], + }, { case: 'unverified senders on the first of several pages', destination: 'broadcast', @@ -69,7 +106,19 @@ describe('loadMessageAudienceWarnings', () => { }, ])( 'reports $case', - async ({ destination, suspended = false, senders = [verified], morePages, cohort, expected }) => { + async ({ + destination, + suspended = false, + senders = [verified], + sandboxFlag = true, + morePages, + cohort, + expected, + }) => { + featureFlagLogic.mount() + featureFlagLogic.actions.setFeatureFlags(sandboxFlag ? [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER] : [], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: sandboxFlag, + }) jest.mocked(hogFlowsEmailSendingSuspensionRetrieve).mockResolvedValue({ email_sending_suspended: suspended, } as never) @@ -104,4 +153,23 @@ describe('loadMessageAudienceWarnings', () => { jest.useRealTimers() } }) + + it('offers to set up email when only the sandbox sender exists', async () => { + jest.mocked(hogFlowsEmailSendingSuspensionRetrieve).mockResolvedValue({ + email_sending_suspended: false, + } as never) + jest.mocked(integrationsList).mockResolvedValue({ count: 1, results: [sandbox], next: null } as never) + featureFlagLogic.mount() + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + }) + const openDialog = jest.spyOn(LemonDialog, 'open').mockImplementation(() => {}) + + await openMessageAudience(audience, 'workflow') + const secondaryButton = openDialog.mock.calls[0][0].secondaryButton + secondaryButton?.onClick?.({} as never) + + expect(secondaryButton?.children).toEqual('Set up email') + expect(router.values.location.pathname).toContain(urls.workflows('channels')) + }) }) diff --git a/products/workflows/frontend/MessageAudience/messageAudienceWarnings.tsx b/products/workflows/frontend/MessageAudience/messageAudienceWarnings.tsx index 03758588a586..a5358954af25 100644 --- a/products/workflows/frontend/MessageAudience/messageAudienceWarnings.tsx +++ b/products/workflows/frontend/MessageAudience/messageAudienceWarnings.tsx @@ -3,6 +3,9 @@ import { router } from 'kea-router' import { LemonDialog } from '@posthog/lemon-ui' import { ApiConfig } from 'lib/api' +import { FEATURE_FLAGS } from 'lib/constants' +import { isSandboxEmailSender } from 'lib/integrations/sandboxEmailSender' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { urls } from 'scenes/urls' import { cohortsRetrieve } from 'products/cohorts/frontend/generated/api' @@ -14,14 +17,22 @@ import { audienceCohortIds, audienceCohortLaunchError, toAudienceCohort } from ' import { EMAIL_SENDING_SUSPENDED_MESSAGE, NO_EMAIL_SENDER_MESSAGE, + SANDBOX_ONLY_SENDER_MESSAGE, UNVERIFIED_SENDER_MESSAGE, } from '../Channels/emailSendingMessages' import { MessageAudience, MessageAudienceDestination, messageAudienceUrl } from './messageAudience' -export function emailSenderWarning(integrations: Pick[]): string | null { - const senders = integrations.filter((integration) => integration.kind === 'email') +export function emailSenderWarning( + integrations: Pick[], + sandboxSenderEnabled: boolean +): string | null { + const emailIntegrations = integrations.filter((integration) => integration.kind === 'email') + // With the flag off, a sandbox sender kept from earlier counts like any other sender, as it did before the flag. + const senders = sandboxSenderEnabled + ? emailIntegrations.filter((integration) => !isSandboxEmailSender(integration)) + : emailIntegrations if (senders.length === 0) { - return NO_EMAIL_SENDER_MESSAGE + return emailIntegrations.length > 0 ? SANDBOX_ONLY_SENDER_MESSAGE : NO_EMAIL_SENDER_MESSAGE } const verified = senders.some( (integration) => (integration.config as { verified?: boolean } | undefined)?.verified === true @@ -29,6 +40,10 @@ export function emailSenderWarning(integrations: Pick (cohort ? audienceCohortLaunchError(cohort) : null)), ].filter((warning): warning is string => !!warning) } @@ -111,10 +126,11 @@ export async function openMessageAudience( onClick: navigate, 'data-attr': `message-audience-${audience.source}-continue`, }, - secondaryButton: - warnings.includes(NO_EMAIL_SENDER_MESSAGE) || warnings.includes(UNVERIFIED_SENDER_MESSAGE) - ? { children: 'Set up email', onClick: () => router.actions.push(urls.workflows('channels')) } - : { children: 'Cancel' }, + secondaryButton: [NO_EMAIL_SENDER_MESSAGE, SANDBOX_ONLY_SENDER_MESSAGE, UNVERIFIED_SENDER_MESSAGE].some( + (message) => warnings.includes(message) + ) + ? { children: 'Set up email', onClick: () => router.actions.push(urls.workflows('channels')) } + : { children: 'Cancel' }, }) } finally { opening = false diff --git a/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.stories.tsx b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.stories.tsx new file mode 100644 index 000000000000..38645b20319e --- /dev/null +++ b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.stories.tsx @@ -0,0 +1,69 @@ +import { Meta, StoryFn } from '@storybook/react' +import { useActions } from 'kea' +import { useEffect } from 'react' + +import { FEATURE_FLAGS } from 'lib/constants' + +import { useStorybookMocks } from '~/mocks/browser' + +import { messageTemplateLogic } from './messageTemplateLogic' +import { messageTemplateTestSendLogic } from './messageTemplateTestSendLogic' +import { SendTestEmailModal } from './SendTestEmailModal' + +const LOGIC_PROPS = { id: 'new' } + +const SANDBOX_SENDER = { + id: 7, + kind: 'email', + display_name: 'Acme via PostHog ', + config: { provider: 'sandbox', name: 'Acme via PostHog', email: 'sandbox@example.com', verified: true }, + created_at: '2026-08-18T00:00:00Z', +} + +const MEMBER_EMAIL = 'teammate@example.com' + +type StoryArgs = { + recipient?: string +} + +const meta: Meta = { + title: 'Products/Workflows/Send test email modal', + parameters: { + layout: 'fullscreen', + viewMode: 'story', + featureFlags: [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], + testOptions: { snapshotBrowsers: [] }, + }, +} +export default meta + +const Template: StoryFn = ({ recipient }) => { + useStorybookMocks({ + get: { + '/api/organizations/:organization_id/members/': { + results: [{ id: 'sandbox-teammate', user: { id: 42, email: MEMBER_EMAIL, is_email_verified: true } }], + }, + '/api/projects/:team_id/integrations/': { results: [SANDBOX_SENDER] }, + '/api/environments/:team_id/messaging_templates/': { results: [] }, + }, + }) + const { setTemplateValue } = useActions(messageTemplateLogic(LOGIC_PROPS)) + const { setModalOpen, setRecipientEmail } = useActions(messageTemplateTestSendLogic(LOGIC_PROPS)) + + useEffect(() => { + setTemplateValue('content.email.subject', 'Welcome to Acme') + setTemplateValue('content.email.text', 'Hello from the team.') + setModalOpen(true) + if (recipient) { + setRecipientEmail(recipient) + } + }, [recipient, setTemplateValue, setModalOpen, setRecipientEmail]) + + return +} + +export const SandboxSenderPreselected: StoryFn = Template.bind({}) +SandboxSenderPreselected.args = { recipient: MEMBER_EMAIL } + +export const RecipientOutsideOrganization: StoryFn = Template.bind({}) +RecipientOutsideOrganization.args = { recipient: 'outsider@example.com' } diff --git a/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.test.tsx b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.test.tsx new file mode 100644 index 000000000000..90ec50d2b277 --- /dev/null +++ b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.test.tsx @@ -0,0 +1,154 @@ +import { MOCK_DEFAULT_ORGANIZATION_MEMBER, MOCK_DEFAULT_USER } from 'lib/api.mock' + +import '@testing-library/jest-dom' + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { expectLogic } from 'kea-test-utils' + +import { FEATURE_FLAGS } from 'lib/constants' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' +import { userLogic } from 'scenes/userLogic' + +import { useMocks } from '~/mocks/jest' +import { initKeaTests } from '~/test/init' + +import { messageTemplateLogic } from './messageTemplateLogic' +import { messageTemplateTestSendLogic } from './messageTemplateTestSendLogic' +import { SendTestEmailModal } from './SendTestEmailModal' + +const sandbox = { + id: 9, + kind: 'email', + display_name: 'Sandbox ', + config: { provider: 'sandbox', verified: true }, +} + +describe('SendTestEmailModal', () => { + let logic: ReturnType + let template: ReturnType + + beforeEach(() => { + initKeaTests() + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + }) + }) + afterEach(() => { + cleanup() + logic?.unmount() + template?.unmount() + }) + + const openModal = (): void => { + template = messageTemplateLogic({ id: 'new' }) + template.mount() + template.actions.setTemplateValue('content.email.subject', 'Welcome') + logic = messageTemplateTestSendLogic({ id: 'new' }) + logic.mount() + logic.actions.setModalOpen(true) + render() + } + + it.each([ + [false, 'Verify your email address to send test emails to yourself.'], + [true, null], + ])('asks a signed-in user with verified=%s to verify their own address', async (verified, expected) => { + useMocks({ + get: { + '/api/projects/:team_id/integrations/': { results: [sandbox] }, + '/api/organizations/:organization_id/members/': { results: [] }, + }, + }) + userLogic.actions.loadUserSuccess({ ...MOCK_DEFAULT_USER, is_email_verified: verified }) + openModal() + await expectLogic(logic).toFinishAllListeners() + logic.actions.setRecipientEmail(MOCK_DEFAULT_USER.email.toUpperCase()) + + await waitFor(() => + expect(screen.queryByText(/isn't among the verified members you can see/) !== null).toBe(verified) + ) + expect( + screen.queryByText('Verify your email address to send test emails to yourself.')?.textContent ?? null + ).toBe(expected) + }) + + it.each([ + [ + 'a suggested teammate', + 'teammate@example.com', + () => fireEvent.click(screen.getByText('teammate@example.com')), + ], + [ + 'a typed address', + 'someone@example.com', + (input: HTMLElement) => { + fireEvent.change(input, { target: { value: 'someone@example.com' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + }, + ], + ])('sends the test to %s', async (_, expectedRecipient, pickRecipient) => { + let sentTo: string | null = null + useMocks({ + get: { + '/api/projects/:team_id/integrations/': { results: [sandbox] }, + '/api/organizations/:organization_id/members/': { + results: [ + { + ...MOCK_DEFAULT_ORGANIZATION_MEMBER, + user: { + ...MOCK_DEFAULT_ORGANIZATION_MEMBER.user, + email: 'teammate@example.com', + is_email_verified: true, + }, + }, + ], + }, + }, + post: { + '/api/environments/:team_id/hog_flows/:id/invocations/': async ({ request }: { request: Request }) => { + const body = await request.json() + const emailAction = body.configuration.actions.find((a: any) => a.type === 'function_email') + sentTo = emailAction.config.inputs.email.value.to.email + return [200, { status: 'success', nextActionId: null }] + }, + }, + }) + openModal() + await expectLogic(logic).toFinishAllListeners() + + const input = screen.getByPlaceholderText('you@example.com') + fireEvent.focus(input) + pickRecipient(input) + fireEvent.click(screen.getByText('Send test email', { selector: 'button *' })) + await expectLogic(logic).toFinishAllListeners() + + expect(sentTo).toBe(expectedRecipient) + }) + + it.each([true, false])('distinguishes member loading from an empty list (%s)', async (loading) => { + let releaseMembers: () => void = () => {} + const gate = new Promise((resolve) => { + releaseMembers = resolve + }) + useMocks({ + get: { + '/api/projects/:team_id/integrations/': { results: [sandbox] }, + '/api/organizations/:organization_id/members/': async () => { + if (loading) { + await gate + } + return [200, { results: [] }] + }, + }, + }) + openModal() + if (!loading) { + await expectLogic(logic).toFinishAllListeners() + } + fireEvent.focus(screen.getByPlaceholderText('you@example.com')) + expect(document.querySelector('.LemonSkeleton') !== null).toBe(loading) + expect(screen.queryByText('Type an email address and press Enter') !== null).toBe(!loading) + releaseMembers() + await expectLogic(logic).toFinishAllListeners() + }) +}) diff --git a/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsx b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsx index 398cc2ca64aa..a059c97f0676 100644 --- a/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsx +++ b/products/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsx @@ -1,20 +1,110 @@ import { useActions, useValues } from 'kea' import { IconExternal } from '@posthog/icons' -import { LemonBanner, LemonButton, LemonInput, LemonLabel, LemonSelect, Spinner } from '@posthog/lemon-ui' +import { + LemonBanner, + LemonButton, + LemonInput, + LemonInputSelect, + LemonLabel, + LemonSelect, + type LemonSelectOption, + LemonTag, + Spinner, +} from '@posthog/lemon-ui' +import { SANDBOX_EMAIL_SENDER_NOTE } from 'lib/integrations/sandboxEmailSender' import { LemonModal } from 'lib/lemon-ui/LemonModal' import { urls } from 'scenes/urls' +import { IntegrationType } from '~/types' + import { MessageTemplateLogicProps } from './messageTemplateLogic' import { messageTemplateTestSendLogic } from './messageTemplateTestSendLogic' +function senderOption( + integration: IntegrationType, + sandboxEmailSender: IntegrationType | null +): LemonSelectOption { + const option = { label: integration.display_name, value: integration.id } + if (integration.id !== sandboxEmailSender?.id) { + return option + } + return { + ...option, + labelInMenu: ( + + {integration.display_name} + Sandbox + + ), + } +} + +function TestEmailRecipientField({ logicProps }: { logicProps: MessageTemplateLogicProps }): JSX.Element { + const logic = messageTemplateTestSendLogic(logicProps) + const { + recipientEmail, + recipientSuggestions, + recipientIsUnverifiedSelf, + recipientNotSuggested, + membersLoading, + sandboxEmailSenderEnabled, + } = useValues(logic) + const { setRecipientEmail } = useActions(logic) + + return ( + <> + {sandboxEmailSenderEnabled ? ( + setRecipientEmail(values[0] ?? '')} + options={recipientSuggestions.map((email) => ({ key: email, label: email }))} + loading={membersLoading} + placeholder="you@example.com" + emptyStateComponent="Type an email address and press Enter" + data-attr="send-test-email-recipient" + /> + ) : ( + + )} + {recipientIsUnverifiedSelf ? ( + Verify your email address to send test emails to yourself. + ) : recipientNotSuggested ? ( + + + {recipientEmail} + {' '} + isn't among the verified members you can see. The sandbox sender may not deliver to it. Add your own + sender to email anyone. + + ) : null} + + ) +} + export function SendTestEmailModal(props: MessageTemplateLogicProps & { isOpen: boolean }): JSX.Element { const { isOpen, ...logicProps } = props const logic = messageTemplateTestSendLogic(logicProps) const { - recipientEmail, senderIntegrationId, + sandboxEmailSender, + isSandboxSenderSelected, emailIntegrations, integrationsLoading, sendDisabledReason, @@ -22,7 +112,7 @@ export function SendTestEmailModal(props: MessageTemplateLogicProps & { isOpen: testSendResultLoading, testSendSkipReason, } = useValues(logic) - const { setModalOpen, setRecipientEmail, setSenderIntegrationId, sendTestEmail } = useActions(logic) + const { setModalOpen, setSenderIntegrationId, sendTestEmail } = useActions(logic) return (
Send to - +
From {integrationsLoading && emailIntegrations.length === 0 ? ( ) : emailIntegrations.length > 0 ? ( - setSenderIntegrationId(id)} - options={emailIntegrations.map((integration) => ({ - label: integration.display_name, - value: integration.id, - }))} - data-attr="send-test-email-sender" - fullWidth - /> + <> + setSenderIntegrationId(id)} + options={emailIntegrations.map((integration) => + senderOption(integration, sandboxEmailSender) + )} + data-attr="send-test-email-sender" + fullWidth + /> + {isSandboxSenderSelected && ( + {SANDBOX_EMAIL_SENDER_NOTE} + )} + ) : (
No email senders configured yet diff --git a/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.ts b/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.ts index 38b4ba36c70f..35de1daf0a4c 100644 --- a/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.ts +++ b/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.ts @@ -1,8 +1,13 @@ +import { MOCK_DEFAULT_ORGANIZATION_MEMBER, MOCK_SECOND_BASIC_USER, MOCK_SECOND_ORGANIZATION_MEMBER } from 'lib/api.mock' + import { expectLogic } from 'kea-test-utils' +import { FEATURE_FLAGS } from 'lib/constants' import { integrationsLogic } from 'lib/integrations/integrationsLogic' +import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { useMocks } from '~/mocks/jest' +import { membersLogic } from '~/models/membersLogic' import { initKeaTests } from '~/test/init' import { messageTemplateLogic } from './messageTemplateLogic' @@ -18,6 +23,11 @@ jest.mock('lib/lemon-ui/LemonToast', () => ({ const mockToast = require('lib/lemon-ui/LemonToast').lemonToast +const VERIFIED_MEMBERS = [MOCK_DEFAULT_ORGANIZATION_MEMBER, MOCK_SECOND_ORGANIZATION_MEMBER].map((member) => ({ + ...member, + user: { ...member.user, is_email_verified: true }, +})) + describe('messageTemplateTestSendLogic', () => { let logic: ReturnType let templateLogic: ReturnType @@ -29,6 +39,7 @@ describe('messageTemplateTestSendLogic', () => { useMocks({ get: { + '/api/organizations/:organization_id/members/': { results: VERIFIED_MEMBERS }, '/api/projects/:team_id/integrations/': { results: [ { @@ -91,6 +102,22 @@ describe('messageTemplateTestSendLogic', () => { }).toMatchValues({ recipientEmail: 'john.doe@posthog.com' }) }) + it('leaves the recipient empty with the sandbox flag on, so the member suggestions show with the user first', async () => { + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + }) + try { + await expectLogic(logic, () => { + logic.actions.setModalOpen(true) + }) + .toDispatchActions(membersLogic, ['loadAllMembersSuccess']) + .toMatchValues({ recipientEmail: '' }) + expect(logic.values.recipientSuggestions[0]).toBe('john.doe@posthog.com') + } finally { + featureFlagLogic.actions.setFeatureFlags([], {}) + } + }) + it('defaults the sender to the first verified email integration, excluding unverified, sandbox and non-email kinds', async () => { await expectLogic(logic).toMatchValues({ senderIntegrationId: 5, @@ -98,6 +125,12 @@ describe('messageTemplateTestSendLogic', () => { }) }) + it('keeps the chosen sender even when the listed senders do not include it', async () => { + logic.actions.setSenderIntegrationId(99) + + await expectLogic(logic).toMatchValues({ senderIntegrationId: 99 }) + }) + it('sends a one-step synthetic workflow carrying the typed recipient and template content, stripping any cc/bcc', async () => { templateLogic.actions.setTemplateValue('content.email.from', { integrationId: 7, @@ -168,6 +201,207 @@ describe('messageTemplateTestSendLogic', () => { expect(logic.values.testSendSkipReason).toBe(expectedReason) }) + describe('sandbox sender', () => { + const SANDBOX_SENDER = { + id: 7, + kind: 'email', + display_name: 'Acme via PostHog ', + config: { provider: 'sandbox', name: 'Acme via PostHog', email: 'sandbox@example.com', verified: true }, + } + const UNVERIFIED_OWN_SENDER = { + id: 4, + kind: 'email', + display_name: 'Unverified sender ', + config: { verified: false }, + } + const VERIFIED_OWN_SENDER = { + id: 5, + kind: 'email', + display_name: 'Sender ', + config: { verified: true }, + } + + let integrationsPayload: Record[] + + const loadIntegrations = async (integrations: Record[]): Promise => { + integrationsPayload = integrations + await expectLogic(integrationsLogic, () => { + integrationsLogic.actions.loadIntegrations() + }).toDispatchActions(['loadIntegrationsSuccess']) + } + + beforeEach(() => { + integrationsPayload = [] + useMocks({ + get: { '/api/projects/:team_id/integrations/': () => [200, { results: integrationsPayload }] }, + }) + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + }) + }) + + afterEach(() => { + featureFlagLogic.actions.setFeatureFlags([], {}) + }) + + it.each([ + { + description: 'preselects the sandbox sender when the project has no verified own sender', + integrations: [UNVERIFIED_OWN_SENDER, SANDBOX_SENDER], + expectedSenderId: 7, + expectedSenderIds: [7], + }, + { + description: 'prefers a verified own sender and lists the sandbox sender last', + integrations: [SANDBOX_SENDER, VERIFIED_OWN_SENDER], + expectedSenderId: 5, + expectedSenderIds: [5, 7], + }, + ])('$description', async ({ integrations, expectedSenderId, expectedSenderIds }) => { + await loadIntegrations(integrations) + + await expectLogic(logic).toMatchValues({ senderIntegrationId: expectedSenderId }) + expect(logic.values.emailIntegrations.map((integration) => integration.id)).toEqual(expectedSenderIds) + }) + + it('lists the sandbox sender only while the flag is on, and drops a selection that is no longer listed', async () => { + await loadIntegrations([SANDBOX_SENDER, VERIFIED_OWN_SENDER]) + logic.actions.setSenderIntegrationId(7) + await expectLogic(logic).toMatchValues({ senderIntegrationId: 7 }) + + featureFlagLogic.actions.setFeatureFlags([], {}) + + await expectLogic(logic).toMatchValues({ + senderIntegrationId: 5, + emailIntegrations: [expect.objectContaining({ id: 5 })], + }) + }) + + it('loads the member suggestions once when the flag lands on an open modal', async () => { + featureFlagLogic.actions.setFeatureFlags([], {}) + await loadIntegrations([SANDBOX_SENDER]) + await expectLogic(logic, () => { + logic.actions.setModalOpen(true) + }).toNotHaveDispatchedActions([membersLogic.actionTypes.loadAllMembers]) + + await expectLogic(logic, () => { + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + }) + }).toDispatchActions(membersLogic, ['loadAllMembersSuccess']) + + await expectLogic(logic).toMatchValues({ + senderIntegrationId: 7, + recipientSuggestions: expect.arrayContaining(['john.doe@posthog.com']), + }) + + await expectLogic(logic, () => { + featureFlagLogic.actions.setFeatureFlags([FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER, 'some-other-flag'], { + [FEATURE_FLAGS.WORKFLOWS_SANDBOX_SENDER]: true, + 'some-other-flag': true, + }) + }).toNotHaveDispatchedActions([membersLogic.actionTypes.loadAllMembers]) + }) + + it('sends a sandbox test without the template Reply-To and custom From, which the sandbox sender ignores', async () => { + await loadIntegrations([SANDBOX_SENDER]) + templateLogic.actions.setTemplateValue('content.email.text', 'Hello!') + templateLogic.actions.setTemplateValue('content.email.replyTo', 'replies@example.com') + templateLogic.actions.setTemplateValue('content.email.from', { + integrationId: 7, + name: 'Security team', + email: 'alerts@example.com', + }) + logic.actions.setRecipientEmail('rose.dawson@posthog.com') + + await expectLogic(logic, () => { + logic.actions.sendTestEmail() + }).toDispatchActions(['sendTestEmailSuccess']) + + const emailAction = capturedBody.configuration.actions.find((a: any) => a.type === 'function_email') + expect(emailAction.config.inputs.email.value.from).toEqual({ integrationId: 7 }) + expect(emailAction.config.inputs.email.value.replyTo).toBe('') + }) + + it('warns about addresses absent from visible verified suggestions without blocking server authorization', async () => { + useMocks({ + get: { + '/api/organizations/:organization_id/members/': { + results: [ + ...VERIFIED_MEMBERS, + { + ...MOCK_SECOND_ORGANIZATION_MEMBER, + id: 'unverified-member', + user: { + ...MOCK_SECOND_BASIC_USER, + id: 303, + email: 'unverified@example.com', + is_email_verified: false, + }, + }, + { + ...MOCK_SECOND_ORGANIZATION_MEMBER, + id: 'verification-unknown-member', + user: { + ...MOCK_SECOND_BASIC_USER, + id: 304, + email: 'unknown@example.com', + is_email_verified: undefined, + }, + }, + ], + }, + }, + }) + await loadIntegrations([SANDBOX_SENDER, VERIFIED_OWN_SENDER]) + templateLogic.actions.setTemplateValue('content.email.text', 'Hello!') + + await expectLogic(logic, () => { + logic.actions.setModalOpen(true) + logic.actions.setSenderIntegrationId(7) + logic.actions.setRecipientEmail('outsider@example.com') + }) + .toDispatchActions(membersLogic, ['loadAllMembersSuccess']) + .toMatchValues({ + recipientNotSuggested: true, + sendDisabledReason: undefined, + recipientSuggestions: expect.arrayContaining(['rose.dawson@posthog.com']), + }) + + logic.actions.setRecipientEmail('Rose.Dawson@posthog.com') + await expectLogic(logic).toMatchValues({ recipientNotSuggested: false, sendDisabledReason: undefined }) + + logic.actions.setRecipientEmail('unverified@example.com') + await expectLogic(logic).toMatchValues({ recipientNotSuggested: true }) + expect(logic.values.recipientSuggestions).not.toContain('unverified@example.com') + + logic.actions.setRecipientEmail('unknown@example.com') + await expectLogic(logic).toMatchValues({ recipientNotSuggested: true }) + expect(logic.values.recipientSuggestions).not.toContain('unknown@example.com') + + logic.actions.setRecipientEmail('outsider@example.com') + logic.actions.setSenderIntegrationId(5) + await expectLogic(logic).toMatchValues({ recipientNotSuggested: false, sendDisabledReason: undefined }) + }) + + it('holds the send while the member list loads, and does not warn before it has loaded', async () => { + await loadIntegrations([SANDBOX_SENDER]) + templateLogic.actions.setTemplateValue('content.email.text', 'Hello!') + logic.actions.setRecipientEmail('outsider@example.com') + + await expectLogic(logic, () => { + logic.actions.setModalOpen(true) + }) + .toDispatchActions(membersLogic, ['loadAllMembers']) + .toMatchValues({ + recipientNotSuggested: false, + sendDisabledReason: 'Checking your organization members', + }) + .toDispatchActions(membersLogic, ['loadAllMembersSuccess']) + .toMatchValues({ recipientNotSuggested: true }) + }) + }) + it('keeps the modal open and surfaces the error when the send fails', async () => { useMocks({ post: { diff --git a/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.ts b/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.ts index cab2df5d0e22..fa101334270f 100644 --- a/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.ts +++ b/products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.ts @@ -1,15 +1,16 @@ import { MakeLogicType, actions, connect, kea, key, listeners, path, props, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' +import { subscriptions } from 'kea-subscriptions' import api from 'lib/api' import { integrationsLogic } from 'lib/integrations/integrationsLogic' -import { isSandboxEmailSender } from 'lib/integrations/sandboxEmailSender' import { lemonToast } from 'lib/lemon-ui/LemonToast' import { isEmail } from 'lib/utils/url' import { teamLogic } from 'scenes/teamLogic' import { userLogic } from 'scenes/userLogic' -import { IntegrationType, UserType } from '~/types' +import { membersLogic } from '~/models/membersLogic' +import { IntegrationType, OrganizationMemberType, UserType } from '~/types' import { senderFrom, @@ -27,16 +28,28 @@ import { MessageTemplate } from './types' // below and passed as `current_action_id` so the CDP worker executes just this one node. const TEST_EMAIL_ACTION_ID = 'send_test_email' +const sameEmail = (a: string, b: string): boolean => a.trim().toLowerCase() === b.trim().toLowerCase() + // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface messageTemplateTestSendLogicValues { - integrations: IntegrationType[] | null // integrationsLogic integrationsLoading: boolean // integrationsLogic + ownEmailIntegrations: IntegrationType[] // integrationsLogic + sandboxEmailSender: IntegrationType | null // integrationsLogic + sandboxEmailSenderEnabled: boolean // integrationsLogic + meFirstMembers: OrganizationMemberType[] // membersLogic + members: OrganizationMemberType[] | null // membersLogic + membersLoading: boolean // membersLogic template: MessageTemplate // messageTemplateLogic currentTeamId: number | null // teamLogic user: UserType | null // userLogic emailIntegrations: IntegrationType[] isModalOpen: boolean + isSandboxSenderSelected: boolean + membersNeeded: boolean recipientEmail: string + recipientIsUnverifiedSelf: boolean + recipientNotSuggested: boolean + recipientSuggestions: string[] sendDisabledReason: string | undefined senderIntegrationId: number | null senderIntegrationIdOverride: number | null @@ -47,6 +60,9 @@ export interface messageTemplateTestSendLogicValues { // Generated by kea-typegen. Update if you're an agent, ignore if you're human. export interface messageTemplateTestSendLogicActions { + loadAllMembers: () => { + value: true + } // membersLogic clearTestSendResult: () => { value: true } @@ -98,17 +114,39 @@ export interface messageTemplateTestSendLogicActions { export interface messageTemplateTestSendLogicMeta { key: string __keaTypeGenInternalSelectorTypes: { - emailIntegrations: (integrations: IntegrationType[] | null) => IntegrationType[] + emailIntegrations: ( + ownEmailIntegrations: IntegrationType[], + sandboxEmailSender: IntegrationType | null + ) => IntegrationType[] senderIntegrationId: ( senderIntegrationIdOverride: number | null, emailIntegrations: IntegrationType[], template: MessageTemplate ) => number | null testSendSkipReason: (testSendResult: HogflowTestResult | null) => string | undefined + membersNeeded: (isModalOpen: boolean, sandboxEmailSenderEnabled: boolean) => boolean + isSandboxSenderSelected: ( + senderIntegrationId: number | null, + sandboxEmailSender: IntegrationType | null + ) => boolean + recipientSuggestions: (meFirstMembers: OrganizationMemberType[]) => string[] + recipientIsUnverifiedSelf: ( + isSandboxSenderSelected: boolean, + recipientEmail: string, + user: UserType | null + ) => boolean + recipientNotSuggested: ( + isSandboxSenderSelected: boolean, + recipientEmail: string, + members: OrganizationMemberType[] | null, + recipientSuggestions: string[] + ) => boolean sendDisabledReason: ( template: MessageTemplate, recipientEmail: string, - senderIntegrationId: number | null + senderIntegrationId: number | null, + isSandboxSenderSelected: boolean, + membersLoading: boolean ) => string | undefined } } @@ -133,8 +171,11 @@ export const messageTemplateTestSendLogic = kea ({ open }), @@ -153,7 +194,8 @@ export const messageTemplateTestSendLogic = kea null, sendTestEmail: async (): Promise => { - const { template, recipientEmail, senderIntegrationId, currentTeamId } = values + const { template, recipientEmail, senderIntegrationId, currentTeamId, isSandboxSenderSelected } = + values const emailAction: HogFlowAction = { id: TEST_EMAIL_ACTION_ID, @@ -172,12 +214,16 @@ export const messageTemplateTestSendLogic = kea [s.integrations], - (integrations: IntegrationType[] | null): IntegrationType[] => - integrations?.filter( - (integration) => - integration.kind === 'email' && - integration.config?.verified === true && - !isSandboxEmailSender(integration) - ) ?? [], + (s) => [s.ownEmailIntegrations, s.sandboxEmailSender], + ( + ownEmailIntegrations: IntegrationType[], + sandboxEmailSender: IntegrationType | null + ): IntegrationType[] => { + const verifiedOwnSenders = ownEmailIntegrations.filter( + (integration) => integration.config?.verified === true + ) + return sandboxEmailSender ? [...verifiedOwnSenders, sandboxEmailSender] : verifiedOwnSenders + }, ], senderIntegrationId: [ (s) => [s.senderIntegrationIdOverride, s.emailIntegrations, s.template], @@ -242,12 +290,52 @@ export const messageTemplateTestSendLogic = kea [s.isModalOpen, s.sandboxEmailSenderEnabled], + (isModalOpen: boolean, sandboxEmailSenderEnabled: boolean): boolean => + isModalOpen && sandboxEmailSenderEnabled, + ], + isSandboxSenderSelected: [ + (s) => [s.senderIntegrationId, s.sandboxEmailSender], + (senderIntegrationId: number | null, sandboxEmailSender: IntegrationType | null): boolean => + sandboxEmailSender !== null && senderIntegrationId === sandboxEmailSender.id, + ], + recipientSuggestions: [ + (s) => [s.meFirstMembers], + (meFirstMembers: OrganizationMemberType[]): string[] => + meFirstMembers + .filter((member) => member.user.is_email_verified === true) + .map((member) => member.user.email), + ], + recipientIsUnverifiedSelf: [ + (s) => [s.isSandboxSenderSelected, s.recipientEmail, s.user], + (isSandboxSenderSelected: boolean, recipientEmail: string, user: UserType | null): boolean => + isSandboxSenderSelected && + !!user && + user.is_email_verified !== true && + sameEmail(user.email, recipientEmail), + ], + recipientNotSuggested: [ + (s) => [s.isSandboxSenderSelected, s.recipientEmail, s.members, s.recipientSuggestions], + ( + isSandboxSenderSelected: boolean, + recipientEmail: string, + members: OrganizationMemberType[] | null, + recipientSuggestions: string[] + ): boolean => + isSandboxSenderSelected && + members !== null && + isEmail(recipientEmail) && + !recipientSuggestions.some((suggestion) => sameEmail(suggestion, recipientEmail)), + ], sendDisabledReason: [ - (s) => [s.template, s.recipientEmail, s.senderIntegrationId], + (s) => [s.template, s.recipientEmail, s.senderIntegrationId, s.isSandboxSenderSelected, s.membersLoading], ( template: MessageTemplate, recipientEmail: string, - senderIntegrationId: number | null + senderIntegrationId: number | null, + isSandboxSenderSelected: boolean, + membersLoading: boolean ): string | undefined => { if (!template.content.email?.subject) { return 'Add a subject first' @@ -261,6 +349,9 @@ export const messageTemplateTestSendLogic = kea ({ + membersNeeded: (membersNeeded: boolean, wereNeeded: boolean | undefined) => { + if (membersNeeded && !wereNeeded) { + actions.loadAllMembers() + } + }, + // When the flag turns off, a chosen sandbox sender falls back to the default sender. + sandboxEmailSender: (sender: IntegrationType | null, previous: IntegrationType | null | undefined) => { + if (previous && !sender && values.senderIntegrationIdOverride === previous.id) { + actions.setSenderIntegrationId(null) + } + }, + })), ])