Skip to content

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Aug 19, 2025

Fix PHPLIB-1708

For some KMS providers, all options can be omitted. The authentication is done using the env var or the system.
https://github.com/mongodb/specifications/blob/master/source/client-side-encryption/client-side-encryption.md#credentialproviders

The approach in doctrine/mongodb-odm#2801 is not sufficient because Symfony is not able to dump the empty object instance into the container.

Unable to dump a service container if a parameter is an object or a resource, got "stdClass".

@GromNaN GromNaN requested review from paulinevos and Copilot August 19, 2025 11:53
@GromNaN GromNaN requested a review from a team as a code owner August 19, 2025 11:53
Copilot

This comment was marked as outdated.

@GromNaN GromNaN force-pushed the PHPLIB-1708 branch 2 times, most recently from 1c52490 to d96aa71 Compare August 19, 2025 12:06

// The server requires an empty document for automatic credentials.
if (isset($options['kmsProviders']) && is_array($options['kmsProviders'])) {
foreach ($options['kmsProviders'] as $name => $provider) {

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type that $provider is being assigned to
@GromNaN
Copy link
Member Author

GromNaN commented Aug 19, 2025

Fixed in Doctrine MongoDB ODM Bundle by setting a DI Definition of stdClass in the parameters instead of an empty stdClass object. doctrine/DoctrineMongoDBBundle@953613d

@@ -37,6 +37,18 @@ public function testConstructorAutoEncryptionOpts(): void
new Client(static::getUri(), [], ['autoEncryption' => $autoEncryptionOpts]);
}

#[DoesNotPerformAssertions]
public function testConstructorEmptyKmsProvider(): void
Copy link
Member

Choose a reason for hiding this comment

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

Would this test have failed without the above patch? If not, is anything being tested at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes:

MongoDB\Driver\Exception\EncryptionException: expected BSON document for field: gcp

@GromNaN GromNaN requested a review from jmikola August 19, 2025 15:20
@GromNaN GromNaN merged commit 21afd09 into mongodb:v1.21 Aug 27, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants