From 3f95513d1142d02cdc6a0f36e7a24dd7f25c9f69 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Fri, 9 May 2025 11:39:57 -0400 Subject: [PATCH] [PM-19029][PM-19203] Addressing `UserService` tech debt around `ITwoFactorIsEnabledQuery` (#5754) * fix : split out the interface from the TwoFactorAuthenticationValidator into separate file. * fix: replacing IUserService.TwoFactorEnabled with ITwoFactorEnabledQuery * fix: combined logic for both bulk and single user look ups for TwoFactorIsEnabledQuery. * fix: return two factor provider enabled on CanGenerate() method. * tech debt: modfifying MFA providers to call the database less to validate if two factor is enabled. * tech debt: removed unused service from AuthenticatorTokenProvider * doc: added documentation to ITwoFactorProviderUsers * doc: updated comments for TwoFactorIsEnabled impl * test: fixing tests for ITwoFactorIsEnabledQuery * test: updating tests to have correct DI and removing test for automatic email of TOTP. * test: adding better test coverage --- .../Public/Controllers/MembersController.cs | 4 +- .../Auth/Controllers/AccountsController.cs | 10 +- .../Billing/Controllers/AccountsController.cs | 6 +- src/Api/Vault/Controllers/SyncController.cs | 8 +- .../OrganizationUsers/AcceptOrgUserCommand.cs | 6 +- src/Core/Auth/Enums/TwoFactorProviderType.cs | 3 +- .../AuthenticatorTokenProvider.cs | 15 +-- .../DuoUniversalTokenProvider.cs | 11 +- .../EmailTwoFactorTokenProvider.cs | 17 +-- .../TokenProviders/WebAuthnTokenProvider.cs | 19 ++- .../TokenProviders/YubicoOtpTokenProvider.cs | 8 +- src/Core/Auth/Identity/UserStore.cs | 6 +- .../Auth/Models/ITwoFactorProvidersUser.cs | 8 ++ .../Interfaces/ITwoFactorIsEnabledQuery.cs | 4 +- .../TwoFactorAuth/TwoFactorIsEnabledQuery.cs | 96 +++++++------- src/Core/Entities/User.cs | 45 ++++--- src/Core/Repositories/IUserRepository.cs | 10 ++ src/Core/Services/IUserService.cs | 8 +- .../Services/Implementations/UserService.cs | 48 +------ .../ITwoFactorAuthenticationValidator.cs | 38 ++++++ .../TwoFactorAuthenticationValidator.cs | 44 +------ .../Repositories/UserRepository.cs | 10 +- .../Repositories/UserRepository.cs | 16 ++- .../Controllers/AccountsControllerTests.cs | 4 + .../Vault/Controllers/SyncControllerTests.cs | 19 ++- .../AcceptOrgUserCommandTests.cs | 6 +- .../Auth/Identity/BaseTokenProviderTests.cs | 5 - ...DuoUniversalTwoFactorTokenProviderTests.cs | 3 + .../TwoFactorIsEnabledQueryTests.cs | 120 +++++++++++++++--- test/Core.Test/Services/UserServiceTests.cs | 6 + .../TwoFactorAuthenticationValidatorTests.cs | 28 +--- 31 files changed, 372 insertions(+), 259 deletions(-) create mode 100644 src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs index 92e5071801..6552684ca3 100644 --- a/src/Api/AdminConsole/Public/Controllers/MembersController.cs +++ b/src/Api/AdminConsole/Public/Controllers/MembersController.cs @@ -76,7 +76,7 @@ public class MembersController : Controller { return new NotFoundResult(); } - var response = new MemberResponseModel(orgUser, await _userService.TwoFactorIsEnabledAsync(orgUser), + var response = new MemberResponseModel(orgUser, await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(orgUser), collections); return new JsonResult(response); } @@ -185,7 +185,7 @@ public class MembersController : Controller { var existingUserDetails = await _organizationUserRepository.GetDetailsByIdAsync(id); response = new MemberResponseModel(existingUserDetails, - await _userService.TwoFactorIsEnabledAsync(existingUserDetails), associations); + await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(existingUserDetails), associations); } else { diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 2134a7fc4e..fdd5fbb290 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -16,6 +16,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; @@ -45,6 +46,7 @@ public class AccountsController : Controller private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand; private readonly IRotateUserKeyCommand _rotateUserKeyCommand; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IFeatureService _featureService; private readonly IRotationValidator, IEnumerable> _cipherValidator; @@ -68,6 +70,7 @@ public class AccountsController : Controller ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand, ITdeOffboardingPasswordCommand tdeOffboardingPasswordCommand, IRotateUserKeyCommand rotateUserKeyCommand, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IFeatureService featureService, IRotationValidator, IEnumerable> cipherValidator, IRotationValidator, IEnumerable> folderValidator, @@ -87,6 +90,7 @@ public class AccountsController : Controller _setInitialMasterPasswordCommand = setInitialMasterPasswordCommand; _tdeOffboardingPasswordCommand = tdeOffboardingPasswordCommand; _rotateUserKeyCommand = rotateUserKeyCommand; + _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _featureService = featureService; _cipherValidator = cipherValidator; _folderValidator = folderValidator; @@ -389,7 +393,7 @@ public class AccountsController : Controller await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); - var twoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); + var twoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var hasPremiumFromOrg = await _userService.HasPremiumFromOrganization(user); var organizationIdsClaimingActiveUser = await GetOrganizationIdsClaimingUserAsync(user.Id); @@ -423,7 +427,7 @@ public class AccountsController : Controller await _userService.SaveUserAsync(model.ToUser(user)); - var twoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); + var twoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var hasPremiumFromOrg = await _userService.HasPremiumFromOrganization(user); var organizationIdsClaimingActiveUser = await GetOrganizationIdsClaimingUserAsync(user.Id); @@ -442,7 +446,7 @@ public class AccountsController : Controller } await _userService.SaveUserAsync(model.ToUser(user), true); - var userTwoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); + var userTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var userHasPremiumFromOrganization = await _userService.HasPremiumFromOrganization(user); var organizationIdsClaimingActiveUser = await GetOrganizationIdsClaimingUserAsync(user.Id); diff --git a/src/Api/Billing/Controllers/AccountsController.cs b/src/Api/Billing/Controllers/AccountsController.cs index bc263691a8..49ff679bb8 100644 --- a/src/Api/Billing/Controllers/AccountsController.cs +++ b/src/Api/Billing/Controllers/AccountsController.cs @@ -3,6 +3,7 @@ using Bit.Api.Models.Request; using Bit.Api.Models.Request.Accounts; using Bit.Api.Models.Response; using Bit.Api.Utilities; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Models; using Bit.Core.Billing.Services; using Bit.Core.Context; @@ -22,7 +23,8 @@ namespace Bit.Api.Billing.Controllers; [Route("accounts")] [Authorize("Application")] public class AccountsController( - IUserService userService) : Controller + IUserService userService, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) : Controller { [HttpPost("premium")] public async Task PostPremiumAsync( @@ -56,7 +58,7 @@ public class AccountsController( model.PaymentMethodType!.Value, model.AdditionalStorageGb.GetValueOrDefault(0), license, new TaxInfo { BillingAddressCountry = model.Country, BillingAddressPostalCode = model.PostalCode }); - var userTwoFactorEnabled = await userService.TwoFactorIsEnabledAsync(user); + var userTwoFactorEnabled = await twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var userHasPremiumFromOrganization = await userService.HasPremiumFromOrganization(user); var organizationIdsClaimingActiveUser = await GetOrganizationIdsClaimingUserAsync(user.Id); diff --git a/src/Api/Vault/Controllers/SyncController.cs b/src/Api/Vault/Controllers/SyncController.cs index 4b66c7f2bd..568c05d651 100644 --- a/src/Api/Vault/Controllers/SyncController.cs +++ b/src/Api/Vault/Controllers/SyncController.cs @@ -3,6 +3,7 @@ using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -37,6 +38,7 @@ public class SyncController : Controller private readonly Version _sshKeyCipherMinimumVersion = new(Constants.SSHKeyCipherMinimumVersion); private readonly IFeatureService _featureService; private readonly IApplicationCacheService _applicationCacheService; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; public SyncController( IUserService userService, @@ -51,7 +53,8 @@ public class SyncController : Controller GlobalSettings globalSettings, ICurrentContext currentContext, IFeatureService featureService, - IApplicationCacheService applicationCacheService) + IApplicationCacheService applicationCacheService, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) { _userService = userService; _folderRepository = folderRepository; @@ -66,6 +69,7 @@ public class SyncController : Controller _currentContext = currentContext; _featureService = featureService; _applicationCacheService = applicationCacheService; + _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; } [HttpGet("")] @@ -102,7 +106,7 @@ public class SyncController : Controller collectionCiphersGroupDict = collectionCiphers.GroupBy(c => c.CipherId).ToDictionary(s => s.Key); } - var userTwoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); + var userTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var userHasPremiumFromOrganization = await _userService.HasPremiumFromOrganization(user); var organizationClaimingActiveUser = await _userService.GetOrganizationsClaimingUserAsync(user.Id); var organizationIdsClaimingActiveUser = organizationClaimingActiveUser.Select(o => o.Id); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 756bd2ae46..f3426efddc 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; @@ -24,6 +25,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand private readonly IPolicyService _policyService; private readonly IMailService _mailService; private readonly IUserRepository _userRepository; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public AcceptOrgUserCommand( @@ -34,6 +36,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand IPolicyService policyService, IMailService mailService, IUserRepository userRepository, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { @@ -45,6 +48,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand _policyService = policyService; _mailService = mailService; _userRepository = userRepository; + _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } @@ -192,7 +196,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand } // Enforce Two Factor Authentication Policy of organization user is trying to join - if (!await userService.TwoFactorIsEnabledAsync(user)) + if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) { var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); diff --git a/src/Core/Auth/Enums/TwoFactorProviderType.cs b/src/Core/Auth/Enums/TwoFactorProviderType.cs index 07a52dc429..c3613785bc 100644 --- a/src/Core/Auth/Enums/TwoFactorProviderType.cs +++ b/src/Core/Auth/Enums/TwoFactorProviderType.cs @@ -6,7 +6,8 @@ public enum TwoFactorProviderType : byte Email = 1, Duo = 2, YubiKey = 3, - U2f = 4, // Deprecated + [Obsolete("Deprecated in favor of WebAuthn.")] + U2f = 4, Remember = 5, OrganizationDuo = 6, WebAuthn = 7, diff --git a/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs index 9468e4d571..5a3d9522f3 100644 --- a/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs @@ -1,6 +1,5 @@ using Bit.Core.Auth.Enums; using Bit.Core.Entities; -using Bit.Core.Services; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; @@ -12,16 +11,13 @@ public class AuthenticatorTokenProvider : IUserTwoFactorTokenProvider { private const string CacheKeyFormat = "Authenticator_TOTP_{0}_{1}"; - private readonly IServiceProvider _serviceProvider; private readonly IDistributedCache _distributedCache; private readonly DistributedCacheEntryOptions _distributedCacheEntryOptions; public AuthenticatorTokenProvider( - IServiceProvider serviceProvider, [FromKeyedServices("persistent")] IDistributedCache distributedCache) { - _serviceProvider = serviceProvider; _distributedCache = distributedCache; _distributedCacheEntryOptions = new DistributedCacheEntryOptions { @@ -29,15 +25,14 @@ public class AuthenticatorTokenProvider : IUserTwoFactorTokenProvider }; } - public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) + public Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Authenticator); - if (string.IsNullOrWhiteSpace((string)provider?.MetaData["Key"])) + var authenticatorProvider = user.GetTwoFactorProvider(TwoFactorProviderType.Authenticator); + if (string.IsNullOrWhiteSpace((string)authenticatorProvider?.MetaData["Key"])) { - return false; + return Task.FromResult(false); } - return await _serviceProvider.GetRequiredService() - .TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Authenticator, user); + return Task.FromResult(authenticatorProvider.Enabled); } public Task GenerateAsync(string purpose, UserManager manager, User user) diff --git a/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs index cbb994fa09..3f2a44915c 100644 --- a/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs @@ -17,7 +17,7 @@ public class DuoUniversalTokenProvider( { /// /// We need the IServiceProvider to resolve the . There is a complex dependency dance - /// occurring between , which extends the , and the usage + /// occurring between , which extends the , and the usage /// of the within this class. Trying to resolve the using /// the DI pipeline will not allow the server to start and it will hang and give no helpful indication as to the /// problem. @@ -29,12 +29,13 @@ public class DuoUniversalTokenProvider( public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - var provider = await GetDuoTwoFactorProvider(user, userService); - if (provider == null) + var duoUniversalTokenProvider = await GetDuoTwoFactorProvider(user, userService); + if (duoUniversalTokenProvider == null) { return false; } - return await userService.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Duo, user); + + return duoUniversalTokenProvider.Enabled; } public async Task GenerateAsync(string purpose, UserManager manager, User user) @@ -58,7 +59,7 @@ public class DuoUniversalTokenProvider( } /// - /// Get the Duo Two Factor Provider for the user if they have access to Duo + /// Get the Duo Two Factor Provider for the user if they have premium access to Duo /// /// Active User /// null or Duo TwoFactorProvider diff --git a/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs index b0ad9bd480..718e44ae5f 100644 --- a/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs @@ -1,7 +1,6 @@ using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Entities; -using Bit.Core.Services; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; @@ -10,31 +9,25 @@ namespace Bit.Core.Auth.Identity.TokenProviders; public class EmailTwoFactorTokenProvider : EmailTokenProvider { - private readonly IServiceProvider _serviceProvider; - public EmailTwoFactorTokenProvider( - IServiceProvider serviceProvider, [FromKeyedServices("persistent")] IDistributedCache distributedCache) : base(distributedCache) { - _serviceProvider = serviceProvider; - TokenAlpha = false; TokenNumeric = true; TokenLength = 6; } - public override async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) + public override Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Email); - if (!HasProperMetaData(provider)) + var emailTokenProvider = user.GetTwoFactorProvider(TwoFactorProviderType.Email); + if (!HasProperMetaData(emailTokenProvider)) { - return false; + return Task.FromResult(false); } - return await _serviceProvider.GetRequiredService(). - TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Email, user); + return Task.FromResult(emailTokenProvider.Enabled); } public override Task GenerateAsync(string purpose, UserManager manager, User user) diff --git a/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs index 202ba3a38c..0bf75d0fc3 100644 --- a/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs @@ -25,17 +25,16 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider _globalSettings = globalSettings; } - public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) + public Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { - var userService = _serviceProvider.GetRequiredService(); - var webAuthnProvider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); + // null check happens in this method if (!HasProperMetaData(webAuthnProvider)) { - return false; + return Task.FromResult(false); } - return await userService.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.WebAuthn, user); + return Task.FromResult(webAuthnProvider.Enabled); } public async Task GenerateAsync(string purpose, UserManager manager, User user) @@ -81,7 +80,7 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); var keys = LoadKeys(provider); - if (!provider.MetaData.ContainsKey("login")) + if (!provider.MetaData.TryGetValue("login", out var value)) { return false; } @@ -89,7 +88,7 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider var clientResponse = JsonSerializer.Deserialize(token, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); - var jsonOptions = provider.MetaData["login"].ToString(); + var jsonOptions = value.ToString(); var options = AssertionOptions.FromJson(jsonOptions); var webAuthCred = keys.Find(k => k.Item2.Descriptor.Id.SequenceEqual(clientResponse.Id)); @@ -126,6 +125,12 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider } + /// + /// Checks if the provider has proper metadata. + /// This is used to determine if the provider has been properly configured. + /// + /// + /// true if metadata is present; false if empty or null private bool HasProperMetaData(TwoFactorProvider provider) { return provider?.MetaData?.Any() ?? false; diff --git a/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs b/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs index 9794a51ae9..b33d2fc0c9 100644 --- a/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs +++ b/src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs @@ -23,19 +23,21 @@ public class YubicoOtpTokenProvider : IUserTwoFactorTokenProvider public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { + // Ensure the user has access to premium var userService = _serviceProvider.GetRequiredService(); if (!await userService.CanAccessPremium(user)) { return false; } - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.YubiKey); - if (!provider?.MetaData.Values.Any(v => !string.IsNullOrWhiteSpace((string)v)) ?? true) + // Check if the user has a YubiKey provider configured + var yubicoProvider = user.GetTwoFactorProvider(TwoFactorProviderType.YubiKey); + if (!yubicoProvider?.MetaData.Values.Any(v => !string.IsNullOrWhiteSpace((string)v)) ?? true) { return false; } - return await userService.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.YubiKey, user); + return yubicoProvider.Enabled; } public Task GenerateAsync(string purpose, UserManager manager, User user) diff --git a/src/Core/Auth/Identity/UserStore.cs b/src/Core/Auth/Identity/UserStore.cs index 3716d75b6a..41323f05b7 100644 --- a/src/Core/Auth/Identity/UserStore.cs +++ b/src/Core/Auth/Identity/UserStore.cs @@ -1,7 +1,7 @@ -using Bit.Core.Context; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Repositories; -using Bit.Core.Services; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; @@ -167,7 +167,7 @@ public class UserStore : public async Task GetTwoFactorEnabledAsync(User user, CancellationToken cancellationToken) { - return await _serviceProvider.GetRequiredService().TwoFactorIsEnabledAsync(user); + return await _serviceProvider.GetRequiredService().TwoFactorIsEnabledAsync(user); } public Task SetSecurityStampAsync(User user, string stamp, CancellationToken cancellationToken) diff --git a/src/Core/Auth/Models/ITwoFactorProvidersUser.cs b/src/Core/Auth/Models/ITwoFactorProvidersUser.cs index 5d9ae4b362..f953e4570e 100644 --- a/src/Core/Auth/Models/ITwoFactorProvidersUser.cs +++ b/src/Core/Auth/Models/ITwoFactorProvidersUser.cs @@ -1,10 +1,18 @@ using Bit.Core.Auth.Enums; +using Bit.Core.Services; namespace Bit.Core.Auth.Models; public interface ITwoFactorProvidersUser { string TwoFactorProviders { get; } + /// + /// Get the two factor providers for the user. Currently it can be assumed providers are enabled + /// if they exists in the dictionary. When two factor providers are disabled they are removed + /// from the dictionary. + /// + /// + /// Dictionary of providers with the type enum as the key Dictionary GetTwoFactorProviders(); Guid? GetUserId(); bool GetPremium(); diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs index 203ef3accb..697c10690c 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs @@ -2,6 +2,7 @@ namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; + public interface ITwoFactorIsEnabledQuery { /// @@ -16,7 +17,8 @@ public interface ITwoFactorIsEnabledQuery /// The type of user in the list. Must implement . Task> TwoFactorIsEnabledAsync(IEnumerable users) where T : ITwoFactorProvidersUser; /// - /// Returns whether two factor is enabled for the user. + /// Returns whether two factor is enabled for the user. A user is able to have a TwoFactorProvider that is enabled but requires Premium. + /// If the user does not have premium then the TwoFactorProvider is considered _not_ enabled. /// /// The user to check. Task TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user); diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs index bda2094f24..8d4bd49e42 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs @@ -1,17 +1,13 @@ -using Bit.Core.Auth.Models; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Repositories; namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth; -public class TwoFactorIsEnabledQuery : ITwoFactorIsEnabledQuery +public class TwoFactorIsEnabledQuery(IUserRepository userRepository) : ITwoFactorIsEnabledQuery { - private readonly IUserRepository _userRepository; - - public TwoFactorIsEnabledQuery(IUserRepository userRepository) - { - _userRepository = userRepository; - } + private readonly IUserRepository _userRepository = userRepository; public async Task> TwoFactorIsEnabledAsync(IEnumerable userIds) { @@ -21,26 +17,15 @@ public class TwoFactorIsEnabledQuery : ITwoFactorIsEnabledQuery return result; } - var userDetails = await _userRepository.GetManyWithCalculatedPremiumAsync(userIds.ToList()); - + var userDetails = await _userRepository.GetManyWithCalculatedPremiumAsync([.. userIds]); foreach (var userDetail in userDetails) { - var hasTwoFactor = false; - var providers = userDetail.GetTwoFactorProviders(); - if (providers != null) - { - // Get all enabled providers - var enabledProviderKeys = from provider in providers - where provider.Value?.Enabled ?? false - select provider.Key; - - // Find the first provider that is enabled and passes the premium check - hasTwoFactor = enabledProviderKeys - .Select(type => userDetail.HasPremiumAccess || !TwoFactorProvider.RequiresPremium(type)) - .FirstOrDefault(); - } - - result.Add((userDetail.Id, hasTwoFactor)); + result.Add( + (userDetail.Id, + await TwoFactorEnabledAsync(userDetail.GetTwoFactorProviders(), + () => Task.FromResult(userDetail.HasPremiumAccess)) + ) + ); } return result; @@ -83,41 +68,56 @@ public class TwoFactorIsEnabledQuery : ITwoFactorIsEnabledQuery return false; } - var providers = user.GetTwoFactorProviders(); - if (providers == null || !providers.Any()) + return await TwoFactorEnabledAsync( + user.GetTwoFactorProviders(), + async () => + { + var calcUser = await _userRepository.GetCalculatedPremiumAsync(userId.Value); + return calcUser?.HasPremiumAccess ?? false; + }); + } + + /// + /// Checks to see what kind of two-factor is enabled. + /// We use a delegate to check if the user has premium access, since there are multiple ways to + /// determine if a user has premium access. + /// + /// dictionary of two factor providers + /// function to check if the user has premium access + /// true if the user has two factor enabled; false otherwise; + private async static Task TwoFactorEnabledAsync( + Dictionary providers, + Func> hasPremiumAccessDelegate) + { + // If there are no providers, then two factor is not enabled + if (providers == null || providers.Count == 0) { return false; } // Get all enabled providers - var enabledProviderKeys = providers - .Where(provider => provider.Value?.Enabled ?? false) - .Select(provider => provider.Key); + // TODO: PM-21210: In practice we don't save disabled providers to the database, worth looking into. + var enabledProviderKeys = from provider in providers + where provider.Value?.Enabled ?? false + select provider.Key; + // If no providers are enabled then two factor is not enabled if (!enabledProviderKeys.Any()) { return false; } - // Determine if any enabled provider passes the premium check - var hasTwoFactor = enabledProviderKeys - .Select(type => user.GetPremium() || !TwoFactorProvider.RequiresPremium(type)) - .FirstOrDefault(); - - // If no enabled provider passes the check, check the repository for organization premium access - if (!hasTwoFactor) + // If there are only premium two factor options then standard two factor is not enabled + var onlyHasPremiumTwoFactor = enabledProviderKeys.All(TwoFactorProvider.RequiresPremium); + if (onlyHasPremiumTwoFactor) { - var userDetails = await _userRepository.GetManyWithCalculatedPremiumAsync(new List { userId.Value }); - var userDetail = userDetails.FirstOrDefault(); - - if (userDetail != null) - { - hasTwoFactor = enabledProviderKeys - .Select(type => userDetail.HasPremiumAccess || !TwoFactorProvider.RequiresPremium(type)) - .FirstOrDefault(); - } + // There are no Standard two factor options, check if the user has premium access + // If the user has premium access, then two factor is enabled + var premiumAccess = await hasPremiumAccessDelegate(); + return premiumAccess; } - return hasTwoFactor; + // The user has at least one non-premium two factor option + return true; } } diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index 9878c96c1c..b3a6a9592e 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -128,6 +128,10 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac public bool IsExpired() => PremiumExpirationDate.HasValue && PremiumExpirationDate.Value <= DateTime.UtcNow; + /// + /// Deserializes the User.TwoFactorProviders property from JSON to the appropriate C# dictionary. + /// + /// Dictionary of TwoFactor providers public Dictionary? GetTwoFactorProviders() { if (string.IsNullOrWhiteSpace(TwoFactorProviders)) @@ -137,19 +141,17 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac try { - if (_twoFactorProviders == null) - { - _twoFactorProviders = - JsonHelpers.LegacyDeserialize>( - TwoFactorProviders); - } + _twoFactorProviders ??= + JsonHelpers.LegacyDeserialize>( + TwoFactorProviders); - // U2F is no longer supported, and all users keys should have been migrated to WebAuthn. - // To prevent issues with accounts being prompted for unsupported U2F we remove them - if (_twoFactorProviders.ContainsKey(TwoFactorProviderType.U2f)) - { - _twoFactorProviders.Remove(TwoFactorProviderType.U2f); - } + /* + U2F is no longer supported, and all users keys should have been migrated to WebAuthn. + To prevent issues with accounts being prompted for unsupported U2F we remove them. + This will probably exist in perpetuity since there is no way to know for sure if any + given user does or doesn't have this enabled. It is a non-zero chance. + */ + _twoFactorProviders?.Remove(TwoFactorProviderType.U2f); return _twoFactorProviders; } @@ -169,6 +171,10 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac return Premium; } + /// + /// Serializes the C# object to the User.TwoFactorProviders property in JSON format. + /// + /// Dictionary of Two Factor providers public void SetTwoFactorProviders(Dictionary providers) { // When replacing with system.text remember to remove the extra serialization in WebAuthnTokenProvider. @@ -176,20 +182,21 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac _twoFactorProviders = providers; } - public void ClearTwoFactorProviders() - { - SetTwoFactorProviders(new Dictionary()); - } - + /// + /// Checks if the user has a specific TwoFactorProvider configured. If a user has a premium TwoFactor + /// configured it will still be found, even if the user's premium subscription has ended. + /// + /// TwoFactor provider being searched for + /// TwoFactorProvider if found; null otherwise. public TwoFactorProvider? GetTwoFactorProvider(TwoFactorProviderType provider) { var providers = GetTwoFactorProviders(); - if (providers == null || !providers.ContainsKey(provider)) + if (providers == null || !providers.TryGetValue(provider, out var value)) { return null; } - return providers[provider]; + return value; } public long StorageBytesRemaining() diff --git a/src/Core/Repositories/IUserRepository.cs b/src/Core/Repositories/IUserRepository.cs index 0e59b9998f..22effb4329 100644 --- a/src/Core/Repositories/IUserRepository.cs +++ b/src/Core/Repositories/IUserRepository.cs @@ -25,6 +25,16 @@ public interface IUserRepository : IRepository /// Task> GetManyWithCalculatedPremiumAsync(IEnumerable ids); /// + /// Retrieves the data for the requested user ID and includes additional property indicating + /// whether the user has premium access directly or through an organization. + /// + /// Calls the same stored procedure as GetManyWithCalculatedPremiumAsync but handles the query + /// for a single user. + /// + /// The user ID to retrieve data for. + /// User data with calculated premium access; null if nothing is found + Task GetCalculatedPremiumAsync(Guid userId); + /// /// Sets a new user key and updates all encrypted data. /// Warning: Any user key encrypted data not included will be lost. /// diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 9b12713218..228b8543d7 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -71,11 +71,13 @@ public interface IUserService Task GenerateLicenseAsync(User user, SubscriptionInfo subscriptionInfo = null, int? version = null); Task CheckPasswordAsync(User user, string password); + /// + /// Checks if the user has access to premium features, either through a personal subscription or through an organization. + /// + /// user being acted on + /// true if they can access premium; false otherwise. Task CanAccessPremium(ITwoFactorProvidersUser user); Task HasPremiumFromOrganization(ITwoFactorProvidersUser user); - [Obsolete("Use ITwoFactorIsEnabledQuery instead.")] - Task TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user); - Task TwoFactorProviderIsEnabledAsync(TwoFactorProviderType provider, ITwoFactorProvidersUser user); Task GenerateSignInTokenAsync(User user, string purpose); Task UpdatePasswordHash(User user, string newPassword, diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 95ee4544fa..23617a0fcd 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -11,6 +11,7 @@ using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Constants; using Bit.Core.Billing.Models; using Bit.Core.Billing.Models.Sales; @@ -77,6 +78,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IPremiumUserBillingService _premiumUserBillingService; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IRevokeNonCompliantOrganizationUserCommand _revokeNonCompliantOrganizationUserCommand; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IDistributedCache _distributedCache; public UserService( @@ -115,6 +117,7 @@ public class UserService : UserManager, IUserService, IDisposable IPremiumUserBillingService premiumUserBillingService, IRemoveOrganizationUserCommand removeOrganizationUserCommand, IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IDistributedCache distributedCache) : base( store, @@ -158,6 +161,7 @@ public class UserService : UserManager, IUserService, IDisposable _premiumUserBillingService = premiumUserBillingService; _removeOrganizationUserCommand = removeOrganizationUserCommand; _revokeNonCompliantOrganizationUserCommand = revokeNonCompliantOrganizationUserCommand; + _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _distributedCache = distributedCache; } @@ -918,7 +922,7 @@ public class UserService : UserManager, IUserService, IDisposable await SaveUserAsync(user); await _eventService.LogUserEventAsync(user.Id, EventType.User_Disabled2fa); - if (!await TwoFactorIsEnabledAsync(user)) + if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user)) { await CheckPoliciesOnTwoFactorRemovalAsync(user); } @@ -1280,48 +1284,6 @@ public class UserService : UserManager, IUserService, IDisposable orgAbility.UsersGetPremium && orgAbility.Enabled); } - - public async Task TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user) - { - var providers = user.GetTwoFactorProviders(); - if (providers == null) - { - return false; - } - - foreach (var p in providers) - { - if (p.Value?.Enabled ?? false) - { - if (!TwoFactorProvider.RequiresPremium(p.Key)) - { - return true; - } - if (await CanAccessPremium(user)) - { - return true; - } - } - } - return false; - } - - public async Task TwoFactorProviderIsEnabledAsync(TwoFactorProviderType provider, ITwoFactorProvidersUser user) - { - var providers = user.GetTwoFactorProviders(); - if (providers == null || !providers.ContainsKey(provider) || !providers[provider].Enabled) - { - return false; - } - - if (!TwoFactorProvider.RequiresPremium(provider)) - { - return true; - } - - return await CanAccessPremium(user); - } - public async Task GenerateSignInTokenAsync(User user, string purpose) { var token = await GenerateUserTokenAsync(user, Options.Tokens.PasswordResetTokenProvider, diff --git a/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs new file mode 100644 index 0000000000..cc45fcb3eb --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/ITwoFactorAuthenticationValidator.cs @@ -0,0 +1,38 @@ + +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Entities; +using Duende.IdentityServer.Validation; + +namespace Bit.Identity.IdentityServer.RequestValidators; + +public interface ITwoFactorAuthenticationValidator +{ + /// + /// Check if the user is required to use two-factor authentication to login. This is based on the user's + /// enabled two-factor providers, the user's organizations enabled two-factor providers, and the grant type. + /// Client credentials and webauthn grant types do not require two-factor authentication. + /// + /// the active user for the request + /// the request that contains the grant types + /// boolean + Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request); + /// + /// Builds the two-factor authentication result for the user based on the available two-factor providers + /// from either their user account or Organization. + /// + /// user trying to login + /// organization associated with the user; Can be null + /// Dictionary with the TwoFactorProviderType as the Key and the Provider Metadata as the Value + Task> BuildTwoFactorResultAsync(User user, Organization organization); + /// + /// Uses the built in userManager methods to verify the two-factor token for the user. If the organization uses + /// organization duo, it will use the organization duo token provider to verify the token. + /// + /// the active User + /// organization of user; can be null + /// Two Factor Provider to use to verify the token + /// secret passed from the user and consumed by the two-factor provider's verify method + /// boolean + Task VerifyTwoFactorAsync(User user, Organization organization, TwoFactorProviderType twoFactorProviderType, string token); +} diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs index e733d4f410..80b3b6e1f4 100644 --- a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -4,6 +4,7 @@ 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.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Models.Data.Organizations; @@ -16,56 +17,25 @@ using Microsoft.AspNetCore.Identity; namespace Bit.Identity.IdentityServer.RequestValidators; -public interface ITwoFactorAuthenticationValidator -{ - /// - /// Check if the user is required to use two-factor authentication to login. This is based on the user's - /// enabled two-factor providers, the user's organizations enabled two-factor providers, and the grant type. - /// Client credentials and webauthn grant types do not require two-factor authentication. - /// - /// the active user for the request - /// the request that contains the grant types - /// boolean - Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request); - /// - /// Builds the two-factor authentication result for the user based on the available two-factor providers - /// from either their user account or Organization. - /// - /// user trying to login - /// organization associated with the user; Can be null - /// Dictionary with the TwoFactorProviderType as the Key and the Provider Metadata as the Value - Task> BuildTwoFactorResultAsync(User user, Organization organization); - /// - /// Uses the built in userManager methods to verify the two-factor token for the user. If the organization uses - /// organization duo, it will use the organization duo token provider to verify the token. - /// - /// the active User - /// organization of user; can be null - /// Two Factor Provider to use to verify the token - /// secret passed from the user and consumed by the two-factor provider's verify method - /// boolean - Task VerifyTwoFactorAsync(User user, Organization organization, TwoFactorProviderType twoFactorProviderType, string token); -} - public class TwoFactorAuthenticationValidator( IUserService userService, UserManager userManager, IOrganizationDuoUniversalTokenProvider organizationDuoWebTokenProvider, - IFeatureService featureService, IApplicationCacheService applicationCacheService, IOrganizationUserRepository organizationUserRepository, IOrganizationRepository organizationRepository, IDataProtectorTokenFactory ssoEmail2faSessionTokeFactory, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, ICurrentContext currentContext) : ITwoFactorAuthenticationValidator { private readonly IUserService _userService = userService; private readonly UserManager _userManager = userManager; private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider = organizationDuoWebTokenProvider; - private readonly IFeatureService _featureService = featureService; private readonly IApplicationCacheService _applicationCacheService = applicationCacheService; private readonly IOrganizationUserRepository _organizationUserRepository = organizationUserRepository; private readonly IOrganizationRepository _organizationRepository = organizationRepository; private readonly IDataProtectorTokenFactory _ssoEmail2faSessionTokeFactory = ssoEmail2faSessionTokeFactory; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; private readonly ICurrentContext _currentContext = currentContext; public async Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request) @@ -161,7 +131,7 @@ public class TwoFactorAuthenticationValidator( // These cases we want to always return false, U2f is deprecated and OrganizationDuo // uses a different flow than the other two factor providers, it follows the same - // structure of a UserTokenProvider but has it's logic ran outside the usual token + // structure of a UserTokenProvider but has it's logic runs outside the usual token // provider flow. See IOrganizationDuoUniversalTokenProvider.cs if (type is TwoFactorProviderType.U2f or TwoFactorProviderType.OrganizationDuo) { @@ -171,12 +141,12 @@ public class TwoFactorAuthenticationValidator( // Now we are concerning the rest of the Two Factor Provider Types // The intent of this check is to make sure that the user is using a 2FA provider that - // is enabled and allowed by their premium status. The exception for Remember - // is because it is a "special" 2FA type that isn't ever explicitly + // is enabled and allowed by their premium status. + // The exception for Remember is because it is a "special" 2FA type that isn't ever explicitly // enabled by a user, so we can't check the user's 2FA providers to see if they're // enabled. We just have to check if the token is valid. if (type != TwoFactorProviderType.Remember && - !await _userService.TwoFactorProviderIsEnabledAsync(type, user)) + user.GetTwoFactorProvider(type) == null) { return false; } diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index 28478a0c41..6b11d64cda 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -253,7 +253,6 @@ public class UserRepository : Repository, IUserRepository } } - public async Task UpdateUserKeyAndEncryptedDataV2Async( User user, IEnumerable updateDataActions) @@ -289,7 +288,6 @@ public class UserRepository : Repository, IUserRepository UnprotectData(user); } - public async Task> GetManyAsync(IEnumerable ids) { using (var connection = new SqlConnection(ReadOnlyConnectionString)) @@ -318,6 +316,14 @@ public class UserRepository : Repository, IUserRepository } } + public async Task GetCalculatedPremiumAsync(Guid userId) + { + var result = await GetManyWithCalculatedPremiumAsync([userId]); + + UnprotectData(result); + return result.SingleOrDefault(); + } + private async Task ProtectDataAndSaveAsync(User user, Func saveTask) { if (user == null) diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index 127646ed59..bd70e27e78 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -1,10 +1,10 @@ using AutoMapper; using Bit.Core.KeyManagement.UserKey; +using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Infrastructure.EntityFramework.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; -using DataModel = Bit.Core.Models.Data; #nullable enable @@ -38,13 +38,13 @@ public class UserRepository : Repository, IUserR } } - public async Task GetKdfInformationByEmailAsync(string email) + public async Task GetKdfInformationByEmailAsync(string email) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); return await GetDbSet(dbContext).Where(e => e.Email == email) - .Select(e => new DataModel.UserKdfInformation + .Select(e => new UserKdfInformation { Kdf = e.Kdf, KdfIterations = e.KdfIterations, @@ -251,13 +251,13 @@ public class UserRepository : Repository, IUserR } } - public async Task> GetManyWithCalculatedPremiumAsync(IEnumerable ids) + public async Task> GetManyWithCalculatedPremiumAsync(IEnumerable ids) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); var users = dbContext.Users.Where(x => ids.Contains(x.Id)); - return await users.Select(e => new DataModel.UserWithCalculatedPremium(e) + return await users.Select(e => new UserWithCalculatedPremium(e) { HasPremiumAccess = e.Premium || dbContext.OrganizationUsers .Any(ou => ou.UserId == e.Id && @@ -269,6 +269,12 @@ public class UserRepository : Repository, IUserR } } + public async Task GetCalculatedPremiumAsync(Guid id) + { + var result = await GetManyWithCalculatedPremiumAsync([id]); + return result.FirstOrDefault(); + } + public override async Task DeleteAsync(Core.Entities.User user) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index bd22fd9346..c617f6c9a9 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -14,6 +14,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Exceptions; @@ -40,6 +41,7 @@ public class AccountsControllerTests : IDisposable private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; private readonly IRotateUserKeyCommand _rotateUserKeyCommand; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand; private readonly IFeatureService _featureService; @@ -64,6 +66,7 @@ public class AccountsControllerTests : IDisposable _policyService = Substitute.For(); _setInitialMasterPasswordCommand = Substitute.For(); _rotateUserKeyCommand = Substitute.For(); + _twoFactorIsEnabledQuery = Substitute.For(); _tdeOffboardingPasswordCommand = Substitute.For(); _featureService = Substitute.For(); _cipherValidator = @@ -87,6 +90,7 @@ public class AccountsControllerTests : IDisposable _setInitialMasterPasswordCommand, _tdeOffboardingPasswordCommand, _rotateUserKeyCommand, + _twoFactorIsEnabledQuery, _featureService, _cipherValidator, _folderValidator, diff --git a/test/Api.Test/Vault/Controllers/SyncControllerTests.cs b/test/Api.Test/Vault/Controllers/SyncControllerTests.cs index 03c05ef0f4..ebbfc2a2ba 100644 --- a/test/Api.Test/Vault/Controllers/SyncControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/SyncControllerTests.cs @@ -8,6 +8,7 @@ using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Models.Data.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Models; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -64,6 +65,7 @@ public class SyncControllerTests { // Get dependencies var userService = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); var organizationUserRepository = sutProvider.GetDependency(); var providerUserRepository = sutProvider.GetDependency(); var folderRepository = sutProvider.GetDependency(); @@ -119,7 +121,7 @@ public class SyncControllerTests collectionRepository.GetManyByUserIdAsync(user.Id).Returns(collections); collectionCipherRepository.GetManyByUserIdAsync(user.Id).Returns(new List()); // Back to standard test setup - userService.TwoFactorIsEnabledAsync(user).Returns(false); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); userService.HasPremiumFromOrganization(user).Returns(false); // Execute GET @@ -129,7 +131,7 @@ public class SyncControllerTests // Asserts // Assert that methods are called var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); - await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository, + await this.AssertMethodsCalledAsync(userService, twoFactorIsEnabledQuery, organizationUserRepository, providerUserRepository, folderRepository, cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs); Assert.IsType(result); @@ -155,6 +157,7 @@ public class SyncControllerTests { // Get dependencies var userService = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); var organizationUserRepository = sutProvider.GetDependency(); var providerUserRepository = sutProvider.GetDependency(); var folderRepository = sutProvider.GetDependency(); @@ -205,7 +208,7 @@ public class SyncControllerTests policyRepository.GetManyByUserIdAsync(user.Id).Returns(policies); - userService.TwoFactorIsEnabledAsync(user).Returns(false); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); userService.HasPremiumFromOrganization(user).Returns(false); // Execute GET @@ -216,7 +219,7 @@ public class SyncControllerTests // Assert that methods are called var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); - await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository, + await this.AssertMethodsCalledAsync(userService, twoFactorIsEnabledQuery, organizationUserRepository, providerUserRepository, folderRepository, cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs); Assert.IsType(result); @@ -244,6 +247,7 @@ public class SyncControllerTests { // Get dependencies var userService = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); var organizationUserRepository = sutProvider.GetDependency(); var providerUserRepository = sutProvider.GetDependency(); var folderRepository = sutProvider.GetDependency(); @@ -283,7 +287,7 @@ public class SyncControllerTests collectionRepository.GetManyByUserIdAsync(user.Id).Returns(collections); collectionCipherRepository.GetManyByUserIdAsync(user.Id).Returns(new List()); // Back to standard test setup - userService.TwoFactorIsEnabledAsync(user).Returns(false); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); userService.HasPremiumFromOrganization(user).Returns(false); // Execute GET @@ -293,7 +297,7 @@ public class SyncControllerTests // Assert that methods are called var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); - await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository, + await this.AssertMethodsCalledAsync(userService, twoFactorIsEnabledQuery, organizationUserRepository, providerUserRepository, folderRepository, cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs); Assert.IsType(result); @@ -315,6 +319,7 @@ public class SyncControllerTests private async Task AssertMethodsCalledAsync(IUserService userService, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IOrganizationUserRepository organizationUserRepository, IProviderUserRepository providerUserRepository, IFolderRepository folderRepository, ICipherRepository cipherRepository, ISendRepository sendRepository, @@ -356,7 +361,7 @@ public class SyncControllerTests .GetManyByUserIdAsync(default); } - await userService.ReceivedWithAnyArgs(1) + await twoFactorIsEnabledQuery.ReceivedWithAnyArgs(1) .TwoFactorIsEnabledAsync(default(ITwoFactorProvidersUser)); await userService.ReceivedWithAnyArgs(1) .HasPremiumFromOrganization(default); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 2dda23481a..baf844acae 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; @@ -28,6 +29,7 @@ namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; public class AcceptOrgUserCommandTests { private readonly IUserService _userService = Substitute.For(); + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery = Substitute.For(); private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory = Substitute.For(); private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); @@ -165,7 +167,7 @@ public class AcceptOrgUserCommandTests SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); // User doesn't have 2FA enabled - _userService.TwoFactorIsEnabledAsync(user).Returns(false); + _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); // Organization they are trying to join requires 2FA var twoFactorPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; @@ -646,7 +648,7 @@ public class AcceptOrgUserCommandTests .Returns(false); // User doesn't have 2FA enabled - _userService.TwoFactorIsEnabledAsync(user).Returns(false); + _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user).Returns(false); // Org does not require 2FA sutProvider.GetDependency().GetPoliciesApplicableToUserAsync(user.Id, diff --git a/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs b/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs index da2d4a282a..ff09e1f141 100644 --- a/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs +++ b/test/Core.Test/Auth/Identity/BaseTokenProviderTests.cs @@ -44,9 +44,6 @@ public abstract class BaseTokenProviderTests protected virtual void SetupUserService(IUserService userService, User user) { - userService - .TwoFactorProviderIsEnabledAsync(TwoFactorProviderType, user) - .Returns(true); userService .CanAccessPremium(user) .Returns(true); @@ -85,8 +82,6 @@ public abstract class BaseTokenProviderTests var userManager = SubstituteUserManager(); MockDatabase(user, metaData); - AdditionalSetup(sutProvider, user); - var response = await sutProvider.Sut.CanGenerateTwoFactorTokenAsync(userManager, user); Assert.Equal(expectedResponse, response); } diff --git a/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs b/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs index 85c687119b..5715403974 100644 --- a/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs +++ b/test/Core.Test/Auth/Identity/DuoUniversalTwoFactorTokenProviderTests.cs @@ -83,6 +83,7 @@ public class DuoUniversalTwoFactorTokenProviderTests : BaseTokenProviderTests sutProvider) { // Arrange + AdditionalSetup(sutProvider, user); user.Premium = true; user.PremiumExpirationDate = DateTime.UtcNow.AddDays(1); @@ -100,6 +101,8 @@ public class DuoUniversalTwoFactorTokenProviderTests : BaseTokenProviderTests sutProvider) { // Arrange + AdditionalSetup(sutProvider, user); + user.Premium = false; sutProvider.GetDependency() diff --git a/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs index 8011c52ead..adeac45d06 100644 --- a/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs +++ b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs @@ -5,6 +5,7 @@ using Bit.Core.Entities; using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; +using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -53,6 +54,39 @@ public class TwoFactorIsEnabledQueryTests } } + [Theory, BitAutoData] + public async Task TwoFactorIsEnabledQuery_DatabaseReturnsEmpty_ResultEmpty( + SutProvider sutProvider, + List usersWithCalculatedPremium) + { + // Arrange + var userIds = usersWithCalculatedPremium.Select(u => u.Id).ToList(); + + sutProvider.GetDependency() + .GetManyWithCalculatedPremiumAsync(Arg.Any>()) + .Returns([]); + + // Act + var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(userIds); + + // Assert + Assert.Empty(result); + } + + [Theory] + [BitAutoData((IEnumerable)null)] + [BitAutoData([])] + public async Task TwoFactorIsEnabledQuery_UserIdsNullorEmpty_ResultEmpty( + IEnumerable userIds, + SutProvider sutProvider) + { + // Act + var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(userIds); + + // Assert + Assert.Empty(result); + } + [Theory] [BitAutoData] public async Task TwoFactorIsEnabledQuery_WithNoTwoFactorEnabled_ReturnsAllTwoFactorDisabled( @@ -122,8 +156,11 @@ public class TwoFactorIsEnabledQueryTests } [Theory] - [BitAutoData] - public async Task TwoFactorIsEnabledQuery_WithNullTwoFactorProviders_ReturnsAllTwoFactorDisabled( + [BitAutoData("")] + [BitAutoData("{}")] + [BitAutoData((string)null)] + public async Task TwoFactorIsEnabledQuery_WithNullOrEmptyTwoFactorProviders_ReturnsAllTwoFactorDisabled( + string twoFactorProviders, SutProvider sutProvider, List usersWithCalculatedPremium) { @@ -132,7 +169,7 @@ public class TwoFactorIsEnabledQueryTests foreach (var user in usersWithCalculatedPremium) { - user.TwoFactorProviders = null; // No two-factor providers configured + user.TwoFactorProviders = twoFactorProviders; // No two-factor providers configured } sutProvider.GetDependency() @@ -176,6 +213,24 @@ public class TwoFactorIsEnabledQueryTests .GetManyWithCalculatedPremiumAsync(default); } + [Theory] + [BitAutoData] + public async Task TwoFactorIsEnabledQuery_UserIdNull_ReturnsFalse( + SutProvider sutProvider) + { + // Arrange + var user = new TestTwoFactorProviderUser + { + Id = null + }; + + // Act + var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(user); + + // Assert + Assert.False(result); + } + [Theory] [BitAutoData(TwoFactorProviderType.Authenticator)] [BitAutoData(TwoFactorProviderType.Email)] @@ -193,10 +248,8 @@ public class TwoFactorIsEnabledQueryTests { freeProviderType, new TwoFactorProvider { Enabled = true } } }; - user.Premium = false; user.SetTwoFactorProviders(twoFactorProviders); - // Act var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(user); @@ -205,7 +258,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() - .GetManyWithCalculatedPremiumAsync(default); + .GetCalculatedPremiumAsync(default); } [Theory] @@ -230,7 +283,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() - .GetManyWithCalculatedPremiumAsync(default); + .GetCalculatedPremiumAsync(default); } [Theory] @@ -252,14 +305,18 @@ public class TwoFactorIsEnabledQueryTests user.SetTwoFactorProviders(twoFactorProviders); sutProvider.GetDependency() - .GetManyWithCalculatedPremiumAsync(Arg.Is>(i => i.Contains(user.Id))) - .Returns(new List { user }); + .GetCalculatedPremiumAsync(user.Id) + .Returns(user); // Act var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(user); // Assert Assert.False(result); + + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(1) + .GetCalculatedPremiumAsync(default); } [Theory] @@ -268,7 +325,7 @@ public class TwoFactorIsEnabledQueryTests public async Task TwoFactorIsEnabledQuery_WithProviderTypeRequiringPremium_WithUserPremium_ReturnsTrue( TwoFactorProviderType premiumProviderType, SutProvider sutProvider, - User user) + UserWithCalculatedPremium user) { // Arrange var twoFactorProviders = new Dictionary @@ -276,9 +333,14 @@ public class TwoFactorIsEnabledQueryTests { premiumProviderType, new TwoFactorProvider { Enabled = true } } }; - user.Premium = true; + user.Premium = false; + user.HasPremiumAccess = true; user.SetTwoFactorProviders(twoFactorProviders); + sutProvider.GetDependency() + .GetCalculatedPremiumAsync(user.Id) + .Returns(user); + // Act var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(user); @@ -286,8 +348,8 @@ public class TwoFactorIsEnabledQueryTests Assert.True(result); await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .GetManyWithCalculatedPremiumAsync(default); + .ReceivedWithAnyArgs(1) + .GetCalculatedPremiumAsync(default); } [Theory] @@ -309,14 +371,18 @@ public class TwoFactorIsEnabledQueryTests user.SetTwoFactorProviders(twoFactorProviders); sutProvider.GetDependency() - .GetManyWithCalculatedPremiumAsync(Arg.Is>(i => i.Contains(user.Id))) - .Returns(new List { user }); + .GetCalculatedPremiumAsync(user.Id) + .Returns(user); // Act var result = await sutProvider.Sut.TwoFactorIsEnabledAsync(user); // Assert Assert.True(result); + + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(1) + .GetCalculatedPremiumAsync(default); } [Theory] @@ -333,5 +399,29 @@ public class TwoFactorIsEnabledQueryTests // Assert Assert.False(result); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .GetCalculatedPremiumAsync(default); + } + + private class TestTwoFactorProviderUser : ITwoFactorProvidersUser + { + public Guid? Id { get; set; } + public string TwoFactorProviders { get; set; } + public bool Premium { get; set; } + public Dictionary GetTwoFactorProviders() + { + return JsonHelpers.LegacyDeserialize>(TwoFactorProviders); + } + + public Guid? GetUserId() + { + return Id; + } + + public bool GetPremium() + { + return Premium; + } } } diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 02ff24d9bf..d9bb2beaca 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -9,6 +9,7 @@ using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Services; using Bit.Core.Context; using Bit.Core.Entities; @@ -324,6 +325,7 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), + sutProvider.GetDependency(), sutProvider.GetDependency() ); @@ -476,6 +478,9 @@ public class UserServiceTests sutProvider.GetDependency() .GetByIdAsync(organization.Id) .Returns(organization); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(user) + .Returns(true); var expectedSavedProviders = JsonHelpers.LegacySerialize(new Dictionary { [TwoFactorProviderType.Remember] = new() { Enabled = true } @@ -911,6 +916,7 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), + sutProvider.GetDependency(), sutProvider.GetDependency() ); } diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs index fb4d7c321a..1f075a0147 100644 --- a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -2,6 +2,7 @@ using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity.TokenProviders; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Models.Data.Organizations; @@ -27,11 +28,11 @@ public class TwoFactorAuthenticationValidatorTests private readonly IUserService _userService; private readonly UserManagerTestWrapper _userManager; private readonly IOrganizationDuoUniversalTokenProvider _organizationDuoUniversalTokenProvider; - private readonly IFeatureService _featureService; private readonly IApplicationCacheService _applicationCacheService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationRepository _organizationRepository; private readonly IDataProtectorTokenFactory _ssoEmail2faSessionTokenable; + private readonly ITwoFactorIsEnabledQuery _twoFactorenabledQuery; private readonly ICurrentContext _currentContext; private readonly TwoFactorAuthenticationValidator _sut; @@ -40,22 +41,22 @@ public class TwoFactorAuthenticationValidatorTests _userService = Substitute.For(); _userManager = SubstituteUserManager(); _organizationDuoUniversalTokenProvider = Substitute.For(); - _featureService = Substitute.For(); _applicationCacheService = Substitute.For(); _organizationUserRepository = Substitute.For(); _organizationRepository = Substitute.For(); _ssoEmail2faSessionTokenable = Substitute.For>(); + _twoFactorenabledQuery = Substitute.For(); _currentContext = Substitute.For(); _sut = new TwoFactorAuthenticationValidator( _userService, _userManager, _organizationDuoUniversalTokenProvider, - _featureService, _applicationCacheService, _organizationUserRepository, _organizationRepository, _ssoEmail2faSessionTokenable, + _twoFactorenabledQuery, _currentContext); } @@ -263,9 +264,6 @@ public class TwoFactorAuthenticationValidatorTests _userManager.SUPPORTS_TWO_FACTOR = true; _userManager.TWO_FACTOR_PROVIDERS = [providerType.ToString()]; - _userService.TwoFactorProviderIsEnabledAsync(Arg.Any(), user) - .Returns(true); - // Act var result = await _sut.BuildTwoFactorResultAsync(user, null); @@ -322,9 +320,6 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _userService.TwoFactorProviderIsEnabledAsync( - TwoFactorProviderType.Email, user).Returns(true); - _userManager.TWO_FACTOR_PROVIDERS = ["email"]; // Act @@ -342,10 +337,8 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _userService.TwoFactorProviderIsEnabledAsync( - TwoFactorProviderType.Email, user).Returns(false); - _userManager.TWO_FACTOR_PROVIDERS = ["email"]; + user.TwoFactorProviders = ""; // Act var result = await _sut.VerifyTwoFactorAsync( @@ -362,9 +355,6 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _userService.TwoFactorProviderIsEnabledAsync( - TwoFactorProviderType.OrganizationDuo, user).Returns(false); - _userManager.TWO_FACTOR_PROVIDERS = ["OrganizationDuo"]; // Act @@ -387,11 +377,9 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _userService.TwoFactorProviderIsEnabledAsync( - providerType, user).Returns(true); - _userManager.TWO_FACTOR_ENABLED = true; _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); // Act var result = await _sut.VerifyTwoFactorAsync(user, null, providerType, token); @@ -412,11 +400,9 @@ public class TwoFactorAuthenticationValidatorTests string token) { // Arrange - _userService.TwoFactorProviderIsEnabledAsync( - providerType, user).Returns(true); - _userManager.TWO_FACTOR_ENABLED = true; _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); // Act var result = await _sut.VerifyTwoFactorAsync(user, null, providerType, token);