PHPLIB-664, PHPLIB-676, and PHPLIB-703: APM redaction and observeSensitiveCommands (#848)

* PHPLIB-664 and PHPLIB-676: Sync unified command-monitoring tests

Implements observeSensitiveCommands, which was introduced in unified test format schema version 1.5.

Synced with mongodb/specifications@289f405

* PHPLIB-703: Integration tests for observeSensitiveCommands

Synced with mongodb/specifications@6847048

Co-authored-by: Andreas Braun <[email protected]>
Co-authored-by: Tanli Su <[email protected]>

Author: 3 people

tests/UnifiedSpecTests/Context.php

@@ -248,11 +248,21 @@ static function (string $tag): array {
 
     private function createClient(string $id, stdClass $o): void
     {
-        Util::assertHasOnlyKeys($o, ['id', 'uriOptions', 'useMultipleMongoses', 'observeEvents', 'ignoreCommandMonitoringEvents', 'serverApi', 'storeEventsAsEntities']);
+        Util::assertHasOnlyKeys($o, [
+            'id',
+            'uriOptions',
+            'useMultipleMongoses',
+            'observeEvents',
+            'ignoreCommandMonitoringEvents',
+            'observeSensitiveCommands',
+            'serverApi',
+            'storeEventsAsEntities',
+        ]);
 
         $useMultipleMongoses = $o->useMultipleMongoses ?? null;
         $observeEvents = $o->observeEvents ?? null;
         $ignoreCommandMonitoringEvents = $o->ignoreCommandMonitoringEvents ?? [];
+        $observeSensitiveCommands = $o->observeSensitiveCommands ?? false;
         $serverApi = $o->serverApi ?? null;
         $storeEventsAsEntities = $o->storeEventsAsEntities ?? null;
 
@@ -289,8 +299,9 @@ private function createClient(string $id, stdClass $o): void
         if (isset($observeEvents)) {
             assertIsArray($observeEvents);
             assertIsArray($ignoreCommandMonitoringEvents);
+            assertIsBool($observeSensitiveCommands);
 
-            $this->eventObserversByClient[$id] = new EventObserver($observeEvents, $ignoreCommandMonitoringEvents, $id, $this);
+            $this->eventObserversByClient[$id] = new EventObserver($observeEvents, $ignoreCommandMonitoringEvents, $observeSensitiveCommands, $id, $this);
         }
 
         if (isset($storeEventsAsEntities)) {

tests/UnifiedSpecTests/EventObserver.php

@@ -12,7 +12,6 @@
 use PHPUnit\Framework\Assert;
 use stdClass;
 
-use function array_fill_keys;
 use function array_reverse;
 use function count;
 use function current;
@@ -38,22 +37,36 @@
  */
 final class EventObserver implements CommandSubscriber
 {
-    /** @var array */
-    private static $defaultIgnoreCommands = [
-        // failPoint and targetedFailPoint operations
-        'configureFailPoint',
-        // See: https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
-        'authenticate',
-        'saslStart',
-        'saslContinue',
-        'getnonce',
-        'createUser',
-        'updateUser',
-        'copydbgetnonce',
-        'copydbsaslstart',
-        'copydb',
-        'isMaster',
-        'hello',
+    /**
+     * These commands are always considered sensitive (i.e. command and reply
+     * documents should be redacted).
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
+     * @var array
+     */
+    private static $sensitiveCommands = [
+        'authenticate' => 1,
+        'saslStart' => 1,
+        'saslContinue' => 1,
+        'getnonce' => 1,
+        'createUser' => 1,
+        'updateUser' => 1,
+        'copydbgetnonce' => 1,
+        'copydbsaslstart' => 1,
+        'copydb' => 1,
+    ];
+
+    /**
+     * These commands are only considered sensitive when the command or reply
+     * document includes a speculativeAuthenticate field.
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/command-monitoring/command-monitoring.rst#security
+     * @var array
+     */
+    private static $sensitiveCommandsWithSpeculativeAuthenticate = [
+        'ismaster' => 1,
+        'isMaster' => 1,
+        'hello' => 1,
     ];
 
     /** @var array */
@@ -72,13 +85,21 @@ final class EventObserver implements CommandSubscriber
     /** @var Context */
     private $context;
 
-    /** @var array */
-    private $ignoreCommands = [];
+    /**
+     * The configureFailPoint command (used by failPoint and targetedFailPoint
+     * operations) is always ignored.
+     *
+     * @var array
+     */
+    private $ignoreCommands = ['configureFailPoint' => 1];
 
     /** @var array */
     private $observeEvents = [];
 
-    public function __construct(array $observeEvents, array $ignoreCommands, string $clientId, Context $context)
+    /** @var bool */
+    private $observeSensitiveCommands;
+
+    public function __construct(array $observeEvents, array $ignoreCommands, bool $observeSensitiveCommands, string $clientId, Context $context)
     {
         assertNotEmpty($observeEvents);
 
@@ -88,13 +109,12 @@ public function __construct(array $observeEvents, array $ignoreCommands, string
             $this->observeEvents[self::$supportedEvents[$event]] = 1;
         }
 
-        $this->ignoreCommands = array_fill_keys(self::$defaultIgnoreCommands, 1);
-
         foreach ($ignoreCommands as $command) {
             assertIsString($command);
             $this->ignoreCommands[$command] = 1;
         }
 
+        $this->observeSensitiveCommands = $observeSensitiveCommands;
         $this->clientId = $clientId;
         $this->context = $context;
     }
@@ -270,6 +290,30 @@ private function handleEvent($event): void
             return;
         }
 
+        if (! $this->observeSensitiveCommands && $this->isSensitiveCommand($event)) {
+            return;
+        }
+
         $this->actualEvents[] = $event;
     }
+
+    /** @param CommandStartedEvent|CommandSucceededEvent|CommandFailedEvent $event */
+    private function isSensitiveCommand($event): bool
+    {
+        if (isset(self::$sensitiveCommands[$event->getCommandName()])) {
+            return true;
+        }
+
+        /* If the command or reply included a speculativeAuthenticate field,
+         * libmongoc will already have redacted it (CDRIVER-4000). Therefore, we
+         * can infer that the command was sensitive if its command or reply is
+         * empty. */
+        if (isset(self::$sensitiveCommandsWithSpeculativeAuthenticate[$event->getCommandName()])) {
+            $commandOrReply = $event instanceof CommandStartedEvent ? $event->getCommand() : $event->getReply();
+
+            return (array) $commandOrReply === [];
+        }
+
+        return false;
+    }
 }

tests/UnifiedSpecTests/UnifiedSpecTest.php

@@ -97,6 +97,19 @@ public function provideCollectionManagementTests()
         return $this->provideTests(__DIR__ . '/collection-management/*.json');
     }
 
+    /**
+     * @dataProvider provideCommandMonitoringTests
+     */
+    public function testCommandMonitoring(UnifiedTestCase $test): void
+    {
+        self::$runner->run($test);
+    }
+
+    public function provideCommandMonitoringTests()
+    {
+        return $this->provideTests(__DIR__ . '/command-monitoring/*.json');
+    }
+
     /**
      * @dataProvider provideCrudTests
      */

tests/UnifiedSpecTests/UnifiedTestRunner.php

@@ -46,7 +46,7 @@ final class UnifiedTestRunner
     public const SERVER_ERROR_UNAUTHORIZED = 13;
 
     public const MIN_SCHEMA_VERSION = '1.0';
-    public const MAX_SCHEMA_VERSION = '1.4';
+    public const MAX_SCHEMA_VERSION = '1.5';
 
     /** @var MongoDB\Client */
     private $internalClient;