Give every validation failure one coded problem shape (string matching)

src/Api/AdminConsole/Controllers/ValidationErrorTypedResultsExtensions.cs

@@ -18,11 +18,11 @@ public static BitwardenValidationProblemResult BitwardenValidationProblem(IValid
             ArgumentNullException.ThrowIfNull(validationError);
 
             return TypedResults.BitwardenValidationProblem(
-                errors: new Dictionary<string, BitwardenTypedResultsExtensions.ErrorCode[]>
+                errors: new Dictionary<string, ErrorCode[]>
                 {
                     {
                         validationError.PropertyName,
-                        [new BitwardenTypedResultsExtensions.ErrorCode(validationError.Type, validationError.Message)]
+                        [new ErrorCode(validationError.Type, validationError.Message)]
                     }
                 });
         }

src/Api/Utilities/ModelStateValidationFilterAttribute.cs

@@ -14,15 +14,19 @@ public ModelStateValidationFilterAttribute(bool publicApi)
         _publicApi = publicApi;
     }
 
+    /// <remarks>
+    /// Only the internal API takes the coded document. The public API's error shape is a published contract with
+    /// its own versioning, so it keeps answering as it always has until that contract is revised deliberately.
+    /// </remarks>
     protected override void OnModelStateInvalid(ActionExecutingContext context)
     {
         if (_publicApi)
         {
             context.Result = new BadRequestObjectResult(new ErrorResponseModel(context.ModelState));
+            return;
         }
-        else
-        {
-            context.Result = new BadRequestObjectResult(new InternalApi.ErrorResponseModel(context.ModelState));
-        }
+
+        context.Result = TryCodedProblem(context)
+            ?? new BadRequestObjectResult(new InternalApi.ErrorResponseModel(context.ModelState));
     }
 }

src/Core/Constants.cs

@@ -243,6 +243,7 @@ public static partial class FeatureFlagKeys
     public const string OrgCipherPushFanout = "pm-35168-org-cipher-push-fanout";
     public const string FedRampGovRegion = "fedramp-gov-region";
     public const string ManagedDeviceFramework = "pm-27719-managed-device-framework";
+    public const string CodedValidationProblems = "coded-validation-problems";
 
     /* Tools Team */
     public const string UseSdkPasswordGenerators = "pm-19976-use-sdk-password-generators";

src/HttpExtensions/BitwardenTypedResultsExtensions.cs

@@ -7,38 +7,88 @@ public static class BitwardenTypedResultsExtensions
     extension(TypedResults)
     {
         /// <summary>
-        /// Produces a 400 Bad Request RFC 7807 problem response, mirroring
-        /// <c>TypedResults.ValidationProblem</c> but typing each error entry as an array of
-        /// <see cref="ErrorCode"/> rather than <c>string[]</c>.
+        /// Produces an RFC 7807 problem response, mirroring <c>TypedResults.ValidationProblem</c> but typing
+        /// each error entry as an array of <see cref="ErrorCode"/> rather than <c>string[]</c>.
         /// <remarks>
         /// WARNING: This is currently experimental and may change in the future.
         /// </remarks>
         /// </summary>
+        /// <param name="statusCode">
+        /// The response status. Defaults to 400 Bad Request. Pass another 4xx when the failure is carried by this
+        /// same body but is not bad input — 409 Conflict for a state conflict, say — so a caller that only reads
+        /// the status still learns something before it reads the codes.
+        /// </param>
+        /// <param name="extensions">
+        /// Further problem members. A member named <c>errors</c> is dropped: that name belongs to
+        /// <paramref name="errors"/>, and a document carrying it twice is not parseable.
+        /// </param>
         public static BitwardenValidationProblemResult BitwardenValidationProblem(
             IDictionary<string, ErrorCode[]> errors,
             string? detail = null,
             string? instance = null,
             string title = "One or more validation errors occurred.",
             string type = "validation_error",
-            IDictionary<string, object?>? extensions = null)
+            IDictionary<string, object?>? extensions = null,
+            int statusCode = StatusCodes.Status400BadRequest)
         {
             ArgumentNullException.ThrowIfNull(errors);
 
-            var problemExtensions = extensions is null
-                ? new Dictionary<string, object?>()
-                : new Dictionary<string, object?>(extensions);
+            var problemDetails = new BitwardenValidationProblemDetails
+            {
+                Detail = detail,
+                Instance = instance,
+                Status = statusCode,
+                Title = title,
+                Type = type,
+                Errors = new Dictionary<string, ErrorCode[]>(errors),
+            };
 
-            problemExtensions["errors"] = errors;
+            if (extensions is not null)
+            {
+                foreach (var (key, value) in extensions
+                             .Where(extension => extension.Key != BitwardenValidationProblemDetails.ErrorsMember))
+                {
+                    problemDetails.Extensions[key] = value;
+                }
+            }
 
-            return new BitwardenValidationProblemResult(TypedResults.Problem(
+            return new BitwardenValidationProblemResult(problemDetails);
+        }
+
+        /// <summary>
+        /// Produces an RFC 7807 problem response from a flat sequence of property/code pairs, grouping them by
+        /// property so a caller that discovers failures one at a time does not have to build the dictionary itself.
+        /// <remarks>
+        /// WARNING: This is currently experimental and may change in the future.
+        /// </remarks>
+        /// </summary>
+        /// <param name="errors">
+        /// The failures, in the order they should appear under each property. More than one pair may name the same
+        /// property; they are collected into that property's array rather than overwriting one another.
+        /// </param>
+        public static BitwardenValidationProblemResult BitwardenValidationProblem(
+            IEnumerable<(string PropertyName, ErrorCode Code)> errors,
+            string? detail = null,
+            string? instance = null,
+            string title = "One or more validation errors occurred.",
+            string type = "validation_error",
+            IDictionary<string, object?>? extensions = null,
+            int statusCode = StatusCodes.Status400BadRequest)
+        {
+            ArgumentNullException.ThrowIfNull(errors);
+
+            var grouped = errors
+                .GroupBy(error => error.PropertyName)
+                .ToDictionary(group => group.Key, group => group.Select(error => error.Code).ToArray());
+
+            return TypedResults.BitwardenValidationProblem(
+                errors: grouped,
                 detail: detail,
                 instance: instance,
-                statusCode: StatusCodes.Status400BadRequest,
                 title: title,
                 type: type,
-                extensions: problemExtensions));
+                extensions: extensions,
+                statusCode: statusCode);
         }
     }
-
-    public record ErrorCode(string Type, string Detail);
 }

src/HttpExtensions/BitwardenValidationProblemDetails.cs

@@ -0,0 +1,31 @@
+using System.Text.Json.Serialization;
+using Microsoft.AspNetCore.Mvc;
+
+namespace Bit.HttpExtensions;
+
+/// <summary>
+/// The body a <see cref="BitwardenValidationProblemResult"/> writes: the RFC 7807 members plus an
+/// <c>errors</c> map keyed by the property that failed, each entry carrying a code the client switches on.
+/// </summary>
+/// <remarks>
+/// Named as a type rather than assembled into <see cref="ProblemDetails.Extensions"/> so that the document a
+/// client parses and the document OpenAPI describes are the same declaration, and neither can drift from the
+/// other.
+/// </remarks>
+public sealed class BitwardenValidationProblemDetails : ProblemDetails
+{
+    /// <summary>The document member the errors map is written under.</summary>
+    internal const string ErrorsMember = "errors";
+
+    /// <summary>
+    /// The failures, keyed by the property name as it appeared on the wire. A property that failed several ways
+    /// carries an entry per failure.
+    /// </summary>
+    /// <remarks>
+    /// Ordered after the inherited members so the code that reads the document meets <c>type</c> and
+    /// <c>status</c> before the detail of what went wrong, as it does in every other problem response.
+    /// </remarks>
+    [JsonPropertyOrder(100)]
+    [JsonPropertyName(ErrorsMember)]
+    public IDictionary<string, ErrorCode[]> Errors { get; init; } = new Dictionary<string, ErrorCode[]>();
+}

src/HttpExtensions/BitwardenValidationProblemResult.cs

@@ -1,39 +1,62 @@
-using Microsoft.AspNetCore.Http;
+using System.Reflection;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.HttpResults;
+using Microsoft.AspNetCore.Http.Metadata;
 using Microsoft.AspNetCore.Mvc;
 
 namespace Bit.HttpExtensions;
 
 /// <summary>
 /// A Bitwarden-flavored RFC 7807 validation problem result. Wraps an inner
-/// <see cref="ProblemHttpResult"/> so we have room to grow — for example, implementing
-/// <c>IEndpointMetadataProvider</c> for OpenAPI — without changing the public signature of
+/// <see cref="ProblemHttpResult"/> so we have room to grow without changing the public signature of
 /// <c>TypedResults.BitwardenValidationProblem</c>.
 /// </summary>
 public sealed class BitwardenValidationProblemResult :
     IResult,
+    IEndpointMetadataProvider,
     IStatusCodeHttpResult,
     IContentTypeHttpResult,
     IValueHttpResult,
     IValueHttpResult<ProblemDetails>
 {
     private readonly ProblemHttpResult _inner;
 
-    internal BitwardenValidationProblemResult(ProblemHttpResult inner)
+    internal BitwardenValidationProblemResult(BitwardenValidationProblemDetails problemDetails)
     {
-        ArgumentNullException.ThrowIfNull(inner);
-        _inner = inner;
+        ArgumentNullException.ThrowIfNull(problemDetails);
+        _inner = TypedResults.Problem(problemDetails);
+        ProblemDetails = problemDetails;
     }
 
-    public ProblemDetails ProblemDetails => _inner.ProblemDetails;
+    public BitwardenValidationProblemDetails ProblemDetails { get; }
 
     public int? StatusCode => _inner.StatusCode;
 
     public string? ContentType => _inner.ContentType;
 
-    object? IValueHttpResult.Value => _inner.ProblemDetails;
+    object? IValueHttpResult.Value => ProblemDetails;
 
-    ProblemDetails? IValueHttpResult<ProblemDetails>.Value => _inner.ProblemDetails;
+    ProblemDetails? IValueHttpResult<ProblemDetails>.Value => ProblemDetails;
 
     public Task ExecuteAsync(HttpContext httpContext) => _inner.ExecuteAsync(httpContext);
+
+    /// <summary>
+    /// Declares the problem document on every endpoint whose handler can return this result, so a client reads
+    /// the coded shape out of OpenAPI rather than out of our source.
+    /// </summary>
+    /// <remarks>
+    /// Declares 400, the status the result defaults to. An endpoint that carries this same body on another
+    /// status — 409 for a state conflict — declares that one itself with <c>Produces</c>.
+    /// </remarks>
+    public static void PopulateMetadata(MethodInfo method, EndpointBuilder builder)
+    {
+        ArgumentNullException.ThrowIfNull(method);
+        ArgumentNullException.ThrowIfNull(builder);
+
+        builder.Metadata.Add(new ProducesResponseTypeMetadata(
+            StatusCodes.Status400BadRequest,
+            typeof(BitwardenValidationProblemDetails),
+            ["application/problem+json"]));
+    }
 }

src/HttpExtensions/ErrorCode.cs

@@ -0,0 +1,33 @@
+using System.Text.Json.Nodes;
+using System.Text.Json.Serialization;
+
+namespace Bit.HttpExtensions;
+
+/// <summary>
+/// One failure under a property: a stable <paramref name="Type"/> the client switches on, a human-readable
+/// <paramref name="Detail"/>, and the substitutions it needs to render that detail in another language.
+/// </summary>
+/// <param name="Type">
+/// The machine-readable code. Names what went wrong and never the property it is keyed under — <c>required</c>,
+/// not <c>name_required</c>. Draw it from <see cref="ValidationCodes"/> rather than spelling one locally.
+/// </param>
+/// <param name="Detail">
+/// The English message. A client that localizes renders from <paramref name="Type"/> and its
+/// <paramref name="Parameters"/> instead, so this is a fallback rather than the contract.
+/// </param>
+/// <param name="Parameters">
+/// The substitutions a client needs to render its own message for <paramref name="Type"/> — a length limit, a
+/// range bound. Omitted from the body when null, so a code needing no substitution costs nothing. Carries the
+/// limit that was breached and never anything derived from the value that breached it: a length ceiling, never
+/// the string that overran it. Key it from <see cref="ValidationParameters"/>.
+/// </param>
+/// <remarks>
+/// <see cref="JsonObject"/> rather than a dictionary of <c>object</c> so the document stays serializable without
+/// reflection. An <c>object</c> value has to have its runtime type resolved when it is written, which is the one
+/// thing a source-generated — and so trim- and AOT-safe — serializer cannot do.
+/// </remarks>
+public sealed record ErrorCode(
+    string Type,
+    string Detail,
+    [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
+    JsonObject? Parameters = null);

src/HttpExtensions/HttpExtensions.csproj

@@ -1,5 +1,15 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
+  <PropertyGroup>
+    <!--
+        Turns the trim and AOT analyzers on for this assembly so it stays publishable from a trimmed or
+        ahead-of-time minimal API. Reflective paths here are reachable only from MVC, which is unsupported under
+        both, and say so with [RequiresUnreferencedCode]; this keeps anything new from quietly joining them.
+    -->
+    <IsAotCompatible>true</IsAotCompatible>
+    <TrimmerSingleWarn>false</TrimmerSingleWarn>
+  </PropertyGroup>
+
   <ItemGroup>
     <FrameworkReference Include="Microsoft.AspNetCore.App" />
   </ItemGroup>