nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-21 01:21:42 -05:00
Code Activity

[PM-19029][PM-19203] Addressing UserService tech debt around ITwoFactorIsEnabledQuery (#5754)

Browse Source 
* 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
This commit is contained in:
Ike
2025-05-09 11:39:57 -04:00
committed by GitHub
parent 80e7a0afd6
commit 3f95513d11
31 changed files with 372 additions and 259 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/Core/Auth/Enums/TwoFactorProviderType.cs
View File

@ -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,

15
src/Core/Auth/Identity/TokenProviders/AuthenticatorTokenProvider.cs
View File

@ -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<User>
{
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<User>
};
}
public async Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
public Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> 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<IUserService>()
.TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Authenticator, user);
return Task.FromResult(authenticatorProvider.Enabled);
}
public Task<string> GenerateAsync(string purpose, UserManager<User> manager, User user)

11
src/Core/Auth/Identity/TokenProviders/DuoUniversalTokenProvider.cs
View File

@ -17,7 +17,7 @@ public class DuoUniversalTokenProvider(
{
/// <summary>
/// We need the IServiceProvider to resolve the <see cref="IUserService"/>. There is a complex dependency dance
/// occurring between <see cref="IUserService"/>, which extends the <see cref="UserManager{User}"/>, and the usage
/// occurring between <see cref="IUserService"/>, which extends the <see cref="UserManager{User}"/>, and the usage
/// of the <see cref="UserManager{User}"/> within this class. Trying to resolve the <see cref="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.
@ -29,12 +29,13 @@ public class DuoUniversalTokenProvider(
public async Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
{
var userService = _serviceProvider.GetRequiredService<IUserService>();
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<string> GenerateAsync(string purpose, UserManager<User> manager, User user)
@ -58,7 +59,7 @@ public class DuoUniversalTokenProvider(
}
/// <summary>
/// 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
/// </summary>
/// <param name="user">Active User</param>
/// <returns>null or Duo TwoFactorProvider</returns>

17
src/Core/Auth/Identity/TokenProviders/EmailTwoFactorTokenProvider.cs
View File

@ -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<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
public override Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> 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<IUserService>().
TwoFactorProviderIsEnabledAsync(TwoFactorProviderType.Email, user);
return Task.FromResult(emailTokenProvider.Enabled);
}
public override Task<string> GenerateAsync(string purpose, UserManager<User> manager, User user)

19
src/Core/Auth/Identity/TokenProviders/WebAuthnTokenProvider.cs
View File

@ -25,17 +25,16 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider<User>
_globalSettings = globalSettings;
}
public async Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
public Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
{
var userService = _serviceProvider.GetRequiredService<IUserService>();
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<string> GenerateAsync(string purpose, UserManager<User> manager, User user)
@ -81,7 +80,7 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider<User>
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<User>
var clientResponse = JsonSerializer.Deserialize<AuthenticatorAssertionRawResponse>(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<User>
}
/// <summary>
/// Checks if the provider has proper metadata.
/// This is used to determine if the provider has been properly configured.
/// </summary>
/// <param name="provider"></param>
/// <returns>true if metadata is present; false if empty or null</returns>
private bool HasProperMetaData(TwoFactorProvider provider)
{
return provider?.MetaData?.Any() ?? false;

8
src/Core/Auth/Identity/TokenProviders/YubicoOtpTokenProvider.cs
View File

@ -23,19 +23,21 @@ public class YubicoOtpTokenProvider : IUserTwoFactorTokenProvider<User>
public async Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
{
// Ensure the user has access to premium
var userService = _serviceProvider.GetRequiredService<IUserService>();
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<string> GenerateAsync(string purpose, UserManager<User> manager, User user)

6
src/Core/Auth/Identity/UserStore.cs
View File

@ -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<bool> GetTwoFactorEnabledAsync(User user, CancellationToken cancellationToken)
{
return await _serviceProvider.GetRequiredService<IUserService>().TwoFactorIsEnabledAsync(user);
return await _serviceProvider.GetRequiredService<ITwoFactorIsEnabledQuery>().TwoFactorIsEnabledAsync(user);
}
public Task SetSecurityStampAsync(User user, string stamp, CancellationToken cancellationToken)

8
src/Core/Auth/Models/ITwoFactorProvidersUser.cs
View File

@ -1,10 +1,18 @@
using Bit.Core.Auth.Enums;
using Bit.Core.Services;
namespace Bit.Core.Auth.Models;
public interface ITwoFactorProvidersUser
{
string TwoFactorProviders { get; }
/// <summary>
/// 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. <see cref="IUserService.DisableTwoFactorProviderAsync"/>
/// <see cref="IOrganizationService.DisableTwoFactorProviderAsync"/>
/// </summary>
/// <returns>Dictionary of providers with the type enum as the key</returns>
Dictionary<TwoFactorProviderType, TwoFactorProvider> GetTwoFactorProviders();
Guid? GetUserId();
bool GetPremium();

4
src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs
View File

@ -2,6 +2,7 @@
namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
public interface ITwoFactorIsEnabledQuery
{
/// <summary>
@ -16,7 +17,8 @@ public interface ITwoFactorIsEnabledQuery
/// <typeparam name="T">The type of user in the list. Must implement <see cref="ITwoFactorProvidersUser"/>.</typeparam>
Task<IEnumerable<(T user, bool twoFactorIsEnabled)>> TwoFactorIsEnabledAsync<T>(IEnumerable<T> users) where T : ITwoFactorProvidersUser;
/// <summary>
/// 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.
/// </summary>
/// <param name="user">The user to check.</param>
Task<bool> TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user);

96
src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs
View File

@ -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<IEnumerable<(Guid userId, bool twoFactorIsEnabled)>> TwoFactorIsEnabledAsync(IEnumerable<Guid> 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;
});
}
/// <summary>
/// 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.
/// </summary>
/// <param name="providers">dictionary of two factor providers</param>
/// <param name="hasPremiumAccessDelegate">function to check if the user has premium access</param>
/// <returns> true if the user has two factor enabled; false otherwise;</returns>
private async static Task<bool> TwoFactorEnabledAsync(
Dictionary<TwoFactorProviderType, TwoFactorProvider> providers,
Func<Task<bool>> 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<Guid> { 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;
}
}