bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2025-12-06 00:03:34 +00:00
Code Issues Releases Wiki Activity

[PM-26430] Remove Type property from PolicyRequestModel to use route parameter only (#6472)

Browse Source 
* 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.

* Remove Type property from PolicyRequestModel to use route parameter only

* Run dotnet format

* 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: Remove PolicyType from PolicyRequestModel in PoliciesControllerTests

* test: Update PolicyDataValidatorTests to validate organization data ownership metadata

* Refactor PoliciesControllerTests to include policy type in PutVNext method calls
This commit is contained in:
Rui Tomé
2025-11-10 15:27:44 +00:00
committed by GitHub
parent 7d39efe29f
commit e7f3b6b12f
6 changed files with 30 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/Api/AdminConsole/Controllers/PoliciesController.cs
View File

@@ -209,23 +209,17 @@ public class PoliciesController : Controller
throw new NotFoundException();
}
if (type != model.Type)
{
throw new BadRequestException("Mismatched policy type");
}
var policyUpdate = await model.ToPolicyUpdateAsync(orgId, _currentContext);
var policyUpdate = await model.ToPolicyUpdateAsync(orgId, type, _currentContext);
var policy = await _savePolicyCommand.SaveAsync(policyUpdate);
return new PolicyResponseModel(policy);
}
[HttpPut("{type}/vnext")]
[RequireFeatureAttribute(FeatureFlagKeys.CreateDefaultLocation)]
[Authorize<ManagePoliciesRequirement>]
public async Task<PolicyResponseModel> PutVNext(Guid orgId, [FromBody] SavePolicyRequest model)
public async Task<PolicyResponseModel> PutVNext(Guid orgId, PolicyType type, [FromBody] SavePolicyRequest model)
{
var savePolicyRequest = await model.ToSavePolicyModelAsync(orgId, _currentContext);
var savePolicyRequest = await model.ToSavePolicyModelAsync(orgId, type, _currentContext);
var policy = _featureService.IsEnabled(FeatureFlagKeys.PolicyValidatorsRefactor) ?
await _vNextSavePolicyCommand.SaveAsync(savePolicyRequest) :
@@ -233,5 +227,4 @@ public class PoliciesController : Controller
return new PolicyResponseModel(policy);
}
}

8
src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs
View File

@@ -9,20 +9,18 @@ namespace Bit.Api.AdminConsole.Models.Request;
public class PolicyRequestModel
{
[Required]
public PolicyType? Type { get; set; }
[Required]
public bool? Enabled { get; set; }
public Dictionary<string, object>? Data { get; set; }
public async Task<PolicyUpdate> ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext)
public async Task<PolicyUpdate> ToPolicyUpdateAsync(Guid organizationId, PolicyType type, ICurrentContext currentContext)
{
var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, Type!.Value);
var serializedData = PolicyDataValidator.ValidateAndSerialize(Data, type);
var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId));
return new()
{
Type = Type!.Value,
Type = type,
OrganizationId = organizationId,
Data = serializedData,
Enabled = Enabled.GetValueOrDefault(),

7
src/Api/AdminConsole/Models/Request/SavePolicyRequest.cs
View File

@@ -1,4 +1,5 @@
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;
@@ -13,10 +14,10 @@ public class SavePolicyRequest
public Dictionary<string, object>? Metadata { get; set; }
public async Task<SavePolicyModel> ToSavePolicyModelAsync(Guid organizationId, ICurrentContext currentContext)
public async Task<SavePolicyModel> ToSavePolicyModelAsync(Guid organizationId, PolicyType type, ICurrentContext currentContext)
{
var policyUpdate = await Policy.ToPolicyUpdateAsync(organizationId, currentContext);
var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, Policy.Type!.Value);
var policyUpdate = await Policy.ToPolicyUpdateAsync(organizationId, type, currentContext);
var metadata = PolicyDataValidator.ValidateAndDeserializeMetadata(Metadata, type);
var performedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId));
return new SavePolicyModel(policyUpdate, performedBy, metadata);

10
test/Api.IntegrationTest/AdminConsole/Controllers/PoliciesControllerTests.cs
View File

@@ -67,7 +67,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
},
Metadata = new Dictionary<string, object>
@@ -148,7 +147,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -218,7 +216,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
var policyType = PolicyType.MasterPassword;
var request = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -244,7 +241,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
var policyType = PolicyType.SendOptions;
var request = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -267,7 +263,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
var policyType = PolicyType.ResetPassword;
var request = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -292,7 +287,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -321,7 +315,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -347,7 +340,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = new Dictionary<string, object>
{
@@ -371,7 +363,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
var policyType = PolicyType.SingleOrg;
var request = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = null
};
@@ -393,7 +384,6 @@ public class PoliciesControllerTests : IClassFixture<ApiApplicationFactory>, IAs
{
Policy = new PolicyRequestModel
{
Type = policyType,
Enabled = true,
Data = null
},

34
test/Api.Test/AdminConsole/Models/Request/SavePolicyRequestTests.cs
View File

@@ -24,11 +24,11 @@ public class SavePolicyRequestTests
currentContext.OrganizationOwner(organizationId).Returns(true);
var testData = new Dictionary<string, object> { { "test", "value" } };
var policyType = PolicyType.TwoFactorAuthentication;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.TwoFactorAuthentication,
Enabled = true,
Data = testData
},
@@ -36,7 +36,7 @@ public class SavePolicyRequestTests
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.Equal(PolicyType.TwoFactorAuthentication, result.PolicyUpdate.Type);
@@ -63,17 +63,17 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(false);
var policyType = PolicyType.SingleOrg;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.SingleOrg,
Enabled = false
}
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.Null(result.PolicyUpdate.Data);
@@ -93,17 +93,17 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(true);
var policyType = PolicyType.SingleOrg;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.SingleOrg,
Enabled = false
}
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.Null(result.PolicyUpdate.Data);
@@ -124,11 +124,11 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(true);
var policyType = PolicyType.OrganizationDataOwnership;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.OrganizationDataOwnership,
Enabled = true
},
Metadata = new Dictionary<string, object>
@@ -138,7 +138,7 @@ public class SavePolicyRequestTests
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.IsType<OrganizationModelOwnershipPolicyModel>(result.Metadata);
@@ -156,17 +156,17 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(true);
var policyType = PolicyType.OrganizationDataOwnership;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.OrganizationDataOwnership,
Enabled = true
}
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.NotNull(result);
@@ -193,12 +193,11 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(true);
var policyType = PolicyType.ResetPassword;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.ResetPassword,
Enabled = true,
Data = _complexData
},
@@ -206,7 +205,7 @@ public class SavePolicyRequestTests
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
var deserializedData = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(result.PolicyUpdate.Data);
@@ -234,11 +233,11 @@ public class SavePolicyRequestTests
currentContext.UserId.Returns(userId);
currentContext.OrganizationOwner(organizationId).Returns(true);
var policyType = PolicyType.MaximumVaultTimeout;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.MaximumVaultTimeout,
Enabled = true
},
Metadata = new Dictionary<string, object>
@@ -248,7 +247,7 @@ public class SavePolicyRequestTests
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.NotNull(result);
@@ -266,19 +265,18 @@ public class SavePolicyRequestTests
currentContext.OrganizationOwner(organizationId).Returns(true);
var errorDictionary = BuildErrorDictionary();
var policyType = PolicyType.OrganizationDataOwnership;
var model = new SavePolicyRequest
{
Policy = new PolicyRequestModel
{
Type = PolicyType.OrganizationDataOwnership,
Enabled = true
},
Metadata = errorDictionary
};
// Act
var result = await model.ToSavePolicyModelAsync(organizationId, currentContext);
var result = await model.ToSavePolicyModelAsync(organizationId, policyType, currentContext);
// Assert
Assert.NotNull(result);

8
test/Api.Test/Controllers/PoliciesControllerTests.cs
View File

@@ -487,14 +487,14 @@ public class PoliciesControllerTests
.Returns(policy);
// Act
var result = await sutProvider.Sut.PutVNext(orgId, model);
var result = await sutProvider.Sut.PutVNext(orgId, policy.Type, model);
// Assert
await sutProvider.GetDependency<IVNextSavePolicyCommand>()
.Received(1)
.SaveAsync(Arg.Is<SavePolicyModel>(
m => m.PolicyUpdate.OrganizationId == orgId &&
m.PolicyUpdate.Type == model.Policy.Type &&
m.PolicyUpdate.Type == policy.Type &&
m.PolicyUpdate.Enabled == model.Policy.Enabled &&
m.PerformedBy.UserId == userId &&
m.PerformedBy.IsOrganizationOwnerOrProvider == true));
@@ -534,14 +534,14 @@ public class PoliciesControllerTests
.Returns(policy);
// Act
var result = await sutProvider.Sut.PutVNext(orgId, model);
var result = await sutProvider.Sut.PutVNext(orgId, policy.Type, model);
// Assert
await sutProvider.GetDependency<ISavePolicyCommand>()
.Received(1)
.VNextSaveAsync(Arg.Is<SavePolicyModel>(
m => m.PolicyUpdate.OrganizationId == orgId &&
m.PolicyUpdate.Type == model.Policy.Type &&
m.PolicyUpdate.Type == policy.Type &&
m.PolicyUpdate.Enabled == model.Policy.Enabled &&
m.PerformedBy.UserId == userId &&
m.PerformedBy.IsOrganizationOwnerOrProvider == true));