From df79c2cf487390c5342334141a502a51fe6d66c1 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Fri, 13 Nov 2020 16:28:15 +0900 Subject: [PATCH] Improve tests of webhooks --- BTCPayServer.Client/Models/InvoiceStatus.cs | 2 +- BTCPayServer.Client/Models/WebhookEvent.cs | 8 ++ .../Models/WebhookInvoiceEvent.cs | 76 +++++++++++++++++ BTCPayServer.Tests/ApiKeysTests.cs | 2 +- BTCPayServer.Tests/FakeServer.cs | 5 +- BTCPayServer.Tests/GreenfieldAPITests.cs | 35 +++++++- BTCPayServer.Tests/ServerTester.cs | 5 +- BTCPayServer.Tests/TestAccount.cs | 85 +++++++++++++++++++ BTCPayServer.Tests/UnitTest1.cs | 19 ++++- .../Controllers/InvoiceController.API.cs | 6 +- BTCPayServer/Events/InvoiceEvent.cs | 2 +- .../WebhookNotificationManager.cs | 45 ++++++---- .../swagger/v1/swagger.template.webhooks.json | 9 +- 13 files changed, 269 insertions(+), 30 deletions(-) diff --git a/BTCPayServer.Client/Models/InvoiceStatus.cs b/BTCPayServer.Client/Models/InvoiceStatus.cs index a3d866acd..b30e4e73a 100644 --- a/BTCPayServer.Client/Models/InvoiceStatus.cs +++ b/BTCPayServer.Client/Models/InvoiceStatus.cs @@ -9,4 +9,4 @@ namespace BTCPayServer.Client.Models Complete, Confirmed } -} \ No newline at end of file +} diff --git a/BTCPayServer.Client/Models/WebhookEvent.cs b/BTCPayServer.Client/Models/WebhookEvent.cs index 0a27f01e8..51eb76f4d 100644 --- a/BTCPayServer.Client/Models/WebhookEvent.cs +++ b/BTCPayServer.Client/Models/WebhookEvent.cs @@ -9,6 +9,14 @@ namespace BTCPayServer.Client.Models { public class WebhookEvent { + public readonly static JsonSerializerSettings DefaultSerializerSettings; + static WebhookEvent() + { + DefaultSerializerSettings = new JsonSerializerSettings(); + DefaultSerializerSettings.ContractResolver = new Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver(); + NBitcoin.JsonConverters.Serializer.RegisterFrontConverters(DefaultSerializerSettings); + DefaultSerializerSettings.Formatting = Formatting.None; + } public string DeliveryId { get; set; } public string OrignalDeliveryId { get; set; } [JsonConverter(typeof(StringEnumConverter))] diff --git a/BTCPayServer.Client/Models/WebhookInvoiceEvent.cs b/BTCPayServer.Client/Models/WebhookInvoiceEvent.cs index 8e6e09425..564933c86 100644 --- a/BTCPayServer.Client/Models/WebhookInvoiceEvent.cs +++ b/BTCPayServer.Client/Models/WebhookInvoiceEvent.cs @@ -8,9 +8,85 @@ namespace BTCPayServer.Client.Models { public class WebhookInvoiceEvent : WebhookEvent { + public WebhookInvoiceEvent() + { + + } + public WebhookInvoiceEvent(WebhookEventType evtType) + { + this.Type = evtType; + } [JsonProperty(Order = 1)] public string StoreId { get; set; } [JsonProperty(Order = 2)] public string InvoiceId { get; set; } + + public T ReadAs() + { + var str = JsonConvert.SerializeObject(this, DefaultSerializerSettings); + return JsonConvert.DeserializeObject(str, DefaultSerializerSettings); + } + } + + public class WebhookInvoiceConfirmedEvent : WebhookInvoiceEvent + { + public WebhookInvoiceConfirmedEvent() + { + + } + public WebhookInvoiceConfirmedEvent(WebhookEventType evtType) : base(evtType) + { + } + + public bool ManuallyMarked { get; set; } + } + public class WebhookInvoiceInvalidEvent : WebhookInvoiceEvent + { + public WebhookInvoiceInvalidEvent() + { + + } + public WebhookInvoiceInvalidEvent(WebhookEventType evtType) : base(evtType) + { + } + + public bool ManuallyMarked { get; set; } + } + public class WebhookInvoicePaidEvent : WebhookInvoiceEvent + { + public WebhookInvoicePaidEvent() + { + + } + public WebhookInvoicePaidEvent(WebhookEventType evtType) : base(evtType) + { + } + + public bool OverPaid { get; set; } + public bool PaidAfterExpiration { get; set; } + } + public class WebhookInvoiceReceivedPaymentEvent : WebhookInvoiceEvent + { + public WebhookInvoiceReceivedPaymentEvent() + { + + } + public WebhookInvoiceReceivedPaymentEvent(WebhookEventType evtType) : base(evtType) + { + } + + public bool AfterExpiration { get; set; } + } + public class WebhookInvoiceExpiredEvent : WebhookInvoiceEvent + { + public WebhookInvoiceExpiredEvent() + { + + } + public WebhookInvoiceExpiredEvent(WebhookEventType evtType) : base(evtType) + { + } + + public bool PartiallyPaid { get; set; } } } diff --git a/BTCPayServer.Tests/ApiKeysTests.cs b/BTCPayServer.Tests/ApiKeysTests.cs index a286f6243..99d5536c7 100644 --- a/BTCPayServer.Tests/ApiKeysTests.cs +++ b/BTCPayServer.Tests/ApiKeysTests.cs @@ -89,7 +89,7 @@ namespace BTCPayServer.Tests s.Driver.FindElement(By.Id("AddApiKey")).Click(); s.Driver.FindElement(By.CssSelector("button[value='btcpay.store.canmodifystoresettings:change-store-mode']")).Click(); //there should be a store already by default in the dropdown - var dropdown = s.Driver.FindElement(By.Name("PermissionValues[3].SpecificStores[0]")); + var dropdown = s.Driver.FindElement(By.Name("PermissionValues[4].SpecificStores[0]")); var option = dropdown.FindElement(By.TagName("option")); var storeId = option.GetAttribute("value"); option.Click(); diff --git a/BTCPayServer.Tests/FakeServer.cs b/BTCPayServer.Tests/FakeServer.cs index 44062c07d..a9692a2d7 100644 --- a/BTCPayServer.Tests/FakeServer.cs +++ b/BTCPayServer.Tests/FakeServer.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading; using System.Threading.Channels; using System.Threading.Tasks; +using ExchangeSharp; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server.Features; @@ -62,9 +63,9 @@ namespace BTCPayServer.Tests semaphore.Dispose(); } - public async Task GetNextRequest() + public async Task GetNextRequest(CancellationToken cancellationToken = default) { - return await _channel.Reader.ReadAsync(); + return await _channel.Reader.ReadAsync(cancellationToken); } } } diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 500762f2b..2a7ddd303 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -619,7 +619,7 @@ namespace BTCPayServer.Tests { Assert.True(hook.Enabled); Assert.True(hook.AuthorizedEvents.Everything); - Assert.True(hook.AutomaticRedelivery); + Assert.False(hook.AutomaticRedelivery); Assert.Equal(fakeServer.ServerUri.AbsoluteUri, hook.Url); } using var tester = ServerTester.Create(); @@ -913,6 +913,7 @@ namespace BTCPayServer.Tests var user = tester.NewAccount(); await user.GrantAccessAsync(); await user.MakeAdmin(); + await user.SetupWebhook(); var client = await user.CreateClient(Policies.Unrestricted); var viewOnly = await user.CreateClient(Policies.CanViewInvoices); @@ -970,6 +971,38 @@ namespace BTCPayServer.Tests await client.UnarchiveInvoice(user.StoreId, invoice.Id); Assert.NotNull(await client.GetInvoice(user.StoreId, invoice.Id)); + + foreach (var marked in new[] { InvoiceStatus.Complete, InvoiceStatus.Invalid }) + { + var inv = await client.CreateInvoice(user.StoreId, + new CreateInvoiceRequest() { Currency = "USD", Amount = 100 }); + await user.PayInvoice(inv.Id); + await client.MarkInvoiceStatus(user.StoreId, inv.Id, new MarkInvoiceStatusRequest() + { + Status = marked + }); + var result = await client.GetInvoice(user.StoreId, inv.Id); + if (marked == InvoiceStatus.Complete) + { + Assert.Equal(InvoiceStatus.Complete, result.Status); + user.AssertHasWebhookEvent(WebhookEventType.InvoiceConfirmed, + o => + { + Assert.Equal(inv.Id, o.InvoiceId); + Assert.True(o.ManuallyMarked); + }); + } + if (marked == InvoiceStatus.Invalid) + { + Assert.Equal(InvoiceStatus.Invalid, result.Status); + user.AssertHasWebhookEvent(WebhookEventType.InvoiceInvalid, + o => + { + Assert.Equal(inv.Id, o.InvoiceId); + Assert.True(o.ManuallyMarked); + }); + } + } } } diff --git a/BTCPayServer.Tests/ServerTester.cs b/BTCPayServer.Tests/ServerTester.cs index 541c1a4e2..3e72f9ba5 100644 --- a/BTCPayServer.Tests/ServerTester.cs +++ b/BTCPayServer.Tests/ServerTester.cs @@ -23,6 +23,7 @@ namespace BTCPayServer.Tests return new ServerTester(scope, newDb); } + public List Resources = new List(); readonly string _Directory; public ServerTester(string scope, bool newDb) { @@ -145,7 +146,7 @@ namespace BTCPayServer.Tests var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var sub = PayTester.GetService().Subscribe(evt => { - if(correctEvent is null) + if (correctEvent is null) tcs.TrySetResult(evt); else if (correctEvent(evt)) { @@ -207,6 +208,8 @@ namespace BTCPayServer.Tests public void Dispose() { + foreach (var r in this.Resources) + r.Dispose(); Logs.Tester.LogInformation("Disposing the BTCPayTester..."); foreach (var store in Stores) { diff --git a/BTCPayServer.Tests/TestAccount.cs b/BTCPayServer.Tests/TestAccount.cs index 7b3c1d9d8..af00dd0a6 100644 --- a/BTCPayServer.Tests/TestAccount.cs +++ b/BTCPayServer.Tests/TestAccount.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Net.Http; using System.Text; +using System.Threading; using System.Threading.Tasks; using BTCPayServer.Client; using BTCPayServer.Client.Models; @@ -23,8 +25,10 @@ using NBitcoin.Payment; using NBitpayClient; using NBXplorer.DerivationStrategy; using NBXplorer.Models; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Xunit; +using Xunit.Sdk; namespace BTCPayServer.Tests { @@ -427,5 +431,86 @@ namespace BTCPayServer.Tests return null; return parsedBip21; } + + class WebhookListener : IDisposable + { + private Client.Models.StoreWebhookData _wh; + private FakeServer _server; + private readonly List _webhookEvents; + private CancellationTokenSource _cts; + public WebhookListener(Client.Models.StoreWebhookData wh, FakeServer server, List webhookEvents) + { + _wh = wh; + _server = server; + _webhookEvents = webhookEvents; + _cts = new CancellationTokenSource(); + _ = Listen(_cts.Token); + } + + async Task Listen(CancellationToken cancellation) + { + while (!cancellation.IsCancellationRequested) + { + var req = await _server.GetNextRequest(cancellation); + var bytes = await req.Request.Body.ReadBytesAsync((int)req.Request.Headers.ContentLength); + var callback = Encoding.UTF8.GetString(bytes); + _webhookEvents.Add(JsonConvert.DeserializeObject(callback)); + req.Response.StatusCode = 200; + _server.Done(); + } + } + public void Dispose() + { + _cts.Cancel(); + _server.Dispose(); + } + } + + public List WebhookEvents { get; set; } = new List(); + public TEvent AssertHasWebhookEvent(WebhookEventType eventType, Action assert) where TEvent : class + { + foreach (var evt in WebhookEvents) + { + if (evt.Type == eventType) + { + var typedEvt = evt.ReadAs(); + try + { + assert(typedEvt); + return typedEvt; + } + catch (XunitException ex) + { + } + } + } + Assert.True(false, "No webhook event match the assertion"); + return null; + } + public async Task SetupWebhook() + { + FakeServer server = new FakeServer(); + await server.Start(); + var client = await CreateClient(Policies.CanModifyStoreWebhooks); + var wh = await client.CreateWebhook(StoreId, new CreateStoreWebhookRequest() + { + AutomaticRedelivery = false, + Url = server.ServerUri.AbsoluteUri + }); + + parent.Resources.Add(new WebhookListener(wh, server, WebhookEvents)); + } + + public async Task PayInvoice(string invoiceId) + { + var inv = await BitPay.GetInvoiceAsync(invoiceId); + var net = parent.ExplorerNode.Network; + this.parent.ExplorerNode.SendToAddress(BitcoinAddress.Create(inv.BitcoinAddress, net), inv.BtcDue); + await TestUtils.EventuallyAsync(async () => + { + var localInvoice = await BitPay.GetInvoiceAsync(invoiceId, Facade.Merchant); + Assert.Equal("paid", localInvoice.Status); + }); + } } } diff --git a/BTCPayServer.Tests/UnitTest1.cs b/BTCPayServer.Tests/UnitTest1.cs index fe51e7325..739e943d1 100644 --- a/BTCPayServer.Tests/UnitTest1.cs +++ b/BTCPayServer.Tests/UnitTest1.cs @@ -2526,6 +2526,7 @@ namespace BTCPayServer.Tests var user = tester.NewAccount(); user.GrantAccess(); user.RegisterDerivationScheme("BTC"); + await user.SetupWebhook(); var invoice = user.BitPay.CreateInvoice( new Invoice() { @@ -2582,7 +2583,6 @@ namespace BTCPayServer.Tests var cashCow = tester.ExplorerNode; var invoiceAddress = BitcoinAddress.Create(invoice.BitcoinAddress, cashCow.Network); - var iii = ctx.AddressInvoices.ToArray(); Assert.True(IsMapped(invoice, ctx)); cashCow.SendToAddress(invoiceAddress, firstPayment); @@ -2686,6 +2686,23 @@ namespace BTCPayServer.Tests Assert.Equal(Money.Zero, localInvoice.BtcDue); Assert.Equal("paidOver", (string)((JValue)localInvoice.ExceptionStatus).Value); }); + + // Test on the webhooks + user.AssertHasWebhookEvent(WebhookEventType.InvoiceConfirmed, + c => + { + Assert.False(c.ManuallyMarked); + }); + user.AssertHasWebhookEvent(WebhookEventType.InvoicePaidInFull, + c => + { + Assert.True(c.OverPaid); + }); + user.AssertHasWebhookEvent(WebhookEventType.InvoiceReceivedPayment, + c => + { + Assert.False(c.AfterExpiration); + }); } } diff --git a/BTCPayServer/Controllers/InvoiceController.API.cs b/BTCPayServer/Controllers/InvoiceController.API.cs index fca1ded2b..ea8f8e972 100644 --- a/BTCPayServer/Controllers/InvoiceController.API.cs +++ b/BTCPayServer/Controllers/InvoiceController.API.cs @@ -51,7 +51,7 @@ namespace BTCPayServer.Controllers } [HttpGet] [Route("invoices")] - public async Task> GetInvoices( + public async Task GetInvoices( string token, DateTimeOffset? dateStart = null, DateTimeOffset? dateEnd = null, @@ -61,6 +61,8 @@ namespace BTCPayServer.Controllers int? limit = null, int? offset = null) { + if (User.Identity.AuthenticationType == Security.Bitpay.BitpayAuthenticationTypes.Anonymous) + return Forbid(Security.Bitpay.BitpayAuthenticationTypes.Anonymous); if (dateEnd != null) dateEnd = dateEnd.Value + TimeSpan.FromDays(1); //Should include the end day @@ -79,7 +81,7 @@ namespace BTCPayServer.Controllers var entities = (await _InvoiceRepository.GetInvoices(query)) .Select((o) => o.EntityToDTO()).ToArray(); - return DataWrapper.Create(entities); + return Json(DataWrapper.Create(entities)); } } } diff --git a/BTCPayServer/Events/InvoiceEvent.cs b/BTCPayServer/Events/InvoiceEvent.cs index f6839b9f8..783f2e0be 100644 --- a/BTCPayServer/Events/InvoiceEvent.cs +++ b/BTCPayServer/Events/InvoiceEvent.cs @@ -38,7 +38,7 @@ namespace BTCPayServer.Events {ReceivedPayment, InvoiceEventCode.ReceivedPayment}, {PaidInFull, InvoiceEventCode.PaidInFull}, {Expired, InvoiceEventCode.Expired}, - {Confirmed, InvoiceEventCode.Completed}, + {Confirmed, InvoiceEventCode.Confirmed}, {Completed, InvoiceEventCode.Completed}, {MarkedInvalid, InvoiceEventCode.MarkedInvalid}, {FailedToConfirm, InvoiceEventCode.FailedToConfirm}, diff --git a/BTCPayServer/HostedServices/WebhookNotificationManager.cs b/BTCPayServer/HostedServices/WebhookNotificationManager.cs index 0e576d444..758425907 100644 --- a/BTCPayServer/HostedServices/WebhookNotificationManager.cs +++ b/BTCPayServer/HostedServices/WebhookNotificationManager.cs @@ -43,10 +43,7 @@ namespace BTCPayServer.HostedServices public readonly static JsonSerializerSettings DefaultSerializerSettings; static WebhookNotificationManager() { - DefaultSerializerSettings = new JsonSerializerSettings(); - DefaultSerializerSettings.ContractResolver = new CamelCasePropertyNamesContractResolver(); - NBitcoin.JsonConverters.Serializer.RegisterFrontConverters(DefaultSerializerSettings); - DefaultSerializerSettings.Formatting = Formatting.None; + DefaultSerializerSettings = WebhookEvent.DefaultSerializerSettings; } public const string OnionNamedClient = "greenfield-webhook.onion"; public const string ClearnetNamedClient = "greenfield-webhook.clearnet"; @@ -113,7 +110,6 @@ namespace BTCPayServer.HostedServices var webhookEvent = newDeliveryBlob.ReadRequestAs(); webhookEvent.DeliveryId = newDelivery.Id; webhookEvent.OrignalDeliveryId ??= deliveryId; - webhookEvent.Timestamp = newDelivery.Timestamp; newDeliveryBlob.Request = ToBytes(webhookEvent); newDelivery.SetBlob(newDeliveryBlob); return new WebhookDeliveryRequest(webhookDelivery.Webhook.Id, webhookEvent, newDelivery, webhookDelivery.Webhook.GetBlob()); @@ -126,16 +122,14 @@ namespace BTCPayServer.HostedServices foreach (var webhook in webhooks) { var webhookBlob = webhook.GetBlob(); - if (!(GetWebhookEvent(invoiceEvent.EventCode) is WebhookEventType webhookEventType)) + if (!(GetWebhookEvent(invoiceEvent) is WebhookInvoiceEvent webhookEvent)) continue; - if (!ShouldDeliver(webhookEventType, webhookBlob)) + if (!ShouldDeliver(webhookEvent.Type, webhookBlob)) continue; Data.WebhookDeliveryData delivery = NewDelivery(); delivery.WebhookId = webhook.Id; - var webhookEvent = new WebhookInvoiceEvent(); webhookEvent.InvoiceId = invoiceEvent.InvoiceId; webhookEvent.StoreId = invoiceEvent.Invoice.StoreId; - webhookEvent.Type = webhookEventType; webhookEvent.DeliveryId = delivery.Id; webhookEvent.OrignalDeliveryId = delivery.Id; webhookEvent.Timestamp = delivery.Timestamp; @@ -158,28 +152,45 @@ namespace BTCPayServer.HostedServices _ = Process(context.WebhookId, channel); } - private WebhookEventType? GetWebhookEvent(InvoiceEventCode eventCode) + private WebhookInvoiceEvent GetWebhookEvent(InvoiceEvent invoiceEvent) { + var eventCode = invoiceEvent.EventCode; switch (eventCode) { case InvoiceEventCode.Completed: return null; case InvoiceEventCode.Confirmed: case InvoiceEventCode.MarkedCompleted: - return WebhookEventType.InvoiceConfirmed; + return new WebhookInvoiceConfirmedEvent(WebhookEventType.InvoiceConfirmed) + { + ManuallyMarked = eventCode == InvoiceEventCode.MarkedCompleted + }; case InvoiceEventCode.Created: - return WebhookEventType.InvoiceCreated; + return new WebhookInvoiceEvent(WebhookEventType.InvoiceCreated); case InvoiceEventCode.Expired: case InvoiceEventCode.ExpiredPaidPartial: - return WebhookEventType.InvoiceExpired; + return new WebhookInvoiceExpiredEvent(WebhookEventType.InvoiceExpired) + { + PartiallyPaid = eventCode == InvoiceEventCode.ExpiredPaidPartial + }; case InvoiceEventCode.FailedToConfirm: case InvoiceEventCode.MarkedInvalid: - return WebhookEventType.InvoiceInvalid; + return new WebhookInvoiceInvalidEvent(WebhookEventType.InvoiceInvalid) + { + ManuallyMarked = eventCode == InvoiceEventCode.MarkedInvalid + }; case InvoiceEventCode.PaidInFull: - return WebhookEventType.InvoicePaidInFull; - case InvoiceEventCode.ReceivedPayment: case InvoiceEventCode.PaidAfterExpiration: - return WebhookEventType.InvoiceReceivedPayment; + return new WebhookInvoicePaidEvent(WebhookEventType.InvoicePaidInFull) + { + OverPaid = invoiceEvent.Invoice.ExceptionStatus == InvoiceExceptionStatus.PaidOver, + PaidAfterExpiration = eventCode == InvoiceEventCode.PaidAfterExpiration + }; + case InvoiceEventCode.ReceivedPayment: + return new WebhookInvoiceReceivedPaymentEvent(WebhookEventType.InvoiceReceivedPayment) + { + AfterExpiration = invoiceEvent.Invoice.Status == InvoiceStatus.Expired || invoiceEvent.Invoice.Status == InvoiceStatus.Invalid + }; default: return null; } diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.webhooks.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.webhooks.json index f3d255bfb..3fa03e61e 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.webhooks.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.webhooks.json @@ -553,12 +553,14 @@ "enabled": { "type": "boolean", "description": "Whether this webhook is enabled or not", - "nullable": false + "nullable": false, + "default": true }, "automaticRedelivery": { "type": "boolean", "description": "If true, BTCPay Server will retry to redeliver any failed delivery after 10 seconds, 1 minutes and up to 6 times after 10 minutes.", - "nullable": false + "nullable": false, + "default": true }, "url": { "type": "string", @@ -572,7 +574,8 @@ "everything": { "type": "string", "description": "If true, the endpoint will receive all events related to the store.", - "nullable": false + "nullable": false, + "default": true }, "specificEvents": { "type": "string",