diff --git a/src/Api/Public/Controllers/CollectionsController.cs b/src/Api/Public/Controllers/CollectionsController.cs index d4e0932caa..ec282a0e4d 100644 --- a/src/Api/Public/Controllers/CollectionsController.cs +++ b/src/Api/Public/Controllers/CollectionsController.cs @@ -2,6 +2,7 @@ using Bit.Api.Models.Public.Request; using Bit.Api.Models.Public.Response; using Bit.Core.Context; +using Bit.Core.Enums; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -116,6 +117,12 @@ public class CollectionsController : Controller { return new NotFoundResult(); } + + if (collection.Type == CollectionType.DefaultUserCollection) + { + return new BadRequestObjectResult(new ErrorResponseModel("You cannot delete a collection with the type as DefaultUserCollection.")); + } + await _collectionRepository.DeleteAsync(collection); return new OkResult(); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs index 1b53716537..3347e77c37 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs @@ -163,6 +163,11 @@ public class UpdateGroupCommand : IUpdateGroupCommand // Use generic error message to avoid enumeration throw new NotFoundException(); } + + if (collections.Any(c => c.Type == CollectionType.DefaultUserCollection)) + { + throw new BadRequestException("You cannot modify group access for collections with the type as DefaultUserCollection."); + } } private async Task ValidateMemberAccessAsync(Group originalGroup, diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index 8d1e693e8b..b72505c195 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -199,6 +199,11 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand // Use generic error message to avoid enumeration throw new NotFoundException(); } + + if (collections.Any(c => c.Type == CollectionType.DefaultUserCollection)) + { + throw new BadRequestException("You cannot modify member access for collections with the type as DefaultUserCollection."); + } } private async Task ValidateGroupAccessAsync(OrganizationUser originalUser, diff --git a/src/Core/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommand.cs b/src/Core/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommand.cs index 1d7eb8f2ba..929c236ef2 100644 --- a/src/Core/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommand.cs @@ -52,6 +52,11 @@ public class BulkAddCollectionAccessCommand : IBulkAddCollectionAccessCommand throw new BadRequestException("No collections were provided."); } + if (collections.Any(c => c.Type == CollectionType.DefaultUserCollection)) + { + throw new BadRequestException("You cannot add access to collections with the type as DefaultUserCollection."); + } + var orgId = collections.First().OrganizationId; if (collections.Any(c => c.OrganizationId != orgId)) diff --git a/src/Core/OrganizationFeatures/OrganizationCollections/CreateCollectionCommand.cs b/src/Core/OrganizationFeatures/OrganizationCollections/CreateCollectionCommand.cs index 1cec2f5cc4..d83e30ad9c 100644 --- a/src/Core/OrganizationFeatures/OrganizationCollections/CreateCollectionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationCollections/CreateCollectionCommand.cs @@ -1,4 +1,5 @@ using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Data; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; @@ -26,6 +27,11 @@ public class CreateCollectionCommand : ICreateCollectionCommand public async Task CreateAsync(Collection collection, IEnumerable groups = null, IEnumerable users = null) { + if (collection.Type == CollectionType.DefaultUserCollection) + { + throw new BadRequestException("You cannot create a collection with the type as DefaultUserCollection."); + } + var org = await _organizationRepository.GetByIdAsync(collection.OrganizationId); if (org == null) { diff --git a/src/Core/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommand.cs b/src/Core/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommand.cs index 11f29f228f..4f678633a9 100644 --- a/src/Core/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommand.cs @@ -1,4 +1,6 @@ using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -20,6 +22,11 @@ public class DeleteCollectionCommand : IDeleteCollectionCommand public async Task DeleteAsync(Collection collection) { + if (collection.Type == CollectionType.DefaultUserCollection) + { + throw new BadRequestException("You cannot delete a collection with the type as DefaultUserCollection."); + } + await _collectionRepository.DeleteAsync(collection); await _eventService.LogCollectionEventAsync(collection, Enums.EventType.Collection_Deleted, DateTime.UtcNow); } @@ -33,6 +40,11 @@ public class DeleteCollectionCommand : IDeleteCollectionCommand public async Task DeleteManyAsync(IEnumerable collections) { + if (collections.Any(c => c.Type == Enums.CollectionType.DefaultUserCollection)) + { + throw new BadRequestException("You cannot delete collections with the type as DefaultUserCollection."); + } + await _collectionRepository.DeleteManyAsync(collections.Select(c => c.Id)); await _eventService.LogCollectionEventsAsync(collections.Select(c => (c, Enums.EventType.Collection_Deleted, (DateTime?)DateTime.UtcNow))); } diff --git a/src/Core/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommand.cs b/src/Core/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommand.cs index 3985b6a919..19ad47a0a5 100644 --- a/src/Core/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommand.cs @@ -1,4 +1,5 @@ using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Data; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; @@ -26,6 +27,11 @@ public class UpdateCollectionCommand : IUpdateCollectionCommand public async Task UpdateAsync(Collection collection, IEnumerable groups = null, IEnumerable users = null) { + if (collection.Type == CollectionType.DefaultUserCollection) + { + throw new BadRequestException("You cannot edit a collection with the type as DefaultUserCollection."); + } + var org = await _organizationRepository.GetByIdAsync(collection.OrganizationId); if (org == null) { diff --git a/src/Core/Services/Implementations/CollectionService.cs b/src/Core/Services/Implementations/CollectionService.cs index 2a3f8c42dc..3b828955af 100644 --- a/src/Core/Services/Implementations/CollectionService.cs +++ b/src/Core/Services/Implementations/CollectionService.cs @@ -22,10 +22,13 @@ public class CollectionService : ICollectionService _collectionRepository = collectionRepository; } - - public async Task DeleteUserAsync(Collection collection, Guid organizationUserId) { + if (collection.Type == Enums.CollectionType.DefaultUserCollection) + { + throw new BadRequestException("You cannot modify member access for collections with the type as DefaultUserCollection."); + } + var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); if (orgUser == null || orgUser.OrganizationId != collection.OrganizationId) { diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommandTests.cs index 41486e1c00..b9f6964123 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommandTests.cs @@ -156,6 +156,24 @@ public class UpdateGroupCommandTests () => sutProvider.Sut.UpdateGroupAsync(group, organization, collectionAccess)); } + [Theory, OrganizationCustomize(UseGroups = true), BitAutoData] + public async Task UpdateGroup_WithDefaultUserCollectionType_Throws(SutProvider sutProvider, + Group group, Group oldGroup, Organization organization, List collectionAccess) + { + ArrangeGroup(sutProvider, group, oldGroup); + ArrangeUsers(sutProvider, group); + + // Return collections with DefaultUserCollection type + sutProvider.GetDependency() + .GetManyByManyIdsAsync(Arg.Any>()) + .Returns(callInfo => callInfo.Arg>() + .Select(guid => new Collection { Id = guid, OrganizationId = group.OrganizationId, Type = CollectionType.DefaultUserCollection }).ToList()); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateGroupAsync(group, organization, collectionAccess)); + Assert.Contains("You cannot modify group access for collections with the type as DefaultUserCollection.", exception.Message); + } + [Theory, OrganizationCustomize(UseGroups = true), BitAutoData] public async Task UpdateGroup_MemberBelongsToDifferentOrganization_Throws(SutProvider sutProvider, Group group, Group oldGroup, Organization organization, IEnumerable userAccess) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index 5c07ce0347..bd112c5d40 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -244,6 +244,24 @@ public class UpdateOrganizationUserCommandTests Assert.Contains("User can only be an admin of one free organization.", exception.Message); } + [Theory, BitAutoData] + public async Task UpdateUserAsync_WithDefaultUserCollectionType_Throws(OrganizationUser user, OrganizationUser originalUser, + List collectionAccess, Guid? savingUserId, SutProvider sutProvider, + Organization organization) + { + Setup(sutProvider, organization, user, originalUser); + + // Return collections with DefaultUserCollection type + sutProvider.GetDependency() + .GetManyByManyIdsAsync(Arg.Any>()) + .Returns(callInfo => callInfo.Arg>() + .Select(guid => new Collection { Id = guid, OrganizationId = user.OrganizationId, Type = CollectionType.DefaultUserCollection }).ToList()); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateUserAsync(user, OrganizationUserType.User, savingUserId, collectionAccess, null)); + Assert.Contains("You cannot modify member access for collections with the type as DefaultUserCollection.", exception.Message); + } + private void Setup(SutProvider sutProvider, Organization organization, OrganizationUser newUser, OrganizationUser oldUser) { diff --git a/test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs index 63f2bac896..713edeefbf 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs @@ -27,6 +27,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + sutProvider.GetDependency() .GetManyAsync( Arg.Is>(ids => ids.SequenceEqual(collectionUsers.Select(u => u.OrganizationUserId))) @@ -107,6 +109,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + collections.First().OrganizationId = Guid.NewGuid(); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AddAccessAsync(collections, @@ -127,6 +131,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + organizationUsers.RemoveAt(0); sutProvider.GetDependency() @@ -155,6 +161,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + organizationUsers.First().OrganizationId = Guid.NewGuid(); sutProvider.GetDependency() @@ -184,6 +192,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + groups.RemoveAt(0); sutProvider.GetDependency() @@ -221,6 +231,8 @@ public class BulkAddCollectionAccessCommandTests IEnumerable collectionUsers, IEnumerable collectionGroups) { + SetCollectionsToSharedType(collections); + groups.First().OrganizationId = Guid.NewGuid(); sutProvider.GetDependency() @@ -250,6 +262,37 @@ public class BulkAddCollectionAccessCommandTests ); } + [Theory, BitAutoData, CollectionCustomization] + public async Task AddAccessAsync_WithDefaultUserCollectionType_ThrowsBadRequest(SutProvider sutProvider, + IList collections, + IEnumerable collectionUsers, + IEnumerable collectionGroups) + { + // Arrange + collections.First().Type = CollectionType.DefaultUserCollection; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AddAccessAsync(collections, + ToAccessSelection(collectionUsers), + ToAccessSelection(collectionGroups) + )); + + Assert.Contains("You cannot add access to collections with the type as DefaultUserCollection.", exception.Message); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateOrUpdateAccessForManyAsync(default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCollectionEventsAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByManyIds(default); + } + + private static void SetCollectionsToSharedType(IEnumerable collections) + { + foreach (var collection in collections) + { + collection.Type = CollectionType.SharedCollection; + } + } + private static ICollection ToAccessSelection(IEnumerable collectionUsers) { return collectionUsers.Select(cu => new CollectionAccessSelection diff --git a/test/Core.Test/OrganizationFeatures/OrganizationCollections/CreateCollectionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationCollections/CreateCollectionCommandTests.cs index d180bad432..8937e8628d 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationCollections/CreateCollectionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationCollections/CreateCollectionCommandTests.cs @@ -199,4 +199,27 @@ public class CreateCollectionCommandTests .DidNotReceiveWithAnyArgs() .LogCollectionEventAsync(default, default); } + + [Theory, BitAutoData] + public async Task CreateAsync_WithDefaultUserCollectionType_ThrowsBadRequest( + Organization organization, Collection collection, SutProvider sutProvider) + { + collection.Id = default; + collection.Type = CollectionType.DefaultUserCollection; + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(collection)); + Assert.Contains("You cannot create a collection with the type as DefaultUserCollection.", ex.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .CreateAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .CreateAsync(default, default, default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogCollectionEventAsync(default, default); + } } diff --git a/test/Core.Test/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommandTests.cs index 99eca20a09..efe9223f1b 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationCollections/DeleteCollectionCommandTests.cs @@ -1,6 +1,6 @@ - -using Bit.Core.Entities; +using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.OrganizationFeatures.OrganizationCollections; using Bit.Core.Repositories; using Bit.Core.Services; @@ -34,6 +34,7 @@ public class DeleteCollectionCommandTests { // Arrange var collectionIds = new[] { collection.Id, collection2.Id }; + collection.Type = collection2.Type = CollectionType.SharedCollection; sutProvider.GetDependency() .GetManyByManyIdsAsync(collectionIds) @@ -51,5 +52,42 @@ public class DeleteCollectionCommandTests a.All(c => collectionIds.Contains(c.Item1.Id) && c.Item2 == EventType.Collection_Deleted))); } + [Theory, BitAutoData] + [OrganizationCustomize] + public async Task DeleteAsync_WithDefaultUserCollectionType_ThrowsBadRequest(Collection collection, SutProvider sutProvider) + { + // Arrange + collection.Type = CollectionType.DefaultUserCollection; + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAsync(collection)); + Assert.Contains("You cannot delete a collection with the type as DefaultUserCollection.", ex.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogCollectionEventAsync(default, default, default); + } + + [Theory, BitAutoData] + [OrganizationCustomize] + public async Task DeleteManyAsync_WithDefaultUserCollectionType_ThrowsBadRequest(Collection collection, Collection collection2, SutProvider sutProvider) + { + // Arrange + collection.Type = CollectionType.DefaultUserCollection; + collection2.Type = CollectionType.SharedCollection; + var collections = new List { collection, collection2 }; + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteManyAsync(collections)); + Assert.Contains("You cannot delete collections with the type as DefaultUserCollection.", ex.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteManyAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogCollectionEventsAsync(default); + } } diff --git a/test/Core.Test/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommandTests.cs index 5147157750..2b8c180989 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationCollections/UpdateCollectionCommandTests.cs @@ -166,4 +166,26 @@ public class UpdateCollectionCommandTests .DidNotReceiveWithAnyArgs() .LogCollectionEventAsync(default, default); } + + [Theory, BitAutoData] + public async Task UpdateAsync_WithDefaultUserCollectionType_ThrowsBadRequest( + Organization organization, Collection collection, SutProvider sutProvider) + { + collection.Type = CollectionType.DefaultUserCollection; + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(collection)); + Assert.Contains("You cannot edit a collection with the type as DefaultUserCollection.", ex.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .ReplaceAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .ReplaceAsync(default, default, default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogCollectionEventAsync(default, default); + } } diff --git a/test/Core.Test/Services/CollectionServiceTests.cs b/test/Core.Test/Services/CollectionServiceTests.cs index 2f99467700..118c0fa6b2 100644 --- a/test/Core.Test/Services/CollectionServiceTests.cs +++ b/test/Core.Test/Services/CollectionServiceTests.cs @@ -49,4 +49,22 @@ public class CollectionServiceTest await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .LogOrganizationUserEventAsync(default, default); } + + [Theory, BitAutoData] + public async Task DeleteUserAsync_WithDefaultUserCollectionType_ThrowsBadRequest(Collection collection, + Organization organization, OrganizationUser organizationUser, SutProvider sutProvider) + { + collection.Type = CollectionType.DefaultUserCollection; + collection.OrganizationId = organization.Id; + organizationUser.OrganizationId = organization.Id; + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.DeleteUserAsync(collection, organizationUser.Id)); + Assert.Contains("You cannot modify member access for collections with the type as DefaultUserCollection.", exception.Message); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetByIdAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(default, default); + } }