From 5f2da6e4b701b2c307cd6a9ade419d392b0efe85 Mon Sep 17 00:00:00 2001
From: Matt Gibson <mgibson@bitwarden.com>
Date: Wed, 2 Feb 2022 14:26:15 -0500
Subject: [PATCH] Fix/identity service model validation (#1837)

* Fix indentation

* Fix comment

* Extract ModelStateValidationFilter add to Indentity MVC opts

* Remove unnecessary base constructor call
---
 src/Api/Startup.cs                            |  2 +-
 .../ModelStateValidationFilterAttribute.cs    | 21 ++++--------
 .../Controllers/AccountsController.cs         |  4 +--
 src/Identity/Startup.cs                       |  6 +++-
 .../ModelStateValidationFilterAttribute.cs    | 33 +++++++++++++++++++
 5 files changed, 47 insertions(+), 19 deletions(-)
 create mode 100644 src/SharedWeb/Utilities/ModelStateValidationFilterAttribute.cs

diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs
index 1953064071..57e0c1acd3 100644
--- a/src/Api/Startup.cs
+++ b/src/Api/Startup.cs
@@ -124,7 +124,7 @@ namespace Bit.Api
             services.AddCoreLocalizationServices();
 
 #if OSS
-                services.AddOosServices();
+            services.AddOosServices();
 #else
             services.AddCommCoreServices();
 #endif
diff --git a/src/Api/Utilities/ModelStateValidationFilterAttribute.cs b/src/Api/Utilities/ModelStateValidationFilterAttribute.cs
index 8cda1e5423..749df76879 100644
--- a/src/Api/Utilities/ModelStateValidationFilterAttribute.cs
+++ b/src/Api/Utilities/ModelStateValidationFilterAttribute.cs
@@ -6,7 +6,7 @@ using InternalApi = Bit.Core.Models.Api;
 
 namespace Bit.Api.Utilities
 {
-    public class ModelStateValidationFilterAttribute : ActionFilterAttribute
+    public class ModelStateValidationFilterAttribute : SharedWeb.Utilities.ModelStateValidationFilterAttribute
     {
         private readonly bool _publicApi;
 
@@ -15,24 +15,15 @@ namespace Bit.Api.Utilities
             _publicApi = publicApi;
         }
 
-        public override void OnActionExecuting(ActionExecutingContext context)
+        protected override void OnModelStateInvalid(ActionExecutingContext context)
         {
-            var model = context.ActionArguments.FirstOrDefault(a => a.Key == "model");
-            if (model.Key == "model" && model.Value == null)
+            if (_publicApi)
             {
-                context.ModelState.AddModelError(string.Empty, "Body is empty.");
+                context.Result = new BadRequestObjectResult(new ErrorResponseModel(context.ModelState));
             }
-
-            if (!context.ModelState.IsValid)
+            else
             {
-                if (_publicApi)
-                {
-                    context.Result = new BadRequestObjectResult(new ErrorResponseModel(context.ModelState));
-                }
-                else
-                {
-                    context.Result = new BadRequestObjectResult(new InternalApi.ErrorResponseModel(context.ModelState));
-                }
+                context.Result = new BadRequestObjectResult(new InternalApi.ErrorResponseModel(context.ModelState));
             }
         }
     }
diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs
index b56f5277be..e507b22996 100644
--- a/src/Identity/Controllers/AccountsController.cs
+++ b/src/Identity/Controllers/AccountsController.cs
@@ -30,7 +30,7 @@ namespace Bit.Identity.Controllers
             _userService = userService;
         }
 
-        // Moved from API, If you modify this endpoint, please update Identity as well.
+        // Moved from API, If you modify this endpoint, please update API as well.
         [HttpPost("register")]
         [CaptchaProtected]
         public async Task PostRegister([FromBody] RegisterRequestModel model)
@@ -51,7 +51,7 @@ namespace Bit.Identity.Controllers
             throw new BadRequestException(ModelState);
         }
 
-        // Moved from API, If you modify this endpoint, please update Identity as well.
+        // Moved from API, If you modify this endpoint, please update API as well.
         [HttpPost("prelogin")]
         public async Task<PreloginResponseModel> PostPrelogin([FromBody] PreloginRequestModel model)
         {
diff --git a/src/Identity/Startup.cs b/src/Identity/Startup.cs
index 7584bf99f3..37afbb88be 100644
--- a/src/Identity/Startup.cs
+++ b/src/Identity/Startup.cs
@@ -58,7 +58,11 @@ namespace Bit.Identity
             services.AddMemoryCache();
 
             // Mvc
-            services.AddMvc();
+            // MVC
+            services.AddMvc(config =>
+            {
+                config.Filters.Add(new ModelStateValidationFilterAttribute());
+            });
 
             if (!globalSettings.SelfHosted)
             {
diff --git a/src/SharedWeb/Utilities/ModelStateValidationFilterAttribute.cs b/src/SharedWeb/Utilities/ModelStateValidationFilterAttribute.cs
new file mode 100644
index 0000000000..f0f087301f
--- /dev/null
+++ b/src/SharedWeb/Utilities/ModelStateValidationFilterAttribute.cs
@@ -0,0 +1,33 @@
+using System.Linq;
+using Bit.Core.Models.Api;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Filters;
+
+namespace Bit.SharedWeb.Utilities
+{
+    public class ModelStateValidationFilterAttribute : ActionFilterAttribute
+    {
+        public ModelStateValidationFilterAttribute()
+        {
+        }
+
+        public override void OnActionExecuting(ActionExecutingContext context)
+        {
+            var model = context.ActionArguments.FirstOrDefault(a => a.Key == "model");
+            if (model.Key == "model" && model.Value == null)
+            {
+                context.ModelState.AddModelError(string.Empty, "Body is empty.");
+            }
+
+            if (!context.ModelState.IsValid)
+            {
+                OnModelStateInvalid(context);
+            }
+        }
+
+        protected virtual void OnModelStateInvalid(ActionExecutingContext context)
+        {
+            context.Result = new BadRequestObjectResult(new ErrorResponseModel(context.ModelState));
+        }
+    }
+}