From 8ebac62dfff41d5478a2f82c874856c2640fe6cf Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Mon, 9 Oct 2023 13:55:14 +1000 Subject: [PATCH] Restore old logic behind flags --- src/Api/Controllers/CollectionsController.cs | 113 ++++++++++++++++-- .../CollectionAuthorizationHandler.cs | 19 ++- src/Core/Constants.cs | 1 + 3 files changed, 121 insertions(+), 12 deletions(-) diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index e4010d0018..8ac47954fe 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -1,12 +1,14 @@ using Bit.Api.Models.Request; using Bit.Api.Models.Response; using Bit.Api.Vault.AuthorizationHandlers.Collections; +using Bit.Core; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -23,6 +25,7 @@ public class CollectionsController : Controller private readonly IAuthorizationService _authorizationService; private readonly ICurrentContext _currentContext; private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand; + private readonly IFeatureService _featureService; public CollectionsController( ICollectionRepository collectionRepository, @@ -31,7 +34,8 @@ public class CollectionsController : Controller IUserService userService, IAuthorizationService authorizationService, ICurrentContext currentContext, - IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand) + IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand, + IFeatureService featureService) { _collectionRepository = collectionRepository; _collectionService = collectionService; @@ -40,6 +44,7 @@ public class CollectionsController : Controller _authorizationService = authorizationService; _currentContext = currentContext; _bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand; + _featureService = featureService; } [HttpGet("{id}")] @@ -152,8 +157,10 @@ public class CollectionsController : Controller { var collection = model.ToCollection(orgId); - var result = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Create); - if (!result.Succeeded) + var authorized = FlexibleCollectionsIsEnabled() + ? (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Create)).Succeeded + : await CanCreateCollection(orgId, collection.Id) && await CanEditCollectionAsync(orgId, collection.Id); + if (!authorized) { throw new NotFoundException(); } @@ -221,8 +228,11 @@ public class CollectionsController : Controller public async Task Delete(Guid orgId, Guid id) { var collection = await GetCollectionAsync(id, orgId); - var result = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete); - if (!result.Succeeded) + + var authorized = FlexibleCollectionsIsEnabled() + ? (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete)).Succeeded + : await CanDeleteCollectionAsync(orgId, id); + if (!authorized) { throw new NotFoundException(); } @@ -232,16 +242,37 @@ public class CollectionsController : Controller [HttpDelete("")] [HttpPost("delete")] - public async Task DeleteMany([FromBody] CollectionBulkDeleteRequestModel model) + public async Task DeleteMany(Guid orgId, [FromBody] CollectionBulkDeleteRequestModel model) { - var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Ids); - var result = await _authorizationService.AuthorizeAsync(User, collections, CollectionOperations.Delete); - if (!result.Succeeded) + if (FlexibleCollectionsIsEnabled()) + { + // New flexible collections logic + var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Ids); + var result = await _authorizationService.AuthorizeAsync(User, collections, CollectionOperations.Delete); + if (!result.Succeeded) + { + throw new NotFoundException(); + } + + await _deleteCollectionCommand.DeleteManyAsync(collections); + } + + // Old pre-flexible collections logic follows + if (!await _currentContext.DeleteAssignedCollections(orgId) && !await DeleteAnyCollection(orgId)) { throw new NotFoundException(); } - await _deleteCollectionCommand.DeleteManyAsync(collections); + var userCollections = await _collectionService.GetOrganizationCollectionsAsync(orgId); + var filteredCollections = userCollections + .Where(c => model.Ids.Contains(c.Id) && c.OrganizationId == orgId); + + if (!filteredCollections.Any()) + { + throw new BadRequestException("No collections found."); + } + + await _deleteCollectionCommand.DeleteManyAsync(filteredCollections); } [HttpDelete("{id}/user/{orgUserId}")] @@ -272,6 +303,33 @@ public class CollectionsController : Controller return collection; } + private bool FlexibleCollectionsIsEnabled() + { + return _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + } + + 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 CanCreateCollection(Guid orgId, Guid collectionId) + { + DeprecatedPermissionsGuard(); + + if (collectionId != default) + { + return false; + } + + return await _currentContext.OrganizationManager(orgId) || (_currentContext.Organizations?.Any(o => o.Id == orgId && + (o.Permissions?.CreateNewCollections ?? false)) ?? false); + } + private async Task CanEditCollectionAsync(Guid orgId, Guid collectionId) { if (collectionId == default) @@ -294,6 +352,41 @@ public class CollectionsController : Controller return false; } + [Obsolete("Pre-Flexible Collections logic. Will be replaced by CollectionsAuthorizationHandler.")] + private async Task CanDeleteCollectionAsync(Guid orgId, Guid collectionId) + { + DeprecatedPermissionsGuard(); + + if (collectionId == default) + { + return false; + } + + if (await DeleteAnyCollection(orgId)) + { + return true; + } + + if (await _currentContext.DeleteAssignedCollections(orgId)) + { + var collectionDetails = + await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + return collectionDetails != null; + } + + return false; + } + + [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); + } + private async Task CanViewCollectionAsync(Guid orgId, Guid collectionId) { if (collectionId == default) diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs index b3e15e281f..626402f715 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs @@ -1,27 +1,42 @@ -using Bit.Core.Context; +using Bit.Core; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; namespace Bit.Api.Vault.AuthorizationHandlers.Collections; +/// +/// Handles authorization logic for Collection objects, including access permissions for users and groups. +/// This uses new logic implemented in the Flexible Collections initiative. +/// public class CollectionAuthorizationHandler : BulkAuthorizationHandler { private readonly ICurrentContext _currentContext; private readonly ICollectionRepository _collectionRepository; + private readonly IFeatureService _featureService; - public CollectionAuthorizationHandler(ICurrentContext currentContext, ICollectionRepository collectionRepository) + public CollectionAuthorizationHandler(ICurrentContext currentContext, ICollectionRepository collectionRepository, + IFeatureService featureService) { _currentContext = currentContext; _collectionRepository = collectionRepository; + _featureService = featureService; } protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, ICollection resources) { + if (!_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext)) + { + // 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()) { diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 7d3a71c155..5861bde022 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -41,6 +41,7 @@ public static class FeatureFlagKeys public const string AutofillV2 = "autofill-v2"; public const string BrowserFilelessImport = "browser-fileless-import"; public const string FlexibleCollections = "flexible-collections"; + public const string BulkCollectionAccess = "bulk-collection-access"; public static List GetAllKeys() {