From 5272670e3411d56fe7f95f44eb7aeacec4fb5ce2 Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 2 Jul 2026 10:27:06 -0400 Subject: [PATCH 1/2] create default collecion for demoted admin --- .../OrganizationUsersController.cs | 2 +- .../OrganizationUserRequestModels.cs | 2 + .../IUpdateOrganizationUserCommand.cs | 3 +- .../UpdateOrganizationUserCommand.cs | 13 +++- .../UpdateOrganizationUserCommandTests.cs | 61 +++++++++++++++++++ 5 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index d436b1a7ad3f..cb8bb0e7bc8a 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -464,7 +464,7 @@ public async Task Put(Guid orgId, Guid id, [FromBody] OrganizationUserUpdateRequ var existingUserType = organizationUser.Type; await _updateOrganizationUserCommand.UpdateUserAsync(model.ToOrganizationUser(organizationUser), existingUserType, userId, - collectionsToSave, groupsToSave); + collectionsToSave, groupsToSave, model.DefaultUserCollectionName); } [HttpPost("{id}")] diff --git a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs index c2c88e14139b..357647dfeb74 100644 --- a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs @@ -109,6 +109,8 @@ public class OrganizationUserUpdateRequestModel [StringLength(50)] public string? Name { get; set; } + + public string? DefaultUserCollectionName { get; set; } #nullable disable public OrganizationUser ToOrganizationUser(OrganizationUser existingUser) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs index d1fc9fbbec74..833910283002 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs @@ -8,5 +8,6 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interface public interface IUpdateOrganizationUserCommand { Task UpdateUserAsync(OrganizationUser organizationUser, OrganizationUserType existingUserType, Guid? savingUserId, - List? collectionAccess, IEnumerable? groupAccess); + List? collectionAccess, IEnumerable? groupAccess, + string? defaultUserCollectionName = null); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index 7d77518f7d56..0c258857ddea 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -65,7 +65,8 @@ public UpdateOrganizationUserCommand( /// public async Task UpdateUserAsync(OrganizationUser organizationUser, OrganizationUserType existingUserType, Guid? savingUserId, - List? collectionAccess, IEnumerable? groupAccess) + List? collectionAccess, IEnumerable? groupAccess, + string? defaultUserCollectionName = null) { // Avoid multiple enumeration var collectionAccessList = collectionAccess?.ToList() ?? []; @@ -145,6 +146,16 @@ public async Task UpdateUserAsync(OrganizationUser organizationUser, Organizatio await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupAccess, _timeProvider.GetUtcNow().UtcDateTime); } + var isDemotedFromPrivilegedRole = existingUserType is OrganizationUserType.Admin or OrganizationUserType.Owner + && organizationUser.Type is not (OrganizationUserType.Admin or OrganizationUserType.Owner); + if (isDemotedFromPrivilegedRole && !string.IsNullOrWhiteSpace(defaultUserCollectionName)) + { + await _collectionRepository.CreateDefaultCollectionsAsync( + organizationUser.OrganizationId, + [organizationUser.Id], + defaultUserCollectionName); + } + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Updated); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index ed8d3b23466b..4bb24beeb465 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -282,6 +282,67 @@ await sutProvider.GetDependency().Received(1).Repla ); } + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCollectionName_CreatesDefaultCollection( + OrganizationUserType existingUserType, + Organization organization, + OrganizationUser oldUserData, + OrganizationUser newUserData, + string defaultUserCollectionName, + SutProvider sutProvider) + { + newUserData.Type = OrganizationUserType.User; + Setup(sutProvider, organization, newUserData, oldUserData); + + await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null, defaultUserCollectionName); + + await sutProvider.GetDependency().Received(1).CreateDefaultCollectionsAsync( + newUserData.OrganizationId, + Arg.Is>(ids => ids.Contains(newUserData.Id)), + defaultUserCollectionName); + } + + [Theory] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task UpdateUserAsync_WhenExistingUserIsNotPrivileged_WithDefaultCollectionName_DoesNotCreateDefaultCollection( + OrganizationUserType existingUserType, + Organization organization, + OrganizationUser oldUserData, + OrganizationUser newUserData, + string defaultUserCollectionName, + SutProvider sutProvider) + { + newUserData.Type = OrganizationUserType.User; + Setup(sutProvider, organization, newUserData, oldUserData); + + await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null, defaultUserCollectionName); + + await sutProvider.GetDependency().DidNotReceive().CreateDefaultCollectionsAsync( + Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithoutDefaultCollectionName_DoesNotCreateDefaultCollection( + OrganizationUserType existingUserType, + Organization organization, + OrganizationUser oldUserData, + OrganizationUser newUserData, + SutProvider sutProvider) + { + newUserData.Type = OrganizationUserType.User; + Setup(sutProvider, organization, newUserData, oldUserData); + + await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null); + + await sutProvider.GetDependency().DidNotReceive().CreateDefaultCollectionsAsync( + Arg.Any(), Arg.Any>(), Arg.Any()); + } + private void Setup(SutProvider sutProvider, Organization organization, OrganizationUser newUser, OrganizationUser oldUser) { From 27d921a98ca003cf5c7aefa6fa3f4971507ed55d Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 2 Jul 2026 12:59:03 -0400 Subject: [PATCH 2/2] fix tests, add org ability check and policyRequirement check --- .../UpdateOrganizationUserCommand.cs | 13 ++++- .../OrganizationUserControllerPutTests.cs | 12 ++-- .../UpdateOrganizationUserCommandTests.cs | 58 ++++++++++++++++++- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index 0c258857ddea..502948397adb 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -1,5 +1,7 @@ #nullable enable using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Enums; using Bit.Core.Billing.Pricing; @@ -27,6 +29,7 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; private readonly IPricingClient _pricingClient; private readonly TimeProvider _timeProvider; + private readonly IPolicyRequirementQuery _policyRequirementQuery; public UpdateOrganizationUserCommand( IEventService eventService, @@ -39,7 +42,8 @@ public UpdateOrganizationUserCommand( IGroupRepository groupRepository, IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery, IPricingClient pricingClient, - TimeProvider timeProvider) + TimeProvider timeProvider, + IPolicyRequirementQuery policyRequirementQuery) { _eventService = eventService; _organizationService = organizationService; @@ -52,6 +56,7 @@ public UpdateOrganizationUserCommand( _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; _pricingClient = pricingClient; _timeProvider = timeProvider; + _policyRequirementQuery = policyRequirementQuery; } /// @@ -148,7 +153,11 @@ public async Task UpdateUserAsync(OrganizationUser organizationUser, Organizatio var isDemotedFromPrivilegedRole = existingUserType is OrganizationUserType.Admin or OrganizationUserType.Owner && organizationUser.Type is not (OrganizationUserType.Admin or OrganizationUserType.Owner); - if (isDemotedFromPrivilegedRole && !string.IsNullOrWhiteSpace(defaultUserCollectionName)) + if (isDemotedFromPrivilegedRole + && organizationUser.UserId.HasValue + && organization.UseMyItems + && !string.IsNullOrWhiteSpace(defaultUserCollectionName) + && (await _policyRequirementQuery.GetAsync(organizationUser.UserId.Value)).State == OrganizationDataOwnershipState.Enabled) { await _collectionRepository.CreateDefaultCollectionsAsync( organizationUser.OrganizationId, diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUserControllerPutTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUserControllerPutTests.cs index 1ce22f2cfc0b..00cdbc1db83b 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUserControllerPutTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUserControllerPutTests.cs @@ -58,7 +58,8 @@ await sutProvider.GetDependency().Received(1).Up savingUserId, Arg.Is>(cas => cas.All(c => model.Collections.Any(m => m.Id == c.Id))), - model.Groups); + model.Groups, + model.DefaultUserCollectionName); } [Theory] @@ -110,7 +111,8 @@ await sutProvider.GetDependency().Received(1).Up Arg.Is>(cas => cas.All(c => model.Collections.Any(m => m.Id == c.Id))), // Main assertion: groups are not updated (are null) - null); + null, + model.DefaultUserCollectionName); } [Theory] @@ -146,7 +148,8 @@ await sutProvider.GetDependency().Received(1).Up savingUserId, Arg.Is>(cas => cas.All(c => model.Collections.Any(m => m.Id == c.Id))), - model.Groups); + model.Groups, + model.DefaultUserCollectionName); } [Theory] @@ -227,7 +230,8 @@ await sutProvider.GetDependency().Received(1).Up cas.First(c => c.Id == editedCollectionId).Manage == true && cas.First(c => c.Id == editedCollectionId).ReadOnly == false && cas.First(c => c.Id == editedCollectionId).HidePasswords == false), - model.Groups); + model.Groups, + model.DefaultUserCollectionName); } [Theory] diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index 4bb24beeb465..b105558f9195 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -2,6 +2,8 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Enums; using Bit.Core.Entities; @@ -285,7 +287,7 @@ await sutProvider.GetDependency().Received(1).Repla [Theory] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] - public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCollectionName_CreatesDefaultCollection( + public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCollectionName_AndUseMyItemsEnabled_AndPolicyEnabled_CreatesDefaultCollection( OrganizationUserType existingUserType, Organization organization, OrganizationUser oldUserData, @@ -293,8 +295,10 @@ public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCo string defaultUserCollectionName, SutProvider sutProvider) { + organization.UseMyItems = true; newUserData.Type = OrganizationUserType.User; Setup(sutProvider, organization, newUserData, oldUserData); + SetupDataOwnershipPolicy(sutProvider, newUserData.UserId!.Value, OrganizationDataOwnershipState.Enabled); await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null, defaultUserCollectionName); @@ -304,6 +308,49 @@ await sutProvider.GetDependency().Received(1).CreateDefau defaultUserCollectionName); } + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCollectionName_AndUseMyItemsDisabled_DoesNotCreateDefaultCollection( + OrganizationUserType existingUserType, + Organization organization, + OrganizationUser oldUserData, + OrganizationUser newUserData, + string defaultUserCollectionName, + SutProvider sutProvider) + { + organization.UseMyItems = false; + newUserData.Type = OrganizationUserType.User; + Setup(sutProvider, organization, newUserData, oldUserData); + + await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null, defaultUserCollectionName); + + await sutProvider.GetDependency().DidNotReceive().CreateDefaultCollectionsAsync( + Arg.Any(), Arg.Any>(), Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task UpdateUserAsync_WhenDemotingPrivilegedUserToUser_WithDefaultCollectionName_AndPolicyDisabled_DoesNotCreateDefaultCollection( + OrganizationUserType existingUserType, + Organization organization, + OrganizationUser oldUserData, + OrganizationUser newUserData, + string defaultUserCollectionName, + SutProvider sutProvider) + { + organization.UseMyItems = true; + newUserData.Type = OrganizationUserType.User; + Setup(sutProvider, organization, newUserData, oldUserData); + SetupDataOwnershipPolicy(sutProvider, newUserData.UserId!.Value, OrganizationDataOwnershipState.Disabled); + + await sutProvider.Sut.UpdateUserAsync(newUserData, existingUserType, null, null, null, defaultUserCollectionName); + + await sutProvider.GetDependency().DidNotReceive().CreateDefaultCollectionsAsync( + Arg.Any(), Arg.Any>(), Arg.Any()); + } + [Theory] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Custom)] @@ -362,4 +409,13 @@ private void Setup(SutProvider sutProvider, Organ Arg.Is>(i => i.Contains(oldUser.Id))) .Returns(true); } + + private void SetupDataOwnershipPolicy(SutProvider sutProvider, + Guid userId, OrganizationDataOwnershipState state) + { + var requirement = new OrganizationDataOwnershipPolicyRequirement(state, []); + sutProvider.GetDependency() + .GetAsync(userId) + .Returns(requirement); + } }