From 54d59b3b92711cd4263f85ef6a18c9c9652e39b3 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 14 Feb 2025 11:09:01 +1000 Subject: [PATCH] [PM-16812] Shortcut duplicate group patch requests (#5354) * Copy PatchGroupCommand to vNext and refactor * Detect duplicate add requests and return early * Update read repository method to use HA replica * Add new write repository method --- .../Scim/Controllers/v2/GroupsController.cs | 27 +- .../Interfaces/IPatchGroupCommandvNext.cs | 9 + .../src/Scim/Groups/PatchGroupCommandvNext.cs | 170 ++++++++ .../src/Scim/Utilities/ScimConstants.cs | 13 + .../ScimServiceCollectionExtensions.cs | 1 + .../v2/GroupsControllerPatchTests.cs | 237 +++++++++++ .../v2/GroupsControllerPatchTestsvNext.cs | 251 ++++++++++++ .../Controllers/v2/GroupsControllerTests.cs | 221 +--------- .../Factories/ScimApplicationFactory.cs | 40 +- .../Groups/PatchGroupCommandvNextTests.cs | 381 ++++++++++++++++++ .../Repositories/IGroupRepository.cs | 20 +- src/Core/Constants.cs | 1 + .../Repositories/GroupRepository.cs | 19 +- .../Repositories/GroupRepository.cs | 27 +- .../Stored Procedures/GroupUser_AddUsers.sql | 39 ++ .../AdminConsole/OrganizationTestHelpers.cs | 57 +++ .../Repositories/GroupRepositoryTests.cs | 129 ++++++ .../OrganizationDomainRepositoryTests.cs | 2 +- .../OrganizationRepositoryTests.cs | 2 +- .../OrganizationUserRepositoryTests.cs | 2 +- .../2025-02-13_00_GroupUser_AddUsers.sql | 39 ++ 21 files changed, 1437 insertions(+), 250 deletions(-) create mode 100644 bitwarden_license/src/Scim/Groups/Interfaces/IPatchGroupCommandvNext.cs create mode 100644 bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs create mode 100644 bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTests.cs create mode 100644 bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTestsvNext.cs create mode 100644 bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandvNextTests.cs create mode 100644 src/Sql/dbo/Stored Procedures/GroupUser_AddUsers.sql create mode 100644 test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs create mode 100644 test/Infrastructure.IntegrationTest/AdminConsole/Repositories/GroupRepositoryTests.cs create mode 100644 util/Migrator/DbScripts/2025-02-13_00_GroupUser_AddUsers.sql diff --git a/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs b/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs index 5df0b29216..8c21793a9d 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs @@ -1,8 +1,10 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; +using Bit.Core; +using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Scim.Groups.Interfaces; using Bit.Scim.Models; using Bit.Scim.Utilities; @@ -22,9 +24,10 @@ public class GroupsController : Controller private readonly IGetGroupsListQuery _getGroupsListQuery; private readonly IDeleteGroupCommand _deleteGroupCommand; private readonly IPatchGroupCommand _patchGroupCommand; + private readonly IPatchGroupCommandvNext _patchGroupCommandvNext; private readonly IPostGroupCommand _postGroupCommand; private readonly IPutGroupCommand _putGroupCommand; - private readonly ILogger _logger; + private readonly IFeatureService _featureService; public GroupsController( IGroupRepository groupRepository, @@ -32,18 +35,21 @@ public class GroupsController : Controller IGetGroupsListQuery getGroupsListQuery, IDeleteGroupCommand deleteGroupCommand, IPatchGroupCommand patchGroupCommand, + IPatchGroupCommandvNext patchGroupCommandvNext, IPostGroupCommand postGroupCommand, IPutGroupCommand putGroupCommand, - ILogger logger) + IFeatureService featureService + ) { _groupRepository = groupRepository; _organizationRepository = organizationRepository; _getGroupsListQuery = getGroupsListQuery; _deleteGroupCommand = deleteGroupCommand; _patchGroupCommand = patchGroupCommand; + _patchGroupCommandvNext = patchGroupCommandvNext; _postGroupCommand = postGroupCommand; _putGroupCommand = putGroupCommand; - _logger = logger; + _featureService = featureService; } [HttpGet("{id}")] @@ -97,8 +103,21 @@ public class GroupsController : Controller [HttpPatch("{id}")] public async Task Patch(Guid organizationId, Guid id, [FromBody] ScimPatchModel model) { + if (_featureService.IsEnabled(FeatureFlagKeys.ShortcutDuplicatePatchRequests)) + { + var group = await _groupRepository.GetByIdAsync(id); + if (group == null || group.OrganizationId != organizationId) + { + throw new NotFoundException("Group not found."); + } + + await _patchGroupCommandvNext.PatchGroupAsync(group, model); + return new NoContentResult(); + } + var organization = await _organizationRepository.GetByIdAsync(organizationId); await _patchGroupCommand.PatchGroupAsync(organization, id, model); + return new NoContentResult(); } diff --git a/bitwarden_license/src/Scim/Groups/Interfaces/IPatchGroupCommandvNext.cs b/bitwarden_license/src/Scim/Groups/Interfaces/IPatchGroupCommandvNext.cs new file mode 100644 index 0000000000..f51cc54079 --- /dev/null +++ b/bitwarden_license/src/Scim/Groups/Interfaces/IPatchGroupCommandvNext.cs @@ -0,0 +1,9 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Scim.Models; + +namespace Bit.Scim.Groups.Interfaces; + +public interface IPatchGroupCommandvNext +{ + Task PatchGroupAsync(Group group, ScimPatchModel model); +} diff --git a/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs b/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs new file mode 100644 index 0000000000..359df4bc94 --- /dev/null +++ b/bitwarden_license/src/Scim/Groups/PatchGroupCommandvNext.cs @@ -0,0 +1,170 @@ +using System.Text.Json; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.AdminConsole.Services; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Scim.Groups.Interfaces; +using Bit.Scim.Models; +using Bit.Scim.Utilities; + +namespace Bit.Scim.Groups; + +public class PatchGroupCommandvNext : IPatchGroupCommandvNext +{ + private readonly IGroupRepository _groupRepository; + private readonly IGroupService _groupService; + private readonly IUpdateGroupCommand _updateGroupCommand; + private readonly ILogger _logger; + private readonly IOrganizationRepository _organizationRepository; + + public PatchGroupCommandvNext( + IGroupRepository groupRepository, + IGroupService groupService, + IUpdateGroupCommand updateGroupCommand, + ILogger logger, + IOrganizationRepository organizationRepository) + { + _groupRepository = groupRepository; + _groupService = groupService; + _updateGroupCommand = updateGroupCommand; + _logger = logger; + _organizationRepository = organizationRepository; + } + + public async Task PatchGroupAsync(Group group, ScimPatchModel model) + { + foreach (var operation in model.Operations) + { + await HandleOperationAsync(group, operation); + } + } + + private async Task HandleOperationAsync(Group group, ScimPatchModel.OperationModel operation) + { + switch (operation.Op?.ToLowerInvariant()) + { + // Replace a list of members + case PatchOps.Replace when operation.Path?.ToLowerInvariant() == PatchPaths.Members: + { + var ids = GetOperationValueIds(operation.Value); + await _groupRepository.UpdateUsersAsync(group.Id, ids); + break; + } + + // Replace group name from path + case PatchOps.Replace when operation.Path?.ToLowerInvariant() == PatchPaths.DisplayName: + { + group.Name = operation.Value.GetString(); + var organization = await _organizationRepository.GetByIdAsync(group.OrganizationId); + if (organization == null) + { + throw new NotFoundException(); + } + await _updateGroupCommand.UpdateGroupAsync(group, organization, EventSystemUser.SCIM); + break; + } + + // Replace group name from value object + case PatchOps.Replace when + string.IsNullOrWhiteSpace(operation.Path) && + operation.Value.TryGetProperty("displayName", out var displayNameProperty): + { + group.Name = displayNameProperty.GetString(); + var organization = await _organizationRepository.GetByIdAsync(group.OrganizationId); + if (organization == null) + { + throw new NotFoundException(); + } + await _updateGroupCommand.UpdateGroupAsync(group, organization, EventSystemUser.SCIM); + break; + } + + // Add a single member + case PatchOps.Add when + !string.IsNullOrWhiteSpace(operation.Path) && + operation.Path.StartsWith("members[value eq ", StringComparison.OrdinalIgnoreCase) && + TryGetOperationPathId(operation.Path, out var addId): + { + await AddMembersAsync(group, [addId]); + break; + } + + // Add a list of members + case PatchOps.Add when + operation.Path?.ToLowerInvariant() == PatchPaths.Members: + { + await AddMembersAsync(group, GetOperationValueIds(operation.Value)); + break; + } + + // Remove a single member + case PatchOps.Remove when + !string.IsNullOrWhiteSpace(operation.Path) && + operation.Path.StartsWith("members[value eq ", StringComparison.OrdinalIgnoreCase) && + TryGetOperationPathId(operation.Path, out var removeId): + { + await _groupService.DeleteUserAsync(group, removeId, EventSystemUser.SCIM); + break; + } + + // Remove a list of members + case PatchOps.Remove when + operation.Path?.ToLowerInvariant() == PatchPaths.Members: + { + var orgUserIds = (await _groupRepository.GetManyUserIdsByIdAsync(group.Id)).ToHashSet(); + foreach (var v in GetOperationValueIds(operation.Value)) + { + orgUserIds.Remove(v); + } + await _groupRepository.UpdateUsersAsync(group.Id, orgUserIds); + break; + } + + default: + { + _logger.LogWarning("Group patch operation not handled: {OperationOp}:{OperationPath}", operation.Op, operation.Path); + break; + } + } + } + + private async Task AddMembersAsync(Group group, HashSet 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 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)) + { + _logger.LogDebug("Ignoring duplicate SCIM request to add members {Members} to group {Group}", usersToAdd, group.Id); + return; + } + + await _groupRepository.AddGroupUsersByIdAsync(group.Id, usersToAdd); + } + + private static HashSet GetOperationValueIds(JsonElement objArray) + { + var ids = new HashSet(); + foreach (var obj in objArray.EnumerateArray()) + { + if (obj.TryGetProperty("value", out var valueProperty)) + { + if (valueProperty.TryGetGuid(out var guid)) + { + ids.Add(guid); + } + } + } + return ids; + } + + private static bool TryGetOperationPathId(string path, out Guid pathId) + { + // Parse Guid from string like: members[value eq "{GUID}"}] + return Guid.TryParse(path.Substring(18).Replace("\"]", string.Empty), out pathId); + } +} diff --git a/bitwarden_license/src/Scim/Utilities/ScimConstants.cs b/bitwarden_license/src/Scim/Utilities/ScimConstants.cs index 219be6534f..0836a72c7f 100644 --- a/bitwarden_license/src/Scim/Utilities/ScimConstants.cs +++ b/bitwarden_license/src/Scim/Utilities/ScimConstants.cs @@ -7,3 +7,16 @@ public static class ScimConstants public const string Scim2SchemaUser = "urn:ietf:params:scim:schemas:core:2.0:User"; public const string Scim2SchemaGroup = "urn:ietf:params:scim:schemas:core:2.0:Group"; } + +public static class PatchOps +{ + public const string Replace = "replace"; + public const string Add = "add"; + public const string Remove = "remove"; +} + +public static class PatchPaths +{ + public const string Members = "members"; + public const string DisplayName = "displayname"; +} diff --git a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs index 75b60a71fc..b5d866524a 100644 --- a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs +++ b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs @@ -10,6 +10,7 @@ public static class ScimServiceCollectionExtensions public static void AddScimGroupCommands(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); } diff --git a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTests.cs b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTests.cs new file mode 100644 index 0000000000..eaa5b3dcd7 --- /dev/null +++ b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTests.cs @@ -0,0 +1,237 @@ +using System.Text.Json; +using Bit.Scim.IntegrationTest.Factories; +using Bit.Scim.Models; +using Bit.Scim.Utilities; +using Bit.Test.Common.Helpers; +using Xunit; + +namespace Bit.Scim.IntegrationTest.Controllers.v2; + +public class GroupsControllerPatchTests : IClassFixture, IAsyncLifetime +{ + private readonly ScimApplicationFactory _factory; + + public GroupsControllerPatchTests(ScimApplicationFactory factory) + { + _factory = factory; + } + + public Task InitializeAsync() + { + var databaseContext = _factory.GetDatabaseContext(); + _factory.ReinitializeDbForTests(databaseContext); + return Task.CompletedTask; + } + + Task IAsyncLifetime.DisposeAsync() => Task.CompletedTask; + + [Fact] + public async Task Patch_ReplaceDisplayName_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var newDisplayName = "Patch Display Name"; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "replace", + Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); + Assert.Equal(newDisplayName, group.Name); + + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount, databaseContext.GroupUsers.Count()); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); + } + + [Fact] + public async Task Patch_ReplaceMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "replace", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Single(databaseContext.GroupUsers); + + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); + var groupUser = databaseContext.GroupUsers.FirstOrDefault(); + Assert.Equal(ScimApplicationFactory.TestOrganizationUserId2, groupUser.OrganizationUserId); + } + + [Fact] + public async Task Patch_AddSingleMember_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "add", + Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId2}\"]", + Value = JsonDocument.Parse("{}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount + 1, databaseContext.GroupUsers.Count()); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); + } + + [Fact] + public async Task Patch_AddListMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId2; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "add", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}},{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId3}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId3)); + } + + [Fact] + public async Task Patch_RemoveSingleMember_ReplaceDisplayName_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var newDisplayName = "Patch Display Name"; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "remove", + Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId1}\"]", + Value = JsonDocument.Parse("{}").RootElement + }, + new ScimPatchModel.OperationModel + { + Op = "replace", + Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupCount, databaseContext.Groups.Count()); + + var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); + Assert.Equal(newDisplayName, group.Name); + } + + [Fact] + public async Task Patch_RemoveListMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "remove", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId1}\"}}, {{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId4}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Empty(databaseContext.GroupUsers); + } + + [Fact] + public async Task Patch_NotFound() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = Guid.NewGuid(); + var inputModel = new Models.ScimPatchModel + { + Operations = new List(), + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + var expectedResponse = new ScimErrorResponseModel + { + Status = StatusCodes.Status404NotFound, + Detail = "Group not found.", + Schemas = new List { ScimConstants.Scim2SchemaError } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status404NotFound, context.Response.StatusCode); + + var responseModel = JsonSerializer.Deserialize(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); + } +} diff --git a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTestsvNext.cs b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTestsvNext.cs new file mode 100644 index 0000000000..f66184a8a2 --- /dev/null +++ b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerPatchTestsvNext.cs @@ -0,0 +1,251 @@ +using System.Text.Json; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Services; +using Bit.Scim.Groups.Interfaces; +using Bit.Scim.IntegrationTest.Factories; +using Bit.Scim.Models; +using Bit.Scim.Utilities; +using Bit.Test.Common.Helpers; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Bit.Scim.IntegrationTest.Controllers.v2; + +public class GroupsControllerPatchTestsvNext : IClassFixture, IAsyncLifetime +{ + private readonly ScimApplicationFactory _factory; + + public GroupsControllerPatchTestsvNext(ScimApplicationFactory factory) + { + _factory = factory; + + // Enable the feature flag for new PatchGroupsCommand and stub out the old command to be safe + _factory.SubstituteService((IFeatureService featureService) + => featureService.IsEnabled(FeatureFlagKeys.ShortcutDuplicatePatchRequests).Returns(true)); + _factory.SubstituteService((IPatchGroupCommand patchGroupCommand) + => patchGroupCommand.PatchGroupAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .ThrowsAsync(new Exception("This test suite should be testing the vNext command, but the existing command was called."))); + } + + public Task InitializeAsync() + { + var databaseContext = _factory.GetDatabaseContext(); + _factory.ReinitializeDbForTests(databaseContext); + + return Task.CompletedTask; + } + + Task IAsyncLifetime.DisposeAsync() => Task.CompletedTask; + + [Fact] + public async Task Patch_ReplaceDisplayName_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var newDisplayName = "Patch Display Name"; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "replace", + Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); + Assert.Equal(newDisplayName, group.Name); + + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount, databaseContext.GroupUsers.Count()); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); + } + + [Fact] + public async Task Patch_ReplaceMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "replace", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Single(databaseContext.GroupUsers); + + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); + var groupUser = databaseContext.GroupUsers.FirstOrDefault(); + Assert.Equal(ScimApplicationFactory.TestOrganizationUserId2, groupUser.OrganizationUserId); + } + + [Fact] + public async Task Patch_AddSingleMember_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "add", + Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId2}\"]", + Value = JsonDocument.Parse("{}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount + 1, databaseContext.GroupUsers.Count()); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); + } + + [Fact] + public async Task Patch_AddListMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId2; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "add", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}},{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId3}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); + Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId3)); + } + + [Fact] + public async Task Patch_RemoveSingleMember_ReplaceDisplayName_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var newDisplayName = "Patch Display Name"; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "remove", + Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId1}\"]", + Value = JsonDocument.Parse("{}").RootElement + }, + new ScimPatchModel.OperationModel + { + Op = "replace", + Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupCount, databaseContext.Groups.Count()); + + var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); + Assert.Equal(newDisplayName, group.Name); + } + + [Fact] + public async Task Patch_RemoveListMembers_Success() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = ScimApplicationFactory.TestGroupId1; + var inputModel = new ScimPatchModel + { + Operations = new List() + { + new ScimPatchModel.OperationModel + { + Op = "remove", + Path = "members", + Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId1}\"}}, {{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId4}\"}}]").RootElement + } + }, + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); + + var databaseContext = _factory.GetDatabaseContext(); + Assert.Empty(databaseContext.GroupUsers); + } + + [Fact] + public async Task Patch_NotFound() + { + var organizationId = ScimApplicationFactory.TestOrganizationId1; + var groupId = Guid.NewGuid(); + var inputModel = new Models.ScimPatchModel + { + Operations = new List(), + Schemas = new List() { ScimConstants.Scim2SchemaGroup } + }; + var expectedResponse = new ScimErrorResponseModel + { + Status = StatusCodes.Status404NotFound, + Detail = "Group not found.", + Schemas = new List { ScimConstants.Scim2SchemaError } + }; + + var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); + + Assert.Equal(StatusCodes.Status404NotFound, context.Response.StatusCode); + + var responseModel = JsonSerializer.Deserialize(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); + } +} diff --git a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerTests.cs b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerTests.cs index f8150fc1c5..5f562a30c5 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerTests.cs +++ b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerTests.cs @@ -9,9 +9,6 @@ namespace Bit.Scim.IntegrationTest.Controllers.v2; public class GroupsControllerTests : IClassFixture, IAsyncLifetime { - private const int _initialGroupCount = 3; - private const int _initialGroupUsersCount = 2; - private readonly ScimApplicationFactory _factory; public GroupsControllerTests(ScimApplicationFactory factory) @@ -237,10 +234,10 @@ public class GroupsControllerTests : IClassFixture, IAsy AssertHelper.AssertPropertyEqual(expectedResponse, responseModel, "Id"); var databaseContext = _factory.GetDatabaseContext(); - Assert.Equal(_initialGroupCount + 1, databaseContext.Groups.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupCount + 1, databaseContext.Groups.Count()); Assert.True(databaseContext.Groups.Any(g => g.Name == displayName && g.ExternalId == externalId)); - Assert.Equal(_initialGroupUsersCount + 1, databaseContext.GroupUsers.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupUsersCount + 1, databaseContext.GroupUsers.Count()); Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == responseModel.Id && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); } @@ -281,7 +278,7 @@ public class GroupsControllerTests : IClassFixture, IAsy Assert.Equal(StatusCodes.Status409Conflict, context.Response.StatusCode); var databaseContext = _factory.GetDatabaseContext(); - Assert.Equal(_initialGroupCount, databaseContext.Groups.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupCount, databaseContext.Groups.Count()); Assert.False(databaseContext.Groups.Any(g => g.Name == "New Group")); } @@ -354,216 +351,6 @@ public class GroupsControllerTests : IClassFixture, IAsy AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); } - [Fact] - public async Task Patch_ReplaceDisplayName_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId1; - var newDisplayName = "Patch Display Name"; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "replace", - Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); - Assert.Equal(newDisplayName, group.Name); - - Assert.Equal(_initialGroupUsersCount, databaseContext.GroupUsers.Count()); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); - } - - [Fact] - public async Task Patch_ReplaceMembers_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId1; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "replace", - Path = "members", - Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}}]").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - Assert.Single(databaseContext.GroupUsers); - - Assert.Equal(_initialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); - var groupUser = databaseContext.GroupUsers.FirstOrDefault(); - Assert.Equal(ScimApplicationFactory.TestOrganizationUserId2, groupUser.OrganizationUserId); - } - - [Fact] - public async Task Patch_AddSingleMember_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId1; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "add", - Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId2}\"]", - Value = JsonDocument.Parse("{}").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - Assert.Equal(_initialGroupUsersCount + 1, databaseContext.GroupUsers.Count()); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId1)); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId4)); - } - - [Fact] - public async Task Patch_AddListMembers_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId2; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "add", - Path = "members", - Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId2}\"}},{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId3}\"}}]").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId2)); - Assert.True(databaseContext.GroupUsers.Any(gu => gu.GroupId == groupId && gu.OrganizationUserId == ScimApplicationFactory.TestOrganizationUserId3)); - } - - [Fact] - public async Task Patch_RemoveSingleMember_ReplaceDisplayName_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId1; - var newDisplayName = "Patch Display Name"; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "remove", - Path = $"members[value eq \"{ScimApplicationFactory.TestOrganizationUserId1}\"]", - Value = JsonDocument.Parse("{}").RootElement - }, - new ScimPatchModel.OperationModel - { - Op = "replace", - Value = JsonDocument.Parse($"{{\"displayName\":\"{newDisplayName}\"}}").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - Assert.Equal(_initialGroupUsersCount - 1, databaseContext.GroupUsers.Count()); - Assert.Equal(_initialGroupCount, databaseContext.Groups.Count()); - - var group = databaseContext.Groups.FirstOrDefault(g => g.Id == groupId); - Assert.Equal(newDisplayName, group.Name); - } - - [Fact] - public async Task Patch_RemoveListMembers_Success() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = ScimApplicationFactory.TestGroupId1; - var inputModel = new ScimPatchModel - { - Operations = new List() - { - new ScimPatchModel.OperationModel - { - Op = "remove", - Path = "members", - Value = JsonDocument.Parse($"[{{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId1}\"}}, {{\"value\":\"{ScimApplicationFactory.TestOrganizationUserId4}\"}}]").RootElement - } - }, - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); - - var databaseContext = _factory.GetDatabaseContext(); - Assert.Empty(databaseContext.GroupUsers); - } - - [Fact] - public async Task Patch_NotFound() - { - var organizationId = ScimApplicationFactory.TestOrganizationId1; - var groupId = Guid.NewGuid(); - var inputModel = new Models.ScimPatchModel - { - Operations = new List(), - Schemas = new List() { ScimConstants.Scim2SchemaGroup } - }; - var expectedResponse = new ScimErrorResponseModel - { - Status = StatusCodes.Status404NotFound, - Detail = "Group not found.", - Schemas = new List { ScimConstants.Scim2SchemaError } - }; - - var context = await _factory.GroupsPatchAsync(organizationId, groupId, inputModel); - - Assert.Equal(StatusCodes.Status404NotFound, context.Response.StatusCode); - - var responseModel = JsonSerializer.Deserialize(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); - AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); - } - [Fact] public async Task Delete_Success() { @@ -575,7 +362,7 @@ public class GroupsControllerTests : IClassFixture, IAsy Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode); var databaseContext = _factory.GetDatabaseContext(); - Assert.Equal(_initialGroupCount - 1, databaseContext.Groups.Count()); + Assert.Equal(ScimApplicationFactory.InitialGroupCount - 1, databaseContext.Groups.Count()); Assert.True(databaseContext.Groups.FirstOrDefault(g => g.Id == groupId) == null); } diff --git a/bitwarden_license/test/Scim.IntegrationTest/Factories/ScimApplicationFactory.cs b/bitwarden_license/test/Scim.IntegrationTest/Factories/ScimApplicationFactory.cs index b9c6191f75..1a5cdde41c 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Factories/ScimApplicationFactory.cs +++ b/bitwarden_license/test/Scim.IntegrationTest/Factories/ScimApplicationFactory.cs @@ -9,8 +9,6 @@ using Bit.Infrastructure.EntityFramework.Repositories; using Bit.IntegrationTestCommon.Factories; using Bit.Scim.Models; using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Mvc.Testing; -using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; @@ -18,7 +16,8 @@ namespace Bit.Scim.IntegrationTest.Factories; public class ScimApplicationFactory : WebApplicationFactoryBase { - public readonly new TestServer Server; + public const int InitialGroupCount = 3; + public const int InitialGroupUsersCount = 2; public static readonly Guid TestUserId1 = Guid.Parse("2e8173db-8e8d-4de1-ac38-91b15c6d8dcb"); public static readonly Guid TestUserId2 = Guid.Parse("b57846fc-0e94-4c93-9de5-9d0389eeadfb"); @@ -33,32 +32,29 @@ public class ScimApplicationFactory : WebApplicationFactoryBase public static readonly Guid TestOrganizationUserId3 = Guid.Parse("be2f9045-e2b6-4173-ad44-4c69c3ea8140"); public static readonly Guid TestOrganizationUserId4 = Guid.Parse("1f5689b7-e96e-4840-b0b1-eb3d5b5fd514"); - public ScimApplicationFactory() + protected override void ConfigureWebHost(IWebHostBuilder builder) { - WebApplicationFactory webApplicationFactory = WithWebHostBuilder(builder => + base.ConfigureWebHost(builder); + + builder.ConfigureServices(services => { - builder.ConfigureServices(services => + services + .AddAuthentication("Test") + .AddScheme("Test", options => { }); + + // Override to bypass SCIM authorization + services.AddAuthorization(config => { - services - .AddAuthentication("Test") - .AddScheme("Test", options => { }); - - // Override to bypass SCIM authorization - services.AddAuthorization(config => + config.AddPolicy("Scim", policy => { - config.AddPolicy("Scim", policy => - { - policy.RequireAssertion(a => true); - }); + policy.RequireAssertion(a => true); }); - - var mailService = services.First(sd => sd.ServiceType == typeof(IMailService)); - services.Remove(mailService); - services.AddSingleton(); }); - }); - Server = webApplicationFactory.Server; + var mailService = services.First(sd => sd.ServiceType == typeof(IMailService)); + services.Remove(mailService); + services.AddSingleton(); + }); } public async Task GroupsGetAsync(Guid organizationId, Guid id) diff --git a/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandvNextTests.cs b/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandvNextTests.cs new file mode 100644 index 0000000000..b9877f0b71 --- /dev/null +++ b/bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandvNextTests.cs @@ -0,0 +1,381 @@ +using System.Text.Json; +using AutoFixture; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.AdminConsole.Services; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Scim.Groups; +using Bit.Scim.Models; +using Bit.Scim.Utilities; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Scim.Test.Groups; + +[SutProviderCustomize] +public class PatchGroupCommandvNextTests +{ + [Theory] + [BitAutoData] + public async Task PatchGroup_ReplaceListMembers_Success(SutProvider sutProvider, + Organization organization, Group group, IEnumerable userIds) + { + group.OrganizationId = organization.Id; + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "replace", + Path = "members", + Value = JsonDocument.Parse(JsonSerializer.Serialize(userIds.Select(uid => new { value = uid }).ToArray())).RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).UpdateUsersAsync( + group.Id, + Arg.Is>(arg => + arg.Count() == userIds.Count() && + arg.ToHashSet().SetEquals(userIds))); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_ReplaceDisplayNameFromPath_Success( + SutProvider sutProvider, Organization organization, Group group, string displayName) + { + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "replace", + Path = "displayname", + Value = JsonDocument.Parse($"\"{displayName}\"").RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).UpdateGroupAsync(group, organization, EventSystemUser.SCIM); + Assert.Equal(displayName, group.Name); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_ReplaceDisplayNameFromValueObject_Success(SutProvider sutProvider, Organization organization, Group group, string displayName) + { + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "replace", + Value = JsonDocument.Parse($"{{\"displayName\":\"{displayName}\"}}").RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).UpdateGroupAsync(group, organization, EventSystemUser.SCIM); + Assert.Equal(displayName, group.Name); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_AddSingleMember_Success(SutProvider sutProvider, Organization organization, Group group, ICollection existingMembers, Guid userId) + { + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id, true) + .Returns(existingMembers); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "add", + Path = $"members[value eq \"{userId}\"]", + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).AddGroupUsersByIdAsync( + group.Id, + Arg.Is>(arg => arg.Single() == userId)); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_AddSingleMember_ReturnsEarlyIfAlreadyInGroup( + SutProvider sutProvider, + Organization organization, + Group group, + ICollection existingMembers) + { + // User being added is already in group + var userId = existingMembers.First(); + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id, true) + .Returns(existingMembers); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "add", + Path = $"members[value eq \"{userId}\"]", + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .AddGroupUsersByIdAsync(default, default); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_AddListMembers_Success(SutProvider sutProvider, Organization organization, Group group, ICollection existingMembers, ICollection userIds) + { + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id, true) + .Returns(existingMembers); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "add", + Path = $"members", + Value = JsonDocument.Parse(JsonSerializer.Serialize(userIds.Select(uid => new { value = uid }).ToArray())).RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).AddGroupUsersByIdAsync( + group.Id, + Arg.Is>(arg => + arg.Count() == userIds.Count && + arg.ToHashSet().SetEquals(userIds))); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_AddListMembers_IgnoresDuplicatesInRequest( + SutProvider sutProvider, Organization organization, Group group, + ICollection existingMembers) + { + // Create 3 userIds + var fixture = new Fixture { RepeatCount = 3 }; + var userIds = fixture.CreateMany().ToList(); + + // Copy the list and add a duplicate + var userIdsWithDuplicate = userIds.Append(userIds.First()).ToList(); + Assert.Equal(4, userIdsWithDuplicate.Count); + + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id, true) + .Returns(existingMembers); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "add", + Path = $"members", + Value = JsonDocument.Parse(JsonSerializer + .Serialize(userIdsWithDuplicate + .Select(uid => new { value = uid }) + .ToArray())).RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).AddGroupUsersByIdAsync( + group.Id, + Arg.Is>(arg => + arg.Count() == 3 && + arg.ToHashSet().SetEquals(userIds))); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_AddListMembers_SuccessIfOnlySomeUsersAreInGroup( + SutProvider sutProvider, + Organization organization, Group group, + ICollection existingMembers, + ICollection userIds) + { + // A user is already in the group, but some still need to be added + userIds.Add(existingMembers.First()); + + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id, true) + .Returns(existingMembers); + + var scimPatchModel = new ScimPatchModel + { + Operations = new List + { + new() + { + Op = "add", + Path = $"members", + Value = JsonDocument.Parse(JsonSerializer.Serialize(userIds.Select(uid => new { value = uid }).ToArray())).RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency() + .Received(1) + .AddGroupUsersByIdAsync( + group.Id, + Arg.Is>(arg => + arg.Count() == userIds.Count && + arg.ToHashSet().SetEquals(userIds))); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_RemoveSingleMember_Success(SutProvider sutProvider, Organization organization, Group group, Guid userId) + { + group.OrganizationId = organization.Id; + + var scimPatchModel = new Models.ScimPatchModel + { + Operations = new List + { + new ScimPatchModel.OperationModel + { + Op = "remove", + Path = $"members[value eq \"{userId}\"]", + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().Received(1).DeleteUserAsync(group, userId, EventSystemUser.SCIM); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_RemoveListMembers_Success(SutProvider sutProvider, + Organization organization, Group group, ICollection existingMembers) + { + List usersToRemove = [existingMembers.First(), existingMembers.Skip(1).First()]; + group.OrganizationId = organization.Id; + + sutProvider.GetDependency() + .GetManyUserIdsByIdAsync(group.Id) + .Returns(existingMembers); + + var scimPatchModel = new Models.ScimPatchModel + { + Operations = new List + { + new() + { + Op = "remove", + Path = $"members", + Value = JsonDocument.Parse(JsonSerializer.Serialize(usersToRemove.Select(uid => new { value = uid }).ToArray())).RootElement + } + }, + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + var expectedRemainingUsers = existingMembers.Skip(2).ToList(); + await sutProvider.GetDependency() + .Received(1) + .UpdateUsersAsync( + group.Id, + Arg.Is>(arg => + arg.Count() == expectedRemainingUsers.Count && + arg.ToHashSet().SetEquals(expectedRemainingUsers))); + } + + [Theory] + [BitAutoData] + public async Task PatchGroup_NoAction_Success( + SutProvider sutProvider, Organization organization, Group group) + { + group.OrganizationId = organization.Id; + + var scimPatchModel = new Models.ScimPatchModel + { + Operations = new List(), + Schemas = new List { ScimConstants.Scim2SchemaUser } + }; + + await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyUserIdsByIdAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpdateGroupAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default); + } +} diff --git a/src/Core/AdminConsole/Repositories/IGroupRepository.cs b/src/Core/AdminConsole/Repositories/IGroupRepository.cs index 6519b19833..b70331a3f5 100644 --- a/src/Core/AdminConsole/Repositories/IGroupRepository.cs +++ b/src/Core/AdminConsole/Repositories/IGroupRepository.cs @@ -14,11 +14,29 @@ public interface IGroupRepository : IRepository Guid organizationId); Task> GetManyByManyIds(IEnumerable groupIds); Task> GetManyIdsByUserIdAsync(Guid organizationUserId); - Task> GetManyUserIdsByIdAsync(Guid id); + /// + /// Query all OrganizationUserIds who are a member of the specified group. + /// + /// The group id. + /// + /// Whether to use the high-availability database replica. This is for paths with high traffic where immediate data + /// consistency is not required. You generally do not want this. + /// + /// + Task> GetManyUserIdsByIdAsync(Guid id, bool useReadOnlyReplica = false); Task> GetManyGroupUsersByOrganizationIdAsync(Guid organizationId); 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. + /// Ignores members that do not belong to the same organization as the group. + /// Task UpdateUsersAsync(Guid groupId, IEnumerable organizationUserIds); + /// + /// Add members to a group. Gracefully ignores members that are already in the group, + /// duplicate organizationUserIds, and organizationUsers who are not part of the organization. + /// + Task AddGroupUsersByIdAsync(Guid groupId, IEnumerable organizationUserIds); Task DeleteManyAsync(IEnumerable groupIds); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 0b0d21f7bb..21360703a2 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -108,6 +108,7 @@ public static class FeatureFlagKeys public const string IntegrationPage = "pm-14505-admin-console-integration-page"; public const string DeviceApprovalRequestAdminNotifications = "pm-15637-device-approval-request-admin-notifications"; public const string LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission"; + public const string ShortcutDuplicatePatchRequests = "pm-16812-shortcut-duplicate-patch-requests"; public const string PushSyncOrgKeysOnRevokeRestore = "pm-17168-push-sync-org-keys-on-revoke-restore"; /* Tools Team */ diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs index d8245ce719..2b4db3940c 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs @@ -109,9 +109,13 @@ public class GroupRepository : Repository, IGroupRepository } } - public async Task> GetManyUserIdsByIdAsync(Guid id) + public async Task> GetManyUserIdsByIdAsync(Guid id, bool useReadOnlyReplica = false) { - using (var connection = new SqlConnection(ConnectionString)) + var connectionString = useReadOnlyReplica + ? ReadOnlyConnectionString + : ConnectionString; + + using (var connection = new SqlConnection(connectionString)) { var results = await connection.QueryAsync( $"[{Schema}].[GroupUser_ReadOrganizationUserIdsByGroupId]", @@ -186,6 +190,17 @@ public class GroupRepository : Repository, IGroupRepository } } + public async Task AddGroupUsersByIdAsync(Guid groupId, IEnumerable organizationUserIds) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.ExecuteAsync( + "[dbo].[GroupUser_AddUsers]", + 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 0e91bd42ef..305a715d4c 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs @@ -163,8 +163,10 @@ public class GroupRepository : Repository> GetManyUserIdsByIdAsync(Guid id) + public async Task> GetManyUserIdsByIdAsync(Guid id, bool useReadOnlyReplica = false) { + // EF is only used for self-hosted so read-only replica parameter is ignored + using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); @@ -255,6 +257,29 @@ public class GroupRepository : Repository 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 0000000000..362cdce785 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/GroupUser_AddUsers.sql @@ -0,0 +1,39 @@ +CREATE PROCEDURE [dbo].[GroupUser_AddUsers] + @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] (GroupId, OrganizationUserId) + SELECT DISTINCT + @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/test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs b/test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs new file mode 100644 index 0000000000..e631280bb3 --- /dev/null +++ b/test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs @@ -0,0 +1,57 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; + +namespace Bit.Infrastructure.IntegrationTest.AdminConsole; + +/// +/// A set of extension methods used to arrange simple test data. +/// This should only be used for basic, repetitive data arrangement, not for anything complex or for +/// the repository method under test. +/// +public static class OrganizationTestHelpers +{ + public static Task CreateTestUserAsync(this IUserRepository userRepository, string identifier = "test") + { + var id = Guid.NewGuid(); + return userRepository.CreateAsync(new User + { + Id = id, + Name = $"{identifier}-{id}", + Email = $"{id}@example.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + } + + public static Task CreateTestOrganizationAsync(this IOrganizationRepository organizationRepository, + string identifier = "test") + => organizationRepository.CreateAsync(new Organization + { + Name = $"{identifier}-{Guid.NewGuid()}", + BillingEmail = "billing@example.com", // TODO: EF does not enforce this being NOT NULL + Plan = "Test", // TODO: EF does not enforce this being NOT NULl + }); + + public static Task CreateTestOrganizationUserAsync( + this IOrganizationUserRepository organizationUserRepository, + Organization organization, + User user) + => organizationUserRepository.CreateAsync(new OrganizationUser + { + OrganizationId = organization.Id, + UserId = user.Id, + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.Owner + }); + + public static Task CreateTestGroupAsync( + this IGroupRepository groupRepository, + Organization organization, + string identifier = "test") + => groupRepository.CreateAsync( + new Group { OrganizationId = organization.Id, Name = $"{identifier} {Guid.NewGuid()}" } + ); +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/GroupRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/GroupRepositoryTests.cs new file mode 100644 index 0000000000..e2c2cbfa02 --- /dev/null +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/GroupRepositoryTests.cs @@ -0,0 +1,129 @@ +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories; + +public class GroupRepositoryTests +{ + [DatabaseTheory, DatabaseData] + public async Task AddGroupUsersByIdAsync_CreatesGroupUsers( + IGroupRepository groupRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository) + { + // Arrange + var user1 = await userRepository.CreateTestUserAsync("user1"); + var user2 = await userRepository.CreateTestUserAsync("user2"); + var user3 = await userRepository.CreateTestUserAsync("user3"); + + var org = await organizationRepository.CreateTestOrganizationAsync(); + var orgUser1 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user1); + var orgUser2 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user2); + var orgUser3 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user3); + var orgUserIds = new List([orgUser1.Id, orgUser2.Id, orgUser3.Id]); + var group = await groupRepository.CreateTestGroupAsync(org); + + // Act + await groupRepository.AddGroupUsersByIdAsync(group.Id, orgUserIds); + + // Assert + var actual = await groupRepository.GetManyUserIdsByIdAsync(group.Id); + Assert.Equal(orgUserIds!.Order(), actual.Order()); + } + + [DatabaseTheory, DatabaseData] + public async Task AddGroupUsersByIdAsync_IgnoresExistingGroupUsers( + IGroupRepository groupRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository) + { + // Arrange + var user1 = await userRepository.CreateTestUserAsync("user1"); + var user2 = await userRepository.CreateTestUserAsync("user2"); + var user3 = await userRepository.CreateTestUserAsync("user3"); + + var org = await organizationRepository.CreateTestOrganizationAsync(); + var orgUser1 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user1); + var orgUser2 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user2); + var orgUser3 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user3); + var orgUserIds = new List([orgUser1.Id, orgUser2.Id, orgUser3.Id]); + var group = await groupRepository.CreateTestGroupAsync(org); + + // Add user 2 to the group already, make sure this is executed correctly before proceeding + await groupRepository.UpdateUsersAsync(group.Id, [orgUser2.Id]); + var existingUsers = await groupRepository.GetManyUserIdsByIdAsync(group.Id); + Assert.Equal([orgUser2.Id], existingUsers); + + // Act + await groupRepository.AddGroupUsersByIdAsync(group.Id, orgUserIds); + + // Assert - group should contain all users + var actual = await groupRepository.GetManyUserIdsByIdAsync(group.Id); + Assert.Equal(orgUserIds!.Order(), actual.Order()); + } + + [DatabaseTheory, DatabaseData] + public async Task AddGroupUsersByIdAsync_IgnoresUsersNotInOrganization( + IGroupRepository groupRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository) + { + // Arrange + var user1 = await userRepository.CreateTestUserAsync("user1"); + var user2 = await userRepository.CreateTestUserAsync("user2"); + var user3 = await userRepository.CreateTestUserAsync("user3"); + + var org = await organizationRepository.CreateTestOrganizationAsync(); + var orgUser1 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user1); + var orgUser2 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user2); + + // User3 belongs to a different org + var otherOrg = await organizationRepository.CreateTestOrganizationAsync(); + var orgUser3 = await organizationUserRepository.CreateTestOrganizationUserAsync(otherOrg, user3); + + var orgUserIds = new List([orgUser1.Id, orgUser2.Id, orgUser3.Id]); + var group = await groupRepository.CreateTestGroupAsync(org); + + // Act + await groupRepository.AddGroupUsersByIdAsync(group.Id, orgUserIds); + + // Assert + var actual = await groupRepository.GetManyUserIdsByIdAsync(group.Id); + Assert.Equal(2, actual.Count); + Assert.Contains(orgUser1.Id, actual); + Assert.Contains(orgUser2.Id, actual); + Assert.DoesNotContain(orgUser3.Id, actual); + } + + [DatabaseTheory, DatabaseData] + public async Task AddGroupUsersByIdAsync_IgnoresDuplicateUsers( + IGroupRepository groupRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository) + { + // Arrange + var user1 = await userRepository.CreateTestUserAsync("user1"); + var user2 = await userRepository.CreateTestUserAsync("user2"); + + var org = await organizationRepository.CreateTestOrganizationAsync(); + var orgUser1 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user1); + var orgUser2 = await organizationUserRepository.CreateTestOrganizationUserAsync(org, user2); + + var orgUserIds = new List([orgUser1.Id, orgUser2.Id, orgUser2.Id]); // duplicate orgUser2 + var group = await groupRepository.CreateTestGroupAsync(org); + + // Act + await groupRepository.AddGroupUsersByIdAsync(group.Id, orgUserIds); + + // Assert + var actual = await groupRepository.GetManyUserIdsByIdAsync(group.Id); + Assert.Equal(2, actual.Count); + Assert.Contains(orgUser1.Id, actual); + Assert.Contains(orgUser2.Id, actual); + } +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationDomainRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationDomainRepositoryTests.cs index 7f0ed582bf..a1c5f9bd07 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationDomainRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationDomainRepositoryTests.cs @@ -3,7 +3,7 @@ using Bit.Core.Entities; using Bit.Core.Repositories; using Xunit; -namespace Bit.Infrastructure.IntegrationTest.Repositories; +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories; public class OrganizationDomainRepositoryTests { diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs index f6dc4a989d..f7c61ad957 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs @@ -4,7 +4,7 @@ using Bit.Core.Enums; using Bit.Core.Repositories; using Xunit; -namespace Bit.Infrastructure.IntegrationTest.Repositories; +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories; public class OrganizationRepositoryTests { diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs index aee4beb8ce..e82be49173 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs @@ -4,7 +4,7 @@ using Bit.Core.Enums; using Bit.Core.Repositories; using Xunit; -namespace Bit.Infrastructure.IntegrationTest.Repositories; +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories; public class OrganizationUserRepositoryTests { diff --git a/util/Migrator/DbScripts/2025-02-13_00_GroupUser_AddUsers.sql b/util/Migrator/DbScripts/2025-02-13_00_GroupUser_AddUsers.sql new file mode 100644 index 0000000000..46ea72003e --- /dev/null +++ b/util/Migrator/DbScripts/2025-02-13_00_GroupUser_AddUsers.sql @@ -0,0 +1,39 @@ +CREATE OR ALTER PROCEDURE [dbo].[GroupUser_AddUsers] + @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] (GroupId, OrganizationUserId) + SELECT DISTINCT + @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