Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ await collectionRepository.CreateDefaultCollectionsAsync(
/// <returns>The result is a boolean value indicating whether a default collection should be created.</returns>
private async Task<bool> ShouldCreateDefaultCollectionAsync(AutomaticallyConfirmOrganizationUserValidationRequest request) =>
!string.IsNullOrWhiteSpace(request.DefaultUserCollectionName)
&& request.Organization!.UseMyItems
&& (await policyRequirementQuery.GetAsync<OrganizationDataOwnershipPolicyRequirement>(request.OrganizationUser!.UserId!.Value))
.RequiresDefaultCollectionOnConfirm(request.Organization!.Id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,13 @@ public ConfirmOrganizationUserCommand(
public async Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key,
Guid confirmingUserId, string defaultUserCollectionName = null)
{
var organization = await _organizationRepository.GetByIdAsync(organizationId);

var result = await SaveChangesToDatabaseAsync(
organizationId,
new Dictionary<Guid, string>() { { organizationUserId, key } },
confirmingUserId);
confirmingUserId,
organization);

if (!result.Any())
{
Expand All @@ -88,15 +91,17 @@ public async Task<OrganizationUser> ConfirmUserAsync(Guid organizationId, Guid o
throw new BadRequestException(error);
}

await CreateDefaultCollectionAsync(orgUser, defaultUserCollectionName);
await CreateDefaultCollectionAsync(orgUser, organization, defaultUserCollectionName);

return orgUser;
}

public async Task<List<Tuple<OrganizationUser, string>>> ConfirmUsersAsync(Guid organizationId, Dictionary<Guid, string> keys,
Guid confirmingUserId, string defaultUserCollectionName = null)
{
var result = await SaveChangesToDatabaseAsync(organizationId, keys, confirmingUserId);
var organization = await _organizationRepository.GetByIdAsync(organizationId);

var result = await SaveChangesToDatabaseAsync(organizationId, keys, confirmingUserId, organization);

var confirmedOrganizationUsers = result
.Where(r => string.IsNullOrEmpty(r.Item2))
Expand All @@ -105,18 +110,18 @@ public async Task<List<Tuple<OrganizationUser, string>>> ConfirmUsersAsync(Guid

if (confirmedOrganizationUsers.Count == 1)
{
await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), defaultUserCollectionName);
await CreateDefaultCollectionAsync(confirmedOrganizationUsers.Single(), organization, defaultUserCollectionName);
}
else if (confirmedOrganizationUsers.Count > 1)
{
await CreateManyDefaultCollectionsAsync(organizationId, confirmedOrganizationUsers, defaultUserCollectionName);
await CreateManyDefaultCollectionsAsync(organization, confirmedOrganizationUsers, defaultUserCollectionName);
}

return result;
}

private async Task<List<Tuple<OrganizationUser, string>>> SaveChangesToDatabaseAsync(Guid organizationId, Dictionary<Guid, string> keys,
Guid confirmingUserId)
Guid confirmingUserId, Organization organization)
{
var selectedOrganizationUsers = await _organizationUserRepository.GetManyAsync(keys.Keys);
var validSelectedOrganizationUsers = selectedOrganizationUsers
Expand All @@ -129,8 +134,6 @@ private async Task<List<Tuple<OrganizationUser, string>>> SaveChangesToDatabaseA
}

var validSelectedUserIds = validSelectedOrganizationUsers.Select(u => u.UserId.Value).ToList();

var organization = await _organizationRepository.GetByIdAsync(organizationId);
var allUsersOrgs = await _organizationUserRepository.GetManyByManyUsersAsync(validSelectedUserIds);

var users = await _userRepository.GetManyAsync(validSelectedUserIds);
Expand Down Expand Up @@ -278,15 +281,22 @@ private async Task<IEnumerable<string>> GetUserDeviceIdsAsync(Guid userId)
/// Creates a default collection for a single user if required by the Organization Data Ownership policy.
/// </summary>
/// <param name="organizationUser">The organization user who has just been confirmed.</param>
/// <param name="organization">The organization.</param>
/// <param name="defaultUserCollectionName">The encrypted default user collection name.</param>
private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUser, string defaultUserCollectionName)
private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUser, Organization organization, string defaultUserCollectionName)
{
// Skip if no collection name provided (backwards compatibility)
if (string.IsNullOrWhiteSpace(defaultUserCollectionName))
{
return;
}

// Skip if organization has disabled My Items
if (!organization.UseMyItems)
{
return;
}

var organizationDataOwnershipPolicy = await _policyRequirementQuery.GetAsync<OrganizationDataOwnershipPolicyRequirement>(organizationUser.UserId!.Value);
if (!organizationDataOwnershipPolicy.RequiresDefaultCollectionOnConfirm(organizationUser.OrganizationId))
{
Expand All @@ -302,10 +312,10 @@ await _collectionRepository.CreateDefaultCollectionsAsync(
/// <summary>
/// Creates default collections for multiple users if required by the Organization Data Ownership policy.
/// </summary>
/// <param name="organizationId">The organization ID.</param>
/// <param name="organization">The organization.</param>
/// <param name="confirmedOrganizationUsers">The confirmed organization users.</param>
/// <param name="defaultUserCollectionName">The encrypted default user collection name.</param>
private async Task CreateManyDefaultCollectionsAsync(Guid organizationId,
private async Task CreateManyDefaultCollectionsAsync(Organization organization,
IEnumerable<OrganizationUser> confirmedOrganizationUsers, string defaultUserCollectionName)
{
// Skip if no collection name provided (backwards compatibility)
Expand All @@ -314,8 +324,14 @@ private async Task CreateManyDefaultCollectionsAsync(Guid organizationId,
return;
}

// Skip if organization has disabled My Items
if (!organization.UseMyItems)
{
return;
}

var policyEligibleOrganizationUserIds = await _policyRequirementQuery
.GetManyByOrganizationIdAsync<OrganizationDataOwnershipPolicyRequirement>(organizationId);
.GetManyByOrganizationIdAsync<OrganizationDataOwnershipPolicyRequirement>(organization.Id);

var eligibleOrganizationUserIds = confirmedOrganizationUsers
.Where(ou => policyEligibleOrganizationUserIds.Contains(ou.Id))
Expand All @@ -327,7 +343,7 @@ private async Task CreateManyDefaultCollectionsAsync(Guid organizationId,
return;
}

await _collectionRepository.CreateDefaultCollectionsAsync(organizationId, eligibleOrganizationUserIds, defaultUserCollectionName);
await _collectionRepository.CreateDefaultCollectionsAsync(organization.Id, eligibleOrganizationUserIds, defaultUserCollectionName);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser,
await organizationUserRepository.RestoreAsync(organizationUser.Id, status);

if (organizationUser.UserId.HasValue
&& organization.UseMyItems
&& (await policyRequirementQuery.GetAsync<OrganizationDataOwnershipPolicyRequirement>(organizationUser.UserId.Value)).State == OrganizationDataOwnershipState.Enabled
&& status == OrganizationUserStatusType.Confirmed
&& featureService.IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore)
Expand Down Expand Up @@ -253,20 +254,25 @@ public async Task<List<Tuple<OrganizationUser, string>>> RestoreUsersAsync(Guid

if (featureService.IsEnabled(FeatureFlagKeys.DefaultUserCollectionRestore))
{
await CreateDefaultCollectionsForConfirmedUsersAsync(organizationId, defaultCollectionName,
await CreateDefaultCollectionsForConfirmedUsersAsync(organization, defaultCollectionName,
result.Where(r => r.Item2 == "").Select(x => x.Item1).ToList());
}

return result;
}

private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizationId, string defaultCollectionName,
private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Organization organization, string defaultCollectionName,
ICollection<OrganizationUser> restoredUsers)
{
if (!organization.UseMyItems)
{
return;
}

if (!string.IsNullOrWhiteSpace(defaultCollectionName))
{
var organizationUsersDataOwnershipEnabled = (await policyRequirementQuery
.GetManyByOrganizationIdAsync<OrganizationDataOwnershipPolicyRequirement>(organizationId))
.GetManyByOrganizationIdAsync<OrganizationDataOwnershipPolicyRequirement>(organization.Id))
.ToList();

var usersToCreateDefaultCollectionsFor = restoredUsers.Where(x =>
Expand All @@ -275,7 +281,7 @@ private async Task CreateDefaultCollectionsForConfirmedUsersAsync(Guid organizat

if (usersToCreateDefaultCollectionsFor.Count != 0)
{
await collectionRepository.CreateDefaultCollectionsAsync(organizationId,
await collectionRepository.CreateDefaultCollectionsAsync(organization.Id,
usersToCreateDefaultCollectionsFor.Select(x => x.Id),
defaultCollectionName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators;
public class OrganizationDataOwnershipPolicyValidator(
IPolicyRepository policyRepository,
ICollectionRepository collectionRepository,
IOrganizationRepository organizationRepository,
IEnumerable<IPolicyRequirementFactory<IPolicyRequirement>> factories)
: OrganizationPolicyValidator(policyRepository, factories), IPostSavePolicySideEffect, IOnPolicyPostUpdateEvent
{
Expand Down Expand Up @@ -52,6 +53,20 @@ public async Task ExecuteSideEffectsAsync(

private async Task UpsertDefaultCollectionsForUsersAsync(PolicyUpdate policyUpdate, string defaultCollectionName)
{
// FIXME: we should use the organizationAbility cache here, but it is currently flaky
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @eliykat , I’m curious, how are we tracking this? I totally agree with the decision here, but I want to make sure it doesn’t get lost. Maybe we should add this to the ability cache initiative. It doesn’t have to be included in epic PM-32104.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I do think that epic is probably the best place to track this (and any other instance where we've sidestepped the cache). I'll add a ticket as follow-up work after release of that feature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I added it to the tech spec.

// and it's not obvious how to handle a cache failure.
// https://bitwarden.atlassian.net/browse/PM-32699
var organization = await organizationRepository.GetByIdAsync(policyUpdate.OrganizationId);
if (organization == null)
{
throw new InvalidOperationException($"Organization with ID {policyUpdate.OrganizationId} not found.");
}

if (!organization.UseMyItems)
{
return;
}

var requirements = await GetUserPolicyRequirementsByOrganizationIdAsync<OrganizationDataOwnershipPolicyRequirement>(policyUpdate.OrganizationId, policyUpdate.Type);

var userOrgIds = requirements
Expand Down
Loading
Loading