From f52a630197da17534c08dc055ab88e9180e4b07c Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Mon, 17 Nov 2025 13:40:30 -0700 Subject: [PATCH] PM-27248 [Defect] An unhandled error is returned when a MSP tries to import data (#6563) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../ImportFeatures/ImportCiphersCommand.cs | 35 +++++++++---- .../ImportCiphersAsyncCommandTests.cs | 51 +++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs index c7f7e3aff7..fa558f5963 100644 --- a/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs +++ b/src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs @@ -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 diff --git a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs index 8c1cb1d5ff..b92477e73d 100644 --- a/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs +++ b/test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs @@ -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 collections, + List ciphers, + SutProvider sutProvider) + { + organization.MaxCollections = null; + + foreach (var collection in collections) + { + collection.OrganizationId = organization.Id; + } + + foreach (var cipher in ciphers) + { + cipher.OrganizationId = organization.Id; + } + + KeyValuePair[] collectionRelationships = { + new(0, 0), + new(1, 1), + new(2, 2) + }; + + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + // Simulate provider-created org with no members - importing user is NOT an org member + sutProvider.GetDependency() + .GetByOrganizationAsync(organization.Id, importingUserId) + .Returns((OrganizationUser)null); + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns(new List()); + + 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().Received(1).CreateAsync( + ciphers, + Arg.Is>(cols => cols.Count() == collections.Count), + Arg.Is>(cc => cc.Count() == ciphers.Count), + Arg.Is>(cus => !cus.Any())); + + await sutProvider.GetDependency().Received(1).PushSyncVaultAsync(importingUserId); + } }