Merge pull request #15404 from mongodb-js/csfle-review-comments

chore: address comments on CSFLE feature branch review

Author: vkarpov15

docs/field-level-encryption.md

@@ -164,9 +164,9 @@ const UserModel = connection.model('User', encryptedUserSchema);
 
 ### Connecting and configuring encryption options
 
-CSFLE/QE in Mongoose work by generating the encryption schema that the MongoDB driver expects for each encrypted model on the connection.  This happens automatically the model's connection is established.
+CSFLE/QE in Mongoose work by generating the encryption schema that the MongoDB driver expects for each encrypted model on the connection.  This happens automatically when the model's connection is established.
 
-Queryable encryption and CSFLE requires all the same configuration as outlined in <>, except for the schemaMap or encryptedFieldsMap options.
+Queryable encryption and CSFLE requires all the same configuration as outlined in the [MongoDB encryption in-use documentation](https://www.mongodb.com/docs/manual/core/security-in-use-encryption/), except for the schemaMap or encryptedFieldsMap options.
 
 ```javascript
 const keyVaultNamespace = 'client.encryption';
@@ -215,7 +215,7 @@ const ModelWithBirthday = model.discriminator('ModelWithBirthday', new Schema({
 }));
 ```
 
-When generating encryption schemas, Mongoose merges all discriminators together for the all discriminators declared on the same namespace.  As a result, discriminators that declare the same key with different types are not supported.  Furthermore, all discriminators must share the same encryption type - it is not possible to configure discriminators on the same model for both CSFLE and QE.
+When generating encryption schemas, Mongoose merges all discriminators together for all of the discriminators declared on the same namespace.  As a result, discriminators that declare the same key with different types are not supported.  Furthermore, all discriminators must share the same encryption type - it is not possible to configure discriminators on the same model for both CSFLE and QE.
 
 ## Managing Data Keys
 

lib/model.js

@@ -4879,6 +4879,12 @@ Model.compile = function compile(name, schema, collectionName, connection, base)
   return model;
 };
 
+/**
+ * If auto encryption is enabled, returns a ClientEncryption instance that is configured with the same settings that
+ * Mongoose's underlying MongoClient is using.  If the client has not yet been configured, returns null.
+ *
+ * @returns {ClientEncryption | null}
+ */
 Model.clientEncryption = function clientEncryption() {
   const ClientEncryption = this.base.driver.get().ClientEncryption;
   if (!ClientEncryption) {

lib/schema.js

@@ -499,7 +499,7 @@ Schema.prototype.pick = function(paths, options) {
   }
 
   for (const path of paths) {
-    if (path in this.encryptedFields) {
+    if (this._hasEncryptedField(path)) {
       const encrypt = this.encryptedFields[path];
       const schemaType = this.path(path);
       newSchema.add({
@@ -508,8 +508,7 @@ Schema.prototype.pick = function(paths, options) {
           [this.options.typeKey]: schemaType
         }
       });
-    }
-    else if (this.nested[path]) {
+    } else if (this.nested[path]) {
       newSchema.add({ [path]: get(this.tree, path) });
     } else {
       const schematype = this.path(path);
@@ -689,17 +688,16 @@ Schema.prototype._defaultToObjectOptions = function(json) {
  * Sets the encryption type of the schema, if a value is provided, otherwise
  * returns the encryption type.
  *
- * @param {'csfle' | 'queryableEncryption' | undefined} encryptionType plain object with paths to add, or another schema
+ * @param {'csfle' | 'queryableEncryption' | null | undefined} encryptionType plain object with paths to add, or another schema
  */
 Schema.prototype.encryptionType = function encryptionType(encryptionType) {
   if (arguments.length === 0) {
     return this.options.encryptionType;
   }
-  if (typeof encryptionType === 'string' || encryptionType === null) {
-    this.options.encryptionType = encryptionType;
-  } else {
-    throw new TypeError('Invalid encryptionType. Expected a string, null, or no arguments.');
+  if (!(typeof encryptionType === 'string' || encryptionType === null)) {
+    throw new Error('invalid `encryptionType`: ${encryptionType}');
   }
+  this.options.encryptionType = encryptionType;
 };
 
 /**
@@ -863,8 +861,7 @@ Schema.prototype.add = function add(obj, prefix) {
         const path = fullPath + '.' + encryptedField;
         this._addEncryptedField(path, encryptedFieldConfig);
       }
-    }
-    else if (typeof val === 'object' && 'encrypt' in val) {
+    } else if (typeof val === 'object' && 'encrypt' in val) {
       // schema.add({ field: { type: <schema type>, encrypt: { ... }}})
       const { encrypt } = val;
 
@@ -903,6 +900,8 @@ Schema.prototype._addEncryptedField = function _addEncryptedField(path, fieldCon
 };
 
 /**
+ * @param {string} path
+ *
  * @api private
  */
 Schema.prototype._removeEncryptedField = function _removeEncryptedField(path) {
@@ -911,12 +910,17 @@ Schema.prototype._removeEncryptedField = function _removeEncryptedField(path) {
 
 /**
  * @api private
+ *
+ * @returns {boolean}
  */
 Schema.prototype._hasEncryptedFields = function _hasEncryptedFields() {
   return Object.keys(this.encryptedFields).length > 0;
 };
 
 /**
+ * @param {string} path
+ * @returns {boolean}
+ *
  * @api private
  */
 Schema.prototype._hasEncryptedField = function _hasEncryptedField(path) {
@@ -926,6 +930,8 @@ Schema.prototype._hasEncryptedField = function _hasEncryptedField(path) {
 
 /**
  * Builds an encryptedFieldsMap for the schema.
+ *
+ * @api private
  */
 Schema.prototype._buildEncryptedFields = function() {
   const fields = Object.entries(this.encryptedFields).map(
@@ -940,6 +946,8 @@ Schema.prototype._buildEncryptedFields = function() {
 
 /**
  * Builds a schemaMap for the schema, if the schema is configured for client-side field level encryption.
+ *
+ * @api private
  */
 Schema.prototype._buildSchemaMap = function() {
   /**
@@ -950,7 +958,7 @@ Schema.prototype._buildSchemaMap = function() {
    * `{ a: { bsonType: 'object', properties: { b: < encryption configuration > } } }`
    *
    * This function takes an array of path segments, an output object (that gets mutated) and
-   * a value to associated with the full path, and constructs a valid CSFLE JSON schema path for
+   * a value to be associated with the full path, and constructs a valid CSFLE JSON schema path for
    * the object.  This works for deeply nested properties as well.
    *
    * @param {string[]} path array of path components

lib/schema/uuid.js

@@ -298,6 +298,10 @@ SchemaUUID.prototype.toJSONSchema = function toJSONSchema(options) {
   return createJSONSchemaTypeDefinition('string', 'binData', options?.useBsonType, isRequired);
 };
 
+SchemaUUID.prototype.autoEncryptionType = function autoEncryptionType() {
+  return 'binData';
+};
+
 /*!
  * Module exports.
  */

package.json

@@ -148,4 +148,5 @@
       "target": "ES2017"
     }
   }
-}
+}
+

scripts/setup-encryption-tests.js

@@ -1,11 +1,11 @@
 'use strict';
+
 const { downloadMongoDb } = require('@mongodb-js/mongodb-downloader');
 const { instances, start } = require('mongodb-runner');
 const { rm, readdir, writeFile } = require('fs/promises');
 const { tmpdir } = require('os');
 const { join, resolve } = require('path');
 
-
 async function main() {
   const runnerDir = join(resolve(__dirname), '../data');
   const serverVersion = '8.0';
@@ -38,12 +38,14 @@ async function main() {
       'sharded', runnerDir, tmpDir: tmpdir() });
 
     for await (const instance of instances({ runnerDir })) {
-      if (instance.id === 'encryption-test-cluster') return {
-        cryptShared, uri: instance.connectionString
-      };
+      if (instance.id === 'encryption-test-cluster') {
+        return {
+          cryptShared, uri: instance.connectionString
+        };
+      }
     }
 
-    throw new Error('Unable to location newly configured instance of mongod - should never happen.');
+    throw new Error('Unable to locate newly configured instance of mongod - should never happen.');
   }
 }
 

test/encryptedSchema.test.js

@@ -1,4 +1,3 @@
-
 'use strict';
 
 const assert = require('assert');
@@ -13,7 +12,7 @@ const Schema = mongoose.Schema;
  *
  * @param {import('../lib').Schema} object
  * @param {Array<string> | string} path
- * @returns
+ * @returns { boolean }
  */
 function schemaHasEncryptedProperty(schema, path) {
   path = [path].flat();
@@ -240,7 +239,7 @@ describe('encrypted schema declaration', function() {
       });
     });
 
-    describe('When a schema is instantiated with a custom schema type plugin', function() {
+    describe('When a schema is instantiated with a custom schema type plugin that does not have a encryption type', function() {
       class Int8 extends mongoose.SchemaType {
         constructor(key, options) {
           super(key, options, 'Int8');
@@ -266,6 +265,33 @@ describe('encrypted schema declaration', function() {
       });
     });
 
+    describe('When a schema is instantiated with a custom schema type plugin that does have a encryption type', function() {
+      class Int8 extends mongoose.SchemaType {
+        constructor(key, options) {
+          super(key, options, 'Int8');
+        }
+
+        autoEncryptionType() {
+          return 'int';
+        }
+      }
+
+      beforeEach(function() {
+        // Don't forget to add `Int8` to the type registry
+        mongoose.Schema.Types.Int8 = Int8;
+      });
+      afterEach(function() {
+        delete mongoose.Schema.Types.Int8;
+      });
+
+      it('No error is thrown', function() {
+        new Schema({
+          field: {
+            type: Int8, encrypt: { keyId: KEY_ID, algorithm }
+          }
+        }, { encryptionType: 'csfle' });
+      });
+    });
   });
 
   describe('options.encryptionType', function() {
@@ -746,6 +772,58 @@ function primitiveSchemaMapTests() {
         ]
       }
     },
+    {
+      name: 'uuid',
+      encryptionType: 'csfle',
+      type: Schema.Types.UUID,
+      schemaMap: {
+        bsonType: 'object',
+        properties: {
+          field: {
+            encrypt: {
+              keyId: '9fbdace3-4e48-412d-88df-3807e8009522',
+              algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic',
+              bsonType: 'binData'
+            }
+          }
+        }
+      },
+      encryptedFields: {
+        fields: [
+          {
+            path: 'field',
+            bsonType: 'binData',
+            keyId: '9fbdace3-4e48-412d-88df-3807e8009522',
+            algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic'
+          }
+        ]
+      }
+    },
+    {
+      name: 'uuid',
+      encryptionType: 'queryableEncryption',
+      type: Schema.Types.UUID,
+      schemaMap: {
+        bsonType: 'object',
+        properties: {
+          field: {
+            encrypt: {
+              keyId: '9fbdace3-4e48-412d-88df-3807e8009522',
+              bsonType: 'binData'
+            }
+          }
+        }
+      },
+      encryptedFields: {
+        fields: [
+          {
+            path: 'field',
+            bsonType: 'binData',
+            keyId: '9fbdace3-4e48-412d-88df-3807e8009522'
+          }
+        ]
+      }
+    },
     {
       name: 'buffer',
       encryptionType: 'csfle',