[PM-26429] Add validation to policy data and metadata (#6460)

* Enhance PolicyRequestModel and SavePolicyRequest with validation for policy data and metadata.

* Add integration tests for policy updates to validate handling of invalid data types in PolicyRequestModel and SavePolicyRequest.

* Add missing using

* Update PolicyRequestModel for null safety by making Data and ValidateAndSerializePolicyData nullable

* Add integration tests for public PoliciesController to validate handling of invalid data types in policy updates.

* Add PolicyDataValidator class for validating and serializing policy data and metadata based on policy type.

* Refactor PolicyRequestModel, SavePolicyRequest, and PolicyUpdateRequestModel to utilize PolicyDataValidator for data validation and serialization, removing redundant methods and improving code clarity.

* Update PolicyRequestModel and SavePolicyRequest to initialize Data and Metadata properties with empty dictionaries.

* Refactor PolicyDataValidator to remove null checks for input data in validation methods

* Rename test methods in SavePolicyRequestTests to reflect handling of empty data and metadata, and remove null assignments in test cases for improved clarity.

* Enhance error handling in PolicyDataValidator to include field-specific details in BadRequestException messages.

* Enhance PoliciesControllerTests to verify error messages for BadRequest responses by checking for specific field names in the response content.

* refactor: Update PolicyRequestModel and SavePolicyRequest to use nullable dictionaries for Data and Metadata properties; enhance validation methods in PolicyDataValidator to handle null cases.

* test: Add integration tests for handling policies with null data in PoliciesController

* fix: Catch specific JsonException in PolicyDataValidator to improve error handling

* test: Add unit tests for PolicyDataValidator to validate and serialize policy data and metadata

* test: Update PolicyDataValidatorTests to validate organization data ownership metadata

Author: r-tome

src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs

@@ -1,11 +1,8 @@
-// FIXME: Update this file to be null safe and then delete the line below
-#nullable disable
-
-using System.ComponentModel.DataAnnotations;
-using System.Text.Json;
+using System.ComponentModel.DataAnnotations;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Data;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.AdminConsole.Utilities;
 using Bit.Core.Context;
 
 namespace Bit.Api.AdminConsole.Models.Request;
@@ -16,14 +13,20 @@ public class PolicyRequestModel
     public PolicyType? Type { get; set; }
     [Required]
     public bool? Enabled { get; set; }
-    public Dictionary<string, object> Data { get; set; }
+    public Dictionary<string, object>? Data { get; set; }
 
-    public async Task<PolicyUpdate> ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext) => new()
+    public async Task<PolicyUpdate> ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext)
     {
-        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))
-    };
+        var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, Type!.Value);
+        var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId));
+
+        return new()
+        {
+            Type = Type!.Value,
+            OrganizationId = organizationId,
+            Data = serializedData,
+            Enabled = Enabled.GetValueOrDefault(),
+            PerformedBy = performedBy
+        };
+    }
 }

src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs

@@ -1,10 +1,8 @@
 using System.ComponentModel.DataAnnotations;
-using System.Text.Json;
-using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Data;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.AdminConsole.Utilities;
 using Bit.Core.Context;
-using Bit.Core.Utilities;
 
 namespace Bit.Api.AdminConsole.Models.Request;
 
@@ -17,45 +15,10 @@ public class SavePolicyRequest
 
     public async Task<SavePolicyModel> ToSavePolicyModelAsync(Guid organizationId, ICurrentContext currentContext)
     {
+        var policyUpdate = await Policy.ToPolicyUpdateAsync(organizationId, currentContext);
+        var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, Policy.Type!.Value);
         var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId));
 
-        var updatedPolicy = new PolicyUpdate()
-        {
-            Type = Policy.Type!.Value,
-            OrganizationId = organizationId,
-            Data = Policy.Data != null ? JsonSerializer.Serialize(Policy.Data) : null,
-            Enabled = Policy.Enabled.GetValueOrDefault(),
-        };
-
-        var metadata = MapToPolicyMetadata();
-
-        return new SavePolicyModel(updatedPolicy, performedBy, metadata);
-    }
-
-    private IPolicyMetadataModel MapToPolicyMetadata()
-    {
-        if (Metadata == null)
-        {
-            return new EmptyMetadataModel();
-        }
-
-        return Policy?.Type switch
-        {
-            PolicyType.OrganizationDataOwnership => MapToPolicyMetadata<OrganizationModelOwnershipPolicyModel>(),
-            _ => new EmptyMetadataModel()
-        };
-    }
-
-    private IPolicyMetadataModel MapToPolicyMetadata<T>() where T : IPolicyMetadataModel, new()
-    {
-        try
-        {
-            var json = JsonSerializer.Serialize(Metadata);
-            return CoreHelpers.LoadClassFromJsonData<T>(json);
-        }
-        catch
-        {
-            return new EmptyMetadataModel();
-        }
+        return new SavePolicyModel(policyUpdate, performedBy, metadata);
     }
 }

src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs

@@ -1,19 +1,24 @@
-using System.Text.Json;
-using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Data;
 using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.AdminConsole.Utilities;
 using Bit.Core.Enums;
 
 namespace Bit.Api.AdminConsole.Public.Models.Request;
 
 public class PolicyUpdateRequestModel : PolicyBaseModel
 {
-    public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type) => new()
+    public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type)
     {
-        Type = type,
-        OrganizationId = organizationId,
-        Data = Data != null ? JsonSerializer.Serialize(Data) : null,
-        Enabled = Enabled.GetValueOrDefault(),
-        PerformedBy = new SystemUser(EventSystemUser.PublicApi)
-    };
+        var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, type);
+
+        return new()
+        {
+            Type = type,
+            OrganizationId = organizationId,
+            Data = serializedData,
+            Enabled = Enabled.GetValueOrDefault(),
+            PerformedBy = new SystemUser(EventSystemUser.PublicApi)
+        };
+    }
 }

src/Core/AdminConsole/Utilities/PolicyDataValidator.cs

@@ -0,0 +1,81 @@
+using System.Text.Json;
+using Bit.Core.AdminConsole.Enums;
+using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
+using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
+using Bit.Core.Exceptions;
+using Bit.Core.Utilities;
+
+namespace Bit.Core.AdminConsole.Utilities;
+
+public static class PolicyDataValidator
+{
+    /// <summary>
+    /// Validates and serializes policy data based on the policy type.
+    /// </summary>
+    /// <param name="data">The policy data to validate</param>
+    /// <param name="policyType">The type of policy</param>
+    /// <returns>Serialized JSON string if data is valid, null if data is null or empty</returns>
+    /// <exception cref="BadRequestException">Thrown when data validation fails</exception>
+    public static string? ValidateAndSerialize(Dictionary<string, object>? data, PolicyType policyType)
+    {
+        if (data == null || data.Count == 0)
+        {
+            return null;
+        }
+
+        try
+        {
+            var json = JsonSerializer.Serialize(data);
+
+            switch (policyType)
+            {
+                case PolicyType.MasterPassword:
+                    CoreHelpers.LoadClassFromJsonData<MasterPasswordPolicyData>(json);
+                    break;
+                case PolicyType.SendOptions:
+                    CoreHelpers.LoadClassFromJsonData<SendOptionsPolicyData>(json);
+                    break;
+                case PolicyType.ResetPassword:
+                    CoreHelpers.LoadClassFromJsonData<ResetPasswordDataModel>(json);
+                    break;
+            }
+
+            return json;
+        }
+        catch (JsonException ex)
+        {
+            var fieldInfo = !string.IsNullOrEmpty(ex.Path) ? $": field '{ex.Path}' has invalid type" : "";
+            throw new BadRequestException($"Invalid data for {policyType} policy{fieldInfo}.");
+        }
+    }
+
+    /// <summary>
+    /// Validates and deserializes policy metadata based on the policy type.
+    /// </summary>
+    /// <param name="metadata">The policy metadata to validate</param>
+    /// <param name="policyType">The type of policy</param>
+    /// <returns>Deserialized metadata model, or EmptyMetadataModel if metadata is null, empty, or validation fails</returns>
+    public static IPolicyMetadataModel ValidateAndDeserializeMetadata(Dictionary<string, object>? metadata, PolicyType policyType)
+    {
+        if (metadata == null || metadata.Count == 0)
+        {
+            return new EmptyMetadataModel();
+        }
+
+        try
+        {
+            var json = JsonSerializer.Serialize(metadata);
+
+            return policyType switch
+            {
+                PolicyType.OrganizationDataOwnership =>
+                    CoreHelpers.LoadClassFromJsonData<OrganizationModelOwnershipPolicyModel>(json),
+                _ => new EmptyMetadataModel()
+            };
+        }
+        catch (JsonException)
+        {
+            return new EmptyMetadataModel();
+        }
+    }
+}