App Service: Validate IDs when parsing items template (#6228)

Validates missing and duplicate IDs on the edit actions and when creating/updating apps via the API.
Fails gracefully by excluding existing items without ID or with duplicate ID for the rest of the cases.

Fixes #6227.
This commit is contained in:
d11n 2024-09-26 08:52:16 +02:00 committed by GitHub
parent 7013e618de
commit 443a350bad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 130 additions and 24 deletions

View File

@ -368,6 +368,27 @@ namespace BTCPayServer.Tests
}
)
);
var template = @"[
{
""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
""id"": ""green-tea"",
""image"": ""~/img/pos-sample/green-tea.jpg"",
""priceType"": ""Fixed"",
""price"": ""1"",
""title"": ""Green Tea"",
""disabled"": false
}
]";
await AssertValidationError(new[] { "Template" },
async () => await client.CreatePointOfSaleApp(
user.StoreId,
new PointOfSaleAppRequest
{
AppName = "good name",
Template = template.Replace(@"""id"": ""green-tea"",", "")
}
)
);
// Test creating a POS app successfully
var app = await client.CreatePointOfSaleApp(
@ -376,7 +397,8 @@ namespace BTCPayServer.Tests
{
AppName = "test app from API",
Currency = "JPY",
Title = "test app title"
Title = "test app title",
Template = template
}
);
Assert.Equal("test app from API", app.AppName);
@ -559,6 +581,27 @@ namespace BTCPayServer.Tests
}
)
);
var template = @"[
{
""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
""id"": ""green-tea"",
""image"": ""~/img/pos-sample/green-tea.jpg"",
""priceType"": ""Fixed"",
""price"": ""1"",
""title"": ""Green Tea"",
""disabled"": false
}
]";
await AssertValidationError(new[] { "PerksTemplate" },
async () => await client.CreateCrowdfundApp(
user.StoreId,
new CrowdfundAppRequest
{
AppName = "good name",
PerksTemplate = template.Replace(@"""id"": ""green-tea"",", "")
}
)
);
// Test creating a crowdfund app
var app = await client.CreateCrowdfundApp(
@ -566,7 +609,8 @@ namespace BTCPayServer.Tests
new CrowdfundAppRequest
{
AppName = "test app from API",
Title = "test app title"
Title = "test app title",
PerksTemplate = template
}
);
Assert.Equal("test app from API", app.AppName);

View File

@ -1,3 +1,4 @@
using System;
using System.Threading.Tasks;
using BTCPayServer.Client;
using BTCPayServer.Controllers;
@ -90,6 +91,54 @@ fruit tea:
Assert.Null( parsedDefault[4].AdditionalData);
Assert.Null( parsedDefault[4].PaymentMethods);
}
[Fact]
[Trait("Fast", "Fast")]
public void CanParseAppTemplate()
{
var template = @"[
{
""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
""id"": ""green-tea"",
""image"": ""~/img/pos-sample/green-tea.jpg"",
""priceType"": ""Fixed"",
""price"": ""1"",
""title"": ""Green Tea"",
""disabled"": false
},
{
""description"": ""Tian Jian Tian Jian means 'heavenly tippy tea' in Chinese, and it describes the finest grade of dark tea. Our Tian Jian dark tea is from Hunan province which is famous for making some of the best dark teas available."",
""id"": ""black-tea"",
""image"": ""~/img/pos-sample/black-tea.jpg"",
""priceType"": ""Fixed"",
""price"": ""1"",
""title"": ""Black Tea"",
""disabled"": false
}
]";
var items = AppService.Parse(template);
Assert.Equal(2, items.Length);
Assert.Equal("green-tea", items[0].Id);
Assert.Equal("black-tea", items[1].Id);
// Fails gracefully for missing ID
var missingId = template.Replace(@"""id"": ""green-tea"",", "");
items = AppService.Parse(missingId);
Assert.Single(items);
Assert.Equal("black-tea", items[0].Id);
// Throws for missing ID
Assert.Throws<ArgumentException>(() => AppService.Parse(missingId, true, true));
// Fails gracefully for duplicate IDs
var duplicateId = template.Replace(@"""id"": ""green-tea"",", @"""id"": ""black-tea"",");
items = AppService.Parse(duplicateId);
Assert.Empty(items);
// Throws for duplicate IDs
Assert.Throws<ArgumentException>(() => AppService.Parse(duplicateId, true, true));
}
[Fact(Timeout = LongRunningTestTimeout)]
[Trait("Integration", "Integration")]

View File

@ -1253,6 +1253,15 @@ namespace BTCPayServer.Tests
s.ClickPagePrimary();
Assert.Contains("App updated", s.FindAlertMessage().Text);
s.Driver.ScrollTo(By.Id("CodeTabButton"));
s.Driver.FindElement(By.Id("CodeTabButton")).Click();
template = s.Driver.FindElement(By.Id("TemplateConfig")).GetAttribute("value");
s.Driver.FindElement(By.Id("TemplateConfig")).Clear();
s.Driver.FindElement(By.Id("TemplateConfig")).SendKeys(template.Replace(@"""id"": ""green-tea"",", ""));
s.ClickPagePrimary();
Assert.Contains("Invalid template: Missing ID for item \"Green Tea\".", s.Driver.FindElement(By.CssSelector(".validation-summary-errors")).Text);
s.Driver.FindElement(By.Id("ViewApp")).Click();
var windows = s.Driver.WindowHandles;

View File

@ -402,11 +402,11 @@ namespace BTCPayServer.Controllers.Greenfield
try
{
// Just checking if we can serialize
AppService.SerializeTemplate(AppService.Parse(request.Template));
AppService.SerializeTemplate(AppService.Parse(request.Template, true, true));
}
catch
catch (Exception ex)
{
ModelState.AddModelError(nameof(request.Template), "Invalid template");
ModelState.AddModelError(nameof(request.Template), ex.Message);
}
}
}
@ -486,11 +486,11 @@ namespace BTCPayServer.Controllers.Greenfield
try
{
// Just checking if we can serialize
AppService.SerializeTemplate(AppService.Parse(request.PerksTemplate));
AppService.SerializeTemplate(AppService.Parse(request.PerksTemplate, true, true));
}
catch
catch (Exception ex)
{
ModelState.AddModelError(nameof(request.PerksTemplate), "Invalid template");
ModelState.AddModelError(nameof(request.PerksTemplate), $"Invalid template: {ex.Message}");
}
}

View File

@ -447,11 +447,11 @@ namespace BTCPayServer.Plugins.Crowdfund.Controllers
try
{
vm.PerksTemplate = AppService.SerializeTemplate(AppService.Parse(vm.PerksTemplate));
vm.PerksTemplate = AppService.SerializeTemplate(AppService.Parse(vm.PerksTemplate, true, true));
}
catch
catch (Exception ex)
{
ModelState.AddModelError(nameof(vm.PerksTemplate), "Invalid template");
ModelState.AddModelError(nameof(vm.PerksTemplate), $"Invalid template: {ex.Message}");
}
if (vm.TargetAmount is decimal v && v == 0.0m)
{

View File

@ -647,11 +647,11 @@ namespace BTCPayServer.Plugins.PointOfSale.Controllers
ModelState.AddModelError(nameof(vm.Currency), "Invalid currency");
try
{
vm.Template = AppService.SerializeTemplate(AppService.Parse(vm.Template));
vm.Template = AppService.SerializeTemplate(AppService.Parse(vm.Template, true, true));
}
catch
catch (Exception ex)
{
ModelState.AddModelError(nameof(vm.Template), "Invalid template");
ModelState.AddModelError(nameof(vm.Template), $"Invalid template: {ex.Message}");
}
if (!ModelState.IsValid)
{

View File

@ -348,12 +348,17 @@ namespace BTCPayServer.Services.Apps
{
return JsonConvert.SerializeObject(items, Formatting.Indented, _defaultSerializer);
}
public static ViewPointOfSaleViewModel.Item[] Parse(string template, bool includeDisabled = true)
public static ViewPointOfSaleViewModel.Item[] Parse(string template, bool includeDisabled = true, bool throws = false)
{
if (string.IsNullOrWhiteSpace(template))
return Array.Empty<ViewPointOfSaleViewModel.Item>();
return JsonConvert.DeserializeObject<ViewPointOfSaleViewModel.Item[]>(template, _defaultSerializer)!.Where(item => includeDisabled || !item.Disabled).ToArray();
if (string.IsNullOrWhiteSpace(template)) return [];
var allItems = JsonConvert.DeserializeObject<ViewPointOfSaleViewModel.Item[]>(template, _defaultSerializer)!;
// ensure all items have an id, which is also unique
var itemsWithoutId = allItems.Where(i => string.IsNullOrEmpty(i.Id)).ToList();
if (itemsWithoutId.Any() && throws) throw new ArgumentException($"Missing ID for item \"{itemsWithoutId.First().Title}\".");
// find items with duplicate IDs
var duplicateIds = allItems.GroupBy(i => i.Id).Where(g => g.Count() > 1).Select(g => g.Key).ToList();
if (duplicateIds.Any() && throws) throw new ArgumentException($"Duplicate ID \"{duplicateIds.First()}\".");
return allItems.Where(item => (includeDisabled || !item.Disabled) && !itemsWithoutId.Contains(item) && !duplicateIds.Contains(item.Id)).ToArray();
}
#nullable restore
#nullable enable

View File

@ -48,7 +48,7 @@
@if (!ViewContext.ModelState.IsValid)
{
<div asp-validation-summary="ModelOnly"></div>
<div asp-validation-summary="All" class="@(ViewContext.ModelState.ErrorCount.Equals(1) ? "no-marker" : "")"></div>
}
<div class="row">
<div class="col-sm-10 col-md-9 col-xl-7 col-xxl-6">

View File

@ -61,7 +61,7 @@
@if (!ViewContext.ModelState.IsValid)
{
<div asp-validation-summary="ModelOnly"></div>
<div asp-validation-summary="All" class="@(ViewContext.ModelState.ErrorCount.Equals(1) ? "no-marker" : "")"></div>
}
<div class="row">
<div class="col-sm-10 col-md-9 col-xl-7 col-xxl-6">

View File

@ -118,13 +118,12 @@
</template>
<div id="TemplateEditor" class="editor" v-cloak>
<h3 class="mt-5 mb-4" v-pre>@Model.title</h3>
<h3 class="mt-5 mb-3" v-pre>@Model.title</h3>
@if (ViewContext.ViewData.ModelState.TryGetValue(Model.templateId, out var errors))
{
foreach (var error in errors.Errors)
{
<br />
<span class="text-danger" v-pre>@error.ErrorMessage</span>
<p class="text-danger" v-pre>@error.ErrorMessage</p>
}
}
<div class="d-flex flex-wrap align-items-end justify-content-between gap-3 mb-3">