From 5ff1a59a99b9b639a5f9082c73bcea1f8765541c Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Fri, 25 Nov 2022 16:11:13 +0900 Subject: [PATCH] Make sure the form is properly validated --- BTCPayServer.Abstractions/Form/Field.cs | 27 ++++++++----- BTCPayServer.Abstractions/Form/Form.cs | 14 ++++++- .../Form/HtmlInputField.cs | 4 +- .../Controllers/UIPaymentRequestController.cs | 40 +++++++++++-------- BTCPayServer/Forms/UIFormsController.cs | 24 +++++------ .../Controllers/UIPointOfSaleController.cs | 15 ++++--- 6 files changed, 76 insertions(+), 48 deletions(-) diff --git a/BTCPayServer.Abstractions/Form/Field.cs b/BTCPayServer.Abstractions/Form/Field.cs index d64603783..29bc20b93 100644 --- a/BTCPayServer.Abstractions/Form/Field.cs +++ b/BTCPayServer.Abstractions/Form/Field.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.Linq; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -19,16 +20,22 @@ public class Field // If this is the first the user sees the form, then value and original value are the same. Value changes as the user starts interacting with the form. public string Value; - // The field is considered "valid" if there are no validation errors - public List ValidationErrors = new List(); - - public virtual bool IsValid() - { - return ValidationErrors.Count == 0 && Fields.All(field => field.IsValid()); - } - + public bool Required; [JsonExtensionData] public IDictionary AdditionalData { get; set; } public List Fields { get; set; } = new(); - - + + public virtual void Validate(ModelStateDictionary modelState) + { + if (Required && string.IsNullOrEmpty(Value)) + { + modelState.AddModelError(Name, "This field is required"); + } + } + + public bool IsValid() + { + ModelStateDictionary modelState = new ModelStateDictionary(); + Validate(modelState); + return modelState.IsValid; + } } diff --git a/BTCPayServer.Abstractions/Form/Form.cs b/BTCPayServer.Abstractions/Form/Form.cs index 39530a843..077aa5d24 100644 --- a/BTCPayServer.Abstractions/Form/Form.cs +++ b/BTCPayServer.Abstractions/Form/Form.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Newtonsoft.Json.Linq; namespace BTCPayServer.Abstractions.Form; @@ -28,7 +29,7 @@ public class Form // Are all the fields valid in the form? public bool IsValid() { - return Fields.All(field => field.IsValid()); + return Validate(null); } public Field GetFieldByName(string name) @@ -63,6 +64,17 @@ public class Form } return null; } + +#nullable enable + public bool Validate(ModelStateDictionary? modelState) + { + modelState ??= new ModelStateDictionary(); + foreach (var field in Fields) + field.Validate(modelState); + return modelState.IsValid; + } +#nullable restore + public List GetAllNames() { return GetAllNames(Fields); diff --git a/BTCPayServer.Abstractions/Form/HtmlInputField.cs b/BTCPayServer.Abstractions/Form/HtmlInputField.cs index 3bfb6806a..96a35ba32 100644 --- a/BTCPayServer.Abstractions/Form/HtmlInputField.cs +++ b/BTCPayServer.Abstractions/Form/HtmlInputField.cs @@ -1,3 +1,5 @@ +using Microsoft.AspNetCore.Mvc.ModelBinding; + namespace BTCPayServer.Abstractions.Form; public class HtmlInputField : Field @@ -11,7 +13,6 @@ public class HtmlInputField : Field // A useful note shown below the field or via a tooltip / info icon. Should be translated for the user. public string HelpText; - public bool Required; public HtmlInputField(string label, string name, string value, bool required, string helpText, string type = "text") { Label = label; @@ -22,6 +23,5 @@ public class HtmlInputField : Field HelpText = helpText; Type = type; } - // TODO JSON parsing from string to objects again probably won't work out of the box because of the different field types. } diff --git a/BTCPayServer/Controllers/UIPaymentRequestController.cs b/BTCPayServer/Controllers/UIPaymentRequestController.cs index 661dfdd47..6f499df61 100644 --- a/BTCPayServer/Controllers/UIPaymentRequestController.cs +++ b/BTCPayServer/Controllers/UIPaymentRequestController.cs @@ -5,6 +5,7 @@ using System.Threading; using System.Threading.Tasks; using BTCPayServer.Abstractions.Constants; using BTCPayServer.Abstractions.Extensions; +using BTCPayServer.Abstractions.Form; using BTCPayServer.Client; using BTCPayServer.Client.Models; using BTCPayServer.Data; @@ -181,7 +182,7 @@ namespace BTCPayServer.Controllers [HttpGet("{payReqId}/form")] [HttpPost("{payReqId}/form")] [AllowAnonymous] - public async Task ViewPaymentRequestForm(string payReqId, [FromForm] string formId, [FromForm] string formData) + public async Task ViewPaymentRequestForm(string payReqId) { var result = await _PaymentRequestRepository.FindPaymentRequest(payReqId, GetUserId()); if (result == null) @@ -195,32 +196,37 @@ namespace BTCPayServer.Controllers { case null: case { } when string.IsNullOrEmpty(prFormId): - break; - + case { } when Request.Method == "GET" && prBlob.FormResponse is not null: + return RedirectToAction("ViewPaymentRequest", new { payReqId }); + case { } when Request.Method == "GET" && prBlob.FormResponse is null: + break; default: // POST case: Handle form submit - if (!string.IsNullOrEmpty(formData) && formId == prFormId) + var formData = Form.Parse(Forms.UIFormsController.GetFormData(prFormId).Config); + formData.ApplyValuesFromForm(this.Request.Form); + if (formData.IsValid()) { - prBlob.FormResponse = JObject.Parse(formData); + prBlob.FormResponse = JObject.FromObject(formData.GetValues()); result.SetBlob(prBlob); await _PaymentRequestRepository.CreateOrUpdatePaymentRequest(result); return RedirectToAction("PayPaymentRequest", new { payReqId }); } - - // GET or empty form data case: Redirect to form - return View("PostRedirect", new PostRedirectViewModel - { - AspController = "UIForms", - AspAction = "ViewPublicForm", - FormParameters = + break; + } + + return View("PostRedirect", new PostRedirectViewModel + { + AspController = "UIForms", + AspAction = "ViewPublicForm", + RouteParameters = + { + { "formId", prFormId } + }, + FormParameters = { - { "formId", prFormId }, { "redirectUrl", Request.GetCurrentUrl() } } - }); - } - - return RedirectToAction("ViewPaymentRequest", new { payReqId }); + }); } [HttpGet("{payReqId}/pay")] diff --git a/BTCPayServer/Forms/UIFormsController.cs b/BTCPayServer/Forms/UIFormsController.cs index a65304882..2d8f00bae 100644 --- a/BTCPayServer/Forms/UIFormsController.cs +++ b/BTCPayServer/Forms/UIFormsController.cs @@ -45,38 +45,36 @@ public class UIFormsController : Controller [AllowAnonymous] [HttpPost("~/forms/{formId}")] public IActionResult SubmitForm( - string formId, string? redirectUrl, + string formId, + string? redirectUrl, [FromServices] StoreRepository storeRepository, [FromServices] UIInvoiceController invoiceController) { var formData = GetFormData(formId); if (formData?.Config is null) - { return NotFound(); - } + var conf = Form.Parse(formData.Config); + conf.ApplyValuesFromForm(Request.Form); + if (!conf.Validate(ModelState)) + return View("View", new FormViewModel() { FormData = formData, RedirectUrl = redirectUrl }); - var dbForm = Form.Parse(formData.Config); - dbForm.ApplyValuesFromForm(Request.Form); - Dictionary data = dbForm.GetValues(); - + var form = new MultiValueDictionary(); + foreach (var kv in Request.Form) + form.Add(kv.Key, kv.Value); // With redirect, the form comes from another entity that we need to send the data back to if (!string.IsNullOrEmpty(redirectUrl)) { return View("PostRedirect", new PostRedirectViewModel { FormUrl = redirectUrl, - FormParameters = - { - { "formId", formId }, - { "formData", JsonConvert.SerializeObject(data) } - } + FormParameters = form }); } return NotFound(); } - private FormData? GetFormData(string id) + internal static FormData? GetFormData(string id) { FormData? form = id switch { diff --git a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs index 1bd461316..a17cf41a7 100644 --- a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs +++ b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using BTCPayServer.Abstractions.Constants; using BTCPayServer.Abstractions.Extensions; +using BTCPayServer.Abstractions.Form; using BTCPayServer.Abstractions.Models; using BTCPayServer.Client; using BTCPayServer.Controllers; @@ -118,8 +119,6 @@ namespace BTCPayServer.Plugins.PointOfSale.Controllers string notificationUrl, string redirectUrl, string choiceKey, - string formId = null, - string formData = null, string posData = null, RequiresRefundEmail requiresRefundEmail = RequiresRefundEmail.InheritFromStore, CancellationToken cancellationToken = default) @@ -230,9 +229,12 @@ namespace BTCPayServer.Plugins.PointOfSale.Controllers default: // POST case: Handle form submit - if (!string.IsNullOrEmpty(formData) && formId == posFormId) + var formData = Form.Parse(Forms.UIFormsController.GetFormData(posFormId).Config); + formData.ApplyValuesFromForm(this.Request.Form); + + if (formData.IsValid()) { - formResponse = JObject.Parse(formData); + formResponse = JObject.FromObject(formData.GetValues()); break; } @@ -247,9 +249,12 @@ namespace BTCPayServer.Plugins.PointOfSale.Controllers { AspController = "UIForms", AspAction = "ViewPublicForm", + RouteParameters = + { + { "formId", posFormId } + }, FormParameters = { - { "formId", posFormId }, { "redirectUrl", Request.GetCurrentUrl() + query } } });