mirror of
https://github.com/bitwarden/server
synced 2025-12-15 15:53:59 +00:00
[PM-22241] Add DefaultUserCollectionName support to bulk organization user confirmation (#6153)
* Implement GetByOrganizationAsync method in PolicyRequirementQuery and add corresponding unit tests * Refactor ConfirmOrganizationUserCommand for clarity and add bulk support * Update ConfirmOrganizationUserCommandTests to use GetByOrganizationAsync for policy requirement queries * Add DefaultUserCollectionName property to OrganizationUserBulkConfirmRequestModel with encryption attributes * Update ConfirmUsersAsync method to include DefaultUserCollectionName parameter in OrganizationUsersController * Add EnableOrganizationDataOwnershipPolicyAsync method to OrganizationTestHelpers * Add integration tests for confirming organization users in OrganizationUserControllerTests - Implemented Confirm_WithValidUser test to verify successful confirmation of a single user. - Added BulkConfirm_WithValidUsers test to ensure multiple users can be confirmed successfully. * Refactor organization user confirmation integration tests to also test when the organization data ownership policy is disabled * Refactor ConfirmOrganizationUserCommand to consolidate confirmation side effects handling - Replaced single and bulk confirmation side effect methods with a unified HandleConfirmationSideEffectsAsync method. - Updated related logic to handle confirmed organization users more efficiently. - Adjusted unit tests to reflect changes in the collection creation process for confirmed users. * Refactor OrganizationUserControllerTests to simplify feature flag handling and consolidate test logic - Removed redundant feature flag checks in Confirm and BulkConfirm tests. - Updated tests to directly enable the Organization Data Ownership policy without conditional checks. - Ensured verification of DefaultUserCollection for confirmed users remains intact. * Refactor OrganizationUserControllerTests to enhance clarity and reduce redundancy - Simplified user creation and confirmation logic in tests by introducing helper methods. - Consolidated verification of confirmed users and their associated collections. - Removed unnecessary comments and streamlined test flow for better readability.
This commit is contained in:
Rui Tomé
GitHub
@@ -2,19 +2,34 @@
|
||||
using Bit.Api.AdminConsole.Models.Request.Organizations;
|
||||
using Bit.Api.IntegrationTest.Factories;
|
||||
using Bit.Api.IntegrationTest.Helpers;
|
||||
using Bit.Core;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.Billing.Enums;
|
||||
using Bit.Core.Entities;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Models.Data;
|
||||
using Bit.Core.Repositories;
|
||||
using Bit.Core.Services;
|
||||
using NSubstitute;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Api.IntegrationTest.AdminConsole.Controllers;
|
||||
|
||||
public class OrganizationUserControllerTests : IClassFixture<ApiApplicationFactory>, IAsyncLifetime
|
||||
{
|
||||
private static readonly string _mockEncryptedString =
|
||||
"2.AOs41Hd8OQiCPXjyJKCiDA==|O6OHgt2U2hJGBSNGnimJmg==|iD33s8B69C8JhYYhSa4V1tArjvLr8eEaGqOV7BRo5Jk=";
|
||||
|
||||
|
||||
public OrganizationUserControllerTests(ApiApplicationFactory apiFactory)
|
||||
{
|
||||
_factory = apiFactory;
|
||||
_factory.SubstituteService<IFeatureService>(featureService =>
|
||||
{
|
||||
featureService
|
||||
.IsEnabled(FeatureFlagKeys.CreateDefaultLocation)
|
||||
.Returns(true);
|
||||
});
|
||||
_client = _factory.CreateClient();
|
||||
_loginHelper = new LoginHelper(_factory, _client);
|
||||
}
|
||||
@@ -93,9 +108,113 @@ public class OrganizationUserControllerTests : IClassFixture<ApiApplicationFacto
|
||||
ownerEmail: _ownerEmail, passwordManagerSeats: 5, paymentMethod: PaymentMethodType.Card);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Confirm_WithValidUser_ReturnsSuccess()
|
||||
{
|
||||
await OrganizationTestHelpers.EnableOrganizationDataOwnershipPolicyAsync(_factory, _organization.Id);
|
||||
|
||||
var acceptedOrgUser = (await CreateAcceptedUsersAsync(new[] { "test1@bitwarden.com" })).First();
|
||||
|
||||
await _loginHelper.LoginAsync(_ownerEmail);
|
||||
|
||||
var confirmModel = new OrganizationUserConfirmRequestModel
|
||||
{
|
||||
Key = "test-key",
|
||||
DefaultUserCollectionName = _mockEncryptedString
|
||||
};
|
||||
var confirmResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/{acceptedOrgUser.Id}/confirm", confirmModel);
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, confirmResponse.StatusCode);
|
||||
|
||||
await VerifyUserConfirmedAsync(acceptedOrgUser, "test-key");
|
||||
await VerifyDefaultCollectionCreatedAsync(acceptedOrgUser);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BulkConfirm_WithValidUsers_ReturnsSuccess()
|
||||
{
|
||||
const string testKeyFormat = "test-key-{0}";
|
||||
await OrganizationTestHelpers.EnableOrganizationDataOwnershipPolicyAsync(_factory, _organization.Id);
|
||||
|
||||
var emails = new[] { "test1@example.com", "test2@example.com", "test3@example.com" };
|
||||
var acceptedUsers = await CreateAcceptedUsersAsync(emails);
|
||||
|
||||
await _loginHelper.LoginAsync(_ownerEmail);
|
||||
|
||||
var bulkConfirmModel = new OrganizationUserBulkConfirmRequestModel
|
||||
{
|
||||
Keys = acceptedUsers.Select((organizationUser, index) => new OrganizationUserBulkConfirmRequestModelEntry
|
||||
{
|
||||
Id = organizationUser.Id,
|
||||
Key = string.Format(testKeyFormat, index)
|
||||
}),
|
||||
DefaultUserCollectionName = _mockEncryptedString
|
||||
};
|
||||
|
||||
var bulkConfirmResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/confirm", bulkConfirmModel);
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, bulkConfirmResponse.StatusCode);
|
||||
|
||||
await VerifyMultipleUsersConfirmedAsync(acceptedUsers.Select((organizationUser, index) =>
|
||||
(organizationUser, string.Format(testKeyFormat, index))).ToList());
|
||||
await VerifyMultipleUsersHaveDefaultCollectionsAsync(acceptedUsers);
|
||||
}
|
||||
|
||||
public Task DisposeAsync()
|
||||
{
|
||||
_client.Dispose();
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
private async Task<List<OrganizationUser>> CreateAcceptedUsersAsync(IEnumerable<string> emails)
|
||||
{
|
||||
var acceptedUsers = new List<OrganizationUser>();
|
||||
|
||||
foreach (var email in emails)
|
||||
{
|
||||
await _factory.LoginWithNewAccount(email);
|
||||
|
||||
var acceptedOrgUser = await OrganizationTestHelpers.CreateUserAsync(_factory, _organization.Id, email,
|
||||
OrganizationUserType.User, userStatusType: OrganizationUserStatusType.Accepted);
|
||||
|
||||
acceptedUsers.Add(acceptedOrgUser);
|
||||
}
|
||||
|
||||
return acceptedUsers;
|
||||
}
|
||||
|
||||
private async Task VerifyDefaultCollectionCreatedAsync(OrganizationUser orgUser)
|
||||
{
|
||||
var collectionRepository = _factory.GetService<ICollectionRepository>();
|
||||
var collections = await collectionRepository.GetManyByUserIdAsync(orgUser.UserId!.Value);
|
||||
Assert.Single(collections);
|
||||
Assert.Equal(_mockEncryptedString, collections.First().Name);
|
||||
}
|
||||
|
||||
private async Task VerifyUserConfirmedAsync(OrganizationUser orgUser, string expectedKey)
|
||||
{
|
||||
await VerifyMultipleUsersConfirmedAsync(new List<(OrganizationUser orgUser, string key)> { (orgUser, expectedKey) });
|
||||
}
|
||||
|
||||
private async Task VerifyMultipleUsersConfirmedAsync(List<(OrganizationUser orgUser, string key)> acceptedOrganizationUsers)
|
||||
{
|
||||
var orgUserRepository = _factory.GetService<IOrganizationUserRepository>();
|
||||
for (int i = 0; i < acceptedOrganizationUsers.Count; i++)
|
||||
{
|
||||
var confirmedUser = await orgUserRepository.GetByIdAsync(acceptedOrganizationUsers[i].orgUser.Id);
|
||||
Assert.Equal(OrganizationUserStatusType.Confirmed, confirmedUser.Status);
|
||||
Assert.Equal(acceptedOrganizationUsers[i].key, confirmedUser.Key);
|
||||
}
|
||||
}
|
||||
|
||||
private async Task VerifyMultipleUsersHaveDefaultCollectionsAsync(List<OrganizationUser> acceptedOrganizationUsers)
|
||||
{
|
||||
var collectionRepository = _factory.GetService<ICollectionRepository>();
|
||||
foreach (var acceptedOrganizationUser in acceptedOrganizationUsers)
|
||||
{
|
||||
var collections = await collectionRepository.GetManyByUserIdAsync(acceptedOrganizationUser.UserId!.Value);
|
||||
Assert.Single(collections);
|
||||
Assert.Equal(_mockEncryptedString, collections.First().Name);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
using System.Diagnostics;
|
||||
using Bit.Api.IntegrationTest.Factories;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Organizations;
|
||||
using Bit.Core.AdminConsole.Repositories;
|
||||
using Bit.Core.Billing.Enums;
|
||||
@@ -148,4 +149,23 @@ public static class OrganizationTestHelpers
|
||||
await groupRepository.CreateAsync(group, new List<CollectionAccessSelection>());
|
||||
return group;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Enables the Organization Data Ownership policy for the specified organization.
|
||||
/// </summary>
|
||||
public static async Task EnableOrganizationDataOwnershipPolicyAsync<T>(
|
||||
WebApplicationFactoryBase<T> factory,
|
||||
Guid organizationId) where T : class
|
||||
{
|
||||
var policyRepository = factory.GetService<IPolicyRepository>();
|
||||
|
||||
var policy = new Policy
|
||||
{
|
||||
OrganizationId = organizationId,
|
||||
Type = PolicyType.OrganizationDataOwnership,
|
||||
Enabled = true
|
||||
};
|
||||
|
||||
await policyRepository.CreateAsync(policy);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -473,7 +473,7 @@ public class ConfirmOrganizationUserCommandTests
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
|
||||
|
||||
sutProvider.GetDependency<IPolicyRequirementQuery>()
|
||||
.GetAsync<OrganizationDataOwnershipPolicyRequirement>(user.Id)
|
||||
.GetByOrganizationAsync<OrganizationDataOwnershipPolicyRequirement>(organization.Id)
|
||||
.Returns(new OrganizationDataOwnershipPolicyRequirement(
|
||||
OrganizationDataOwnershipState.Enabled,
|
||||
[organization.Id]));
|
||||
@@ -482,15 +482,10 @@ public class ConfirmOrganizationUserCommandTests
|
||||
|
||||
await sutProvider.GetDependency<ICollectionRepository>()
|
||||
.Received(1)
|
||||
.CreateAsync(
|
||||
Arg.Is<Collection>(c => c.Name == collectionName &&
|
||||
c.OrganizationId == organization.Id &&
|
||||
c.Type == CollectionType.DefaultUserCollection),
|
||||
Arg.Is<IEnumerable<CollectionAccessSelection>>(groups => groups == null),
|
||||
Arg.Is<IEnumerable<CollectionAccessSelection>>(u =>
|
||||
u.Count() == 1 &&
|
||||
u.First().Id == orgUser.Id &&
|
||||
u.First().Manage == true));
|
||||
.CreateDefaultCollectionsAsync(
|
||||
organization.Id,
|
||||
Arg.Is<IEnumerable<Guid>>(ids => ids.Contains(orgUser.Id)),
|
||||
collectionName);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
@@ -510,7 +505,7 @@ public class ConfirmOrganizationUserCommandTests
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
|
||||
|
||||
sutProvider.GetDependency<IPolicyRequirementQuery>()
|
||||
.GetAsync<OrganizationDataOwnershipPolicyRequirement>(user.Id)
|
||||
.GetByOrganizationAsync<OrganizationDataOwnershipPolicyRequirement>(org.Id)
|
||||
.Returns(new OrganizationDataOwnershipPolicyRequirement(
|
||||
OrganizationDataOwnershipState.Enabled,
|
||||
[org.Id]));
|
||||
@@ -538,7 +533,7 @@ public class ConfirmOrganizationUserCommandTests
|
||||
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
|
||||
|
||||
sutProvider.GetDependency<IPolicyRequirementQuery>()
|
||||
.GetAsync<OrganizationDataOwnershipPolicyRequirement>(user.Id)
|
||||
.GetByOrganizationAsync<OrganizationDataOwnershipPolicyRequirement>(org.Id)
|
||||
.Returns(new OrganizationDataOwnershipPolicyRequirement(
|
||||
OrganizationDataOwnershipState.Enabled,
|
||||
[Guid.NewGuid()]));
|
||||
|
||||
@@ -79,4 +79,73 @@ public class PolicyRequirementQueryTests
|
||||
|
||||
Assert.Empty(requirement.Policies);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetByOrganizationAsync_IgnoresOtherPolicyTypes(Guid organizationId)
|
||||
{
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var thisPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, UserId = Guid.NewGuid() };
|
||||
var otherPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.RequireSso, UserId = Guid.NewGuid() };
|
||||
// Force the repository to return both policies even though that is not the expected result
|
||||
policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg)
|
||||
.Returns([thisPolicy, otherPolicy]);
|
||||
|
||||
var factory = new TestPolicyRequirementFactory(_ => true);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
var requirement = await sut.GetByOrganizationAsync<TestPolicyRequirement>(organizationId);
|
||||
|
||||
await policyRepository.Received(1).GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg);
|
||||
|
||||
Assert.Contains(thisPolicy, requirement.Policies.Cast<OrganizationPolicyDetails>());
|
||||
Assert.DoesNotContain(otherPolicy, requirement.Policies.Cast<OrganizationPolicyDetails>());
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetByOrganizationAsync_CallsEnforceCallback(Guid organizationId)
|
||||
{
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var thisPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, UserId = Guid.NewGuid() };
|
||||
var otherPolicy = new OrganizationPolicyDetails { PolicyType = PolicyType.SingleOrg, UserId = Guid.NewGuid() };
|
||||
policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg).Returns([thisPolicy, otherPolicy]);
|
||||
|
||||
var callback = Substitute.For<Func<PolicyDetails, bool>>();
|
||||
callback(Arg.Any<PolicyDetails>()).Returns(x => x.Arg<PolicyDetails>() == thisPolicy);
|
||||
|
||||
var factory = new TestPolicyRequirementFactory(callback);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
var requirement = await sut.GetByOrganizationAsync<TestPolicyRequirement>(organizationId);
|
||||
|
||||
Assert.Contains(thisPolicy, requirement.Policies.Cast<OrganizationPolicyDetails>());
|
||||
Assert.DoesNotContain(otherPolicy, requirement.Policies.Cast<OrganizationPolicyDetails>());
|
||||
callback.Received()(Arg.Is<PolicyDetails>(p => p == thisPolicy));
|
||||
callback.Received()(Arg.Is<PolicyDetails>(p => p == otherPolicy));
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetByOrganizationAsync_ThrowsIfNoFactoryRegistered(Guid organizationId)
|
||||
{
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var sut = new PolicyRequirementQuery(policyRepository, []);
|
||||
|
||||
var exception = await Assert.ThrowsAsync<NotImplementedException>(()
|
||||
=> sut.GetByOrganizationAsync<TestPolicyRequirement>(organizationId));
|
||||
|
||||
Assert.Contains("No Requirement Factory found", exception.Message);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetByOrganizationAsync_HandlesNoPolicies(Guid organizationId)
|
||||
{
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
policyRepository.GetPolicyDetailsByOrganizationIdAsync(organizationId, PolicyType.SingleOrg).Returns([]);
|
||||
|
||||
var factory = new TestPolicyRequirementFactory(x => x.IsProvider);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
var requirement = await sut.GetByOrganizationAsync<TestPolicyRequirement>(organizationId);
|
||||
|
||||
Assert.Empty(requirement.Policies);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user