From c2b4ee7eacafb3cd55ef3f4fa548e666cd2f0752 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Mon, 29 Jan 2024 14:46:34 +0000 Subject: [PATCH] [AC-1782] Import can manage (#3453) * Changed Import permissions validation to check if the user CanCreate a Collection * Corrected authorized to import validation allowing import without collections when the user is admin * Added validation to check if user can import ciphers into existing collections * swapped feature flag flexible collections with org property * Removed unused feature service from ImportCiphersController * Improved code readability * added null protection against empty org when checking for FlexibleCollections flag --- .../Controllers/ImportCiphersController.cs | 63 +++++++++++++++++-- .../BulkCollectionAuthorizationHandler.cs | 1 + .../Collections/BulkCollectionOperations.cs | 1 + 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/Api/Tools/Controllers/ImportCiphersController.cs b/src/Api/Tools/Controllers/ImportCiphersController.cs index 7c9752076..fab503704 100644 --- a/src/Api/Tools/Controllers/ImportCiphersController.cs +++ b/src/Api/Tools/Controllers/ImportCiphersController.cs @@ -1,6 +1,8 @@ using Bit.Api.Tools.Models.Request.Accounts; using Bit.Api.Tools.Models.Request.Organizations; +using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -20,20 +22,28 @@ public class ImportCiphersController : Controller private readonly ICurrentContext _currentContext; private readonly ILogger _logger; private readonly GlobalSettings _globalSettings; + private readonly ICollectionRepository _collectionRepository; + private readonly IAuthorizationService _authorizationService; + private readonly IOrganizationRepository _organizationRepository; public ImportCiphersController( - ICollectionCipherRepository collectionCipherRepository, ICipherService cipherService, IUserService userService, ICurrentContext currentContext, ILogger logger, - GlobalSettings globalSettings) + GlobalSettings globalSettings, + ICollectionRepository collectionRepository, + IAuthorizationService authorizationService, + IOrganizationRepository organizationRepository) { _cipherService = cipherService; _userService = userService; _currentContext = currentContext; _logger = logger; _globalSettings = globalSettings; + _collectionRepository = collectionRepository; + _authorizationService = authorizationService; + _organizationRepository = organizationRepository; } [HttpPost("import")] @@ -64,14 +74,59 @@ public class ImportCiphersController : Controller } var orgId = new Guid(organizationId); - if (!await _currentContext.AccessImportExport(orgId)) + var collections = model.Collections.Select(c => c.ToCollection(orgId)).ToList(); + + + //An User is allowed to import if CanCreate Collections or has AccessToImportExport + var authorized = await CheckOrgImportPermission(collections, orgId); + + if (!authorized) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User).Value; - var collections = model.Collections.Select(c => c.ToCollection(orgId)).ToList(); var ciphers = model.Ciphers.Select(l => l.ToOrganizationCipherDetails(orgId)).ToList(); await _cipherService.ImportCiphersAsync(collections, ciphers, model.CollectionRelationships, userId); } + + private async Task CheckOrgImportPermission(List collections, Guid orgId) + { + //Users are allowed to import if they have the AccessToImportExport permission + if (await _currentContext.AccessImportExport(orgId)) + { + return true; + } + + //If flexible collections is disabled the user cannot continue with the import + var orgFlexibleCollections = await _organizationRepository.GetByIdAsync(orgId); + if (!orgFlexibleCollections?.FlexibleCollections ?? false) + { + return false; + } + + //Users allowed to import if they CanCreate Collections + if (!(await _authorizationService.AuthorizeAsync(User, collections, BulkCollectionOperations.Create)).Succeeded) + { + return false; + } + + //Calling Repository instead of Service as we want to get all the collections, regardless of permission + //Permissions check will be done later on AuthorizationService + var orgCollectionIds = + (await _collectionRepository.GetManyByOrganizationIdAsync(orgId)) + .Select(c => c.Id) + .ToHashSet(); + + //We need to verify if the user is trying to import into existing collections + var existingCollections = collections.Where(tc => orgCollectionIds.Contains(tc.Id)); + + //When importing into existing collection, we need to verify if the user has permissions + if (existingCollections.Any() && !(await _authorizationService.AuthorizeAsync(User, existingCollections, BulkCollectionOperations.ImportCiphers)).Succeeded) + { + return false; + }; + + return true; + } } diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index fb47602bc..afdd7c012 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -76,6 +76,7 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler public static readonly BulkCollectionOperationRequirement ModifyAccess = new() { Name = nameof(ModifyAccess) }; public static readonly BulkCollectionOperationRequirement Delete = new() { Name = nameof(Delete) }; + public static readonly BulkCollectionOperationRequirement ImportCiphers = new() { Name = nameof(ImportCiphers) }; }