1
0
mirror of https://github.com/bitwarden/server synced 2025-12-06 00:03:34 +00:00

PM-27248 [Defect] An unhandled error is returned when a MSP tries to import data (#6563)

* Revert previous commits and implement logic to avoid null references:

• reverted previous commits and force pushed to remove inconsistent formatting
• implemented logic to avoid null reference access (with comments)

* fix typo in comment

* fix logical error and add test coverage

* extracted logic common to all code paths per review comment
This commit is contained in:
John Harrington
2025-11-17 13:40:30 -07:00
committed by GitHub
parent c2075781ca
commit f52a630197
2 changed files with 77 additions and 9 deletions

View File

@@ -150,17 +150,34 @@ public class ImportCiphersCommand : IImportCiphersCommand
foreach (var collection in collections)
{
if (!organizationCollectionsIds.Contains(collection.Id))
// If the collection already exists, skip it
if (organizationCollectionsIds.Contains(collection.Id))
{
collection.SetNewId();
newCollections.Add(collection);
newCollectionUsers.Add(new CollectionUser
{
CollectionId = collection.Id,
OrganizationUserId = importingOrgUser.Id,
Manage = true
});
continue;
}
// Create new collections if not already present
collection.SetNewId();
newCollections.Add(collection);
/*
* If the organization was created by a Provider, the organization may have zero members (users)
* In this situation importingOrgUser will be null, and accessing importingOrgUser.Id will
* result in a null reference exception.
*
* Avoid user assignment, but proceed with adding the collection.
*/
if (importingOrgUser == null)
{
continue;
}
newCollectionUsers.Add(new CollectionUser
{
CollectionId = collection.Id,
OrganizationUserId = importingOrgUser.Id,
Manage = true
});
}
// Create associations based on the newly assigned ids

View File

@@ -238,4 +238,55 @@ public class ImportCiphersAsyncCommandTests
Assert.Equal("This organization can only have a maximum of " +
$"{organization.MaxCollections} collections.", exception.Message);
}
[Theory, BitAutoData]
public async Task ImportIntoOrganizationalVaultAsync_WithNullImportingOrgUser_SkipsCollectionUserCreation(
Organization organization,
Guid importingUserId,
List<Collection> collections,
List<CipherDetails> ciphers,
SutProvider<ImportCiphersCommand> sutProvider)
{
organization.MaxCollections = null;
foreach (var collection in collections)
{
collection.OrganizationId = organization.Id;
}
foreach (var cipher in ciphers)
{
cipher.OrganizationId = organization.Id;
}
KeyValuePair<int, int>[] collectionRelationships = {
new(0, 0),
new(1, 1),
new(2, 2)
};
sutProvider.GetDependency<IOrganizationRepository>()
.GetByIdAsync(organization.Id)
.Returns(organization);
// Simulate provider-created org with no members - importing user is NOT an org member
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organization.Id, importingUserId)
.Returns((OrganizationUser)null);
sutProvider.GetDependency<ICollectionRepository>()
.GetManyByOrganizationIdAsync(organization.Id)
.Returns(new List<Collection>());
await sutProvider.Sut.ImportIntoOrganizationalVaultAsync(collections, ciphers, collectionRelationships, importingUserId);
// Verify ciphers were created but no CollectionUser entries were created (because the organization user (importingUserId) is null)
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(
ciphers,
Arg.Is<IEnumerable<Collection>>(cols => cols.Count() == collections.Count),
Arg.Is<IEnumerable<CollectionCipher>>(cc => cc.Count() == ciphers.Count),
Arg.Is<IEnumerable<CollectionUser>>(cus => !cus.Any()));
await sutProvider.GetDependency<IPushNotificationService>().Received(1).PushSyncVaultAsync(importingUserId);
}
}