From 505508a4163c61f052c1aa4c435b5d6be9c2c959 Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:13:29 +0100 Subject: [PATCH 01/12] [PM-5553] Move Org-Export to tools (#3639) * Move Org-Export to tools * Make linter happy --------- Co-authored-by: Daniel James Smith --- .../{ => Tools}/Controllers/OrganizationExportController.cs | 3 ++- .../Models/Response/OrganizationExportResponseModel.cs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) rename src/Api/{ => Tools}/Controllers/OrganizationExportController.cs (97%) rename src/Api/{ => Tools}/Models/Response/OrganizationExportResponseModel.cs (85%) diff --git a/src/Api/Controllers/OrganizationExportController.cs b/src/Api/Tools/Controllers/OrganizationExportController.cs similarity index 97% rename from src/Api/Controllers/OrganizationExportController.cs rename to src/Api/Tools/Controllers/OrganizationExportController.cs index be5aa0e14a..b3c0643b28 100644 --- a/src/Api/Controllers/OrganizationExportController.cs +++ b/src/Api/Tools/Controllers/OrganizationExportController.cs @@ -1,4 +1,5 @@ using Bit.Api.Models.Response; +using Bit.Api.Tools.Models.Response; using Bit.Api.Vault.Models.Response; using Bit.Core.Context; using Bit.Core.Entities; @@ -9,7 +10,7 @@ using Bit.Core.Vault.Services; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -namespace Bit.Api.Controllers; +namespace Bit.Api.Tools.Controllers; [Route("organizations/{organizationId}")] [Authorize("Application")] diff --git a/src/Api/Models/Response/OrganizationExportResponseModel.cs b/src/Api/Tools/Models/Response/OrganizationExportResponseModel.cs similarity index 85% rename from src/Api/Models/Response/OrganizationExportResponseModel.cs rename to src/Api/Tools/Models/Response/OrganizationExportResponseModel.cs index 1bfca7e3d5..a4b35d8de1 100644 --- a/src/Api/Models/Response/OrganizationExportResponseModel.cs +++ b/src/Api/Tools/Models/Response/OrganizationExportResponseModel.cs @@ -1,7 +1,8 @@ -using Bit.Api.Vault.Models.Response; +using Bit.Api.Models.Response; +using Bit.Api.Vault.Models.Response; using Bit.Core.Models.Api; -namespace Bit.Api.Models.Response; +namespace Bit.Api.Tools.Models.Response; public class OrganizationExportResponseModel : ResponseModel { From 95139def0f2aa5865bedd66d605903e4030cae7f Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 12 Jan 2024 10:38:47 -0500 Subject: [PATCH 02/12] [AC-1758] Implement `RemoveOrganizationFromProviderCommand` (#3515) * Add RemovePaymentMethod to StripePaymentService * Add SendProviderUpdatePaymentMethod to HandlebarsMailService * Add RemoveOrganizationFromProviderCommand * Use RemoveOrganizationFromProviderCommand in ProviderOrganizationController * Remove RemoveOrganizationAsync from ProviderService * Add RemoveOrganizationFromProviderCommandTests * PR review feedback and refactoring * Remove RemovePaymentMethod from StripePaymentService * Review feedback * Add Organization RisksSubscriptionFailure endpoint * fix build error * Review feedback * [AC-1359] Bitwarden Portal Unlink Provider Buttons (#3588) * Added ability to unlink organization from provider from provider edit page * Refreshing provider edit page after removing an org * Added button to organization to remove the org from the provider * Updated based on product feedback * Removed organization name from alert message * Temporary logging * Remove coupon from Stripe org after disconnected from MSP * Updated test * Change payment terms on org disconnect from MSP * Set Stripe account email to new billing email * Remove logging --------- Co-authored-by: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Co-authored-by: Conner Turnbull --- .../RemoveOrganizationFromProviderCommand.cs | 98 +++++ .../AdminConsole/Services/ProviderService.cs | 17 - .../Utilities/ServiceCollectionExtensions.cs | 1 + ...oveOrganizationFromProviderCommandTests.cs | 132 +++++++ .../Services/ProviderServiceTests.cs | 59 --- .../Controllers/OrganizationsController.cs | 45 ++- .../ProviderOrganizationsController.cs | 67 ++++ src/Admin/Startup.cs | 2 + src/Admin/Views/Organizations/Edit.cshtml | 12 +- .../Views/Providers/Organizations.cshtml | 26 +- .../_ProviderOrganizationScripts.cshtml | 21 + .../Shared/_OrganizationFormScripts.cshtml | 20 + .../Controllers/OrganizationsController.cs | 21 +- .../ProviderOrganizationsController.cs | 46 ++- ...onRisksSubscriptionFailureResponseModel.cs | 17 + src/Api/Startup.cs | 2 + .../IRemoveOrganizationFromProviderCommand.cs | 12 + .../AdminConsole/Services/IProviderService.cs | 1 - .../Commands/IRemovePaymentMethodCommand.cs | 8 + .../RemovePaymentMethodCommand.cs | 140 +++++++ .../Extensions/ServiceCollectionExtensions.cs | 14 + .../ProviderUpdatePaymentMethod.html.hbs | 27 ++ .../ProviderUpdatePaymentMethod.text.hbs | 7 + .../ProviderUpdatePaymentMethodViewModel.cs | 11 + src/Core/Services/IMailService.cs | 5 + src/Core/Services/IPaymentService.cs | 1 + src/Core/Services/IStripeAdapter.cs | 1 + .../Implementations/HandlebarsMailService.cs | 24 ++ .../Services/Implementations/StripeAdapter.cs | 3 + .../Implementations/StripePaymentService.cs | 17 + .../NoopImplementations/NoopMailService.cs | 3 + .../Repositories/OrganizationRepository.cs | 13 +- .../Repositories/OrganizationRepository.cs | 16 +- .../OrganizationsControllerTests.cs | 31 +- .../RemovePaymentMethodCommandTests.cs | 367 ++++++++++++++++++ 35 files changed, 1168 insertions(+), 119 deletions(-) create mode 100644 bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs create mode 100644 src/Admin/Controllers/ProviderOrganizationsController.cs create mode 100644 src/Admin/Views/Providers/_ProviderOrganizationScripts.cshtml create mode 100644 src/Api/AdminConsole/Models/Response/Organizations/OrganizationRisksSubscriptionFailureResponseModel.cs create mode 100644 src/Core/AdminConsole/Providers/Interfaces/IRemoveOrganizationFromProviderCommand.cs create mode 100644 src/Core/Billing/Commands/IRemovePaymentMethodCommand.cs create mode 100644 src/Core/Billing/Commands/Implementations/RemovePaymentMethodCommand.cs create mode 100644 src/Core/Billing/Extensions/ServiceCollectionExtensions.cs create mode 100644 src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.html.hbs create mode 100644 src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.text.hbs create mode 100644 src/Core/Models/Mail/Provider/ProviderUpdatePaymentMethodViewModel.cs create mode 100644 test/Core.Test/Billing/Commands/RemovePaymentMethodCommandTests.cs diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs new file mode 100644 index 0000000000..778cd62c26 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs @@ -0,0 +1,98 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Providers.Interfaces; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.Extensions.Logging; +using Stripe; + +namespace Bit.Commercial.Core.AdminConsole.Providers; + +public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProviderCommand +{ + private readonly IEventService _eventService; + private readonly ILogger _logger; + private readonly IMailService _mailService; + private readonly IOrganizationRepository _organizationRepository; + private readonly IOrganizationService _organizationService; + private readonly IProviderOrganizationRepository _providerOrganizationRepository; + private readonly IStripeAdapter _stripeAdapter; + + public RemoveOrganizationFromProviderCommand( + IEventService eventService, + ILogger logger, + IMailService mailService, + IOrganizationRepository organizationRepository, + IOrganizationService organizationService, + IProviderOrganizationRepository providerOrganizationRepository, + IStripeAdapter stripeAdapter) + { + _eventService = eventService; + _logger = logger; + _mailService = mailService; + _organizationRepository = organizationRepository; + _organizationService = organizationService; + _providerOrganizationRepository = providerOrganizationRepository; + _stripeAdapter = stripeAdapter; + } + + public async Task RemoveOrganizationFromProvider( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization) + { + if (provider == null || + providerOrganization == null || + organization == null || + providerOrganization.ProviderId != provider.Id) + { + throw new BadRequestException("Failed to remove organization. Please contact support."); + } + + if (!await _organizationService.HasConfirmedOwnersExceptAsync( + providerOrganization.OrganizationId, + Array.Empty(), + includeProvider: false)) + { + throw new BadRequestException("Organization must have at least one confirmed owner."); + } + + var organizationOwnerEmails = + (await _organizationRepository.GetOwnerEmailAddressesById(organization.Id)).ToList(); + + organization.BillingEmail = organizationOwnerEmails.MinBy(email => email); + + await _organizationRepository.ReplaceAsync(organization); + + var customerUpdateOptions = new CustomerUpdateOptions + { + Coupon = string.Empty, + Email = organization.BillingEmail + }; + + await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId, customerUpdateOptions); + + var subscriptionUpdateOptions = new SubscriptionUpdateOptions + { + CollectionMethod = "send_invoice", + DaysUntilDue = 30 + }; + + await _stripeAdapter.SubscriptionUpdateAsync(organization.GatewaySubscriptionId, subscriptionUpdateOptions); + + await _mailService.SendProviderUpdatePaymentMethod( + organization.Id, + organization.Name, + provider.Name, + organizationOwnerEmails); + + await _providerOrganizationRepository.DeleteAsync(providerOrganization); + + await _eventService.LogProviderOrganizationEventAsync( + providerOrganization, + EventType.ProviderOrganization_Removed); + } +} diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs index c8b64da19a..f9049de072 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs @@ -527,23 +527,6 @@ public class ProviderService : IProviderService return providerOrganization; } - public async Task RemoveOrganizationAsync(Guid providerId, Guid providerOrganizationId, Guid removingUserId) - { - var providerOrganization = await _providerOrganizationRepository.GetByIdAsync(providerOrganizationId); - if (providerOrganization == null || providerOrganization.ProviderId != providerId) - { - throw new BadRequestException("Invalid organization."); - } - - if (!await _organizationService.HasConfirmedOwnersExceptAsync(providerOrganization.OrganizationId, new Guid[] { }, includeProvider: false)) - { - throw new BadRequestException("Organization needs to have at least one confirmed owner."); - } - - await _providerOrganizationRepository.DeleteAsync(providerOrganization); - await _eventService.LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_Removed); - } - public async Task ResendProviderSetupInviteEmailAsync(Guid providerId, Guid ownerId) { var provider = await _providerRepository.GetByIdAsync(providerId); diff --git a/bitwarden_license/src/Commercial.Core/Utilities/ServiceCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/Utilities/ServiceCollectionExtensions.cs index 09788406de..53c089f9fa 100644 --- a/bitwarden_license/src/Commercial.Core/Utilities/ServiceCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/Utilities/ServiceCollectionExtensions.cs @@ -12,5 +12,6 @@ public static class ServiceCollectionExtensions { services.AddScoped(); services.AddScoped(); + services.AddScoped(); } } diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs new file mode 100644 index 0000000000..7148bcb17b --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs @@ -0,0 +1,132 @@ +using Bit.Commercial.Core.AdminConsole.Providers; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Stripe; +using Xunit; + +namespace Bit.Commercial.Core.Test.AdminConsole.ProviderFeatures; + +[SutProviderCustomize] +public class RemoveOrganizationFromProviderCommandTests +{ + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_NoProvider_BadRequest( + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RemoveOrganizationFromProvider(null, null, null)); + + Assert.Equal("Failed to remove organization. Please contact support.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_NoProviderOrganization_BadRequest( + Provider provider, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RemoveOrganizationFromProvider(provider, null, null)); + + Assert.Equal("Failed to remove organization. Please contact support.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_NoOrganization_BadRequest( + Provider provider, + ProviderOrganization providerOrganization, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RemoveOrganizationFromProvider( + provider, providerOrganization, null)); + + Assert.Equal("Failed to remove organization. Please contact support.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_MismatchedProviderOrganization_BadRequest( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RemoveOrganizationFromProvider(provider, providerOrganization, organization)); + + Assert.Equal("Failed to remove organization. Please contact support.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_NoConfirmedOwners_BadRequest( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization, + SutProvider sutProvider) + { + providerOrganization.ProviderId = provider.Id; + + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + providerOrganization.OrganizationId, + Array.Empty(), + includeProvider: false) + .Returns(false); + + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RemoveOrganizationFromProvider(provider, providerOrganization, organization)); + + Assert.Equal("Organization must have at least one confirmed owner.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_MakesCorrectInvocations( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization, + SutProvider sutProvider) + { + providerOrganization.ProviderId = provider.Id; + + var organizationRepository = sutProvider.GetDependency(); + + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + providerOrganization.OrganizationId, + Array.Empty(), + includeProvider: false) + .Returns(true); + + var organizationOwnerEmails = new List { "a@gmail.com", "b@gmail.com" }; + + organizationRepository.GetOwnerEmailAddressesById(organization.Id).Returns(organizationOwnerEmails); + + await sutProvider.Sut.RemoveOrganizationFromProvider(provider, providerOrganization, organization); + + await organizationRepository.Received(1).ReplaceAsync(Arg.Is( + org => org.Id == organization.Id && org.BillingEmail == "a@gmail.com")); + + var stripeAdapter = sutProvider.GetDependency(); + + await stripeAdapter.Received(1).CustomerUpdateAsync( + organization.GatewayCustomerId, Arg.Is( + options => options.Coupon == string.Empty && options.Email == "a@gmail.com")); + + await stripeAdapter.Received(1).SubscriptionUpdateAsync( + organization.GatewaySubscriptionId, Arg.Is( + options => options.CollectionMethod == "send_invoice" && options.DaysUntilDue == 30)); + + await sutProvider.GetDependency().Received(1).SendProviderUpdatePaymentMethod( + organization.Id, + organization.Name, + provider.Name, + Arg.Is>(emails => emails.Contains("a@gmail.com") && emails.Contains("b@gmail.com"))); + + await sutProvider.GetDependency().Received(1) + .DeleteAsync(providerOrganization); + + await sutProvider.GetDependency().Received(1).LogProviderOrganizationEventAsync( + providerOrganization, + EventType.ProviderOrganization_Removed); + } +} diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs index 24167e7141..eea0ac53f0 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs @@ -541,65 +541,6 @@ public class ProviderServiceTests t.First().Item2 == null)); } - [Theory, BitAutoData] - public async Task RemoveOrganization_ProviderOrganizationIsInvalid_Throws(Provider provider, - ProviderOrganization providerOrganization, User user, SutProvider sutProvider) - { - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); - sutProvider.GetDependency().GetByIdAsync(providerOrganization.Id) - .ReturnsNull(); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); - Assert.Equal("Invalid organization.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveOrganization_ProviderOrganizationBelongsToWrongProvider_Throws(Provider provider, - ProviderOrganization providerOrganization, User user, SutProvider sutProvider) - { - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); - sutProvider.GetDependency().GetByIdAsync(providerOrganization.Id) - .Returns(providerOrganization); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); - Assert.Equal("Invalid organization.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveOrganization_HasNoOwners_Throws(Provider provider, - ProviderOrganization providerOrganization, User user, SutProvider sutProvider) - { - providerOrganization.ProviderId = provider.Id; - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); - sutProvider.GetDependency().GetByIdAsync(providerOrganization.Id) - .Returns(providerOrganization); - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default, default) - .ReturnsForAnyArgs(false); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); - Assert.Equal("Organization needs to have at least one confirmed owner.", exception.Message); - } - - [Theory, BitAutoData] - public async Task RemoveOrganization_Success(Provider provider, - ProviderOrganization providerOrganization, User user, SutProvider sutProvider) - { - providerOrganization.ProviderId = provider.Id; - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); - var providerOrganizationRepository = sutProvider.GetDependency(); - providerOrganizationRepository.GetByIdAsync(providerOrganization.Id).Returns(providerOrganization); - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default, default) - .ReturnsForAnyArgs(true); - - await sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id); - await providerOrganizationRepository.Received().DeleteAsync(providerOrganization); - await sutProvider.GetDependency().Received() - .LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_Removed); - } - [Theory, BitAutoData] public async Task AddOrganization_CreateAfterNov162023_PlanTypeDoesNotUpdated(Provider provider, Organization organization, string key, SutProvider sutProvider) diff --git a/src/Admin/Controllers/OrganizationsController.cs b/src/Admin/Controllers/OrganizationsController.cs index aebdcefe4f..d665ebddef 100644 --- a/src/Admin/Controllers/OrganizationsController.cs +++ b/src/Admin/Controllers/OrganizationsController.cs @@ -3,7 +3,9 @@ using Bit.Admin.Models; using Bit.Admin.Services; using Bit.Admin.Utilities; using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Commands; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -48,6 +50,9 @@ public class OrganizationsController : Controller private readonly ISecretRepository _secretRepository; private readonly IProjectRepository _projectRepository; private readonly IServiceAccountRepository _serviceAccountRepository; + private readonly IProviderOrganizationRepository _providerOrganizationRepository; + private readonly IRemoveOrganizationFromProviderCommand _removeOrganizationFromProviderCommand; + private readonly IRemovePaymentMethodCommand _removePaymentMethodCommand; public OrganizationsController( IOrganizationService organizationService, @@ -71,7 +76,10 @@ public class OrganizationsController : Controller ICurrentContext currentContext, ISecretRepository secretRepository, IProjectRepository projectRepository, - IServiceAccountRepository serviceAccountRepository) + IServiceAccountRepository serviceAccountRepository, + IProviderOrganizationRepository providerOrganizationRepository, + IRemoveOrganizationFromProviderCommand removeOrganizationFromProviderCommand, + IRemovePaymentMethodCommand removePaymentMethodCommand) { _organizationService = organizationService; _organizationRepository = organizationRepository; @@ -95,6 +103,9 @@ public class OrganizationsController : Controller _secretRepository = secretRepository; _projectRepository = projectRepository; _serviceAccountRepository = serviceAccountRepository; + _providerOrganizationRepository = providerOrganizationRepository; + _removeOrganizationFromProviderCommand = removeOrganizationFromProviderCommand; + _removePaymentMethodCommand = removePaymentMethodCommand; } [RequirePermission(Permission.Org_List_View)] @@ -286,6 +297,38 @@ public class OrganizationsController : Controller return Json(null); } + + [HttpPost] + [RequirePermission(Permission.Provider_Edit)] + public async Task UnlinkOrganizationFromProviderAsync(Guid id) + { + var organization = await _organizationRepository.GetByIdAsync(id); + if (organization is null) + { + return RedirectToAction("Index"); + } + + var provider = await _providerRepository.GetByOrganizationIdAsync(id); + if (provider is null) + { + return RedirectToAction("Edit", new { id }); + } + + var providerOrganization = await _providerOrganizationRepository.GetByOrganizationId(id); + if (providerOrganization is null) + { + return RedirectToAction("Edit", new { id }); + } + + await _removeOrganizationFromProviderCommand.RemoveOrganizationFromProvider( + provider, + providerOrganization, + organization); + + await _removePaymentMethodCommand.RemovePaymentMethod(organization); + + return Json(null); + } private async Task GetOrganization(Guid id, OrganizationEditModel model) { var organization = await _organizationRepository.GetByIdAsync(id); diff --git a/src/Admin/Controllers/ProviderOrganizationsController.cs b/src/Admin/Controllers/ProviderOrganizationsController.cs new file mode 100644 index 0000000000..e21e1297f6 --- /dev/null +++ b/src/Admin/Controllers/ProviderOrganizationsController.cs @@ -0,0 +1,67 @@ +using Bit.Admin.Enums; +using Bit.Admin.Utilities; +using Bit.Core.AdminConsole.Providers.Interfaces; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Commands; +using Bit.Core.Repositories; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; + +namespace Bit.Admin.Controllers; + +[Authorize] +[SelfHosted(NotSelfHostedOnly = true)] +public class ProviderOrganizationsController : Controller +{ + private readonly IProviderRepository _providerRepository; + private readonly IProviderOrganizationRepository _providerOrganizationRepository; + private readonly IOrganizationRepository _organizationRepository; + private readonly IRemoveOrganizationFromProviderCommand _removeOrganizationFromProviderCommand; + private readonly IRemovePaymentMethodCommand _removePaymentMethodCommand; + + public ProviderOrganizationsController(IProviderRepository providerRepository, + IProviderOrganizationRepository providerOrganizationRepository, + IOrganizationRepository organizationRepository, + IRemoveOrganizationFromProviderCommand removeOrganizationFromProviderCommand, + IRemovePaymentMethodCommand removePaymentMethodCommand) + { + _providerRepository = providerRepository; + _providerOrganizationRepository = providerOrganizationRepository; + _organizationRepository = organizationRepository; + _removeOrganizationFromProviderCommand = removeOrganizationFromProviderCommand; + _removePaymentMethodCommand = removePaymentMethodCommand; + } + + [HttpPost] + [RequirePermission(Permission.Provider_Edit)] + public async Task DeleteAsync(Guid providerId, Guid id) + { + var provider = await _providerRepository.GetByIdAsync(providerId); + if (provider is null) + { + return RedirectToAction("Index", "Providers"); + } + + var providerOrganization = await _providerOrganizationRepository.GetByIdAsync(id); + if (providerOrganization is null) + { + return RedirectToAction("View", "Providers", new { id = providerId }); + } + + var organization = await _organizationRepository.GetByIdAsync(providerOrganization.OrganizationId); + if (organization == null) + { + return RedirectToAction("View", "Providers", new { id = providerId }); + } + + await _removeOrganizationFromProviderCommand.RemoveOrganizationFromProvider( + provider, + providerOrganization, + organization); + + await _removePaymentMethodCommand.RemovePaymentMethod(organization); + + return Json(null); + } +} diff --git a/src/Admin/Startup.cs b/src/Admin/Startup.cs index a10cd4d2de..4c2bfdb7df 100644 --- a/src/Admin/Startup.cs +++ b/src/Admin/Startup.cs @@ -9,6 +9,7 @@ using Stripe; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.Extensions.DependencyInjection.Extensions; using Bit.Admin.Services; +using Bit.Core.Billing.Extensions; #if !OSS using Bit.Commercial.Core.Utilities; @@ -87,6 +88,7 @@ public class Startup services.AddBaseServices(globalSettings); services.AddDefaultServices(globalSettings); services.AddScoped(); + services.AddBillingCommands(); #if OSS services.AddOosServices(); diff --git a/src/Admin/Views/Organizations/Edit.cshtml b/src/Admin/Views/Organizations/Edit.cshtml index e3f6d50905..ad4e4f8482 100644 --- a/src/Admin/Views/Organizations/Edit.cshtml +++ b/src/Admin/Views/Organizations/Edit.cshtml @@ -8,6 +8,7 @@ var canViewBillingInformation = AccessControlService.UserHasPermission(Permission.Org_BillingInformation_View); var canInitiateTrial = AccessControlService.UserHasPermission(Permission.Org_InitiateTrial); var canDelete = AccessControlService.UserHasPermission(Permission.Org_Delete); + var canUnlinkFromProvider = AccessControlService.UserHasPermission(Permission.Provider_Edit); } @section Scripts { @@ -81,7 +82,7 @@
- @if (canInitiateTrial) + @if (canInitiateTrial && Model.Provider is null) { } + @if (canUnlinkFromProvider && Model.Provider is not null) + { + + } @if (canDelete) {
Provider Organizations
@@ -32,26 +40,28 @@ } else { - @foreach (var org in Model.ProviderOrganizations) + @foreach (var providerOrganization in Model.ProviderOrganizations) { - @org.OrganizationName + @providerOrganization.OrganizationName - @org.Status + @providerOrganization.Status
- @if (org.Status == OrganizationStatusType.Pending) + @if (canUnlinkFromProvider) { - - + + Unlink provider } - else + @if (providerOrganization.Status == OrganizationStatusType.Pending) { - + + Resend invitation + }
diff --git a/src/Admin/Views/Providers/_ProviderOrganizationScripts.cshtml b/src/Admin/Views/Providers/_ProviderOrganizationScripts.cshtml new file mode 100644 index 0000000000..b8fefb4c14 --- /dev/null +++ b/src/Admin/Views/Providers/_ProviderOrganizationScripts.cshtml @@ -0,0 +1,21 @@ + diff --git a/src/Admin/Views/Shared/_OrganizationFormScripts.cshtml b/src/Admin/Views/Shared/_OrganizationFormScripts.cshtml index fc527750aa..85d62f6b08 100644 --- a/src/Admin/Views/Shared/_OrganizationFormScripts.cshtml +++ b/src/Admin/Views/Shared/_OrganizationFormScripts.cshtml @@ -113,6 +113,26 @@ } } + function unlinkProvider(id) { + if (confirm('Are you sure you want to unlink this organization from its provider?')) { + $.ajax({ + type: "POST", + url: `@Url.Action("UnlinkOrganizationFromProvider", "Organizations")?id=${id}`, + dataType: 'json', + contentType: false, + processData: false, + success: function (response) { + alert("Successfully unlinked provider"); + window.location.href = `@Url.Action("Edit", "Organizations")?id=${id}`; + }, + error: function (response) { + alert("Error!"); + } + }); + } + return false; + } + /*** * Set Secrets Manager values based on current usage (for migrating from SM beta or reinstating an old subscription) */ diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 3293041a2b..1683af2b68 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -40,7 +40,6 @@ public class OrganizationsController : Controller private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPolicyRepository _policyRepository; - private readonly IProviderRepository _providerRepository; private readonly IOrganizationService _organizationService; private readonly IUserService _userService; private readonly IPaymentService _paymentService; @@ -51,7 +50,6 @@ public class OrganizationsController : Controller private readonly IRotateOrganizationApiKeyCommand _rotateOrganizationApiKeyCommand; private readonly ICreateOrganizationApiKeyCommand _createOrganizationApiKeyCommand; private readonly IOrganizationApiKeyRepository _organizationApiKeyRepository; - private readonly IUpdateOrganizationLicenseCommand _updateOrganizationLicenseCommand; private readonly ICloudGetOrganizationLicenseQuery _cloudGetOrganizationLicenseQuery; private readonly IFeatureService _featureService; private readonly GlobalSettings _globalSettings; @@ -64,7 +62,6 @@ public class OrganizationsController : Controller IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, IPolicyRepository policyRepository, - IProviderRepository providerRepository, IOrganizationService organizationService, IUserService userService, IPaymentService paymentService, @@ -75,7 +72,6 @@ public class OrganizationsController : Controller IRotateOrganizationApiKeyCommand rotateOrganizationApiKeyCommand, ICreateOrganizationApiKeyCommand createOrganizationApiKeyCommand, IOrganizationApiKeyRepository organizationApiKeyRepository, - IUpdateOrganizationLicenseCommand updateOrganizationLicenseCommand, ICloudGetOrganizationLicenseQuery cloudGetOrganizationLicenseQuery, IFeatureService featureService, GlobalSettings globalSettings, @@ -87,7 +83,6 @@ public class OrganizationsController : Controller _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _policyRepository = policyRepository; - _providerRepository = providerRepository; _organizationService = organizationService; _userService = userService; _paymentService = paymentService; @@ -98,7 +93,6 @@ public class OrganizationsController : Controller _rotateOrganizationApiKeyCommand = rotateOrganizationApiKeyCommand; _createOrganizationApiKeyCommand = createOrganizationApiKeyCommand; _organizationApiKeyRepository = organizationApiKeyRepository; - _updateOrganizationLicenseCommand = updateOrganizationLicenseCommand; _cloudGetOrganizationLicenseQuery = cloudGetOrganizationLicenseQuery; _featureService = featureService; _globalSettings = globalSettings; @@ -245,6 +239,21 @@ public class OrganizationsController : Controller return new OrganizationAutoEnrollStatusResponseModel(organization.Id, data?.AutoEnrollEnabled ?? false); } + [HttpGet("{id}/risks-subscription-failure")] + public async Task RisksSubscriptionFailure(Guid id) + { + if (!await _currentContext.EditPaymentMethods(id)) + { + return new OrganizationRisksSubscriptionFailureResponseModel(id, false); + } + + var organization = await _organizationRepository.GetByIdAsync(id); + + var risksSubscriptionFailure = await _paymentService.RisksSubscriptionFailure(organization); + + return new OrganizationRisksSubscriptionFailureResponseModel(id, risksSubscriptionFailure); + } + [HttpPost("")] [SelfHosted(NotSelfHostedOnly = true)] public async Task Post([FromBody] OrganizationCreateRequestModel model) diff --git a/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs b/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs index 4d734e7cad..136119848a 100644 --- a/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs @@ -1,10 +1,13 @@ using Bit.Api.AdminConsole.Models.Request.Providers; using Bit.Api.AdminConsole.Models.Response.Providers; using Bit.Api.Models.Response; +using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; +using Bit.Core.Billing.Commands; using Bit.Core.Context; using Bit.Core.Exceptions; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; @@ -16,22 +19,33 @@ namespace Bit.Api.AdminConsole.Controllers; [Authorize("Application")] public class ProviderOrganizationsController : Controller { - - private readonly IProviderOrganizationRepository _providerOrganizationRepository; - private readonly IProviderService _providerService; - private readonly IUserService _userService; private readonly ICurrentContext _currentContext; + private readonly IOrganizationRepository _organizationRepository; + private readonly IProviderOrganizationRepository _providerOrganizationRepository; + private readonly IProviderRepository _providerRepository; + private readonly IProviderService _providerService; + private readonly IRemoveOrganizationFromProviderCommand _removeOrganizationFromProviderCommand; + private readonly IRemovePaymentMethodCommand _removePaymentMethodCommand; + private readonly IUserService _userService; public ProviderOrganizationsController( + ICurrentContext currentContext, + IOrganizationRepository organizationRepository, IProviderOrganizationRepository providerOrganizationRepository, + IProviderRepository providerRepository, IProviderService providerService, - IUserService userService, - ICurrentContext currentContext) + IRemoveOrganizationFromProviderCommand removeOrganizationFromProviderCommand, + IRemovePaymentMethodCommand removePaymentMethodCommand, + IUserService userService) { - _providerOrganizationRepository = providerOrganizationRepository; - _providerService = providerService; - _userService = userService; _currentContext = currentContext; + _organizationRepository = organizationRepository; + _providerOrganizationRepository = providerOrganizationRepository; + _providerRepository = providerRepository; + _providerService = providerService; + _removeOrganizationFromProviderCommand = removeOrganizationFromProviderCommand; + _removePaymentMethodCommand = removePaymentMethodCommand; + _userService = userService; } [HttpGet("")] @@ -87,7 +101,17 @@ public class ProviderOrganizationsController : Controller throw new NotFoundException(); } - var userId = _userService.GetProperUserId(User); - await _providerService.RemoveOrganizationAsync(providerId, id, userId.Value); + var provider = await _providerRepository.GetByIdAsync(providerId); + + var providerOrganization = await _providerOrganizationRepository.GetByIdAsync(id); + + var organization = await _organizationRepository.GetByIdAsync(providerOrganization.OrganizationId); + + await _removeOrganizationFromProviderCommand.RemoveOrganizationFromProvider( + provider, + providerOrganization, + organization); + + await _removePaymentMethodCommand.RemovePaymentMethod(organization); } } diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationRisksSubscriptionFailureResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationRisksSubscriptionFailureResponseModel.cs new file mode 100644 index 0000000000..e91275da3c --- /dev/null +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationRisksSubscriptionFailureResponseModel.cs @@ -0,0 +1,17 @@ +using Bit.Core.Models.Api; + +namespace Bit.Api.AdminConsole.Models.Response.Organizations; + +public class OrganizationRisksSubscriptionFailureResponseModel : ResponseModel +{ + public Guid OrganizationId { get; } + public bool RisksSubscriptionFailure { get; } + + public OrganizationRisksSubscriptionFailureResponseModel( + Guid organizationId, + bool risksSubscriptionFailure) : base("organizationRisksSubscriptionFailure") + { + OrganizationId = organizationId; + RisksSubscriptionFailure = risksSubscriptionFailure; + } +} diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index b82b2e1c27..7b5067f3fe 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -26,6 +26,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Bit.Core.Auth.Identity; using Bit.Core.Auth.UserFeatures; using Bit.Core.Entities; +using Bit.Core.Billing.Extensions; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions; using Bit.Core.Tools.Entities; using Bit.Core.Vault.Entities; @@ -169,6 +170,7 @@ public class Startup services.AddDefaultServices(globalSettings); services.AddOrganizationSubscriptionServices(); services.AddCoreLocalizationServices(); + services.AddBillingCommands(); // Authorization Handlers services.AddAuthorizationHandlers(); diff --git a/src/Core/AdminConsole/Providers/Interfaces/IRemoveOrganizationFromProviderCommand.cs b/src/Core/AdminConsole/Providers/Interfaces/IRemoveOrganizationFromProviderCommand.cs new file mode 100644 index 0000000000..84013adc19 --- /dev/null +++ b/src/Core/AdminConsole/Providers/Interfaces/IRemoveOrganizationFromProviderCommand.cs @@ -0,0 +1,12 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; + +namespace Bit.Core.AdminConsole.Providers.Interfaces; + +public interface IRemoveOrganizationFromProviderCommand +{ + Task RemoveOrganizationFromProvider( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization); +} diff --git a/src/Core/AdminConsole/Services/IProviderService.cs b/src/Core/AdminConsole/Services/IProviderService.cs index f71403e80c..fdaef4c03b 100644 --- a/src/Core/AdminConsole/Services/IProviderService.cs +++ b/src/Core/AdminConsole/Services/IProviderService.cs @@ -23,7 +23,6 @@ public interface IProviderService Task AddOrganizationsToReseller(Guid providerId, IEnumerable organizationIds); Task CreateOrganizationAsync(Guid providerId, OrganizationSignup organizationSignup, string clientOwnerEmail, User user); - Task RemoveOrganizationAsync(Guid providerId, Guid providerOrganizationId, Guid removingUserId); Task LogProviderAccessToOrganizationAsync(Guid organizationId); Task ResendProviderSetupInviteEmailAsync(Guid providerId, Guid ownerId); Task SendProviderSetupInviteEmailAsync(Provider provider, string ownerEmail); diff --git a/src/Core/Billing/Commands/IRemovePaymentMethodCommand.cs b/src/Core/Billing/Commands/IRemovePaymentMethodCommand.cs new file mode 100644 index 0000000000..62bf0d0926 --- /dev/null +++ b/src/Core/Billing/Commands/IRemovePaymentMethodCommand.cs @@ -0,0 +1,8 @@ +using Bit.Core.AdminConsole.Entities; + +namespace Bit.Core.Billing.Commands; + +public interface IRemovePaymentMethodCommand +{ + Task RemovePaymentMethod(Organization organization); +} diff --git a/src/Core/Billing/Commands/Implementations/RemovePaymentMethodCommand.cs b/src/Core/Billing/Commands/Implementations/RemovePaymentMethodCommand.cs new file mode 100644 index 0000000000..c5dbb6d927 --- /dev/null +++ b/src/Core/Billing/Commands/Implementations/RemovePaymentMethodCommand.cs @@ -0,0 +1,140 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Services; +using Braintree; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.Billing.Commands.Implementations; + +public class RemovePaymentMethodCommand : IRemovePaymentMethodCommand +{ + private readonly IBraintreeGateway _braintreeGateway; + private readonly ILogger _logger; + private readonly IStripeAdapter _stripeAdapter; + + public RemovePaymentMethodCommand( + IBraintreeGateway braintreeGateway, + ILogger logger, + IStripeAdapter stripeAdapter) + { + _braintreeGateway = braintreeGateway; + _logger = logger; + _stripeAdapter = stripeAdapter; + } + + public async Task RemovePaymentMethod(Organization organization) + { + const string braintreeCustomerIdKey = "btCustomerId"; + + if (organization == null) + { + throw new ArgumentNullException(nameof(organization)); + } + + if (organization.Gateway is not GatewayType.Stripe || string.IsNullOrEmpty(organization.GatewayCustomerId)) + { + throw ContactSupport(); + } + + var stripeCustomer = await _stripeAdapter.CustomerGetAsync(organization.GatewayCustomerId, new Stripe.CustomerGetOptions + { + Expand = new List { "invoice_settings.default_payment_method", "sources" } + }); + + if (stripeCustomer == null) + { + _logger.LogError("Could not find Stripe customer ({ID}) when removing payment method", organization.GatewayCustomerId); + + throw ContactSupport(); + } + + if (stripeCustomer.Metadata?.TryGetValue(braintreeCustomerIdKey, out var braintreeCustomerId) ?? false) + { + await RemoveBraintreePaymentMethodAsync(braintreeCustomerId); + } + else + { + await RemoveStripePaymentMethodsAsync(stripeCustomer); + } + } + + private async Task RemoveBraintreePaymentMethodAsync(string braintreeCustomerId) + { + var customer = await _braintreeGateway.Customer.FindAsync(braintreeCustomerId); + + if (customer == null) + { + _logger.LogError("Failed to retrieve Braintree customer ({ID}) when removing payment method", braintreeCustomerId); + + throw ContactSupport(); + } + + if (customer.DefaultPaymentMethod != null) + { + var existingDefaultPaymentMethod = customer.DefaultPaymentMethod; + + var updateCustomerResult = await _braintreeGateway.Customer.UpdateAsync( + braintreeCustomerId, + new CustomerRequest { DefaultPaymentMethodToken = null }); + + if (!updateCustomerResult.IsSuccess()) + { + _logger.LogError("Failed to update payment method for Braintree customer ({ID}) | Message: {Message}", + braintreeCustomerId, updateCustomerResult.Message); + + throw ContactSupport(); + } + + var deletePaymentMethodResult = await _braintreeGateway.PaymentMethod.DeleteAsync(existingDefaultPaymentMethod.Token); + + if (!deletePaymentMethodResult.IsSuccess()) + { + await _braintreeGateway.Customer.UpdateAsync( + braintreeCustomerId, + new CustomerRequest { DefaultPaymentMethodToken = existingDefaultPaymentMethod.Token }); + + _logger.LogError( + "Failed to delete Braintree payment method for Customer ({ID}), re-linked payment method. Message: {Message}", + braintreeCustomerId, deletePaymentMethodResult.Message); + + throw ContactSupport(); + } + } + else + { + _logger.LogWarning("Tried to remove non-existent Braintree payment method for Customer ({ID})", braintreeCustomerId); + } + } + + private async Task RemoveStripePaymentMethodsAsync(Stripe.Customer customer) + { + if (customer.Sources != null && customer.Sources.Any()) + { + foreach (var source in customer.Sources) + { + switch (source) + { + case Stripe.BankAccount: + await _stripeAdapter.BankAccountDeleteAsync(customer.Id, source.Id); + break; + case Stripe.Card: + await _stripeAdapter.CardDeleteAsync(customer.Id, source.Id); + break; + } + } + } + + var paymentMethods = _stripeAdapter.PaymentMethodListAutoPagingAsync(new Stripe.PaymentMethodListOptions + { + Customer = customer.Id + }); + + await foreach (var paymentMethod in paymentMethods) + { + await _stripeAdapter.PaymentMethodDetachAsync(paymentMethod.Id, new Stripe.PaymentMethodDetachOptions()); + } + } + + private static GatewayException ContactSupport() => new("Could not remove your payment method. Please contact support for assistance."); +} diff --git a/src/Core/Billing/Extensions/ServiceCollectionExtensions.cs b/src/Core/Billing/Extensions/ServiceCollectionExtensions.cs new file mode 100644 index 0000000000..37857cf3ce --- /dev/null +++ b/src/Core/Billing/Extensions/ServiceCollectionExtensions.cs @@ -0,0 +1,14 @@ +using Bit.Core.Billing.Commands; +using Bit.Core.Billing.Commands.Implementations; + +namespace Bit.Core.Billing.Extensions; + +using Microsoft.Extensions.DependencyInjection; + +public static class ServiceCollectionExtensions +{ + public static void AddBillingCommands(this IServiceCollection services) + { + services.AddSingleton(); + } +} diff --git a/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.html.hbs b/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.html.hbs new file mode 100644 index 0000000000..7b666cdd9a --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.html.hbs @@ -0,0 +1,27 @@ +{{#>FullHtmlLayout}} + + + + + + + + + + + + + +
+ Your organization, {{OrganizationName}}, is no longer managed by {{ProviderName}}. Please update your billing information. +
+ To maintain your subscription, update your organization billing information by navigating to the web vault -> Organization -> Billing -> Payment Method. +
+ For more information, please refer to the following help article: Update billing information for organizations +
+ + Add payment method + +
+
+{{/FullHtmlLayout}} diff --git a/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.text.hbs b/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.text.hbs new file mode 100644 index 0000000000..56a857a6e4 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/Provider/ProviderUpdatePaymentMethod.text.hbs @@ -0,0 +1,7 @@ +{{#>BasicTextLayout}} + Your organization, {{OrganizationName}}, is no longer managed by {{ProviderName}}. Please update your billing information. + + To maintain your subscription, update your organization billing information by navigating to the web vault -> Organization -> Billing -> Payment Method. + + Or click the following link: {{{link PaymentMethodUrl}}} +{{/BasicTextLayout}} diff --git a/src/Core/Models/Mail/Provider/ProviderUpdatePaymentMethodViewModel.cs b/src/Core/Models/Mail/Provider/ProviderUpdatePaymentMethodViewModel.cs new file mode 100644 index 0000000000..114aaa7c95 --- /dev/null +++ b/src/Core/Models/Mail/Provider/ProviderUpdatePaymentMethodViewModel.cs @@ -0,0 +1,11 @@ +namespace Bit.Core.Models.Mail.Provider; + +public class ProviderUpdatePaymentMethodViewModel : BaseMailModel +{ + public string OrganizationId { get; set; } + public string OrganizationName { get; set; } + public string ProviderName { get; set; } + + public string PaymentMethodUrl => + $"{WebVaultUrl}/organizations/{OrganizationId}/billing/payment-method"; +} diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index c2d81d6edb..93c6fd6e33 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -60,6 +60,11 @@ public interface IMailService Task SendProviderInviteEmailAsync(string providerName, ProviderUser providerUser, string token, string email); Task SendProviderConfirmedEmailAsync(string providerName, string email); Task SendProviderUserRemoved(string providerName, string email); + Task SendProviderUpdatePaymentMethod( + Guid organizationId, + string organizationName, + string providerName, + IEnumerable emails); Task SendUpdatedTempPasswordEmailAsync(string email, string userName); Task SendFamiliesForEnterpriseOfferEmailAsync(string sponsorOrgName, string email, bool existingAccount, string token); Task BulkSendFamiliesForEnterpriseOfferEmailAsync(string SponsorOrgName, IEnumerable<(string Email, bool ExistingAccount, string Token)> invites); diff --git a/src/Core/Services/IPaymentService.cs b/src/Core/Services/IPaymentService.cs index a66d227d3e..70cc88c206 100644 --- a/src/Core/Services/IPaymentService.cs +++ b/src/Core/Services/IPaymentService.cs @@ -49,4 +49,5 @@ public interface IPaymentService Task ArchiveTaxRateAsync(TaxRate taxRate); Task AddSecretsManagerToSubscription(Organization org, Plan plan, int additionalSmSeats, int additionalServiceAccount, DateTime? prorationDate = null); + Task RisksSubscriptionFailure(Organization organization); } diff --git a/src/Core/Services/IStripeAdapter.cs b/src/Core/Services/IStripeAdapter.cs index 60d14ffad8..073d5cdacd 100644 --- a/src/Core/Services/IStripeAdapter.cs +++ b/src/Core/Services/IStripeAdapter.cs @@ -23,6 +23,7 @@ public interface IStripeAdapter Task InvoiceDeleteAsync(string id, Stripe.InvoiceDeleteOptions options = null); Task InvoiceVoidInvoiceAsync(string id, Stripe.InvoiceVoidOptions options = null); IEnumerable PaymentMethodListAutoPaging(Stripe.PaymentMethodListOptions options); + IAsyncEnumerable PaymentMethodListAutoPagingAsync(Stripe.PaymentMethodListOptions options); Task PaymentMethodAttachAsync(string id, Stripe.PaymentMethodAttachOptions options = null); Task PaymentMethodDetachAsync(string id, Stripe.PaymentMethodDetachOptions options = null); Task TaxRateCreateAsync(Stripe.TaxRateCreateOptions options); diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index 8805e3af56..90b273bed2 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -754,6 +754,30 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } + public async Task SendProviderUpdatePaymentMethod( + Guid organizationId, + string organizationName, + string providerName, + IEnumerable emails) + { + var message = CreateDefaultMessage("Update your billing information", emails); + + var model = new ProviderUpdatePaymentMethodViewModel + { + OrganizationId = organizationId.ToString(), + OrganizationName = CoreHelpers.SanitizeForEmail(organizationName), + ProviderName = CoreHelpers.SanitizeForEmail(providerName), + SiteName = _globalSettings.SiteName, + WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash + }; + + await AddMessageContentAsync(message, "Provider.ProviderUpdatePaymentMethod", model); + + message.Category = "ProviderUpdatePaymentMethod"; + + await _mailDeliveryService.SendEmailAsync(message); + } + public async Task SendUpdatedTempPasswordEmailAsync(string email, string userName) { var message = CreateDefaultMessage("Master Password Has Been Changed", email); diff --git a/src/Core/Services/Implementations/StripeAdapter.cs b/src/Core/Services/Implementations/StripeAdapter.cs index 747510d052..ef8d13aea8 100644 --- a/src/Core/Services/Implementations/StripeAdapter.cs +++ b/src/Core/Services/Implementations/StripeAdapter.cs @@ -138,6 +138,9 @@ public class StripeAdapter : IStripeAdapter return _paymentMethodService.ListAutoPaging(options); } + public IAsyncEnumerable PaymentMethodListAutoPagingAsync(Stripe.PaymentMethodListOptions options) + => _paymentMethodService.ListAutoPagingAsync(options); + public Task PaymentMethodAttachAsync(string id, Stripe.PaymentMethodAttachOptions options = null) { return _paymentMethodService.AttachAsync(id, options); diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index 8eae90ea2c..1aeda88076 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -1614,6 +1614,23 @@ public class StripePaymentService : IPaymentService return await FinalizeSubscriptionChangeAsync(org, new SecretsManagerSubscribeUpdate(org, plan, additionalSmSeats, additionalServiceAccount), prorationDate); } + public async Task RisksSubscriptionFailure(Organization organization) + { + var subscriptionInfo = await GetSubscriptionAsync(organization); + + if (subscriptionInfo.Subscription is not { Status: "active" or "trialing" or "past_due" } || + subscriptionInfo.UpcomingInvoice == null) + { + return false; + } + + var customer = await GetCustomerAsync(organization.GatewayCustomerId); + + var paymentSource = await GetBillingPaymentSourceAsync(customer); + + return paymentSource == null; + } + private Stripe.PaymentMethod GetLatestCardPaymentMethod(string customerId) { var cardPaymentMethods = _stripeAdapter.PaymentMethodListAutoPaging( diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 92e548e0d5..81419b1864 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -197,6 +197,9 @@ public class NoopMailService : IMailService return Task.FromResult(0); } + public Task SendProviderUpdatePaymentMethod(Guid organizationId, string organizationName, string providerName, + IEnumerable emails) => Task.FromResult(0); + public Task SendUpdatedTempPasswordEmailAsync(string email, string userName) { return Task.FromResult(0); diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs index 467fb8f8a8..f4c771adec 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs @@ -7,14 +7,21 @@ using Bit.Core.Repositories; using Bit.Core.Settings; using Dapper; using Microsoft.Data.SqlClient; +using Microsoft.Extensions.Logging; namespace Bit.Infrastructure.Dapper.Repositories; public class OrganizationRepository : Repository, IOrganizationRepository { - public OrganizationRepository(GlobalSettings globalSettings) + private readonly ILogger _logger; + + public OrganizationRepository( + GlobalSettings globalSettings, + ILogger logger) : this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString) - { } + { + _logger = logger; + } public OrganizationRepository(string connectionString, string readOnlyConnectionString) : base(connectionString, readOnlyConnectionString) @@ -153,6 +160,8 @@ public class OrganizationRepository : Repository, IOrganizat public async Task> GetOwnerEmailAddressesById(Guid organizationId) { + _logger.LogInformation("AC-1758: Executing GetOwnerEmailAddressesById (Dapper)"); + await using var connection = new SqlConnection(ConnectionString); return await connection.QueryAsync( diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 6ad8cfbb4e..acc36c9449 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -5,15 +5,23 @@ using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Organization = Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization; namespace Bit.Infrastructure.EntityFramework.Repositories; public class OrganizationRepository : Repository, IOrganizationRepository { - public OrganizationRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) - : base(serviceScopeFactory, mapper, (DatabaseContext context) => context.Organizations) - { } + private readonly ILogger _logger; + + public OrganizationRepository( + IServiceScopeFactory serviceScopeFactory, + IMapper mapper, + ILogger logger) + : base(serviceScopeFactory, mapper, context => context.Organizations) + { + _logger = logger; + } public async Task GetByIdentifierAsync(string identifier) { @@ -240,6 +248,8 @@ public class OrganizationRepository : Repository> GetOwnerEmailAddressesById(Guid organizationId) { + _logger.LogInformation("AC-1758: Executing GetOwnerEmailAddressesById (Entity Framework)"); + using var scope = ServiceScopeFactory.CreateScope(); var dbContext = GetDatabaseContext(scope); diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index fd24c47af2..0e4ae48877 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs @@ -37,7 +37,6 @@ public class OrganizationsControllerTests : IDisposable private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPaymentService _paymentService; private readonly IPolicyRepository _policyRepository; - private readonly IProviderRepository _providerRepository; private readonly ISsoConfigRepository _ssoConfigRepository; private readonly ISsoConfigService _ssoConfigService; private readonly IUserService _userService; @@ -46,7 +45,6 @@ public class OrganizationsControllerTests : IDisposable private readonly IOrganizationApiKeyRepository _organizationApiKeyRepository; private readonly ICloudGetOrganizationLicenseQuery _cloudGetOrganizationLicenseQuery; private readonly ICreateOrganizationApiKeyCommand _createOrganizationApiKeyCommand; - private readonly IUpdateOrganizationLicenseCommand _updateOrganizationLicenseCommand; private readonly IFeatureService _featureService; private readonly ILicensingService _licensingService; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; @@ -64,7 +62,6 @@ public class OrganizationsControllerTests : IDisposable _organizationUserRepository = Substitute.For(); _paymentService = Substitute.For(); _policyRepository = Substitute.For(); - _providerRepository = Substitute.For(); _ssoConfigRepository = Substitute.For(); _ssoConfigService = Substitute.For(); _getOrganizationApiKeyQuery = Substitute.For(); @@ -73,19 +70,33 @@ public class OrganizationsControllerTests : IDisposable _userService = Substitute.For(); _cloudGetOrganizationLicenseQuery = Substitute.For(); _createOrganizationApiKeyCommand = Substitute.For(); - _updateOrganizationLicenseCommand = Substitute.For(); _featureService = Substitute.For(); _licensingService = Substitute.For(); _updateSecretsManagerSubscriptionCommand = Substitute.For(); _upgradeOrganizationPlanCommand = Substitute.For(); _addSecretsManagerSubscriptionCommand = Substitute.For(); - _sut = new OrganizationsController(_organizationRepository, _organizationUserRepository, - _policyRepository, _providerRepository, _organizationService, _userService, _paymentService, _currentContext, - _ssoConfigRepository, _ssoConfigService, _getOrganizationApiKeyQuery, _rotateOrganizationApiKeyCommand, - _createOrganizationApiKeyCommand, _organizationApiKeyRepository, _updateOrganizationLicenseCommand, - _cloudGetOrganizationLicenseQuery, _featureService, _globalSettings, _licensingService, - _updateSecretsManagerSubscriptionCommand, _upgradeOrganizationPlanCommand, _addSecretsManagerSubscriptionCommand); + _sut = new OrganizationsController( + _organizationRepository, + _organizationUserRepository, + _policyRepository, + _organizationService, + _userService, + _paymentService, + _currentContext, + _ssoConfigRepository, + _ssoConfigService, + _getOrganizationApiKeyQuery, + _rotateOrganizationApiKeyCommand, + _createOrganizationApiKeyCommand, + _organizationApiKeyRepository, + _cloudGetOrganizationLicenseQuery, + _featureService, + _globalSettings, + _licensingService, + _updateSecretsManagerSubscriptionCommand, + _upgradeOrganizationPlanCommand, + _addSecretsManagerSubscriptionCommand); } public void Dispose() diff --git a/test/Core.Test/Billing/Commands/RemovePaymentMethodCommandTests.cs b/test/Core.Test/Billing/Commands/RemovePaymentMethodCommandTests.cs new file mode 100644 index 0000000000..5de14f006f --- /dev/null +++ b/test/Core.Test/Billing/Commands/RemovePaymentMethodCommandTests.cs @@ -0,0 +1,367 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Billing.Commands.Implementations; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; +using BT = Braintree; +using S = Stripe; + +namespace Bit.Core.Test.Billing.Commands; + +[SutProviderCustomize] +public class RemovePaymentMethodCommandTests +{ + [Theory, BitAutoData] + public async Task RemovePaymentMethod_NullOrganization_ArgumentNullException( + SutProvider sutProvider) => + await Assert.ThrowsAsync(() => sutProvider.Sut.RemovePaymentMethod(null)); + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_NonStripeGateway_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.BitPay; + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_NoGatewayCustomerId_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + organization.GatewayCustomerId = null; + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_NoStripeCustomer_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + sutProvider.GetDependency() + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .ReturnsNull(); + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Braintree_NoCustomer_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + const string braintreeCustomerId = "1"; + + var stripeCustomer = new S.Customer + { + Metadata = new Dictionary + { + { "btCustomerId", braintreeCustomerId } + } + }; + + sutProvider.GetDependency() + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + var (braintreeGateway, customerGateway, paymentMethodGateway) = Setup(sutProvider.GetDependency()); + + customerGateway.FindAsync(braintreeCustomerId).ReturnsNull(); + + braintreeGateway.Customer.Returns(customerGateway); + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + + await customerGateway.Received(1).FindAsync(braintreeCustomerId); + + await customerGateway.DidNotReceiveWithAnyArgs() + .UpdateAsync(Arg.Any(), Arg.Any()); + + await paymentMethodGateway.DidNotReceiveWithAnyArgs().DeleteAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Braintree_NoPaymentMethod_NoOp( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + const string braintreeCustomerId = "1"; + + var stripeCustomer = new S.Customer + { + Metadata = new Dictionary + { + { "btCustomerId", braintreeCustomerId } + } + }; + + sutProvider.GetDependency() + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + var (_, customerGateway, paymentMethodGateway) = Setup(sutProvider.GetDependency()); + + var braintreeCustomer = Substitute.For(); + + braintreeCustomer.PaymentMethods.Returns(Array.Empty()); + + customerGateway.FindAsync(braintreeCustomerId).Returns(braintreeCustomer); + + await sutProvider.Sut.RemovePaymentMethod(organization); + + await customerGateway.Received(1).FindAsync(braintreeCustomerId); + + await customerGateway.DidNotReceiveWithAnyArgs().UpdateAsync(Arg.Any(), Arg.Any()); + + await paymentMethodGateway.DidNotReceiveWithAnyArgs().DeleteAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Braintree_CustomerUpdateFails_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + const string braintreeCustomerId = "1"; + const string braintreePaymentMethodToken = "TOKEN"; + + var stripeCustomer = new S.Customer + { + Metadata = new Dictionary + { + { "btCustomerId", braintreeCustomerId } + } + }; + + sutProvider.GetDependency() + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + var (_, customerGateway, paymentMethodGateway) = Setup(sutProvider.GetDependency()); + + var braintreeCustomer = Substitute.For(); + + var paymentMethod = Substitute.For(); + paymentMethod.Token.Returns(braintreePaymentMethodToken); + paymentMethod.IsDefault.Returns(true); + + braintreeCustomer.PaymentMethods.Returns(new[] + { + paymentMethod + }); + + customerGateway.FindAsync(braintreeCustomerId).Returns(braintreeCustomer); + + var updateBraintreeCustomerResult = Substitute.For>(); + updateBraintreeCustomerResult.IsSuccess().Returns(false); + + customerGateway.UpdateAsync( + braintreeCustomerId, + Arg.Is(request => request.DefaultPaymentMethodToken == null)) + .Returns(updateBraintreeCustomerResult); + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + + await customerGateway.Received(1).FindAsync(braintreeCustomerId); + + await customerGateway.Received(1).UpdateAsync(braintreeCustomerId, Arg.Is(request => + request.DefaultPaymentMethodToken == null)); + + await paymentMethodGateway.DidNotReceiveWithAnyArgs().DeleteAsync(paymentMethod.Token); + + await customerGateway.DidNotReceive().UpdateAsync(braintreeCustomerId, Arg.Is(request => + request.DefaultPaymentMethodToken == paymentMethod.Token)); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Braintree_PaymentMethodDeleteFails_RollBack_ContactSupport( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + const string braintreeCustomerId = "1"; + const string braintreePaymentMethodToken = "TOKEN"; + + var stripeCustomer = new S.Customer + { + Metadata = new Dictionary + { + { "btCustomerId", braintreeCustomerId } + } + }; + + sutProvider.GetDependency() + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + var (_, customerGateway, paymentMethodGateway) = Setup(sutProvider.GetDependency()); + + var braintreeCustomer = Substitute.For(); + + var paymentMethod = Substitute.For(); + paymentMethod.Token.Returns(braintreePaymentMethodToken); + paymentMethod.IsDefault.Returns(true); + + braintreeCustomer.PaymentMethods.Returns(new[] + { + paymentMethod + }); + + customerGateway.FindAsync(braintreeCustomerId).Returns(braintreeCustomer); + + var updateBraintreeCustomerResult = Substitute.For>(); + updateBraintreeCustomerResult.IsSuccess().Returns(true); + + customerGateway.UpdateAsync(braintreeCustomerId, Arg.Any()) + .Returns(updateBraintreeCustomerResult); + + var deleteBraintreePaymentMethodResult = Substitute.For>(); + deleteBraintreePaymentMethodResult.IsSuccess().Returns(false); + + paymentMethodGateway.DeleteAsync(paymentMethod.Token).Returns(deleteBraintreePaymentMethodResult); + + await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization)); + + await customerGateway.Received(1).FindAsync(braintreeCustomerId); + + await customerGateway.Received(1).UpdateAsync(braintreeCustomerId, Arg.Is(request => + request.DefaultPaymentMethodToken == null)); + + await paymentMethodGateway.Received(1).DeleteAsync(paymentMethod.Token); + + await customerGateway.Received(1).UpdateAsync(braintreeCustomerId, Arg.Is(request => + request.DefaultPaymentMethodToken == paymentMethod.Token)); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Stripe_Legacy_RemovesSources( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + + const string bankAccountId = "bank_account_id"; + const string cardId = "card_id"; + + var sources = new List + { + new S.BankAccount { Id = bankAccountId }, new S.Card { Id = cardId } + }; + + var stripeCustomer = new S.Customer { Sources = new S.StripeList { Data = sources } }; + + var stripeAdapter = sutProvider.GetDependency(); + + stripeAdapter + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + stripeAdapter + .PaymentMethodListAutoPagingAsync(Arg.Any()) + .Returns(GetPaymentMethodsAsync(new List())); + + await sutProvider.Sut.RemovePaymentMethod(organization); + + await stripeAdapter.Received(1).BankAccountDeleteAsync(stripeCustomer.Id, bankAccountId); + + await stripeAdapter.Received(1).CardDeleteAsync(stripeCustomer.Id, cardId); + + await stripeAdapter.DidNotReceiveWithAnyArgs() + .PaymentMethodDetachAsync(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RemovePaymentMethod_Stripe_DetachesPaymentMethods( + Organization organization, + SutProvider sutProvider) + { + organization.Gateway = GatewayType.Stripe; + const string bankAccountId = "bank_account_id"; + const string cardId = "card_id"; + + var sources = new List(); + + var stripeCustomer = new S.Customer { Sources = new S.StripeList { Data = sources } }; + + var stripeAdapter = sutProvider.GetDependency(); + + stripeAdapter + .CustomerGetAsync(organization.GatewayCustomerId, Arg.Any()) + .Returns(stripeCustomer); + + stripeAdapter + .PaymentMethodListAutoPagingAsync(Arg.Any()) + .Returns(GetPaymentMethodsAsync(new List + { + new () + { + Id = bankAccountId + }, + new () + { + Id = cardId + } + })); + + await sutProvider.Sut.RemovePaymentMethod(organization); + + await stripeAdapter.DidNotReceiveWithAnyArgs().BankAccountDeleteAsync(Arg.Any(), Arg.Any()); + + await stripeAdapter.DidNotReceiveWithAnyArgs().CardDeleteAsync(Arg.Any(), Arg.Any()); + + await stripeAdapter.Received(1) + .PaymentMethodDetachAsync(bankAccountId, Arg.Any()); + + await stripeAdapter.Received(1) + .PaymentMethodDetachAsync(cardId, Arg.Any()); + } + + private static async IAsyncEnumerable GetPaymentMethodsAsync( + IEnumerable paymentMethods) + { + foreach (var paymentMethod in paymentMethods) + { + yield return paymentMethod; + } + + await Task.CompletedTask; + } + + private static (BT.IBraintreeGateway, BT.ICustomerGateway, BT.IPaymentMethodGateway) Setup( + BT.IBraintreeGateway braintreeGateway) + { + var customerGateway = Substitute.For(); + var paymentMethodGateway = Substitute.For(); + + braintreeGateway.Customer.Returns(customerGateway); + braintreeGateway.PaymentMethod.Returns(paymentMethodGateway); + + return (braintreeGateway, customerGateway, paymentMethodGateway); + } + + private static async Task ThrowsContactSupportAsync(Func function) + { + const string message = "Could not remove your payment method. Please contact support for assistance."; + + var exception = await Assert.ThrowsAsync(function); + + Assert.Equal(message, exception.Message); + } +} From da907c879b9560c82cce6ae6d99bf208a1712700 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:18:05 -0500 Subject: [PATCH 03/12] [deps] SM: Update Dapper to v2.1.28 (#3665) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Infrastructure.Dapper/Infrastructure.Dapper.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Infrastructure.Dapper/Infrastructure.Dapper.csproj b/src/Infrastructure.Dapper/Infrastructure.Dapper.csproj index d8a57b3303..6c7ad57d19 100644 --- a/src/Infrastructure.Dapper/Infrastructure.Dapper.csproj +++ b/src/Infrastructure.Dapper/Infrastructure.Dapper.csproj @@ -5,7 +5,7 @@ - + From 2df5fe1340ec968d427ae24207027aeb607a8ad0 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:30:23 -0700 Subject: [PATCH 04/12] [deps] SM: Update EntityFrameworkCore to v7.0.15 (#3666) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .config/dotnet-tools.json | 2 +- .../Infrastructure.EntityFramework.csproj | 6 +++--- util/MySqlMigrations/MySqlMigrations.csproj | 2 +- util/PostgresMigrations/PostgresMigrations.csproj | 2 +- util/SqlServerEFScaffold/SqlServerEFScaffold.csproj | 2 +- util/SqliteMigrations/SqliteMigrations.csproj | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 3a7def9a18..a3850de029 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -7,7 +7,7 @@ "commands": ["swagger"] }, "dotnet-ef": { - "version": "7.0.14", + "version": "7.0.15", "commands": ["dotnet-ef"] } } diff --git a/src/Infrastructure.EntityFramework/Infrastructure.EntityFramework.csproj b/src/Infrastructure.EntityFramework/Infrastructure.EntityFramework.csproj index 51e7d935db..0a5cdaed5b 100644 --- a/src/Infrastructure.EntityFramework/Infrastructure.EntityFramework.csproj +++ b/src/Infrastructure.EntityFramework/Infrastructure.EntityFramework.csproj @@ -3,9 +3,9 @@ - - - + + + diff --git a/util/MySqlMigrations/MySqlMigrations.csproj b/util/MySqlMigrations/MySqlMigrations.csproj index 4a10d7119a..0bf3bd76a5 100644 --- a/util/MySqlMigrations/MySqlMigrations.csproj +++ b/util/MySqlMigrations/MySqlMigrations.csproj @@ -10,7 +10,7 @@ - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/util/PostgresMigrations/PostgresMigrations.csproj b/util/PostgresMigrations/PostgresMigrations.csproj index f81892da62..370d3e8db5 100644 --- a/util/PostgresMigrations/PostgresMigrations.csproj +++ b/util/PostgresMigrations/PostgresMigrations.csproj @@ -6,7 +6,7 @@ - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/util/SqlServerEFScaffold/SqlServerEFScaffold.csproj b/util/SqlServerEFScaffold/SqlServerEFScaffold.csproj index ceb6a8199d..179f291e43 100644 --- a/util/SqlServerEFScaffold/SqlServerEFScaffold.csproj +++ b/util/SqlServerEFScaffold/SqlServerEFScaffold.csproj @@ -1,6 +1,6 @@ - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/util/SqliteMigrations/SqliteMigrations.csproj b/util/SqliteMigrations/SqliteMigrations.csproj index 7c41eae070..6973cdee90 100644 --- a/util/SqliteMigrations/SqliteMigrations.csproj +++ b/util/SqliteMigrations/SqliteMigrations.csproj @@ -12,7 +12,7 @@ - + runtime; build; native; contentfiles; analyzers; buildtransitive all From 52f3fa0f95dc1f767a1c41fb8422fdcc9d5710d7 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 16 Jan 2024 08:38:20 -0500 Subject: [PATCH 05/12] Make billing email field uneditable for organizations' (#3591) Co-authored-by: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> --- src/Admin/Views/Shared/_OrganizationForm.cshtml | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/Admin/Views/Shared/_OrganizationForm.cshtml b/src/Admin/Views/Shared/_OrganizationForm.cshtml index 72076cbd37..97b6b949e0 100644 --- a/src/Admin/Views/Shared/_OrganizationForm.cshtml +++ b/src/Admin/Views/Shared/_OrganizationForm.cshtml @@ -276,20 +276,7 @@
- @if (Model.Provider?.Type == ProviderType.Reseller) - { - - } - else - { - - } +
From c12c09897bd3359434e2f84ee4080c8b296299f8 Mon Sep 17 00:00:00 2001 From: Matt Bishop Date: Tue, 16 Jan 2024 09:08:09 -0500 Subject: [PATCH 06/12] Remove Renovate .NET constraint (#3670) --- .github/renovate.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index 714baad8e1..8fb079a7de 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -203,10 +203,5 @@ "reviewers": ["team:team-vault-dev"] } ], - "force": { - "constraints": { - "dotnet": "6.0.100" - } - }, "ignoreDeps": ["dotnet-sdk"] } From b97a1a9ed2dd5a84cd87d018634fe65da736b01f Mon Sep 17 00:00:00 2001 From: Matt Bishop Date: Tue, 16 Jan 2024 09:08:55 -0500 Subject: [PATCH 07/12] [PM-5519] [PM-5526] [PM-5624] [PM-5600] More Grant SQL fixes (#3668) * SQLite scripts to apply autoincrementing Id key * Drop erroneous Id column if created --- .../GrantEntityTypeConfiguration.cs | 1 + .../20231214162533_GrantIdWithIndexes.cs | 17 +++- .../2023-12-04_00_Down_GrantIndexes.sql | 45 +++++++++ .../2023-12-04_00_Up_GrantIndexes.sql | 46 +++++++++ .../20231214162537_GrantIdWithIndexes.cs | 97 ++----------------- util/SqliteMigrations/SqliteMigrations.csproj | 5 + 6 files changed, 119 insertions(+), 92 deletions(-) create mode 100644 util/SqliteMigrations/HelperScripts/2023-12-04_00_Down_GrantIndexes.sql create mode 100644 util/SqliteMigrations/HelperScripts/2023-12-04_00_Up_GrantIndexes.sql diff --git a/src/Infrastructure.EntityFramework/Auth/Configurations/GrantEntityTypeConfiguration.cs b/src/Infrastructure.EntityFramework/Auth/Configurations/GrantEntityTypeConfiguration.cs index 599be37a84..77d8d1eb9f 100644 --- a/src/Infrastructure.EntityFramework/Auth/Configurations/GrantEntityTypeConfiguration.cs +++ b/src/Infrastructure.EntityFramework/Auth/Configurations/GrantEntityTypeConfiguration.cs @@ -10,6 +10,7 @@ public class GrantEntityTypeConfiguration : IEntityTypeConfiguration { builder .HasKey(s => s.Id) + .HasName("PK_Grant") .IsClustered(); builder diff --git a/util/MySqlMigrations/Migrations/20231214162533_GrantIdWithIndexes.cs b/util/MySqlMigrations/Migrations/20231214162533_GrantIdWithIndexes.cs index c9fd5638b0..1e4c178ade 100644 --- a/util/MySqlMigrations/Migrations/20231214162533_GrantIdWithIndexes.cs +++ b/util/MySqlMigrations/Migrations/20231214162533_GrantIdWithIndexes.cs @@ -72,7 +72,22 @@ public partial class GrantIdWithIndexes : Migration .Annotation("MySql:CharSet", "utf8mb4") .OldAnnotation("MySql:CharSet", "utf8mb4"); - migrationBuilder.Sql("ALTER TABLE `Grant` ADD COLUMN `Id` INT AUTO_INCREMENT UNIQUE;"); + migrationBuilder.Sql(@" + DROP PROCEDURE IF EXISTS GrantSchemaChange; + + CREATE PROCEDURE GrantSchemaChange() + BEGIN + IF EXISTS (SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_NAME = 'Grant' AND COLUMN_NAME = 'Id') THEN + ALTER TABLE `Grant` DROP COLUMN `Id`; + END IF; + + ALTER TABLE `Grant` ADD COLUMN `Id` INT AUTO_INCREMENT UNIQUE; + END; + + CALL GrantSchemaChange(); + + DROP PROCEDURE GrantSchemaChange;" + ); migrationBuilder.AddPrimaryKey( name: "PK_Grant", diff --git a/util/SqliteMigrations/HelperScripts/2023-12-04_00_Down_GrantIndexes.sql b/util/SqliteMigrations/HelperScripts/2023-12-04_00_Down_GrantIndexes.sql new file mode 100644 index 0000000000..5ea59ef753 --- /dev/null +++ b/util/SqliteMigrations/HelperScripts/2023-12-04_00_Down_GrantIndexes.sql @@ -0,0 +1,45 @@ +ALTER TABLE +"Grant" RENAME TO "Old_Grant"; + +CREATE TABLE "Grant" +( + "Key" TEXT NOT NULL CONSTRAINT "PK_Grant" PRIMARY KEY, + "Type" TEXT NULL, + "SubjectId" TEXT NULL, + "SessionId" TEXT NULL, + "ClientId" TEXT NULL, + "Description" TEXT NULL, + "CreationDate" TEXT NOT NULL, + "ExpirationDate" TEXT NULL, + "ConsumedDate" TEXT NULL, + "Data" TEXT NULL +); + +INSERT INTO +"Grant" + ( + "Key", + "Type", + "SubjectId", + "SessionId", + "ClientId", + "Description", + "CreationDate", + "ExpirationDate", + "ConsumedDate", + "Data" + ) +SELECT + "Key", + "Type", + "SubjectId", + "SessionId", + "ClientId", + "Description", + "CreationDate", + "ExpirationDate", + "ConsumedDate", + "Data" +FROM "Old_Grant"; + +DROP TABLE "Old_Grant"; diff --git a/util/SqliteMigrations/HelperScripts/2023-12-04_00_Up_GrantIndexes.sql b/util/SqliteMigrations/HelperScripts/2023-12-04_00_Up_GrantIndexes.sql new file mode 100644 index 0000000000..028214df67 --- /dev/null +++ b/util/SqliteMigrations/HelperScripts/2023-12-04_00_Up_GrantIndexes.sql @@ -0,0 +1,46 @@ +ALTER TABLE +"Grant" RENAME TO "Old_Grant"; + +CREATE TABLE "Grant" +( + "Id" INTEGER PRIMARY KEY AUTOINCREMENT, + "Key" TEXT NOT NULL, + "Type" TEXT NOT NULL, + "SubjectId" TEXT NULL, + "SessionId" TEXT NULL, + "ClientId" TEXT NOT NULL, + "Description" TEXT NULL, + "CreationDate" TEXT NOT NULL, + "ExpirationDate" TEXT NULL, + "ConsumedDate" TEXT NULL, + "Data" TEXT NOT NULL +); + +INSERT INTO +"Grant" + ( + "Key", + "Type", + "SubjectId", + "SessionId", + "ClientId", + "Description", + "CreationDate", + "ExpirationDate", + "ConsumedDate", + "Data" + ) +SELECT + "Key", + "Type", + "SubjectId", + "SessionId", + "ClientId", + "Description", + "CreationDate", + "ExpirationDate", + "ConsumedDate", + "Data" +FROM "Old_Grant"; + +DROP TABLE "Old_Grant"; diff --git a/util/SqliteMigrations/Migrations/20231214162537_GrantIdWithIndexes.cs b/util/SqliteMigrations/Migrations/20231214162537_GrantIdWithIndexes.cs index 1409fd055d..74f8d9b608 100644 --- a/util/SqliteMigrations/Migrations/20231214162537_GrantIdWithIndexes.cs +++ b/util/SqliteMigrations/Migrations/20231214162537_GrantIdWithIndexes.cs @@ -1,4 +1,5 @@ -using Microsoft.EntityFrameworkCore.Migrations; +using Bit.EfShared; +using Microsoft.EntityFrameworkCore.Migrations; #nullable disable @@ -7,59 +8,12 @@ namespace Bit.SqliteMigrations.Migrations; /// public partial class GrantIdWithIndexes : Migration { + private const string _scriptLocationTemplate = "2023-12-04_00_{0}_GrantIndexes.sql"; + /// protected override void Up(MigrationBuilder migrationBuilder) { - migrationBuilder.DropPrimaryKey( - name: "PK_Grant", - table: "Grant"); - - migrationBuilder.AlterColumn( - name: "Type", - table: "Grant", - type: "TEXT", - maxLength: 50, - nullable: false, - defaultValue: "", - oldClrType: typeof(string), - oldType: "TEXT", - oldMaxLength: 50, - oldNullable: true); - - migrationBuilder.AlterColumn( - name: "Data", - table: "Grant", - type: "TEXT", - nullable: false, - defaultValue: "", - oldClrType: typeof(string), - oldType: "TEXT", - oldNullable: true); - - migrationBuilder.AlterColumn( - name: "ClientId", - table: "Grant", - type: "TEXT", - maxLength: 200, - nullable: false, - defaultValue: "", - oldClrType: typeof(string), - oldType: "TEXT", - oldMaxLength: 200, - oldNullable: true); - - migrationBuilder.AddColumn( - name: "Id", - table: "Grant", - type: "INTEGER", - nullable: false, - defaultValue: 0) - .Annotation("Sqlite:Autoincrement", true); - - migrationBuilder.AddPrimaryKey( - name: "PK_Grant", - table: "Grant", - column: "Id"); + migrationBuilder.SqlResource(_scriptLocationTemplate); migrationBuilder.CreateIndex( name: "IX_Grant_Key", @@ -71,49 +25,10 @@ public partial class GrantIdWithIndexes : Migration /// protected override void Down(MigrationBuilder migrationBuilder) { - migrationBuilder.DropPrimaryKey( - name: "PK_Grant", - table: "Grant"); + migrationBuilder.SqlResource(_scriptLocationTemplate); migrationBuilder.DropIndex( name: "IX_Grant_Key", table: "Grant"); - - migrationBuilder.DropColumn( - name: "Id", - table: "Grant"); - - migrationBuilder.AlterColumn( - name: "Type", - table: "Grant", - type: "TEXT", - maxLength: 50, - nullable: true, - oldClrType: typeof(string), - oldType: "TEXT", - oldMaxLength: 50); - - migrationBuilder.AlterColumn( - name: "Data", - table: "Grant", - type: "TEXT", - nullable: true, - oldClrType: typeof(string), - oldType: "TEXT"); - - migrationBuilder.AlterColumn( - name: "ClientId", - table: "Grant", - type: "TEXT", - maxLength: 200, - nullable: true, - oldClrType: typeof(string), - oldType: "TEXT", - oldMaxLength: 200); - - migrationBuilder.AddPrimaryKey( - name: "PK_Grant", - table: "Grant", - column: "Key"); } } diff --git a/util/SqliteMigrations/SqliteMigrations.csproj b/util/SqliteMigrations/SqliteMigrations.csproj index 6973cdee90..ba322375c4 100644 --- a/util/SqliteMigrations/SqliteMigrations.csproj +++ b/util/SqliteMigrations/SqliteMigrations.csproj @@ -22,4 +22,9 @@ + + + + + From 40d5e6ac7311e20bca291e670934d7556285d58d Mon Sep 17 00:00:00 2001 From: Bitwarden DevOps <106330231+bitwarden-devops-bot@users.noreply.github.com> Date: Tue, 16 Jan 2024 09:39:33 -0500 Subject: [PATCH 08/12] Bumped version to 2024.1.1 (#3673) --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 98d030daa7..c677489378 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,7 +2,7 @@ net6.0 - 2024.1.0 + 2024.1.1 Bit.$(MSBuildProjectName) enable false From dca8d00f5447e0bf0233dcd51d5d717d29435415 Mon Sep 17 00:00:00 2001 From: Bitwarden DevOps <106330231+bitwarden-devops-bot@users.noreply.github.com> Date: Tue, 16 Jan 2024 12:02:24 -0500 Subject: [PATCH 09/12] Bumped version to 2024.1.2 (#3674) --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index c677489378..5c11359b54 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,7 +2,7 @@ net6.0 - 2024.1.1 + 2024.1.2 Bit.$(MSBuildProjectName) enable false From f09bc43b047448301b3cc69651d3ec1e501dd064 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 16 Jan 2024 15:46:22 -0500 Subject: [PATCH 10/12] [deps] Billing: Update BenchmarkDotNet to v0.13.12 (#3677) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- perf/MicroBenchmarks/MicroBenchmarks.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perf/MicroBenchmarks/MicroBenchmarks.csproj b/perf/MicroBenchmarks/MicroBenchmarks.csproj index 7a2bdb02d9..d3ee14a684 100644 --- a/perf/MicroBenchmarks/MicroBenchmarks.csproj +++ b/perf/MicroBenchmarks/MicroBenchmarks.csproj @@ -8,7 +8,7 @@ - + From ef37cdc71a503c059a6b11af7ce124b98a596749 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 16 Jan 2024 15:47:26 -0500 Subject: [PATCH 11/12] [deps] Billing: Update Braintree to v5.23.0 (#3678) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 9b664a788d..6752cca078 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -52,7 +52,7 @@ - + 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 12/12] [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)