mirror of
https://github.com/bitwarden/server.git
synced 2025-07-01 16:12:49 -05:00
[AC-2027] Update Flexible Collections logic to use organization property (#3644)
* Update optionality to use org.FlexibleCollections Also break old feature flag key to ensure it's never enabled * Add logic to set defaults for collection management setting * Update optionality logic to use org property * Add comments * Add helper method for getting individual orgAbility * Fix validate user update permissions interface * Fix tests * dotnet format * Fix more tests * Simplify self-hosted update logic * Fix mapping * Use new getOrganizationAbility method * Refactor invite and save orgUser methods Pass in whole organization object instead of using OrganizationAbility * fix CipherService tests * dotnet format * Remove manager check to simplify this set of changes * Misc cleanup before review * Fix undefined variable * Refactor bulk-access endpoint to avoid early repo call * Restore manager check * Add tests for UpdateOrganizationLicenseCommand * Add nullable regions * Delete unused dependency * dotnet format * Fix test
This commit is contained in:
Thomas Rittson
@ -15,6 +15,7 @@ using Bit.Core.Enums;
|
||||
using Bit.Core.Exceptions;
|
||||
using Bit.Core.Models.Business;
|
||||
using Bit.Core.Models.Data;
|
||||
using Bit.Core.Models.Data.Organizations;
|
||||
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
|
||||
using Bit.Core.Models.Mail;
|
||||
using Bit.Core.Models.StaticStore;
|
||||
@ -972,21 +973,23 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task InviteUser_WithFCEnabled_WhenInvitingManager_Throws(Organization organization, OrganizationUserInvite invite,
|
||||
OrganizationUser invitor, SutProvider<OrganizationService> sutProvider)
|
||||
public async Task InviteUser_WithFCEnabled_WhenInvitingManager_Throws(OrganizationAbility organizationAbility,
|
||||
OrganizationUserInvite invite, OrganizationUser invitor, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
invite.Type = OrganizationUserType.Manager;
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
|
||||
sutProvider.GetDependency<IFeatureService>()
|
||||
.IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>())
|
||||
.Returns(true);
|
||||
sutProvider.GetDependency<IApplicationCacheService>()
|
||||
.GetOrganizationAbilityAsync(organizationAbility.Id)
|
||||
.Returns(organizationAbility);
|
||||
|
||||
sutProvider.GetDependency<ICurrentContext>()
|
||||
.ManageUsers(organization.Id)
|
||||
.ManageUsers(organizationAbility.Id)
|
||||
.Returns(true);
|
||||
|
||||
var exception = await Assert.ThrowsAsync<BadRequestException>(
|
||||
() => sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new (OrganizationUserInvite, string)[] { (invite, null) }));
|
||||
() => sutProvider.Sut.InviteUsersAsync(organizationAbility.Id, invitor.UserId,
|
||||
new (OrganizationUserInvite, string)[] { (invite, null) }));
|
||||
|
||||
Assert.Contains("manager role is deprecated", exception.Message.ToLowerInvariant());
|
||||
}
|
||||
@ -1273,19 +1276,20 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task SaveUser_WithFCEnabled_WhenUpgradingToManager_Throws(
|
||||
Organization organization,
|
||||
OrganizationAbility organizationAbility,
|
||||
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData,
|
||||
[OrganizationUser(type: OrganizationUserType.Manager)] OrganizationUser newUserData,
|
||||
IEnumerable<CollectionAccessSelection> collections,
|
||||
IEnumerable<Guid> groups,
|
||||
SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
sutProvider.GetDependency<IFeatureService>()
|
||||
.IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>())
|
||||
.Returns(true);
|
||||
organizationAbility.FlexibleCollections = true;
|
||||
sutProvider.GetDependency<IApplicationCacheService>()
|
||||
.GetOrganizationAbilityAsync(organizationAbility.Id)
|
||||
.Returns(organizationAbility);
|
||||
|
||||
sutProvider.GetDependency<ICurrentContext>()
|
||||
.ManageUsers(organization.Id)
|
||||
.ManageUsers(organizationAbility.Id)
|
||||
.Returns(true);
|
||||
|
||||
sutProvider.GetDependency<IOrganizationUserRepository>()
|
||||
@ -1294,7 +1298,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
|
||||
|
||||
newUserData.Id = oldUserData.Id;
|
||||
newUserData.UserId = oldUserData.UserId;
|
||||
newUserData.OrganizationId = oldUserData.OrganizationId = organization.Id;
|
||||
newUserData.OrganizationId = oldUserData.OrganizationId = organizationAbility.Id;
|
||||
newUserData.Permissions = CoreHelpers.ClassToJsonData(new Permissions());
|
||||
|
||||
var exception = await Assert.ThrowsAsync<BadRequestException>(
|
||||
|
||||
@ -1,75 +0,0 @@
|
||||
using System.Reflection;
|
||||
using AutoFixture;
|
||||
using AutoFixture.Kernel;
|
||||
using Bit.Core.Context;
|
||||
using Bit.Core.Services;
|
||||
using Bit.Test.Common.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using NSubstitute;
|
||||
|
||||
namespace Bit.Core.Test.AutoFixture;
|
||||
|
||||
internal class FeatureServiceBuilder : ISpecimenBuilder
|
||||
{
|
||||
private readonly string _enabledFeatureFlag;
|
||||
|
||||
public FeatureServiceBuilder(string enabledFeatureFlag)
|
||||
{
|
||||
_enabledFeatureFlag = enabledFeatureFlag;
|
||||
}
|
||||
|
||||
public object Create(object request, ISpecimenContext context)
|
||||
{
|
||||
if (context == null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(context));
|
||||
}
|
||||
|
||||
if (request is not ParameterInfo pi)
|
||||
{
|
||||
return new NoSpecimen();
|
||||
}
|
||||
|
||||
if (pi.ParameterType == typeof(IFeatureService))
|
||||
{
|
||||
var fixture = new Fixture();
|
||||
var featureService = fixture.WithAutoNSubstitutions().Create<IFeatureService>();
|
||||
featureService
|
||||
.IsEnabled(_enabledFeatureFlag, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
|
||||
.Returns(true);
|
||||
return featureService;
|
||||
}
|
||||
|
||||
return new NoSpecimen();
|
||||
}
|
||||
}
|
||||
|
||||
internal class FeatureServiceCustomization : ICustomization
|
||||
{
|
||||
private readonly string _enabledFeatureFlag;
|
||||
|
||||
public FeatureServiceCustomization(string enabledFeatureFlag)
|
||||
{
|
||||
_enabledFeatureFlag = enabledFeatureFlag;
|
||||
}
|
||||
|
||||
public void Customize(IFixture fixture)
|
||||
{
|
||||
fixture.Customizations.Add(new FeatureServiceBuilder(_enabledFeatureFlag));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Arranges the IFeatureService mock to enable the specified feature flag
|
||||
/// </summary>
|
||||
public class FeatureServiceCustomizeAttribute : BitCustomizeAttribute
|
||||
{
|
||||
private readonly string _enabledFeatureFlag;
|
||||
|
||||
public FeatureServiceCustomizeAttribute(string enabledFeatureFlag)
|
||||
{
|
||||
_enabledFeatureFlag = enabledFeatureFlag;
|
||||
}
|
||||
|
||||
public override ICustomization GetCustomization() => new FeatureServiceCustomization(_enabledFeatureFlag);
|
||||
}
|
||||
@ -0,0 +1,100 @@
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Models.Business;
|
||||
using Bit.Core.Models.Data.Organizations;
|
||||
using Bit.Core.OrganizationFeatures.OrganizationLicenses;
|
||||
using Bit.Core.Services;
|
||||
using Bit.Core.Settings;
|
||||
using Bit.Test.Common.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using Bit.Test.Common.Helpers;
|
||||
using NSubstitute;
|
||||
using Xunit;
|
||||
using JsonSerializer = System.Text.Json.JsonSerializer;
|
||||
|
||||
namespace Bit.Core.Test.OrganizationFeatures.OrganizationLicenses;
|
||||
|
||||
[SutProviderCustomize]
|
||||
public class UpdateOrganizationLicenseCommandTests
|
||||
{
|
||||
private static string LicenseDirectory => Path.GetDirectoryName(OrganizationLicenseDirectory.Value);
|
||||
private static Lazy<string> OrganizationLicenseDirectory => new(() =>
|
||||
{
|
||||
// Create a temporary directory to write the license file to
|
||||
var directory = Path.Combine(Path.GetTempPath(), "bitwarden/");
|
||||
if (!Directory.Exists(directory))
|
||||
{
|
||||
Directory.CreateDirectory(directory);
|
||||
}
|
||||
return directory;
|
||||
});
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task UpdateLicenseAsync_UpdatesLicenseFileAndOrganization(
|
||||
SelfHostedOrganizationDetails selfHostedOrg,
|
||||
OrganizationLicense license,
|
||||
SutProvider<UpdateOrganizationLicenseCommand> sutProvider)
|
||||
{
|
||||
var globalSettings = sutProvider.GetDependency<IGlobalSettings>();
|
||||
globalSettings.LicenseDirectory = LicenseDirectory;
|
||||
globalSettings.SelfHosted = true;
|
||||
|
||||
// Passing values for OrganizationLicense.CanUse
|
||||
// NSubstitute cannot override non-virtual members so we have to ensure the real method passes
|
||||
license.Enabled = true;
|
||||
license.Issued = DateTime.Now.AddDays(-1);
|
||||
license.Expires = DateTime.Now.AddDays(1);
|
||||
license.Version = OrganizationLicense.CurrentLicenseFileVersion;
|
||||
license.InstallationId = globalSettings.Installation.Id;
|
||||
license.LicenseType = LicenseType.Organization;
|
||||
sutProvider.GetDependency<ILicensingService>().VerifyLicense(license).Returns(true);
|
||||
|
||||
// Passing values for SelfHostedOrganizationDetails.CanUseLicense
|
||||
// NSubstitute cannot override non-virtual members so we have to ensure the real method passes
|
||||
license.Seats = null;
|
||||
license.MaxCollections = null;
|
||||
license.UseGroups = true;
|
||||
license.UsePolicies = true;
|
||||
license.UseSso = true;
|
||||
license.UseKeyConnector = true;
|
||||
license.UseScim = true;
|
||||
license.UseCustomPermissions = true;
|
||||
license.UseResetPassword = true;
|
||||
|
||||
try
|
||||
{
|
||||
await sutProvider.Sut.UpdateLicenseAsync(selfHostedOrg, license, null);
|
||||
|
||||
// Assertion: should have saved the license file to disk
|
||||
var filePath = Path.Combine(LicenseDirectory, "organization", $"{selfHostedOrg.Id}.json");
|
||||
await using var fs = File.OpenRead(filePath);
|
||||
var licenseFromFile = await JsonSerializer.DeserializeAsync<OrganizationLicense>(fs);
|
||||
|
||||
AssertHelper.AssertPropertyEqual(license, licenseFromFile, "SignatureBytes");
|
||||
|
||||
// Assertion: should have updated and saved the organization
|
||||
// Properties excluded from the comparison below are exceptions to the rule that the Organization mirrors
|
||||
// the OrganizationLicense
|
||||
await sutProvider.GetDependency<IOrganizationService>()
|
||||
.Received(1)
|
||||
.ReplaceAndUpdateCacheAsync(Arg.Is<Organization>(
|
||||
org => AssertPropertyEqual(license, org,
|
||||
"Id", "MaxStorageGb", "Issued", "Refresh", "Version", "Trial", "LicenseType",
|
||||
"Hash", "Signature", "SignatureBytes", "InstallationId", "Expires", "ExpirationWithoutGracePeriod") &&
|
||||
// Same property but different name, use explicit mapping
|
||||
org.ExpirationDate == license.Expires));
|
||||
}
|
||||
finally
|
||||
{
|
||||
// Clean up temporary directory
|
||||
Directory.Delete(OrganizationLicenseDirectory.Value, true);
|
||||
}
|
||||
}
|
||||
|
||||
// Wrapper to compare 2 objects that are different types
|
||||
private bool AssertPropertyEqual(OrganizationLicense expected, Organization actual, params string[] excludedPropertyStrings)
|
||||
{
|
||||
AssertHelper.AssertPropertyEqual(expected, actual, excludedPropertyStrings);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@ -1,5 +1,4 @@
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.Context;
|
||||
using Bit.Core.Entities;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Exceptions;
|
||||
@ -36,6 +35,7 @@ public class CipherServiceTests
|
||||
SutProvider<CipherService> sutProvider)
|
||||
{
|
||||
organization.MaxCollections = null;
|
||||
organization.FlexibleCollections = false;
|
||||
importingOrganizationUser.OrganizationId = organization.Id;
|
||||
|
||||
foreach (var collection in collections)
|
||||
@ -62,10 +62,6 @@ public class CipherServiceTests
|
||||
.GetByOrganizationAsync(organization.Id, importingUserId)
|
||||
.Returns(importingOrganizationUser);
|
||||
|
||||
sutProvider.GetDependency<IFeatureService>()
|
||||
.IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
|
||||
.Returns(false);
|
||||
|
||||
// Set up a collection that already exists in the organization
|
||||
sutProvider.GetDependency<ICollectionRepository>()
|
||||
.GetManyByOrganizationIdAsync(organization.Id)
|
||||
@ -95,6 +91,7 @@ public class CipherServiceTests
|
||||
SutProvider<CipherService> sutProvider)
|
||||
{
|
||||
organization.MaxCollections = null;
|
||||
organization.FlexibleCollections = true;
|
||||
importingOrganizationUser.OrganizationId = organization.Id;
|
||||
|
||||
foreach (var collection in collections)
|
||||
@ -121,10 +118,6 @@ public class CipherServiceTests
|
||||
.GetByOrganizationAsync(organization.Id, importingUserId)
|
||||
.Returns(importingOrganizationUser);
|
||||
|
||||
sutProvider.GetDependency<IFeatureService>()
|
||||
.IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
|
||||
.Returns(true);
|
||||
|
||||
// Set up a collection that already exists in the organization
|
||||
sutProvider.GetDependency<ICollectionRepository>()
|
||||
.GetManyByOrganizationIdAsync(organization.Id)
|
||||
|
||||
Reference in New Issue
Block a user