[AC-1698] Check if a user has 2FA enabled more efficiently (bitwarden…

…#4524) * feat: Add stored procedure for reading organization user details with premium access by organization ID The code changes include: - Addition of a new stored procedure [dbo].[OrganizationUserUserDetailsWithPremiumAccess_ReadByOrganizationId] to read organization user details with premium access by organization ID - Modification of the IUserService interface to include an optional parameter for checking two-factor authentication with premium access - Modification of the UserService class to handle the new optional parameter in the TwoFactorIsEnabledAsync method - Addition of a new method GetManyDetailsWithPremiumAccessByOrganizationAsync in the IOrganizationUserRepository interface to retrieve organization user details with premium access by organization ID - Addition of a new view [dbo].[OrganizationUserUserDetailsWithPremiumAccessView] to retrieve organization user details with premium access * Add IUserRepository.SearchDetailsAsync that includes the field HasPremiumAccess * Check the feature flag on Admin.UsersController to see if the optimization runs * Modify PolicyService to run query optimization if the feature flag is enabled * Refactor the parameter check on UserService.TwoFactorIsEnabledAsync * Run query optimization on public MembersController if feature flag is enabled * Restore refactor * Reverted change used for development * Add unit tests for OrganizationService.RestoreUser * Separate new CheckPoliciesBeforeRestoreAsync optimization into new method * Add more unit tests * Apply refactor to bulk restore * Add GetManyDetailsAsync method to IUserRepository. Add ConfirmUsersAsync_vNext method to IOrganizationService * Add unit tests for ConfirmUser_vNext * Refactor the optimization to use the new TwoFactorIsEnabledAsync method instead of changing the existing one * Removed unused sql scripts and added migration script * Remove unnecessary view * chore: Remove unused SearchDetailsAsync method from IUserRepository and UserRepository * refactor: Use UserDetails constructor in UserRepository * Add summary to IUserRepository.GetManyDetailsAsync * Add summary descriptions to IUserService.TwoFactorIsEnabledAsync * Remove obsolete annotation from IUserRepository.UpdateUserKeyAndEncryptedDataAsync * refactor: Rename UserDetails to UserWithCalculatedPremium across the codebase * Extract IUserService.TwoFactorIsEnabledAsync into a new TwoFactorIsEnabledQuery class * Add unit tests for TwoFactorIsEnabledQuery * Update TwoFactorIsEnabledQueryTests to include additional provider types * Refactor TwoFactorIsEnabledQuery * Refactor TwoFactorIsEnabledQuery and update tests * refactor: Update TwoFactorIsEnabledQueryTests to include test for null TwoFactorProviders * refactor: Improve TwoFactorIsEnabledQuery and update tests * refactor: Improve TwoFactorIsEnabledQuery and update tests * Remove empty <returns> from summary * Update User_ReadByIdsWithCalculatedPremium stored procedure to accept JSON array of IDs
vgrassia · Aug 8, 2024 · 8d69bb0 · 8d69bb0
1 parent 19dc7c3
commit 8d69bb0
Show file tree

Hide file tree

Showing 21 changed files with 1,481 additions and 25 deletions.
diff --git a/src/Admin/Controllers/UsersController.cs b/src/Admin/Controllers/UsersController.cs
@@ -2,6 +2,9 @@
 using Bit.Admin.Models;
 using Bit.Admin.Services;
 using Bit.Admin.Utilities;
+using Bit.Core;
+using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
+using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
@@ -21,19 +24,28 @@ public class UsersController : Controller
     private readonly IPaymentService _paymentService;
     private readonly GlobalSettings _globalSettings;
     private readonly IAccessControlService _accessControlService;
+    private readonly ICurrentContext _currentContext;
+    private readonly IFeatureService _featureService;
+    private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
 
     public UsersController(
         IUserRepository userRepository,
         ICipherRepository cipherRepository,
         IPaymentService paymentService,
         GlobalSettings globalSettings,
-        IAccessControlService accessControlService)
+        IAccessControlService accessControlService,
+        ICurrentContext currentContext,
+        IFeatureService featureService,
+        ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery)
     {
         _userRepository = userRepository;
         _cipherRepository = cipherRepository;
         _paymentService = paymentService;
         _globalSettings = globalSettings;
         _accessControlService = accessControlService;
+        _currentContext = currentContext;
+        _featureService = featureService;
+        _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
     }
 
     [RequirePermission(Permission.User_List_View)]
@@ -51,6 +63,12 @@ public async Task<IActionResult> Index(string email, int page = 1, int count = 2
 
         var skip = (page - 1) * count;
         var users = await _userRepository.SearchAsync(email, skip, count);
+
+        if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization))
+        {
+            TempData["UsersTwoFactorIsEnabled"] = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(users.Select(u => u.Id));
+        }
+
         return View(new UsersModel
         {
             Items = users as List<User>,

diff --git a/src/Admin/Views/Users/Index.cshtml b/src/Admin/Views/Users/Index.cshtml
@@ -1,5 +1,6 @@
 @model UsersModel
 @inject Bit.Core.Services.IUserService userService
+@inject Bit.Core.Services.IFeatureService featureService
 @{
     ViewData["Title"] = "Users";
 }
@@ -69,13 +70,28 @@
                             {
                                 <i class="fa fa-times-circle-o fa-lg fa-fw text-muted" title="Email Not Verified"></i>
                             }
-                            @if(await userService.TwoFactorIsEnabledAsync(user))
+                            @if (featureService.IsEnabled(Bit.Core.FeatureFlagKeys.MembersTwoFAQueryOptimization))
                             {
-                                <i class="fa fa-lock fa-lg fa-fw" title="2FA Enabled"></i>
+                                var usersTwoFactorIsEnabled = TempData["UsersTwoFactorIsEnabled"] as IEnumerable<(Guid userId, bool twoFactorIsEnabled)>;
+                                @if(usersTwoFactorIsEnabled.FirstOrDefault(tuple => tuple.userId == user.Id).twoFactorIsEnabled)
+                                {
+                                    <i class="fa fa-lock fa-lg fa-fw" title="2FA Enabled"></i>
+                                }
+                                else
+                                {
+                                    <i class="fa fa-unlock fa-lg fa-fw text-muted" title="2FA Not Enabled"></i>
+                                }
                             }
                             else
                             {
-                                <i class="fa fa-unlock fa-lg fa-fw text-muted" title="2FA Not Enabled"></i>
+                                @if(await userService.TwoFactorIsEnabledAsync(user))
+                                {
+                                    <i class="fa fa-lock fa-lg fa-fw" title="2FA Enabled"></i>
+                                }
+                                else
+                                {
+                                    <i class="fa fa-unlock fa-lg fa-fw text-muted" title="2FA Not Enabled"></i>
+                                }
                             }
                         </td>
                     </tr>

diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
@@ -12,6 +12,7 @@
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.Auth.Enums;
 using Bit.Core.Auth.Repositories;
+using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
 using Bit.Core.Context;
 using Bit.Core.Enums;
 using Bit.Core.Exceptions;
@@ -48,6 +49,7 @@ public class OrganizationUsersController : Controller
     private readonly IApplicationCacheService _applicationCacheService;
     private readonly IFeatureService _featureService;
     private readonly ISsoConfigRepository _ssoConfigRepository;
+    private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
 
     public OrganizationUsersController(
         IOrganizationRepository organizationRepository,
@@ -66,7 +68,8 @@ public OrganizationUsersController(
         IAuthorizationService authorizationService,
         IApplicationCacheService applicationCacheService,
         IFeatureService featureService,
-        ISsoConfigRepository ssoConfigRepository)
+        ISsoConfigRepository ssoConfigRepository,
+        ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery)
     {
         _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
@@ -85,6 +88,7 @@ public OrganizationUsersController(
         _applicationCacheService = applicationCacheService;
         _featureService = featureService;
         _ssoConfigRepository = ssoConfigRepository;
+        _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
     }
 
     [HttpGet("{id}")]
@@ -126,8 +130,12 @@ public async Task<ListResponseModel<OrganizationUserUserDetailsResponseModel>> G
             throw new NotFoundException();
         }
 
-        var organizationUsers = await _organizationUserRepository
-            .GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections);
+        if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization))
+        {
+            return await Get_vNext(orgId, includeGroups, includeCollections);
+        }
+
+        var organizationUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections);
         var responseTasks = organizationUsers
             .Select(async o =>
             {
@@ -332,7 +340,9 @@ public async Task<ListResponseModel<OrganizationUserBulkResponseModel>> BulkConf
         }
 
         var userId = _userService.GetProperUserId(User);
-        var results = await _organizationService.ConfirmUsersAsync(orgGuidId, model.ToDictionary(), userId.Value,
+        var results = _featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)
+            ? await _organizationService.ConfirmUsersAsync_vNext(orgGuidId, model.ToDictionary(), userId.Value)
+            : await _organizationService.ConfirmUsersAsync(orgGuidId, model.ToDictionary(), userId.Value,
             _userService);
 
         return new ListResponseModel<OrganizationUserBulkResponseModel>(results.Select(r =>
@@ -681,4 +691,32 @@ permissions is
 
         return type;
     }
+
+    private async Task<ListResponseModel<OrganizationUserUserDetailsResponseModel>> Get_vNext(Guid orgId,
+        bool includeGroups = false, bool includeCollections = false)
+    {
+        var organizationUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(orgId, includeGroups, includeCollections);
+        var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers);
+        var responseTasks = organizationUsers
+            .Select(async o =>
+            {
+                var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == o.Id).twoFactorIsEnabled;
+                var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled);
+
+                // Downgrade Custom users with no other permissions than 'Edit/Delete Assigned Collections' to User
+                orgUser.Type = GetFlexibleCollectionsUserType(orgUser.Type, orgUser.Permissions);
+
+                // Set 'Edit/Delete Assigned Collections' custom permissions to false
+                if (orgUser.Permissions is not null)
+                {
+                    orgUser.Permissions.EditAssignedCollections = false;
+                    orgUser.Permissions.DeleteAssignedCollections = false;
+                }
+
+                return orgUser;
+            });
+        var responses = await Task.WhenAll(responseTasks);
+
+        return new ListResponseModel<OrganizationUserUserDetailsResponseModel>(responses);
+    }
 }
diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs
@@ -2,9 +2,12 @@
 using Bit.Api.AdminConsole.Public.Models.Request;
 using Bit.Api.AdminConsole.Public.Models.Response;
 using Bit.Api.Models.Public.Response;
+using Bit.Core;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
 using Bit.Core.AdminConsole.Repositories;
+using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
 using Bit.Core.Context;
+using Bit.Core.Models.Data.Organizations.OrganizationUsers;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
 using Microsoft.AspNetCore.Authorization;
@@ -26,6 +29,8 @@ public class MembersController : Controller
     private readonly IApplicationCacheService _applicationCacheService;
     private readonly IPaymentService _paymentService;
     private readonly IOrganizationRepository _organizationRepository;
+    private readonly IFeatureService _featureService;
+    private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
 
     public MembersController(
         IOrganizationUserRepository organizationUserRepository,
@@ -37,7 +42,9 @@ public MembersController(
         IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand,
         IApplicationCacheService applicationCacheService,
         IPaymentService paymentService,
-        IOrganizationRepository organizationRepository)
+        IOrganizationRepository organizationRepository,
+        IFeatureService featureService,
+        ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery)
     {
         _organizationUserRepository = organizationUserRepository;
         _groupRepository = groupRepository;
@@ -49,6 +56,8 @@ public MembersController(
         _applicationCacheService = applicationCacheService;
         _paymentService = paymentService;
         _organizationRepository = organizationRepository;
+        _featureService = featureService;
+        _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
     }
 
     /// <summary>
@@ -108,11 +117,18 @@ public async Task<IActionResult> GetGroupIds(Guid id)
     [ProducesResponseType(typeof(ListResponseModel<MemberResponseModel>), (int)HttpStatusCode.OK)]
     public async Task<IActionResult> List()
     {
-        var users = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(
-            _currentContext.OrganizationId.Value);
+        var organizationUserUserDetails = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(_currentContext.OrganizationId.Value);
         // TODO: Get all CollectionUser associations for the organization and marry them up here for the response.
-        var memberResponsesTasks = users.Select(async u => new MemberResponseModel(u,
-            await _userService.TwoFactorIsEnabledAsync(u), null));
+
+        if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization))
+        {
+            return await List_vNext(organizationUserUserDetails);
+        }
+
+        var memberResponsesTasks = organizationUserUserDetails.Select(async u =>
+        {
+            return new MemberResponseModel(u, await _userService.TwoFactorIsEnabledAsync(u), null);
+        });
         var memberResponses = await Task.WhenAll(memberResponsesTasks);
         var response = new ListResponseModel<MemberResponseModel>(memberResponses);
         return new JsonResult(response);
@@ -252,4 +268,15 @@ public async Task<IActionResult> PostReinvite(Guid id)
         await _organizationService.ResendInviteAsync(_currentContext.OrganizationId.Value, null, id);
         return new OkResult();
     }
+
+    private async Task<JsonResult> List_vNext(ICollection<OrganizationUserUserDetails> organizationUserUserDetails)
+    {
+        var orgUsersTwoFactorIsEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUserUserDetails);
+        var memberResponses = organizationUserUserDetails.Select(u =>
+        {
+            return new MemberResponseModel(u, orgUsersTwoFactorIsEnabled.FirstOrDefault(tuple => tuple.user == u).twoFactorIsEnabled, null);
+        });
+        var response = new ListResponseModel<MemberResponseModel>(memberResponses);
+        return new JsonResult(response);
+    }
 }
diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs
@@ -53,6 +53,8 @@ Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid organizationUs
         Guid confirmingUserId, IUserService userService);
     Task<List<Tuple<OrganizationUser, string>>> ConfirmUsersAsync(Guid organizationId, Dictionary<Guid, string> keys,
         Guid confirmingUserId, IUserService userService);
+    Task<List<Tuple<OrganizationUser, string>>> ConfirmUsersAsync_vNext(Guid organizationId, Dictionary<Guid, string> keys,
+        Guid confirmingUserId);
     [Obsolete("IDeleteOrganizationUserCommand should be used instead. To be removed by EC-607.")]
     Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId);
     [Obsolete("IDeleteOrganizationUserCommand should be used instead. To be removed by EC-607.")]