Skip to content

[PM-26690] Wire VNextSavePolicyCommand behind PolicyValidatorsRefactor feature flag #6483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: ac/pm-26429/add-validation-to-policy-data-and-metadata
Choose a base branch
from
Open

[PM-26690] Wire VNextSavePolicyCommand behind PolicyValidatorsRefactor feature flag #6483

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ac52048
Add PolicyValidatorsRefactor constant to FeatureFlagKeys in Constants.cs
r-tome Oct 22, 2025
2c5490b
Add Metadata property and ToSavePolicyModel method to PolicyUpdateReqโ€ฆ
r-tome Oct 22, 2025
dd28442
Refactor PoliciesController to utilize IVNextSavePolicyCommand based โ€ฆ
r-tome Oct 22, 2025
172ca7e
Update public PoliciesController to to utilize IVNextSavePolicyCommanโ€ฆ
r-tome Oct 22, 2025
d10e525
Refactor VerifyOrganizationDomainCommand to utilize IVNextSavePolicyCโ€ฆ
r-tome Oct 22, 2025
6c161e9
Enhance SsoConfigService to utilize IVNextSavePolicyCommand based on โ€ฆ
r-tome Oct 22, 2025
f6d582f
Merge branch 'ac/pm-26429/add-validation-to-policy-data-and-metadata'โ€ฆ
r-tome Oct 24, 2025
cf6d0d1
Refactor SavePolicyModel to simplify constructor usage by removing Emโ€ฆ
r-tome Oct 27, 2025
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
14 changes: 11 additions & 3 deletions src/Api/AdminConsole/Controllers/PoliciesController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Context;
Expand Down Expand Up @@ -41,8 +42,9 @@ public class PoliciesController : Controller
private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory;
private readonly IPolicyRepository _policyRepository;
private readonly IUserService _userService;

private readonly IFeatureService _featureService;
private readonly ISavePolicyCommand _savePolicyCommand;
private readonly IVNextSavePolicyCommand _vNextSavePolicyCommand;

public PoliciesController(IPolicyRepository policyRepository,
IOrganizationUserRepository organizationUserRepository,
Expand All @@ -53,7 +55,9 @@ public PoliciesController(IPolicyRepository policyRepository,
IDataProtectorTokenFactory<OrgUserInviteTokenable> orgUserInviteTokenDataFactory,
IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery,
IOrganizationRepository organizationRepository,
ISavePolicyCommand savePolicyCommand)
IFeatureService featureService,
ISavePolicyCommand savePolicyCommand,
IVNextSavePolicyCommand vNextSavePolicyCommand)
{
_policyRepository = policyRepository;
_organizationUserRepository = organizationUserRepository;
Expand All @@ -65,7 +69,9 @@ public PoliciesController(IPolicyRepository policyRepository,
_organizationRepository = organizationRepository;
_orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory;
_organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery;
_featureService = featureService;
_savePolicyCommand = savePolicyCommand;
_vNextSavePolicyCommand = vNextSavePolicyCommand;
}

[HttpGet("{type}")]
Expand Down Expand Up @@ -221,7 +227,9 @@ public async Task<PolicyResponseModel> PutVNext(Guid orgId, [FromBody] SavePolic
{
var savePolicyRequest = await model.ToSavePolicyModelAsync(orgId, _currentContext);

var policy = await _savePolicyCommand.VNextSaveAsync(savePolicyRequest);
var policy = _featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor) ?
await _vNextSavePolicyCommand.SaveAsync(savePolicyRequest) :
await _savePolicyCommand.VNextSaveAsync(savePolicyRequest);

return new PolicyResponseModel(policy);
}
Expand Down
25 changes: 22 additions & 3 deletions src/Api/AdminConsole/Public/Controllers/PoliciesController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
using Bit.Api.AdminConsole.Public.Models.Request;
using Bit.Api.AdminConsole.Public.Models.Response;
using Bit.Api.Models.Public.Response;
using Bit.Core;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Context;
using Bit.Core.Services;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;

Expand All @@ -22,18 +26,24 @@ public class PoliciesController : Controller
private readonly IPolicyRepository _policyRepository;
private readonly IPolicyService _policyService;
private readonly ICurrentContext _currentContext;
private readonly IFeatureService _featureService;
private readonly ISavePolicyCommand _savePolicyCommand;
private readonly IVNextSavePolicyCommand _vNextSavePolicyCommand;

public PoliciesController(
IPolicyRepository policyRepository,
IPolicyService policyService,
ICurrentContext currentContext,
ISavePolicyCommand savePolicyCommand)
IFeatureService featureService,
ISavePolicyCommand savePolicyCommand,
IVNextSavePolicyCommand vNextSavePolicyCommand)
{
_policyRepository = policyRepository;
_policyService = policyService;
_currentContext = currentContext;
_featureService = featureService;
_savePolicyCommand = savePolicyCommand;
_vNextSavePolicyCommand = vNextSavePolicyCommand;
}

/// <summary>
Expand Down Expand Up @@ -87,8 +97,17 @@ public async Task<IActionResult> List()
[ProducesResponseType((int)HttpStatusCode.NotFound)]
public async Task<IActionResult> Put(PolicyType type, [FromBody] PolicyUpdateRequestModel model)
{
var policyUpdate = model.ToPolicyUpdate(_currentContext.OrganizationId!.Value, type);
var policy = await _savePolicyCommand.SaveAsync(policyUpdate);
Policy policy;
if (_featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor))
{
var savePolicyModel = model.ToSavePolicyModel(_currentContext.OrganizationId!.Value, type);
policy = await _vNextSavePolicyCommand.SaveAsync(savePolicyModel);
}
else
{
var policyUpdate = model.ToPolicyUpdate(_currentContext.OrganizationId!.Value, type);
policy = await _savePolicyCommand.SaveAsync(policyUpdate);
}

var response = new PolicyResponseModel(policy);
return new JsonResult(response);
Expand Down
20 changes: 20 additions & 0 deletions src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ namespace Bit.Api.AdminConsole.Public.Models.Request;

public class PolicyUpdateRequestModel : PolicyBaseModel
{
public Dictionary<string, object> Metadata { get; set; } = new();

public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type)
{
var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, type);
Expand All @@ -21,4 +23,22 @@ public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type)
PerformedBy = new SystemUser(EventSystemUser.PublicApi)
};
}

public SavePolicyModel ToSavePolicyModel(Guid organizationId, PolicyType type)
{
var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, type);

var policyUpdate = new PolicyUpdate
{
Type = type,
OrganizationId = organizationId,
Data = serializedData,
Enabled = Enabled.GetValueOrDefault()
};

var performedBy = new SystemUser(EventSystemUser.PublicApi);
var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, type);

return new SavePolicyModel(policyUpdate, performedBy, metadata);
}
}
32 changes: 23 additions & 9 deletions .../AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
Expand All @@ -24,7 +25,9 @@ public class VerifyOrganizationDomainCommand(
IEventService eventService,
IGlobalSettings globalSettings,
ICurrentContext currentContext,
IFeatureService featureService,
ISavePolicyCommand savePolicyCommand,
IVNextSavePolicyCommand vNextSavePolicyCommand,
IMailService mailService,
IOrganizationUserRepository organizationUserRepository,
IOrganizationRepository organizationRepository,
Expand Down Expand Up @@ -131,15 +134,26 @@ private async Task DomainVerificationSideEffectsAsync(OrganizationDomain domain,
await SendVerifiedDomainUserEmailAsync(domain);
}

private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId, IActingUser actingUser) =>
await savePolicyCommand.SaveAsync(
new PolicyUpdate
{
OrganizationId = organizationId,
Type = PolicyType.SingleOrg,
Enabled = true,
PerformedBy = actingUser
});
private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId, IActingUser actingUser)
{
var policyUpdate = new PolicyUpdate
{
OrganizationId = organizationId,
Type = PolicyType.SingleOrg,
Enabled = true,
PerformedBy = actingUser
};

if (featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor))
{
var savePolicyModel = new SavePolicyModel(policyUpdate, actingUser, new EmptyMetadataModel());
await vNextSavePolicyCommand.SaveAsync(savePolicyModel);
}
else
{
await savePolicyCommand.SaveAsync(policyUpdate);
}
}

private async Task SendVerifiedDomainUserEmailAsync(OrganizationDomain domain)
{
Expand Down
34 changes: 27 additions & 7 deletions src/Core/Auth/Services/Implementations/SsoConfigService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Enums;
Expand All @@ -24,22 +26,28 @@ public class SsoConfigService : ISsoConfigService
private readonly IOrganizationRepository _organizationRepository;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IEventService _eventService;
private readonly IFeatureService _featureService;
private readonly ISavePolicyCommand _savePolicyCommand;
private readonly IVNextSavePolicyCommand _vNextSavePolicyCommand;

public SsoConfigService(
ISsoConfigRepository ssoConfigRepository,
IPolicyRepository policyRepository,
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
IEventService eventService,
ISavePolicyCommand savePolicyCommand)
IFeatureService featureService,
ISavePolicyCommand savePolicyCommand,
IVNextSavePolicyCommand vNextSavePolicyCommand)
{
_ssoConfigRepository = ssoConfigRepository;
_policyRepository = policyRepository;
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
_eventService = eventService;
_featureService = featureService;
_savePolicyCommand = savePolicyCommand;
_vNextSavePolicyCommand = vNextSavePolicyCommand;
}

public async Task SaveAsync(SsoConfig config, Organization organization)
Expand Down Expand Up @@ -67,13 +75,12 @@ public async Task SaveAsync(SsoConfig config, Organization organization)
// Automatically enable account recovery, SSO required, and single org policies if trusted device encryption is selected
if (config.GetData().MemberDecryptionType == MemberDecryptionType.TrustedDeviceEncryption)
{

await _savePolicyCommand.SaveAsync(new()
var singleOrgPolicy = new PolicyUpdate
{
OrganizationId = config.OrganizationId,
Type = PolicyType.SingleOrg,
Enabled = true
});
};

var resetPasswordPolicy = new PolicyUpdate
{
Expand All @@ -82,14 +89,27 @@ await _savePolicyCommand.SaveAsync(new()
Enabled = true,
};
resetPasswordPolicy.SetDataModel(new ResetPasswordDataModel { AutoEnrollEnabled = true });
await _savePolicyCommand.SaveAsync(resetPasswordPolicy);

await _savePolicyCommand.SaveAsync(new()
var requireSsoPolicy = new PolicyUpdate
{
OrganizationId = config.OrganizationId,
Type = PolicyType.RequireSso,
Enabled = true
});
};

if (_featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor))
{
var performedBy = new SystemUser(EventSystemUser.Unknown);
await _vNextSavePolicyCommand.SaveAsync(new SavePolicyModel(singleOrgPolicy, performedBy, new EmptyMetadataModel()));
await _vNextSavePolicyCommand.SaveAsync(new SavePolicyModel(resetPasswordPolicy, performedBy, new EmptyMetadataModel()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a different constructor for SavePolicyModel, so the client can simply pass in what it has, and the SavePolicyModel object will fill in the default values.

Currently, the client needs to be aware that EmptyMetadataModel is the default value. We're also calling this logic in multiple places, so centralizing it would be nice, even if the value is low.

So something like this.

public record SavePolicyModel(PolicyUpdate PolicyUpdate, IActingUser? PerformedBy, IPolicyMetadataModel Metadata)
{
    public SavePolicyModel(PolicyUpdate PolicyUpdate)
        : this(PolicyUpdate, null, new EmptyMetadataModel())
    {
    }

    public SavePolicyModel(PolicyUpdate PolicyUpdate, IActingUser performedBy)
        : this(PolicyUpdate, performedBy, new EmptyMetadataModel())
    {
    }

    public SavePolicyModel(PolicyUpdate PolicyUpdate, EmptyMetadataModel metadata)
        : this(PolicyUpdate, null, metadata)
    {
    }
}

await _vNextSavePolicyCommand.SaveAsync(new SavePolicyModel(requireSsoPolicy, performedBy, new EmptyMetadataModel()));
}
else
{
await _savePolicyCommand.SaveAsync(singleOrgPolicy);
await _savePolicyCommand.SaveAsync(resetPasswordPolicy);
await _savePolicyCommand.SaveAsync(requireSsoPolicy);
}
}

await LogEventsAsync(config, oldConfig);
Expand Down
1 change: 1 addition & 0 deletions src/Core/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public static class FeatureFlagKeys
public const string CreateDefaultLocation = "pm-19467-create-default-location";
public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users";
public const string PM23845_VNextApplicationCache = "pm-24957-refactor-memory-application-cache";
public const string PolicyValidatorsRefactor = "pm-26423-refactor-policy-side-effects";

/* Auth Team */
public const string TwoFactorExtensionDataPersistence = "pm-9115-two-factor-extension-data-persistence";
Expand Down
87 changes: 87 additions & 0 deletions test/Api.Test/AdminConsole/Public/Controllers/PoliciesControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
๏ปฟusing Bit.Api.AdminConsole.Public.Controllers;
using Bit.Api.AdminConsole.Public.Models.Request;
using Bit.Core;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.Context;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
using Xunit;

namespace Bit.Api.Test.AdminConsole.Public.Controllers;

[ControllerCustomize(typeof(PoliciesController))]
[SutProviderCustomize]
public class PoliciesControllerTests
{
[Theory]
[BitAutoData]
public async Task Put_WhenPolicyValidatorsRefactorEnabled_UsesVNextSavePolicyCommand(
Guid organizationId,
PolicyType policyType,
PolicyUpdateRequestModel model,
Policy policy,
SutProvider<PoliciesController> sutProvider)
{
// Arrange
policy.Data = null;
sutProvider.GetDependency<ICurrentContext>()
.OrganizationId.Returns(organizationId);
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor)
.Returns(true);
sutProvider.GetDependency<IVNextSavePolicyCommand>()
.SaveAsync(Arg.Any<SavePolicyModel>())
.Returns(policy);

// Act
await sutProvider.Sut.Put(policyType, model);

// Assert
await sutProvider.GetDependency<IVNextSavePolicyCommand>()
.Received(1)
.SaveAsync(Arg.Is<SavePolicyModel>(m =>
m.PolicyUpdate.OrganizationId == organizationId &&
m.PolicyUpdate.Type == policyType &&
m.PolicyUpdate.Enabled == model.Enabled.GetValueOrDefault() &&
m.PerformedBy is SystemUser));
}

[Theory]
[BitAutoData]
public async Task Put_WhenPolicyValidatorsRefactorDisabled_UsesLegacySavePolicyCommand(
Guid organizationId,
PolicyType policyType,
PolicyUpdateRequestModel model,
Policy policy,
SutProvider<PoliciesController> sutProvider)
{
// Arrange
policy.Data = null;
sutProvider.GetDependency<ICurrentContext>()
.OrganizationId.Returns(organizationId);
sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor)
.Returns(false);
sutProvider.GetDependency<ISavePolicyCommand>()
.SaveAsync(Arg.Any<PolicyUpdate>())
.Returns(policy);

// Act
await sutProvider.Sut.Put(policyType, model);

// Assert
await sutProvider.GetDependency<ISavePolicyCommand>()
.Received(1)
.SaveAsync(Arg.Is<PolicyUpdate>(p =>
p.OrganizationId == organizationId &&
p.Type == policyType &&
p.Enabled == model.Enabled));
}
}
Loading
Loading