bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2025-12-24 04:03:25 +00:00
Code Issues Releases Wiki Activity

[PM-21031] Optimize GET Members endpoint performance (#5907)

Browse Source 
* Add new feature flag for Members Get Endpoint Optimization

* Add a new version of OrganizationUser_ReadByOrganizationIdWithClaimedDomains that uses CTE for better performance

* Add stored procedure OrganizationUserUserDetails_ReadByOrganizationId_V2 for retrieving user details, group associations, and collection associations by organization ID.

* Add the sql migration script to add the new stored procedures

* Introduce GetManyDetailsByOrganizationAsync_vNext and GetManyByOrganizationWithClaimedDomainsAsync_vNext in IOrganizationUserRepository to enhance performance by reducing database round trips.

* Updated GetOrganizationUsersClaimedStatusQuery to use an optimized query when the feature flag is enabled

* Updated OrganizationUserUserDetailsQuery to use optimized queries when the feature flag is enabled

* Add integration tests for GetManyDetailsByOrganizationAsync_vNext

* Add integration tests for GetManyByOrganizationWithClaimedDomainsAsync_vNext to validate behavior with verified and unverified domains.

* Optimize performance by conditionally setting permissions only for Custom user types in OrganizationUserUserDetailsQuery.

* Create UserEmailDomainView to extract email domains from users' email addresses

* Create stored procedure Organization_ReadByClaimedUserEmailDomain_V2 that uses UserEmailDomainView to fetch Email domains

* Add GetByVerifiedUserEmailDomainAsync_vNext method to IOrganizationRepository and its implementations

* Refactor OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2 stored procedure to use UserEmailDomainView for email domain extraction, improving query efficiency and clarity.

* Enhance IOrganizationUserRepository with detailed documentation for GetManyDetailsByOrganizationAsync method, clarifying its purpose and performance optimizations. Added remarks for better understanding of its functionality.

* Fix missing newline at the end of Organization_ReadByClaimedUserEmailDomain_V2.sql to adhere to coding standards.

* Update the database migration script to include UserEmailDomainView

* Bumped the date on the migration script

* Remove GetByVerifiedUserEmailDomainAsync_vNext method and its stored procedure.

* Refactor UserEmailDomainView index creation to check for existence before creation

* Update OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2 to use CTE and add indexes

* Remove creation of unique clustered index from UserEmailDomainView and related migration script adjustments

* Update indexes and sproc

* Fix index name when checking if it already exists

* Bump up date on migration script
This commit is contained in:
Rui Tomé
2025-07-23 10:04:20 +01:00
committed by GitHub
parent 947ae8db51
commit acd556d56f
14 changed files with 836 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/GetOrganizationUsersClaimedStatusQuery.cs
View File

@@ -8,13 +8,16 @@ public class GetOrganizationUsersClaimedStatusQuery : IGetOrganizationUsersClaim
{
private readonly IApplicationCacheService _applicationCacheService;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IFeatureService _featureService;
public GetOrganizationUsersClaimedStatusQuery(
IApplicationCacheService applicationCacheService,
IOrganizationUserRepository organizationUserRepository)
IOrganizationUserRepository organizationUserRepository,
IFeatureService featureService)
{
_applicationCacheService = applicationCacheService;
_organizationUserRepository = organizationUserRepository;
_featureService = featureService;
}
public async Task<IDictionary<Guid, bool>> GetUsersOrganizationClaimedStatusAsync(Guid organizationId, IEnumerable<Guid> organizationUserIds)
@@ -27,7 +30,9 @@ public class GetOrganizationUsersClaimedStatusQuery : IGetOrganizationUsersClaim
if (organizationAbility is { Enabled: true, UseOrganizationDomains: true })
{
// Get all organization users with claimed domains by the organization
var organizationUsersWithClaimedDomain = await _organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organizationId);
var organizationUsersWithClaimedDomain = _featureService.IsEnabled(FeatureFlagKeys.MembersGetEndpointOptimization)
? await _organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync_vNext(organizationId)
: await _organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organizationId);
// Create a dictionary with the OrganizationUserId and a boolean indicating if the user is claimed by the organization
return organizationUserIds.ToDictionary(ouId => ouId, ouId => organizationUsersWithClaimedDomain.Any(ou => ou.Id == ouId));

83
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/OrganizationUserUserDetailsQuery.cs
View File

@@ -1,4 +1,5 @@
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
@@ -43,9 +44,12 @@ public class OrganizationUserUserDetailsQuery : IOrganizationUserUserDetailsQuer
return organizationUsers
.Select(o =>
{
var userPermissions = o.GetPermissions();
o.Permissions = CoreHelpers.ClassToJsonData(userPermissions);
// Only set permissions for Custom user types for performance optimization
if (o.Type == OrganizationUserType.Custom)
{
var userPermissions = o.GetPermissions();
o.Permissions = CoreHelpers.ClassToJsonData(userPermissions);
}
return o;
});
@@ -59,6 +63,11 @@ public class OrganizationUserUserDetailsQuery : IOrganizationUserUserDetailsQuer
/// <returns>List of OrganizationUserUserDetails</returns>
public async Task<IEnumerable<(OrganizationUserUserDetails OrgUser, bool TwoFactorEnabled, bool ClaimedByOrganization)>> Get(OrganizationUserUserDetailsQueryRequest request)
{
if (_featureService.IsEnabled(FeatureFlagKeys.MembersGetEndpointOptimization))
{
return await Get_vNext(request);
}
var organizationUsers = await GetOrganizationUserUserDetails(request);
var organizationUsersTwoFactorEnabled = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers)).ToDictionary(u => u.user.Id);
@@ -77,6 +86,11 @@ public class OrganizationUserUserDetailsQuery : IOrganizationUserUserDetailsQuer
/// <returns>List of OrganizationUserUserDetails</returns>
public async Task<IEnumerable<(OrganizationUserUserDetails OrgUser, bool TwoFactorEnabled, bool ClaimedByOrganization)>> GetAccountRecoveryEnrolledUsers(OrganizationUserUserDetailsQueryRequest request)
{
if (_featureService.IsEnabled(FeatureFlagKeys.MembersGetEndpointOptimization))
{
return await GetAccountRecoveryEnrolledUsers_vNext(request);
}
var organizationUsers = (await GetOrganizationUserUserDetails(request))
.Where(o => o.Status.Equals(OrganizationUserStatusType.Confirmed) && o.UsesKeyConnector == false && !String.IsNullOrEmpty(o.ResetPasswordKey));
@@ -88,4 +102,65 @@ public class OrganizationUserUserDetailsQuery : IOrganizationUserUserDetailsQuer
return responses;
}
private async Task<IEnumerable<(OrganizationUserUserDetails OrgUser, bool TwoFactorEnabled, bool ClaimedByOrganization)>> Get_vNext(OrganizationUserUserDetailsQueryRequest request)
{
var organizationUsers = await _organizationUserRepository
.GetManyDetailsByOrganizationAsync_vNext(request.OrganizationId, request.IncludeGroups, request.IncludeCollections);
var twoFactorTask = _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers);
var claimedStatusTask = _getOrganizationUsersClaimedStatusQuery.GetUsersOrganizationClaimedStatusAsync(request.OrganizationId, organizationUsers.Select(o => o.Id));
await Task.WhenAll(twoFactorTask, claimedStatusTask);
var organizationUsersTwoFactorEnabled = twoFactorTask.Result.ToDictionary(u => u.user.Id, u => u.twoFactorIsEnabled);
var organizationUsersClaimedStatus = claimedStatusTask.Result;
var responses = organizationUsers.Select(organizationUserDetails =>
{
// Only set permissions for Custom user types for performance optimization
if (organizationUserDetails.Type == OrganizationUserType.Custom)
{
var organizationUserPermissions = organizationUserDetails.GetPermissions();
organizationUserDetails.Permissions = CoreHelpers.ClassToJsonData(organizationUserPermissions);
}
var userHasTwoFactorEnabled = organizationUsersTwoFactorEnabled[organizationUserDetails.Id];
var userIsClaimedByOrganization = organizationUsersClaimedStatus[organizationUserDetails.Id];
return (organizationUserDetails, userHasTwoFactorEnabled, userIsClaimedByOrganization);
});
return responses;
}
private async Task<IEnumerable<(OrganizationUserUserDetails OrgUser, bool TwoFactorEnabled, bool ClaimedByOrganization)>> GetAccountRecoveryEnrolledUsers_vNext(OrganizationUserUserDetailsQueryRequest request)
{
var organizationUsers = (await _organizationUserRepository
.GetManyDetailsByOrganizationAsync_vNext(request.OrganizationId, request.IncludeGroups, request.IncludeCollections))
.Where(o => o.Status.Equals(OrganizationUserStatusType.Confirmed) && o.UsesKeyConnector == false && !String.IsNullOrEmpty(o.ResetPasswordKey))
.ToArray();
var twoFactorTask = _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers);
var claimedStatusTask = _getOrganizationUsersClaimedStatusQuery.GetUsersOrganizationClaimedStatusAsync(request.OrganizationId, organizationUsers.Select(o => o.Id));
await Task.WhenAll(twoFactorTask, claimedStatusTask);
var organizationUsersTwoFactorEnabled = twoFactorTask.Result.ToDictionary(u => u.user.Id, u => u.twoFactorIsEnabled);
var organizationUsersClaimedStatus = claimedStatusTask.Result;
var responses = organizationUsers.Select(organizationUserDetails =>
{
// Only set permissions for Custom user types for performance optimization
if (organizationUserDetails.Type == OrganizationUserType.Custom)
{
var organizationUserPermissions = organizationUserDetails.GetPermissions();
organizationUserDetails.Permissions = CoreHelpers.ClassToJsonData(organizationUserPermissions);
}
var userHasTwoFactorEnabled = organizationUsersTwoFactorEnabled[organizationUserDetails.Id];
var userIsClaimedByOrganization = organizationUsersClaimedStatus[organizationUserDetails.Id];
return (organizationUserDetails, userHasTwoFactorEnabled, userIsClaimedByOrganization);
});
return responses;
}
}

11
src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs
View File

@@ -36,6 +36,12 @@ public interface IOrganizationUserRepository : IRepository<OrganizationUser, Gui
/// <param name="includeCollections">Whether to include collections</param>
/// <returns>A list of OrganizationUserUserDetails</returns>
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups = false, bool includeCollections = false);
/// <inheritdoc cref="GetManyDetailsByOrganizationAsync"/>
/// <remarks>
/// This method is optimized for performance.
/// Reduces database round trips by fetching all data in fewer queries.
/// </remarks>
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups = false, bool includeCollections = false);
Task<ICollection<OrganizationUserOrganizationDetails>> GetManyDetailsByUserAsync(Guid userId,
OrganizationUserStatusType? status = null);
Task<OrganizationUserOrganizationDetails?> GetDetailsByUserAsync(Guid userId, Guid organizationId,
@@ -70,7 +76,10 @@ public interface IOrganizationUserRepository : IRepository<OrganizationUser, Gui
/// Returns a list of OrganizationUsers with email domains that match one of the Organization's claimed domains.
/// </summary>
Task<ICollection<OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync(Guid organizationId);
/// <summary>
/// Optimized version of <see cref="GetManyByOrganizationWithClaimedDomainsAsync"/> with better performance.
/// </summary>
Task<ICollection<OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync_vNext(Guid organizationId);
Task RevokeManyByIdAsync(IEnumerable<Guid> organizationUserIds);
/// <summary>

1
src/Core/Constants.cs
View File

@@ -115,6 +115,7 @@ public static class FeatureFlagKeys
public const string SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions";
public const string ImportAsyncRefactor = "pm-22583-refactor-import-async";
public const string CreateDefaultLocation = "pm-19467-create-default-location";
public const string MembersGetEndpointOptimization = "pm-21031-members-get-endpoint-optimization";
/* Auth Team */
public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence";

75
src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs
View File

@@ -268,6 +268,68 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
}
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups, bool includeCollections)
{
using (var connection = new SqlConnection(ConnectionString))
{
// Use a single call that returns multiple result sets
var results = await connection.QueryMultipleAsync(
"[dbo].[OrganizationUserUserDetails_ReadByOrganizationId_V2]",
new
{
OrganizationId = organizationId,
IncludeGroups = includeGroups,
IncludeCollections = includeCollections
},
commandType: CommandType.StoredProcedure);
// Read the user details (first result set)
var users = (await results.ReadAsync<OrganizationUserUserDetails>()).ToList();
// Read group associations (second result set, if requested)
Dictionary<Guid, List<Guid>>? userGroupMap = null;
if (includeGroups)
{
var groupUsers = await results.ReadAsync<GroupUser>();
userGroupMap = groupUsers
.GroupBy(gu => gu.OrganizationUserId)
.ToDictionary(g => g.Key, g => g.Select(gu => gu.GroupId).ToList());
}
// Read collection associations (third result set, if requested)
Dictionary<Guid, List<CollectionAccessSelection>>? userCollectionMap = null;
if (includeCollections)
{
var collectionUsers = await results.ReadAsync<CollectionUser>();
userCollectionMap = collectionUsers
.GroupBy(cu => cu.OrganizationUserId)
.ToDictionary(g => g.Key, g => g.Select(cu => new CollectionAccessSelection
{
Id = cu.CollectionId,
ReadOnly = cu.ReadOnly,
HidePasswords = cu.HidePasswords,
Manage = cu.Manage
}).ToList());
}
// Map the associations to users
foreach (var user in users)
{
if (userGroupMap != null)
{
user.Groups = userGroupMap.GetValueOrDefault(user.Id, new List<Guid>());
}
if (userCollectionMap != null)
{
user.Collections = userCollectionMap.GetValueOrDefault(user.Id, new List<CollectionAccessSelection>());
}
}
return users;
}
}
public async Task<ICollection<OrganizationUserOrganizationDetails>> GetManyDetailsByUserAsync(Guid userId,
OrganizationUserStatusType? status = null)
{
@@ -558,6 +620,19 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
}
}
public async Task<ICollection<OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync_vNext(Guid organizationId)
{
using (var connection = new SqlConnection(ConnectionString))
{
var results = await connection.QueryAsync<OrganizationUser>(
$"[{Schema}].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2]",
new { OrganizationId = organizationId },
commandType: CommandType.StoredProcedure);
return results.ToList();
}
}
public async Task RevokeManyByIdAsync(IEnumerable<Guid> organizationUserIds)
{
await using var connection = new SqlConnection(ConnectionString);

56
src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs
View File

@@ -404,6 +404,56 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
}
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(
Guid organizationId, bool includeGroups, bool includeCollections)
{
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);
var query = from ou in dbContext.OrganizationUsers
where ou.OrganizationId == organizationId
select new OrganizationUserUserDetails
{
Id = ou.Id,
UserId = ou.UserId,
OrganizationId = ou.OrganizationId,
Name = ou.User.Name,
Email = ou.User.Email ?? ou.Email,
AvatarColor = ou.User.AvatarColor,
TwoFactorProviders = ou.User.TwoFactorProviders,
Premium = ou.User.Premium,
Status = ou.Status,
Type = ou.Type,
ExternalId = ou.ExternalId,
SsoExternalId = ou.User.SsoUsers
.Where(su => su.OrganizationId == ou.OrganizationId)
.Select(su => su.ExternalId)
.FirstOrDefault(),
Permissions = ou.Permissions,
ResetPasswordKey = ou.ResetPasswordKey,
UsesKeyConnector = ou.User != null && ou.User.UsesKeyConnector,
AccessSecretsManager = ou.AccessSecretsManager,
HasMasterPassword = ou.User != null && !string.IsNullOrWhiteSpace(ou.User.MasterPassword),
// Project directly from navigation properties with conditional loading
Groups = includeGroups
? ou.GroupUsers.Select(gu => gu.GroupId).ToList()
: new List<Guid>(),
Collections = includeCollections
? ou.CollectionUsers.Select(cu => new CollectionAccessSelection
{
Id = cu.CollectionId,
ReadOnly = cu.ReadOnly,
HidePasswords = cu.HidePasswords,
Manage = cu.Manage
}).ToList()
: new List<CollectionAccessSelection>()
};
return await query.ToListAsync();
}
public async Task<ICollection<OrganizationUserOrganizationDetails>> GetManyDetailsByUserAsync(Guid userId,
OrganizationUserStatusType? status = null)
{
@@ -732,6 +782,12 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
}
}
public async Task<ICollection<Core.Entities.OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync_vNext(Guid organizationId)
{
// No EF optimization is required for this query
return await GetManyByOrganizationWithClaimedDomainsAsync(organizationId);
}
public async Task RevokeManyByIdAsync(IEnumerable<Guid> organizationUserIds)
{
using var scope = ServiceScopeFactory.CreateScope();

31
src/Sql/dbo/Stored Procedures/OrganizationUserUserDetails_ReadByOrganizationId_V2.sql Normal file
View File

@@ -0,0 +1,31 @@
CREATE PROCEDURE [dbo].[OrganizationUserUserDetails_ReadByOrganizationId_V2]
@OrganizationId UNIQUEIDENTIFIER,
@IncludeGroups BIT = 0,
@IncludeCollections BIT = 0
AS
BEGIN
SET NOCOUNT ON
-- Result Set 1: User Details (always returned)
SELECT *
FROM [dbo].[OrganizationUserUserDetailsView]
WHERE OrganizationId = @OrganizationId
-- Result Set 2: Group associations (if requested)
IF @IncludeGroups = 1
BEGIN
SELECT gu.*
FROM [dbo].[GroupUser] gu
INNER JOIN [dbo].[OrganizationUser] ou ON gu.OrganizationUserId = ou.Id
WHERE ou.OrganizationId = @OrganizationId
END
-- Result Set 3: Collection associations (if requested)
IF @IncludeCollections = 1
BEGIN
SELECT cu.*
FROM [dbo].[CollectionUser] cu
INNER JOIN [dbo].[OrganizationUser] ou ON cu.OrganizationUserId = ou.Id
WHERE ou.OrganizationId = @OrganizationId
END
END

27
src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2.sql Normal file
View File

@@ -0,0 +1,27 @@
CREATE PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2]
@OrganizationId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;
WITH OrgUsers AS (
SELECT *
FROM [dbo].[OrganizationUserView]
WHERE [OrganizationId] = @OrganizationId
),
UserDomains AS (
SELECT U.[Id], U.[EmailDomain]
FROM [dbo].[UserEmailDomainView] U
WHERE EXISTS (
SELECT 1
FROM [dbo].[OrganizationDomainView] OD
WHERE OD.[OrganizationId] = @OrganizationId
AND OD.[VerifiedDate] IS NOT NULL
AND OD.[DomainName] = U.[EmailDomain]
)
)
SELECT OU.*
FROM OrgUsers OU
JOIN UserDomains UD ON OU.[UserId] = UD.[Id]
OPTION (RECOMPILE);
END

8
src/Sql/dbo/Tables/OrganizationDomain.sql
View File

@@ -25,5 +25,11 @@ GO
CREATE NONCLUSTERED INDEX [IX_OrganizationDomain_DomainNameVerifiedDateOrganizationId]
ON [dbo].[OrganizationDomain] ([DomainName],[VerifiedDate])
INCLUDE ([OrganizationId])
INCLUDE ([OrganizationId]);
GO
CREATE NONCLUSTERED INDEX [IX_OrganizationDomain_OrganizationId_VerifiedDate]
ON [dbo].[OrganizationDomain] ([OrganizationId], [VerifiedDate])
INCLUDE ([DomainName])
WHERE [VerifiedDate] IS NOT NULL;
GO

7
src/Sql/dbo/Tables/OrganizationUser.sql
View File

@@ -27,3 +27,10 @@ GO
CREATE NONCLUSTERED INDEX [IX_OrganizationUser_OrganizationId]
ON [dbo].[OrganizationUser]([OrganizationId] ASC);
GO
CREATE NONCLUSTERED INDEX [IX_OrganizationUser_OrganizationId_UserId]
ON [dbo].[OrganizationUser] ([OrganizationId], [UserId])
INCLUDE ([Email], [Status], [Type], [ExternalId], [CreationDate],
[RevisionDate], [Permissions], [ResetPasswordKey], [AccessSecretsManager]);
GO

4
src/Sql/dbo/Tables/User.sql
View File

@@ -54,3 +54,7 @@ GO
CREATE NONCLUSTERED INDEX [IX_User_Premium_PremiumExpirationDate_RenewalReminderDate]
ON [dbo].[User]([Premium] ASC, [PremiumExpirationDate] ASC, [RenewalReminderDate] ASC);
GO
CREATE NONCLUSTERED INDEX [IX_User_Id_EmailDomain]
ON [dbo].[User]([Id] ASC, [Email] ASC);

10
src/Sql/dbo/Views/UserEmailDomainView.sql Normal file
View File

@@ -0,0 +1,10 @@
CREATE VIEW [dbo].[UserEmailDomainView]
AS
SELECT
Id,
Email,
SUBSTRING(Email, CHARINDEX('@', Email) + 1, LEN(Email)) AS EmailDomain
FROM dbo.[User]
WHERE Email IS NOT NULL
AND CHARINDEX('@', Email) > 0
GO