-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat(NODE-6507): generate encryption configuration on mongoose connect #15320
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
Changes from 12 commits
db8eef7
f1c986c
682fb11
cda0e26
edfbdfa
1a48b98
065bc99
a8f37eb
53d31a6
888c0a8
5d3b51f
2c02d45
ecb3f7c
b7016de
14a40ff
59af7cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ const pkg = require('../../../package.json'); | |
const processConnectionOptions = require('../../helpers/processConnectionOptions'); | ||
const setTimeout = require('../../helpers/timers').setTimeout; | ||
const utils = require('../../utils'); | ||
const Schema = require('../../schema'); | ||
|
||
/** | ||
* A [node-mongodb-native](https://github.com/mongodb/node-mongodb-native) connection implementation. | ||
|
@@ -320,6 +321,20 @@ NativeConnection.prototype.createClient = async function createClient(uri, optio | |
}; | ||
} | ||
|
||
const { schemaMap, encryptedFieldsMap } = this._buildEncryptionSchemas(); | ||
|
||
if ((Object.keys(schemaMap).length > 0 || Object.keys(encryptedFieldsMap).length) && !options.autoEncryption) { | ||
throw new Error('Must provide `autoEncryption` when connecting with encrypted schemas.'); | ||
} | ||
|
||
if (Object.keys(schemaMap).length > 0) { | ||
options.autoEncryption.schemaMap = schemaMap; | ||
} | ||
|
||
if (Object.keys(encryptedFieldsMap).length > 0) { | ||
options.autoEncryption.encryptedFieldsMap = encryptedFieldsMap; | ||
} | ||
|
||
this.readyState = STATES.connecting; | ||
this._connectionString = uri; | ||
|
||
|
@@ -343,6 +358,56 @@ NativeConnection.prototype.createClient = async function createClient(uri, optio | |
return this; | ||
}; | ||
|
||
/** | ||
* Given a connection, which may or may not have encrypted models, build | ||
* a schemaMap and/or an encryptedFieldsMap for the connection, combining all models | ||
* into a single schemaMap and encryptedFields map. | ||
* | ||
* @returns the generated schemaMap and encryptedFieldsMap | ||
*/ | ||
NativeConnection.prototype._buildEncryptionSchemas = function() { | ||
const qeMappings = {}; | ||
const csfleMappings = {}; | ||
|
||
const encryptedModels = Object.values(this.models).filter(model => model.schema._hasEncryptedFields()); | ||
|
||
// If discriminators are configured for the collection, there might be multiple models | ||
// pointing to the same namespace. For this scenario, we merge all the schemas for each namespace | ||
// into a single schema and then generate a schemaMap/encryptedFieldsMap for the combined schema. | ||
for (const model of encryptedModels) { | ||
const { schema, collection: { collectionName } } = model; | ||
const namespace = `${this.$dbName}.${collectionName}`; | ||
const mappings = schema.encryptionType() === 'csfle' ? csfleMappings : qeMappings; | ||
|
||
mappings[namespace] ??= new Schema({}, { encryptionType: schema.encryptionType() }); | ||
|
||
const isNonRootDiscriminator = schema.discriminatorMapping && !schema.discriminatorMapping.isRoot; | ||
if (isNonRootDiscriminator) { | ||
const rootSchema = schema._baseSchema; | ||
schema.eachPath((pathname) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential edge case here: discriminator base schema defines nested path, discriminator child schema defines subdocument with same path but different options. const schema = new Schema({
name: {
first: { type: String, encrypt: { keyId: [keyId], algorithm } }
}
});
const discriminatorSchema = new Schema({
name: new Schema({ first: Number }) // Different type, no encryption, stored as same field in MDB
});
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch - I wrote a test for this scenario and you were right. I've been trying out some different approaches and I can't find a better solution than something like this: function* allPaths(schema, prefix) {
for (const path of Object.keys(schema.paths)) {
const fullPath = prefix != null ? `${prefix}.${path}` : path;
if (schema.path(path).instance === 'Embedded') {
yield* allPaths(schema.path(path).schema, fullPath);
} else {
yield fullPath;
}
}
}
const paths = new Set(allPaths(schema));
for (const path of allPaths(model.schema)) {
if (paths.has(path) && (model.schema._hasEncryptedField(path) || schema._hasEncryptedField(path))) {
throw new Error(`cannot declare an encrypted field on child schema overriding base schema. key=${path}`);
}
} This generates all possible paths in the schema (the above doesn't handle arrays, but encryption on fields in arrays isn't supported so that's out of scope here). Not my first choice, because this feels brittle. I'll keep looking at it but
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think recursively checking all paths is necessary here because you just want to find conflicts in the top-level paths. So if discriminator schema has a path There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily, right? That would throw an error in scenarios where the child schema provides a subdocument with the same root path but doesn't modify the encrypted path. ex: const schema = new Schema({
name: {
first: { type: String, encrypt: { keyId: [keyId], algorithm } }
}
});
const discriminatorSchema = new Schema({
name: new Schema({ age: Number }) // Different path, no encryption
}); I'd expect this to be fine, because there isn't a conflicting path for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case you described, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried the approach of determining conflicting base paths, as you suggested but I ran into two complications:
I decided that the two above points complicated the approach. The changes in this PR were much simpler for me to work with.. This approach works by:
Let me know what you think - I can rework it if you would prefer a different approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, this is a reasonable approach. This approach is more restrictive, but simpler implementation. If it proves to be too restrictive, we can come up with a more flexible approach. |
||
if (rootSchema.path(pathname)) return; | ||
if (!mappings[namespace]._hasEncryptedField(pathname)) return; | ||
|
||
throw new Error(`Cannot have duplicate keys in discriminators with encryption. key=${pathname}`); | ||
}); | ||
} | ||
|
||
mappings[namespace].add(schema); | ||
} | ||
|
||
const schemaMap = Object.fromEntries(Object.entries(csfleMappings).map( | ||
([namespace, schema]) => ([namespace, schema._buildSchemaMap()]) | ||
)); | ||
|
||
const encryptedFieldsMap = Object.fromEntries(Object.entries(qeMappings).map( | ||
([namespace, schema]) => ([namespace, schema._buildEncryptedFields()]) | ||
)); | ||
|
||
return { | ||
schemaMap, encryptedFieldsMap | ||
}; | ||
}; | ||
|
||
/*! | ||
* ignore | ||
*/ | ||
|
Uh oh!
There was an error while loading. Please reload this page.