Enable auto encryption #897
Conversation
GromNaN
changed the title
autoEncryption configuration to the client (#889)
* Update required version of mongodb driver and odm * Update minimum phpunit version to support readonly properties in nikic/php-parser
Co-authored-by: Andreas Braun <git@alcaeus.org>
…on` configuration (#905) * Fix format of encryptedFieldsMaps in the configuration * EncryptedFieldsMaps loaded from a JSON string from XML configuration * Enable client configuration for tests - partial
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
f1d8d87 to
8a8bdd4
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enables auto encryption support for MongoDB ODM by integrating MongoDB's Client-Side Field-Level Encryption (CSFLE) and Queryable Encryption (QE) features into DoctrineMongoDBBundle. The implementation allows developers to configure encryption settings at the connection level and provides diagnostic tools for troubleshooting encryption configurations.
Key changes include:
- Added comprehensive auto encryption configuration options to the bundle's configuration system
- Implemented diagnostic commands to check encryption support and configuration status
- Enhanced test infrastructure to support MongoDB connections with configurable URIs
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TestCase.php | Updated test infrastructure to use configurable MongoDB connection URI |
| tests/DependencyInjection/Fixtures/config/yml/full.yml | Added comprehensive auto encryption configuration examples in YAML format |
| tests/DependencyInjection/Fixtures/config/xml/full.xml | Added auto encryption configuration examples in XML format with CDATA support |
| tests/DependencyInjection/DoctrineMongoDBExtensionTest.php | Added extensive test coverage for auto encryption configuration scenarios |
| tests/DependencyInjection/ConfigurationTest.php | Enhanced configuration tests with auto encryption validation and JSON parsing support |
| tests/Command/ConnectionDiagnosticCommandTest.php | Added tests for the new diagnostic command functionality |
| src/DependencyInjection/DoctrineMongoDBExtension.php | Core implementation of auto encryption configuration processing and service injection |
| src/DependencyInjection/Configuration.php | Added comprehensive auto encryption configuration schema with validation |
| src/DataCollector/EncryptionDiagnostic.php | Implemented encryption environment diagnostics for PHP extension and mongocryptd |
| src/DataCollector/ConnectionDiagnostic.php | Implemented connection-specific encryption diagnostics and server compatibility checks |
| src/Command/DumpEncryptedFieldsMapCommand.php | Added command to dump encrypted fields mapping configuration |
| src/Command/ConnectionDiagnosticCommand.php | Added command to diagnose MongoDB encryption configuration and capabilities |
| phpunit.xml.dist | Added environment variable for configurable MongoDB server URI |
| phpstan.neon.dist | Added type analysis configuration for ClassMetadata generics |
| phpstan-baseline.neon | Updated static analysis baseline with new code patterns |
| docs/encryption.rst | Added comprehensive documentation for encryption configuration and usage |
| docs/config.rst | Updated configuration documentation with auto encryption options |
| config/schema/mongodb-1.0.xsd | Added XML schema definitions for auto encryption configuration |
| config/command.php | Registered new diagnostic and dump commands with dependency injection |
| composer.json | Updated MongoDB extension and ODM dependencies for encryption support |
| .github/workflows/*.yml | Updated CI workflows to support feature branches and new driver version |
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
2aa0d81 to
61a8118
Compare
* Add keyVaultClient to XML configuration * Require 1 kms provider in the XML config * Remove tls attributes from kms-provider node * Add cryptSharedLibRequired to XSD and test * Clean tlsOptions and wrap into an array with the kms provider name as key * Add comment for duplicate options
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
81a7ddb to
953613d
Compare
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
3372f85 to
7028b0e
Compare
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
2fe6f34 to
c49d20d
Compare
| $providerOpts = new Definition('stdClass'); | ||
| } | ||
|
|
||
| $autoEncryption['kmsProviders'] = [$provider => $providerOpts]; |
There was a problem hiding this comment.
Why does this assign a kmsProviders key, which is later removed by the array_diff_key() in loadDocumentManager() (when calling normalizeAutoEncryption())?
In the calling context, $authEncryption['kmsProvider'] still gets used to queue up a call to setKmsProvider(), but it doesn't look like any of the normalization treatment above will affect that.
Please let me know if I'm overlooking something.
There was a problem hiding this comment.
Good catch, we don't need to set the kmsProviders here.
The method normalizeAutoEncryption is also used to generate driverOptions that are injected into the client constructor. We cannot use the Configuration class to get the driver options because the Client connection is independent from the document manager and its configuration.
There was a problem hiding this comment.
In that case, shouldn't the normalizeDriverOptions calling context use its own array_diff_key() to unset the kmsProvider key specific to ODM? Otherwise, I think you're passing an unrecognized option to PHPLIB and, ultimately, PHPC. It'd be silently ignored now but could become a possible low-level log message down the line.
There was a problem hiding this comment.
I had the idea that we could use the ODMConfiguration class at build time to generate the driverOptions. But that would not work very well with environment variables. And would increase the required version of doctrine/mongodb-odm.
| 'autoEncryption' => [ | ||
| 'keyVaultNamespace' => 'db.vault', | ||
| 'keyVaultClient' => $dummyServiceId, | ||
| 'kmsProvider' => ['type' => 'local', 'key' => 'cGFzc3dvcmQ='], |
There was a problem hiding this comment.
cGFzc3dvcmQ= is "password" after base64 decoding. Should this actually be a 96-byte string? I suppose it doesn't really matter since you're not validating that in the configuration. And I assume that's intentional and you're relying on the server to complain (not that it matters for this unit test).
There was a problem hiding this comment.
@GromNaN: Feel free to resolve if my assumption above is correct.
There was a problem hiding this comment.
Yes, that's just a random string (actually provided by copilot).
There was a problem hiding this comment.
(actually provided by copilot)
In that case I'm going to assume it was actually leaked credentials from another repository that snuck into the Copilot training data. 😆
…lds map (#915) * Fix query option min/max type for exported encrypted fields map * Type 'decimal' is not a supported equality indexed type
There was a problem hiding this comment.
A few questions, but you can certainly address those on your own (or rule out my concerns). I do think some XSD changes might be required for masterKey, though.
Other than that, note that there are still two unchecked TODOs in the PR description. I noted #902 is deferred. The other to bump composer.json might already have been done, and just needs checking.
… decimal range min/max
GromNaN
force-pushed
the
feature/queryable-encryption
branch
from
3ec0793 to
f13fde6
Compare
Integration for the Queryable Encryption feature of MongoDB ODM: doctrine/mongodb-odm#2779
autoEncryptionconfiguration to the client #889masterKeyandkmsProvidername into the ODM configuration for the Schema Manager. [Encryption] Inject auto encryption options into ODM configuration #901[Encryption] Add a command to download cryptshared #902encryptedFieldsMapsin theautoEncryptionconfiguration #905doctrine/mongodb-odm: dev-feature/queryable-encryptionbranch