Keep access tokens and raw PII out of the Messenger transport

README.md

@@ -124,6 +124,16 @@ framework:
 
 With a transport, Messenger also retries a failed send and moves it to the failure transport when it keeps failing.
 
+What ends up in the transport is the finished payload: the user data is already normalised and hashed by the SDK, and
+only pixel ids travel. Access tokens are resolved when the event is sent, through `AccessTokenResolverInterface`,
+whose default implementation reads them from the `pixels` configuration. Alias it if your pixels come from somewhere
+else:
+
+```yaml
+services:
+    Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface: '@App\Provider\MyAccessTokenResolver'
+```
+
 Either way, a send that fails is logged as an error and never propagates into the response, so an expired access
 token or an outage at Meta cannot break the page.
 

UPGRADE.md

@@ -75,6 +75,48 @@ anything to `framework.messenger`. `SendEvent` is dispatched on your application
   `?ConsentContextInterface $consentContext` and `bool $consentEnabled` / `bool $clientSideEnabled` /
   `bool $serverSideEnabled` arguments. Adapt subclasses, decorators and custom service definitions.
 
+## The SendEvent command changed shape
+
+`SendEvent` no longer carries the `Setono\MetaConversionsApi\Event\Event` object. It carries the finished payload
+instead:
+
+```php
+new SendEvent(
+    string $eventName,
+    string $eventId,
+    array $payload,      // already normalized and hashed by the SDK
+    array $pixelIds,     // ids only, no access tokens
+    ?string $testEventCode = null,
+);
+```
+
+Build one from an event with `SendEvent::fromEvent($event)`.
+
+**Why:** when the command is routed to a transport it is written to that transport's storage, and to the failure
+transport when it fails. Previously that storage received the Conversions API access token and every raw email
+address, phone number and name the application had attached, because hashing only happened later inside
+`Client::sendEvent()`. Failure transports are often kept indefinitely, which made that a retention problem too.
+
+Access tokens are now resolved when the event is sent, through the new
+`Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface`. The default implementation reads
+them from the `pixels` configuration. If your pixels come from your own `PixelProviderInterface`, alias the resolver
+as well:
+
+```yaml
+services:
+    Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface: '@App\Provider\MyAccessTokenResolver'
+```
+
+Note that the resolver runs in the worker, so it must not depend on the current request.
+
+If you wrote your own handler or middleware for `SendEvent`, read `$message->payload` and `$message->pixelIds`
+instead of `$message->event`.
+
+`SendEventHandler::__construct()` takes the resolver as its second argument, so its signature changed from
+`(ClientInterface $client, ?LoggerInterface $logger)` to
+`(ClientInterface $client, AccessTokenResolverInterface $accessTokenResolver, ?LoggerInterface $logger)`. Update the
+service definition if you decorated or redefined it.
+
 ## Failures no longer propagate
 
 `DispatchOnCommandBusSubscriber` catches and logs anything thrown while dispatching, at error level on the

src/AccessTokenResolver/AccessTokenResolverInterface.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\MetaConversionsApiBundle\AccessTokenResolver;
+
+interface AccessTokenResolverInterface
+{
+    /**
+     * Returns the Conversions API access token for the given pixel, or null when there is none
+     *
+     * This is called when the event is sent, which may be in a worker process long after the request that raised
+     * it, so the implementation must not depend on the current request
+     */
+    public function resolve(string $pixelId): ?string;
+}

src/AccessTokenResolver/ConfigurationBasedAccessTokenResolver.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\MetaConversionsApiBundle\AccessTokenResolver;
+
+final class ConfigurationBasedAccessTokenResolver implements AccessTokenResolverInterface
+{
+    /**
+     * Keyed by pixel id. Note that PHP casts numeric string keys to integers, and pixel ids are numeric, so this
+     * is an array-key map rather than a string map. Lookups with the string id still resolve, because PHP applies
+     * the same cast on the way in
+     *
+     * @var array<array-key, string>
+     */
+    private readonly array $accessTokens;
+
+    /**
+     * @param list<array{id: string, access_token?: string|null}> $pixels
+     */
+    public function __construct(array $pixels)
+    {
+        $accessTokens = [];
+
+        foreach ($pixels as $pixel) {
+            $accessToken = $pixel['access_token'] ?? null;
+            if (null === $accessToken || '' === $accessToken) {
+                continue;
+            }
+
+            $accessTokens[$pixel['id']] = $accessToken;
+        }
+
+        $this->accessTokens = $accessTokens;
+    }
+
+    public function resolve(string $pixelId): ?string
+    {
+        return $this->accessTokens[$pixelId] ?? null;
+    }
+}

src/EventSubscriber/DispatchOnCommandBusSubscriber.php

@@ -43,7 +43,7 @@ public function dispatch(ConversionsApiEventRaised $event): void
         }
 
         try {
-            $this->commandBus->dispatch(new SendEvent($event->event));
+            $this->commandBus->dispatch(SendEvent::fromEvent($event->event));
         } catch (\Throwable $e) {
             // Tracking must never take the page down. Two things can throw here:
             //

src/Message/Command/SendEvent.php

@@ -5,13 +5,43 @@
 namespace Setono\MetaConversionsApiBundle\Message\Command;
 
 use Setono\MetaConversionsApi\Event\Event;
+use Setono\MetaConversionsApi\Pixel\Pixel;
 
 /**
  * Send a conversions api event to Meta/Facebook
+ *
+ * This deliberately carries the finished payload rather than the Event object. When the command is routed to a
+ * transport it is written to that transport's storage, and to the failure transport when it fails, so it must not
+ * carry anything that does not belong there:
+ *
+ * - The payload is already normalized and hashed by the SDK, so no raw email addresses or phone numbers are stored.
+ * - Only pixel ids travel. The access tokens are resolved when the event is sent, by an AccessTokenResolverInterface.
+ *
+ * As a side effect everything in here is a scalar or an array, so the message also survives the Symfony serializer
  */
 final class SendEvent implements CommandInterface
 {
-    public function __construct(public Event $event)
+    /**
+     * @param array<string, mixed> $payload The normalized and hashed payload, ready to be posted
+     * @param list<string> $pixelIds
+     */
+    public function __construct(
+        public readonly string $eventName,
+        public readonly string $eventId,
+        public readonly array $payload,
+        public readonly array $pixelIds,
+        public readonly ?string $testEventCode = null,
+    ) {
+    }
+
+    public static function fromEvent(Event $event): self
     {
+        return new self(
+            $event->eventName,
+            $event->eventId,
+            $event->getPayload(),
+            array_map(static fn (Pixel $pixel): string => $pixel->id, $event->pixels),
+            $event->testEventCode,
+        );
     }
 }

src/Message/Handler/SendEventHandler.php

@@ -7,56 +7,54 @@
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Setono\MetaConversionsApi\Client\ClientInterface;
-use Setono\MetaConversionsApi\Event\Event;
 use Setono\MetaConversionsApi\Pixel\Pixel;
+use Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface;
 use Setono\MetaConversionsApiBundle\Message\Command\SendEvent;
+use Setono\MetaConversionsApiBundle\Message\PreparedEvent;
 
 final class SendEventHandler
 {
     private readonly LoggerInterface $logger;
 
     public function __construct(
         private readonly ClientInterface $client,
+        private readonly AccessTokenResolverInterface $accessTokenResolver,
         ?LoggerInterface $logger = null,
     ) {
         $this->logger = $logger ?? new NullLogger();
     }
 
     public function __invoke(SendEvent $message): void
     {
-        $event = $message->event;
+        $pixels = [];
 
-        // A pixel without an access token cannot be used server side: Meta answers 400, the SDK throws, and
-        // Messenger retries the message until it ends up in the failure transport. One warning is more useful.
-        // Client side tracking is unaffected, because rendering fbq() calls only needs the pixel id
-        $pixels = array_values(array_filter(
-            $event->pixels,
-            fn (Pixel $pixel): bool => $this->hasAccessToken($pixel, $event),
-        ));
+        foreach ($message->pixelIds as $pixelId) {
+            $accessToken = $this->accessTokenResolver->resolve($pixelId);
+
+            // A pixel without an access token cannot be used server side: Meta answers 400, the SDK throws, and
+            // Messenger retries the message until it ends up in the failure transport. One warning is more useful.
+            // Client side tracking is unaffected, because rendering fbq() calls only needs the pixel id
+            if (null === $accessToken) {
+                $this->logger->warning('The pixel {pixel} has no access token, so the event {event_name} ({event_id}) was not sent to it', [
+                    'pixel' => $pixelId,
+                    'event_name' => $message->eventName,
+                    'event_id' => $message->eventId,
+                ]);
+
+                continue;
+            }
+
+            $pixels[] = new Pixel($pixelId, $accessToken);
+        }
 
         if ([] === $pixels) {
             return;
         }
 
-        // Cloned so the event the application still holds is not mutated when the command is handled synchronously
-        $event = clone $event;
+        $event = new PreparedEvent($message->eventName, $message->payload);
         $event->pixels = $pixels;
+        $event->testEventCode = $message->testEventCode;
 
         $this->client->sendEvent($event);
     }
-
-    private function hasAccessToken(Pixel $pixel, Event $event): bool
-    {
-        if (null !== $pixel->accessToken) {
-            return true;
-        }
-
-        $this->logger->warning('The pixel {pixel} has no access token, so the event {event_name} ({event_id}) was not sent to it', [
-            'pixel' => $pixel->id,
-            'event_name' => $event->eventName,
-            'event_id' => $event->eventId,
-        ]);
-
-        return false;
-    }
 }

src/Message/PreparedEvent.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\MetaConversionsApiBundle\Message;
+
+use Setono\MetaConversionsApi\Event\Event;
+
+/**
+ * An event whose payload was already built, so it can be handed to the SDK client as is
+ *
+ * The payload is computed when the event is raised, i.e. while the request that produced it is still around, and
+ * travels through the transport ready to post. This class exists to give that payload back to
+ * ClientInterface::sendEvent(), which takes an Event
+ *
+ * @internal
+ */
+final class PreparedEvent extends Event
+{
+    /**
+     * @param array<string, mixed> $payload
+     */
+    public function __construct(string $eventName, private readonly array $payload)
+    {
+        parent::__construct($eventName);
+    }
+
+    public function getPayload(string $context = self::PAYLOAD_CONTEXT_SERVER): array
+    {
+        return $this->payload;
+    }
+}

src/Resources/config/services/conditional/server_side.xml

@@ -12,8 +12,16 @@
             <tag name="monolog.logger" channel="setono_meta_conversions_api"/>
         </service>
 
+        <service id="Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface"
+                 alias="Setono\MetaConversionsApiBundle\AccessTokenResolver\ConfigurationBasedAccessTokenResolver"/>
+
+        <service id="Setono\MetaConversionsApiBundle\AccessTokenResolver\ConfigurationBasedAccessTokenResolver">
+            <argument>%setono_meta_conversions_api.pixels%</argument>
+        </service>
+
         <service id="Setono\MetaConversionsApiBundle\Message\Handler\SendEventHandler">
             <argument type="service" id="Setono\MetaConversionsApi\Client\ClientInterface"/>
+            <argument type="service" id="Setono\MetaConversionsApiBundle\AccessTokenResolver\AccessTokenResolverInterface"/>
             <argument type="service" id="logger" on-invalid="null"/>
 
             <tag name="messenger.message_handler"/>

tests/Unit/AccessTokenResolver/ConfigurationBasedAccessTokenResolverTest.php

@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\MetaConversionsApiBundle\Tests\Unit\AccessTokenResolver;
+
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+use Setono\MetaConversionsApiBundle\AccessTokenResolver\ConfigurationBasedAccessTokenResolver;
+
+#[CoversClass(ConfigurationBasedAccessTokenResolver::class)]
+final class ConfigurationBasedAccessTokenResolverTest extends TestCase
+{
+    #[Test]
+    public function it_resolves_a_configured_access_token(): void
+    {
+        $resolver = new ConfigurationBasedAccessTokenResolver([
+            ['id' => '1234', 'access_token' => 's3cr3t'],
+        ]);
+
+        // Pixel ids are numeric strings, which PHP turns into integer array keys. The lookup must survive that
+        self::assertSame('s3cr3t', $resolver->resolve('1234'));
+    }
+
+    #[Test]
+    public function it_returns_null_for_an_unknown_pixel(): void
+    {
+        $resolver = new ConfigurationBasedAccessTokenResolver([
+            ['id' => '1234', 'access_token' => 's3cr3t'],
+        ]);
+
+        self::assertNull($resolver->resolve('4321'));
+    }
+
+    #[Test]
+    public function it_returns_null_for_a_pixel_without_an_access_token(): void
+    {
+        $resolver = new ConfigurationBasedAccessTokenResolver([
+            ['id' => '1234'],
+            ['id' => '4321', 'access_token' => ''],
+            ['id' => '9999', 'access_token' => null],
+        ]);
+
+        self::assertNull($resolver->resolve('1234'));
+        self::assertNull($resolver->resolve('4321'));
+        self::assertNull($resolver->resolve('9999'));
+    }
+}

tests/Unit/EventSubscriber/DispatchOnCommandBusSubscriberTest.php

@@ -34,7 +34,8 @@ public function it_dispatches_the_command(): void
 
         self::assertCount(1, $dispatched);
         self::assertInstanceOf(SendEvent::class, $dispatched[0]);
-        self::assertSame($metaEvent, $dispatched[0]->event);
+        self::assertSame($metaEvent->eventName, $dispatched[0]->eventName);
+        self::assertSame($metaEvent->eventId, $dispatched[0]->eventId);
     }
 
     #[Test]