Skip to content

[PM-332124] Finalize PolicyRequirement + 2FA Feature Flag #7209

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.

Sign up for GitHub

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

Open
sven-bitwarden wants to merge 2 commits into
base: main
Choose a base branch
from
+199 βˆ’575
Open

[PM-332124] Finalize PolicyRequirement + 2FA Feature Flag #7209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2cf2a10
Remove 2FA feature flag
sven-bitwarden Mar 13, 2026
171629c
Merge branch 'main' into ac/pm-33214/remove-2fa-policy-requirement-flag
sven-bitwarden Mar 13, 2026
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
30 changes: 5 additions & 25 deletions src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
ο»Ώ// FIXME: Update this file to be null safe and then delete the line below
#nullable disable

using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements.Errors;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Auth.UserFeatures.EmergencyAccess.Interfaces;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
Expand All @@ -26,7 +24,6 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand
{
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IOrganizationRepository _organizationRepository;
private readonly IPolicyService _policyService;
private readonly IMailService _mailService;
private readonly IUserRepository _userRepository;
private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
Expand All @@ -40,7 +37,6 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand
public AcceptOrgUserCommand(
IOrganizationUserRepository organizationUserRepository,
IOrganizationRepository organizationRepository,
IPolicyService policyService,
IMailService mailService,
IUserRepository userRepository,
ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery,
Expand All @@ -53,7 +49,6 @@ public AcceptOrgUserCommand(
{
_organizationUserRepository = organizationUserRepository;
_organizationRepository = organizationRepository;
_policyService = policyService;
_mailService = mailService;
_userRepository = userRepository;
_twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
Expand Down Expand Up @@ -226,31 +221,16 @@ private async Task ValidateSingleOrganizationPolicyAsync(OrganizationUser orgUse

private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId)
{
if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements))
if (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user))
{
if (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user))
{
// If the user has two-step login enabled, we skip checking the 2FA policy
return;
}

var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId))
{
throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account.");
}

// If the user has two-step login enabled, we skip checking the 2FA policy
return;
}

if (!await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user))
var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId))
{
var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id,
PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited);
if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == organizationId))
{
throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account.");
}
throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account.");
}
}

Expand Down
25 changes: 4 additions & 21 deletions ...ore/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
#nullable disable

using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.OrganizationConfirmation;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements.Errors;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.UserFeatures.EmergencyAccess.Interfaces;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
using Bit.Core.Billing.Enums;
Expand All @@ -31,7 +29,6 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
private readonly IPushNotificationService _pushNotificationService;
private readonly IPushRegistrationService _pushRegistrationService;
private readonly IPolicyService _policyService;
private readonly IDeviceRepository _deviceRepository;
private readonly IPolicyRequirementQuery _policyRequirementQuery;
private readonly IFeatureService _featureService;
Expand All @@ -48,7 +45,6 @@ public ConfirmOrganizationUserCommand(
ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery,
IPushNotificationService pushNotificationService,
IPushRegistrationService pushRegistrationService,
IPolicyService policyService,
IDeviceRepository deviceRepository,
IPolicyRequirementQuery policyRequirementQuery,
IFeatureService featureService,
Expand All @@ -64,7 +60,6 @@ public ConfirmOrganizationUserCommand(
_twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
_pushNotificationService = pushNotificationService;
_pushRegistrationService = pushRegistrationService;
_policyService = policyService;
_deviceRepository = deviceRepository;
_policyRequirementQuery = policyRequirementQuery;
_featureService = featureService;
Expand Down Expand Up @@ -237,26 +232,14 @@ private async Task CheckPoliciesAsync(Guid organizationId, User user,

private async Task ValidateTwoFactorAuthenticationPolicyAsync(User user, Guid organizationId, bool userTwoFactorEnabled)
{
if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements))
if (userTwoFactorEnabled)
{
if (userTwoFactorEnabled)
{
// If the user has two-step login enabled, we skip checking the 2FA policy
return;
}

var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId))
{
throw new BadRequestException("User does not have two-step login enabled.");
}

// If the user has two-step login enabled, we skip checking the 2FA policy
return;
}

var orgRequiresTwoFactor = (await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication))
.Any(p => p.OrganizationId == organizationId);
if (orgRequiresTwoFactor && !userTwoFactorEnabled)
var twoFactorPolicyRequirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (twoFactorPolicyRequirement.IsTwoFactorRequiredForOrganization(organizationId))
{
throw new BadRequestException("User does not have two-step login enabled.");
}
Expand Down
14 changes: 2 additions & 12 deletions ...e/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
#nullable disable

using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements.Errors;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.UserFeatures.EmergencyAccess.Interfaces;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
using Bit.Core.Billing.Enums;
Expand All @@ -28,7 +26,6 @@ public class RestoreOrganizationUserCommand(
IOrganizationUserRepository organizationUserRepository,
IOrganizationRepository organizationRepository,
ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery,
IPolicyService policyService,
IUserRepository userRepository,
IOrganizationService organizationService,
IFeatureService featureService,
Expand Down Expand Up @@ -373,14 +370,7 @@ private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, boo

private async Task<bool> IsTwoFactorRequiredForOrganizationAsync(Guid userId, Guid organizationId)
{
if (featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements))
{
var requirement = await policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(userId);
return requirement.IsTwoFactorRequiredForOrganization(organizationId);
}

var invitedTwoFactorPolicies = await policyService.GetPoliciesApplicableToUserAsync(userId,
PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked);
return invitedTwoFactorPolicies.Any(p => p.OrganizationId == organizationId);
var requirement = await policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(userId);
return requirement.IsTwoFactorRequiredForOrganization(organizationId);
}
}
78 changes: 27 additions & 51 deletions src/Core/Services/Implementations/UserService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
using Bit.Core.Billing.Licenses;
Expand All @@ -30,7 +29,6 @@
using Bit.Core.Repositories;
using Bit.Core.Settings;
using Bit.Core.Utilities;
using Fido2NetLib;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Logging;
Expand All @@ -54,11 +52,8 @@
private readonly IEnumerable<IPasswordValidator<User>> _passwordValidators;
private readonly ILicensingService _licenseService;
private readonly IEventService _eventService;
private readonly IApplicationCacheService _applicationCacheService;
private readonly IStripePaymentService _paymentService;
private readonly IPolicyQuery _policyQuery;
private readonly IPolicyService _policyService;
private readonly IFido2 _fido2;
private readonly ICurrentContext _currentContext;
private readonly IGlobalSettings _globalSettings;
private readonly IAcceptOrgUserCommand _acceptOrgUserCommand;
Expand Down Expand Up @@ -90,11 +85,8 @@
ILogger<UserManager<User>> logger,
ILicensingService licenseService,
IEventService eventService,
IApplicationCacheService applicationCacheService,
IStripePaymentService paymentService,
IPolicyQuery policyQuery,
IPolicyService policyService,
IFido2 fido2,
ICurrentContext currentContext,
IGlobalSettings globalSettings,
IAcceptOrgUserCommand acceptOrgUserCommand,
Expand Down Expand Up @@ -130,11 +122,8 @@
_passwordValidators = passwordValidators;
_licenseService = licenseService;
_eventService = eventService;
_applicationCacheService = applicationCacheService;
_paymentService = paymentService;
_policyQuery = policyQuery;
_policyService = policyService;
_fido2 = fido2;
_currentContext = currentContext;
_globalSettings = globalSettings;
_acceptOrgUserCommand = acceptOrgUserCommand;
Expand Down Expand Up @@ -727,7 +716,7 @@
{
if (!CoreHelpers.FixedTimeEquals(
user.TwoFactorRecoveryCode,
recoveryCode.Replace(" ", string.Empty).Trim().ToLower()))

Check warning on line 719 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Notifications, ./src, true) 

The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'UserService.RecoverTwoFactorAsync(User, string)' with a call to 'string.ToLower(CultureInfo)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1304)

Check warning on line 719 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Events, ./src, true) 

The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'UserService.RecoverTwoFactorAsync(User, string)' with a call to 'string.ToLower(CultureInfo)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1304)

Check warning on line 719 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Billing, ./src, true) 

The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'UserService.RecoverTwoFactorAsync(User, string)' with a call to 'string.ToLower(CultureInfo)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1304)
{
return false;
}
Expand Down Expand Up @@ -1058,51 +1047,38 @@

private async Task CheckPoliciesOnTwoFactorRemovalAsync(User user)
{
if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements))
var requirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (!requirement.OrganizationsRequiringTwoFactor.Any())
{
var requirement = await _policyRequirementQuery.GetAsync<RequireTwoFactorPolicyRequirement>(user.Id);
if (!requirement.OrganizationsRequiringTwoFactor.Any())
{
Logger.LogInformation("No organizations requiring two factor for user {userId}.", user.Id);
return;
}

var organizationIds = requirement.OrganizationsRequiringTwoFactor.Select(o => o.OrganizationId).ToList();
var organizations = await _organizationRepository.GetManyByIdsAsync(organizationIds);
var organizationLookup = organizations.ToDictionary(org => org.Id);

var revokeOrgUserTasks = requirement.OrganizationsRequiringTwoFactor
.Where(o => organizationLookup.ContainsKey(o.OrganizationId))
.Select(async o =>
{
var organization = organizationLookup[o.OrganizationId];
await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
new RevokeOrganizationUsersRequest(
o.OrganizationId,
[new OrganizationUserUserDetails { Id = o.OrganizationUserId, OrganizationId = o.OrganizationId }],
new SystemUser(EventSystemUser.TwoFactorDisabled)));
await _mailService.SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization.DisplayName(), user.Email);
}).ToArray();

await Task.WhenAll(revokeOrgUserTasks);

Logger.LogInformation("No organizations requiring two factor for user {userId}.", user.Id);
return;
}

var twoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication);
var organizationIds = requirement.OrganizationsRequiringTwoFactor.Select(o => o.OrganizationId).ToList();
var organizations = await _organizationRepository.GetManyByIdsAsync(organizationIds);
var organizationLookup = organizations.ToDictionary(org => org.Id);

var legacyRevokeOrgUserTasks = twoFactorPolicies.Select(async p =>
{
var organization = await _organizationRepository.GetByIdAsync(p.OrganizationId);
await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
new RevokeOrganizationUsersRequest(
p.OrganizationId,
[new OrganizationUserUserDetails { Id = p.OrganizationUserId, OrganizationId = p.OrganizationId }],
new SystemUser(EventSystemUser.TwoFactorDisabled)));
await _mailService.SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization.DisplayName(), user.Email);
}).ToArray();

await Task.WhenAll(legacyRevokeOrgUserTasks);
var revokeOrgUserTasks = requirement.OrganizationsRequiringTwoFactor
.Where(o => organizationLookup.ContainsKey(o.OrganizationId))
.Select(async o =>
{
var organization = organizationLookup[o.OrganizationId];
await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
new RevokeOrganizationUsersRequest(
o.OrganizationId,
[
new OrganizationUserUserDetails
{
Id = o.OrganizationUserId,
OrganizationId = o.OrganizationId
}
],
new SystemUser(EventSystemUser.TwoFactorDisabled)));
await _mailService.SendOrganizationUserRevokedForTwoFactorPolicyEmailAsync(organization.DisplayName(),
user.Email);
}).ToArray();

await Task.WhenAll(revokeOrgUserTasks);
}

public async Task RotateApiKeyAsync(User user)
Expand Down Expand Up @@ -1159,14 +1135,14 @@

public async Task<bool> ActiveNewDeviceVerificationException(Guid userId)
{
var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString());

Check warning on line 1138 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Notifications, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ActiveNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)

Check warning on line 1138 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Events, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ActiveNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)

Check warning on line 1138 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Billing, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ActiveNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)
var cacheValue = await _distributedCache.GetAsync(cacheKey);
return cacheValue != null;
}

public async Task ToggleNewDeviceVerificationException(Guid userId)
{
var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString());

Check warning on line 1145 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Notifications, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ToggleNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)

Check warning on line 1145 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Events, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ToggleNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)

Check warning on line 1145 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Build Docker images (Billing, ./src, true) 

The behavior of 'string.Format(string, object)' could vary based on the current user's locale settings. Replace this call in 'UserService.ToggleNewDeviceVerificationException(Guid)' with a call to 'string.Format(IFormatProvider, string, params object[])'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305)
var cacheValue = await _distributedCache.GetAsync(cacheKey);
if (cacheValue != null)
{
Expand Down
Loading
Loading