Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f83a790
Add PremiumAccessCacheCheck feature flag to Constants.cs
r-tome Dec 4, 2025
d30f3bd
Add IPremiumAccessQuery interface and PremiumAccessQuery implementatiโ€ฆ
r-tome Dec 4, 2025
377ae31
Add unit tests for PremiumAccessQuery to validate user premium accessโ€ฆ
r-tome Dec 4, 2025
683a6cb
Add XML documentation to Premium in OrganizationUserUserDetails and Uโ€ฆ
r-tome Dec 4, 2025
08e1413
Add PremiumAccessQueries to UserServiceCollectionExtensions
r-tome Dec 4, 2025
e494a10
Refactor TwoFactorIsEnabledQuery to incorporate PremiumAccessQuery anโ€ฆ
r-tome Dec 4, 2025
0cdadd1
Mark methods in IUserRepository and IUserService as obsolete, directiโ€ฆ
r-tome Dec 4, 2025
192052b
Rename CanAccessPremiumBulkAsync to CanAccessPremiumAsync in IPremiumโ€ฆ
r-tome Dec 5, 2025
2d31237
Update TwoFactorIsEnabledQuery to use CanAccessPremiumAsync for premiโ€ฆ
r-tome Dec 5, 2025
123f579
Refactor TwoFactorIsEnabledQuery to introduce VNextAsync methods for โ€ฆ
r-tome Dec 5, 2025
57252a7
Refactor IPremiumAccessQuery and PremiumAccessQuery to remove the oveโ€ฆ
r-tome Dec 5, 2025
bf0cc7a
Add new sync static method to determine if TwoFactor is enabled
r-tome Dec 5, 2025
65941d4
Enhance XML documentation for Premium property in OrganizationUserUseโ€ฆ
r-tome Dec 5, 2025
2a966e3
Refactor IPremiumAccessQuery and PremiumAccessQuery to replace User pโ€ฆ
r-tome Dec 5, 2025
96a3615
Update feature flag references in IUserRepository and IUserService toโ€ฆ
r-tome Dec 5, 2025
516824a
Rename IPremiumAccessQuery to IHasPremiumAccessQuery and move to Billโ€ฆ
r-tome Dec 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ public class OrganizationUserUserDetails : IExternal, ITwoFactorProvidersUser, I
public string Email { get; set; }
public string AvatarColor { get; set; }
public string TwoFactorProviders { get; set; }
/// <summary>
/// Indicates whether the user has a personal premium subscription.
/// Does not include premium access from organizations -
/// do not use this to check whether the user can access premium features.
/// Null when the organization user is in Invited status (UserId is null).
/// </summary>
public bool? Premium { get; set; }
public OrganizationUserStatusType Status { get; set; }
public OrganizationUserType Type { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
๏ปฟusing Bit.Core.Auth.Models;
using Bit.Core.Entities;

namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;

Expand All @@ -22,4 +23,25 @@ public interface ITwoFactorIsEnabledQuery
/// </summary>
/// <param name="user">The user to check.</param>
Task<bool> TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user);

/// <summary>
/// Returns a list of user IDs and whether two factor is enabled for each user.
/// This version uses HasPremiumAccessQuery with cached organization abilities for better performance.
/// </summary>
/// <param name="userIds">The list of user IDs to check.</param>
Task<IEnumerable<(Guid userId, bool twoFactorIsEnabled)>> TwoFactorIsEnabledVNextAsync(IEnumerable<Guid> userIds);
/// <summary>
/// Returns a list of users and whether two factor is enabled for each user.
/// This version uses HasPremiumAccessQuery with cached organization abilities for better performance.
/// </summary>
/// <param name="users">The list of users to check.</param>
/// <typeparam name="T">The type of user in the list. Must implement <see cref="ITwoFactorProvidersUser"/>.</typeparam>
Task<IEnumerable<(T user, bool twoFactorIsEnabled)>> TwoFactorIsEnabledVNextAsync<T>(IEnumerable<T> users) where T : ITwoFactorProvidersUser;
/// <summary>
/// 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.
/// This version uses HasPremiumAccessQuery with cached organization abilities for better performance.
/// </summary>
/// <param name="user">The user to check.</param>
Task<bool> TwoFactorIsEnabledVNextAsync(User user);
}
128 changes: 120 additions & 8 deletions src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still need to wire up the new methods here. (preferably just have the old methods call them and return the result)

Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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]);
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;
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;
}
Comment on lines +184 to +189
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was previously done to avoid the db call, which was passed in as a delegate to defer that work. But now we're doing our database call upfront, I think this whole method could be simplified to:

        return providers != null && providers.Values.Any(p => p.Enabled);

which means you probably don't need the private method at all. I think that's a great simplification of this logic.

That said, Auth might have greater insight into any performance requirements here re: that database call (and this logic generally).


// The user has at least one non-premium two factor option
return true;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces;
using Bit.Core.Auth.UserFeatures.WebAuthnLogin;
using Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations;
using Bit.Core.Billing.Premium.Queries;
using Bit.Core.KeyManagement.UserKey;
using Bit.Core.KeyManagement.UserKey.Implementations;
using Bit.Core.Services;
Expand All @@ -27,6 +28,7 @@ public static void AddUserServices(this IServiceCollection services, IGlobalSett
services.AddUserRegistrationCommands();
services.AddWebAuthnLoginCommands();
services.AddTdeOffboardingPasswordCommands();
services.AddPremiumAccessQueries();
services.AddTwoFactorQueries();
services.AddSsoQueries();
}
Expand Down Expand Up @@ -65,6 +67,11 @@ private static void AddWebAuthnLoginCommands(this IServiceCollection services)
services.AddScoped<IAssertWebAuthnLoginCredentialCommand, AssertWebAuthnLoginCredentialCommand>();
}

private static void AddPremiumAccessQueries(this IServiceCollection services)
{
services.AddScoped<IHasPremiumAccessQuery, HasPremiumAccessQuery>();
}

private static void AddTwoFactorQueries(this IServiceCollection services)
{
services.AddScoped<ITwoFactorIsEnabledQuery, TwoFactorIsEnabledQuery>();
Expand Down
44 changes: 44 additions & 0 deletions src/Core/Billing/Premium/Queries/HasPremiumAccessQuery.cs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so simple now ๐Ÿ‘

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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'IUserRepository.GetCalculatedPremiumAsync(Guid)' is obsolete: 'Use IUserService.CanAccessPremium instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.'

See more on https://sonarcloud.io/project/issues?id=bitwarden_server&issues=AZrviTT9rh6_zY1RwvBt&open=AZrviTT9rh6_zY1RwvBt&pullRequest=6688
return user?.HasPremiumAccess ?? false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no user, something has gone wrong. I think it should throw an exception in this case.

}

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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'IUserRepository.GetManyWithCalculatedPremiumAsync(IEnumerable<Guid>)' is obsolete: 'Use IUserService.CanAccessPremiumBulk instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.'

See more on https://sonarcloud.io/project/issues?id=bitwarden_server&issues=AZrviTT9rh6_zY1RwvBu&open=AZrviTT9rh6_zY1RwvBu&pullRequest=6688
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the other method, and with the interface for this query (which promises a key for each userId), I think this should also check and throw for missing users.

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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'IUserRepository.GetCalculatedPremiumAsync(Guid)' is obsolete: 'Use IUserService.CanAccessPremium instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.'

See more on https://sonarcloud.io/project/issues?id=bitwarden_server&issues=AZrviTT9rh6_zY1RwvBv&open=AZrviTT9rh6_zY1RwvBv&pullRequest=6688
if (user == null)
{
return false;
}

// Has org premium if has premium access but not personal premium
return user.HasPremiumAccess && !user.Premium;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is subtly different to the current implementation in UserService.HasPremiumFromOrganization: that current logic will return false if the user is not a part of any orgs, but will not return false if they have both personal premium and premium from an org. That doesn't seem right either, but it's unclear what the intent is.

That said, I have no idea why we need this logic: it's only synced to clients but doesn't seem to be used there either. I would be interested to know if we could remove it.

Any ideas @amorask-bitwarden ?

}
}



41 changes: 41 additions & 0 deletions src/Core/Billing/Premium/Queries/IHasPremiumAccessQuery.cs
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);
Comment on lines +23 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently unused, I recommend removing it if we don't have a clear use case for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will eventually replace

public async Task<bool> HasPremiumFromOrganization(ITwoFactorProvidersUser user)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be wired up in that method though? It's not called from anywhere at the moment (as far as I can see).


/// <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);
}



1 change: 1 addition & 0 deletions src/Core/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public static class FeatureFlagKeys
public const string BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration";
public const string PolicyValidatorsRefactor = "pm-26423-refactor-policy-side-effects";
public const string IncreaseBulkReinviteLimitForCloud = "pm-28251-increase-bulk-reinvite-limit-for-cloud";
public const string PremiumAccessQuery = "pm-21411-premium-access-query";

/* Architecture */
public const string DesktopMigrationMilestone1 = "desktop-ui-migration-milestone-1";
Expand Down
5 changes: 5 additions & 0 deletions src/Core/Entities/User.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public class User : ITableObject<Guid>, IStorableSubscriber, IRevisable, ITwoFac
/// The security state is a signed object attesting to the version of the user's account.
/// </summary>
public string? SecurityState { get; set; }
/// <summary>
/// Indicates whether the user has a personal premium subscription.
/// Does not include premium access from organizations -
/// do not use this to check whether the user can access premium features.
/// </summary>
public bool Premium { get; set; }
public DateTime? PremiumExpirationDate { get; set; }
public DateTime? RenewalReminderDate { get; set; }
Expand Down
2 changes: 2 additions & 0 deletions src/Core/Repositories/IUserRepository.cs
Copy link
Member

@eliykat eliykat Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods are no longer obsolete, because your new query is using it. Although you may still want to mention that this is for internal use only, and devs should use the query instead.

All of that said - I think this sproc could be simplified per my last review, and now is a good time to do it. (i.e. by making a new sproc used by the new code, then deleting this when you clean up your feature flag). I'll leave it up to you though. It could also be a separate ticket.

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public interface IUserRepository : IRepository<User, Guid>
/// Retrieves the data for the requested user IDs and includes an additional property indicating
/// whether the user has premium access directly or through an organization.
/// </summary>
[Obsolete("Use IUserService.CanAccessPremiumBulk instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.")]
Task<IEnumerable<UserWithCalculatedPremium>> GetManyWithCalculatedPremiumAsync(IEnumerable<Guid> ids);
/// <summary>
/// Retrieves the data for the requested user ID and includes additional property indicating
Expand All @@ -33,6 +34,7 @@ public interface IUserRepository : IRepository<User, Guid>
/// </summary>
/// <param name="userId">The user ID to retrieve data for.</param>
/// <returns>User data with calculated premium access; null if nothing is found</returns>
[Obsolete("Use IUserService.CanAccessPremium instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.")]
Task<UserWithCalculatedPremium?> GetCalculatedPremiumAsync(Guid userId);
/// <summary>
/// Sets a new user key and updates all encrypted data.
Expand Down
2 changes: 2 additions & 0 deletions src/Core/Services/IUserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ Task<UserLicense> GenerateLicenseAsync(User user, SubscriptionInfo subscriptionI
/// </summary>
/// <param name="user">user being acted on</param>
/// <returns>true if they can access premium; false otherwise.</returns>
[Obsolete("Use IHasPremiumAccessQuery.HasPremiumAccessAsync instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.")]
Task<bool> CanAccessPremium(User user);

/// <summary>
Expand All @@ -74,6 +75,7 @@ Task<UserLicense> GenerateLicenseAsync(User user, SubscriptionInfo subscriptionI
/// </summary>
/// <param name="user"></param>
/// <returns></returns>
[Obsolete("Use IHasPremiumAccessQuery.HasPremiumFromOrganizationAsync instead. This method is only used when feature flag 'PremiumAccessQuery' is disabled.")]
Task<bool> HasPremiumFromOrganization(User user);
Task<string> GenerateSignInTokenAsync(User user, string purpose);

Expand Down
Loading
Loading