diff --git a/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs b/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs index 537f9a61f5fd..703f3061019f 100644 --- a/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs +++ b/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs @@ -87,7 +87,7 @@ private async Task HandleOperationAsync(Group group, ScimPatchModel.OperationMod case PatchOps.Add when operation.Path?.ToLowerInvariant() == PatchPaths.Members: { - await AddMembersAsync(group, GetOperationValueIds(operation.Value).ToHashSet()); + await AddMembersAsync(group, GetOperationValueIds(operation.Value)); break; } @@ -122,20 +122,19 @@ private async Task HandleOperationAsync(Group group, ScimPatchModel.OperationMod } } - private async Task AddMembersAsync(Group group, HashSet usersToAdd) + private async Task AddMembersAsync(Group group, List usersToAdd) { // Azure Entra ID is known to send redundant "add" requests for each existing member every time any member - // is removed. To avoid excessive load on the database we detect these and return early. + // is removed. To avoid excessive load on the database, we check against the high availability replica and + // return early if they already exist. var groupMembers = await _groupRepository.GetManyUserIdsByIdAsync(group.Id, useReadOnlyReplica: true); - if (usersToAdd.IsSubsetOf(groupMembers)) + if (usersToAdd.ToHashSet().IsSubsetOf(groupMembers)) { _logger.LogDebug("Ignoring duplicate SCIM request to add members {Members} to group {Group}", usersToAdd, group.Id); return; } - // TODO: don't use the results of the previous query, just write a new upsert operation - var updatedMembers = groupMembers.Concat(usersToAdd).ToHashSet(); - await _groupRepository.UpdateUsersAsync(group.Id, updatedMembers); + await _groupRepository.AddGroupUsersByIdAsync(group.Id, usersToAdd); } private List GetOperationValueIds(JsonElement objArray) diff --git a/src/Core/AdminConsole/Repositories/IGroupRepository.cs b/src/Core/AdminConsole/Repositories/IGroupRepository.cs index 62cc5052e7e5..27a00e011d90 100644 --- a/src/Core/AdminConsole/Repositories/IGroupRepository.cs +++ b/src/Core/AdminConsole/Repositories/IGroupRepository.cs @@ -28,6 +28,13 @@ Task>>> GetManyW Task CreateAsync(Group obj, IEnumerable collections); Task ReplaceAsync(Group obj, IEnumerable collections); Task DeleteUserAsync(Guid groupId, Guid organizationUserId); + /// + /// Update a group's members. Replaces all members currently in the group. + /// Task UpdateUsersAsync(Guid groupId, IEnumerable organizationUserIds); + /// + /// Add members to a group. Ignores members that are already in the group. + /// + Task AddGroupUsersByIdAsync(Guid groupId, IEnumerable organizationUserIds); Task DeleteManyAsync(IEnumerable groupIds); } diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs index 6287507dc9da..ebac7eec952c 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs @@ -190,6 +190,17 @@ public async Task UpdateUsersAsync(Guid groupId, IEnumerable organizationU } } + public async Task AddGroupUsersByIdAsync(Guid groupId, IEnumerable organizationUserIds) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.ExecuteAsync( + "[dbo].[GroupUser_UpsertUsers]", + new { GroupId = groupId, OrganizationUserIds = organizationUserIds.ToGuidIdArrayTVP() }, + commandType: CommandType.StoredProcedure); + } + } + public async Task DeleteManyAsync(IEnumerable groupIds) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs index 35d486ca45b2..305a715d4c34 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs @@ -257,6 +257,29 @@ where organizationUserIds.Contains(ou.Id) && } } + public async Task AddGroupUsersByIdAsync(Guid groupId, IEnumerable organizationUserIds) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var orgId = (await dbContext.Groups.FindAsync(groupId)).OrganizationId; + var insert = from ou in dbContext.OrganizationUsers + where organizationUserIds.Contains(ou.Id) && + ou.OrganizationId == orgId && + !dbContext.GroupUsers.Any(gu => gu.GroupId == groupId && ou.Id == gu.OrganizationUserId) + select new GroupUser + { + GroupId = groupId, + OrganizationUserId = ou.Id, + }; + await dbContext.AddRangeAsync(insert); + + await dbContext.SaveChangesAsync(); + await dbContext.UserBumpAccountRevisionDateByOrganizationIdAsync(orgId); + await dbContext.SaveChangesAsync(); + } + } + public async Task DeleteManyAsync(IEnumerable groupIds) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/src/Sql/dbo/Stored Procedures/GroupUser_AddUsers.sql b/src/Sql/dbo/Stored Procedures/GroupUser_AddUsers.sql new file mode 100644 index 000000000000..1b6e799fee0e --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/GroupUser_AddUsers.sql @@ -0,0 +1,39 @@ +CREATE PROCEDURE [dbo].[GroupUser_UpdateUsers] + @GroupId UNIQUEIDENTIFIER, + @OrganizationUserIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DECLARE @OrgId UNIQUEIDENTIFIER = ( + SELECT TOP 1 + [OrganizationId] + FROM + [dbo].[Group] + WHERE + [Id] = @GroupId + ) + + -- Insert + INSERT INTO + [dbo].[GroupUser] + SELECT + @GroupId, + [Source].[Id] + FROM + @OrganizationUserIds AS [Source] + INNER JOIN + [dbo].[OrganizationUser] OU ON [Source].[Id] = OU.[Id] AND OU.[OrganizationId] = @OrgId + WHERE + NOT EXISTS ( + SELECT + 1 + FROM + [dbo].[GroupUser] + WHERE + [GroupId] = @GroupId + AND [OrganizationUserId] = [Source].[Id] + ) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId +END diff --git a/util/Migrator/DbScripts/2025-02-03_00_GroupUser_AddUsers.sql b/util/Migrator/DbScripts/2025-02-03_00_GroupUser_AddUsers.sql new file mode 100644 index 000000000000..e1baef23b7d3 --- /dev/null +++ b/util/Migrator/DbScripts/2025-02-03_00_GroupUser_AddUsers.sql @@ -0,0 +1,39 @@ +CREATE OR ALTER PROCEDURE [dbo].[GroupUser_UpdateUsers] + @GroupId UNIQUEIDENTIFIER, + @OrganizationUserIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DECLARE @OrgId UNIQUEIDENTIFIER = ( + SELECT TOP 1 + [OrganizationId] + FROM + [dbo].[Group] + WHERE + [Id] = @GroupId + ) + + -- Insert + INSERT INTO + [dbo].[GroupUser] + SELECT + @GroupId, + [Source].[Id] + FROM + @OrganizationUserIds AS [Source] + INNER JOIN + [dbo].[OrganizationUser] OU ON [Source].[Id] = OU.[Id] AND OU.[OrganizationId] = @OrgId + WHERE + NOT EXISTS ( + SELECT + 1 + FROM + [dbo].[GroupUser] + WHERE + [GroupId] = @GroupId + AND [OrganizationUserId] = [Source].[Id] + ) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId +END