From 96f9fbb9511abbec0b68107404ddf256d23add96 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 17 Jan 2024 22:33:35 +1000 Subject: [PATCH] [AC-2027] Update Flexible Collections logic to use organization property (#3644) * Update optionality to use org.FlexibleCollections Also break old feature flag key to ensure it's never enabled * Add logic to set defaults for collection management setting * Update optionality logic to use org property * Add comments * Add helper method for getting individual orgAbility * Fix validate user update permissions interface * Fix tests * dotnet format * Fix more tests * Simplify self-hosted update logic * Fix mapping * Use new getOrganizationAbility method * Refactor invite and save orgUser methods Pass in whole organization object instead of using OrganizationAbility * fix CipherService tests * dotnet format * Remove manager check to simplify this set of changes * Misc cleanup before review * Fix undefined variable * Refactor bulk-access endpoint to avoid early repo call * Restore manager check * Add tests for UpdateOrganizationLicenseCommand * Add nullable regions * Delete unused dependency * dotnet format * Fix test --- .../Controllers/GroupsController.cs | 18 +- .../OrganizationUsersController.cs | 19 +- .../Controllers/OrganizationsController.cs | 6 +- src/Api/Controllers/CollectionsController.cs | 96 ++++----- .../BulkCollectionAuthorizationHandler.cs | 17 +- .../CollectionAuthorizationHandler.cs | 10 - .../Groups/GroupAuthorizationHandler.cs | 15 +- .../OrganizationUserAuthorizationHandler.cs | 15 +- .../AdminConsole/Entities/Organization.cs | 14 +- .../SelfHostedOrganizationDetails.cs | 3 +- .../Implementations/OrganizationService.cs | 48 +++-- src/Core/Constants.cs | 7 +- src/Core/Context/CurrentContext.cs | 23 -- .../UpdateOrganizationLicenseCommand.cs | 13 +- src/Core/Services/IApplicationCacheService.cs | 3 + .../InMemoryApplicationCacheService.cs | 9 + .../Services/Implementations/CipherService.cs | 2 +- src/Events/Controllers/CollectController.cs | 2 - .../Controllers/CollectionsControllerTests.cs | 202 ++++++++++++++---- ...BulkCollectionAuthorizationHandlerTests.cs | 56 ++--- .../CollectionAuthorizationHandlerTests.cs | 3 - .../GroupAuthorizationHandlerTests.cs | 43 ++-- ...ganizationUserAuthorizationHandlerTests.cs | 43 ++-- .../Services/OrganizationServiceTests.cs | 30 +-- .../AutoFixture/FeatureServiceFixtures.cs | 75 ------- .../UpdateOrganizationLicenseCommandTests.cs | 100 +++++++++ .../Vault/Services/CipherServiceTests.cs | 11 +- 27 files changed, 472 insertions(+), 411 deletions(-) delete mode 100644 test/Core.Test/AutoFixture/FeatureServiceFixtures.cs create mode 100644 test/Core.Test/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommandTests.cs diff --git a/src/Api/AdminConsole/Controllers/GroupsController.cs b/src/Api/AdminConsole/Controllers/GroupsController.cs index 447ea4bdc7..3a256043a0 100644 --- a/src/Api/AdminConsole/Controllers/GroupsController.cs +++ b/src/Api/AdminConsole/Controllers/GroupsController.cs @@ -3,7 +3,6 @@ using Bit.Api.AdminConsole.Models.Response; using Bit.Api.Models.Response; using Bit.Api.Utilities; using Bit.Api.Vault.AuthorizationHandlers.Groups; -using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; @@ -27,10 +26,8 @@ public class GroupsController : Controller private readonly ICurrentContext _currentContext; private readonly ICreateGroupCommand _createGroupCommand; private readonly IUpdateGroupCommand _updateGroupCommand; - private readonly IFeatureService _featureService; private readonly IAuthorizationService _authorizationService; - - private bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + private readonly IApplicationCacheService _applicationCacheService; public GroupsController( IGroupRepository groupRepository, @@ -41,7 +38,8 @@ public class GroupsController : Controller IUpdateGroupCommand updateGroupCommand, IDeleteGroupCommand deleteGroupCommand, IFeatureService featureService, - IAuthorizationService authorizationService) + IAuthorizationService authorizationService, + IApplicationCacheService applicationCacheService) { _groupRepository = groupRepository; _groupService = groupService; @@ -50,8 +48,8 @@ public class GroupsController : Controller _createGroupCommand = createGroupCommand; _updateGroupCommand = updateGroupCommand; _deleteGroupCommand = deleteGroupCommand; - _featureService = featureService; _authorizationService = authorizationService; + _applicationCacheService = applicationCacheService; } [HttpGet("{id}")] @@ -81,7 +79,7 @@ public class GroupsController : Controller [HttpGet("")] public async Task> Get(Guid orgId) { - if (UseFlexibleCollections) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await Get_vNext(orgId); @@ -217,4 +215,10 @@ public class GroupsController : Controller var responses = groups.Select(g => new GroupDetailsResponseModel(g.Item1, g.Item2)); return new ListResponseModel(responses); } + + private async Task FlexibleCollectionsIsEnabledAsync(Guid organizationId) + { + var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId); + return organizationAbility?.FlexibleCollections ?? false; + } } diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 703696c6ad..1eacab68b8 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -4,7 +4,6 @@ using Bit.Api.Models.Request.Organizations; using Bit.Api.Models.Response; using Bit.Api.Utilities; using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; -using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; @@ -39,10 +38,8 @@ public class OrganizationUsersController : Controller private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; - private readonly IFeatureService _featureService; private readonly IAuthorizationService _authorizationService; - - private bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + private readonly IApplicationCacheService _applicationCacheService; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -57,8 +54,8 @@ public class OrganizationUsersController : Controller IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand, IAcceptOrgUserCommand acceptOrgUserCommand, - IFeatureService featureService, - IAuthorizationService authorizationService) + IAuthorizationService authorizationService, + IApplicationCacheService applicationCacheService) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -72,8 +69,8 @@ public class OrganizationUsersController : Controller _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; _acceptOrgUserCommand = acceptOrgUserCommand; - _featureService = featureService; _authorizationService = authorizationService; + _applicationCacheService = applicationCacheService; } [HttpGet("{id}")] @@ -98,7 +95,7 @@ public class OrganizationUsersController : Controller [HttpGet("")] public async Task> Get(Guid orgId, bool includeGroups = false, bool includeCollections = false) { - var authorized = UseFlexibleCollections + var authorized = await FlexibleCollectionsIsEnabledAsync(orgId) ? (await _authorizationService.AuthorizeAsync(User, OrganizationUserOperations.ReadAll(orgId))).Succeeded : await _currentContext.ViewAllCollections(orgId) || await _currentContext.ViewAssignedCollections(orgId) || @@ -518,4 +515,10 @@ public class OrganizationUsersController : Controller return new ListResponseModel(result.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } + + private async Task FlexibleCollectionsIsEnabledAsync(Guid organizationId) + { + var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId); + return organizationAbility?.FlexibleCollections ?? false; + } } diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 1683af2b68..e4f0c3aa50 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -784,7 +784,6 @@ public class OrganizationsController : Controller } [HttpPut("{id}/collection-management")] - [RequireFeature(FeatureFlagKeys.FlexibleCollections)] [SelfHosted(NotSelfHostedOnly = true)] public async Task PutCollectionManagement(Guid id, [FromBody] OrganizationCollectionManagementUpdateRequestModel model) { @@ -799,6 +798,11 @@ public class OrganizationsController : Controller throw new NotFoundException(); } + if (!organization.FlexibleCollections) + { + throw new BadRequestException("Organization does not have collection enhancements enabled"); + } + var v1Enabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext); if (!v1Enabled) diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 7ae76ba750..4072518017 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -28,8 +28,8 @@ public class CollectionsController : Controller private readonly IAuthorizationService _authorizationService; private readonly ICurrentContext _currentContext; private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand; - private readonly IFeatureService _featureService; private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IApplicationCacheService _applicationCacheService; public CollectionsController( ICollectionRepository collectionRepository, @@ -39,8 +39,8 @@ public class CollectionsController : Controller IAuthorizationService authorizationService, ICurrentContext currentContext, IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand, - IFeatureService featureService, - IOrganizationUserRepository organizationUserRepository) + IOrganizationUserRepository organizationUserRepository, + IApplicationCacheService applicationCacheService) { _collectionRepository = collectionRepository; _organizationUserRepository = organizationUserRepository; @@ -50,16 +50,14 @@ public class CollectionsController : Controller _authorizationService = authorizationService; _currentContext = currentContext; _bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand; - _featureService = featureService; _organizationUserRepository = organizationUserRepository; + _applicationCacheService = applicationCacheService; } - private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - [HttpGet("{id}")] public async Task Get(Guid orgId, Guid id) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await Get_vNext(id); @@ -79,7 +77,7 @@ public class CollectionsController : Controller [HttpGet("{id}/details")] public async Task GetDetails(Guid orgId, Guid id) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await GetDetails_vNext(id); @@ -104,7 +102,7 @@ public class CollectionsController : Controller else { (var collection, var access) = await _collectionRepository.GetByIdWithAccessAsync(id, - _currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + _currentContext.UserId.Value, false); if (collection == null || collection.OrganizationId != orgId) { throw new NotFoundException(); @@ -117,7 +115,7 @@ public class CollectionsController : Controller [HttpGet("details")] public async Task> GetManyWithDetails(Guid orgId) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await GetManyWithDetails_vNext(orgId); @@ -132,7 +130,7 @@ public class CollectionsController : Controller // We always need to know which collections the current user is assigned to var assignedOrgCollections = await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId, - FlexibleCollectionsIsEnabled); + false); if (await _currentContext.ViewAllCollections(orgId) || await _currentContext.ManageUsers(orgId)) { @@ -159,7 +157,7 @@ public class CollectionsController : Controller [HttpGet("")] public async Task> Get(Guid orgId) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await GetByOrgId_vNext(orgId); @@ -191,7 +189,7 @@ public class CollectionsController : Controller public async Task> GetUser() { var collections = await _collectionRepository.GetManyByUserIdAsync( - _userService.GetProperUserId(User).Value, FlexibleCollectionsIsEnabled); + _userService.GetProperUserId(User).Value, false); var responses = collections.Select(c => new CollectionDetailsResponseModel(c)); return new ListResponseModel(responses); } @@ -199,7 +197,7 @@ public class CollectionsController : Controller [HttpGet("{id}/users")] public async Task> GetUsers(Guid orgId, Guid id) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await GetUsers_vNext(id); @@ -217,7 +215,8 @@ public class CollectionsController : Controller { var collection = model.ToCollection(orgId); - var authorized = FlexibleCollectionsIsEnabled + var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(orgId); + var authorized = flexibleCollectionsIsEnabled ? (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.Create)).Succeeded : await CanCreateCollection(orgId, collection.Id) || await CanEditCollectionAsync(orgId, collection.Id); if (!authorized) @@ -230,7 +229,7 @@ public class CollectionsController : Controller // Pre-flexible collections logic assigned Managers to collections they create var assignUserToCollection = - !FlexibleCollectionsIsEnabled && + !flexibleCollectionsIsEnabled && !await _currentContext.EditAnyCollection(orgId) && await _currentContext.EditAssignedCollections(orgId); var isNewCollection = collection.Id == default; @@ -258,7 +257,7 @@ public class CollectionsController : Controller [HttpPost("{id}")] public async Task Put(Guid orgId, Guid id, [FromBody] CollectionRequestModel model) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic return await Put_vNext(id, model); @@ -280,7 +279,7 @@ public class CollectionsController : Controller [HttpPut("{id}/users")] public async Task PutUsers(Guid orgId, Guid id, [FromBody] IEnumerable model) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic await PutUsers_vNext(id, model); @@ -299,14 +298,17 @@ public class CollectionsController : Controller [HttpPost("bulk-access")] [RequireFeature(FeatureFlagKeys.BulkCollectionAccess)] - // Also gated behind Flexible Collections flag because it only has new authorization logic. - // Could be removed if legacy authorization logic were implemented for many collections. - [RequireFeature(FeatureFlagKeys.FlexibleCollections)] - public async Task PostBulkCollectionAccess([FromBody] BulkCollectionAccessRequestModel model) + public async Task PostBulkCollectionAccess(Guid orgId, [FromBody] BulkCollectionAccessRequestModel model) { - var collections = await _collectionRepository.GetManyByManyIdsAsync(model.CollectionIds); + // Authorization logic assumes flexible collections is enabled + // Remove after all organizations have been migrated + if (!await FlexibleCollectionsIsEnabledAsync(orgId)) + { + throw new NotFoundException("Feature disabled."); + } - if (collections.Count != model.CollectionIds.Count()) + var collections = await _collectionRepository.GetManyByManyIdsAsync(model.CollectionIds); + if (collections.Count(c => c.OrganizationId == orgId) != model.CollectionIds.Count()) { throw new NotFoundException("One or more collections not found."); } @@ -328,7 +330,7 @@ public class CollectionsController : Controller [HttpPost("{id}/delete")] public async Task Delete(Guid orgId, Guid id) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic await Delete_vNext(id); @@ -349,7 +351,7 @@ public class CollectionsController : Controller [HttpPost("delete")] public async Task DeleteMany(Guid orgId, [FromBody] CollectionBulkDeleteRequestModel model) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Ids); @@ -385,7 +387,7 @@ public class CollectionsController : Controller [HttpPost("{id}/delete-user/{orgUserId}")] public async Task DeleteUser(Guid orgId, Guid id, Guid orgUserId) { - if (FlexibleCollectionsIsEnabled) + if (await FlexibleCollectionsIsEnabledAsync(orgId)) { // New flexible collections logic await DeleteUser_vNext(id, orgUserId); @@ -397,19 +399,9 @@ public class CollectionsController : Controller await _collectionService.DeleteUserAsync(collection, orgUserId); } - private void DeprecatedPermissionsGuard() - { - if (FlexibleCollectionsIsEnabled) - { - throw new FeatureUnavailableException("Flexible Collections is ON when it should be OFF."); - } - } - [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task GetCollectionAsync(Guid id, Guid orgId) { - DeprecatedPermissionsGuard(); - Collection collection = default; if (await _currentContext.ViewAllCollections(orgId)) { @@ -417,7 +409,7 @@ public class CollectionsController : Controller } else if (await _currentContext.ViewAssignedCollections(orgId)) { - collection = await _collectionRepository.GetByIdAsync(id, _currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + collection = await _collectionRepository.GetByIdAsync(id, _currentContext.UserId.Value, false); } if (collection == null || collection.OrganizationId != orgId) @@ -431,8 +423,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task CanCreateCollection(Guid orgId, Guid collectionId) { - DeprecatedPermissionsGuard(); - if (collectionId != default) { return false; @@ -445,8 +435,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task CanEditCollectionAsync(Guid orgId, Guid collectionId) { - DeprecatedPermissionsGuard(); - if (collectionId == default) { return false; @@ -460,7 +448,7 @@ public class CollectionsController : Controller if (await _currentContext.EditAssignedCollections(orgId)) { var collectionDetails = - await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, false); return collectionDetails != null; } @@ -470,8 +458,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task CanDeleteCollectionAsync(Guid orgId, Guid collectionId) { - DeprecatedPermissionsGuard(); - if (collectionId == default) { return false; @@ -485,7 +471,7 @@ public class CollectionsController : Controller if (await _currentContext.DeleteAssignedCollections(orgId)) { var collectionDetails = - await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, false); return collectionDetails != null; } @@ -495,8 +481,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task DeleteAnyCollection(Guid orgId) { - DeprecatedPermissionsGuard(); - return await _currentContext.OrganizationAdmin(orgId) || (_currentContext.Organizations?.Any(o => o.Id == orgId && (o.Permissions?.DeleteAnyCollection ?? false)) ?? false); @@ -505,8 +489,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task CanViewCollectionAsync(Guid orgId, Guid collectionId) { - DeprecatedPermissionsGuard(); - if (collectionId == default) { return false; @@ -520,7 +502,7 @@ public class CollectionsController : Controller if (await _currentContext.ViewAssignedCollections(orgId)) { var collectionDetails = - await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value, false); return collectionDetails != null; } @@ -530,8 +512,6 @@ public class CollectionsController : Controller [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] private async Task ViewAtLeastOneCollectionAsync(Guid orgId) { - DeprecatedPermissionsGuard(); - return await _currentContext.ViewAllCollections(orgId) || await _currentContext.ViewAssignedCollections(orgId); } @@ -564,7 +544,7 @@ public class CollectionsController : Controller { // We always need to know which collections the current user is assigned to var assignedOrgCollections = await _collectionRepository - .GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId, FlexibleCollectionsIsEnabled); + .GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId, false); var readAllAuthorized = (await _authorizationService.AuthorizeAsync(User, CollectionOperations.ReadAllWithAccess(orgId))).Succeeded; @@ -604,7 +584,7 @@ public class CollectionsController : Controller } else { - var assignedCollections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + var assignedCollections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, false); orgCollections = assignedCollections.Where(c => c.OrganizationId == orgId && c.Manage).ToList(); } @@ -676,4 +656,10 @@ public class CollectionsController : Controller await _collectionService.DeleteUserAsync(collection, orgUserId); } + + private async Task FlexibleCollectionsIsEnabledAsync(Guid organizationId) + { + var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId); + return organizationAbility?.FlexibleCollections ?? false; + } } diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index 45e8bc9458..fb47602bc9 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -1,5 +1,4 @@ #nullable enable -using Bit.Core; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -20,33 +19,22 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public BulkCollectionAuthorizationHandler( ICurrentContext currentContext, ICollectionRepository collectionRepository, - IFeatureService featureService, IApplicationCacheService applicationCacheService) { _currentContext = currentContext; _collectionRepository = collectionRepository; - _featureService = featureService; _applicationCacheService = applicationCacheService; } protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, BulkCollectionOperationRequirement requirement, ICollection? resources) { - if (!FlexibleCollectionsIsEnabled) - { - // Flexible collections is OFF, should not be using this handler - throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); - } - // Establish pattern of authorization handler null checking passed resources if (resources == null || !resources.Any()) { @@ -281,9 +269,6 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public CollectionAuthorizationHandler( ICurrentContext currentContext, IFeatureService featureService) @@ -30,12 +26,6 @@ public class CollectionAuthorizationHandler : AuthorizationHandler _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public GroupAuthorizationHandler( ICurrentContext currentContext, IFeatureService featureService, @@ -34,12 +30,6 @@ public class GroupAuthorizationHandler : AuthorizationHandler _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public OrganizationUserAuthorizationHandler( ICurrentContext currentContext, IFeatureService featureService, @@ -34,12 +30,6 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler, ISubscriber, IStorable, IStorabl return providers[provider]; } - public void UpdateFromLicense( - OrganizationLicense license, - bool flexibleCollectionsMvpIsEnabled, - bool flexibleCollectionsV1IsEnabled) + public void UpdateFromLicense(OrganizationLicense license) { + // The following properties are intentionally excluded from being updated: + // - Id - self-hosted org will have its own unique Guid + // - MaxStorageGb - not enforced for self-hosted because we're not providing the storage + // - FlexibleCollections - the self-hosted organization must do its own data migration to set this property, it cannot be updated from cloud + Name = license.Name; BusinessName = license.BusinessName; BillingEmail = license.BillingEmail; @@ -275,7 +277,7 @@ public class Organization : ITableObject, ISubscriber, IStorable, IStorabl UseSecretsManager = license.UseSecretsManager; SmSeats = license.SmSeats; SmServiceAccounts = license.SmServiceAccounts; - LimitCollectionCreationDeletion = !flexibleCollectionsMvpIsEnabled || license.LimitCollectionCreationDeletion; - AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled || license.AllowAdminAccessToAllCollectionItems; + LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion; + AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems; } } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs b/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs index ddca0d3c8a..39cc5a1d98 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/SelfHostedOrganizationDetails.cs @@ -145,7 +145,8 @@ public class SelfHostedOrganizationDetails : Organization MaxAutoscaleSeats = MaxAutoscaleSeats, OwnersNotifiedOfAutoscaling = OwnersNotifiedOfAutoscaling, LimitCollectionCreationDeletion = LimitCollectionCreationDeletion, - AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems + AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems, + FlexibleCollections = FlexibleCollections }; } } diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 3bd493e309..1355a15120 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -65,8 +65,6 @@ public class OrganizationService : IOrganizationService private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; private readonly IFeatureService _featureService; - private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public OrganizationService( IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, @@ -418,6 +416,9 @@ public class OrganizationService : IOrganizationService } } + /// + /// Create a new organization in a cloud environment + /// public async Task> SignUpAsync(OrganizationSignup signup, bool provider = false) { @@ -440,8 +441,9 @@ public class OrganizationService : IOrganizationService await ValidateSignUpPoliciesAsync(signup.Owner.Id); } - var flexibleCollectionsIsEnabled = - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + var flexibleCollectionsSignupEnabled = + _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsSignup, _currentContext); + var flexibleCollectionsV1IsEnabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext); @@ -482,7 +484,15 @@ public class OrganizationService : IOrganizationService Status = OrganizationStatusType.Created, UsePasswordManager = true, UseSecretsManager = signup.UseSecretsManager, - LimitCollectionCreationDeletion = !flexibleCollectionsIsEnabled, + + // This feature flag indicates that new organizations should be automatically onboarded to + // Flexible Collections enhancements + FlexibleCollections = flexibleCollectionsSignupEnabled, + + // These collection management settings smooth the migration for existing organizations by disabling some FC behavior. + // If the organization is onboarded to Flexible Collections on signup, we turn them OFF to enable all new behaviour. + // If the organization is NOT onboarded now, they will have to be migrated later, so they default to ON to limit FC changes on migration. + LimitCollectionCreationDeletion = !flexibleCollectionsSignupEnabled, AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled }; @@ -534,6 +544,9 @@ public class OrganizationService : IOrganizationService } } + /// + /// Create a new organization on a self-hosted instance + /// public async Task> SignUpAsync( OrganizationLicense license, User owner, string ownerKey, string collectionName, string publicKey, string privateKey) @@ -558,10 +571,8 @@ public class OrganizationService : IOrganizationService await ValidateSignUpPoliciesAsync(owner.Id); - var flexibleCollectionsMvpIsEnabled = - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - var flexibleCollectionsV1IsEnabled = - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext); + var flexibleCollectionsSignupEnabled = + _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsSignup, _currentContext); var organization = new Organization { @@ -603,8 +614,12 @@ public class OrganizationService : IOrganizationService UseSecretsManager = license.UseSecretsManager, SmSeats = license.SmSeats, SmServiceAccounts = license.SmServiceAccounts, - LimitCollectionCreationDeletion = !flexibleCollectionsMvpIsEnabled || license.LimitCollectionCreationDeletion, - AllowAdminAccessToAllCollectionItems = !flexibleCollectionsV1IsEnabled || license.AllowAdminAccessToAllCollectionItems + LimitCollectionCreationDeletion = license.LimitCollectionCreationDeletion, + AllowAdminAccessToAllCollectionItems = license.AllowAdminAccessToAllCollectionItems, + + // This feature flag indicates that new organizations should be automatically onboarded to + // Flexible Collections enhancements + FlexibleCollections = flexibleCollectionsSignupEnabled, }; var result = await SignUpAsync(organization, owner.Id, ownerKey, collectionName, false); @@ -616,6 +631,10 @@ public class OrganizationService : IOrganizationService return result; } + /// + /// Private helper method to create a new organization. + /// This is common code used by both the cloud and self-hosted methods. + /// private async Task> SignUpAsync(Organization organization, Guid ownerId, string ownerKey, string collectionName, bool withPayment) { @@ -829,6 +848,7 @@ public class OrganizationService : IOrganizationService { var inviteTypes = new HashSet(invites.Where(i => i.invite.Type.HasValue) .Select(i => i.invite.Type.Value)); + if (invitingUserId.HasValue && inviteTypes.Count > 0) { foreach (var (invite, _) in invites) @@ -2008,7 +2028,11 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Custom users can only grant the same custom permissions that they have."); } - if (FlexibleCollectionsIsEnabled && newType == OrganizationUserType.Manager && oldType is not OrganizationUserType.Manager) + // TODO: pass in the whole organization object when this is refactored into a command/query + // See AC-2036 + var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId); + var flexibleCollectionsEnabled = organizationAbility?.FlexibleCollections ?? false; + if (flexibleCollectionsEnabled && newType == OrganizationUserType.Manager && oldType is not OrganizationUserType.Manager) { throw new BadRequestException("Manager role is deprecated after Flexible Collections."); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index a71032ea23..3f5d618e6c 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -96,12 +96,17 @@ public static class FeatureFlagKeys public const string VaultOnboarding = "vault-onboarding"; public const string AutofillV2 = "autofill-v2"; public const string BrowserFilelessImport = "browser-fileless-import"; - public const string FlexibleCollections = "flexible-collections"; + /// + /// Deprecated - never used, do not use. Will always default to false. Will be deleted as part of Flexible Collections cleanup + /// + public const string FlexibleCollections = "flexible-collections-disabled-do-not-use"; public const string FlexibleCollectionsV1 = "flexible-collections-v-1"; // v-1 is intentional public const string BulkCollectionAccess = "bulk-collection-access"; public const string AutofillOverlay = "autofill-overlay"; public const string ItemShare = "item-share"; public const string KeyRotationImprovements = "key-rotation-improvements"; + public const string FlexibleCollectionsMigration = "flexible-collections-migration"; + public const string FlexibleCollectionsSignup = "flexible-collections-signup"; public static List GetAllKeys() { diff --git a/src/Core/Context/CurrentContext.cs b/src/Core/Context/CurrentContext.cs index b346c20f3e..129e90e39d 100644 --- a/src/Core/Context/CurrentContext.cs +++ b/src/Core/Context/CurrentContext.cs @@ -5,7 +5,6 @@ using Bit.Core.AdminConsole.Models.Data.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Entities; using Bit.Core.Enums; -using Bit.Core.Exceptions; using Bit.Core.Identity; using Bit.Core.Models.Data; using Bit.Core.Repositories; @@ -26,8 +25,6 @@ public class CurrentContext : ICurrentContext private IEnumerable _providerOrganizationProviderDetails; private IEnumerable _providerUserOrganizations; - private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, this); - public virtual HttpContext HttpContext { get; set; } public virtual Guid? UserId { get; set; } public virtual User User { get; set; } @@ -283,11 +280,6 @@ public class CurrentContext : ICurrentContext public async Task OrganizationManager(Guid orgId) { - if (FlexibleCollectionsIsEnabled) - { - throw new FeatureUnavailableException("Flexible Collections is ON when it should be OFF."); - } - return await OrganizationAdmin(orgId) || (Organizations?.Any(o => o.Id == orgId && o.Type == OrganizationUserType.Manager) ?? false); } @@ -350,22 +342,12 @@ public class CurrentContext : ICurrentContext public async Task EditAssignedCollections(Guid orgId) { - if (FlexibleCollectionsIsEnabled) - { - throw new FeatureUnavailableException("Flexible Collections is ON when it should be OFF."); - } - return await OrganizationManager(orgId) || (Organizations?.Any(o => o.Id == orgId && (o.Permissions?.EditAssignedCollections ?? false)) ?? false); } public async Task DeleteAssignedCollections(Guid orgId) { - if (FlexibleCollectionsIsEnabled) - { - throw new FeatureUnavailableException("Flexible Collections is ON when it should be OFF."); - } - return await OrganizationManager(orgId) || (Organizations?.Any(o => o.Id == orgId && (o.Permissions?.DeleteAssignedCollections ?? false)) ?? false); } @@ -378,11 +360,6 @@ public class CurrentContext : ICurrentContext * This entire method will be moved to the CollectionAuthorizationHandler in the future */ - if (FlexibleCollectionsIsEnabled) - { - throw new FeatureUnavailableException("Flexible Collections is ON when it should be OFF."); - } - var org = GetOrganization(orgId); return await EditAssignedCollections(orgId) || await DeleteAssignedCollections(orgId) diff --git a/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs b/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs index ac2e1b1012..62c46460aa 100644 --- a/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommand.cs @@ -2,7 +2,6 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities; -using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; @@ -18,21 +17,15 @@ public class UpdateOrganizationLicenseCommand : IUpdateOrganizationLicenseComman private readonly ILicensingService _licensingService; private readonly IGlobalSettings _globalSettings; private readonly IOrganizationService _organizationService; - private readonly IFeatureService _featureService; - private readonly ICurrentContext _currentContext; public UpdateOrganizationLicenseCommand( ILicensingService licensingService, IGlobalSettings globalSettings, - IOrganizationService organizationService, - IFeatureService featureService, - ICurrentContext currentContext) + IOrganizationService organizationService) { _licensingService = licensingService; _globalSettings = globalSettings; _organizationService = organizationService; - _featureService = featureService; - _currentContext = currentContext; } public async Task UpdateLicenseAsync(SelfHostedOrganizationDetails selfHostedOrganization, @@ -65,10 +58,8 @@ public class UpdateOrganizationLicenseCommand : IUpdateOrganizationLicenseComman private async Task UpdateOrganizationAsync(SelfHostedOrganizationDetails selfHostedOrganizationDetails, OrganizationLicense license) { - var flexibleCollectionsMvpIsEnabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - var flexibleCollectionsV1IsEnabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext); var organization = selfHostedOrganizationDetails.ToOrganization(); - organization.UpdateFromLicense(license, flexibleCollectionsMvpIsEnabled, flexibleCollectionsV1IsEnabled); + organization.UpdateFromLicense(license); await _organizationService.ReplaceAndUpdateCacheAsync(organization); } diff --git a/src/Core/Services/IApplicationCacheService.cs b/src/Core/Services/IApplicationCacheService.cs index 9c9b8ca550..ee47cf29fd 100644 --- a/src/Core/Services/IApplicationCacheService.cs +++ b/src/Core/Services/IApplicationCacheService.cs @@ -8,6 +8,9 @@ namespace Bit.Core.Services; public interface IApplicationCacheService { Task> GetOrganizationAbilitiesAsync(); +#nullable enable + Task GetOrganizationAbilityAsync(Guid orgId); +#nullable disable Task> GetProviderAbilitiesAsync(); Task UpsertOrganizationAbilityAsync(Organization organization); Task UpsertProviderAbilityAsync(Provider provider); diff --git a/src/Core/Services/Implementations/InMemoryApplicationCacheService.cs b/src/Core/Services/Implementations/InMemoryApplicationCacheService.cs index 63db4a88b6..256a9a08a4 100644 --- a/src/Core/Services/Implementations/InMemoryApplicationCacheService.cs +++ b/src/Core/Services/Implementations/InMemoryApplicationCacheService.cs @@ -30,6 +30,15 @@ public class InMemoryApplicationCacheService : IApplicationCacheService return _orgAbilities; } +#nullable enable + public async Task GetOrganizationAbilityAsync(Guid organizationId) + { + (await GetOrganizationAbilitiesAsync()) + .TryGetValue(organizationId, out var organizationAbility); + return organizationAbility; + } +#nullable disable + public virtual async Task> GetProviderAbilitiesAsync() { await InitProviderAbilitiesAsync(); diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 5517be1689..665f99aadf 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -788,7 +788,7 @@ public class CipherService : ICipherService { collection.SetNewId(); newCollections.Add(collection); - if (UseFlexibleCollections) + if (org.FlexibleCollections) { newCollectionUsers.Add(new CollectionUser { diff --git a/src/Events/Controllers/CollectController.cs b/src/Events/Controllers/CollectController.cs index 7c7962309c..144c248e46 100644 --- a/src/Events/Controllers/CollectController.cs +++ b/src/Events/Controllers/CollectController.cs @@ -35,8 +35,6 @@ public class CollectController : Controller _featureService = featureService; } - bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - [HttpPost] public async Task Post([FromBody] IEnumerable model) { diff --git a/test/Api.Test/Controllers/CollectionsControllerTests.cs b/test/Api.Test/Controllers/CollectionsControllerTests.cs index f8f3b890bb..6155ad0f77 100644 --- a/test/Api.Test/Controllers/CollectionsControllerTests.cs +++ b/test/Api.Test/Controllers/CollectionsControllerTests.cs @@ -2,16 +2,14 @@ using Bit.Api.Controllers; using Bit.Api.Models.Request; using Bit.Api.Vault.AuthorizationHandlers.Collections; -using Bit.Core; -using Bit.Core.AdminConsole.Entities; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Models.Data; +using Bit.Core.Models.Data.Organizations; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Test.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Authorization; @@ -22,16 +20,17 @@ namespace Bit.Api.Test.Controllers; [ControllerCustomize(typeof(CollectionsController))] [SutProviderCustomize] -[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class CollectionsControllerTests { [Theory, BitAutoData] - public async Task Post_Success(Guid orgId, CollectionRequestModel collectionRequest, + public async Task Post_Success(OrganizationAbility organizationAbility, CollectionRequestModel collectionRequest, SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + Collection ExpectedCollection() => Arg.Is(c => c.Name == collectionRequest.Name && c.ExternalId == collectionRequest.ExternalId && - c.OrganizationId == orgId); + c.OrganizationId == organizationAbility.Id); sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), @@ -39,7 +38,7 @@ public class CollectionsControllerTests Arg.Is>(r => r.Contains(BulkCollectionOperations.Create))) .Returns(AuthorizationResult.Success()); - _ = await sutProvider.Sut.Post(orgId, collectionRequest); + _ = await sutProvider.Sut.Post(organizationAbility.Id, collectionRequest); await sutProvider.GetDependency() .Received(1) @@ -49,8 +48,11 @@ public class CollectionsControllerTests [Theory, BitAutoData] public async Task Put_Success(Collection collection, CollectionRequestModel collectionRequest, - SutProvider sutProvider) + SutProvider sutProvider, OrganizationAbility organizationAbility) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collection.OrganizationId = organizationAbility.Id; + Collection ExpectedCollection() => Arg.Is(c => c.Id == collection.Id && c.Name == collectionRequest.Name && c.ExternalId == collectionRequest.ExternalId && c.OrganizationId == collection.OrganizationId); @@ -75,8 +77,11 @@ public class CollectionsControllerTests [Theory, BitAutoData] public async Task Put_WithNoCollectionPermission_ThrowsNotFound(Collection collection, CollectionRequestModel collectionRequest, - SutProvider sutProvider) + SutProvider sutProvider, OrganizationAbility organizationAbility) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collection.OrganizationId = organizationAbility.Id; + sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), collection, @@ -91,8 +96,11 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_WithReadAllPermissions_GetsAllCollections(Organization organization, Guid userId, SutProvider sutProvider) + public async Task GetOrganizationCollectionsWithGroups_WithReadAllPermissions_GetsAllCollections(OrganizationAbility organizationAbility, + Guid userId, SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency() @@ -102,18 +110,20 @@ public class CollectionsControllerTests Arg.Is>(requirements => requirements.Cast().All(operation => operation.Name == nameof(CollectionOperations.ReadAllWithAccess) - && operation.OrganizationId == organization.Id))) + && operation.OrganizationId == organizationAbility.Id))) .Returns(AuthorizationResult.Success()); - await sutProvider.Sut.GetManyWithDetails(organization.Id); + await sutProvider.Sut.GetManyWithDetails(organizationAbility.Id); - await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organization.Id, Arg.Any()); - await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdWithAccessAsync(organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organizationAbility.Id, Arg.Any()); + await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdWithAccessAsync(organizationAbility.Id); } [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_MissingReadAllPermissions_GetsAssignedCollections(Organization organization, Guid userId, SutProvider sutProvider) + public async Task GetOrganizationCollectionsWithGroups_MissingReadAllPermissions_GetsAssignedCollections( + OrganizationAbility organizationAbility, Guid userId, SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency() @@ -123,7 +133,7 @@ public class CollectionsControllerTests Arg.Is>(requirements => requirements.Cast().All(operation => operation.Name == nameof(CollectionOperations.ReadAllWithAccess) - && operation.OrganizationId == organization.Id))) + && operation.OrganizationId == organizationAbility.Id))) .Returns(AuthorizationResult.Failed()); sutProvider.GetDependency() @@ -135,15 +145,19 @@ public class CollectionsControllerTests operation.Name == nameof(BulkCollectionOperations.ReadWithAccess)))) .Returns(AuthorizationResult.Success()); - await sutProvider.Sut.GetManyWithDetails(organization.Id); + await sutProvider.Sut.GetManyWithDetails(organizationAbility.Id); - await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organization.Id, Arg.Any()); - await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdWithAccessAsync(organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organizationAbility.Id, Arg.Any()); + await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdWithAccessAsync(organizationAbility.Id); } [Theory, BitAutoData] - public async Task GetOrganizationCollections_WithReadAllPermissions_GetsAllCollections(Organization organization, ICollection collections, Guid userId, SutProvider sutProvider) + public async Task GetOrganizationCollections_WithReadAllPermissions_GetsAllCollections( + OrganizationAbility organizationAbility, List collections, Guid userId, SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collections.ForEach(c => c.OrganizationId = organizationAbility.Id); + sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency() @@ -153,26 +167,30 @@ public class CollectionsControllerTests Arg.Is>(requirements => requirements.Cast().All(operation => operation.Name == nameof(CollectionOperations.ReadAll) - && operation.OrganizationId == organization.Id))) + && operation.OrganizationId == organizationAbility.Id))) .Returns(AuthorizationResult.Success()); sutProvider.GetDependency() - .GetManyByOrganizationIdAsync(organization.Id) + .GetManyByOrganizationIdAsync(organizationAbility.Id) .Returns(collections); - var response = await sutProvider.Sut.Get(organization.Id); + var response = await sutProvider.Sut.Get(organizationAbility.Id); - await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdAsync(organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdAsync(organizationAbility.Id); Assert.Equal(collections.Count, response.Data.Count()); } [Theory, BitAutoData] - public async Task GetOrganizationCollections_MissingReadAllPermissions_GetsManageableCollections(Organization organization, ICollection collections, Guid userId, SutProvider sutProvider) + public async Task GetOrganizationCollections_MissingReadAllPermissions_GetsManageableCollections( + OrganizationAbility organizationAbility, List collections, Guid userId, SutProvider sutProvider) { - collections.First().OrganizationId = organization.Id; - collections.First().Manage = true; - collections.Skip(1).First().OrganizationId = organization.Id; + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collections.ForEach(c => c.OrganizationId = organizationAbility.Id); + collections.ForEach(c => c.Manage = false); + + var managedCollection = collections.First(); + managedCollection.Manage = true; sutProvider.GetDependency().UserId.Returns(userId); @@ -183,7 +201,7 @@ public class CollectionsControllerTests Arg.Is>(requirements => requirements.Cast().All(operation => operation.Name == nameof(CollectionOperations.ReadAll) - && operation.OrganizationId == organization.Id))) + && operation.OrganizationId == organizationAbility.Id))) .Returns(AuthorizationResult.Failed()); sutProvider.GetDependency() @@ -196,22 +214,27 @@ public class CollectionsControllerTests .Returns(AuthorizationResult.Success()); sutProvider.GetDependency() - .GetManyByUserIdAsync(userId, true) + .GetManyByUserIdAsync(userId, false) .Returns(collections); - var result = await sutProvider.Sut.Get(organization.Id); + var result = await sutProvider.Sut.Get(organizationAbility.Id); - await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdAsync(organization.Id); - await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(userId, true); + await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdAsync(organizationAbility.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(userId, false); Assert.Single(result.Data); - Assert.All(result.Data, c => Assert.Equal(organization.Id, c.OrganizationId)); + Assert.All(result.Data, c => Assert.Equal(organizationAbility.Id, c.OrganizationId)); + Assert.All(result.Data, c => Assert.Equal(managedCollection.Id, c.Id)); } [Theory, BitAutoData] - public async Task DeleteMany_Success(Guid orgId, Collection collection1, Collection collection2, SutProvider sutProvider) + public async Task DeleteMany_Success(OrganizationAbility organizationAbility, Collection collection1, Collection collection2, + SutProvider sutProvider) { // Arrange + var orgId = organizationAbility.Id; + ArrangeOrganizationAbility(sutProvider, organizationAbility); + var model = new CollectionBulkDeleteRequestModel { Ids = new[] { collection1.Id, collection2.Id } @@ -251,9 +274,13 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task DeleteMany_PermissionDenied_ThrowsNotFound(Guid orgId, Collection collection1, Collection collection2, SutProvider sutProvider) + public async Task DeleteMany_PermissionDenied_ThrowsNotFound(OrganizationAbility organizationAbility, Collection collection1, + Collection collection2, SutProvider sutProvider) { // Arrange + var orgId = organizationAbility.Id; + ArrangeOrganizationAbility(sutProvider, organizationAbility); + var model = new CollectionBulkDeleteRequestModel { Ids = new[] { collection1.Id, collection2.Id } @@ -292,9 +319,13 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task PostBulkCollectionAccess_Success(User actingUser, ICollection collections, SutProvider sutProvider) + public async Task PostBulkCollectionAccess_Success(User actingUser, List collections, + OrganizationAbility organizationAbility, SutProvider sutProvider) { // Arrange + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collections.ForEach(c => c.OrganizationId = organizationAbility.Id); + var userId = Guid.NewGuid(); var groupId = Guid.NewGuid(); var model = new BulkCollectionAccessRequestModel @@ -321,7 +352,7 @@ public class CollectionsControllerTests IEnumerable ExpectedCollectionAccess() => Arg.Is>(cols => cols.SequenceEqual(collections)); // Act - await sutProvider.Sut.PostBulkCollectionAccess(model); + await sutProvider.Sut.PostBulkCollectionAccess(organizationAbility.Id, model); // Assert await sutProvider.GetDependency().Received().AuthorizeAsync( @@ -338,8 +369,13 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task PostBulkCollectionAccess_CollectionsNotFound_Throws(User actingUser, ICollection collections, SutProvider sutProvider) + public async Task PostBulkCollectionAccess_CollectionsNotFound_Throws(User actingUser, + OrganizationAbility organizationAbility, List collections, + SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collections.ForEach(c => c.OrganizationId = organizationAbility.Id); + var userId = Guid.NewGuid(); var groupId = Guid.NewGuid(); var model = new BulkCollectionAccessRequestModel @@ -356,7 +392,8 @@ public class CollectionsControllerTests .GetManyByManyIdsAsync(model.CollectionIds) .Returns(collections.Skip(1).ToList()); - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.PostBulkCollectionAccess(model)); + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.PostBulkCollectionAccess(organizationAbility.Id, model)); Assert.Equal("One or more collections not found.", exception.Message); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().AuthorizeAsync( @@ -369,8 +406,81 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task PostBulkCollectionAccess_AccessDenied_Throws(User actingUser, ICollection collections, SutProvider sutProvider) + public async Task PostBulkCollectionAccess_CollectionsBelongToDifferentOrganizations_Throws(User actingUser, + OrganizationAbility organizationAbility, List collections, + SutProvider sutProvider) { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + + // First collection has a different orgId + collections.Skip(1).ToList().ForEach(c => c.OrganizationId = organizationAbility.Id); + + var userId = Guid.NewGuid(); + var groupId = Guid.NewGuid(); + var model = new BulkCollectionAccessRequestModel + { + CollectionIds = collections.Select(c => c.Id), + Users = new[] { new SelectionReadOnlyRequestModel { Id = userId, Manage = true } }, + Groups = new[] { new SelectionReadOnlyRequestModel { Id = groupId, ReadOnly = true } }, + }; + + sutProvider.GetDependency() + .UserId.Returns(actingUser.Id); + + sutProvider.GetDependency() + .GetManyByManyIdsAsync(model.CollectionIds) + .Returns(collections); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.PostBulkCollectionAccess(organizationAbility.Id, model)); + + Assert.Equal("One or more collections not found.", exception.Message); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().AuthorizeAsync( + Arg.Any(), + Arg.Any>(), + Arg.Any>() + ); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .AddAccessAsync(default, default, default); + } + + [Theory, BitAutoData] + public async Task PostBulkCollectionAccess_FlexibleCollectionsDisabled_Throws(OrganizationAbility organizationAbility, List collections, + SutProvider sutProvider) + { + organizationAbility.FlexibleCollections = false; + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); + + var userId = Guid.NewGuid(); + var groupId = Guid.NewGuid(); + var model = new BulkCollectionAccessRequestModel + { + CollectionIds = collections.Select(c => c.Id), + Users = new[] { new SelectionReadOnlyRequestModel { Id = userId, Manage = true } }, + Groups = new[] { new SelectionReadOnlyRequestModel { Id = groupId, ReadOnly = true } }, + }; + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.PostBulkCollectionAccess(organizationAbility.Id, model)); + + Assert.Equal("Feature disabled.", exception.Message); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().AuthorizeAsync( + Arg.Any(), + Arg.Any>(), + Arg.Any>() + ); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .AddAccessAsync(default, default, default); + } + + [Theory, BitAutoData] + public async Task PostBulkCollectionAccess_AccessDenied_Throws(User actingUser, List collections, + OrganizationAbility organizationAbility, SutProvider sutProvider) + { + ArrangeOrganizationAbility(sutProvider, organizationAbility); + collections.ForEach(c => c.OrganizationId = organizationAbility.Id); + var userId = Guid.NewGuid(); var groupId = Guid.NewGuid(); var model = new BulkCollectionAccessRequestModel @@ -396,7 +506,7 @@ public class CollectionsControllerTests IEnumerable ExpectedCollectionAccess() => Arg.Is>(cols => cols.SequenceEqual(collections)); - await Assert.ThrowsAsync(() => sutProvider.Sut.PostBulkCollectionAccess(model)); + await Assert.ThrowsAsync(() => sutProvider.Sut.PostBulkCollectionAccess(organizationAbility.Id, model)); await sutProvider.GetDependency().Received().AuthorizeAsync( Arg.Any(), ExpectedCollectionAccess(), @@ -406,4 +516,12 @@ public class CollectionsControllerTests await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .AddAccessAsync(default, default, default); } + + private void ArrangeOrganizationAbility(SutProvider sutProvider, OrganizationAbility organizationAbility) + { + organizationAbility.FlexibleCollections = true; + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); + } } diff --git a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs index 092412a3fb..63406c00db 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs @@ -1,6 +1,5 @@ using System.Security.Claims; using Bit.Api.Vault.AuthorizationHandlers.Collections; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -9,7 +8,6 @@ using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Test.AutoFixture; using Bit.Core.Test.Vault.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -20,7 +18,6 @@ using Xunit; namespace Bit.Api.Test.Vault.AuthorizationHandlers; [SutProviderCustomize] -[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class BulkCollectionAuthorizationHandlerTests { [Theory, CollectionCustomization] @@ -35,7 +32,7 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -44,7 +41,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -61,7 +57,7 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, false); + ArrangeOrganizationAbility(sutProvider, organization, false); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -70,7 +66,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -97,7 +92,7 @@ public class BulkCollectionAuthorizationHandlerTests ManageUsers = false }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -106,7 +101,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -117,10 +111,12 @@ public class BulkCollectionAuthorizationHandlerTests [Theory, BitAutoData, CollectionCustomization] public async Task CanCreateAsync_WhenMissingOrgAccess_NoSuccess( Guid userId, - ICollection collections, + CurrentContextOrganization organization, + List collections, SutProvider sutProvider) { - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(collections.First().OrganizationId, true); + collections.ForEach(c => c.OrganizationId = organization.Id); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Create }, @@ -130,7 +126,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -747,7 +742,7 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -756,8 +751,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync() - .Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -778,7 +771,7 @@ public class BulkCollectionAuthorizationHandlerTests DeleteAnyCollection = true }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -787,8 +780,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync() - .Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -806,13 +797,11 @@ public class BulkCollectionAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, false); + ArrangeOrganizationAbility(sutProvider, organization, false); sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId, Arg.Any()).Returns(collections); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync() - .Returns(organizationAbilities); foreach (var c in collections) { @@ -849,7 +838,7 @@ public class BulkCollectionAuthorizationHandlerTests ManageUsers = false }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { BulkCollectionOperations.Delete }, @@ -858,8 +847,6 @@ public class BulkCollectionAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync() - .Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -981,17 +968,16 @@ public class BulkCollectionAuthorizationHandlerTests } } - private static Dictionary ArrangeOrganizationAbilitiesDictionary(Guid orgId, - bool limitCollectionCreationDeletion) + private static void ArrangeOrganizationAbility( + SutProvider sutProvider, + CurrentContextOrganization organization, bool limitCollectionCreationDeletion) { - return new Dictionary - { - { orgId, - new OrganizationAbility - { - LimitCollectionCreationDeletion = limitCollectionCreationDeletion - } - } - }; + var organizationAbility = new OrganizationAbility(); + organizationAbility.Id = organization.Id; + organizationAbility.FlexibleCollections = true; + organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); } } diff --git a/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs index 5bd7b6f849..ad06abd949 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs @@ -1,10 +1,8 @@ using System.Security.Claims; using Bit.Api.Vault.AuthorizationHandlers.Collections; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Models.Data; -using Bit.Core.Test.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Authorization; @@ -14,7 +12,6 @@ using Xunit; namespace Bit.Api.Test.Vault.AuthorizationHandlers; [SutProviderCustomize] -[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class CollectionAuthorizationHandlerTests { [Theory] diff --git a/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs index 79d1ebada5..8ba03930ef 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs @@ -1,12 +1,10 @@ using System.Security.Claims; using Bit.Api.Vault.AuthorizationHandlers.Groups; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations; using Bit.Core.Services; -using Bit.Core.Test.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Authorization; @@ -16,7 +14,6 @@ using Xunit; namespace Bit.Api.Test.Vault.AuthorizationHandlers; [SutProviderCustomize] -[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class GroupAuthorizationHandlerTests { [Theory] @@ -30,7 +27,7 @@ public class GroupAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { GroupOperations.ReadAll(organization.Id) }, @@ -39,7 +36,6 @@ public class GroupAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -54,7 +50,7 @@ public class GroupAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { GroupOperations.ReadAll(organization.Id) }, @@ -64,7 +60,6 @@ public class GroupAuthorizationHandlerTests sutProvider.GetDependency() .UserId .Returns(userId); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency() .ProviderUserForOrgAsync(organization.Id) .Returns(true); @@ -97,7 +92,7 @@ public class GroupAuthorizationHandlerTests ManageUsers = manageUsers }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, limitCollectionCreationDeletion); + ArrangeOrganizationAbility(sutProvider, organization, limitCollectionCreationDeletion); var context = new AuthorizationHandlerContext( new[] { GroupOperations.ReadAll(organization.Id) }, @@ -106,7 +101,6 @@ public class GroupAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -133,7 +127,7 @@ public class GroupAuthorizationHandlerTests AccessImportExport = false }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { GroupOperations.ReadAll(organization.Id) }, @@ -142,7 +136,6 @@ public class GroupAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -153,20 +146,19 @@ public class GroupAuthorizationHandlerTests [Theory, BitAutoData] public async Task CanReadAllAsync_WhenMissingOrgAccess_NoSuccess( Guid userId, - Guid organizationId, + CurrentContextOrganization organization, SutProvider sutProvider) { - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organizationId, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( - new[] { GroupOperations.ReadAll(organizationId) }, + new[] { GroupOperations.ReadAll(organization.Id) }, new ClaimsPrincipal(), null ); sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -210,17 +202,16 @@ public class GroupAuthorizationHandlerTests Assert.True(context.HasFailed); } - private static Dictionary ArrangeOrganizationAbilitiesDictionary(Guid orgId, - bool limitCollectionCreationDeletion) + private static void ArrangeOrganizationAbility( + SutProvider sutProvider, + CurrentContextOrganization organization, bool limitCollectionCreationDeletion) { - return new Dictionary - { - { orgId, - new OrganizationAbility - { - LimitCollectionCreationDeletion = limitCollectionCreationDeletion - } - } - }; + var organizationAbility = new OrganizationAbility(); + organizationAbility.Id = organization.Id; + organizationAbility.FlexibleCollections = true; + organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); } } diff --git a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs index c93d8a0f64..d6c22197fe 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs @@ -1,12 +1,10 @@ using System.Security.Claims; using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; -using Bit.Core; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations; using Bit.Core.Services; -using Bit.Core.Test.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Authorization; @@ -16,7 +14,6 @@ using Xunit; namespace Bit.Api.Test.Vault.AuthorizationHandlers; [SutProviderCustomize] -[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class OrganizationUserAuthorizationHandlerTests { [Theory] @@ -30,7 +27,7 @@ public class OrganizationUserAuthorizationHandlerTests organization.Type = userType; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { OrganizationUserOperations.ReadAll(organization.Id) }, @@ -39,7 +36,6 @@ public class OrganizationUserAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -54,7 +50,7 @@ public class OrganizationUserAuthorizationHandlerTests organization.Type = OrganizationUserType.User; organization.Permissions = new Permissions(); - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { OrganizationUserOperations.ReadAll(organization.Id) }, @@ -64,7 +60,6 @@ public class OrganizationUserAuthorizationHandlerTests sutProvider.GetDependency() .UserId .Returns(userId); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency() .ProviderUserForOrgAsync(organization.Id) .Returns(true); @@ -97,7 +92,7 @@ public class OrganizationUserAuthorizationHandlerTests ManageUsers = manageUsers }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, limitCollectionCreationDeletion); + ArrangeOrganizationAbility(sutProvider, organization, limitCollectionCreationDeletion); var context = new AuthorizationHandlerContext( new[] { OrganizationUserOperations.ReadAll(organization.Id) }, @@ -106,7 +101,6 @@ public class OrganizationUserAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); await sutProvider.Sut.HandleAsync(context); @@ -132,7 +126,7 @@ public class OrganizationUserAuthorizationHandlerTests ManageUsers = false }; - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organization.Id, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( new[] { OrganizationUserOperations.ReadAll(organization.Id) }, @@ -141,7 +135,6 @@ public class OrganizationUserAuthorizationHandlerTests sutProvider.GetDependency().UserId.Returns(actingUserId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -152,20 +145,19 @@ public class OrganizationUserAuthorizationHandlerTests [Theory, BitAutoData] public async Task HandleRequirementAsync_WhenMissingOrgAccess_NoSuccess( Guid userId, - Guid organizationId, + CurrentContextOrganization organization, SutProvider sutProvider) { - var organizationAbilities = ArrangeOrganizationAbilitiesDictionary(organizationId, true); + ArrangeOrganizationAbility(sutProvider, organization, true); var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(organizationId) }, + new[] { OrganizationUserOperations.ReadAll(organization.Id) }, new ClaimsPrincipal(), null ); sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); - sutProvider.GetDependency().GetOrganizationAbilitiesAsync().Returns(organizationAbilities); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); await sutProvider.Sut.HandleAsync(context); @@ -207,17 +199,16 @@ public class OrganizationUserAuthorizationHandlerTests Assert.True(context.HasFailed); } - private static Dictionary ArrangeOrganizationAbilitiesDictionary(Guid orgId, - bool limitCollectionCreationDeletion) + private static void ArrangeOrganizationAbility( + SutProvider sutProvider, + CurrentContextOrganization organization, bool limitCollectionCreationDeletion) { - return new Dictionary - { - { orgId, - new OrganizationAbility - { - LimitCollectionCreationDeletion = limitCollectionCreationDeletion - } - } - }; + var organizationAbility = new OrganizationAbility(); + organizationAbility.Id = organization.Id; + organizationAbility.FlexibleCollections = true; + organizationAbility.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); } } diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index ae2ea4ae9a..d4888a63ed 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -15,6 +15,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; using Bit.Core.Models.Data; +using Bit.Core.Models.Data.Organizations; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Models.Mail; using Bit.Core.Models.StaticStore; @@ -972,21 +973,23 @@ OrganizationUserInvite invite, SutProvider sutProvider) } [Theory, BitAutoData] - public async Task InviteUser_WithFCEnabled_WhenInvitingManager_Throws(Organization organization, OrganizationUserInvite invite, - OrganizationUser invitor, SutProvider sutProvider) + public async Task InviteUser_WithFCEnabled_WhenInvitingManager_Throws(OrganizationAbility organizationAbility, + OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { invite.Type = OrganizationUserType.Manager; + organizationAbility.FlexibleCollections = true; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any()) - .Returns(true); + sutProvider.GetDependency() + .GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); sutProvider.GetDependency() - .ManageUsers(organization.Id) + .ManageUsers(organizationAbility.Id) .Returns(true); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new (OrganizationUserInvite, string)[] { (invite, null) })); + () => sutProvider.Sut.InviteUsersAsync(organizationAbility.Id, invitor.UserId, + new (OrganizationUserInvite, string)[] { (invite, null) })); Assert.Contains("manager role is deprecated", exception.Message.ToLowerInvariant()); } @@ -1273,19 +1276,20 @@ OrganizationUserInvite invite, SutProvider sutProvider) [Theory, BitAutoData] public async Task SaveUser_WithFCEnabled_WhenUpgradingToManager_Throws( - Organization organization, + OrganizationAbility organizationAbility, [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData, [OrganizationUser(type: OrganizationUserType.Manager)] OrganizationUser newUserData, IEnumerable collections, IEnumerable groups, SutProvider sutProvider) { - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any()) - .Returns(true); + organizationAbility.FlexibleCollections = true; + sutProvider.GetDependency() + .GetOrganizationAbilityAsync(organizationAbility.Id) + .Returns(organizationAbility); sutProvider.GetDependency() - .ManageUsers(organization.Id) + .ManageUsers(organizationAbility.Id) .Returns(true); sutProvider.GetDependency() @@ -1294,7 +1298,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; - newUserData.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.OrganizationId = oldUserData.OrganizationId = organizationAbility.Id; newUserData.Permissions = CoreHelpers.ClassToJsonData(new Permissions()); var exception = await Assert.ThrowsAsync( diff --git a/test/Core.Test/AutoFixture/FeatureServiceFixtures.cs b/test/Core.Test/AutoFixture/FeatureServiceFixtures.cs deleted file mode 100644 index 69f771e321..0000000000 --- a/test/Core.Test/AutoFixture/FeatureServiceFixtures.cs +++ /dev/null @@ -1,75 +0,0 @@ -using System.Reflection; -using AutoFixture; -using AutoFixture.Kernel; -using Bit.Core.Context; -using Bit.Core.Services; -using Bit.Test.Common.AutoFixture; -using Bit.Test.Common.AutoFixture.Attributes; -using NSubstitute; - -namespace Bit.Core.Test.AutoFixture; - -internal class FeatureServiceBuilder : ISpecimenBuilder -{ - private readonly string _enabledFeatureFlag; - - public FeatureServiceBuilder(string enabledFeatureFlag) - { - _enabledFeatureFlag = enabledFeatureFlag; - } - - public object Create(object request, ISpecimenContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - if (request is not ParameterInfo pi) - { - return new NoSpecimen(); - } - - if (pi.ParameterType == typeof(IFeatureService)) - { - var fixture = new Fixture(); - var featureService = fixture.WithAutoNSubstitutions().Create(); - featureService - .IsEnabled(_enabledFeatureFlag, Arg.Any(), Arg.Any()) - .Returns(true); - return featureService; - } - - return new NoSpecimen(); - } -} - -internal class FeatureServiceCustomization : ICustomization -{ - private readonly string _enabledFeatureFlag; - - public FeatureServiceCustomization(string enabledFeatureFlag) - { - _enabledFeatureFlag = enabledFeatureFlag; - } - - public void Customize(IFixture fixture) - { - fixture.Customizations.Add(new FeatureServiceBuilder(_enabledFeatureFlag)); - } -} - -/// -/// Arranges the IFeatureService mock to enable the specified feature flag -/// -public class FeatureServiceCustomizeAttribute : BitCustomizeAttribute -{ - private readonly string _enabledFeatureFlag; - - public FeatureServiceCustomizeAttribute(string enabledFeatureFlag) - { - _enabledFeatureFlag = enabledFeatureFlag; - } - - public override ICustomization GetCustomization() => new FeatureServiceCustomization(_enabledFeatureFlag); -} diff --git a/test/Core.Test/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommandTests.cs new file mode 100644 index 0000000000..565f2f32c4 --- /dev/null +++ b/test/Core.Test/OrganizationFeatures/OrganizationLicenses/UpdateOrganizationLicenseCommandTests.cs @@ -0,0 +1,100 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Business; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.OrganizationFeatures.OrganizationLicenses; +using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Helpers; +using NSubstitute; +using Xunit; +using JsonSerializer = System.Text.Json.JsonSerializer; + +namespace Bit.Core.Test.OrganizationFeatures.OrganizationLicenses; + +[SutProviderCustomize] +public class UpdateOrganizationLicenseCommandTests +{ + private static string LicenseDirectory => Path.GetDirectoryName(OrganizationLicenseDirectory.Value); + private static Lazy OrganizationLicenseDirectory => new(() => + { + // Create a temporary directory to write the license file to + var directory = Path.Combine(Path.GetTempPath(), "bitwarden/"); + if (!Directory.Exists(directory)) + { + Directory.CreateDirectory(directory); + } + return directory; + }); + + [Theory, BitAutoData] + public async Task UpdateLicenseAsync_UpdatesLicenseFileAndOrganization( + SelfHostedOrganizationDetails selfHostedOrg, + OrganizationLicense license, + SutProvider sutProvider) + { + var globalSettings = sutProvider.GetDependency(); + globalSettings.LicenseDirectory = LicenseDirectory; + globalSettings.SelfHosted = true; + + // Passing values for OrganizationLicense.CanUse + // NSubstitute cannot override non-virtual members so we have to ensure the real method passes + license.Enabled = true; + license.Issued = DateTime.Now.AddDays(-1); + license.Expires = DateTime.Now.AddDays(1); + license.Version = OrganizationLicense.CurrentLicenseFileVersion; + license.InstallationId = globalSettings.Installation.Id; + license.LicenseType = LicenseType.Organization; + sutProvider.GetDependency().VerifyLicense(license).Returns(true); + + // Passing values for SelfHostedOrganizationDetails.CanUseLicense + // NSubstitute cannot override non-virtual members so we have to ensure the real method passes + license.Seats = null; + license.MaxCollections = null; + license.UseGroups = true; + license.UsePolicies = true; + license.UseSso = true; + license.UseKeyConnector = true; + license.UseScim = true; + license.UseCustomPermissions = true; + license.UseResetPassword = true; + + try + { + await sutProvider.Sut.UpdateLicenseAsync(selfHostedOrg, license, null); + + // Assertion: should have saved the license file to disk + var filePath = Path.Combine(LicenseDirectory, "organization", $"{selfHostedOrg.Id}.json"); + await using var fs = File.OpenRead(filePath); + var licenseFromFile = await JsonSerializer.DeserializeAsync(fs); + + AssertHelper.AssertPropertyEqual(license, licenseFromFile, "SignatureBytes"); + + // Assertion: should have updated and saved the organization + // Properties excluded from the comparison below are exceptions to the rule that the Organization mirrors + // the OrganizationLicense + await sutProvider.GetDependency() + .Received(1) + .ReplaceAndUpdateCacheAsync(Arg.Is( + org => AssertPropertyEqual(license, org, + "Id", "MaxStorageGb", "Issued", "Refresh", "Version", "Trial", "LicenseType", + "Hash", "Signature", "SignatureBytes", "InstallationId", "Expires", "ExpirationWithoutGracePeriod") && + // Same property but different name, use explicit mapping + org.ExpirationDate == license.Expires)); + } + finally + { + // Clean up temporary directory + Directory.Delete(OrganizationLicenseDirectory.Value, true); + } + } + + // Wrapper to compare 2 objects that are different types + private bool AssertPropertyEqual(OrganizationLicense expected, Organization actual, params string[] excludedPropertyStrings) + { + AssertHelper.AssertPropertyEqual(expected, actual, excludedPropertyStrings); + return true; + } +} diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index fc64f80216..a4eb05ac4f 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1,5 +1,4 @@ using Bit.Core.AdminConsole.Entities; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -36,6 +35,7 @@ public class CipherServiceTests SutProvider sutProvider) { organization.MaxCollections = null; + organization.FlexibleCollections = false; importingOrganizationUser.OrganizationId = organization.Id; foreach (var collection in collections) @@ -62,10 +62,6 @@ public class CipherServiceTests .GetByOrganizationAsync(organization.Id, importingUserId) .Returns(importingOrganizationUser); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any(), Arg.Any()) - .Returns(false); - // Set up a collection that already exists in the organization sutProvider.GetDependency() .GetManyByOrganizationIdAsync(organization.Id) @@ -95,6 +91,7 @@ public class CipherServiceTests SutProvider sutProvider) { organization.MaxCollections = null; + organization.FlexibleCollections = true; importingOrganizationUser.OrganizationId = organization.Id; foreach (var collection in collections) @@ -121,10 +118,6 @@ public class CipherServiceTests .GetByOrganizationAsync(organization.Id, importingUserId) .Returns(importingOrganizationUser); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any(), Arg.Any()) - .Returns(true); - // Set up a collection that already exists in the organization sutProvider.GetDependency() .GetManyByOrganizationIdAsync(organization.Id)