-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[PM-21411] Refactor interface for determining premium status and features #6688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 18 commits
5629d6f
c09a973
f83a790
d30f3bd
377ae31
683a6cb
08e1413
e494a10
0cdadd1
192052b
2d31237
123f579
57252a7
bf0cc7a
65941d4
2a966e3
96a3615
516824a
b8ff0ea
415a0b9
303b81c
30ff3da
02c2aa8
2184296
5257dc5
97bcf41
0a42c9e
651d5e3
353a07a
da9f0a1
df7bd08
cd0b3da
245ce71
0279cb5
1415517
2bd00c2
dbb8619
2f11c13
9a68a97
1cd5749
61775fb
19627f4
6a783ff
85e0e1a
f833040
de13a7c
03b0431
125b4de
2081dd4
de42f20
33a464a
97bf24e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,25 @@ | ||
| ๏ปฟ// FIXME: Update this file to be null safe and then delete the line below | ||
| #nullable disable | ||
|
|
||
| using Bit.Core.Auth.Enums; | ||
| ๏ปฟusing Bit.Core.Auth.Enums; | ||
| using Bit.Core.Services; | ||
|
|
||
| namespace Bit.Core.Auth.Models; | ||
|
|
||
| /// <summary> | ||
| /// An interface representing a user entity that supports two-factor providers | ||
| /// </summary> | ||
| public interface ITwoFactorProvidersUser | ||
| { | ||
| string TwoFactorProviders { get; } | ||
| 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(); | ||
| Dictionary<TwoFactorProviderType, TwoFactorProvider>? GetTwoFactorProviders(); | ||
| /// <summary> | ||
| /// The unique `UserId` of the user entity for which there are two-factor providers configured. | ||
| /// </summary> | ||
| /// <returns>The unique identifier for the user</returns> | ||
| Guid? GetUserId(); | ||
| bool GetPremium(); | ||
| } |
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,24 @@ | |
| using Bit.Core.Auth.Enums; | ||
| using Bit.Core.Auth.Models; | ||
| using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; | ||
| using Bit.Core.Billing.Premium.Queries; | ||
| using Bit.Core.Entities; | ||
| using Bit.Core.Repositories; | ||
|
|
||
| namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth; | ||
|
|
||
| public class TwoFactorIsEnabledQuery(IUserRepository userRepository) : ITwoFactorIsEnabledQuery | ||
| public class TwoFactorIsEnabledQuery : ITwoFactorIsEnabledQuery | ||
| { | ||
| private readonly IUserRepository _userRepository = userRepository; | ||
| private readonly IUserRepository _userRepository; | ||
| private readonly IHasPremiumAccessQuery _hasPremiumAccessQuery; | ||
|
|
||
| public TwoFactorIsEnabledQuery( | ||
| IUserRepository userRepository, | ||
| IHasPremiumAccessQuery hasPremiumAccessQuery) | ||
| { | ||
| _userRepository = userRepository; | ||
| _hasPremiumAccessQuery = hasPremiumAccessQuery; | ||
| } | ||
|
|
||
| public async Task<IEnumerable<(Guid userId, bool twoFactorIsEnabled)>> TwoFactorIsEnabledAsync(IEnumerable<Guid> userIds) | ||
| { | ||
|
|
@@ -72,12 +83,113 @@ public async Task<bool> TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user) | |
| } | ||
|
|
||
| return await TwoFactorEnabledAsync( | ||
| user.GetTwoFactorProviders(), | ||
| async () => | ||
| { | ||
| var calcUser = await _userRepository.GetCalculatedPremiumAsync(userId.Value); | ||
| return calcUser?.HasPremiumAccess ?? false; | ||
| }); | ||
| user.GetTwoFactorProviders(), | ||
| async () => | ||
| { | ||
| var calcUser = await _userRepository.GetCalculatedPremiumAsync(userId.Value); | ||
| return calcUser?.HasPremiumAccess ?? false; | ||
| }); | ||
| } | ||
|
|
||
| public async Task<IEnumerable<(Guid userId, bool twoFactorIsEnabled)>> TwoFactorIsEnabledVNextAsync(IEnumerable<Guid> userIds) | ||
| { | ||
| var result = new List<(Guid userId, bool hasTwoFactor)>(); | ||
| if (userIds == null || !userIds.Any()) | ||
| { | ||
| return result; | ||
| } | ||
|
|
||
| var users = await _userRepository.GetManyAsync([.. userIds]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If some user IDs don't exist in the database, Suggested fix: var users = await _userRepository.GetManyAsync([.. userIds]);
if (users.Count() != userIds.Distinct().Count())
{
var foundUserIds = users.Select(u => u.Id).ToHashSet();
var missingIds = userIds.Where(id => !foundUserIds.Contains(id));
throw new NotFoundException($"Users not found: {string.Join(", ", missingIds)}");
}This is especially important since the single-user overload throws |
||
| var premiumStatus = await _hasPremiumAccessQuery.HasPremiumAccessAsync(userIds); | ||
|
|
||
| foreach (var user in users) | ||
| { | ||
| var twoFactorProviders = user.GetTwoFactorProviders(); | ||
| var hasPremiumAccess = premiumStatus.GetValueOrDefault(user.Id, false); | ||
| var twoFactorIsEnabled = TwoFactorIsEnabled(twoFactorProviders, hasPremiumAccess); | ||
|
|
||
| result.Add((user.Id, twoFactorIsEnabled)); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| public async Task<IEnumerable<(T user, bool twoFactorIsEnabled)>> TwoFactorIsEnabledVNextAsync<T>(IEnumerable<T> users) | ||
| where T : ITwoFactorProvidersUser | ||
| { | ||
| var userIds = users | ||
| .Select(u => u.GetUserId()) | ||
| .Where(u => u.HasValue) | ||
| .Select(u => u.Value) | ||
| .ToList(); | ||
|
|
||
| var twoFactorResults = await TwoFactorIsEnabledVNextAsync(userIds); | ||
|
|
||
| var result = new List<(T user, bool twoFactorIsEnabled)>(); | ||
|
|
||
| foreach (var user in users) | ||
| { | ||
| var userId = user.GetUserId(); | ||
| if (userId.HasValue) | ||
| { | ||
| var hasTwoFactor = twoFactorResults.FirstOrDefault(res => res.userId == userId.Value).twoFactorIsEnabled; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Consider: var twoFactorResult = twoFactorResults.FirstOrDefault(res => res.userId == userId.Value);
if (twoFactorResult == default)
{
throw new NotFoundException($"Two-factor status not found for user {userId.Value}");
}
result.Add((user, twoFactorResult.twoFactorIsEnabled));This would make failures more explicit and easier to debug. |
||
| result.Add((user, hasTwoFactor)); | ||
| } | ||
| else | ||
| { | ||
| result.Add((user, false)); | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| public async Task<bool> TwoFactorIsEnabledVNextAsync(User user) | ||
| { | ||
| var providers = user.GetTwoFactorProviders(); | ||
| var hasPremium = await _hasPremiumAccessQuery.HasPremiumAccessAsync(user.Id); | ||
|
|
||
| return TwoFactorIsEnabled(providers, hasPremium); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks to see what kind of two-factor is enabled. | ||
| /// Synchronous version used when premium access status is already known. | ||
| /// </summary> | ||
| /// <param name="providers">dictionary of two factor providers</param> | ||
| /// <param name="hasPremiumAccess">whether the user has premium access</param> | ||
| /// <returns>true if the user has two factor enabled; false otherwise</returns> | ||
| private static bool TwoFactorIsEnabled( | ||
| Dictionary<TwoFactorProviderType, TwoFactorProvider> providers, | ||
| bool hasPremiumAccess) | ||
| { | ||
| // If there are no providers, then two factor is not enabled | ||
| if (providers == null || providers.Count == 0) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Get all enabled providers | ||
| // 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; | ||
| } | ||
|
|
||
| // If there are only premium two factor options then check premium access | ||
| var onlyHasPremiumTwoFactor = enabledProviderKeys.All(TwoFactorProvider.RequiresPremium); | ||
| if (onlyHasPremiumTwoFactor) | ||
| { | ||
| return hasPremiumAccess; | ||
| } | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // The user has at least one non-premium two factor option | ||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| ๏ปฟusing Bit.Core.Repositories; | ||
|
|
||
| namespace Bit.Core.Billing.Premium.Queries; | ||
|
|
||
| /// <summary> | ||
| /// Query for checking premium access status for users using the existing stored procedure | ||
| /// that calculates premium access from personal subscriptions and organization memberships. | ||
| /// </summary> | ||
| public class HasPremiumAccessQuery : IHasPremiumAccessQuery | ||
| { | ||
| private readonly IUserRepository _userRepository; | ||
|
|
||
| public HasPremiumAccessQuery(IUserRepository userRepository) | ||
| { | ||
| _userRepository = userRepository; | ||
| } | ||
|
|
||
| public async Task<bool> HasPremiumAccessAsync(Guid userId) | ||
| { | ||
| var user = await _userRepository.GetCalculatedPremiumAsync(userId); | ||
|
Check warning on line 20 in src/Core/Billing/Premium/Queries/HasPremiumAccessQuery.cs
|
||
| return user?.HasPremiumAccess ?? false; | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| public async Task<Dictionary<Guid, bool>> HasPremiumAccessAsync(IEnumerable<Guid> userIds) | ||
| { | ||
| var usersWithPremium = await _userRepository.GetManyWithCalculatedPremiumAsync(userIds); | ||
|
Check warning on line 26 in src/Core/Billing/Premium/Queries/HasPremiumAccessQuery.cs
|
||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return usersWithPremium.ToDictionary(u => u.Id, u => u.HasPremiumAccess); | ||
| } | ||
|
|
||
| public async Task<bool> HasPremiumFromOrganizationAsync(Guid userId) | ||
| { | ||
| var user = await _userRepository.GetCalculatedPremiumAsync(userId); | ||
|
Check warning on line 32 in src/Core/Billing/Premium/Queries/HasPremiumAccessQuery.cs
|
||
| if (user == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Has org premium if has premium access but not personal premium | ||
| return user.HasPremiumAccess && !user.Premium; | ||
|
||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| ๏ปฟnamespace Bit.Core.Billing.Premium.Queries; | ||
|
|
||
| /// <summary> | ||
| /// Query for checking premium access status for users. | ||
| /// This is the centralized location for determining if a user can access premium features | ||
| /// (either through personal subscription or organization membership). | ||
| /// | ||
| /// <para> | ||
| /// <strong>Note:</strong> This is different from checking User.Premium, which only indicates | ||
| /// personal subscription status. Use these methods to check actual premium feature access. | ||
| /// </para> | ||
| /// </summary> | ||
| public interface IHasPremiumAccessQuery | ||
| { | ||
| /// <summary> | ||
| /// Checks if a user has access to premium features (personal subscription or organization). | ||
| /// This is the definitive way to check premium access for a single user. | ||
| /// </summary> | ||
| /// <param name="userId">The user ID to check for premium access</param> | ||
| /// <returns>True if user can access premium features; false otherwise</returns> | ||
| Task<bool> HasPremiumAccessAsync(Guid userId); | ||
|
|
||
| /// <summary> | ||
| /// Checks if a user has access to premium features through organization membership only. | ||
| /// This is useful for determining the source of premium access (personal vs organization). | ||
| /// </summary> | ||
| /// <param name="userId">The user ID to check for organization premium access</param> | ||
| /// <returns>True if user has premium access through any organization; false otherwise</returns> | ||
| Task<bool> HasPremiumFromOrganizationAsync(Guid userId); | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /// <summary> | ||
| /// Checks if multiple users have access to premium features (optimized bulk operation). | ||
| /// Uses existing stored procedure that calculates premium from personal subscriptions and organizations. | ||
| /// </summary> | ||
| /// <param name="userIds">The user IDs to check for premium access</param> | ||
| /// <returns>Dictionary mapping user IDs to their premium access status (personal or through organization)</returns> | ||
| Task<Dictionary<Guid, bool>> HasPremiumAccessAsync(IEnumerable<Guid> userIds); | ||
| } | ||
|
|
||
|
|
||
|
|
||
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
Uh oh!
There was an error while loading. Please reload this page.