nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-06-30 23:52:50 -05:00
Code Activity

[AC-2646] Remove FC MVP dead code from Core (#4281)

Browse Source 
* chore: remove fc refs in CreateGroup and UpdateGroup commands, refs AC-2646

* chore: remove fc refs and update interface to represent usage/get rid of double enumeration warnings, refs AC-2646

* chore: remove org/provider service fc callers, refs AC-2646

* chore: remove collection service fc callers, refs AC-2646

* chore: remove cipher service import ciphers fc callers, refs AC-2646

* fix: UpdateOrganizationUserCommandTests collections to list, refs AC-2646

* fix: update CreateGroupCommandTests, refs AC-2646

* fix: adjust UpdateGroupCommandTests, refs AC-2646

* fix: adjust UpdateOrganizationUserCommandTests for FC always true, refs AC-2646

* fix: update CollectionServiceTests, refs AC-2646

* fix: remove unnecessary test with fc disabled, refs AC-2646

* fix: update tests to account for AccessAll removal and Manager removal, refs AC-2646

* chore: remove dependence on FC flag for tests, refs AC-2646
This commit is contained in:
Vincent Salucci
2024-07-12 12:25:04 -05:00
committed by GitHub
parent 25dc0c9178
commit 02b3453cd5
22 changed files with 167 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/Core/AdminConsole/Entities/OrganizationUser.cs
View File

@ -19,7 +19,11 @@ public class OrganizationUser : ITableObject<Guid>, IExternal
public string? ResetPasswordKey { get; set; }
public OrganizationUserStatusType Status { get; set; }
public OrganizationUserType Type { get; set; }
public bool AccessAll { get; set; }
/// <summary>
/// AccessAll is deprecated and should always be left as false. Scheduled for removal.
/// </summary>
public bool AccessAll { get; set; } = false;
[MaxLength(300)]
public string? ExternalId { get; set; }
public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;

2
src/Core/AdminConsole/Models/Business/OrganizationUserInvite.cs
View File

@ -7,7 +7,6 @@ public class OrganizationUserInvite
{
public IEnumerable<string> Emails { get; set; }
public Enums.OrganizationUserType? Type { get; set; }
public bool AccessAll { get; set; }
public bool AccessSecretsManager { get; set; }
public Permissions Permissions { get; set; }
public IEnumerable<CollectionAccessSelection> Collections { get; set; }
@ -19,7 +18,6 @@ public class OrganizationUserInvite
{
Emails = requestModel.Emails;
Type = requestModel.Type;
AccessAll = requestModel.AccessAll;
AccessSecretsManager = requestModel.AccessSecretsManager;
Collections = requestModel.Collections;
Groups = requestModel.Groups;

1
src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserInviteData.cs
View File

@ -6,7 +6,6 @@ public class OrganizationUserInviteData
{
public IEnumerable<string> Emails { get; set; }
public OrganizationUserType? Type { get; set; }
public bool AccessAll { get; set; }
public bool AccessSecretsManager { get; set; }
public IEnumerable<CollectionAccessSelection> Collections { get; set; }
public IEnumerable<Guid> Groups { get; set; }

17
src/Core/AdminConsole/OrganizationFeatures/Groups/CreateGroupCommand.cs
View File

@ -115,18 +115,15 @@ public class CreateGroupCommand : ICreateGroupCommand
throw new BadRequestException("This organization cannot use groups.");
}
if (organization.FlexibleCollections)
if (group.AccessAll)
{
if (group.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
}

17
src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs
View File

@ -109,18 +109,15 @@ public class UpdateGroupCommand : IUpdateGroupCommand
throw new BadRequestException("This organization cannot use groups.");
}
if (organization.FlexibleCollections)
if (group.AccessAll)
{
if (group.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
}

2
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs
View File

@ -6,5 +6,5 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interface
public interface IUpdateOrganizationUserCommand
{
Task UpdateUserAsync(OrganizationUser user, Guid? savingUserId, IEnumerable<CollectionAccessSelection> collections, IEnumerable<Guid>? groups);
Task UpdateUserAsync(OrganizationUser user, Guid? savingUserId, List<CollectionAccessSelection> collections, IEnumerable<Guid>? groups);
}

15
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs
View File

@ -37,7 +37,7 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand
}
public async Task UpdateUserAsync(OrganizationUser user, Guid? savingUserId,
IEnumerable<CollectionAccessSelection> collections, IEnumerable<Guid>? groups)
List<CollectionAccessSelection>? collections, IEnumerable<Guid>? groups)
{
if (user.Id.Equals(default(Guid)))
{
@ -59,14 +59,12 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand
throw new BadRequestException("Organization must have at least one confirmed owner.");
}
// If the organization is using Flexible Collections, prevent use of any deprecated permissions
var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId);
if (organization.FlexibleCollections && user.AccessAll)
if (user.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead.");
}
if (organization.FlexibleCollections && collections?.Any() == true)
if (collections?.Count > 0)
{
var invalidAssociations = collections.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations.Any())
@ -74,7 +72,6 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
// End Flexible Collections
// Only autoscale (if required) after all validation has passed so that we know it's a valid request before
// updating Stripe
@ -83,17 +80,13 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand
var additionalSmSeatsRequired = await _countNewSmSeatsRequiredQuery.CountNewSmSeatsRequiredAsync(user.OrganizationId, 1);
if (additionalSmSeatsRequired > 0)
{
var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId);
var update = new SecretsManagerSubscriptionUpdate(organization, true)
.AdjustSeats(additionalSmSeatsRequired);
await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update);
}
}
if (user.AccessAll)
{
// We don't need any collections if we're flagged to have all access.
collections = new List<CollectionAccessSelection>();
}
await _organizationUserRepository.ReplaceAsync(user, collections);
if (groups != null)

57
src/Core/AdminConsole/Services/Implementations/OrganizationService.cs
View File

@ -743,10 +743,6 @@ public class OrganizationService : IOrganizationService
AccessSecretsManager = organization.UseSecretsManager,
Type = OrganizationUserType.Owner,
Status = OrganizationUserStatusType.Confirmed,
// If using Flexible Collections, AccessAll is deprecated and set to false.
// If not using Flexible Collections, set AccessAll to true (previous behavior)
AccessAll = !organization.FlexibleCollections,
CreationDate = organization.CreationDate,
RevisionDate = organization.CreationDate
};
@ -771,9 +767,9 @@ public class OrganizationService : IOrganizationService
RevisionDate = organization.CreationDate
};
// If using Flexible Collections, give the owner Can Manage access over the default collection
// Give the owner Can Manage access over the default collection
List<CollectionAccessSelection> defaultOwnerAccess = null;
if (orgUser != null && organization.FlexibleCollections)
if (orgUser != null)
{
defaultOwnerAccess =
[new CollectionAccessSelection { Id = orgUser.Id, HidePasswords = false, ReadOnly = false, Manage = true }];
@ -965,15 +961,11 @@ public class OrganizationService : IOrganizationService
throw new BadRequestException("This method can only be used to invite a single user.");
}
// Validate Collection associations if org is using latest collection enhancements
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId);
if (organizationAbility?.FlexibleCollections ?? false)
// Validate Collection associations
var invalidAssociations = invite.Collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
var invalidAssociations = invite.Collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
var results = await InviteUsersAsync(organizationId, invitingUserId, systemUser,
@ -1038,13 +1030,6 @@ public class OrganizationService : IOrganizationService
throw new NotFoundException();
}
// If the organization is using Flexible Collections, prevent use of any deprecated permissions
if (organization.FlexibleCollections && invites.Any(i => i.invite.AccessAll))
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead.");
}
// End Flexible Collections
var existingEmails = new HashSet<string>(await _organizationUserRepository.SelectKnownEmailsAsync(
organizationId, invites.SelectMany(i => i.invite.Emails), false), StringComparer.InvariantCultureIgnoreCase);
@ -1087,8 +1072,8 @@ public class OrganizationService : IOrganizationService
throw new BadRequestException("Organization must have at least one confirmed owner.");
}
var orgUsers = new List<OrganizationUser>();
var limitedCollectionOrgUsers = new List<(OrganizationUser, IEnumerable<CollectionAccessSelection>)>();
var orgUsersWithoutCollections = new List<OrganizationUser>();
var orgUsersWithCollections = new List<(OrganizationUser, IEnumerable<CollectionAccessSelection>)>();
var orgUserGroups = new List<(OrganizationUser, IEnumerable<Guid>)>();
var orgUserInvitedCount = 0;
var exceptions = new List<Exception>();
@ -1114,7 +1099,6 @@ public class OrganizationService : IOrganizationService
Key = null,
Type = invite.Type.Value,
Status = OrganizationUserStatusType.Invited,
AccessAll = invite.AccessAll,
AccessSecretsManager = invite.AccessSecretsManager,
ExternalId = externalId,
CreationDate = DateTime.UtcNow,
@ -1126,13 +1110,13 @@ public class OrganizationService : IOrganizationService
orgUser.SetPermissions(invite.Permissions ?? new Permissions());
}
if (!orgUser.AccessAll && invite.Collections.Any())
if (invite.Collections.Any())
{
limitedCollectionOrgUsers.Add((orgUser, invite.Collections));
orgUsersWithCollections.Add((orgUser, invite.Collections));
}
else
{
orgUsers.Add(orgUser);
orgUsersWithoutCollections.Add(orgUser);
}
if (invite.Groups != null && invite.Groups.Any())
@ -1155,10 +1139,14 @@ public class OrganizationService : IOrganizationService
throw new AggregateException("One or more errors occurred while inviting users.", exceptions);
}
var allOrgUsers = orgUsersWithoutCollections
.Concat(orgUsersWithCollections.Select(u => u.Item1))
.ToList();
try
{
await _organizationUserRepository.CreateManyAsync(orgUsers);
foreach (var (orgUser, collections) in limitedCollectionOrgUsers)
await _organizationUserRepository.CreateManyAsync(orgUsersWithoutCollections);
foreach (var (orgUser, collections) in orgUsersWithCollections)
{
await _organizationUserRepository.CreateAsync(orgUser, collections);
}
@ -1180,7 +1168,7 @@ public class OrganizationService : IOrganizationService
await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(smSubscriptionUpdate);
}
await SendInvitesAsync(orgUsers.Concat(limitedCollectionOrgUsers.Select(u => u.Item1)), organization);
await SendInvitesAsync(allOrgUsers, organization);
await _referenceEventService.RaiseEventAsync(
new ReferenceEvent(ReferenceEventType.InvitedUsers, organization, _currentContext)
@ -1191,7 +1179,7 @@ public class OrganizationService : IOrganizationService
catch (Exception e)
{
// Revert any added users.
var invitedOrgUserIds = orgUsers.Select(u => u.Id).Concat(limitedCollectionOrgUsers.Select(u => u.Item1.Id));
var invitedOrgUserIds = allOrgUsers.Select(ou => ou.Id);
await _organizationUserRepository.DeleteManyAsync(invitedOrgUserIds);
var currentOrganization = await _organizationRepository.GetByIdAsync(organization.Id);
@ -1220,7 +1208,7 @@ public class OrganizationService : IOrganizationService
throw new AggregateException("One or more errors occurred while inviting users.", exceptions);
}
return (orgUsers, events);
return (allOrgUsers, events);
}
public async Task<IEnumerable<Tuple<OrganizationUser, string>>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId,
@ -1811,7 +1799,6 @@ public class OrganizationService : IOrganizationService
{
Emails = new List<string> { user.Email },
Type = OrganizationUserType.User,
AccessAll = false,
Collections = new List<CollectionAccessSelection>(),
AccessSecretsManager = hasStandaloneSecretsManager
};
@ -2519,10 +2506,6 @@ public class OrganizationService : IOrganizationService
Key = null,
Type = OrganizationUserType.Owner,
Status = OrganizationUserStatusType.Invited,
// If using Flexible Collections, AccessAll is deprecated and set to false.
// If not using Flexible Collections, set AccessAll to true (previous behavior)
AccessAll = !organization.FlexibleCollections,
};
await _organizationUserRepository.CreateAsync(ownerOrganizationUser);

29
src/Core/Services/Implementations/CollectionService.cs
View File

@ -55,25 +55,22 @@ public class CollectionService : ICollectionService
var groupsList = groups?.ToList();
var usersList = users?.ToList();
if (org.FlexibleCollections)
// Cannot use Manage with ReadOnly/HidePasswords permissions
var invalidAssociations = groupsList?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
// Cannot use Manage with ReadOnly/HidePasswords permissions
var invalidAssociations = groupsList?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
// If using Flexible Collections V1 - a collection should always have someone with Can Manage permissions
if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
// If using Flexible Collections V1 - a collection should always have someone with Can Manage permissions
if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
{
var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems)
{
var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems)
{
throw new BadRequestException(
"At least one member or group must have can manage permission.");
}
throw new BadRequestException(
"At least one member or group must have can manage permission.");
}
}

13
src/Core/Vault/Services/Implementations/CipherService.cs
View File

@ -788,15 +788,12 @@ public class CipherService : ICipherService
{
collection.SetNewId();
newCollections.Add(collection);
if (org.FlexibleCollections)
newCollectionUsers.Add(new CollectionUser
{
newCollectionUsers.Add(new CollectionUser
{
CollectionId = collection.Id,
OrganizationUserId = importingOrgUser.Id,
Manage = true
});
}
CollectionId = collection.Id,
OrganizationUserId = importingOrgUser.Id,
Manage = true
});
}
}