1
0
mirror of https://github.com/bitwarden/server.git synced 2025-06-18 01:53:49 -05:00

[PM-22098] Create default collection when organization member is confirmed (#5944)

* Add RequiresDefaultCollection method to PersonalOwnershipPolicyRequirement

* Add CreateDefaultLocation feature flag to Constants.cs

* Add DefaultUserCollectionName property to OrganizationUserConfirmRequestModel with encryption attributes

* Update PersonalOwnershipPolicyRequirement instantiation in tests to use constructor with parameters instead of property assignment

* Enhance ConfirmOrganizationUserCommand to support default user collection creation. Added logic to check if a default collection is required based on organization policies and feature flags. Updated ConfirmUserAsync method signature to include an optional defaultUserCollectionName parameter. Added corresponding tests to validate the new functionality.

* Refactor Confirm method in OrganizationUsersController to use Guid parameters directly, simplifying the code. Updated ConfirmUserAsync call to include DefaultUserCollectionName from the input model.

* Move logic for handling confirmation side effects into a separate method

* Refactor PersonalOwnershipPolicyRequirement to use enum for ownership state

- Introduced PersonalOwnershipState enum to represent allowed and restricted states.
- Updated PersonalOwnershipPolicyRequirement constructor and properties to utilize the new enum.
- Modified related classes and tests to reflect changes in ownership state handling.
This commit is contained in:
Rui Tomé 2025-06-17 12:20:22 +01:00 committed by GitHub
parent b8244908ec
commit 5ffa937914
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 254 additions and 21 deletions

View File

@ -403,16 +403,15 @@ public class OrganizationUsersController : Controller
}
[HttpPost("{id}/confirm")]
public async Task Confirm(string orgId, string id, [FromBody] OrganizationUserConfirmRequestModel model)
public async Task Confirm(Guid orgId, Guid id, [FromBody] OrganizationUserConfirmRequestModel model)
{
var orgGuidId = new Guid(orgId);
if (!await _currentContext.ManageUsers(orgGuidId))
if (!await _currentContext.ManageUsers(orgId))
{
throw new NotFoundException();
}
var userId = _userService.GetProperUserId(User);
var result = await _confirmOrganizationUserCommand.ConfirmUserAsync(orgGuidId, new Guid(id), model.Key, userId.Value);
var result = await _confirmOrganizationUserCommand.ConfirmUserAsync(orgId, id, model.Key, userId.Value, model.DefaultUserCollectionName);
}
[HttpPost("confirm")]

View File

@ -60,6 +60,10 @@ public class OrganizationUserConfirmRequestModel
{
[Required]
public string Key { get; set; }
[EncryptedString]
[EncryptedStringLength(1000)]
public string DefaultUserCollectionName { get; set; }
}
public class OrganizationUserBulkConfirmRequestModelEntry

View File

@ -8,6 +8,7 @@ using Bit.Core.Billing.Enums;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Platform.Push;
using Bit.Core.Repositories;
using Bit.Core.Services;
@ -28,6 +29,7 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
private readonly IDeviceRepository _deviceRepository;
private readonly IPolicyRequirementQuery _policyRequirementQuery;
private readonly IFeatureService _featureService;
private readonly ICollectionRepository _collectionRepository;
public ConfirmOrganizationUserCommand(
IOrganizationRepository organizationRepository,
@ -41,7 +43,8 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
IPolicyService policyService,
IDeviceRepository deviceRepository,
IPolicyRequirementQuery policyRequirementQuery,
IFeatureService featureService)
IFeatureService featureService,
ICollectionRepository collectionRepository)
{
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
@ -55,10 +58,11 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
_deviceRepository = deviceRepository;
_policyRequirementQuery = policyRequirementQuery;
_featureService = featureService;
_collectionRepository = collectionRepository;
}
public async Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key,
Guid confirmingUserId)
Guid confirmingUserId, string defaultUserCollectionName = null)
{
var result = await ConfirmUsersAsync(
organizationId,
@ -75,6 +79,9 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
{
throw new BadRequestException(error);
}
await HandleConfirmationSideEffectsAsync(organizationId, orgUser, defaultUserCollectionName);
return orgUser;
}
@ -213,4 +220,54 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand
.Where(d => !string.IsNullOrWhiteSpace(d.PushToken))
.Select(d => d.Id.ToString());
}
private async Task HandleConfirmationSideEffectsAsync(Guid organizationId, OrganizationUser organizationUser, string defaultUserCollectionName)
{
// Create DefaultUserCollection type collection for the user if the PersonalOwnership policy is enabled for the organization
var requiresDefaultCollection = await OrganizationRequiresDefaultCollectionAsync(organizationId, organizationUser.UserId.Value, defaultUserCollectionName);
if (requiresDefaultCollection)
{
await CreateDefaultCollectionAsync(organizationId, organizationUser.Id, defaultUserCollectionName);
}
}
private async Task<bool> OrganizationRequiresDefaultCollectionAsync(Guid organizationId, Guid userId, string defaultUserCollectionName)
{
if (!_featureService.IsEnabled(FeatureFlagKeys.CreateDefaultLocation))
{
return false;
}
// Skip if no collection name provided (backwards compatibility)
if (string.IsNullOrWhiteSpace(defaultUserCollectionName))
{
return false;
}
var personalOwnershipRequirement = await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(userId);
return personalOwnershipRequirement.RequiresDefaultCollection(organizationId);
}
private async Task CreateDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName)
{
var collection = new Collection
{
OrganizationId = organizationId,
Name = defaultCollectionName,
Type = CollectionType.DefaultUserCollection
};
var userAccess = new List<CollectionAccessSelection>
{
new CollectionAccessSelection
{
Id = organizationUserId,
ReadOnly = false,
HidePasswords = false,
Manage = true
}
};
await _collectionRepository.CreateAsync(collection, groups: null, users: userAccess);
}
}

View File

@ -15,9 +15,10 @@ public interface IConfirmOrganizationUserCommand
/// <param name="organizationUserId">The ID of the organization user to confirm.</param>
/// <param name="key">The encrypted organization key for the user.</param>
/// <param name="confirmingUserId">The ID of the user performing the confirmation.</param>
/// <param name="defaultUserCollectionName">Optional encrypted collection name for creating a default collection.</param>
/// <returns>The confirmed organization user.</returns>
/// <exception cref="BadRequestException">Thrown when the user is not valid or cannot be confirmed.</exception>
Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId);
Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId, string defaultUserCollectionName = null);
/// <summary>
/// Confirms multiple organization users who have accepted their invitations.

View File

@ -3,15 +3,55 @@ using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
/// <summary>
/// Represents the personal ownership policy state.
/// </summary>
public enum PersonalOwnershipState
{
/// <summary>
/// Personal ownership is allowed - users can save items to their personal vault.
/// </summary>
Allowed,
/// <summary>
/// Personal ownership is restricted - members are required to save items to an organization.
/// </summary>
Restricted
}
/// <summary>
/// Policy requirements for the Disable Personal Ownership policy.
/// </summary>
public class PersonalOwnershipPolicyRequirement : IPolicyRequirement
{
private readonly IEnumerable<Guid> _organizationIdsWithPolicyEnabled;
/// <param name="personalOwnershipState">
/// The personal ownership state for the user.
/// </param>
/// <param name="organizationIdsWithPolicyEnabled">
/// The collection of Organization IDs that have the Disable Personal Ownership policy enabled.
/// </param>
public PersonalOwnershipPolicyRequirement(
PersonalOwnershipState personalOwnershipState,
IEnumerable<Guid> organizationIdsWithPolicyEnabled)
{
_organizationIdsWithPolicyEnabled = organizationIdsWithPolicyEnabled ?? [];
State = personalOwnershipState;
}
/// <summary>
/// Indicates whether Personal Ownership is disabled for the user. If true, members are required to save items to an organization.
/// The personal ownership policy state for the user.
/// </summary>
public bool DisablePersonalOwnership { get; init; }
public PersonalOwnershipState State { get; }
/// <summary>
/// Returns true if the Disable Personal Ownership policy is enforced in that organization.
/// </summary>
public bool RequiresDefaultCollection(Guid organizationId)
{
return _organizationIdsWithPolicyEnabled.Contains(organizationId);
}
}
public class PersonalOwnershipPolicyRequirementFactory : BasePolicyRequirementFactory<PersonalOwnershipPolicyRequirement>
@ -20,7 +60,13 @@ public class PersonalOwnershipPolicyRequirementFactory : BasePolicyRequirementFa
public override PersonalOwnershipPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
{
var result = new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = policyDetails.Any() };
return result;
var personalOwnershipState = policyDetails.Any()
? PersonalOwnershipState.Restricted
: PersonalOwnershipState.Allowed;
var organizationIdsWithPolicyEnabled = policyDetails.Select(p => p.OrganizationId).ToHashSet();
return new PersonalOwnershipPolicyRequirement(
personalOwnershipState,
organizationIdsWithPolicyEnabled);
}
}

View File

@ -112,6 +112,7 @@ public static class FeatureFlagKeys
public const string EventBasedOrganizationIntegrations = "event-based-organization-integrations";
public const string OptimizeNestedTraverseTypescript = "pm-21695-optimize-nested-traverse-typescript";
public const string SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions";
public const string CreateDefaultLocation = "pm-19467-create-default-location";
/* Auth Team */
public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence";

View File

@ -56,7 +56,7 @@ public class ImportCiphersCommand : IImportCiphersCommand
{
// Make sure the user can save new ciphers to their personal vault
var isPersonalVaultRestricted = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(importingUserId)).DisablePersonalOwnership
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(importingUserId)).State == PersonalOwnershipState.Restricted
: await _policyService.AnyPoliciesApplicableToUserAsync(importingUserId, PolicyType.PersonalOwnership);
if (isPersonalVaultRestricted)

View File

@ -143,7 +143,7 @@ public class CipherService : ICipherService
else
{
var isPersonalVaultRestricted = _featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(savingUserId)).DisablePersonalOwnership
? (await _policyRequirementQuery.GetAsync<PersonalOwnershipPolicyRequirement>(savingUserId)).State == PersonalOwnershipState.Restricted
: await _policyService.AnyPoliciesApplicableToUserAsync(savingUserId, PolicyType.PersonalOwnership);
if (isPersonalVaultRestricted)

View File

@ -10,6 +10,7 @@ using Bit.Core.Billing.Enums;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Services;
@ -442,4 +443,98 @@ public class ConfirmOrganizationUserCommandTests
await sutProvider.GetDependency<IMailService>().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email, orgUser.AccessSecretsManager);
await organizationUserRepository.Received(1).ReplaceManyAsync(Arg.Is<List<OrganizationUser>>(users => users.Contains(orgUser) && users.Count == 1));
}
[Theory, BitAutoData]
public async Task ConfirmUserAsync_WithCreateDefaultLocationEnabled_WithPersonalOwnershipPolicyApplicable_WithValidCollectionName_CreatesDefaultCollection(
Organization organization, OrganizationUser confirmingUser,
[OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user,
string key, string collectionName, SutProvider<ConfirmOrganizationUserCommand> sutProvider)
{
organization.PlanType = PlanType.EnterpriseAnnually;
orgUser.OrganizationId = confirmingUser.OrganizationId = organization.Id;
orgUser.UserId = user.Id;
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
sutProvider.GetDependency<IUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { user });
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(user.Id)
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Restricted,
[organization.Id]));
await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName);
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));
}
[Theory, BitAutoData]
public async Task ConfirmUserAsync_WithCreateDefaultLocationEnabled_WithPersonalOwnershipPolicyApplicable_WithInvalidCollectionName_DoesNotCreateDefaultCollection(
Organization org, OrganizationUser confirmingUser,
[OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user,
string key, SutProvider<ConfirmOrganizationUserCommand> sutProvider)
{
org.PlanType = PlanType.EnterpriseAnnually;
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
orgUser.UserId = user.Id;
sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(org.Id).Returns(org);
sutProvider.GetDependency<IUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { user });
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(user.Id)
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Restricted,
[org.Id]));
await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, "");
await sutProvider.GetDependency<ICollectionRepository>()
.DidNotReceive()
.CreateAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(), Arg.Any<IEnumerable<CollectionAccessSelection>>());
}
[Theory, BitAutoData]
public async Task ConfirmUserAsync_WithCreateDefaultLocationEnabled_WithPersonalOwnershipPolicyNotApplicable_DoesNotCreateDefaultCollection(
Organization org, OrganizationUser confirmingUser,
[OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user,
string key, string collectionName, SutProvider<ConfirmOrganizationUserCommand> sutProvider)
{
org.PlanType = PlanType.EnterpriseAnnually;
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
orgUser.UserId = user.Id;
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(org.Id).Returns(org);
sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
sutProvider.GetDependency<IUserRepository>().GetManyAsync(default).ReturnsForAnyArgs(new[] { user });
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.CreateDefaultLocation).Returns(true);
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(user.Id)
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Restricted,
[Guid.NewGuid()]));
await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, collectionName);
await sutProvider.GetDependency<ICollectionRepository>()
.DidNotReceive()
.CreateAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(), Arg.Any<IEnumerable<CollectionAccessSelection>>());
}
}

View File

@ -12,20 +12,42 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequire
public class PersonalOwnershipPolicyRequirementFactoryTests
{
[Theory, BitAutoData]
public void DisablePersonalOwnership_WithNoPolicies_ReturnsFalse(SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
public void State_WithNoPolicies_ReturnsAllowed(SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
{
var actual = sutProvider.Sut.Create([]);
Assert.False(actual.DisablePersonalOwnership);
Assert.Equal(PersonalOwnershipState.Allowed, actual.State);
}
[Theory, BitAutoData]
public void DisablePersonalOwnership_WithPersonalOwnershipPolicies_ReturnsTrue(
public void State_WithPersonalOwnershipPolicies_ReturnsRestricted(
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails[] policies,
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
{
var actual = sutProvider.Sut.Create(policies);
Assert.True(actual.DisablePersonalOwnership);
Assert.Equal(PersonalOwnershipState.Restricted, actual.State);
}
[Theory, BitAutoData]
public void RequiresDefaultCollection_WithNoPolicies_ReturnsFalse(
Guid organizationId,
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
{
var actual = sutProvider.Sut.Create([]);
Assert.False(actual.RequiresDefaultCollection(organizationId));
}
[Theory, BitAutoData]
public void RequiresDefaultCollection_WithPersonalOwnershipPolicies_ReturnsCorrectResult(
[PolicyDetails(PolicyType.PersonalOwnership)] PolicyDetails[] policies,
Guid nonPolicyOrganizationId,
SutProvider<PersonalOwnershipPolicyRequirementFactory> sutProvider)
{
var actual = sutProvider.Sut.Create(policies);
Assert.True(actual.RequiresDefaultCollection(policies[0].OrganizationId));
Assert.False(actual.RequiresDefaultCollection(nonPolicyOrganizationId));
}
}

View File

@ -62,7 +62,9 @@ public class ImportCiphersAsyncCommandTests
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(importingUserId)
.Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = false });
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Allowed,
[]));
sutProvider.GetDependency<IFolderRepository>()
.GetManyByUserIdAsync(importingUserId)
@ -116,7 +118,9 @@ public class ImportCiphersAsyncCommandTests
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(userId)
.Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = true });
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Restricted,
[Guid.NewGuid()]));
var folderRelationships = new List<KeyValuePair<int, int>>();

View File

@ -171,7 +171,9 @@ public class CipherServiceTests
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(savingUserId)
.Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = true });
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Restricted,
[Guid.NewGuid()]));
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null));
@ -195,7 +197,9 @@ public class CipherServiceTests
sutProvider.GetDependency<IPolicyRequirementQuery>()
.GetAsync<PersonalOwnershipPolicyRequirement>(savingUserId)
.Returns(new PersonalOwnershipPolicyRequirement { DisablePersonalOwnership = false });
.Returns(new PersonalOwnershipPolicyRequirement(
PersonalOwnershipState.Allowed,
[]));
await sutProvider.Sut.SaveDetailsAsync(cipher, savingUserId, null);