Skip to content

PHPLIB-1702: Always consult server encryptedFieldsMap when dropping collections #1742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: v2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ public function drop(array $options = []): void

if (! isset($options['encryptedFields'])) {
$options['encryptedFields'] = get_encrypted_fields_from_driver($this->databaseName, $this->collectionName, $this->manager)
?? get_encrypted_fields_from_server($this->databaseName, $this->collectionName, $this->manager, $server);
?? get_encrypted_fields_from_server($this->databaseName, $this->collectionName, $server);
}

$operation = isset($options['encryptedFields'])
Expand Down
2 changes: 1 addition & 1 deletion src/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ public function dropCollection(string $collectionName, array $options = []): voi

if (! isset($options['encryptedFields'])) {
$options['encryptedFields'] = get_encrypted_fields_from_driver($this->databaseName, $collectionName, $this->manager)
?? get_encrypted_fields_from_server($this->databaseName, $collectionName, $this->manager, $server);
?? get_encrypted_fields_from_server($this->databaseName, $collectionName, $server);
}

$operation = isset($options['encryptedFields'])
Expand Down
7 changes: 1 addition & 6 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,8 @@ function get_encrypted_fields_from_driver(string $databaseName, string $collecti
* @see Collection::drop()
* @see Database::dropCollection()
*/
function get_encrypted_fields_from_server(string $databaseName, string $collectionName, Manager $manager, Server $server): array|object|null
function get_encrypted_fields_from_server(string $databaseName, string $collectionName, Server $server): array|object|null
{
// No-op if the encryptedFieldsMap autoEncryption driver option was omitted
if ($manager->getEncryptedFieldsMap() === null) {
return null;
}

$collectionInfoIterator = (new ListCollections($databaseName, ['filter' => ['name' => $collectionName]]))->execute($server);

foreach ($collectionInfoIterator as $collectionInfo) {
Expand Down
64 changes: 64 additions & 0 deletions tests/Collection/DropEncryptedCollectionFunctionalTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

namespace MongoDB\Tests\Collection;

use MongoDB\BSON\Binary;
use MongoDB\Database;
use MongoDB\Driver\ClientEncryption;
use MongoDB\Driver\WriteConcern;

use function iterator_count;
use function str_repeat;

class DropEncryptedCollectionFunctionalTest extends FunctionalTestCase
{
protected ClientEncryption $clientEncryption;
protected Database $database;

public function setUp(): void
{
parent::setUp();

$this->skipIfClientSideEncryptionIsNotSupported();

if ($this->isStandalone()) {
$this->markTestSkipped('Queryable encryption requires replica sets');
}

$this->skipIfServerVersion('<', '7.0.0', 'Queryable encryption requires MongoDB 7.0 or later');

$client = static::createTestClient();

// Ensure the key vault collection is dropped before each test
$collection = $client->selectCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll get used to the new name of this method.

Suggested change
$collection = $client->selectCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);
$collection = $client->getCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);

Why do you need to set the writeConcern, as that doesn't guarantee that all replica are up to date.

$collection->drop();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also drop this collection in tearDown.


$this->clientEncryption = $client->createClientEncryption([
'keyVaultNamespace' => 'keyvault.datakeys',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent any typo:

Suggested change
'keyVaultNamespace' => 'keyvault.datakeys',
'keyVaultNamespace' => $collection->getNamespace(),

Otherwise you don't really need the $collection variable.

'kmsProviders' => ['local' => ['key' => new Binary(str_repeat("\0", 96)) ]],
]);

$this->database = $client->getDatabase($this->getDatabaseName());
}

/** @see https://jira.mongodb.org/browse/PHPLIB-1702 */
public function testDropConsultsEncryptedFieldsFromServer(): void
{
$originalNumCollections = iterator_count($this->database->listCollectionNames());

$this->database->createEncryptedCollection(
$this->getCollectionName(),
$this->clientEncryption,
'local',
null,
['encryptedFields' => ['fields' => []]],
);

// createEncryptedCollection should create three collections
$this->assertCount($originalNumCollections + 3, $this->database->listCollectionNames());

$this->collection->drop();

$this->assertCount($originalNumCollections, $this->database->listCollectionNames());
}
}
60 changes: 60 additions & 0 deletions tests/Database/DropEncryptedCollectionFunctionalTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

namespace MongoDB\Tests\Database;

use MongoDB\BSON\Binary;
use MongoDB\Driver\ClientEncryption;
use MongoDB\Driver\WriteConcern;

use function iterator_count;
use function str_repeat;

class DropEncryptedCollectionFunctionalTest extends FunctionalTestCase
{
protected ClientEncryption $clientEncryption;

public function setUp(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both test classes has the same setUp method. They should probably be a single test class with 2 test methods.

{
parent::setUp();

$this->skipIfClientSideEncryptionIsNotSupported();

if ($this->isStandalone()) {
$this->markTestSkipped('Queryable encryption requires replica sets');
}

$this->skipIfServerVersion('<', '7.0.0', 'Queryable encryption requires MongoDB 7.0 or later');

$client = static::createTestClient();

// Ensure the key vault collection is dropped before each test
$collection = $client->selectCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$collection = $client->selectCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);
$collection = $client->getCollection('keyvault', 'datakeys', ['writeConcern' => new WriteConcern(WriteConcern::MAJORITY)]);

$collection->drop();

$this->clientEncryption = $client->createClientEncryption([
'keyVaultNamespace' => 'keyvault.datakeys',
'kmsProviders' => ['local' => ['key' => new Binary(str_repeat("\0", 96)) ]],
]);
}

/** @see https://jira.mongodb.org/browse/PHPLIB-1702 */
public function testDropCollectionConsultsEncryptedFieldsFromServer(): void
{
$originalNumCollections = iterator_count($this->database->listCollectionNames());

$this->database->createEncryptedCollection(
$this->getCollectionName(),
$this->clientEncryption,
'local',
null,
['encryptedFields' => ['fields' => []]],
);

// createEncryptedCollection should create three collections
$this->assertCount($originalNumCollections + 3, $this->database->listCollectionNames());

$this->database->dropCollection($this->getCollectionName());

$this->assertCount($originalNumCollections, $this->database->listCollectionNames());
}
}
Loading