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

[r-tome]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/AC-1698

📔 Objective

The members page loads slowly for large organizations because the database calls to check each user's 2FA status are not optimized.

Current code flow:
OrganizationUsersController.Get has a looped async call to userService.TwoFactorIsEnabledAsync (per user): https://github.com/bitwarden/server/blob/master/src/Api/Controllers/OrganizationUsersController.cs#L93-L94
that calls CanAccessPremium if they have a premium 2FA method (e.g. yubikey)
that calls hasPremiumFromOrganization
that calls the db to get every orgUser for that user
So for example, if an org has 500 users with yubikey enabled, it’ll hit the database 500 times for the 1 request.
We need a more efficient way to check this for many orgUsers at once, preferably starting with their common organization.

This PR improves performance by making a single database call to check 2FA status for multiple users at once.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[codecov]

Codecov Report

Attention: Patch coverage is 45.15366% with 232 lines in your changes missing coverage. Please review.

Project coverage is 41.83%. Comparing base (19dc7c3) to head (fa708e3).

Files	Patch %	Lines
...nConsole/Services/Implementations/PolicyService.cs	10.71%	49 Missing and 1 partial ⚠️
src/Core/Models/Data/UserWithCalculatedPremium.cs	4.25%	45 Missing ⚠️
...le/Services/Implementations/OrganizationService.cs	73.48%	28 Missing and 7 partials ⚠️
...Console/Controllers/OrganizationUsersController.cs	11.76%	29 Missing and 1 partial ⚠️
...ure.EntityFramework/Repositories/UserRepository.cs	0.00%	15 Missing ⚠️
src/Admin/Views/Users/Index.cshtml	0.00%	12 Missing ⚠️
...minConsole/Public/Controllers/MembersController.cs	45.45%	11 Missing and 1 partial ⚠️
...rFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs	85.00%	7 Missing and 5 partials ⚠️
src/Admin/Controllers/UsersController.cs	0.00%	11 Missing ⚠️
...frastructure.Dapper/Repositories/UserRepository.cs	0.00%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4524      +/-   ##
==========================================
+ Coverage   41.74%   41.83%   +0.08%     
==========================================
  Files        1286     1288       +2     
  Lines       60671    61076     +405     
  Branches     5566     5613      +47     
==========================================
+ Hits        25329    25552     +223     
- Misses      34160    34331     +171     
- Partials     1182     1193      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Checkmarx One – Scan Summary & Details – f620cde3-084d-4e22-ab90-3bd3c2623759

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	CSRF	/src/Api/Controllers/CollectionsController.cs: 143	Attack Vector
	CSRF	/src/Billing/Controllers/PayPalController.cs: 66	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/TwoFactorController.cs: 488	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/WebAuthnController.cs: 178	Attack Vector
	Privacy_Violation	/src/Api/Controllers/DevicesController.cs: 133	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 412	Attack Vector
	Privacy_Violation	/src/Api/Vault/Controllers/CiphersController.cs: 953	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 847	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 829	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 549	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 260	Attack Vector
	Privacy_Violation	/src/Api/Controllers/DevicesController.cs: 159	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 376	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 429	Attack Vector
	Privacy_Violation	/src/Core/Auth/Services/Implementations/AuthRequestService.cs: 147	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 68	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 153	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 85	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 839	Attack Vector
	Log_Forging	/src/Api/Controllers/DevicesController.cs: 124	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 541	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 821	Attack Vector
	Log_Forging	/src/Api/Vault/Controllers/CiphersController.cs: 945	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 404	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 240	Attack Vector
	Log_Forging	/src/Api/Controllers/DevicesController.cs: 150	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 341	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 404	Attack Vector
	Open_Redirect	/src/Admin/Auth/Controllers/LoginController.cs: 50	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	CSRF	/src/Billing/Controllers/StripeController.cs: 130
	CSRF	/src/Billing/Controllers/StripeController.cs: 117
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 89
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 217
	Privacy_Violation	/src/Core/Auth/Services/Implementations/AuthRequestService.cs: 158

[rkac-bw]

Not sure if in scope, but we have been moving away from using data types to pass in arrays, now we would pass in json :

CREATE OR ALTER PROCEDURE [dbo].[User_ReadByIdsWithCalculatedPremium]
    @Ids NVARCHAR(MAX)
AS
BEGIN
    SET NOCOUNT ON;


    -- Declare a table variable to hold the parsed JSON data
    DECLARE @ParsedIds TABLE (Id UNIQUEIDENTIFIER);

    -- Parse the JSON input into the table variable
    INSERT INTO @ParsedIds (Id)
    SELECT value
    FROM OPENJSON(@Ids);

    -- Check if the input table is empty
    IF (SELECT COUNT(1) FROM @ParsedIds) < 1
    BEGIN
        RETURN(-1);
    END

        -- Main query to fetch user details and calculate premium access
        SELECT
            U.*,
            CASE
                WHEN U.[Premium] = 1
                    OR EXISTS (
                        SELECT 1
                        FROM [dbo].[OrganizationUser] OU
                        JOIN [dbo].[Organization] O ON OU.[OrganizationId] = O.[Id]
                        WHERE OU.[UserId] = U.[Id]
                          AND O.[UsersGetPremium] = 1
                          AND O.[Enabled] = 1
                    )
                    THEN 1
                ELSE 0
            END AS HasPremiumAccess
        FROM
            [dbo].[UserView] U
        WHERE
            U.[Id] IN (SELECT [Id] FROM @ParsedIds);
    
END;`''''

[r-tome]

I've implemented that change. Thanks for providing the updated sproc!

[rkac-bw]

lgtm

[patrick-bitwarden]

Approving for AC after conflict resolution