diff --git a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs index 0e31deacd1..f9b9c18993 100644 --- a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs +++ b/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 Data { get; set; } + public Dictionary? Data { get; set; } - public async Task ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext) => new() + public async Task 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 + }; + } } diff --git a/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs b/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs index fcdc49882b..5c1acc1c36 100644 --- a/src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs +++ b/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 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(), - _ => new EmptyMetadataModel() - }; - } - - private IPolicyMetadataModel MapToPolicyMetadata() where T : IPolicyMetadataModel, new() - { - try - { - var json = JsonSerializer.Serialize(Metadata); - return CoreHelpers.LoadClassFromJsonData(json); - } - catch - { - return new EmptyMetadataModel(); - } + return new SavePolicyModel(policyUpdate, performedBy, metadata); } } diff --git a/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs b/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs index eb56690462..34675a6046 100644 --- a/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs +++ b/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) + }; + } } diff --git a/src/Core/AdminConsole/Utilities/PolicyDataValidator.cs b/src/Core/AdminConsole/Utilities/PolicyDataValidator.cs new file mode 100644 index 0000000000..84e63f2a20 --- /dev/null +++ b/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 +{ + /// + /// Validates and serializes policy data based on the policy type. + /// + /// The policy data to validate + /// The type of policy + /// Serialized JSON string if data is valid, null if data is null or empty + /// Thrown when data validation fails + public static string? ValidateAndSerialize(Dictionary? data, PolicyType policyType) + { + if (data == null || data.Count == 0) + { + return null; + } + + try + { + var json = JsonSerializer.Serialize(data); + + switch (policyType) + { + case PolicyType.MasterPassword: + CoreHelpers.LoadClassFromJsonData(json); + break; + case PolicyType.SendOptions: + CoreHelpers.LoadClassFromJsonData(json); + break; + case PolicyType.ResetPassword: + CoreHelpers.LoadClassFromJsonData(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}."); + } + } + + /// + /// Validates and deserializes policy metadata based on the policy type. + /// + /// The policy metadata to validate + /// The type of policy + /// Deserialized metadata model, or EmptyMetadataModel if metadata is null, empty, or validation fails + public static IPolicyMetadataModel ValidateAndDeserializeMetadata(Dictionary? 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(json), + _ => new EmptyMetadataModel() + }; + } + catch (JsonException) + { + return new EmptyMetadataModel(); + } + } +} diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs index 1efc2f843d..79c31f956d 100644 --- a/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs @@ -211,4 +211,200 @@ public class PoliciesControllerTests : IClassFixture, IAs } } + [Fact] + public async Task Put_MasterPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.MasterPassword; + var request = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "minLength", "not a number" }, // Wrong type - should be int + { "requireUpper", true } + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("minLength", content); // Verify field name is in error message + } + + [Fact] + public async Task Put_SendOptionsPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.SendOptions; + var request = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "disableHideEmail", "not a boolean" } // Wrong type - should be bool + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task Put_ResetPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.ResetPassword; + var request = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "autoEnrollEnabled", 123 } // Wrong type - should be bool + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PutVNext_MasterPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.MasterPassword; + var request = new SavePolicyRequest + { + Policy = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "minComplexity", "not a number" }, // Wrong type - should be int + { "minLength", 12 } + } + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}/vnext", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("minComplexity", content); // Verify field name is in error message + } + + [Fact] + public async Task PutVNext_SendOptionsPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.SendOptions; + var request = new SavePolicyRequest + { + Policy = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "disableHideEmail", "not a boolean" } // Wrong type - should be bool + } + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}/vnext", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PutVNext_ResetPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.ResetPassword; + var request = new SavePolicyRequest + { + Policy = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = new Dictionary + { + { "autoEnrollEnabled", 123 } // Wrong type - should be bool + } + } + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}/vnext", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task Put_PolicyWithNullData_Success() + { + // Arrange + var policyType = PolicyType.SingleOrg; + var request = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = null + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task PutVNext_PolicyWithNullData_Success() + { + // Arrange + var policyType = PolicyType.TwoFactorAuthentication; + var request = new SavePolicyRequest + { + Policy = new PolicyRequestModel + { + Type = policyType, + Enabled = true, + Data = null + }, + Metadata = null + }; + + // Act + var response = await _client.PutAsync($"/organizations/{_organization.Id}/policies/{policyType}/vnext", + JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } } diff --git a/test/Api.IntegrationTest/AdminConsole/Public/Controllers/PoliciesControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Public/Controllers/PoliciesControllerTests.cs index f034426f98..0b5ab660b9 100644 --- a/test/Api.IntegrationTest/AdminConsole/Public/Controllers/PoliciesControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Public/Controllers/PoliciesControllerTests.cs @@ -160,4 +160,86 @@ public class PoliciesControllerTests : IClassFixture, IAs Assert.Equal(15, data.MinLength); Assert.Equal(true, data.RequireUpper); } + + [Fact] + public async Task Put_MasterPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.MasterPassword; + var request = new PolicyUpdateRequestModel + { + Enabled = true, + Data = new Dictionary + { + { "minLength", "not a number" }, // Wrong type - should be int + { "requireUpper", true } + } + }; + + // Act + var response = await _client.PutAsync($"/public/policies/{policyType}", JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task Put_SendOptionsPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.SendOptions; + var request = new PolicyUpdateRequestModel + { + Enabled = true, + Data = new Dictionary + { + { "disableHideEmail", "not a boolean" } // Wrong type - should be bool + } + }; + + // Act + var response = await _client.PutAsync($"/public/policies/{policyType}", JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task Put_ResetPasswordPolicy_InvalidDataType_ReturnsBadRequest() + { + // Arrange + var policyType = PolicyType.ResetPassword; + var request = new PolicyUpdateRequestModel + { + Enabled = true, + Data = new Dictionary + { + { "autoEnrollEnabled", 123 } // Wrong type - should be bool + } + }; + + // Act + var response = await _client.PutAsync($"/public/policies/{policyType}", JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task Put_PolicyWithNullData_Success() + { + // Arrange + var policyType = PolicyType.DisableSend; + var request = new PolicyUpdateRequestModel + { + Enabled = true, + Data = null + }; + + // Act + var response = await _client.PutAsync($"/public/policies/{policyType}", JsonContent.Create(request)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } } diff --git a/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs b/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs index 057680425a..75236fd719 100644 --- a/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs +++ b/test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs @@ -54,7 +54,7 @@ public class SavePolicyRequestTests } [Theory, BitAutoData] - public async Task ToSavePolicyModelAsync_WithNullData_HandlesCorrectly( + public async Task ToSavePolicyModelAsync_WithEmptyData_HandlesCorrectly( Guid organizationId, Guid userId) { @@ -68,10 +68,8 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.SingleOrg, - Enabled = false, - Data = null - }, - Metadata = null + Enabled = false + } }; // Act @@ -100,10 +98,8 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.SingleOrg, - Enabled = false, - Data = null - }, - Metadata = null + Enabled = false + } }; // Act @@ -133,8 +129,7 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.OrganizationDataOwnership, - Enabled = true, - Data = null + Enabled = true }, Metadata = new Dictionary { @@ -152,7 +147,7 @@ public class SavePolicyRequestTests } [Theory, BitAutoData] - public async Task ToSavePolicyModelAsync_OrganizationDataOwnership_WithNullMetadata_ReturnsEmptyMetadata( + public async Task ToSavePolicyModelAsync_OrganizationDataOwnership_WithEmptyMetadata_ReturnsEmptyMetadata( Guid organizationId, Guid userId) { @@ -166,10 +161,8 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.OrganizationDataOwnership, - Enabled = true, - Data = null - }, - Metadata = null + Enabled = true + } }; // Act @@ -246,8 +239,7 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.MaximumVaultTimeout, - Enabled = true, - Data = null + Enabled = true }, Metadata = new Dictionary { @@ -280,8 +272,7 @@ public class SavePolicyRequestTests Policy = new PolicyRequestModel { Type = PolicyType.OrganizationDataOwnership, - Enabled = true, - Data = null + Enabled = true }, Metadata = errorDictionary }; diff --git a/test/Core.Test/AdminConsole/Utilities/PolicyDataValidatorTests.cs b/test/Core.Test/AdminConsole/Utilities/PolicyDataValidatorTests.cs new file mode 100644 index 0000000000..43725d23e0 --- /dev/null +++ b/test/Core.Test/AdminConsole/Utilities/PolicyDataValidatorTests.cs @@ -0,0 +1,59 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; +using Bit.Core.AdminConsole.Utilities; +using Bit.Core.Exceptions; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.Utilities; + +public class PolicyDataValidatorTests +{ + [Fact] + public void ValidateAndSerialize_NullData_ReturnsNull() + { + var result = PolicyDataValidator.ValidateAndSerialize(null, PolicyType.MasterPassword); + + Assert.Null(result); + } + + [Fact] + public void ValidateAndSerialize_ValidData_ReturnsSerializedJson() + { + var data = new Dictionary { { "minLength", 12 } }; + + var result = PolicyDataValidator.ValidateAndSerialize(data, PolicyType.MasterPassword); + + Assert.NotNull(result); + Assert.Contains("\"minLength\":12", result); + } + + [Fact] + public void ValidateAndSerialize_InvalidDataType_ThrowsBadRequestException() + { + var data = new Dictionary { { "minLength", "not a number" } }; + + var exception = Assert.Throws(() => + PolicyDataValidator.ValidateAndSerialize(data, PolicyType.MasterPassword)); + + Assert.Contains("Invalid data for MasterPassword policy", exception.Message); + Assert.Contains("minLength", exception.Message); + } + + [Fact] + public void ValidateAndDeserializeMetadata_NullMetadata_ReturnsEmptyMetadataModel() + { + var result = PolicyDataValidator.ValidateAndDeserializeMetadata(null, PolicyType.SingleOrg); + + Assert.IsType(result); + } + + [Fact] + public void ValidateAndDeserializeMetadata_ValidMetadata_ReturnsModel() + { + var metadata = new Dictionary { { "defaultUserCollectionName", "collection name" } }; + + var result = PolicyDataValidator.ValidateAndDeserializeMetadata(metadata, PolicyType.OrganizationDataOwnership); + + Assert.IsType(result); + } +}