[PM-14245] Remove policy definitions feature flag (#5095)

* Remove PolicyService.SaveAsync and use command instead * Delete feature flag definition * Add public api integration tests
bitwarden · Dec 4, 2024 · 44b6879 · 44b6879
1 parent c9aa61b
commit 44b6879
Show file tree

Hide file tree

Showing 17 changed files with 292 additions and 1,128 deletions.
diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs
@@ -6,8 +6,8 @@
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
 using Bit.Core.AdminConsole.Repositories;
-using Bit.Core.AdminConsole.Services;
 using Bit.Core.Auth.Models.Business.Tokenables;
 using Bit.Core.Context;
 using Bit.Core.Enums;
@@ -28,7 +28,6 @@ namespace Bit.Api.AdminConsole.Controllers;
 public class PoliciesController : Controller
 {
     private readonly IPolicyRepository _policyRepository;
-    private readonly IPolicyService _policyService;
     private readonly IOrganizationUserRepository _organizationUserRepository;
     private readonly IUserService _userService;
     private readonly ICurrentContext _currentContext;
@@ -37,21 +36,21 @@ public class PoliciesController : Controller
     private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory;
     private readonly IFeatureService _featureService;
     private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery;
+    private readonly ISavePolicyCommand _savePolicyCommand;
 
     public PoliciesController(
         IPolicyRepository policyRepository,
-        IPolicyService policyService,
         IOrganizationUserRepository organizationUserRepository,
         IUserService userService,
         ICurrentContext currentContext,
         GlobalSettings globalSettings,
         IDataProtectionProvider dataProtectionProvider,
         IDataProtectorTokenFactory<OrgUserInviteTokenable> orgUserInviteTokenDataFactory,
         IFeatureService featureService,
-        IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery)
+        IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery,
+        ISavePolicyCommand savePolicyCommand)
     {
         _policyRepository = policyRepository;
-        _policyService = policyService;
         _organizationUserRepository = organizationUserRepository;
         _userService = userService;
         _currentContext = currentContext;
@@ -62,6 +61,7 @@ public PoliciesController(
         _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory;
         _featureService = featureService;
         _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery;
+        _savePolicyCommand = savePolicyCommand;
     }
 
     [HttpGet("{type}")]
@@ -178,25 +178,20 @@ public async Task<PolicyResponseModel> GetMasterPasswordPolicy(Guid orgId)
     }
 
     [HttpPut("{type}")]
-    public async Task<PolicyResponseModel> Put(string orgId, int type, [FromBody] PolicyRequestModel model)
+    public async Task<PolicyResponseModel> Put(Guid orgId, PolicyType type, [FromBody] PolicyRequestModel model)
     {
-        var orgIdGuid = new Guid(orgId);
-        if (!await _currentContext.ManagePolicies(orgIdGuid))
+        if (!await _currentContext.ManagePolicies(orgId))
         {
             throw new NotFoundException();
         }
-        var policy = await _policyRepository.GetByOrganizationIdTypeAsync(new Guid(orgId), (PolicyType)type);
-        if (policy == null)
-        {
-            policy = model.ToPolicy(orgIdGuid);
-        }
-        else
+
+        if (type != model.Type)
         {
-            policy = model.ToPolicy(policy);
+            throw new BadRequestException("Mismatched policy type");
         }
 
-        var userId = _userService.GetProperUserId(User);
-        await _policyService.SaveAsync(policy, userId);
+        var policyUpdate = await model.ToPolicyUpdateAsync(orgId, _currentContext);
+        var policy = await _savePolicyCommand.SaveAsync(policyUpdate);
         return new PolicyResponseModel(policy);
     }
 }
diff --git a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs
@@ -1,7 +1,9 @@
 using System.ComponentModel.DataAnnotations;
 using System.Text.Json;
-using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Models.Data;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.Context;
 
 namespace Bit.Api.AdminConsole.Models.Request;
 
@@ -13,19 +15,12 @@ public class PolicyRequestModel
     public bool? Enabled { get; set; }
     public Dictionary<string, object> Data { get; set; }
 
-    public Policy ToPolicy(Guid orgId)
+    public async Task<PolicyUpdate> ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext) => new()
     {
-        return ToPolicy(new Policy
-        {
-            Type = Type.Value,
-            OrganizationId = orgId
-        });
-    }
-
-    public Policy ToPolicy(Policy existingPolicy)
-    {
-        existingPolicy.Enabled = Enabled.GetValueOrDefault();
-        existingPolicy.Data = Data != null ? JsonSerializer.Serialize(Data) : null;
-        return existingPolicy;
-    }
+        Type = Type!.Value,
+        OrganizationId = organizationId,
+        Data = Data != null ? JsonSerializer.Serialize(Data) : null,
+        Enabled = Enabled.GetValueOrDefault(),
+        PerformedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId))
+    };
 }
diff --git a/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs
@@ -3,6 +3,7 @@
 using Bit.Api.AdminConsole.Public.Models.Response;
 using Bit.Api.Models.Public.Response;
 using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.AdminConsole.Services;
 using Bit.Core.Context;
@@ -18,15 +19,18 @@ public class PoliciesController : Controller
     private readonly IPolicyRepository _policyRepository;
     private readonly IPolicyService _policyService;
     private readonly ICurrentContext _currentContext;
+    private readonly ISavePolicyCommand _savePolicyCommand;
 
     public PoliciesController(
         IPolicyRepository policyRepository,
         IPolicyService policyService,
-        ICurrentContext currentContext)
+        ICurrentContext currentContext,
+        ISavePolicyCommand savePolicyCommand)
     {
         _policyRepository = policyRepository;
         _policyService = policyService;
         _currentContext = currentContext;
+        _savePolicyCommand = savePolicyCommand;
     }
 
     /// <summary>
@@ -80,17 +84,9 @@ public async Task<IActionResult> List()
     [ProducesResponseType((int)HttpStatusCode.NotFound)]
     public async Task<IActionResult> Put(PolicyType type, [FromBody] PolicyUpdateRequestModel model)
     {
-        var policy = await _policyRepository.GetByOrganizationIdTypeAsync(
-            _currentContext.OrganizationId.Value, type);
-        if (policy == null)
-        {
-            policy = model.ToPolicy(_currentContext.OrganizationId.Value, type);
-        }
-        else
-        {
-            policy = model.ToPolicy(policy);
-        }
-        await _policyService.SaveAsync(policy, null);
+        var policyUpdate = model.ToPolicyUpdate(_currentContext.OrganizationId!.Value, type);
+        var policy = await _savePolicyCommand.SaveAsync(policyUpdate);
+
         var response = new PolicyResponseModel(policy);
         return new JsonResult(response);
     }

diff --git a/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs b/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs
@@ -1,26 +1,19 @@
 using System.Text.Json;
-using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Models.Data;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.Enums;
 
 namespace Bit.Api.AdminConsole.Public.Models.Request;
 
 public class PolicyUpdateRequestModel : PolicyBaseModel
 {
-    public Policy ToPolicy(Guid orgId, PolicyType type)
+    public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type) => new()
     {
-        return ToPolicy(new Policy
-        {
-            OrganizationId = orgId,
-            Enabled = Enabled.GetValueOrDefault(),
-            Data = Data != null ? JsonSerializer.Serialize(Data) : null,
-            Type = type
-        });
-    }
-
-    public virtual Policy ToPolicy(Policy existingPolicy)
-    {
-        existingPolicy.Enabled = Enabled.GetValueOrDefault();
-        existingPolicy.Data = Data != null ? JsonSerializer.Serialize(Data) : null;
-        return existingPolicy;
-    }
+        Type = type,
+        OrganizationId = organizationId,
+        Data = Data != null ? JsonSerializer.Serialize(Data) : null,
+        Enabled = Enabled.GetValueOrDefault(),
+        PerformedBy = new SystemUser(EventSystemUser.PublicApi)
+    };
 }
diff --git a/src/Api/AdminConsole/Public/Models/Response/PolicyResponseModel.cs b/src/Api/AdminConsole/Public/Models/Response/PolicyResponseModel.cs
@@ -1,8 +1,9 @@
 using System.ComponentModel.DataAnnotations;
-using System.Text.Json;
 using Bit.Api.Models.Public.Response;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
+using Newtonsoft.Json;
+using JsonSerializer = System.Text.Json.JsonSerializer;
 
 namespace Bit.Api.AdminConsole.Public.Models.Response;
 
@@ -11,6 +12,9 @@ namespace Bit.Api.AdminConsole.Public.Models.Response;
 /// </summary>
 public class PolicyResponseModel : PolicyBaseModel, IResponseModel
 {
+    [JsonConstructor]
+    public PolicyResponseModel() { }
+
     public PolicyResponseModel(Policy policy)
     {
         if (policy == null)

diff --git a/.../AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs b/.../AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs
@@ -1,8 +1,8 @@
-using Bit.Core.AdminConsole.Entities;
-using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Data;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
-using Bit.Core.AdminConsole.Services;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
 using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
@@ -19,9 +19,9 @@ public class VerifyOrganizationDomainCommand(
     IDnsResolverService dnsResolverService,
     IEventService eventService,
     IGlobalSettings globalSettings,
-    IPolicyService policyService,
     IFeatureService featureService,
     ICurrentContext currentContext,
+    ISavePolicyCommand savePolicyCommand,
     ILogger<VerifyOrganizationDomainCommand> logger)
     : IVerifyOrganizationDomainCommand
 {
@@ -125,10 +125,15 @@ private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId, IAct
     {
         if (featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
         {
-            await policyService.SaveAsync(
-                new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true },
-                savingUserId: actingUser is StandardUser standardUser ? standardUser.UserId : null,
-                eventSystemUser: actingUser is SystemUser systemUser ? systemUser.SystemUserType : null);
+            var policyUpdate = new PolicyUpdate
+            {
+                OrganizationId = organizationId,
+                Type = PolicyType.SingleOrg,
+                Enabled = true,
+                PerformedBy = actingUser
+            };
+
+            await savePolicyCommand.SaveAsync(policyUpdate);
         }
     }
 }
diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs
@@ -1,8 +1,9 @@
-using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.AdminConsole.Entities;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
 
 namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies;
 
 public interface ISavePolicyCommand
 {
-    Task SaveAsync(PolicyUpdate policy);
+    Task<Policy> SaveAsync(PolicyUpdate policy);
 }
diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs
@@ -42,7 +42,7 @@ public SavePolicyCommand(
         _policyValidators = policyValidatorsDict;
     }
 
-    public async Task SaveAsync(PolicyUpdate policyUpdate)
+    public async Task<Policy> SaveAsync(PolicyUpdate policyUpdate)
     {
         var org = await _applicationCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId);
         if (org == null)
@@ -74,6 +74,8 @@ public async Task SaveAsync(PolicyUpdate policyUpdate)
 
         await _policyRepository.UpsertAsync(policy);
         await _eventService.LogPolicyEventAsync(policy, EventType.Policy_Updated);
+
+        return policy;
     }
 
     private async Task RunValidatorAsync(IPolicyValidator validator, PolicyUpdate policyUpdate)

diff --git a/src/Core/AdminConsole/Services/IPolicyService.cs b/src/Core/AdminConsole/Services/IPolicyService.cs
@@ -1,5 +1,4 @@
-using Bit.Core.AdminConsole.Entities;
-using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
@@ -9,8 +8,6 @@ namespace Bit.Core.AdminConsole.Services;
 
 public interface IPolicyService
 {
-    Task SaveAsync(Policy policy, Guid? savingUserId, EventSystemUser? eventSystemUser = null);
-
     /// <summary>
     /// Get the combined master password policy options for the specified user.
     /// </summary>