Skip to content

feat(NODE-4986)!: remove callbacks from ClientEncryption encrypt, decrypt, and createDataKey #3797

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

Merged
merged 8 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 0 additions & 2 deletions .evergreen/run-custom-csfle-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ bash ./etc/build-static.sh

npm run rebuild # just in case this is necessary?

ls
ls lib
BINDINGS_DIR=$(pwd)
popd # libmongocrypt/bindings/node
popd # ../csfle-deps-tmp
Expand Down
3 changes: 2 additions & 1 deletion src/bson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ export {
MinKey,
ObjectId,
serialize,
Timestamp
Timestamp,
UUID
} from 'bson';

/**
Expand Down
116 changes: 22 additions & 94 deletions src/client-side-encryption/client_encryption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {
MongoCryptOptions
} from 'mongodb-client-encryption';

import { type Binary, type Document, type Long, serialize } from '../bson';
import { type Binary, type Document, type Long, serialize, type UUID } from '../bson';
import { type AnyBulkWriteOperation, type BulkWriteResult } from '../bulk/common';
import { type ProxyOptions } from '../cmap/connection';
import { type Collection } from '../collection';
Expand All @@ -16,8 +16,7 @@ import { type MongoClient } from '../mongo_client';
import { type Filter } from '../mongo_types';
import { type CreateCollectionOptions } from '../operations/create_collection';
import { type DeleteResult } from '../operations/delete';
import { type Callback, MongoDBCollectionNamespace } from '../utils';
import { maybeCallback, promiseOrCallback } from './common';
import { MongoDBCollectionNamespace } from '../utils';
import * as cryptoCallbacks from './crypto_callbacks';
import {
MongoCryptCreateDataKeyError,
Expand All @@ -35,7 +34,7 @@ import {
* The schema for a DataKey in the key vault collection.
*/
export interface DataKey {
_id: Binary;
_id: UUID;
version?: number;
keyAltNames?: string[];
keyMaterial: Binary;
Expand Down Expand Up @@ -125,18 +124,6 @@ export class ClientEncryption implements StateMachineExecutable {
*
* @example
* ```ts
* // Using callbacks to create a local key
* clientEncryption.createDataKey('local', (err, dataKey) => {
* if (err) {
* // This means creating the key failed.
* } else {
* // key creation succeeded
* }
* });
* ```
*
* @example
* ```ts
* // Using async/await to create a local key
* const dataKeyId = await clientEncryption.createDataKey('local');
* ```
Expand Down Expand Up @@ -164,19 +151,10 @@ export class ClientEncryption implements StateMachineExecutable {
* });
* ```
*/
createDataKey(
async createDataKey(
provider: KMSProvider,
options?: ClientEncryptionCreateDataKeyProviderOptions,
callback?: Callback<DataKey>
) {
if (typeof options === 'function') {
callback = options;
options = {};
}
if (options == null) {
options = {};
}

options: ClientEncryptionCreateDataKeyProviderOptions = {}
): Promise<UUID> {
const dataKey = Object.assign({ provider }, options.masterKey);

if (options.keyAltNames && !Array.isArray(options.keyAltNames)) {
Expand Down Expand Up @@ -213,32 +191,17 @@ export class ClientEncryption implements StateMachineExecutable {
tlsOptions: this._tlsOptions
});

// @ts-expect-error We did not convert promiseOrCallback to TS
return promiseOrCallback(callback, cb => {
stateMachine.execute<DataKey>(this, context, (err, dataKey) => {
if (err || !dataKey) {
cb(err, null);
return;
}
const updatedDataKey = await stateMachine.executeAsync<DataKey>(this, context);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to choose another name here because we were shadowing. Up for any suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just remove the first declaration and name this dataKey? it's not proving much value imo and it actually makes this more confusing, because the data key is created by stateMachine.execute.

    const dataKeyBson = serialize({ provider, masterKey: options.masterKey });


const { db: dbName, collection: collectionName } = MongoDBCollectionNamespace.fromString(
this._keyVaultNamespace
);
const { db: dbName, collection: collectionName } = MongoDBCollectionNamespace.fromString(
this._keyVaultNamespace
);

this._keyVaultClient
.db(dbName)
.collection<DataKey>(collectionName)
.insertOne(dataKey, { writeConcern: { w: 'majority' } })
.then(
result => {
return cb(null, result.insertedId);
},
err => {
cb(err, null);
}
);
});
});
const { insertedId } = await this._keyVaultClient
.db(dbName)
.collection<DataKey>(collectionName)
.insertOne(updatedDataKey, { writeConcern: { w: 'majority' } });
return insertedId;
}

/**
Expand Down Expand Up @@ -590,21 +553,7 @@ export class ClientEncryption implements StateMachineExecutable {
*
* @param value - The value that you wish to serialize. Must be of a type that can be serialized into BSON
* @param options -
* @param callback - Optional callback to invoke when value is encrypted
* @returns If no callback is provided, returns a Promise that either resolves with the encrypted value, or rejects with an error. If a callback is provided, returns nothing.
*
* @example
* ```ts
* // Encryption with callback API
* function encryptMyData(value, callback) {
* clientEncryption.createDataKey('local', (err, keyId) => {
* if (err) {
* return callback(err);
* }
* clientEncryption.encrypt(value, { keyId, algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic' }, callback);
* });
* }
* ```
* @returns a Promise that either resolves with the encrypted value, or rejects with an error.
*
* @example
* ```ts
Expand All @@ -624,12 +573,8 @@ export class ClientEncryption implements StateMachineExecutable {
* }
* ```
*/
encrypt(
value: unknown,
options: ClientEncryptionEncryptOptions,
callback: Callback<Binary>
): Promise<Binary> | void {
return maybeCallback(() => this._encrypt(value, false, options), callback);
async encrypt(value: unknown, options: ClientEncryptionEncryptOptions): Promise<Binary> {
return this._encrypt(value, false, options);
}

/**
Expand Down Expand Up @@ -661,16 +606,7 @@ export class ClientEncryption implements StateMachineExecutable {
* Explicitly decrypt a provided encrypted value
*
* @param value - An encrypted value
* @param callback - Optional callback to invoke when value is decrypted
* @returns If no callback is provided, returns a Promise that either resolves with the decrypted value, or rejects with an error. If a callback is provided, returns nothing.
*
* ```ts
* @example
* // Decrypting value with callback API
* function decryptMyValue(value, callback) {
* clientEncryption.decrypt(value, callback);
* }
* ```
* @returns a Promise that either resolves with the decrypted value, or rejects with an error
*
* @example
* ```ts
Expand All @@ -680,7 +616,7 @@ export class ClientEncryption implements StateMachineExecutable {
* }
* ```
*/
decrypt<T = any>(value: Binary, callback?: Callback<T>): Promise<T> | void {
async decrypt<T = any>(value: Binary): Promise<T> {
const valueBuffer = serialize({ v: value });
const context = this._mongoCrypt.makeExplicitDecryptionContext(valueBuffer);

Expand All @@ -689,17 +625,9 @@ export class ClientEncryption implements StateMachineExecutable {
tlsOptions: this._tlsOptions
});

// @ts-expect-error We did not convert promiseOrCallback to TS
return promiseOrCallback(callback, cb => {
stateMachine.execute<{ v: T }>(this, context, (err, result) => {
if (err || !result) {
cb(err, null);
return;
}
const { v } = await stateMachine.executeAsync<{ v: T }>(this, context);

cb(null, result.v);
});
});
return v;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export {
MaxKey,
MinKey,
ObjectId,
Timestamp
Timestamp,
UUID
} from './bson';
export { AnyBulkWriteOperation, BulkWriteOptions, MongoBulkWriteError } from './bulk/common';
export { ChangeStreamCursor } from './cursor/change_stream_cursor';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as BSON from 'bson';
import { expect } from 'chai';
import { readFileSync } from 'fs';
import * as path from 'path';
import * as util from 'util';

// eslint-disable-next-line @typescript-eslint/no-restricted-imports
import { ClientEncryption } from '../../../src/client-side-encryption/client_encryption';
Expand Down Expand Up @@ -138,11 +137,8 @@ describe('Connection Pool Deadlock Prevention', function () {
keyVaultClient: this.keyVaultClient,
extraOptions: getEncryptExtraOptions()
});
this.clientEncryption.encryptPromisified = util.promisify(
this.clientEncryption.encrypt.bind(this.clientEncryption)
);

this.ciphertext = await this.clientEncryption.encryptPromisified('string0', {
this.ciphertext = await this.clientEncryption.encrypt('string0', {
algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic',
keyAltName: 'local'
});
Expand Down
55 changes: 34 additions & 21 deletions test/integration/node-specific/client_encryption.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import { MongoCryptInvalidArgumentError } from '../../../src/client-side-encryption/errors';
/* eslint-disable @typescript-eslint/no-restricted-imports */
import { StateMachine } from '../../../src/client-side-encryption/state_machine';
import { Binary, type Collection, Int32, Long, type MongoClient } from '../../mongodb';
import { Binary, type Collection, Int32, Long, type MongoClient, UUID } from '../../mongodb';

function readHttpResponse(path) {
let data = readFileSync(path, 'utf8').toString();
Expand Down Expand Up @@ -160,7 +160,7 @@ describe('ClientEncryption integration tests', function () {
}
);

it('should fail to create a data key if keyMaterial is wrong', metadata, function (done) {
it('should fail to create a data key if keyMaterial is wrong', metadata, async function () {
const encryption = new ClientEncryption(client, {
keyVaultNamespace: 'client.encryption',
kmsProviders: { local: { key: 'A'.repeat(128) } }
Expand All @@ -169,13 +169,8 @@ describe('ClientEncryption integration tests', function () {
const dataKeyOptions = {
keyMaterial: new Binary(Buffer.alloc(97))
};
try {
encryption.createDataKey('local', dataKeyOptions);
expect.fail('missed exception');
} catch (err) {
expect(err.message).to.equal('keyMaterial should have length 96, but has length 97');
done();
}
const error = await encryption.createDataKey('local', dataKeyOptions).catch(error => error);
expect(error.message).to.equal('keyMaterial should have length 96, but has length 97');
});

it(
Expand Down Expand Up @@ -496,6 +491,26 @@ describe('ClientEncryption integration tests', function () {
});
});

describe('createDataKey()', () => {
let clientEncryption;

beforeEach(function () {
clientEncryption = new ClientEncryption(client, {
keyVaultNamespace: 'client.encryption',
kmsProviders: { local: { key: Buffer.alloc(96) } }
});
});

it('returns a UUID instance', async () => {
const dataKey = await clientEncryption.createDataKey('local', {
name: 'local',
kmsProviders: { local: { key: Buffer.alloc(96) } }
});

expect(dataKey).to.be.instanceOf(UUID);
});
});

describe('ClientEncryptionKeyAltNames', function () {
let client: MongoClient;
let clientEncryption: ClientEncryption;
Expand Down Expand Up @@ -539,23 +554,21 @@ describe('ClientEncryption integration tests', function () {
}

describe('errors', function () {
[42, 'hello', { keyAltNames: 'foobar' }, /foobar/].forEach(val => {
it(`should fail if typeof keyAltNames = ${typeof val}`, metadata, function () {
for (const val of [42, 'hello', { keyAltNames: 'foobar' }, /foobar/]) {
it(`should fail if typeof keyAltNames = ${typeof val}`, metadata, async function () {
const options = makeOptions(val);
expect(() => clientEncryption.createDataKey('aws', options, () => undefined)).to.throw(
MongoCryptInvalidArgumentError
);
const error = await clientEncryption.createDataKey('aws', options).catch(error => error);
expect(error).to.be.instanceOf(MongoCryptInvalidArgumentError);
});
});
}

[undefined, null, 42, { keyAltNames: 'foobar' }, ['foobar'], /foobar/].forEach(val => {
it(`should fail if typeof keyAltNames[x] = ${typeof val}`, metadata, function () {
for (const val of [undefined, null, 42, { keyAltNames: 'foobar' }, ['foobar'], /foobar/]) {
it(`should fail if typeof keyAltNames[x] = ${typeof val}`, metadata, async function () {
const options = makeOptions([val]);
expect(() => clientEncryption.createDataKey('aws', options, () => undefined)).to.throw(
MongoCryptInvalidArgumentError
);
const error = await clientEncryption.createDataKey('aws', options).catch(error => error);
expect(error).to.be.instanceOf(MongoCryptInvalidArgumentError);
});
});
}
});

it('should create a key with keyAltNames', metadata, async function () {
Expand Down
Loading