Skip to content
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

CSHARP-4255: Fix bug and some tests. #993

Merged
merged 6 commits into from Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/MongoDB.Driver/CreateCollectionOptions.cs
Expand Up @@ -196,6 +196,27 @@ public TimeSeriesOptions TimeSeriesOptions
get { return _validationLevel; }
set { _validationLevel = value; }
}

internal virtual CreateCollectionOptions Clone() =>
new CreateCollectionOptions
{
_autoIndexId = _autoIndexId,
_capped = _capped,
_changeStreamPreAndPostImagesOptions = _changeStreamPreAndPostImagesOptions,
_collation = _collation,
_encryptedFields = _encryptedFields,
_expireAfter = _expireAfter,
_indexOptionDefaults = _indexOptionDefaults,
_maxDocuments = _maxDocuments,
_maxSize = _maxSize,
_noPadding = _noPadding,
_serializerRegistry = _serializerRegistry,
_storageEngine = _storageEngine,
_timeSeriesOptions = _timeSeriesOptions,
_usePowerOf2Sizes = _usePowerOf2Sizes,
_validationAction = _validationAction,
_validationLevel = _validationLevel
};
}

/// <summary>
Expand Down Expand Up @@ -282,5 +303,32 @@ public FilterDefinition<TDocument> Validator
get { return _validator; }
set { _validator = value; }
}

internal override CreateCollectionOptions Clone() =>
new CreateCollectionOptions<TDocument>
{
#pragma warning disable CS0618 // Type or member is obsolete
AutoIndexId = base.AutoIndexId,
#pragma warning restore CS0618 // Type or member is obsolete
Capped = base.Capped,
ChangeStreamPreAndPostImagesOptions = base.ChangeStreamPreAndPostImagesOptions,
Collation = base.Collation,
EncryptedFields = base.EncryptedFields,
ExpireAfter = base.ExpireAfter,
IndexOptionDefaults = base.IndexOptionDefaults,
MaxDocuments = base.MaxDocuments,
MaxSize = base.MaxSize,
NoPadding = base.NoPadding,
SerializerRegistry = base.SerializerRegistry,
StorageEngine = base.StorageEngine,
TimeSeriesOptions = base.TimeSeriesOptions,
UsePowerOf2Sizes = base.UsePowerOf2Sizes,
ValidationAction = base.ValidationAction,
ValidationLevel = base.ValidationLevel,

_clusteredIndex = _clusteredIndex,
_documentSerializer = _documentSerializer,
_validator = _validator
};
}
}
62 changes: 44 additions & 18 deletions src/MongoDB.Driver/Encryption/ClientEncryption.cs
Expand Up @@ -82,59 +82,85 @@ public ClientEncryption(ClientEncryptionOptions clientEncryptionOptions)
/// <summary>
/// Create encrypted collection.
/// </summary>
/// <param name="collectionNamespace">The collection namespace.</param>
/// <param name="database">The database.</param>
/// <param name="collectionName">The collection name.</param>
/// <param name="createCollectionOptions">The create collection options.</param>
/// <param name="kmsProvider">The kms provider.</param>
/// <param name="dataKeyOptions">The datakey options.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The operation result.</returns>
/// <remarks>
/// if EncryptionFields contains a keyId with a null value, a data key will be automatically generated and assigned to keyId value.
/// </remarks>
public void CreateEncryptedCollection<TCollection>(CollectionNamespace collectionNamespace, CreateCollectionOptions createCollectionOptions, string kmsProvider, DataKeyOptions dataKeyOptions, CancellationToken cancellationToken = default)
public CreateEncryptedCollectionResult CreateEncryptedCollection(IMongoDatabase database, string collectionName, CreateCollectionOptions createCollectionOptions, string kmsProvider, DataKeyOptions dataKeyOptions, CancellationToken cancellationToken = default)
{
Ensure.IsNotNull(collectionNamespace, nameof(collectionNamespace));
Ensure.IsNotNull(database, nameof(database));
Ensure.IsNotNull(collectionName, nameof(collectionName));
Ensure.IsNotNull(createCollectionOptions, nameof(createCollectionOptions));
Ensure.IsNotNull(dataKeyOptions, nameof(dataKeyOptions));
Ensure.IsNotNull(kmsProvider, nameof(kmsProvider));

foreach (var fieldDocument in EncryptedCollectionHelper.IterateEmptyKeyIds(collectionNamespace, createCollectionOptions.EncryptedFields))
var encryptedFields = createCollectionOptions.EncryptedFields?.DeepClone()?.AsBsonDocument;
try
{
var dataKey = CreateDataKey(kmsProvider, dataKeyOptions, cancellationToken);
EncryptedCollectionHelper.ModifyEncryptedFields(fieldDocument, dataKey);
foreach (var fieldDocument in EncryptedCollectionHelper.IterateEmptyKeyIds(new CollectionNamespace(database.DatabaseNamespace.DatabaseName, collectionName), encryptedFields))
{
var dataKey = CreateDataKey(kmsProvider, dataKeyOptions, cancellationToken);
EncryptedCollectionHelper.ModifyEncryptedFields(fieldDocument, dataKey);
}

var effectiveCreateEncryptionOptions = createCollectionOptions.Clone();
effectiveCreateEncryptionOptions.EncryptedFields = encryptedFields;
database.CreateCollection(collectionName, effectiveCreateEncryptionOptions, cancellationToken);
}
catch (Exception ex)
{
throw new MongoEncryptionCreateCollectionException(ex, encryptedFields);
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning operation data, which is unrelated to exception, within exception is a bit unusual, but I am fine with this here.
@JamesKovacs this is follow up for our slack discussion, please double check that you are ok with this as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed with Dima via Slack. While from an API perspective it feels odd to return successfully-created keys in the exception, I don't see another reasonable way to avoid phantom encryption keys. While we could modify CreateCollectionOptions as we generate dataKeys, it violates the principle of least surprise to modify one's input parameters unexpectedly. I think this is the best option given the available trade-offs.

}

var database = _libMongoCryptController.KeyVaultClient.GetDatabase(collectionNamespace.DatabaseNamespace.DatabaseName);

database.CreateCollection(collectionNamespace.CollectionName, createCollectionOptions, cancellationToken);
return new CreateEncryptedCollectionResult(encryptedFields);
}

/// <summary>
/// Create encrypted collection.
/// </summary>
/// <param name="collectionNamespace">The collection namespace.</param>
/// <param name="database">The database.</param>
/// <param name="collectionName">The collection name.</param>
/// <param name="createCollectionOptions">The create collection options.</param>
/// <param name="kmsProvider">The kms provider.</param>
/// <param name="dataKeyOptions">The datakey options.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The operation result.</returns>
/// <remarks>
/// if EncryptionFields contains a keyId with a null value, a data key will be automatically generated and assigned to keyId value.
/// </remarks>
public async Task CreateEncryptedCollectionAsync<TCollection>(CollectionNamespace collectionNamespace, CreateCollectionOptions createCollectionOptions, string kmsProvider, DataKeyOptions dataKeyOptions, CancellationToken cancellationToken = default)
public async Task<CreateEncryptedCollectionResult> CreateEncryptedCollectionAsync(IMongoDatabase database, string collectionName, CreateCollectionOptions createCollectionOptions, string kmsProvider, DataKeyOptions dataKeyOptions, CancellationToken cancellationToken = default)
{
Ensure.IsNotNull(collectionNamespace, nameof(collectionNamespace));
Ensure.IsNotNull(database, nameof(database));
Ensure.IsNotNull(collectionName, nameof(collectionName));
Ensure.IsNotNull(createCollectionOptions, nameof(createCollectionOptions));
Ensure.IsNotNull(dataKeyOptions, nameof(dataKeyOptions));
Ensure.IsNotNull(kmsProvider, nameof(kmsProvider));

foreach (var fieldDocument in EncryptedCollectionHelper.IterateEmptyKeyIds(collectionNamespace, createCollectionOptions.EncryptedFields))
var encryptedFields = createCollectionOptions.EncryptedFields?.DeepClone()?.AsBsonDocument;
try
{
var dataKey = await CreateDataKeyAsync(kmsProvider, dataKeyOptions, cancellationToken).ConfigureAwait(false);
EncryptedCollectionHelper.ModifyEncryptedFields(fieldDocument, dataKey);
foreach (var fieldDocument in EncryptedCollectionHelper.IterateEmptyKeyIds(new CollectionNamespace(database.DatabaseNamespace.DatabaseName, collectionName), encryptedFields))
{
var dataKey = await CreateDataKeyAsync(kmsProvider, dataKeyOptions, cancellationToken).ConfigureAwait(false);
EncryptedCollectionHelper.ModifyEncryptedFields(fieldDocument, dataKey);
}

var effectiveCreateEncryptionOptions = createCollectionOptions.Clone();
effectiveCreateEncryptionOptions.EncryptedFields = encryptedFields;
await database.CreateCollectionAsync(collectionName, effectiveCreateEncryptionOptions, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
throw new MongoEncryptionCreateCollectionException(ex, encryptedFields);
}

var database = _libMongoCryptController.KeyVaultClient.GetDatabase(collectionNamespace.DatabaseNamespace.DatabaseName);

await database.CreateCollectionAsync(collectionNamespace.CollectionName, createCollectionOptions, cancellationToken).ConfigureAwait(false);
return new CreateEncryptedCollectionResult(encryptedFields);
}

/// <summary>
Expand Down
@@ -0,0 +1,38 @@
/* Copyright 2010-present MongoDB Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

using MongoDB.Bson;

namespace MongoDB.Driver.Encryption
{
/// <summary>
/// Represents the result of a create encrypted collection.
/// </summary>
BorisDog marked this conversation as resolved.
Show resolved Hide resolved
public sealed class CreateEncryptedCollectionResult
{
private readonly BsonDocument _encryptedFields;

/// <summary>
/// Initializes a new instance of the <see cref="CreateEncryptedCollectionResult"/> class.
/// </summary>
/// <param name="encryptedFields">The encrypted fields document.</param>
public CreateEncryptedCollectionResult(BsonDocument encryptedFields) => _encryptedFields = encryptedFields;

/// <summary>
/// The encrypted fields document.
/// </summary>
public BsonDocument EncryptedFields => _encryptedFields;
}
}
@@ -0,0 +1,69 @@
/* Copyright 2010-present MongoDB Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

using System;
using System.Runtime.Serialization;
using MongoDB.Bson;

namespace MongoDB.Driver.Encryption
{
/// <summary>
/// Represents an encryption exception.
/// </summary>
[Serializable]
public class MongoEncryptionCreateCollectionException : MongoEncryptionException
BorisDog marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly BsonDocument _encryptedFields;

/// <summary>
/// Initializes a new instance of the <see cref="MongoEncryptionException"/> class.
/// </summary>
/// <param name="innerException">The inner exception.</param>
/// <param name="encryptedFields">The encrypted fields.</param>
public MongoEncryptionCreateCollectionException(Exception innerException, BsonDocument encryptedFields)
: base(innerException)
{
_encryptedFields = encryptedFields;
}

/// <summary>
/// Initializes a new instance of the <see cref="MongoEncryptionCreateCollectionException"/> class (this overload used by deserialization).
/// </summary>
/// <param name="info">The SerializationInfo.</param>
/// <param name="context">The StreamingContext.</param>
protected MongoEncryptionCreateCollectionException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
_encryptedFields = (BsonDocument)info.GetValue(nameof(_encryptedFields), typeof(BsonDocument));
}

/// <summary>
/// The encrypted fields.
/// </summary>
public BsonDocument EncryptedFields => _encryptedFields;

// public methods
/// <summary>
/// Gets the object data.
/// </summary>
/// <param name="info">The information.</param>
/// <param name="context">The context.</param>
public override void GetObjectData(SerializationInfo info, StreamingContext context)
{
base.GetObjectData(info, context);
info.AddValue(nameof(_encryptedFields), _encryptedFields);
}
}
}
Expand Up @@ -15,7 +15,6 @@

using System;
using System.Runtime.Serialization;
using MongoDB.Driver.Core.Misc;

namespace MongoDB.Driver.Encryption
{
Expand Down
17 changes: 10 additions & 7 deletions tests/MongoDB.Bson.TestHelpers/BsonValueEquivalencyComparer.cs
Expand Up @@ -13,6 +13,7 @@
* limitations under the License.
*/

using System;
using System.Collections.Generic;

namespace MongoDB.Bson.TestHelpers
Expand All @@ -22,15 +23,17 @@ public class BsonValueEquivalencyComparer : IEqualityComparer<BsonValue>
#region static
public static BsonValueEquivalencyComparer Instance { get; } = new BsonValueEquivalencyComparer();

public static bool Compare(BsonValue a, BsonValue b)
public static bool Compare(BsonValue a, BsonValue b, Action<BsonValue, BsonValue> massageAction = null)
BorisDog marked this conversation as resolved.
Show resolved Hide resolved
{
massageAction?.Invoke(a, b);

if (a.BsonType == BsonType.Document && b.BsonType == BsonType.Document)
{
return CompareDocuments((BsonDocument)a, (BsonDocument)b);
return CompareDocuments((BsonDocument)a, (BsonDocument)b, massageAction);
}
else if (a.BsonType == BsonType.Array && b.BsonType == BsonType.Array)
{
return CompareArrays((BsonArray)a, (BsonArray)b);
return CompareArrays((BsonArray)a, (BsonArray)b, massageAction);
}
else if (a.BsonType == b.BsonType)
{
Expand All @@ -50,7 +53,7 @@ public static bool Compare(BsonValue a, BsonValue b)
}
}

private static bool CompareArrays(BsonArray a, BsonArray b)
private static bool CompareArrays(BsonArray a, BsonArray b, Action<BsonValue, BsonValue> massageAction = null)
{
if (a.Count != b.Count)
{
Expand All @@ -59,7 +62,7 @@ private static bool CompareArrays(BsonArray a, BsonArray b)

for (var i = 0; i < a.Count; i++)
{
if (!Compare(a[i], b[i]))
if (!Compare(a[i], b[i], massageAction))
{
return false;
}
Expand All @@ -68,7 +71,7 @@ private static bool CompareArrays(BsonArray a, BsonArray b)
return true;
}

private static bool CompareDocuments(BsonDocument a, BsonDocument b)
private static bool CompareDocuments(BsonDocument a, BsonDocument b, Action<BsonValue, BsonValue> massageAction = null)
{
if (a.ElementCount != b.ElementCount)
{
Expand All @@ -83,7 +86,7 @@ private static bool CompareDocuments(BsonDocument a, BsonDocument b)
return false;
}

if (!Compare(aElement.Value, bElement.Value))
if (!Compare(aElement.Value, bElement.Value, massageAction))
{
return false;
}
Expand Down