From df21d574e1c252fa0b4eb8a07492945b109fad0d Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 15 Nov 2024 09:30:03 -0500 Subject: [PATCH 1/9] [PM-11798] Remove `enable-consolidated-billing` feature flag (#5028) * Remove flag from CreateProviderCommand * Remove flag from OrganizationsController * Consolidate provider extensions * Remove flag from ProvidersController * Remove flag from CreateMsp.cshtml * Remove flag from Provider Edit.cshtml Also ensured the editable Gateway fields show for Multi-organization enterprises * Remove flag from OrganizationsController * Remove flag from billing-owned provider controllers * Remove flag from OrganizationService * Remove flag from RemoveOrganizationFromProviderCommand * Remove flag from ProviderService * Remove flag * Run dotnet format * Fix failing tests --- .../Providers/CreateProviderCommand.cs | 33 ++------ .../RemoveOrganizationFromProviderCommand.cs | 11 +-- .../AdminConsole/Services/ProviderService.cs | 68 ++++++--------- ...oveOrganizationFromProviderCommandTests.cs | 7 -- .../Services/ProviderServiceTests.cs | 68 ++++----------- .../Controllers/OrganizationsController.cs | 16 ++-- .../Controllers/ProvidersController.cs | 9 +- .../Views/Providers/CreateMsp.cshtml | 8 -- .../AdminConsole/Views/Providers/Edit.cshtml | 82 +++++++++---------- .../Controllers/OrganizationsController.cs | 7 +- .../Controllers/BaseProviderController.cs | 13 +-- .../Controllers/ProviderBillingController.cs | 3 +- .../Controllers/ProviderClientsController.cs | 3 +- .../Implementations/OrganizationService.cs | 13 +-- .../Billing/Extensions/BillingExtensions.cs | 18 ++-- src/Core/Constants.cs | 1 - .../OrganizationsControllerTests.cs | 7 -- .../OrganizationsControllerTests.cs | 2 - .../ProviderBillingControllerTests.cs | 45 ---------- test/Api.Test/Billing/Utilities.cs | 5 -- .../Services/OrganizationServiceTests.cs | 2 - 21 files changed, 119 insertions(+), 302 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/CreateProviderCommand.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/CreateProviderCommand.cs index 3b01370ef..69b7e67ec 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/CreateProviderCommand.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/CreateProviderCommand.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; @@ -10,7 +9,6 @@ using Bit.Core.Billing.Repositories; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; -using Bit.Core.Services; namespace Bit.Commercial.Core.AdminConsole.Providers; @@ -21,35 +19,28 @@ public class CreateProviderCommand : ICreateProviderCommand private readonly IProviderService _providerService; private readonly IUserRepository _userRepository; private readonly IProviderPlanRepository _providerPlanRepository; - private readonly IFeatureService _featureService; public CreateProviderCommand( IProviderRepository providerRepository, IProviderUserRepository providerUserRepository, IProviderService providerService, IUserRepository userRepository, - IProviderPlanRepository providerPlanRepository, - IFeatureService featureService) + IProviderPlanRepository providerPlanRepository) { _providerRepository = providerRepository; _providerUserRepository = providerUserRepository; _providerService = providerService; _userRepository = userRepository; _providerPlanRepository = providerPlanRepository; - _featureService = featureService; } public async Task CreateMspAsync(Provider provider, string ownerEmail, int teamsMinimumSeats, int enterpriseMinimumSeats) { var providerId = await CreateProviderAsync(provider, ownerEmail); - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (isConsolidatedBillingEnabled) - { - await CreateProviderPlanAsync(providerId, PlanType.TeamsMonthly, teamsMinimumSeats); - await CreateProviderPlanAsync(providerId, PlanType.EnterpriseMonthly, enterpriseMinimumSeats); - } + await Task.WhenAll( + CreateProviderPlanAsync(providerId, PlanType.TeamsMonthly, teamsMinimumSeats), + CreateProviderPlanAsync(providerId, PlanType.EnterpriseMonthly, enterpriseMinimumSeats)); } public async Task CreateResellerAsync(Provider provider) @@ -61,12 +52,7 @@ public class CreateProviderCommand : ICreateProviderCommand { var providerId = await CreateProviderAsync(provider, ownerEmail); - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (isConsolidatedBillingEnabled) - { - await CreateProviderPlanAsync(providerId, plan, minimumSeats); - } + await CreateProviderPlanAsync(providerId, plan, minimumSeats); } private async Task CreateProviderAsync(Provider provider, string ownerEmail) @@ -77,12 +63,7 @@ public class CreateProviderCommand : ICreateProviderCommand throw new BadRequestException("Invalid owner. Owner must be an existing Bitwarden user."); } - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (isConsolidatedBillingEnabled) - { - provider.Gateway = GatewayType.Stripe; - } + provider.Gateway = GatewayType.Stripe; await ProviderRepositoryCreateAsync(provider, ProviderStatusType.Pending); diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs index 045fd5059..ce0c0c933 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs @@ -1,7 +1,5 @@ -using Bit.Core; -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; -using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; @@ -102,11 +100,8 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv Provider provider, IEnumerable organizationOwnerEmails) { - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (isConsolidatedBillingEnabled && - provider.Status == ProviderStatusType.Billable && - organization.Status == OrganizationStatusType.Managed && + if (provider.IsBillable() && + organization.IsValidClient() && !string.IsNullOrEmpty(organization.GatewayCustomerId)) { await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId, new CustomerUpdateOptions diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs index 48ea903ad..e384d71df 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs @@ -8,7 +8,6 @@ using Bit.Core.AdminConsole.Models.Business.Tokenables; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Billing.Enums; -using Bit.Core.Billing.Extensions; using Bit.Core.Billing.Services; using Bit.Core.Context; using Bit.Core.Entities; @@ -101,24 +100,16 @@ public class ProviderService : IProviderService throw new BadRequestException("Invalid owner."); } - if (!_featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling)) + if (taxInfo == null || string.IsNullOrEmpty(taxInfo.BillingAddressCountry) || string.IsNullOrEmpty(taxInfo.BillingAddressPostalCode)) { - provider.Status = ProviderStatusType.Created; - await _providerRepository.UpsertAsync(provider); - } - else - { - if (taxInfo == null || string.IsNullOrEmpty(taxInfo.BillingAddressCountry) || string.IsNullOrEmpty(taxInfo.BillingAddressPostalCode)) - { - throw new BadRequestException("Both address and postal code are required to set up your provider."); - } - var customer = await _providerBillingService.SetupCustomer(provider, taxInfo); - provider.GatewayCustomerId = customer.Id; - var subscription = await _providerBillingService.SetupSubscription(provider); - provider.GatewaySubscriptionId = subscription.Id; - provider.Status = ProviderStatusType.Billable; - await _providerRepository.UpsertAsync(provider); + throw new BadRequestException("Both address and postal code are required to set up your provider."); } + var customer = await _providerBillingService.SetupCustomer(provider, taxInfo); + provider.GatewayCustomerId = customer.Id; + var subscription = await _providerBillingService.SetupSubscription(provider); + provider.GatewaySubscriptionId = subscription.Id; + provider.Status = ProviderStatusType.Billable; + await _providerRepository.UpsertAsync(provider); providerUser.Key = key; await _providerUserRepository.ReplaceAsync(providerUser); @@ -545,13 +536,9 @@ public class ProviderService : IProviderService { var provider = await _providerRepository.GetByIdAsync(providerId); - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) && provider.IsBillable(); + ThrowOnInvalidPlanType(provider.Type, organizationSignup.Plan); - ThrowOnInvalidPlanType(provider.Type, organizationSignup.Plan, consolidatedBillingEnabled); - - var (organization, _, defaultCollection) = consolidatedBillingEnabled - ? await _organizationService.SignupClientAsync(organizationSignup) - : await _organizationService.SignUpAsync(organizationSignup); + var (organization, _, defaultCollection) = await _organizationService.SignupClientAsync(organizationSignup); var providerOrganization = new ProviderOrganization { @@ -687,27 +674,24 @@ public class ProviderService : IProviderService return confirmedOwnersIds.Except(providerUserIds).Any(); } - private void ThrowOnInvalidPlanType(ProviderType providerType, PlanType requestedType, bool consolidatedBillingEnabled = false) + private void ThrowOnInvalidPlanType(ProviderType providerType, PlanType requestedType) { - if (consolidatedBillingEnabled) + switch (providerType) { - switch (providerType) - { - case ProviderType.Msp: - if (requestedType is not (PlanType.TeamsMonthly or PlanType.EnterpriseMonthly)) - { - throw new BadRequestException($"Managed Service Providers cannot manage organizations with the plan type {requestedType}. Only Teams (Monthly) and Enterprise (Monthly) are allowed."); - } - break; - case ProviderType.MultiOrganizationEnterprise: - if (requestedType is not (PlanType.EnterpriseMonthly or PlanType.EnterpriseAnnually)) - { - throw new BadRequestException($"Multi-organization Enterprise Providers cannot manage organizations with the plan type {requestedType}. Only Enterprise (Monthly) and Enterprise (Annually) are allowed."); - } - break; - default: - throw new BadRequestException($"Unsupported provider type {providerType}."); - } + case ProviderType.Msp: + if (requestedType is not (PlanType.TeamsMonthly or PlanType.EnterpriseMonthly)) + { + throw new BadRequestException($"Managed Service Providers cannot manage organizations with the plan type {requestedType}. Only Teams (Monthly) and Enterprise (Monthly) are allowed."); + } + break; + case ProviderType.MultiOrganizationEnterprise: + if (requestedType is not (PlanType.EnterpriseMonthly or PlanType.EnterpriseAnnually)) + { + throw new BadRequestException($"Multi-organization Enterprise Providers cannot manage organizations with the plan type {requestedType}. Only Enterprise (Monthly) and Enterprise (Annually) are allowed."); + } + break; + default: + throw new BadRequestException($"Unsupported provider type {providerType}."); } if (ProviderDisallowedOrganizationTypes.Contains(requestedType)) diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs index e984259e9..f45ab7504 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs @@ -1,5 +1,4 @@ using Bit.Commercial.Core.AdminConsole.Providers; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; @@ -155,9 +154,6 @@ public class RemoveOrganizationFromProviderCommandTests "b@example.com" ]); - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(false); - sutProvider.GetDependency().SubscriptionGetAsync(organization.GatewaySubscriptionId) .Returns(GetSubscription(organization.GatewaySubscriptionId)); @@ -222,9 +218,6 @@ public class RemoveOrganizationFromProviderCommandTests "b@example.com" ]); - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - var stripeAdapter = sutProvider.GetDependency(); stripeAdapter.SubscriptionCreateAsync(Arg.Any()).Returns(new Subscription 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 4aac363b9..2883c9d7e 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs @@ -1,6 +1,5 @@ using Bit.Commercial.Core.AdminConsole.Services; using Bit.Commercial.Core.Test.AdminConsole.AutoFixture; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; @@ -55,36 +54,8 @@ public class ProviderServiceTests } [Theory, BitAutoData] - public async Task CompleteSetupAsync_Success(User user, Provider provider, string key, - [ProviderUser(ProviderUserStatusType.Confirmed, ProviderUserType.ProviderAdmin)] ProviderUser providerUser, - SutProvider sutProvider) - { - providerUser.ProviderId = provider.Id; - providerUser.UserId = user.Id; - var userService = sutProvider.GetDependency(); - userService.GetUserByIdAsync(user.Id).Returns(user); - - var providerUserRepository = sutProvider.GetDependency(); - providerUserRepository.GetByProviderUserAsync(provider.Id, user.Id).Returns(providerUser); - - var dataProtectionProvider = DataProtectionProvider.Create("ApplicationName"); - var protector = dataProtectionProvider.CreateProtector("ProviderServiceDataProtector"); - sutProvider.GetDependency().CreateProtector("ProviderServiceDataProtector") - .Returns(protector); - sutProvider.Create(); - - var token = protector.Protect($"ProviderSetupInvite {provider.Id} {user.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); - - await sutProvider.Sut.CompleteSetupAsync(provider, user.Id, token, key); - - await sutProvider.GetDependency().Received().UpsertAsync(provider); - await sutProvider.GetDependency().Received() - .ReplaceAsync(Arg.Is(pu => pu.UserId == user.Id && pu.ProviderId == provider.Id && pu.Key == key)); - } - - [Theory, BitAutoData] - public async Task CompleteSetupAsync_ConsolidatedBilling_Success(User user, Provider provider, string key, TaxInfo taxInfo, - [ProviderUser(ProviderUserStatusType.Confirmed, ProviderUserType.ProviderAdmin)] ProviderUser providerUser, + public async Task CompleteSetupAsync_Success(User user, Provider provider, string key, TaxInfo taxInfo, + [ProviderUser] ProviderUser providerUser, SutProvider sutProvider) { providerUser.ProviderId = provider.Id; @@ -100,9 +71,6 @@ public class ProviderServiceTests sutProvider.GetDependency().CreateProtector("ProviderServiceDataProtector") .Returns(protector); - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - var providerBillingService = sutProvider.GetDependency(); var customer = new Customer { Id = "customer_id" }; @@ -489,7 +457,7 @@ public class ProviderServiceTests public async Task AddOrganization_OrganizationHasSecretsManager_Throws(Provider provider, Organization organization, string key, SutProvider sutProvider) { - organization.PlanType = PlanType.EnterpriseAnnually; + organization.PlanType = PlanType.EnterpriseMonthly; organization.UseSecretsManager = true; sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); @@ -506,7 +474,7 @@ public class ProviderServiceTests public async Task AddOrganization_Success(Provider provider, Organization organization, string key, SutProvider sutProvider) { - organization.PlanType = PlanType.EnterpriseAnnually; + organization.PlanType = PlanType.EnterpriseMonthly; var providerRepository = sutProvider.GetDependency(); providerRepository.GetByIdAsync(provider.Id).Returns(provider); @@ -549,8 +517,8 @@ public class ProviderServiceTests sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); - var expectedPlanType = PlanType.EnterpriseAnnually; - organization.PlanType = PlanType.EnterpriseAnnually; + var expectedPlanType = PlanType.EnterpriseMonthly; + organization.PlanType = PlanType.EnterpriseMonthly; sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); await sutProvider.Sut.AddOrganization(provider.Id, organization.Id, key); @@ -579,12 +547,12 @@ public class ProviderServiceTests BackdateProviderCreationDate(provider, newCreationDate); provider.Type = ProviderType.Msp; - organization.PlanType = PlanType.EnterpriseAnnually; - organization.Plan = "Enterprise (Annually)"; + organization.PlanType = PlanType.EnterpriseMonthly; + organization.Plan = "Enterprise (Monthly)"; - var expectedPlanType = PlanType.EnterpriseAnnually2020; + var expectedPlanType = PlanType.EnterpriseMonthly2020; - var expectedPlanId = "2020-enterprise-org-seat-annually"; + var expectedPlanId = "2020-enterprise-org-seat-monthly"; sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); @@ -663,11 +631,11 @@ public class ProviderServiceTests public async Task CreateOrganizationAsync_Success(Provider provider, OrganizationSignup organizationSignup, Organization organization, string clientOwnerEmail, User user, SutProvider sutProvider) { - organizationSignup.Plan = PlanType.EnterpriseAnnually; + organizationSignup.Plan = PlanType.EnterpriseMonthly; sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); - sutProvider.GetDependency().SignUpAsync(organizationSignup) + sutProvider.GetDependency().SignupClientAsync(organizationSignup) .Returns((organization, null as OrganizationUser, new Collection())); var providerOrganization = @@ -688,7 +656,7 @@ public class ProviderServiceTests } [Theory, OrganizationCustomize, BitAutoData] - public async Task CreateOrganizationAsync_ConsolidatedBillingEnabled_InvalidPlanType_ThrowsBadRequestException( + public async Task CreateOrganizationAsync_InvalidPlanType_ThrowsBadRequestException( Provider provider, OrganizationSignup organizationSignup, Organization organization, @@ -696,8 +664,6 @@ public class ProviderServiceTests User user, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); - provider.Type = ProviderType.Msp; provider.Status = ProviderStatusType.Billable; @@ -717,7 +683,7 @@ public class ProviderServiceTests } [Theory, OrganizationCustomize, BitAutoData] - public async Task CreateOrganizationAsync_ConsolidatedBillingEnabled_InvokeSignupClientAsync( + public async Task CreateOrganizationAsync_InvokeSignupClientAsync( Provider provider, OrganizationSignup organizationSignup, Organization organization, @@ -725,8 +691,6 @@ public class ProviderServiceTests User user, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); - provider.Type = ProviderType.Msp; provider.Status = ProviderStatusType.Billable; @@ -771,11 +735,11 @@ public class ProviderServiceTests (Provider provider, OrganizationSignup organizationSignup, Organization organization, string clientOwnerEmail, User user, SutProvider sutProvider, Collection defaultCollection) { - organizationSignup.Plan = PlanType.EnterpriseAnnually; + organizationSignup.Plan = PlanType.EnterpriseMonthly; sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); - sutProvider.GetDependency().SignUpAsync(organizationSignup) + sutProvider.GetDependency().SignupClientAsync(organizationSignup) .Returns((organization, null as OrganizationUser, defaultCollection)); var providerOrganization = diff --git a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs index db41e9282..67a80a375 100644 --- a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs @@ -55,8 +55,8 @@ public class OrganizationsController : Controller private readonly IServiceAccountRepository _serviceAccountRepository; private readonly IProviderOrganizationRepository _providerOrganizationRepository; private readonly IRemoveOrganizationFromProviderCommand _removeOrganizationFromProviderCommand; - private readonly IFeatureService _featureService; private readonly IProviderBillingService _providerBillingService; + private readonly IFeatureService _featureService; public OrganizationsController( IOrganizationService organizationService, @@ -82,8 +82,8 @@ public class OrganizationsController : Controller IServiceAccountRepository serviceAccountRepository, IProviderOrganizationRepository providerOrganizationRepository, IRemoveOrganizationFromProviderCommand removeOrganizationFromProviderCommand, - IFeatureService featureService, - IProviderBillingService providerBillingService) + IProviderBillingService providerBillingService, + IFeatureService featureService) { _organizationService = organizationService; _organizationRepository = organizationRepository; @@ -108,8 +108,8 @@ public class OrganizationsController : Controller _serviceAccountRepository = serviceAccountRepository; _providerOrganizationRepository = providerOrganizationRepository; _removeOrganizationFromProviderCommand = removeOrganizationFromProviderCommand; - _featureService = featureService; _providerBillingService = providerBillingService; + _featureService = featureService; } [RequirePermission(Permission.Org_List_View)] @@ -285,9 +285,7 @@ public class OrganizationsController : Controller return RedirectToAction("Index"); } - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (consolidatedBillingEnabled && organization.IsValidClient()) + if (organization.IsValidClient()) { var provider = await _providerRepository.GetByOrganizationIdAsync(organization.Id); @@ -477,12 +475,10 @@ public class OrganizationsController : Controller Organization organization, OrganizationEditModel update) { - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - var scaleMSPOnClientOrganizationUpdate = _featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate); - if (!consolidatedBillingEnabled || !scaleMSPOnClientOrganizationUpdate) + if (!scaleMSPOnClientOrganizationUpdate) { return; } diff --git a/src/Admin/AdminConsole/Controllers/ProvidersController.cs b/src/Admin/AdminConsole/Controllers/ProvidersController.cs index 83e4ce7d5..8a56483a6 100644 --- a/src/Admin/AdminConsole/Controllers/ProvidersController.cs +++ b/src/Admin/AdminConsole/Controllers/ProvidersController.cs @@ -282,9 +282,7 @@ public class ProvidersController : Controller await _providerRepository.ReplaceAsync(provider); await _applicationCacheService.UpsertProviderAbilityAsync(provider); - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (!isConsolidatedBillingEnabled || !provider.IsBillable()) + if (!provider.IsBillable()) { return RedirectToAction("Edit", new { id }); } @@ -340,10 +338,7 @@ public class ProvidersController : Controller var users = await _providerUserRepository.GetManyDetailsByProviderAsync(id); var providerOrganizations = await _providerOrganizationRepository.GetManyDetailsByProviderAsync(id); - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - - if (!isConsolidatedBillingEnabled || !provider.IsBillable()) + if (!provider.IsBillable()) { return new ProviderEditModel(provider, users, providerOrganizations, new List()); } diff --git a/src/Admin/AdminConsole/Views/Providers/CreateMsp.cshtml b/src/Admin/AdminConsole/Views/Providers/CreateMsp.cshtml index dde62b58a..d28348196 100644 --- a/src/Admin/AdminConsole/Views/Providers/CreateMsp.cshtml +++ b/src/Admin/AdminConsole/Views/Providers/CreateMsp.cshtml @@ -1,10 +1,5 @@ -@using Bit.Core.AdminConsole.Enums.Provider -@using Bit.Core - @model CreateMspProviderModel -@inject Bit.Core.Services.IFeatureService FeatureService - @{ ViewData["Title"] = "Create Managed Service Provider"; } @@ -17,8 +12,6 @@ - @if (FeatureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling)) - {
@@ -33,7 +26,6 @@
- } diff --git a/src/Admin/AdminConsole/Views/Providers/Edit.cshtml b/src/Admin/AdminConsole/Views/Providers/Edit.cshtml index 53944d0fc..005c498aa 100644 --- a/src/Admin/AdminConsole/Views/Providers/Edit.cshtml +++ b/src/Admin/AdminConsole/Views/Providers/Edit.cshtml @@ -48,7 +48,7 @@ - @if (FeatureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) && Model.Provider.IsBillable()) + @if (Model.Provider.IsBillable()) { switch (Model.Provider.Type) { @@ -68,46 +68,6 @@ -
-
-
-
- - -
-
-
-
-
-
-
- -
- -
- - - -
-
-
-
-
-
- -
- -
- - - -
-
-
-
-
break; } case ProviderType.MultiOrganizationEnterprise: @@ -141,6 +101,46 @@ break; } } +
+
+
+
+ + +
+
+
+
+
+
+
+ +
+ +
+ + + +
+
+
+
+
+
+ +
+ +
+ + + +
+
+
+
+
} @await Html.PartialAsync("Organizations", Model) diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index e134adc04..88734a6c7 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -289,9 +289,7 @@ public class OrganizationsController : Controller throw new BadRequestException(string.Empty, "User verification failed."); } - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (consolidatedBillingEnabled && organization.IsValidClient()) + if (organization.IsValidClient()) { var provider = await _providerRepository.GetByOrganizationIdAsync(organization.Id); @@ -322,8 +320,7 @@ public class OrganizationsController : Controller throw new BadRequestException("Invalid token."); } - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - if (consolidatedBillingEnabled && organization.IsValidClient()) + if (organization.IsValidClient()) { var provider = await _providerRepository.GetByOrganizationIdAsync(organization.Id); if (provider.IsBillable()) diff --git a/src/Api/Billing/Controllers/BaseProviderController.cs b/src/Api/Billing/Controllers/BaseProviderController.cs index ecc9f23a7..038abfaa9 100644 --- a/src/Api/Billing/Controllers/BaseProviderController.cs +++ b/src/Api/Billing/Controllers/BaseProviderController.cs @@ -1,5 +1,4 @@ -using Bit.Core; -using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Extensions; using Bit.Core.Context; @@ -9,7 +8,6 @@ namespace Bit.Api.Billing.Controllers; public abstract class BaseProviderController( ICurrentContext currentContext, - IFeatureService featureService, ILogger logger, IProviderRepository providerRepository, IUserService userService) : BaseBillingController @@ -26,15 +24,6 @@ public abstract class BaseProviderController( Guid providerId, Func checkAuthorization) { - if (!featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling)) - { - logger.LogError( - "Cannot run Consolidated Billing operation for provider ({ProviderID}) while feature flag is disabled", - providerId); - - return (null, Error.NotFound()); - } - var provider = await providerRepository.GetByIdAsync(providerId); if (provider == null) diff --git a/src/Api/Billing/Controllers/ProviderBillingController.cs b/src/Api/Billing/Controllers/ProviderBillingController.cs index d2209685a..f7ddf0853 100644 --- a/src/Api/Billing/Controllers/ProviderBillingController.cs +++ b/src/Api/Billing/Controllers/ProviderBillingController.cs @@ -19,14 +19,13 @@ namespace Bit.Api.Billing.Controllers; [Authorize("Application")] public class ProviderBillingController( ICurrentContext currentContext, - IFeatureService featureService, ILogger logger, IProviderBillingService providerBillingService, IProviderPlanRepository providerPlanRepository, IProviderRepository providerRepository, ISubscriberService subscriberService, IStripeAdapter stripeAdapter, - IUserService userService) : BaseProviderController(currentContext, featureService, logger, providerRepository, userService) + IUserService userService) : BaseProviderController(currentContext, logger, providerRepository, userService) { [HttpGet("invoices")] public async Task GetInvoicesAsync([FromRoute] Guid providerId) diff --git a/src/Api/Billing/Controllers/ProviderClientsController.cs b/src/Api/Billing/Controllers/ProviderClientsController.cs index 700dd4a2e..0c09fa7ba 100644 --- a/src/Api/Billing/Controllers/ProviderClientsController.cs +++ b/src/Api/Billing/Controllers/ProviderClientsController.cs @@ -14,14 +14,13 @@ namespace Bit.Api.Billing.Controllers; [Route("providers/{providerId:guid}/clients")] public class ProviderClientsController( ICurrentContext currentContext, - IFeatureService featureService, ILogger logger, IOrganizationRepository organizationRepository, IProviderBillingService providerBillingService, IProviderOrganizationRepository providerOrganizationRepository, IProviderRepository providerRepository, IProviderService providerService, - IUserService userService) : BaseProviderController(currentContext, featureService, logger, providerRepository, userService) + IUserService userService) : BaseProviderController(currentContext, logger, providerRepository, userService) { [HttpPost] public async Task CreateAsync( diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index f44ce686f..47c79aa13 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -15,6 +15,7 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Extensions; using Bit.Core.Billing.Models.Sales; using Bit.Core.Billing.Services; using Bit.Core.Context; @@ -444,13 +445,6 @@ public class OrganizationService : IOrganizationService public async Task<(Organization organization, OrganizationUser organizationUser, Collection defaultCollection)> SignupClientAsync(OrganizationSignup signup) { - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (!consolidatedBillingEnabled) - { - throw new InvalidOperationException($"{nameof(SignupClientAsync)} is only for use within Consolidated Billing"); - } - var plan = StaticStore.GetPlan(signup.Plan); ValidatePlan(plan, signup.AdditionalSeats, "Password Manager"); @@ -1443,10 +1437,7 @@ public class OrganizationService : IOrganizationService if (provider is { Enabled: true }) { - var consolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - - if (consolidatedBillingEnabled && provider.Type == ProviderType.Msp && - provider.Status == ProviderStatusType.Billable) + if (provider.IsBillable()) { return (false, "Seat limit has been reached. Please contact your provider to add more seats."); } diff --git a/src/Core/Billing/Extensions/BillingExtensions.cs b/src/Core/Billing/Extensions/BillingExtensions.cs index 93afbb971..39b92e95a 100644 --- a/src/Core/Billing/Extensions/BillingExtensions.cs +++ b/src/Core/Billing/Extensions/BillingExtensions.cs @@ -11,10 +11,11 @@ namespace Bit.Core.Billing.Extensions; public static class BillingExtensions { public static bool IsBillable(this Provider provider) => - provider.SupportsConsolidatedBilling() && provider.Status == ProviderStatusType.Billable; - - public static bool SupportsConsolidatedBilling(this Provider provider) - => provider.Type.SupportsConsolidatedBilling(); + provider is + { + Type: ProviderType.Msp or ProviderType.MultiOrganizationEnterprise, + Status: ProviderStatusType.Billable + }; public static bool SupportsConsolidatedBilling(this ProviderType providerType) => providerType is ProviderType.Msp or ProviderType.MultiOrganizationEnterprise; @@ -24,12 +25,15 @@ public static class BillingExtensions { Seats: not null, Status: OrganizationStatusType.Managed, - PlanType: PlanType.TeamsMonthly or PlanType.EnterpriseMonthly + PlanType: PlanType.TeamsMonthly or PlanType.EnterpriseMonthly or PlanType.EnterpriseAnnually }; public static bool IsStripeEnabled(this ISubscriber subscriber) - => !string.IsNullOrEmpty(subscriber.GatewayCustomerId) && - !string.IsNullOrEmpty(subscriber.GatewaySubscriptionId); + => subscriber is + { + GatewayCustomerId: not null and not "", + GatewaySubscriptionId: not null and not "" + }; public static bool IsUnverifiedBankAccount(this SetupIntent setupIntent) => setupIntent is diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index aed90a893..3e40eb7a1 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -107,7 +107,6 @@ public static class FeatureFlagKeys public const string ItemShare = "item-share"; public const string DuoRedirect = "duo-redirect"; public const string AC2101UpdateTrialInitiationEmail = "AC-2101-update-trial-initiation-email"; - public const string EnableConsolidatedBilling = "enable-consolidated-billing"; public const string AC1795_UpdatedSubscriptionStatusSection = "AC-1795_updated-subscription-status-section"; public const string EmailVerification = "email-verification"; public const string EmailVerificationDisableTimingDelays = "email-verification-disable-timing-delays"; diff --git a/test/Admin.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Admin.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index af98fef03..485126ebb 100644 --- a/test/Admin.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs +++ b/test/Admin.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs @@ -68,7 +68,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Created }; @@ -104,7 +103,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; @@ -147,7 +145,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; @@ -190,7 +187,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; @@ -233,7 +229,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; @@ -278,7 +273,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; @@ -322,7 +316,6 @@ public class OrganizationsControllerTests var featureService = sutProvider.GetDependency(); - featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); featureService.IsEnabled(FeatureFlagKeys.PM14401_ScaleMSPOnClientOrganizationUpdate).Returns(true); var provider = new Provider { Type = ProviderType.Msp, Status = ProviderStatusType.Billable }; diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index 13826888d..27c0f7a7c 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs @@ -218,8 +218,6 @@ public class OrganizationsControllerTests : IDisposable _userService.VerifySecretAsync(user, requestModel.Secret).Returns(true); - _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); - _providerRepository.GetByOrganizationIdAsync(organization.Id).Returns(provider); await _sut.Delete(organizationId.ToString(), requestModel); diff --git a/test/Api.Test/Billing/Controllers/ProviderBillingControllerTests.cs b/test/Api.Test/Billing/Controllers/ProviderBillingControllerTests.cs index 73e3850c8..d46038ae9 100644 --- a/test/Api.Test/Billing/Controllers/ProviderBillingControllerTests.cs +++ b/test/Api.Test/Billing/Controllers/ProviderBillingControllerTests.cs @@ -1,7 +1,6 @@ using Bit.Api.Billing.Controllers; using Bit.Api.Billing.Models.Requests; using Bit.Api.Billing.Models.Responses; -using Bit.Core; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Repositories; @@ -35,27 +34,11 @@ public class ProviderBillingControllerTests { #region GetInvoicesAsync & TryGetBillableProviderForAdminOperations - [Theory, BitAutoData] - public async Task GetInvoicesAsync_FFDisabled_NotFound( - Guid providerId, - SutProvider sutProvider) - { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(false); - - var result = await sutProvider.Sut.GetInvoicesAsync(providerId); - - AssertNotFound(result); - } - [Theory, BitAutoData] public async Task GetInvoicesAsync_NullProvider_NotFound( Guid providerId, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - sutProvider.GetDependency().GetByIdAsync(providerId).ReturnsNull(); var result = await sutProvider.Sut.GetInvoicesAsync(providerId); @@ -68,9 +51,6 @@ public class ProviderBillingControllerTests Provider provider, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); sutProvider.GetDependency().ProviderProviderAdmin(provider.Id) @@ -86,9 +66,6 @@ public class ProviderBillingControllerTests Provider provider, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - provider.Type = ProviderType.Reseller; provider.Status = ProviderStatusType.Created; @@ -229,27 +206,11 @@ public class ProviderBillingControllerTests #region GetSubscriptionAsync & TryGetBillableProviderForServiceUserOperation - [Theory, BitAutoData] - public async Task GetSubscriptionAsync_FFDisabled_NotFound( - Guid providerId, - SutProvider sutProvider) - { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(false); - - var result = await sutProvider.Sut.GetSubscriptionAsync(providerId); - - AssertNotFound(result); - } - [Theory, BitAutoData] public async Task GetSubscriptionAsync_NullProvider_NotFound( Guid providerId, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - sutProvider.GetDependency().GetByIdAsync(providerId).ReturnsNull(); var result = await sutProvider.Sut.GetSubscriptionAsync(providerId); @@ -262,9 +223,6 @@ public class ProviderBillingControllerTests Provider provider, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); sutProvider.GetDependency().ProviderUser(provider.Id) @@ -280,9 +238,6 @@ public class ProviderBillingControllerTests Provider provider, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - provider.Type = ProviderType.Reseller; provider.Status = ProviderStatusType.Created; diff --git a/test/Api.Test/Billing/Utilities.cs b/test/Api.Test/Billing/Utilities.cs index 3f5eee72c..fd79c71ca 100644 --- a/test/Api.Test/Billing/Utilities.cs +++ b/test/Api.Test/Billing/Utilities.cs @@ -1,11 +1,9 @@ using Bit.Api.Billing.Controllers; -using Bit.Core; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Context; using Bit.Core.Models.Api; -using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.HttpResults; @@ -59,9 +57,6 @@ public static class Utilities Provider provider, SutProvider sutProvider) where T : BaseProviderController { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling) - .Returns(true); - provider.Type = ProviderType.Msp; provider.Status = ProviderStatusType.Billable; diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 147162c66..e09293f32 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -420,8 +420,6 @@ public class OrganizationServiceTests OrganizationSignup signup, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling).Returns(true); - signup.Plan = PlanType.TeamsMonthly; var (organization, _, _) = await sutProvider.Sut.SignupClientAsync(signup); From e16cad50b16a8bed011f86c039e6892c4ebfe9cf Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:54:23 -0500 Subject: [PATCH 2/9] Add Teams to SCIM API key generation (#5036) --- src/Api/AdminConsole/Controllers/OrganizationsController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 88734a6c7..0ac750e66 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -354,7 +354,7 @@ public class OrganizationsController : Controller { // Non-enterprise orgs should not be able to create or view an apikey of billing sync/scim key types var plan = StaticStore.GetPlan(organization.PlanType); - if (plan.ProductTier != ProductTierType.Enterprise) + if (plan.ProductTier is not ProductTierType.Enterprise and not ProductTierType.Teams) { throw new NotFoundException(); } From ab5d4738d6a2b08cf8c75db4dd2a1a2c36656fb5 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:58:05 -0800 Subject: [PATCH 3/9] [PM-8107] Remove Duo v2 from server (#4934) refactor(TwoFactorAuthentication): Remove references to old Duo SDK version 2 code and replace them with the Duo SDK version 4 supported library DuoUniversal code. Increased unit test coverage in the Two Factor Authentication code space. We opted to use DI instead of Inheritance for the Duo and OrganizaitonDuo two factor tokens to increase testability, since creating a testing mock of the Duo.Client was non-trivial. Reviewed-by: @JaredSnider-Bitwarden --- .../Auth/Controllers/TwoFactorController.cs | 44 +-- .../Models/Request/TwoFactorRequestModels.cs | 67 ++-- .../TwoFactor/TwoFactorDuoResponseModel.cs | 71 +---- src/Api/Startup.cs | 3 +- src/Core/Auth/Identity/DuoWebTokenProvider.cs | 86 ----- .../OrganizationDuoWebTokenProvider.cs | 76 ----- .../Identity/TemporaryDuoWebV4SDKService.cs | 172 ---------- .../AuthenticatorTokenProvider.cs | 2 +- .../DuoUniversalTokenProvider.cs | 102 ++++++ .../DuoUniversalTokenService.cs | 177 +++++++++++ .../EmailTokenProvider.cs | 2 +- .../EmailTwoFactorTokenProvider.cs | 2 +- .../IOrganizationTwoFactorTokenProvider.cs | 2 +- .../OrganizationDuoUniversalTokenProvider.cs | 81 +++++ .../TwoFactorRememberTokenProvider.cs | 2 +- .../WebAuthnTokenProvider.cs | 2 +- .../YubicoOtpTokenProvider.cs | 6 +- src/Core/Auth/Utilities/DuoApi.cs | 277 ---------------- src/Core/Auth/Utilities/DuoWeb.cs | 240 -------------- .../TwoFactorAuthenticationValidator.cs | 48 +-- .../Utilities/ServiceCollectionExtensions.cs | 7 +- .../Controllers/TwoFactorControllerTests.cs | 295 ++++++++++++++++++ ...ganizationTwoFactorDuoRequestModelTests.cs | 60 ---- ...TwoFactorDuoRequestModelValidationTests.cs | 9 +- .../UserTwoFactorDuoRequestModelTests.cs | 62 ---- ...anizationTwoFactorDuoResponseModelTests.cs | 69 +--- .../UserTwoFactorDuoResponseModelTests.cs | 68 ++-- .../Attributes/BitCustomizeAttribute.cs | 4 +- .../AuthenticationTokenProviderTests.cs | 2 +- .../Auth/Identity/BaseTokenProviderTests.cs | 5 +- ...DuoUniversalTwoFactorTokenProviderTests.cs | 262 ++++++++++++++++ .../EmailTwoFactorTokenProviderTests.cs | 2 +- ...DuoUniversalTwoFactorTokenProviderTests.cs | 289 +++++++++++++++++ .../Services/DuoUniversalTokenServiceTests.cs | 91 ++++++ .../Endpoints/IdentityServerTwoFactorTests.cs | 13 +- .../TwoFactorAuthenticationValidatorTests.cs | 81 +---- 36 files changed, 1412 insertions(+), 1369 deletions(-) delete mode 100644 src/Core/Auth/Identity/DuoWebTokenProvider.cs delete mode 100644 src/Core/Auth/Identity/OrganizationDuoWebTokenProvider.cs delete mode 100644 src/Core/Auth/Identity/TemporaryDuoWebV4SDKService.cs rename src/Core/Auth/Identity/{ => TokenProviders}/AuthenticatorTokenProvider.cs (97%) create mode 100644 src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs create mode 100644 src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenService.cs rename src/Core/Auth/Identity/{ => TokenProviders}/EmailTokenProvider.cs (97%) rename src/Core/Auth/Identity/{ => TokenProviders}/EmailTwoFactorTokenProvider.cs (97%) rename src/Core/Auth/Identity/{ => TokenProviders}/IOrganizationTwoFactorTokenProvider.cs (87%) create mode 100644 src/Core/Auth/Identity/TokenProviders/OrganizationDuoUniversalTokenProvider.cs rename src/Core/Auth/Identity/{ => TokenProviders}/TwoFactorRememberTokenProvider.cs (92%) rename src/Core/Auth/Identity/{ => TokenProviders}/WebAuthnTokenProvider.cs (99%) rename src/Core/Auth/Identity/{ => TokenProviders}/YubicoOtpTokenProvider.cs (93%) delete mode 100644 src/Core/Auth/Utilities/DuoApi.cs delete mode 100644 src/Core/Auth/Utilities/DuoWeb.cs create mode 100644 test/Api.Test/Auth/Controllers/TwoFactorControllerTests.cs create mode 100644 test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs create mode 100644 test/Core.Test/Auth/Identity/OrganizationDuoUniversalTwoFactorTokenProviderTests.cs create mode 100644 test/Core.Test/Auth/Services/DuoUniversalTokenServiceTests.cs diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index f2578fbc6..a00a64313 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -4,15 +4,14 @@ using Bit.Api.Auth.Models.Response.TwoFactor; using Bit.Api.Models.Request; using Bit.Api.Models.Response; using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; using Bit.Core.Auth.Models.Business.Tokenables; -using Bit.Core.Auth.Utilities; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Settings; using Bit.Core.Tokens; using Bit.Core.Utilities; using Fido2NetLib; @@ -29,11 +28,10 @@ public class TwoFactorController : Controller private readonly IUserService _userService; private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationService _organizationService; - private readonly GlobalSettings _globalSettings; private readonly UserManager _userManager; private readonly ICurrentContext _currentContext; private readonly IVerifyAuthRequestCommand _verifyAuthRequestCommand; - private readonly IFeatureService _featureService; + private readonly IDuoUniversalTokenService _duoUniversalTokenService; private readonly IDataProtectorTokenFactory _twoFactorAuthenticatorDataProtector; private readonly IDataProtectorTokenFactory _ssoEmailTwoFactorSessionDataProtector; @@ -41,22 +39,20 @@ public class TwoFactorController : Controller IUserService userService, IOrganizationRepository organizationRepository, IOrganizationService organizationService, - GlobalSettings globalSettings, UserManager userManager, ICurrentContext currentContext, IVerifyAuthRequestCommand verifyAuthRequestCommand, - IFeatureService featureService, + IDuoUniversalTokenService duoUniversalConfigService, IDataProtectorTokenFactory twoFactorAuthenticatorDataProtector, IDataProtectorTokenFactory ssoEmailTwoFactorSessionDataProtector) { _userService = userService; _organizationRepository = organizationRepository; _organizationService = organizationService; - _globalSettings = globalSettings; _userManager = userManager; _currentContext = currentContext; _verifyAuthRequestCommand = verifyAuthRequestCommand; - _featureService = featureService; + _duoUniversalTokenService = duoUniversalConfigService; _twoFactorAuthenticatorDataProtector = twoFactorAuthenticatorDataProtector; _ssoEmailTwoFactorSessionDataProtector = ssoEmailTwoFactorSessionDataProtector; } @@ -184,21 +180,7 @@ public class TwoFactorController : Controller public async Task PutDuo([FromBody] UpdateTwoFactorDuoRequestModel model) { var user = await CheckAsync(model, true); - try - { - // for backwards compatibility - will be removed with PM-8107 - DuoApi duoApi = null; - if (model.ClientId != null && model.ClientSecret != null) - { - duoApi = new DuoApi(model.ClientId, model.ClientSecret, model.Host); - } - else - { - duoApi = new DuoApi(model.IntegrationKey, model.SecretKey, model.Host); - } - await duoApi.JSONApiCall("GET", "/auth/v2/check"); - } - catch (DuoException) + if (!await _duoUniversalTokenService.ValidateDuoConfiguration(model.ClientSecret, model.ClientId, model.Host)) { throw new BadRequestException( "Duo configuration settings are not valid. Please re-check the Duo Admin panel."); @@ -241,21 +223,7 @@ public class TwoFactorController : Controller } var organization = await _organizationRepository.GetByIdAsync(orgIdGuid) ?? throw new NotFoundException(); - try - { - // for backwards compatibility - will be removed with PM-8107 - DuoApi duoApi = null; - if (model.ClientId != null && model.ClientSecret != null) - { - duoApi = new DuoApi(model.ClientId, model.ClientSecret, model.Host); - } - else - { - duoApi = new DuoApi(model.IntegrationKey, model.SecretKey, model.Host); - } - await duoApi.JSONApiCall("GET", "/auth/v2/check"); - } - catch (DuoException) + if (!await _duoUniversalTokenService.ValidateDuoConfiguration(model.ClientId, model.ClientSecret, model.Host)) { throw new BadRequestException( "Duo configuration settings are not valid. Please re-check the Duo Admin panel."); diff --git a/src/Api/Auth/Models/Request/TwoFactorRequestModels.cs b/src/Api/Auth/Models/Request/TwoFactorRequestModels.cs index f2f01a237..357db5ad1 100644 --- a/src/Api/Auth/Models/Request/TwoFactorRequestModels.cs +++ b/src/Api/Auth/Models/Request/TwoFactorRequestModels.cs @@ -2,8 +2,8 @@ using Bit.Api.Auth.Models.Request.Accounts; using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models; -using Bit.Core.Auth.Utilities; using Bit.Core.Entities; using Fido2NetLib; @@ -43,21 +43,16 @@ public class UpdateTwoFactorAuthenticatorRequestModel : SecretVerificationReques public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IValidatableObject { /* - To support both v2 and v4 we need to remove the required annotation from the properties. - todo - the required annotation will be added back in PM-8107. + String lengths based on Duo's documentation + https://github.com/duosecurity/duo_universal_csharp/blob/main/DuoUniversal/Client.cs */ - [StringLength(50)] - public string ClientId { get; set; } - [StringLength(50)] - public string ClientSecret { get; set; } - //todo - will remove SKey and IKey with PM-8107 - [StringLength(50)] - public string IntegrationKey { get; set; } - //todo - will remove SKey and IKey with PM-8107 - [StringLength(50)] - public string SecretKey { get; set; } [Required] - [StringLength(50)] + [StringLength(20, MinimumLength = 20, ErrorMessage = "Client Id must be exactly 20 characters.")] + public string ClientId { get; set; } + [Required] + [StringLength(40, MinimumLength = 40, ErrorMessage = "Client Secret must be exactly 40 characters.")] + public string ClientSecret { get; set; } + [Required] public string Host { get; set; } public User ToUser(User existingUser) @@ -65,22 +60,17 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV var providers = existingUser.GetTwoFactorProviders(); if (providers == null) { - providers = new Dictionary(); + providers = []; } else if (providers.ContainsKey(TwoFactorProviderType.Duo)) { providers.Remove(TwoFactorProviderType.Duo); } - Temporary_SyncDuoParams(); - providers.Add(TwoFactorProviderType.Duo, new TwoFactorProvider { MetaData = new Dictionary { - //todo - will remove SKey and IKey with PM-8107 - ["SKey"] = SecretKey, - ["IKey"] = IntegrationKey, ["ClientSecret"] = ClientSecret, ["ClientId"] = ClientId, ["Host"] = Host @@ -96,22 +86,17 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV var providers = existingOrg.GetTwoFactorProviders(); if (providers == null) { - providers = new Dictionary(); + providers = []; } else if (providers.ContainsKey(TwoFactorProviderType.OrganizationDuo)) { providers.Remove(TwoFactorProviderType.OrganizationDuo); } - Temporary_SyncDuoParams(); - providers.Add(TwoFactorProviderType.OrganizationDuo, new TwoFactorProvider { MetaData = new Dictionary { - //todo - will remove SKey and IKey with PM-8107 - ["SKey"] = SecretKey, - ["IKey"] = IntegrationKey, ["ClientSecret"] = ClientSecret, ["ClientId"] = ClientId, ["Host"] = Host @@ -124,34 +109,22 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV public override IEnumerable Validate(ValidationContext validationContext) { - if (!DuoApi.ValidHost(Host)) + var results = new List(); + if (string.IsNullOrWhiteSpace(ClientId)) { - yield return new ValidationResult("Host is invalid.", [nameof(Host)]); + results.Add(new ValidationResult("ClientId is required.", [nameof(ClientId)])); } - if (string.IsNullOrWhiteSpace(ClientSecret) && string.IsNullOrWhiteSpace(ClientId) && - string.IsNullOrWhiteSpace(SecretKey) && string.IsNullOrWhiteSpace(IntegrationKey)) - { - yield return new ValidationResult("Neither v2 or v4 values are valid.", [nameof(IntegrationKey), nameof(SecretKey), nameof(ClientSecret), nameof(ClientId)]); - } - } - /* - use this method to ensure that both v2 params and v4 params are in sync - todo will be removed in pm-8107 - */ - private void Temporary_SyncDuoParams() - { - // Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret - if (!string.IsNullOrWhiteSpace(ClientSecret) && !string.IsNullOrWhiteSpace(ClientId)) + if (string.IsNullOrWhiteSpace(ClientSecret)) { - SecretKey = ClientSecret; - IntegrationKey = ClientId; + results.Add(new ValidationResult("ClientSecret is required.", [nameof(ClientSecret)])); } - else if (!string.IsNullOrWhiteSpace(SecretKey) && !string.IsNullOrWhiteSpace(IntegrationKey)) + + if (string.IsNullOrWhiteSpace(Host) || !DuoUniversalTokenService.ValidDuoHost(Host)) { - ClientSecret = SecretKey; - ClientId = IntegrationKey; + results.Add(new ValidationResult("Host is invalid.", [nameof(Host)])); } + return results; } } diff --git a/src/Api/Auth/Models/Response/TwoFactor/TwoFactorDuoResponseModel.cs b/src/Api/Auth/Models/Response/TwoFactor/TwoFactorDuoResponseModel.cs index 8b8c36d2e..79012783a 100644 --- a/src/Api/Auth/Models/Response/TwoFactor/TwoFactorDuoResponseModel.cs +++ b/src/Api/Auth/Models/Response/TwoFactor/TwoFactorDuoResponseModel.cs @@ -13,37 +13,26 @@ public class TwoFactorDuoResponseModel : ResponseModel public TwoFactorDuoResponseModel(User user) : base(ResponseObj) { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } + ArgumentNullException.ThrowIfNull(user); var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); Build(provider); } - public TwoFactorDuoResponseModel(Organization org) + public TwoFactorDuoResponseModel(Organization organization) : base(ResponseObj) { - if (org == null) - { - throw new ArgumentNullException(nameof(org)); - } + ArgumentNullException.ThrowIfNull(organization); - var provider = org.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); + var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); Build(provider); } public bool Enabled { get; set; } public string Host { get; set; } - //TODO - will remove SecretKey with PM-8107 - public string SecretKey { get; set; } - //TODO - will remove IntegrationKey with PM-8107 - public string IntegrationKey { get; set; } public string ClientSecret { get; set; } public string ClientId { get; set; } - // updated build to assist in the EDD migration for the Duo 2FA provider private void Build(TwoFactorProvider provider) { if (provider?.MetaData != null && provider.MetaData.Count > 0) @@ -54,36 +43,13 @@ public class TwoFactorDuoResponseModel : ResponseModel { Host = (string)host; } - - //todo - will remove SKey and IKey with PM-8107 - // check Skey and IKey first if they exist - if (provider.MetaData.TryGetValue("SKey", out var sKey)) - { - ClientSecret = MaskKey((string)sKey); - SecretKey = MaskKey((string)sKey); - } - if (provider.MetaData.TryGetValue("IKey", out var iKey)) - { - IntegrationKey = (string)iKey; - ClientId = (string)iKey; - } - - // Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret if (provider.MetaData.TryGetValue("ClientSecret", out var clientSecret)) { - if (!string.IsNullOrWhiteSpace((string)clientSecret)) - { - ClientSecret = MaskKey((string)clientSecret); - SecretKey = MaskKey((string)clientSecret); - } + ClientSecret = MaskSecret((string)clientSecret); } if (provider.MetaData.TryGetValue("ClientId", out var clientId)) { - if (!string.IsNullOrWhiteSpace((string)clientId)) - { - ClientId = (string)clientId; - IntegrationKey = (string)clientId; - } + ClientId = (string)clientId; } } else @@ -92,30 +58,7 @@ public class TwoFactorDuoResponseModel : ResponseModel } } - /* - use this method to ensure that both v2 params and v4 params are in sync - todo will be removed in pm-8107 - */ - private void Temporary_SyncDuoParams() - { - // Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret - if (!string.IsNullOrWhiteSpace(ClientSecret) && !string.IsNullOrWhiteSpace(ClientId)) - { - SecretKey = ClientSecret; - IntegrationKey = ClientId; - } - else if (!string.IsNullOrWhiteSpace(SecretKey) && !string.IsNullOrWhiteSpace(IntegrationKey)) - { - ClientSecret = SecretKey; - ClientId = IntegrationKey; - } - else - { - throw new InvalidDataException("Invalid Duo parameters."); - } - } - - private static string MaskKey(string key) + private static string MaskSecret(string key) { if (string.IsNullOrWhiteSpace(key) || key.Length <= 6) { diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index 7962215a4..65935440c 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -23,7 +23,6 @@ using Microsoft.OpenApi.Models; using Bit.SharedWeb.Utilities; using Microsoft.AspNetCore.Diagnostics.HealthChecks; using Microsoft.Extensions.DependencyInjection.Extensions; -using Bit.Core.Auth.Identity; using Bit.Core.Auth.UserFeatures; using Bit.Core.Entities; using Bit.Core.Billing.Extensions; @@ -32,10 +31,10 @@ using Bit.Core.Tools.Entities; using Bit.Core.Vault.Entities; using Bit.Api.Auth.Models.Request.WebAuthn; using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Tools.ReportFeatures; - #if !OSS using Bit.Commercial.Core.SecretsManager; using Bit.Commercial.Core.Utilities; diff --git a/src/Core/Auth/Identity/DuoWebTokenProvider.cs b/src/Core/Auth/Identity/DuoWebTokenProvider.cs deleted file mode 100644 index 6ab020326..000000000 --- a/src/Core/Auth/Identity/DuoWebTokenProvider.cs +++ /dev/null @@ -1,86 +0,0 @@ -using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Models; -using Bit.Core.Auth.Utilities.Duo; -using Bit.Core.Entities; -using Bit.Core.Services; -using Bit.Core.Settings; -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.DependencyInjection; - -namespace Bit.Core.Auth.Identity; - -public class DuoWebTokenProvider : IUserTwoFactorTokenProvider -{ - private readonly IServiceProvider _serviceProvider; - private readonly GlobalSettings _globalSettings; - - public DuoWebTokenProvider( - IServiceProvider serviceProvider, - GlobalSettings globalSettings) - { - _serviceProvider = serviceProvider; - _globalSettings = globalSettings; - } - - public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) - { - var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) - { - return false; - } - - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); - if (!HasProperMetaData(provider)) - { - return false; - } - - return await userService.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Duo, user); - } - - public async Task GenerateAsync(string purpose, UserManager manager, User user) - { - var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) - { - return null; - } - - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); - if (!HasProperMetaData(provider)) - { - return null; - } - - var signatureRequest = DuoWeb.SignRequest((string)provider.MetaData["IKey"], - (string)provider.MetaData["SKey"], _globalSettings.Duo.AKey, user.Email); - return signatureRequest; - } - - public async Task ValidateAsync(string purpose, string token, UserManager manager, User user) - { - var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) - { - return false; - } - - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); - if (!HasProperMetaData(provider)) - { - return false; - } - - var response = DuoWeb.VerifyResponse((string)provider.MetaData["IKey"], (string)provider.MetaData["SKey"], - _globalSettings.Duo.AKey, token); - - return response == user.Email; - } - - private bool HasProperMetaData(TwoFactorProvider provider) - { - return provider?.MetaData != null && provider.MetaData.ContainsKey("IKey") && - provider.MetaData.ContainsKey("SKey") && provider.MetaData.ContainsKey("Host"); - } -} diff --git a/src/Core/Auth/Identity/OrganizationDuoWebTokenProvider.cs b/src/Core/Auth/Identity/OrganizationDuoWebTokenProvider.cs deleted file mode 100644 index 58bcf5efd..000000000 --- a/src/Core/Auth/Identity/OrganizationDuoWebTokenProvider.cs +++ /dev/null @@ -1,76 +0,0 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Models; -using Bit.Core.Auth.Utilities.Duo; -using Bit.Core.Entities; -using Bit.Core.Settings; - -namespace Bit.Core.Auth.Identity; - -public interface IOrganizationDuoWebTokenProvider : IOrganizationTwoFactorTokenProvider { } - -public class OrganizationDuoWebTokenProvider : IOrganizationDuoWebTokenProvider -{ - private readonly GlobalSettings _globalSettings; - - public OrganizationDuoWebTokenProvider(GlobalSettings globalSettings) - { - _globalSettings = globalSettings; - } - - public Task CanGenerateTwoFactorTokenAsync(Organization organization) - { - if (organization == null || !organization.Enabled || !organization.Use2fa) - { - return Task.FromResult(false); - } - - var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); - var canGenerate = organization.TwoFactorProviderIsEnabled(TwoFactorProviderType.OrganizationDuo) - && HasProperMetaData(provider); - return Task.FromResult(canGenerate); - } - - public Task GenerateAsync(Organization organization, User user) - { - if (organization == null || !organization.Enabled || !organization.Use2fa) - { - return Task.FromResult(null); - } - - var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); - if (!HasProperMetaData(provider)) - { - return Task.FromResult(null); - } - - var signatureRequest = DuoWeb.SignRequest(provider.MetaData["IKey"].ToString(), - provider.MetaData["SKey"].ToString(), _globalSettings.Duo.AKey, user.Email); - return Task.FromResult(signatureRequest); - } - - public Task ValidateAsync(string token, Organization organization, User user) - { - if (organization == null || !organization.Enabled || !organization.Use2fa) - { - return Task.FromResult(false); - } - - var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); - if (!HasProperMetaData(provider)) - { - return Task.FromResult(false); - } - - var response = DuoWeb.VerifyResponse(provider.MetaData["IKey"].ToString(), - provider.MetaData["SKey"].ToString(), _globalSettings.Duo.AKey, token); - - return Task.FromResult(response == user.Email); - } - - private bool HasProperMetaData(TwoFactorProvider provider) - { - return provider?.MetaData != null && provider.MetaData.ContainsKey("IKey") && - provider.MetaData.ContainsKey("SKey") && provider.MetaData.ContainsKey("Host"); - } -} diff --git a/src/Core/Auth/Identity/TemporaryDuoWebV4SDKService.cs b/src/Core/Auth/Identity/TemporaryDuoWebV4SDKService.cs deleted file mode 100644 index f78abdfd1..000000000 --- a/src/Core/Auth/Identity/TemporaryDuoWebV4SDKService.cs +++ /dev/null @@ -1,172 +0,0 @@ -using Bit.Core.Auth.Models; -using Bit.Core.Auth.Models.Business.Tokenables; -using Bit.Core.Context; -using Bit.Core.Entities; -using Bit.Core.Settings; -using Bit.Core.Tokens; -using Microsoft.Extensions.Logging; -using Duo = DuoUniversal; - -namespace Bit.Core.Auth.Identity; - -/* - PM-5156 addresses tech debt - Interface to allow for DI, will end up being removed as part of the removal of the old Duo SDK v2 flows. - This service is to support SDK v4 flows for Duo. At some time in the future we will need - to combine this service with the DuoWebTokenProvider and OrganizationDuoWebTokenProvider to support SDK v4. -*/ -public interface ITemporaryDuoWebV4SDKService -{ - Task GenerateAsync(TwoFactorProvider provider, User user); - Task ValidateAsync(string token, TwoFactorProvider provider, User user); -} - -public class TemporaryDuoWebV4SDKService : ITemporaryDuoWebV4SDKService -{ - private readonly ICurrentContext _currentContext; - private readonly GlobalSettings _globalSettings; - private readonly IDataProtectorTokenFactory _tokenDataFactory; - private readonly ILogger _logger; - - /// - /// Constructor for the DuoUniversalPromptService. Used to supplement v2 implementation of Duo with v4 SDK - /// - /// used to fetch initiating Client - /// used to fetch vault URL for Redirect URL - public TemporaryDuoWebV4SDKService( - ICurrentContext currentContext, - GlobalSettings globalSettings, - IDataProtectorTokenFactory tokenDataFactory, - ILogger logger) - { - _currentContext = currentContext; - _globalSettings = globalSettings; - _tokenDataFactory = tokenDataFactory; - _logger = logger; - } - - /// - /// Provider agnostic (either Duo or OrganizationDuo) method to generate a Duo Auth URL - /// - /// Either Duo or OrganizationDuo - /// self - /// AuthUrl for DUO SDK v4 - public async Task GenerateAsync(TwoFactorProvider provider, User user) - { - if (!HasProperMetaData(provider)) - { - if (!HasProperMetaData_SDKV2(provider)) - { - return null; - } - } - - - var duoClient = await BuildDuoClientAsync(provider); - if (duoClient == null) - { - return null; - } - - var state = _tokenDataFactory.Protect(new DuoUserStateTokenable(user)); - var authUrl = duoClient.GenerateAuthUri(user.Email, state); - - return authUrl; - } - - /// - /// Validates Duo SDK v4 response - /// - /// response form Duo - /// TwoFactorProviderType Duo or OrganizationDuo - /// self - /// true or false depending on result of verification - public async Task ValidateAsync(string token, TwoFactorProvider provider, User user) - { - if (!HasProperMetaData(provider)) - { - if (!HasProperMetaData_SDKV2(provider)) - { - return false; - } - } - - var duoClient = await BuildDuoClientAsync(provider); - if (duoClient == null) - { - return false; - } - - var parts = token.Split("|"); - var authCode = parts[0]; - var state = parts[1]; - - _tokenDataFactory.TryUnprotect(state, out var tokenable); - if (!tokenable.Valid || !tokenable.TokenIsValid(user)) - { - return false; - } - - // duoClient compares the email from the received IdToken with user.Email to verify a bad actor hasn't used - // their authCode with a victims credentials - var res = await duoClient.ExchangeAuthorizationCodeFor2faResult(authCode, user.Email); - // If the result of the exchange doesn't throw an exception and it's not null, then it's valid - return res.AuthResult.Result == "allow"; - } - - private bool HasProperMetaData(TwoFactorProvider provider) - { - return provider?.MetaData != null && provider.MetaData.ContainsKey("ClientId") && - provider.MetaData.ContainsKey("ClientSecret") && provider.MetaData.ContainsKey("Host"); - } - - /// - /// Checks if the metadata for SDK V2 is present. - /// Transitional method to support Duo during v4 database rename - /// - /// The TwoFactorProvider object to check. - /// True if the provider has the proper metadata; otherwise, false. - private bool HasProperMetaData_SDKV2(TwoFactorProvider provider) - { - if (provider?.MetaData != null && - provider.MetaData.TryGetValue("IKey", out var iKey) && - provider.MetaData.TryGetValue("SKey", out var sKey) && - provider.MetaData.ContainsKey("Host")) - { - provider.MetaData.Add("ClientId", iKey); - provider.MetaData.Add("ClientSecret", sKey); - return true; - } - else - { - return false; - } - } - - /// - /// Generates a Duo.Client object for use with Duo SDK v4. This combines the health check and the client generation - /// - /// TwoFactorProvider Duo or OrganizationDuo - /// Duo.Client object or null - private async Task BuildDuoClientAsync(TwoFactorProvider provider) - { - // Fetch Client name from header value since duo auth can be initiated from multiple clients and we want - // to redirect back to the initiating client - _currentContext.HttpContext.Request.Headers.TryGetValue("Bitwarden-Client-Name", out var bitwardenClientName); - var redirectUri = string.Format("{0}/duo-redirect-connector.html?client={1}", - _globalSettings.BaseServiceUri.Vault, bitwardenClientName.FirstOrDefault() ?? "web"); - - var client = new Duo.ClientBuilder( - (string)provider.MetaData["ClientId"], - (string)provider.MetaData["ClientSecret"], - (string)provider.MetaData["Host"], - redirectUri).Build(); - - if (!await client.DoHealthCheck(true)) - { - _logger.LogError("Unable to connect to Duo. Health check failed."); - return null; - } - return client; - } -} diff --git a/src/Core/Auth/Identity/AuthenticatorTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs similarity index 97% rename from src/Core/Auth/Identity/AuthenticatorTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs index fae2d23b1..9468e4d57 100644 --- a/src/Core/Auth/Identity/AuthenticatorTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs @@ -6,7 +6,7 @@ using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; using OtpNet; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class AuthenticatorTokenProvider : IUserTwoFactorTokenProvider { diff --git a/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs new file mode 100644 index 000000000..21311326c --- /dev/null +++ b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs @@ -0,0 +1,102 @@ +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Services; +using Bit.Core.Tokens; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using Duo = DuoUniversal; + +namespace Bit.Core.Auth.Identity.TokenProviders; + +public class DuoUniversalTokenProvider( + IServiceProvider serviceProvider, + IDataProtectorTokenFactory tokenDataFactory, + IDuoUniversalTokenService duoUniversalTokenService) : IUserTwoFactorTokenProvider +{ + /// + /// We need the IServiceProvider to resolve the IUserService. There is a complex dependency dance + /// occurring between IUserService, which extends the UserManager, and the usage of the + /// UserManager within this class. Trying to resolve the IUserService using the DI pipeline + /// will not allow the server to start and it will hang and give no helpful indication as to the problem. + /// + private readonly IServiceProvider _serviceProvider = serviceProvider; + private readonly IDataProtectorTokenFactory _tokenDataFactory = tokenDataFactory; + private readonly IDuoUniversalTokenService _duoUniversalTokenService = duoUniversalTokenService; + + public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) + { + var userService = _serviceProvider.GetRequiredService(); + var provider = await GetDuoTwoFactorProvider(user, userService); + if (provider == null) + { + return false; + } + return await userService.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Duo, user); + } + + public async Task GenerateAsync(string purpose, UserManager manager, User user) + { + var duoClient = await GetDuoClientAsync(user); + if (duoClient == null) + { + return null; + } + return _duoUniversalTokenService.GenerateAuthUrl(duoClient, _tokenDataFactory, user); + } + + public async Task ValidateAsync(string purpose, string token, UserManager manager, User user) + { + var duoClient = await GetDuoClientAsync(user); + if (duoClient == null) + { + return false; + } + return await _duoUniversalTokenService.RequestDuoValidationAsync(duoClient, _tokenDataFactory, user, token); + } + + /// + /// Get the Duo Two Factor Provider for the user if they have access to Duo + /// + /// Active User + /// null or Duo TwoFactorProvider + private async Task GetDuoTwoFactorProvider(User user, IUserService userService) + { + if (!await userService.CanAccessPremium(user)) + { + return null; + } + + var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); + if (!_duoUniversalTokenService.HasProperDuoMetadata(provider)) + { + return null; + } + + return provider; + } + + /// + /// Uses the User to fetch a valid TwoFactorProvider and use it to create a Duo.Client + /// + /// active user + /// null or Duo TwoFactorProvider + private async Task GetDuoClientAsync(User user) + { + var userService = _serviceProvider.GetRequiredService(); + var provider = await GetDuoTwoFactorProvider(user, userService); + if (provider == null) + { + return null; + } + + var duoClient = await _duoUniversalTokenService.BuildDuoTwoFactorClientAsync(provider); + if (duoClient == null) + { + return null; + } + + return duoClient; + } +} diff --git a/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenService.cs b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenService.cs new file mode 100644 index 000000000..8dd07e7ee --- /dev/null +++ b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenService.cs @@ -0,0 +1,177 @@ +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Settings; +using Bit.Core.Tokens; +using Duo = DuoUniversal; + +namespace Bit.Core.Auth.Identity.TokenProviders; + +/// +/// OrganizationDuo and Duo TwoFactorProviderTypes both use the same flows so both of those Token Providers will +/// have this class injected to utilize these methods +/// +public interface IDuoUniversalTokenService +{ + /// + /// Generates the Duo Auth URL for the user to be redirected to Duo for 2FA. This + /// Auth URL also lets the Duo Service know where to redirect the user back to after + /// the 2FA process is complete. + /// + /// A not null valid Duo.Client + /// This service creates the state token for added security + /// currently active user + /// a URL in string format + string GenerateAuthUrl( + Duo.Client duoClient, + IDataProtectorTokenFactory tokenDataFactory, + User user); + + /// + /// Makes the request to Duo to validate the authCode and state token + /// + /// A not null valid Duo.Client + /// Factory for decrypting the state + /// self + /// token received from the client + /// boolean based on result from Duo + Task RequestDuoValidationAsync( + Duo.Client duoClient, + IDataProtectorTokenFactory tokenDataFactory, + User user, + string token); + + /// + /// Generates a Duo.Client object for use with Duo SDK v4. This method is to validate a Duo configuration + /// when adding or updating the configuration. This method makes a web request to Duo to verify the configuration. + /// Throws exception if configuration is invalid. + /// + /// Duo client Secret + /// Duo client Id + /// Duo host + /// Boolean + Task ValidateDuoConfiguration(string clientSecret, string clientId, string host); + + /// + /// Checks provider for the correct Duo metadata: ClientId, ClientSecret, and Host. Does no validation on the data. + /// it is assumed to be correct. The only way to have the data written to the Database is after verification + /// occurs. + /// + /// Host being checked for proper data + /// true if all three are present; false if one is missing or the host is incorrect + bool HasProperDuoMetadata(TwoFactorProvider provider); + + /// + /// Generates a Duo.Client object for use with Duo SDK v4. This combines the health check and the client generation. + /// This method is made public so that it is easier to test. If the method was private then there would not be an + /// easy way to mock the response. Since this makes a web request it is difficult to mock. + /// + /// TwoFactorProvider Duo or OrganizationDuo + /// Duo.Client object or null + Task BuildDuoTwoFactorClientAsync(TwoFactorProvider provider); +} + +public class DuoUniversalTokenService( + ICurrentContext currentContext, + GlobalSettings globalSettings) : IDuoUniversalTokenService +{ + private readonly ICurrentContext _currentContext = currentContext; + private readonly GlobalSettings _globalSettings = globalSettings; + + public string GenerateAuthUrl( + Duo.Client duoClient, + IDataProtectorTokenFactory tokenDataFactory, + User user) + { + var state = tokenDataFactory.Protect(new DuoUserStateTokenable(user)); + var authUrl = duoClient.GenerateAuthUri(user.Email, state); + + return authUrl; + } + + public async Task RequestDuoValidationAsync( + Duo.Client duoClient, + IDataProtectorTokenFactory tokenDataFactory, + User user, + string token) + { + var parts = token.Split("|"); + var authCode = parts[0]; + var state = parts[1]; + tokenDataFactory.TryUnprotect(state, out var tokenable); + if (!tokenable.Valid || !tokenable.TokenIsValid(user)) + { + return false; + } + + // duoClient compares the email from the received IdToken with user.Email to verify a bad actor hasn't used + // their authCode with a victims credentials + var res = await duoClient.ExchangeAuthorizationCodeFor2faResult(authCode, user.Email); + // If the result of the exchange doesn't throw an exception and it's not null, then it's valid + return res.AuthResult.Result == "allow"; + } + + public async Task ValidateDuoConfiguration(string clientSecret, string clientId, string host) + { + // Do some simple checks to ensure data integrity + if (!ValidDuoHost(host) || + string.IsNullOrWhiteSpace(clientSecret) || + string.IsNullOrWhiteSpace(clientId)) + { + return false; + } + // The AuthURI is not important for this health check so we pass in a non-empty string + var client = new Duo.ClientBuilder(clientId, clientSecret, host, "non-empty").Build(); + + // This could throw an exception, the false flag will allow the exception to bubble up + return await client.DoHealthCheck(false); + } + + public bool HasProperDuoMetadata(TwoFactorProvider provider) + { + return provider?.MetaData != null && + provider.MetaData.ContainsKey("ClientId") && + provider.MetaData.ContainsKey("ClientSecret") && + provider.MetaData.ContainsKey("Host") && + ValidDuoHost((string)provider.MetaData["Host"]); + } + + + /// + /// Checks the host string to make sure it meets Duo's Guidelines before attempting to create a Duo.Client. + /// + /// string representing the Duo Host + /// true if the host is valid false otherwise + public static bool ValidDuoHost(string host) + { + if (Uri.TryCreate($"https://{host}", UriKind.Absolute, out var uri)) + { + return (string.IsNullOrWhiteSpace(uri.PathAndQuery) || uri.PathAndQuery == "/") && + uri.Host.StartsWith("api-") && + (uri.Host.EndsWith(".duosecurity.com") || uri.Host.EndsWith(".duofederal.com")); + } + return false; + } + + public async Task BuildDuoTwoFactorClientAsync(TwoFactorProvider provider) + { + // Fetch Client name from header value since duo auth can be initiated from multiple clients and we want + // to redirect back to the initiating client + _currentContext.HttpContext.Request.Headers.TryGetValue("Bitwarden-Client-Name", out var bitwardenClientName); + var redirectUri = string.Format("{0}/duo-redirect-connector.html?client={1}", + _globalSettings.BaseServiceUri.Vault, bitwardenClientName.FirstOrDefault() ?? "web"); + + var client = new Duo.ClientBuilder( + (string)provider.MetaData["ClientId"], + (string)provider.MetaData["ClientSecret"], + (string)provider.MetaData["Host"], + redirectUri).Build(); + + if (!await client.DoHealthCheck(false)) + { + return null; + } + return client; + } +} diff --git a/src/Core/Auth/Identity/EmailTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/EmailTokenProvider.cs similarity index 97% rename from src/Core/Auth/Identity/EmailTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/EmailTokenProvider.cs index 1db9e13ee..be94124c0 100644 --- a/src/Core/Auth/Identity/EmailTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/EmailTokenProvider.cs @@ -5,7 +5,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class EmailTokenProvider : IUserTwoFactorTokenProvider { diff --git a/src/Core/Auth/Identity/EmailTwoFactorTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs similarity index 97% rename from src/Core/Auth/Identity/EmailTwoFactorTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs index 607d86a13..b0ad9bd48 100644 --- a/src/Core/Auth/Identity/EmailTwoFactorTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs @@ -6,7 +6,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class EmailTwoFactorTokenProvider : EmailTokenProvider { diff --git a/src/Core/Auth/Identity/IOrganizationTwoFactorTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/IOrganizationTwoFactorTokenProvider.cs similarity index 87% rename from src/Core/Auth/Identity/IOrganizationTwoFactorTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/IOrganizationTwoFactorTokenProvider.cs index 4226cd036..7e2a8c2ac 100644 --- a/src/Core/Auth/Identity/IOrganizationTwoFactorTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/IOrganizationTwoFactorTokenProvider.cs @@ -1,7 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.Entities; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public interface IOrganizationTwoFactorTokenProvider { diff --git a/src/Core/Auth/Identity/TokenProviders/OrganizationDuoUniversalTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/OrganizationDuoUniversalTokenProvider.cs new file mode 100644 index 000000000..c8007dd6e --- /dev/null +++ b/src/Core/Auth/Identity/TokenProviders/OrganizationDuoUniversalTokenProvider.cs @@ -0,0 +1,81 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Duo = DuoUniversal; + +namespace Bit.Core.Auth.Identity.TokenProviders; + +public interface IOrganizationDuoUniversalTokenProvider : IOrganizationTwoFactorTokenProvider { } + +public class OrganizationDuoUniversalTokenProvider( + IDataProtectorTokenFactory tokenDataFactory, + IDuoUniversalTokenService duoUniversalTokenService) : IOrganizationDuoUniversalTokenProvider +{ + private readonly IDataProtectorTokenFactory _tokenDataFactory = tokenDataFactory; + private readonly IDuoUniversalTokenService _duoUniversalTokenService = duoUniversalTokenService; + + public Task CanGenerateTwoFactorTokenAsync(Organization organization) + { + var provider = GetDuoTwoFactorProvider(organization); + if (provider != null && provider.Enabled) + { + return Task.FromResult(true); + } + return Task.FromResult(false); + } + + public async Task GenerateAsync(Organization organization, User user) + { + var duoClient = await GetDuoClientAsync(organization); + if (duoClient == null) + { + return null; + } + return _duoUniversalTokenService.GenerateAuthUrl(duoClient, _tokenDataFactory, user); + } + + public async Task ValidateAsync(string token, Organization organization, User user) + { + var duoClient = await GetDuoClientAsync(organization); + if (duoClient == null) + { + return false; + } + return await _duoUniversalTokenService.RequestDuoValidationAsync(duoClient, _tokenDataFactory, user, token); + } + + private TwoFactorProvider GetDuoTwoFactorProvider(Organization organization) + { + if (organization == null || !organization.Enabled || !organization.Use2fa) + { + return null; + } + + var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); + if (!_duoUniversalTokenService.HasProperDuoMetadata(provider)) + { + return null; + } + return provider; + } + + private async Task GetDuoClientAsync(Organization organization) + { + var provider = GetDuoTwoFactorProvider(organization); + if (provider == null) + { + return null; + } + + var duoClient = await _duoUniversalTokenService.BuildDuoTwoFactorClientAsync(provider); + if (duoClient == null) + { + return null; + } + + return duoClient; + } +} diff --git a/src/Core/Auth/Identity/TwoFactorRememberTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/TwoFactorRememberTokenProvider.cs similarity index 92% rename from src/Core/Auth/Identity/TwoFactorRememberTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/TwoFactorRememberTokenProvider.cs index cf35eb2eb..44b5d48b8 100644 --- a/src/Core/Auth/Identity/TwoFactorRememberTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/TwoFactorRememberTokenProvider.cs @@ -4,7 +4,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class TwoFactorRememberTokenProvider : DataProtectorTokenProvider { diff --git a/src/Core/Auth/Identity/WebAuthnTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs similarity index 99% rename from src/Core/Auth/Identity/WebAuthnTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs index a3b4aebea..202ba3a38 100644 --- a/src/Core/Auth/Identity/WebAuthnTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs @@ -10,7 +10,7 @@ using Fido2NetLib.Objects; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider { diff --git a/src/Core/Auth/Identity/YubicoOtpTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs similarity index 93% rename from src/Core/Auth/Identity/YubicoOtpTokenProvider.cs rename to src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs index 75785118b..9794a51ae 100644 --- a/src/Core/Auth/Identity/YubicoOtpTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs @@ -6,7 +6,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using YubicoDotNetClient; -namespace Bit.Core.Auth.Identity; +namespace Bit.Core.Auth.Identity.TokenProviders; public class YubicoOtpTokenProvider : IUserTwoFactorTokenProvider { @@ -24,7 +24,7 @@ public class YubicoOtpTokenProvider : IUserTwoFactorTokenProvider public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) + if (!await userService.CanAccessPremium(user)) { return false; } @@ -46,7 +46,7 @@ public class YubicoOtpTokenProvider : IUserTwoFactorTokenProvider public async Task ValidateAsync(string purpose, string token, UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) + if (!await userService.CanAccessPremium(user)) { return false; } diff --git a/src/Core/Auth/Utilities/DuoApi.cs b/src/Core/Auth/Utilities/DuoApi.cs deleted file mode 100644 index 8bf5f16a9..000000000 --- a/src/Core/Auth/Utilities/DuoApi.cs +++ /dev/null @@ -1,277 +0,0 @@ -/* -Original source modified from https://github.com/duosecurity/duo_api_csharp - -============================================================================= -============================================================================= - -Copyright (c) 2018 Duo Security -All rights reserved -*/ - -using System.Globalization; -using System.Net; -using System.Security.Cryptography; -using System.Text; -using System.Text.Json; -using System.Text.RegularExpressions; -using System.Web; -using Bit.Core.Models.Api.Response.Duo; - -namespace Bit.Core.Auth.Utilities; - -public class DuoApi -{ - private const string UrlScheme = "https"; - private const string UserAgent = "Bitwarden_DuoAPICSharp/1.0 (.NET Core)"; - - private readonly string _host; - private readonly string _ikey; - private readonly string _skey; - - private readonly HttpClient _httpClient = new(); - - public DuoApi(string ikey, string skey, string host) - { - _ikey = ikey; - _skey = skey; - _host = host; - - if (!ValidHost(host)) - { - throw new DuoException("Invalid Duo host configured.", new ArgumentException(nameof(host))); - } - } - - public static bool ValidHost(string host) - { - if (Uri.TryCreate($"https://{host}", UriKind.Absolute, out var uri)) - { - return (string.IsNullOrWhiteSpace(uri.PathAndQuery) || uri.PathAndQuery == "/") && - uri.Host.StartsWith("api-") && - (uri.Host.EndsWith(".duosecurity.com") || uri.Host.EndsWith(".duofederal.com")); - } - return false; - } - - public static string CanonicalizeParams(Dictionary parameters) - { - var ret = new List(); - foreach (var pair in parameters) - { - var p = string.Format("{0}={1}", HttpUtility.UrlEncode(pair.Key), HttpUtility.UrlEncode(pair.Value)); - // Signatures require upper-case hex digits. - p = Regex.Replace(p, "(%[0-9A-Fa-f][0-9A-Fa-f])", c => c.Value.ToUpperInvariant()); - // Escape only the expected characters. - p = Regex.Replace(p, "([!'()*])", c => "%" + Convert.ToByte(c.Value[0]).ToString("X")); - p = p.Replace("%7E", "~"); - // UrlEncode converts space (" ") to "+". The - // signature algorithm requires "%20" instead. Actual - // + has already been replaced with %2B. - p = p.Replace("+", "%20"); - ret.Add(p); - } - - ret.Sort(StringComparer.Ordinal); - return string.Join("&", ret.ToArray()); - } - - protected string CanonicalizeRequest(string method, string path, string canonParams, string date) - { - string[] lines = { - date, - method.ToUpperInvariant(), - _host.ToLower(), - path, - canonParams, - }; - return string.Join("\n", lines); - } - - public string Sign(string method, string path, string canonParams, string date) - { - var canon = CanonicalizeRequest(method, path, canonParams, date); - var sig = HmacSign(canon); - var auth = string.Concat(_ikey, ':', sig); - return string.Concat("Basic ", Encode64(auth)); - } - - /// The request timeout, in milliseconds. - /// Specify 0 to use the system-default timeout. Use caution if - /// you choose to specify a custom timeout - some API - /// calls (particularly in the Auth APIs) will not - /// return a response until an out-of-band authentication process - /// has completed. In some cases, this may take as much as a - /// small number of minutes. - private async Task<(string result, HttpStatusCode statusCode)> ApiCall(string method, string path, Dictionary parameters, int timeout) - { - if (parameters == null) - { - parameters = new Dictionary(); - } - - var canonParams = CanonicalizeParams(parameters); - var query = string.Empty; - if (!method.Equals("POST") && !method.Equals("PUT")) - { - if (parameters.Count > 0) - { - query = "?" + canonParams; - } - } - var url = $"{UrlScheme}://{_host}{path}{query}"; - - var dateString = RFC822UtcNow(); - var auth = Sign(method, path, canonParams, dateString); - - var request = new HttpRequestMessage - { - Method = new HttpMethod(method), - RequestUri = new Uri(url), - }; - request.Headers.Add("Authorization", auth); - request.Headers.Add("X-Duo-Date", dateString); - request.Headers.UserAgent.ParseAdd(UserAgent); - - if (timeout > 0) - { - _httpClient.Timeout = TimeSpan.FromMilliseconds(timeout); - } - - if (method.Equals("POST") || method.Equals("PUT")) - { - request.Content = new StringContent(canonParams, Encoding.UTF8, "application/x-www-form-urlencoded"); - } - - var response = await _httpClient.SendAsync(request); - var result = await response.Content.ReadAsStringAsync(); - var statusCode = response.StatusCode; - return (result, statusCode); - } - - public async Task JSONApiCall(string method, string path, Dictionary parameters = null) - { - return await JSONApiCall(method, path, parameters, 0); - } - - /// The request timeout, in milliseconds. - /// Specify 0 to use the system-default timeout. Use caution if - /// you choose to specify a custom timeout - some API - /// calls (particularly in the Auth APIs) will not - /// return a response until an out-of-band authentication process - /// has completed. In some cases, this may take as much as a - /// small number of minutes. - private async Task JSONApiCall(string method, string path, Dictionary parameters, int timeout) - { - var (res, statusCode) = await ApiCall(method, path, parameters, timeout); - try - { - var obj = JsonSerializer.Deserialize(res); - if (obj.Stat == "OK") - { - return obj.Response; - } - - throw new ApiException(obj.Code ?? 0, (int)statusCode, obj.Message, obj.MessageDetail); - } - catch (ApiException) - { - throw; - } - catch (Exception e) - { - throw new BadResponseException((int)statusCode, e); - } - } - - private int? ToNullableInt(string s) - { - int i; - if (int.TryParse(s, out i)) - { - return i; - } - return null; - } - - private string HmacSign(string data) - { - var keyBytes = Encoding.ASCII.GetBytes(_skey); - var dataBytes = Encoding.ASCII.GetBytes(data); - - using (var hmac = new HMACSHA1(keyBytes)) - { - var hash = hmac.ComputeHash(dataBytes); - var hex = BitConverter.ToString(hash); - return hex.Replace("-", string.Empty).ToLower(); - } - } - - private static string Encode64(string plaintext) - { - var plaintextBytes = Encoding.ASCII.GetBytes(plaintext); - return Convert.ToBase64String(plaintextBytes); - } - - private static string RFC822UtcNow() - { - // Can't use the "zzzz" format because it adds a ":" - // between the offset's hours and minutes. - var dateString = DateTime.UtcNow.ToString("ddd, dd MMM yyyy HH:mm:ss", CultureInfo.InvariantCulture); - var offset = 0; - var zone = "+" + offset.ToString(CultureInfo.InvariantCulture).PadLeft(2, '0'); - dateString += " " + zone.PadRight(5, '0'); - return dateString; - } -} - -public class DuoException : Exception -{ - public int HttpStatus { get; private set; } - - public DuoException(string message, Exception inner) - : base(message, inner) - { } - - public DuoException(int httpStatus, string message, Exception inner) - : base(message, inner) - { - HttpStatus = httpStatus; - } -} - -public class ApiException : DuoException -{ - public int Code { get; private set; } - public string ApiMessage { get; private set; } - public string ApiMessageDetail { get; private set; } - - public ApiException(int code, int httpStatus, string apiMessage, string apiMessageDetail) - : base(httpStatus, FormatMessage(code, apiMessage, apiMessageDetail), null) - { - Code = code; - ApiMessage = apiMessage; - ApiMessageDetail = apiMessageDetail; - } - - private static string FormatMessage(int code, string apiMessage, string apiMessageDetail) - { - return string.Format("Duo API Error {0}: '{1}' ('{2}')", code, apiMessage, apiMessageDetail); - } -} - -public class BadResponseException : DuoException -{ - public BadResponseException(int httpStatus, Exception inner) - : base(httpStatus, FormatMessage(httpStatus, inner), inner) - { } - - private static string FormatMessage(int httpStatus, Exception inner) - { - var innerMessage = "(null)"; - if (inner != null) - { - innerMessage = string.Format("'{0}'", inner.Message); - } - return string.Format("Got error {0} with HTTP Status {1}", innerMessage, httpStatus); - } -} diff --git a/src/Core/Auth/Utilities/DuoWeb.cs b/src/Core/Auth/Utilities/DuoWeb.cs deleted file mode 100644 index 98fa974ab..000000000 --- a/src/Core/Auth/Utilities/DuoWeb.cs +++ /dev/null @@ -1,240 +0,0 @@ -/* -Original source modified from https://github.com/duosecurity/duo_dotnet - -============================================================================= -============================================================================= - -ref: https://github.com/duosecurity/duo_dotnet/blob/master/LICENSE - -Copyright (c) 2011, Duo Security, Inc. -All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions -are met: - -1. Redistributions of source code must retain the above copyright - notice, this list of conditions and the following disclaimer. -2. Redistributions in binary form must reproduce the above copyright - notice, this list of conditions and the following disclaimer in the - documentation and/or other materials provided with the distribution. -3. The name of the author may not be used to endorse or promote products - derived from this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR -IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES -OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. -IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, -INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT -NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF -THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -*/ - -using System.Security.Cryptography; -using System.Text; - -namespace Bit.Core.Auth.Utilities.Duo; - -public static class DuoWeb -{ - private const string DuoProfix = "TX"; - private const string AppPrefix = "APP"; - private const string AuthPrefix = "AUTH"; - private const int DuoExpire = 300; - private const int AppExpire = 3600; - private const int IKeyLength = 20; - private const int SKeyLength = 40; - private const int AKeyLength = 40; - - public static string ErrorUser = "ERR|The username passed to sign_request() is invalid."; - public static string ErrorIKey = "ERR|The Duo integration key passed to sign_request() is invalid."; - public static string ErrorSKey = "ERR|The Duo secret key passed to sign_request() is invalid."; - public static string ErrorAKey = "ERR|The application secret key passed to sign_request() must be at least " + - "40 characters."; - public static string ErrorUnknown = "ERR|An unknown error has occurred."; - - // throw on invalid bytes - private static Encoding _encoding = new UTF8Encoding(false, true); - private static DateTime _epoc = new DateTime(1970, 1, 1); - - /// - /// Generate a signed request for Duo authentication. - /// The returned value should be passed into the Duo.init() call - /// in the rendered web page used for Duo authentication. - /// - /// Duo integration key - /// Duo secret key - /// Application secret key - /// Primary-authenticated username - /// (optional) The current UTC time - /// signed request - public static string SignRequest(string ikey, string skey, string akey, string username, - DateTime? currentTime = null) - { - string duoSig; - string appSig; - - var currentTimeValue = currentTime ?? DateTime.UtcNow; - - if (username == string.Empty) - { - return ErrorUser; - } - if (username.Contains("|")) - { - return ErrorUser; - } - if (ikey.Length != IKeyLength) - { - return ErrorIKey; - } - if (skey.Length != SKeyLength) - { - return ErrorSKey; - } - if (akey.Length < AKeyLength) - { - return ErrorAKey; - } - - try - { - duoSig = SignVals(skey, username, ikey, DuoProfix, DuoExpire, currentTimeValue); - appSig = SignVals(akey, username, ikey, AppPrefix, AppExpire, currentTimeValue); - } - catch - { - return ErrorUnknown; - } - - return $"{duoSig}:{appSig}"; - } - - /// - /// Validate the signed response returned from Duo. - /// Returns the username of the authenticated user, or null. - /// - /// Duo integration key - /// Duo secret key - /// Application secret key - /// The signed response POST'ed to the server - /// (optional) The current UTC time - /// authenticated username, or null - public static string VerifyResponse(string ikey, string skey, string akey, string sigResponse, - DateTime? currentTime = null) - { - string authUser = null; - string appUser = null; - var currentTimeValue = currentTime ?? DateTime.UtcNow; - - try - { - var sigs = sigResponse.Split(':'); - var authSig = sigs[0]; - var appSig = sigs[1]; - - authUser = ParseVals(skey, authSig, AuthPrefix, ikey, currentTimeValue); - appUser = ParseVals(akey, appSig, AppPrefix, ikey, currentTimeValue); - } - catch - { - return null; - } - - if (authUser != appUser) - { - return null; - } - - return authUser; - } - - private static string SignVals(string key, string username, string ikey, string prefix, long expire, - DateTime currentTime) - { - var ts = (long)(currentTime - _epoc).TotalSeconds; - expire = ts + expire; - var val = $"{username}|{ikey}|{expire.ToString()}"; - var cookie = $"{prefix}|{Encode64(val)}"; - var sig = Sign(key, cookie); - return $"{cookie}|{sig}"; - } - - private static string ParseVals(string key, string val, string prefix, string ikey, DateTime currentTime) - { - var ts = (long)(currentTime - _epoc).TotalSeconds; - - var parts = val.Split('|'); - if (parts.Length != 3) - { - return null; - } - - var uPrefix = parts[0]; - var uB64 = parts[1]; - var uSig = parts[2]; - - var sig = Sign(key, $"{uPrefix}|{uB64}"); - if (Sign(key, sig) != Sign(key, uSig)) - { - return null; - } - - if (uPrefix != prefix) - { - return null; - } - - var cookie = Decode64(uB64); - var cookieParts = cookie.Split('|'); - if (cookieParts.Length != 3) - { - return null; - } - - var username = cookieParts[0]; - var uIKey = cookieParts[1]; - var expire = cookieParts[2]; - - if (uIKey != ikey) - { - return null; - } - - var expireTs = Convert.ToInt32(expire); - if (ts >= expireTs) - { - return null; - } - - return username; - } - - private static string Sign(string skey, string data) - { - var keyBytes = Encoding.ASCII.GetBytes(skey); - var dataBytes = Encoding.ASCII.GetBytes(data); - - using (var hmac = new HMACSHA1(keyBytes)) - { - var hash = hmac.ComputeHash(dataBytes); - var hex = BitConverter.ToString(hash); - return hex.Replace("-", "").ToLower(); - } - } - - private static string Encode64(string plaintext) - { - var plaintextBytes = _encoding.GetBytes(plaintext); - return Convert.ToBase64String(plaintextBytes); - } - - private static string Decode64(string encoded) - { - var plaintextBytes = Convert.FromBase64String(encoded); - return _encoding.GetString(plaintextBytes); - } -} diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index 4cabcf4fa..e2c6406c8 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -1,9 +1,7 @@ - -using System.Text.Json; -using Bit.Core; +using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; @@ -52,8 +50,7 @@ public interface ITwoFactorAuthenticationValidator public class TwoFactorAuthenticationValidator( IUserService userService, UserManager userManager, - IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, - ITemporaryDuoWebV4SDKService duoWebV4SDKService, + IOrganizationDuoUniversalTokenProvider organizationDuoWebTokenProvider, IFeatureService featureService, IApplicationCacheService applicationCacheService, IOrganizationUserRepository organizationUserRepository, @@ -63,8 +60,7 @@ public class TwoFactorAuthenticationValidator( { private readonly IUserService _userService = userService; private readonly UserManager _userManager = userManager; - private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider = organizationDuoWebTokenProvider; - private readonly ITemporaryDuoWebV4SDKService _duoWebV4SDKService = duoWebV4SDKService; + private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider = organizationDuoWebTokenProvider; private readonly IFeatureService _featureService = featureService; private readonly IApplicationCacheService _applicationCacheService = applicationCacheService; private readonly IOrganizationUserRepository _organizationUserRepository = organizationUserRepository; @@ -153,17 +149,7 @@ public class TwoFactorAuthenticationValidator( { if (organization.TwoFactorProviderIsEnabled(type)) { - // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt - if (_featureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) - { - if (!token.Contains(':')) - { - // We have to send the provider to the DuoWebV4SDKService to create the DuoClient - var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); - return await _duoWebV4SDKService.ValidateAsync(token, provider, user); - } - } - return await _organizationDuoWebTokenProvider.ValidateAsync(token, organization, user); + return await _organizationDuoUniversalTokenProvider.ValidateAsync(token, organization, user); } return false; } @@ -181,19 +167,6 @@ public class TwoFactorAuthenticationValidator( { return false; } - // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt - if (_featureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) - { - if (type == TwoFactorProviderType.Duo) - { - if (!token.Contains(':')) - { - // We have to send the provider to the DuoWebV4SDKService to create the DuoClient - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); - return await _duoWebV4SDKService.ValidateAsync(token, provider, user); - } - } - } return await _userManager.VerifyTwoFactorTokenAsync(user, CoreHelpers.CustomProviderName(type), token); default: @@ -248,10 +221,11 @@ public class TwoFactorAuthenticationValidator( in the future the `AuthUrl` will be the generated "token" - PM-8107 */ if (type == TwoFactorProviderType.OrganizationDuo && - await _organizationDuoWebTokenProvider.CanGenerateTwoFactorTokenAsync(organization)) + await _organizationDuoUniversalTokenProvider.CanGenerateTwoFactorTokenAsync(organization)) { twoFactorParams.Add("Host", provider.MetaData["Host"]); - twoFactorParams.Add("AuthUrl", await _duoWebV4SDKService.GenerateAsync(provider, user)); + twoFactorParams.Add("AuthUrl", + await _organizationDuoUniversalTokenProvider.GenerateAsync(organization, user)); return twoFactorParams; } @@ -261,13 +235,9 @@ public class TwoFactorAuthenticationValidator( CoreHelpers.CustomProviderName(type)); switch (type) { - /* - Note: Duo is in the midst of being updated to use the UserManager built-in TwoFactor class - in the future the `AuthUrl` will be the generated "token" - PM-8107 - */ case TwoFactorProviderType.Duo: twoFactorParams.Add("Host", provider.MetaData["Host"]); - twoFactorParams.Add("AuthUrl", await _duoWebV4SDKService.GenerateAsync(provider, user)); + twoFactorParams.Add("AuthUrl", token); break; case TwoFactorProviderType.WebAuthn: if (token != null) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 1b99b4cc8..ab4108bfe 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -10,6 +10,7 @@ using Bit.Core.AdminConsole.Services.Implementations; using Bit.Core.AdminConsole.Services.NoopImplementations; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.IdentityServer; using Bit.Core.Auth.LoginFeatures; using Bit.Core.Auth.Models.Business.Tokenables; @@ -113,6 +114,7 @@ public static class ServiceCollectionExtensions services.AddSingleton(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddLoginServices(); services.AddScoped(); @@ -388,8 +390,7 @@ public static class ServiceCollectionExtensions public static IdentityBuilder AddCustomIdentityServices( this IServiceCollection services, GlobalSettings globalSettings) { - services.AddScoped(); - services.AddScoped(); + services.AddScoped(); services.Configure(options => options.IterationCount = 100000); services.Configure(options => { @@ -430,7 +431,7 @@ public static class ServiceCollectionExtensions CoreHelpers.CustomProviderName(TwoFactorProviderType.Email)) .AddTokenProvider( CoreHelpers.CustomProviderName(TwoFactorProviderType.YubiKey)) - .AddTokenProvider( + .AddTokenProvider( CoreHelpers.CustomProviderName(TwoFactorProviderType.Duo)) .AddTokenProvider( CoreHelpers.CustomProviderName(TwoFactorProviderType.Remember)) diff --git a/test/Api.Test/Auth/Controllers/TwoFactorControllerTests.cs b/test/Api.Test/Auth/Controllers/TwoFactorControllerTests.cs new file mode 100644 index 000000000..11c8288ed --- /dev/null +++ b/test/Api.Test/Auth/Controllers/TwoFactorControllerTests.cs @@ -0,0 +1,295 @@ +using Bit.Api.Auth.Controllers; +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Response.TwoFactor; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Identity.TokenProviders; +using Bit.Core.Context; +using Bit.Core.Entities; +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 Xunit; + +namespace Bit.Api.Test.Auth.Controllers; + +[ControllerCustomize(typeof(TwoFactorController))] +[SutProviderCustomize] +public class TwoFactorControllerTests +{ + [Theory, BitAutoData] + public async Task CheckAsync_UserNull_ThrowsUnauthorizedException(SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(null as User); + + // Act + var result = () => sutProvider.Sut.GetDuo(request); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task CheckAsync_BadSecret_ThrowsBadRequestException(User user, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + + sutProvider.GetDependency() + .VerifySecretAsync(default, default) + .ReturnsForAnyArgs(false); + + // Act + try + { + await sutProvider.Sut.GetDuo(request); + } + catch (BadRequestException e) + { + // Assert + Assert.Equal("The model state is invalid.", e.Message); + } + } + + [Theory, BitAutoData] + public async Task CheckAsync_CannotAccessPremium_ThrowsBadRequestException(User user, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + + sutProvider.GetDependency() + .VerifySecretAsync(default, default) + .ReturnsForAnyArgs(true); + + sutProvider.GetDependency() + .CanAccessPremium(default) + .ReturnsForAnyArgs(false); + + // Act + try + { + await sutProvider.Sut.GetDuo(request); + } + catch (BadRequestException e) + { + // Assert + Assert.Equal("Premium status is required.", e.Message); + } + } + + [Theory, BitAutoData] + public async Task GetDuo_Success(User user, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + user.TwoFactorProviders = GetUserTwoFactorDuoProvidersJson(); + SetupCheckAsyncToPass(sutProvider, user); + + // Act + var result = await sutProvider.Sut.GetDuo(request); + + // Assert + Assert.NotNull(result); + Assert.IsType(result); + } + + [Theory, BitAutoData] + public async Task PutDuo_InvalidConfiguration_ThrowsBadRequestException(User user, UpdateTwoFactorDuoRequestModel request, SutProvider sutProvider) + { + // Arrange + SetupCheckAsyncToPass(sutProvider, user); + sutProvider.GetDependency() + .ValidateDuoConfiguration(default, default, default) + .Returns(false); + + // Act + try + { + await sutProvider.Sut.PutDuo(request); + } + catch (BadRequestException e) + { + // Assert + Assert.Equal("Duo configuration settings are not valid. Please re-check the Duo Admin panel.", e.Message); + } + } + + [Theory, BitAutoData] + public async Task PutDuo_Success(User user, UpdateTwoFactorDuoRequestModel request, SutProvider sutProvider) + { + // Arrange + user.TwoFactorProviders = GetUserTwoFactorDuoProvidersJson(); + SetupCheckAsyncToPass(sutProvider, user); + + sutProvider.GetDependency() + .ValidateDuoConfiguration(default, default, default) + .ReturnsForAnyArgs(true); + + // Act + var result = await sutProvider.Sut.PutDuo(request); + + // Assert + Assert.NotNull(result); + Assert.IsType(result); + Assert.Equal(user.TwoFactorProviders, request.ToUser(user).TwoFactorProviders); + } + + [Theory, BitAutoData] + public async Task CheckOrganizationAsync_ManagePolicies_ThrowsNotFoundException( + User user, Organization organization, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + organization.TwoFactorProviders = GetOrganizationTwoFactorDuoProvidersJson(); + SetupCheckAsyncToPass(sutProvider, user); + + sutProvider.GetDependency() + .ManagePolicies(default) + .ReturnsForAnyArgs(false); + + // Act + var result = () => sutProvider.Sut.GetOrganizationDuo(organization.Id.ToString(), request); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task CheckOrganizationAsync_GetByIdAsync_ThrowsNotFoundException( + User user, Organization organization, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + organization.TwoFactorProviders = GetOrganizationTwoFactorDuoProvidersJson(); + SetupCheckAsyncToPass(sutProvider, user); + + sutProvider.GetDependency() + .ManagePolicies(default) + .ReturnsForAnyArgs(true); + + sutProvider.GetDependency() + .GetByIdAsync(default) + .ReturnsForAnyArgs(null as Organization); + + // Act + var result = () => sutProvider.Sut.GetOrganizationDuo(organization.Id.ToString(), request); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task GetOrganizationDuo_Success( + User user, Organization organization, SecretVerificationRequestModel request, SutProvider sutProvider) + { + // Arrange + organization.TwoFactorProviders = GetOrganizationTwoFactorDuoProvidersJson(); + SetupCheckAsyncToPass(sutProvider, user); + SetupCheckOrganizationAsyncToPass(sutProvider, organization); + + // Act + var result = await sutProvider.Sut.GetOrganizationDuo(organization.Id.ToString(), request); + + // Assert + Assert.NotNull(result); + Assert.IsType(result); + } + + [Theory, BitAutoData] + public async Task PutOrganizationDuo_InvalidConfiguration_ThrowsBadRequestException( + User user, Organization organization, UpdateTwoFactorDuoRequestModel request, SutProvider sutProvider) + { + // Arrange + SetupCheckAsyncToPass(sutProvider, user); + SetupCheckOrganizationAsyncToPass(sutProvider, organization); + + sutProvider.GetDependency() + .ValidateDuoConfiguration(default, default, default) + .ReturnsForAnyArgs(false); + + // Act + try + { + await sutProvider.Sut.PutOrganizationDuo(organization.Id.ToString(), request); + } + catch (BadRequestException e) + { + // Assert + Assert.Equal("Duo configuration settings are not valid. Please re-check the Duo Admin panel.", e.Message); + } + } + + [Theory, BitAutoData] + public async Task PutOrganizationDuo_Success( + User user, Organization organization, UpdateTwoFactorDuoRequestModel request, SutProvider sutProvider) + { + // Arrange + SetupCheckAsyncToPass(sutProvider, user); + SetupCheckOrganizationAsyncToPass(sutProvider, organization); + organization.TwoFactorProviders = GetUserTwoFactorDuoProvidersJson(); + + sutProvider.GetDependency() + .ValidateDuoConfiguration(default, default, default) + .ReturnsForAnyArgs(true); + + // Act + var result = + await sutProvider.Sut.PutOrganizationDuo(organization.Id.ToString(), request); + + // Assert + Assert.NotNull(result); + Assert.IsType(result); + Assert.Equal(organization.TwoFactorProviders, request.ToOrganization(organization).TwoFactorProviders); + } + + + private string GetUserTwoFactorDuoProvidersJson() + { + return + "{\"2\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } + + private string GetOrganizationTwoFactorDuoProvidersJson() + { + return + "{\"6\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } + + /// + /// Sets up the CheckAsync method to pass. + /// + /// uses bit auto data + /// uses bit auto data + private void SetupCheckAsyncToPass(SutProvider sutProvider, User user) + { + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + + sutProvider.GetDependency() + .VerifySecretAsync(default, default) + .ReturnsForAnyArgs(true); + + sutProvider.GetDependency() + .CanAccessPremium(default) + .ReturnsForAnyArgs(true); + } + + private void SetupCheckOrganizationAsyncToPass(SutProvider sutProvider, Organization organization) + { + sutProvider.GetDependency() + .ManagePolicies(default) + .ReturnsForAnyArgs(true); + + sutProvider.GetDependency() + .GetByIdAsync(default) + .ReturnsForAnyArgs(organization); + } +} diff --git a/test/Api.Test/Auth/Models/Request/OrganizationTwoFactorDuoRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/OrganizationTwoFactorDuoRequestModelTests.cs index 5fbaf8867..361adea53 100644 --- a/test/Api.Test/Auth/Models/Request/OrganizationTwoFactorDuoRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/OrganizationTwoFactorDuoRequestModelTests.cs @@ -18,8 +18,6 @@ public class OrganizationTwoFactorDuoRequestModelTests { ClientId = "clientId", ClientSecret = "clientSecret", - IntegrationKey = "integrationKey", - SecretKey = "secretKey", Host = "example.com" }; @@ -30,8 +28,6 @@ public class OrganizationTwoFactorDuoRequestModelTests Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.OrganizationDuo)); Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientId"]); Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientSecret"]); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["IKey"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["SKey"]); Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["Host"]); Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].Enabled); } @@ -49,8 +45,6 @@ public class OrganizationTwoFactorDuoRequestModelTests { ClientId = "newClientId", ClientSecret = "newClientSecret", - IntegrationKey = "newIntegrationKey", - SecretKey = "newSecretKey", Host = "newExample.com" }; @@ -61,61 +55,7 @@ public class OrganizationTwoFactorDuoRequestModelTests Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.OrganizationDuo)); Assert.Equal("newClientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientId"]); Assert.Equal("newClientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientSecret"]); - Assert.Equal("newClientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["IKey"]); - Assert.Equal("newClientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["SKey"]); Assert.Equal("newExample.com", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["Host"]); Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].Enabled); } - - [Fact] - public void DuoV2ParamsSync_WhenExistingProviderDoesNotExist() - { - // Arrange - var existingOrg = new Organization(); - var model = new UpdateTwoFactorDuoRequestModel - { - IntegrationKey = "integrationKey", - SecretKey = "secretKey", - Host = "example.com" - }; - - // Act - var result = model.ToOrganization(existingOrg); - - // Assert - // IKey and SKey should be the same as ClientId and ClientSecret - Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.OrganizationDuo)); - Assert.Equal("integrationKey", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientId"]); - Assert.Equal("secretKey", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientSecret"]); - Assert.Equal("integrationKey", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["IKey"]); - Assert.Equal("secretKey", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["SKey"]); - Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["Host"]); - Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].Enabled); - } - - [Fact] - public void DuoV4ParamsSync_WhenExistingProviderDoesNotExist() - { - // Arrange - var existingOrg = new Organization(); - var model = new UpdateTwoFactorDuoRequestModel - { - ClientId = "clientId", - ClientSecret = "clientSecret", - Host = "example.com" - }; - - // Act - var result = model.ToOrganization(existingOrg); - - // Assert - // IKey and SKey should be the same as ClientId and ClientSecret - Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.OrganizationDuo)); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientId"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["ClientSecret"]); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["IKey"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["SKey"]); - Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].MetaData["Host"]); - Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.OrganizationDuo].Enabled); - } } diff --git a/test/Api.Test/Auth/Models/Request/TwoFactorDuoRequestModelValidationTests.cs b/test/Api.Test/Auth/Models/Request/TwoFactorDuoRequestModelValidationTests.cs index ab05a94f1..295d7cbb5 100644 --- a/test/Api.Test/Auth/Models/Request/TwoFactorDuoRequestModelValidationTests.cs +++ b/test/Api.Test/Auth/Models/Request/TwoFactorDuoRequestModelValidationTests.cs @@ -39,12 +39,9 @@ public class TwoFactorDuoRequestModelValidationTests var result = model.Validate(new ValidationContext(model)); // Assert - Assert.Single(result); - Assert.Equal("Neither v2 or v4 values are valid.", result.First().ErrorMessage); - Assert.Contains("ClientId", result.First().MemberNames); - Assert.Contains("ClientSecret", result.First().MemberNames); - Assert.Contains("IntegrationKey", result.First().MemberNames); - Assert.Contains("SecretKey", result.First().MemberNames); + Assert.NotEmpty(result); + Assert.True(result.Select(x => x.MemberNames.Contains("ClientId")).Any()); + Assert.True(result.Select(x => x.MemberNames.Contains("ClientSecret")).Any()); } [Fact] diff --git a/test/Api.Test/Auth/Models/Request/UserTwoFactorDuoRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/UserTwoFactorDuoRequestModelTests.cs index 28dfc83a2..56c9af1e0 100644 --- a/test/Api.Test/Auth/Models/Request/UserTwoFactorDuoRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/UserTwoFactorDuoRequestModelTests.cs @@ -17,8 +17,6 @@ public class UserTwoFactorDuoRequestModelTests { ClientId = "clientId", ClientSecret = "clientSecret", - IntegrationKey = "integrationKey", - SecretKey = "secretKey", Host = "example.com" }; @@ -26,12 +24,9 @@ public class UserTwoFactorDuoRequestModelTests var result = model.ToUser(existingUser); // Assert - // IKey and SKey should be the same as ClientId and ClientSecret Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.Duo)); Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientId"]); Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientSecret"]); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["IKey"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["SKey"]); Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["Host"]); Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].Enabled); } @@ -49,8 +44,6 @@ public class UserTwoFactorDuoRequestModelTests { ClientId = "newClientId", ClientSecret = "newClientSecret", - IntegrationKey = "newIntegrationKey", - SecretKey = "newSecretKey", Host = "newExample.com" }; @@ -58,65 +51,10 @@ public class UserTwoFactorDuoRequestModelTests var result = model.ToUser(existingUser); // Assert - // IKey and SKey should be the same as ClientId and ClientSecret Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.Duo)); Assert.Equal("newClientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientId"]); Assert.Equal("newClientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientSecret"]); - Assert.Equal("newClientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["IKey"]); - Assert.Equal("newClientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["SKey"]); Assert.Equal("newExample.com", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["Host"]); Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].Enabled); } - - [Fact] - public void DuoV2ParamsSync_WhenExistingProviderDoesNotExist() - { - // Arrange - var existingUser = new User(); - var model = new UpdateTwoFactorDuoRequestModel - { - IntegrationKey = "integrationKey", - SecretKey = "secretKey", - Host = "example.com" - }; - - // Act - var result = model.ToUser(existingUser); - - // Assert - // IKey and SKey should be the same as ClientId and ClientSecret - Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.Duo)); - Assert.Equal("integrationKey", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientId"]); - Assert.Equal("secretKey", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientSecret"]); - Assert.Equal("integrationKey", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["IKey"]); - Assert.Equal("secretKey", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["SKey"]); - Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["Host"]); - Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].Enabled); - } - - [Fact] - public void DuoV4ParamsSync_WhenExistingProviderDoesNotExist() - { - // Arrange - var existingUser = new User(); - var model = new UpdateTwoFactorDuoRequestModel - { - ClientId = "clientId", - ClientSecret = "clientSecret", - Host = "example.com" - }; - - // Act - var result = model.ToUser(existingUser); - - // Assert - // IKey and SKey should be the same as ClientId and ClientSecret - Assert.True(result.GetTwoFactorProviders().ContainsKey(TwoFactorProviderType.Duo)); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientId"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["ClientSecret"]); - Assert.Equal("clientId", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["IKey"]); - Assert.Equal("clientSecret", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["SKey"]); - Assert.Equal("example.com", result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].MetaData["Host"]); - Assert.True(result.GetTwoFactorProviders()[TwoFactorProviderType.Duo].Enabled); - } } diff --git a/test/Api.Test/Auth/Models/Response/OrganizationTwoFactorDuoResponseModelTests.cs b/test/Api.Test/Auth/Models/Response/OrganizationTwoFactorDuoResponseModelTests.cs index dea76b2cd..dfe27467d 100644 --- a/test/Api.Test/Auth/Models/Response/OrganizationTwoFactorDuoResponseModelTests.cs +++ b/test/Api.Test/Auth/Models/Response/OrganizationTwoFactorDuoResponseModelTests.cs @@ -8,42 +8,6 @@ namespace Bit.Api.Test.Auth.Models.Response; public class OrganizationTwoFactorDuoResponseModelTests { - [Theory] - [BitAutoData] - public void Organization_WithDuoV4_ShouldBuildModel(Organization organization) - { - // Arrange - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoV4ProvidersJson(); - - // Act - var model = new TwoFactorDuoResponseModel(organization); - - // Assert if v4 data Ikey and Skey are set to clientId and clientSecret - Assert.NotNull(model); - Assert.Equal("clientId", model.ClientId); - Assert.Equal("secret************", model.ClientSecret); - Assert.Equal("clientId", model.IntegrationKey); - Assert.Equal("secret************", model.SecretKey); - } - - [Theory] - [BitAutoData] - public void Organization_WithDuoV2_ShouldBuildModel(Organization organization) - { - // Arrange - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoV2ProvidersJson(); - - // Act - var model = new TwoFactorDuoResponseModel(organization); - - // Assert if only v2 data clientId and clientSecret are set to Ikey and Sk - Assert.NotNull(model); - Assert.Equal("IKey", model.ClientId); - Assert.Equal("SKey", model.ClientSecret); - Assert.Equal("IKey", model.IntegrationKey); - Assert.Equal("SKey", model.SecretKey); - } - [Theory] [BitAutoData] public void Organization_WithDuo_ShouldBuildModel(Organization organization) @@ -54,12 +18,10 @@ public class OrganizationTwoFactorDuoResponseModelTests // Act var model = new TwoFactorDuoResponseModel(organization); - /// Assert Even if both versions are present priority is given to v4 data + // Assert Assert.NotNull(model); Assert.Equal("clientId", model.ClientId); Assert.Equal("secret************", model.ClientSecret); - Assert.Equal("clientId", model.IntegrationKey); - Assert.Equal("secret************", model.SecretKey); } [Theory] @@ -72,38 +34,33 @@ public class OrganizationTwoFactorDuoResponseModelTests // Act var model = new TwoFactorDuoResponseModel(organization); - /// Assert + // Assert Assert.False(model.Enabled); } [Theory] [BitAutoData] - public void Organization_WithTwoFactorProvidersNull_ShouldFail(Organization organization) + public void Organization_WithTwoFactorProvidersNull_ShouldThrow(Organization organization) { // Arrange - organization.TwoFactorProviders = "{\"6\" : {}}"; + organization.TwoFactorProviders = null; // Act - var model = new TwoFactorDuoResponseModel(organization); + try + { + var model = new TwoFactorDuoResponseModel(organization); - /// Assert - Assert.False(model.Enabled); + } + catch (Exception ex) + { + // Assert + Assert.IsType(ex); + } } private string GetTwoFactorOrganizationDuoProvidersJson() - { - return - "{\"6\":{\"Enabled\":true,\"MetaData\":{\"SKey\":\"SKey\",\"IKey\":\"IKey\",\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; - } - - private string GetTwoFactorOrganizationDuoV4ProvidersJson() { return "{\"6\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; } - - private string GetTwoFactorOrganizationDuoV2ProvidersJson() - { - return "{\"6\":{\"Enabled\":true,\"MetaData\":{\"SKey\":\"SKey\",\"IKey\":\"IKey\",\"Host\":\"example.com\"}}}"; - } } diff --git a/test/Api.Test/Auth/Models/Response/UserTwoFactorDuoResponseModelTests.cs b/test/Api.Test/Auth/Models/Response/UserTwoFactorDuoResponseModelTests.cs index cb46273a6..2441f104b 100644 --- a/test/Api.Test/Auth/Models/Response/UserTwoFactorDuoResponseModelTests.cs +++ b/test/Api.Test/Auth/Models/Response/UserTwoFactorDuoResponseModelTests.cs @@ -10,38 +10,21 @@ public class UserTwoFactorDuoResponseModelTests { [Theory] [BitAutoData] - public void User_WithDuoV4_ShouldBuildModel(User user) + public void User_WithDuo_UserNull_ThrowsArgumentException(User user) { // Arrange - user.TwoFactorProviders = GetTwoFactorDuoV4ProvidersJson(); + user.TwoFactorProviders = GetTwoFactorDuoProvidersJson(); // Act - var model = new TwoFactorDuoResponseModel(user); - - // Assert if v4 data Ikey and Skey are set to clientId and clientSecret - Assert.NotNull(model); - Assert.Equal("clientId", model.ClientId); - Assert.Equal("secret************", model.ClientSecret); - Assert.Equal("clientId", model.IntegrationKey); - Assert.Equal("secret************", model.SecretKey); - } - - [Theory] - [BitAutoData] - public void User_WithDuov2_ShouldBuildModel(User user) - { - // Arrange - user.TwoFactorProviders = GetTwoFactorDuoV2ProvidersJson(); - - // Act - var model = new TwoFactorDuoResponseModel(user); - - // Assert if only v2 data clientId and clientSecret are set to Ikey and Skey - Assert.NotNull(model); - Assert.Equal("IKey", model.ClientId); - Assert.Equal("SKey", model.ClientSecret); - Assert.Equal("IKey", model.IntegrationKey); - Assert.Equal("SKey", model.SecretKey); + try + { + var model = new TwoFactorDuoResponseModel(null as User); + } + catch (ArgumentNullException e) + { + // Assert + Assert.Equal("Value cannot be null. (Parameter 'user')", e.Message); + } } [Theory] @@ -54,12 +37,10 @@ public class UserTwoFactorDuoResponseModelTests // Act var model = new TwoFactorDuoResponseModel(user); - // Assert Even if both versions are present priority is given to v4 data + // Assert Assert.NotNull(model); Assert.Equal("clientId", model.ClientId); Assert.Equal("secret************", model.ClientSecret); - Assert.Equal("clientId", model.IntegrationKey); - Assert.Equal("secret************", model.SecretKey); } [Theory] @@ -84,26 +65,23 @@ public class UserTwoFactorDuoResponseModelTests user.TwoFactorProviders = null; // Act - var model = new TwoFactorDuoResponseModel(user); + try + { + var model = new TwoFactorDuoResponseModel(user); + + } + catch (Exception ex) + { + // Assert + Assert.IsType(ex); + + } - /// Assert - Assert.False(model.Enabled); } private string GetTwoFactorDuoProvidersJson() - { - return - "{\"2\":{\"Enabled\":true,\"MetaData\":{\"SKey\":\"SKey\",\"IKey\":\"IKey\",\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; - } - - private string GetTwoFactorDuoV4ProvidersJson() { return "{\"2\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; } - - private string GetTwoFactorDuoV2ProvidersJson() - { - return "{\"2\":{\"Enabled\":true,\"MetaData\":{\"SKey\":\"SKey\",\"IKey\":\"IKey\",\"Host\":\"example.com\"}}}"; - } } diff --git a/test/Common/AutoFixture/Attributes/BitCustomizeAttribute.cs b/test/Common/AutoFixture/Attributes/BitCustomizeAttribute.cs index 105a6632d..e8a88c684 100644 --- a/test/Common/AutoFixture/Attributes/BitCustomizeAttribute.cs +++ b/test/Common/AutoFixture/Attributes/BitCustomizeAttribute.cs @@ -13,8 +13,8 @@ namespace Bit.Test.Common.AutoFixture.Attributes; public abstract class BitCustomizeAttribute : Attribute { /// - /// /// Gets a customization for the method's parameters. + /// Gets a customization for the method's parameters. /// - /// A customization for the method's paramters. + /// A customization for the method's parameters. public abstract ICustomization GetCustomization(); } diff --git a/test/Core.Test/Auth/Identity/AuthenticationTokenProviderTests.cs b/test/Core.Test/Auth/Identity/AuthenticationTokenProviderTests.cs index a76216f3f..c9646e627 100644 --- a/test/Core.Test/Auth/Identity/AuthenticationTokenProviderTests.cs +++ b/test/Core.Test/Auth/Identity/AuthenticationTokenProviderTests.cs @@ -1,5 +1,5 @@ using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Entities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; diff --git a/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs b/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs index b90f71ae7..da2d4a282 100644 --- a/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs +++ b/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs @@ -19,7 +19,6 @@ public abstract class BaseTokenProviderTests { public abstract TwoFactorProviderType TwoFactorProviderType { get; } - #region Helpers protected static IEnumerable SetupCanGenerateData(params (Dictionary MetaData, bool ExpectedResponse)[] data) { return data.Select(d => @@ -48,6 +47,9 @@ public abstract class BaseTokenProviderTests userService .TwoFactorProviderIsEnabledAsync(TwoFactorProviderType, user) .Returns(true); + userService + .CanAccessPremium(user) + .Returns(true); } protected static UserManager SubstituteUserManager() @@ -76,7 +78,6 @@ public abstract class BaseTokenProviderTests user.TwoFactorProviders = JsonHelpers.LegacySerialize(providers); } - #endregion public virtual async Task RunCanGenerateTwoFactorTokenAsync(Dictionary metaData, bool expectedResponse, User user, SutProvider sutProvider) diff --git a/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs b/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs new file mode 100644 index 000000000..85c687119 --- /dev/null +++ b/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs @@ -0,0 +1,262 @@ +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Identity.TokenProviders; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; +using Duo = DuoUniversal; + +namespace Bit.Core.Test.Auth.Identity; + +public class DuoUniversalTwoFactorTokenProviderTests : BaseTokenProviderTests +{ + private readonly IDuoUniversalTokenService _duoUniversalTokenService = Substitute.For(); + public override TwoFactorProviderType TwoFactorProviderType => TwoFactorProviderType.Duo; + + public static IEnumerable CanGenerateTwoFactorTokenAsyncData + => SetupCanGenerateData( + ( // correct data + new Dictionary + { + ["ClientId"] = new string('c', 20), + ["ClientSecret"] = new string('s', 40), + ["Host"] = "https://api-abcd1234.duosecurity.com", + }, + true + ), + ( // correct data duo federal + new Dictionary + { + ["ClientId"] = new string('c', 20), + ["ClientSecret"] = new string('s', 40), + ["Host"] = "https://api-abcd1234.duofederal.com", + }, + true + ), + ( // correct data duo federal + new Dictionary + { + ["ClientId"] = new string('c', 20), + ["ClientSecret"] = new string('s', 40), + ["Host"] = "https://api-abcd1234.duofederal.com", + }, + true + ), + ( // invalid host + new Dictionary + { + ["ClientId"] = new string('c', 20), + ["ClientSecret"] = new string('s', 40), + ["Host"] = "", + }, + false + ), + ( // clientId missing + new Dictionary + { + ["ClientSecret"] = new string('s', 40), + ["Host"] = "https://api-abcd1234.duofederal.com", + }, + false + ) + ); + + public static IEnumerable NonPremiumCanGenerateTwoFactorTokenAsyncData + => SetupCanGenerateData( + ( // correct data + new Dictionary + { + ["ClientId"] = new string('c', 20), + ["ClientSecret"] = new string('s', 40), + ["Host"] = "https://api-abcd1234.duosecurity.com", + }, + false + ) + ); + + [Theory, BitMemberAutoData(nameof(CanGenerateTwoFactorTokenAsyncData))] + public override async Task RunCanGenerateTwoFactorTokenAsync(Dictionary metaData, bool expectedResponse, + User user, SutProvider sutProvider) + { + // Arrange + user.Premium = true; + user.PremiumExpirationDate = DateTime.UtcNow.AddDays(1); + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(expectedResponse); + + // Act + // Assert + await base.RunCanGenerateTwoFactorTokenAsync(metaData, expectedResponse, user, sutProvider); + } + + [Theory, BitMemberAutoData(nameof(NonPremiumCanGenerateTwoFactorTokenAsyncData))] + public async Task CanGenerateTwoFactorTokenAsync_UserCanNotAccessPremium_ReturnsNull(Dictionary metaData, bool expectedResponse, + User user, SutProvider sutProvider) + { + // Arrange + user.Premium = false; + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(expectedResponse); + + // Act + // Assert + await base.RunCanGenerateTwoFactorTokenAsync(metaData, expectedResponse, user, sutProvider); + } + + [Theory] + [BitAutoData] + public async Task GenerateToken_Success_ReturnsAuthUrl( + User user, SutProvider sutProvider, string authUrl) + { + // Arrange + SetUpProperDuoUniversalTokenService(user, sutProvider); + + sutProvider.GetDependency() + .GenerateAuthUrl( + Arg.Any(), + Arg.Any>(), + user) + .Returns(authUrl); + + // Act + var token = await sutProvider.Sut.GenerateAsync("purpose", SubstituteUserManager(), user); + + // Assert + Assert.NotNull(token); + Assert.Equal(token, authUrl); + } + + [Theory] + [BitAutoData] + public async Task GenerateToken_DuoClientNull_ReturnsNull( + User user, SutProvider sutProvider) + { + // Arrange + user.Premium = true; + user.TwoFactorProviders = GetTwoFactorDuoProvidersJson(); + AdditionalSetup(sutProvider, user); + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + + sutProvider.GetDependency() + .BuildDuoTwoFactorClientAsync(Arg.Any()) + .Returns(null as Duo.Client); + + // Act + var token = await sutProvider.Sut.GenerateAsync("purpose", SubstituteUserManager(), user); + + // Assert + Assert.Null(token); + } + + [Theory] + [BitAutoData] + public async Task GenerateToken_UserCanNotAccessPremium_ReturnsNull( + User user, SutProvider sutProvider) + { + // Arrange + user.Premium = false; + user.TwoFactorProviders = GetTwoFactorDuoProvidersJson(); + AdditionalSetup(sutProvider, user); + + // Act + var token = await sutProvider.Sut.GenerateAsync("purpose", SubstituteUserManager(), user); + + // Assert + Assert.Null(token); + } + + [Theory] + [BitAutoData] + public async Task ValidateToken_ValidToken_ReturnsTrue( + User user, SutProvider sutProvider, string token) + { + // Arrange + SetUpProperDuoUniversalTokenService(user, sutProvider); + + sutProvider.GetDependency() + .RequestDuoValidationAsync( + Arg.Any(), + Arg.Any>(), + user, + token) + .Returns(true); + + // Act + var response = await sutProvider.Sut.ValidateAsync("purpose", token, SubstituteUserManager(), user); + + // Assert + Assert.True(response); + } + + [Theory] + [BitAutoData] + public async Task ValidateToken_DuoClientNull_ReturnsFalse( + User user, SutProvider sutProvider, string token) + { + user.Premium = true; + user.TwoFactorProviders = GetTwoFactorDuoProvidersJson(); + AdditionalSetup(sutProvider, user); + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + + sutProvider.GetDependency() + .BuildDuoTwoFactorClientAsync(Arg.Any()) + .Returns(null as Duo.Client); + + // Act + var result = await sutProvider.Sut.ValidateAsync("purpose", token, SubstituteUserManager(), user); + + // Assert + Assert.False(result); + } + + /// + /// Ensures that the IDuoUniversalTokenService is properly setup for the test. + /// This ensures that the private GetDuoClientAsync, and GetDuoTwoFactorProvider + /// methods will return true enabling the test to execute on the correct path. + /// + /// user from calling test + /// self + private void SetUpProperDuoUniversalTokenService(User user, SutProvider sutProvider) + { + user.Premium = true; + user.TwoFactorProviders = GetTwoFactorDuoProvidersJson(); + var client = BuildDuoClient(); + + AdditionalSetup(sutProvider, user); + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + + sutProvider.GetDependency() + .BuildDuoTwoFactorClientAsync(Arg.Any()) + .Returns(client); + } + + private Duo.Client BuildDuoClient() + { + var clientId = new string('c', 20); + var clientSecret = new string('s', 40); + return new Duo.ClientBuilder(clientId, clientSecret, "api-abcd1234.duosecurity.com", "redirectUrl").Build(); + } + + private string GetTwoFactorDuoProvidersJson() + { + return + "{\"2\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } +} diff --git a/test/Core.Test/Auth/Identity/EmailTwoFactorTokenProviderTests.cs b/test/Core.Test/Auth/Identity/EmailTwoFactorTokenProviderTests.cs index c5855c234..46bfba549 100644 --- a/test/Core.Test/Auth/Identity/EmailTwoFactorTokenProviderTests.cs +++ b/test/Core.Test/Auth/Identity/EmailTwoFactorTokenProviderTests.cs @@ -1,5 +1,5 @@ using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Entities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; diff --git a/test/Core.Test/Auth/Identity/OrganizationDuoUniversalTwoFactorTokenProviderTests.cs b/test/Core.Test/Auth/Identity/OrganizationDuoUniversalTwoFactorTokenProviderTests.cs new file mode 100644 index 000000000..ed67d89f9 --- /dev/null +++ b/test/Core.Test/Auth/Identity/OrganizationDuoUniversalTwoFactorTokenProviderTests.cs @@ -0,0 +1,289 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Identity.TokenProviders; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; +using Duo = DuoUniversal; + +namespace Bit.Core.Test.Auth.Identity; + +[SutProviderCustomize] +public class OrganizationDuoUniversalTwoFactorTokenProviderTests +{ + private readonly IDuoUniversalTokenService _duoUniversalTokenService = Substitute.For(); + private readonly IDataProtectorTokenFactory _tokenDataFactory = Substitute.For>(); + + // Happy path + [Theory] + [BitAutoData] + public async Task CanGenerateTwoFactorTokenAsync_ReturnsTrue( + Organization organization, SutProvider sutProvider) + { + // Arrange + organization.Enabled = true; + organization.Use2fa = true; + SetUpProperOrganizationDuoUniversalTokenService(null, organization, sutProvider); + + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(organization); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData] + public async Task CanGenerateTwoFactorTokenAsync_DuoTwoFactorNotEnabled_ReturnsFalse( + Organization organization, SutProvider sutProvider) + { + // Arrange + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderNotEnabledJson(); + organization.Use2fa = true; + organization.Enabled = true; + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(null); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async Task CanGenerateTwoFactorTokenAsync_BadMetaData_ProviderNull_ReturnsFalse( + Organization organization, SutProvider sutProvider) + { + // Arrange + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Use2fa = true; + organization.Enabled = true; + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(false); + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(null); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async Task GetDuoTwoFactorProvider_OrganizationNull_ReturnsNull( + SutProvider sutProvider) + { + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(null); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async Task GetDuoTwoFactorProvider_OrganizationNotEnabled_ReturnsNull( + Organization organization, SutProvider sutProvider) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(null, organization, sutProvider); + organization.Enabled = false; + + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(organization); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async Task GetDuoTwoFactorProvider_OrganizationUse2FAFalse_ReturnsNull( + Organization organization, SutProvider sutProvider) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(null, organization, sutProvider); + organization.Use2fa = false; + + // Act + var result = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(organization); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async Task GetDuoClient_ProviderNull_ReturnsNull( + SutProvider sutProvider) + { + // Act + var result = await sutProvider.Sut.GenerateAsync(null, default); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async Task GetDuoClient_DuoClientNull_ReturnsNull( + SutProvider sutProvider, + Organization organization) + { + // Arrange + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Use2fa = true; + organization.Enabled = true; + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + + sutProvider.GetDependency() + .BuildDuoTwoFactorClientAsync(Arg.Any()) + .Returns(null as Duo.Client); + + // Act + var result = await sutProvider.Sut.GenerateAsync(organization, default); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async Task GenerateAsync_ReturnsAuthUrl( + SutProvider sutProvider, + Organization organization, + User user, + string AuthUrl) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(BuildDuoClient(), organization, sutProvider); + + sutProvider.GetDependency() + .GenerateAuthUrl(Arg.Any(), Arg.Any>(), user) + .Returns(AuthUrl); + + // Act + var result = await sutProvider.Sut.GenerateAsync(organization, user); + + // Assert + Assert.NotNull(result); + Assert.Equal(AuthUrl, result); + } + + [Theory] + [BitAutoData] + public async Task GenerateAsync_ClientNull_ReturnsNull( + SutProvider sutProvider, + Organization organization, + User user) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(null, organization, sutProvider); + + // Act + var result = await sutProvider.Sut.GenerateAsync(organization, user); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_TokenValid_ReturnsTrue( + SutProvider sutProvider, + Organization organization, + User user, + string token) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(BuildDuoClient(), organization, sutProvider); + + sutProvider.GetDependency() + .RequestDuoValidationAsync(Arg.Any(), Arg.Any>(), user, token) + .Returns(true); + + // Act + var result = await sutProvider.Sut.ValidateAsync(token, organization, user); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_ClientNull_ReturnsFalse( + SutProvider sutProvider, + Organization organization, + User user, + string token) + { + // Arrange + SetUpProperOrganizationDuoUniversalTokenService(null, organization, sutProvider); + + sutProvider.GetDependency() + .RequestDuoValidationAsync(Arg.Any(), Arg.Any>(), user, token) + .Returns(true); + + // Act + var result = await sutProvider.Sut.ValidateAsync(token, organization, user); + + // Assert + Assert.False(result); + } + + /// + /// Ensures that the IDuoUniversalTokenService is properly setup for the test. + /// This ensures that the private GetDuoClientAsync, and GetDuoTwoFactorProvider + /// methods will return true enabling the test to execute on the correct path. + /// + /// BitAutoData cannot create the Duo.Client since it does not have a public constructor + /// so we have to use the ClientBUilder(), the client is not used meaningfully in the tests. + /// + /// user from calling test + /// self + private void SetUpProperOrganizationDuoUniversalTokenService( + Duo.Client client, Organization organization, SutProvider sutProvider) + { + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + organization.Use2fa = true; + + sutProvider.GetDependency() + .HasProperDuoMetadata(Arg.Any()) + .Returns(true); + + sutProvider.GetDependency() + .BuildDuoTwoFactorClientAsync(Arg.Any()) + .Returns(client); + } + + private Duo.Client BuildDuoClient() + { + var clientId = new string('c', 20); + var clientSecret = new string('s', 40); + return new Duo.ClientBuilder(clientId, clientSecret, "api-abcd1234.duosecurity.com", "redirectUrl").Build(); + } + + private string GetTwoFactorOrganizationDuoProviderJson() + { + return + "{\"6\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } + + private string GetTwoFactorOrganizationDuoProviderNotEnabledJson() + { + return + "{\"6\":{\"Enabled\":false,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } +} diff --git a/test/Core.Test/Auth/Services/DuoUniversalTokenServiceTests.cs b/test/Core.Test/Auth/Services/DuoUniversalTokenServiceTests.cs new file mode 100644 index 000000000..4d205dc44 --- /dev/null +++ b/test/Core.Test/Auth/Services/DuoUniversalTokenServiceTests.cs @@ -0,0 +1,91 @@ +using Bit.Core.Auth.Identity.TokenProviders; +using Bit.Core.Auth.Models; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Core.Test.Auth.Services; + +[SutProviderCustomize] +public class DuoUniversalTokenServiceTests +{ + [Theory] + [BitAutoData("", "ClientId", "ClientSecret")] + [BitAutoData("api-valid.duosecurity.com", "", "ClientSecret")] + [BitAutoData("api-valid.duosecurity.com", "ClientId", "")] + public async void ValidateDuoConfiguration_InvalidConfig_ReturnsFalse( + string host, string clientId, string clientSecret, SutProvider sutProvider) + { + // Arrange + /* AutoData handles arrangement */ + + // Act + var result = await sutProvider.Sut.ValidateDuoConfiguration(clientSecret, clientId, host); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData(true, "api-valid.duosecurity.com")] + [BitAutoData(false, "invalid")] + [BitAutoData(false, "api-valid.duosecurity.com", null, "clientSecret")] + [BitAutoData(false, "api-valid.duosecurity.com", "ClientId", null)] + [BitAutoData(false, "api-valid.duosecurity.com", null, null)] + public void HasProperDuoMetadata_ReturnMatchesExpected( + bool expectedResponse, string host, string clientId, + string clientSecret, SutProvider sutProvider) + { + // Arrange + var metaData = new Dictionary { ["Host"] = host }; + + if (clientId != null) + { + metaData.Add("ClientId", clientId); + } + + if (clientSecret != null) + { + metaData.Add("ClientSecret", clientSecret); + } + + var provider = new TwoFactorProvider + { + MetaData = metaData + }; + + // Act + var result = sutProvider.Sut.HasProperDuoMetadata(provider); + + // Assert + Assert.Equal(result, expectedResponse); + } + + [Theory] + [BitAutoData] + public void HasProperDuoMetadata_ProviderIsNull_ReturnsFalse( + SutProvider sutProvider) + { + // Act + var result = sutProvider.Sut.HasProperDuoMetadata(null); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData("api-valid.duosecurity.com", true)] + [BitAutoData("api-valid.duofederal.com", true)] + [BitAutoData("invalid", false)] + public void ValidDuoHost_HostIsValid_ReturnTrue( + string host, bool expectedResponse) + { + // Act + var result = DuoUniversalTokenService.ValidDuoHost(host); + + // Assert + Assert.Equal(result, expectedResponse); + } + + +} diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs index 468ef5a16..4e598c436 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTwoFactorTests.cs @@ -28,7 +28,7 @@ namespace Bit.Identity.IntegrationTest.Endpoints; public class IdentityServerTwoFactorTests : IClassFixture { - const string _organizationTwoFactor = """{"6":{"Enabled":true,"MetaData":{"IKey":"DIEFB13LB49IEB3459N2","SKey":"0ZnsZHav0KcNPBZTS6EOUwqLPoB0sfMd5aJeWExQ","Host":"api-example.duosecurity.com"}}}"""; + const string _organizationTwoFactor = """{"6":{"Enabled":true,"MetaData":{"ClientId":"DIEFB13LB49IEB3459N2","ClientSecret":"0ZnsZHav0KcNPBZTS6EOUwqLPoB0sfMd5aJeWExQ","Host":"api-example.duosecurity.com"}}}"""; const string _testEmail = "test+2farequired@email.com"; const string _testPassword = "master_password_hash"; const string _userEmailTwoFactor = """{"1": { "Enabled": true, "MetaData": { "Email": "test+2farequired@email.com"}}}"""; @@ -140,7 +140,7 @@ public class IdentityServerTwoFactorTests : IClassFixture context.Request.Headers.Append("Auth-Email", CoreHelpers.Base64UrlEncodeString(_testEmail))); - // Assert + // Assert using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); @@ -168,7 +168,7 @@ public class IdentityServerTwoFactorTests : IClassFixture context.Request.Headers.Append("Auth-Email", CoreHelpers.Base64UrlEncodeString(_testEmail))); - // Assert + // Assert using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); @@ -320,7 +320,7 @@ public class IdentityServerTwoFactorTests : IClassFixture context.Request.Headers.Append("Auth-Email", CoreHelpers.Base64UrlEncodeString(_testEmail))); - // Assert + // Assert var body = await AssertHelper.AssertResponseTypeIs(twoFactorProvidedContext); var root = body.RootElement; @@ -338,6 +338,7 @@ public class IdentityServerTwoFactorTests : IClassFixture context.Request.Headers.Append("Auth-Email", CoreHelpers.Base64UrlEncodeString(_testEmail))); - // Assert + // Assert using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; var error = AssertHelper.AssertJsonProperty(root, "error_description", JsonValueKind.String).GetString(); diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index 5783375ff..dfb877b8d 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -1,8 +1,6 @@ -using Bit.Core; -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; -using Bit.Core.Auth.Models; +using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Entities; @@ -28,8 +26,7 @@ public class TwoFactorAuthenticationValidatorTests { private readonly IUserService _userService; private readonly UserManagerTestWrapper _userManager; - private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider; - private readonly ITemporaryDuoWebV4SDKService _temporaryDuoWebV4SDKService; + private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider; private readonly IFeatureService _featureService; private readonly IApplicationCacheService _applicationCacheService; private readonly IOrganizationUserRepository _organizationUserRepository; @@ -42,8 +39,7 @@ public class TwoFactorAuthenticationValidatorTests { _userService = Substitute.For(); _userManager = SubstituteUserManager(); - _organizationDuoWebTokenProvider = Substitute.For(); - _temporaryDuoWebV4SDKService = Substitute.For(); + _organizationDuoUniversalTokenProvider = Substitute.For(); _featureService = Substitute.For(); _applicationCacheService = Substitute.For(); _organizationUserRepository = Substitute.For(); @@ -54,8 +50,7 @@ public class TwoFactorAuthenticationValidatorTests _sut = new TwoFactorAuthenticationValidator( _userService, _userManager, - _organizationDuoWebTokenProvider, - _temporaryDuoWebV4SDKService, + _organizationDuoUniversalTokenProvider, _featureService, _applicationCacheService, _organizationUserRepository, @@ -439,7 +434,7 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _organizationDuoWebTokenProvider.ValidateAsync( + _organizationDuoUniversalTokenProvider.ValidateAsync( token, organization, user).Returns(true); _userManager.TWO_FACTOR_ENABLED = true; @@ -457,70 +452,6 @@ public class TwoFactorAuthenticationValidatorTests Assert.True(result); } - [Theory] - [BitAutoData(TwoFactorProviderType.Duo)] - [BitAutoData(TwoFactorProviderType.OrganizationDuo)] - public async void VerifyTwoFactorAsync_TemporaryDuoService_ValidToken_ReturnsTrue( - TwoFactorProviderType providerType, - User user, - Organization organization, - string token) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.DuoRedirect).Returns(true); - _userService.TwoFactorProviderIsEnabledAsync(providerType, user).Returns(true); - _temporaryDuoWebV4SDKService.ValidateAsync( - token, Arg.Any(), user).Returns(true); - - user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); - organization.Use2fa = true; - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); - organization.Enabled = true; - - _userManager.TWO_FACTOR_ENABLED = true; - _userManager.TWO_FACTOR_TOKEN = token; - _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; - - // Act - var result = await _sut.VerifyTwoFactor( - user, organization, providerType, token); - - // Assert - Assert.True(result); - } - - [Theory] - [BitAutoData(TwoFactorProviderType.Duo)] - [BitAutoData(TwoFactorProviderType.OrganizationDuo)] - public async void VerifyTwoFactorAsync_TemporaryDuoService_InvalidToken_ReturnsFalse( - TwoFactorProviderType providerType, - User user, - Organization organization, - string token) - { - // Arrange - _featureService.IsEnabled(FeatureFlagKeys.DuoRedirect).Returns(true); - _userService.TwoFactorProviderIsEnabledAsync(providerType, user).Returns(true); - _temporaryDuoWebV4SDKService.ValidateAsync( - token, Arg.Any(), user).Returns(true); - - user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); - organization.Use2fa = true; - organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); - organization.Enabled = true; - - _userManager.TWO_FACTOR_ENABLED = true; - _userManager.TWO_FACTOR_TOKEN = token; - _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; - - // Act - var result = await _sut.VerifyTwoFactor( - user, organization, providerType, token); - - // Assert - Assert.True(result); - } - private static UserManagerTestWrapper SubstituteUserManager() { return new UserManagerTestWrapper( From b2b0f1e70ea2a54d183c24684be6b640e1a81b02 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:04:54 +0000 Subject: [PATCH 4/9] [deps] Auth: Update bootstrap to v5 [SECURITY] (#4881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [deps] Auth: Update bootstrap to v5 [SECURITY] * Update bootstrap and import dependencies in site.scss * Update site.scss to include the theme color 'dark' * Refactor site.scss to merge the 'primary-accent' theme color into the existing theme colors * Update bootstrap classes for v5 * Refactor form layout in Index.cshtml and AddExistingOrganization.cshtml * Revert change to the shield icon in the navbar * Fix organization form select inputs * Fixed search input sizes * Fix elements in Providers and Users search * More bootstrap migration * Revert change to tax rate delete button * Add missing label classes in Users/Edit.cshtml * More component migrations * Refactor form classes and labels in CreateMsp.cshtml and CreateReseller.cshtml * Update package dependencies in Sso * Revert changes to Providers/Edit.cshtml * Refactor CreateMultiOrganizationEnterprise.cshtml and Providers/Edit.cshtml for bootstrap 5 * Refactor webpack.config.js to use @popperjs/core instead of popper.js * Remove popperjs package dependency * Restore Bootstrap 4 link styling behavior - Remove default text decoration - Add underline only on hover * Update Bootstrap to version 5.3.3 * Update deprecated text color classes from 'text-muted' to 'text-body-secondary' across various views * Refactor provider edit view for bootstrap 5 * Remove underline in Add/Create organization links in provider page --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Rui Tome Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com> --- .../src/Sso/Views/Shared/_Layout.cshtml | 2 +- bitwarden_license/src/Sso/package-lock.json | 37 +- bitwarden_license/src/Sso/package.json | 5 +- bitwarden_license/src/Sso/webpack.config.js | 2 - .../Views/Organizations/Edit.cshtml | 10 +- .../Views/Organizations/Index.cshtml | 42 +- .../Providers/AddExistingOrganization.cshtml | 18 +- .../Views/Providers/Create.cshtml | 10 +- .../Views/Providers/CreateMsp.cshtml | 14 +- .../CreateMultiOrganizationEnterprise.cshtml | 20 +- .../Views/Providers/CreateOrganization.cshtml | 2 +- .../Views/Providers/CreateReseller.cshtml | 14 +- .../AdminConsole/Views/Providers/Edit.cshtml | 95 +++-- .../AdminConsole/Views/Providers/Index.cshtml | 26 +- .../Views/Providers/Organizations.cshtml | 12 +- .../Views/Shared/_OrganizationForm.cshtml | 106 +++-- src/Admin/Auth/Views/Login/Index.cshtml | 10 +- .../Views/MigrateProviders/Index.cshtml | 16 +- src/Admin/Sass/site.scss | 16 +- src/Admin/Views/Home/Index.cshtml | 8 +- src/Admin/Views/Shared/_Layout.cshtml | 10 +- .../Tools/CreateUpdateTransaction.cshtml | 74 ++-- src/Admin/Views/Tools/GenerateLicense.cshtml | 16 +- src/Admin/Views/Tools/PromoteAdmin.cshtml | 10 +- .../Views/Tools/StripeSubscriptions.cshtml | 370 +++++++++--------- src/Admin/Views/Tools/TaxRate.cshtml | 16 +- src/Admin/Views/Users/Edit.cshtml | 54 ++- src/Admin/Views/Users/Index.cshtml | 22 +- src/Admin/package-lock.json | 35 +- src/Admin/package.json | 3 +- src/Admin/webpack.config.js | 2 - src/Billing/Views/Login/Index.cshtml | 2 +- 32 files changed, 537 insertions(+), 542 deletions(-) diff --git a/bitwarden_license/src/Sso/Views/Shared/_Layout.cshtml b/bitwarden_license/src/Sso/Views/Shared/_Layout.cshtml index fc57d7e9b..b5330dfa9 100644 --- a/bitwarden_license/src/Sso/Views/Shared/_Layout.cshtml +++ b/bitwarden_license/src/Sso/Views/Shared/_Layout.cshtml @@ -23,7 +23,7 @@ @RenderBody() -