From dfd1c22bae93af7a729398cd51331509665e287f Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 9 Feb 2024 12:44:04 +0100 Subject: [PATCH 1/2] Explore making responses required and nullable --- .../Response/BaseSecretResponseModel.cs | 33 +++++++++---------- .../Models/Response/SecretResponseModel.cs | 4 --- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs index 0579baec07d9..59ba5ac72ae1 100644 --- a/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs @@ -1,4 +1,7 @@ -using Bit.Core.Models.Api; +#nullable enable + +using System.ComponentModel.DataAnnotations; +using Bit.Core.Models.Api; using Bit.Core.SecretsManager.Entities; namespace Bit.Api.SecretsManager.Models.Response; @@ -24,29 +27,25 @@ public BaseSecretResponseModel(Secret secret, string objectName = _objectName) : Projects = secret.Projects?.Select(p => new SecretResponseInnerProject(p)); } - public BaseSecretResponseModel(string objectName = _objectName) : base(objectName) - { - } - - public BaseSecretResponseModel() : base(_objectName) - { - } - - public Guid Id { get; set; } + [Required] + public Guid Id { get; } - public Guid OrganizationId { get; set; } + [Required] + public Guid OrganizationId { get; } - public string Key { get; set; } + public string? Key { get; } - public string Value { get; set; } + public string? Value { get; } - public string Note { get; set; } + public string? Note { get; } - public DateTime CreationDate { get; set; } + [Required] + public DateTime CreationDate { get; } - public DateTime RevisionDate { get; set; } + [Required] + public DateTime RevisionDate { get; } - public IEnumerable Projects { get; set; } + public IEnumerable? Projects { get; init; } public class SecretResponseInnerProject { diff --git a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs index c4633f78f05c..b2953ed123a0 100644 --- a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs @@ -12,10 +12,6 @@ public SecretResponseModel(Secret secret, bool read, bool write) : base(secret, Write = write; } - public SecretResponseModel() : base(_objectName) - { - } - public bool Read { get; set; } public bool Write { get; set; } From 1caac5050af6fd3a85c51ed8d3f9f4bee6af6a1a Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 9 Feb 2024 13:45:22 +0100 Subject: [PATCH 2/2] Replace with Swagger filter --- .../Response/BaseSecretResponseModel.cs | 5 -- .../Utilities/ServiceCollectionExtensions.cs | 1 + .../Swagger/RequireNotNullableSchemaFilter.cs | 66 +++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 src/SharedWeb/Swagger/RequireNotNullableSchemaFilter.cs diff --git a/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs index 59ba5ac72ae1..622499016a5c 100644 --- a/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs @@ -1,6 +1,5 @@ #nullable enable -using System.ComponentModel.DataAnnotations; using Bit.Core.Models.Api; using Bit.Core.SecretsManager.Entities; @@ -27,10 +26,8 @@ public BaseSecretResponseModel(Secret secret, string objectName = _objectName) : Projects = secret.Projects?.Select(p => new SecretResponseInnerProject(p)); } - [Required] public Guid Id { get; } - [Required] public Guid OrganizationId { get; } public string? Key { get; } @@ -39,10 +36,8 @@ public BaseSecretResponseModel(Secret secret, string objectName = _objectName) : public string? Note { get; } - [Required] public DateTime CreationDate { get; } - [Required] public DateTime RevisionDate { get; } public IEnumerable? Projects { get; init; } diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index 310a7758911e..150f9c6d0dd6 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/src/Api/Utilities/ServiceCollectionExtensions.cs @@ -71,6 +71,7 @@ public static void AddSwagger(this IServiceCollection services, GlobalSettings g // config.UseReferencedDefinitionsForEnums(); config.SchemaFilter(); + config.SchemaFilter(); var apiFilePath = Path.Combine(AppContext.BaseDirectory, "Api.xml"); config.IncludeXmlComments(apiFilePath, true); diff --git a/src/SharedWeb/Swagger/RequireNotNullableSchemaFilter.cs b/src/SharedWeb/Swagger/RequireNotNullableSchemaFilter.cs new file mode 100644 index 000000000000..93de13c17ad6 --- /dev/null +++ b/src/SharedWeb/Swagger/RequireNotNullableSchemaFilter.cs @@ -0,0 +1,66 @@ +using System.Reflection; +using Microsoft.OpenApi.Models; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace Bit.SharedWeb.Swagger; + +/// +/// +/// +/// +/// Credits: https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2036#issuecomment-894015122 +/// +public class RequireNotNullableSchemaFilter : ISchemaFilter +{ + public void Apply(OpenApiSchema schema, SchemaFilterContext context) + { + if (schema.Properties == null) + { + return; + } + + FixNullableProperties(schema, context); + + var notNullableProperties = schema + .Properties + .Where(x => !x.Value.Nullable && x.Value.Default == default && !schema.Required.Contains(x.Key)) + .ToList(); + + foreach (var property in notNullableProperties) + { + schema.Required.Add(property.Key); + } + } + + /// + /// Option "SupportNonNullableReferenceTypes" not working with complex types ({ "type": "object" }), + /// so they always have "Nullable = false", + /// see method "SchemaGenerator.GenerateSchemaForMember" + /// + private static void FixNullableProperties(OpenApiSchema schema, SchemaFilterContext context) + { + foreach (var property in schema.Properties) + { + var field = context.Type + .GetMembers(BindingFlags.Public | BindingFlags.Instance) + .FirstOrDefault(x => + string.Equals(x.Name, property.Key, StringComparison.InvariantCultureIgnoreCase)); + + if (field == null) + { + continue; + } + + var fieldType = field switch + { + FieldInfo fieldInfo => fieldInfo.FieldType, + PropertyInfo propertyInfo => propertyInfo.PropertyType, + _ => throw new NotSupportedException(), + }; + + property.Value.Nullable = fieldType.IsValueType + ? Nullable.GetUnderlyingType(fieldType) != null + : !field.IsNonNullableReferenceType(); + } + } +}