Greenfield: Improve error message, do not use internal lightning node on store's lightning API

This commit is contained in:
nicolas.dorier 2021-12-16 12:32:13 +09:00
parent bbddd72780
commit 4f7eeea14e
No known key found for this signature in database
GPG Key ID: 6618763EF09186FE
12 changed files with 149 additions and 64 deletions

View File

@ -54,12 +54,12 @@ namespace BTCPayServer.Client
;
throw new GreenFieldValidationException(err);
}
else if (message.StatusCode == System.Net.HttpStatusCode.BadRequest)
else if (!message.IsSuccessStatusCode && message.Content?.Headers?.ContentType?.MediaType?.StartsWith("application/json", StringComparison.OrdinalIgnoreCase) is true)
{
var err = JsonConvert.DeserializeObject<Models.GreenfieldAPIError>(await message.Content.ReadAsStringAsync());
throw new GreenFieldAPIException(err);
if (err.Code != null)
throw new GreenFieldAPIException((int)message.StatusCode, err);
}
message.EnsureSuccessStatusCode();
}

View File

@ -4,12 +4,14 @@ namespace BTCPayServer.Client
{
public class GreenFieldAPIException : Exception
{
public GreenFieldAPIException(Models.GreenfieldAPIError error) : base(error.Message)
public GreenFieldAPIException(int httpCode, Models.GreenfieldAPIError error) : base(error.Message)
{
if (error == null)
throw new ArgumentNullException(nameof(error));
HttpCode = httpCode;
APIError = error;
}
public Models.GreenfieldAPIError APIError { get; }
public int HttpCode { get; set; }
}
}

View File

@ -26,6 +26,7 @@ using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;
using CreateApplicationUserRequest = BTCPayServer.Client.Models.CreateApplicationUserRequest;
using JsonReader = Newtonsoft.Json.JsonReader;
@ -661,8 +662,15 @@ namespace BTCPayServer.Tests
private async Task AssertHttpError(int code, Func<Task> act)
{
var ex = await Assert.ThrowsAsync<HttpRequestException>(act);
Assert.Contains(code.ToString(), ex.Message);
try
{
// Eventually all exception should be GreenFieldAPIException
var ex = await Assert.ThrowsAsync<HttpRequestException>(act);
Assert.Contains(code.ToString(), ex.Message);
}
catch (ThrowsException e) when (e.InnerException is GreenFieldAPIException ex && ex.HttpCode == code)
{
}
}
[Fact(Timeout = TestTimeout)]
@ -1426,11 +1434,11 @@ namespace BTCPayServer.Tests
Assert.Single(info.NodeURIs);
Assert.NotEqual(0, info.BlockHeight);
var err = await Assert.ThrowsAsync<HttpRequestException>(async () => await client.GetLightningNodeChannels("BTC"));
Assert.Contains("503", err.Message);
var err = await Assert.ThrowsAsync<GreenFieldAPIException>(async () => await client.GetLightningNodeChannels("BTC"));
Assert.Equal(503, err.HttpCode);
// Not permission for the store!
err = await Assert.ThrowsAsync<HttpRequestException>(async () => await client.GetLightningNodeChannels(user.StoreId, "BTC"));
Assert.Contains("403", err.Message);
var err2 = await Assert.ThrowsAsync<HttpRequestException>(async () => await client.GetLightningNodeChannels(user.StoreId, "BTC"));
Assert.Contains("403", err2.Message);
var invoiceData = await client.CreateLightningInvoice("BTC", new CreateLightningInvoiceRequest()
{
Amount = LightMoney.Satoshis(1000),
@ -1443,8 +1451,8 @@ namespace BTCPayServer.Tests
client = await user.CreateClient($"{Policies.CanUseLightningNodeInStore}:{user.StoreId}");
// Not permission for the server
err = await Assert.ThrowsAsync<HttpRequestException>(async () => await client.GetLightningNodeChannels("BTC"));
Assert.Contains("403", err.Message);
err2 = await Assert.ThrowsAsync<HttpRequestException>(async () => await client.GetLightningNodeChannels("BTC"));
Assert.Contains("403", err2.Message);
var data = await client.GetLightningNodeChannels(user.StoreId, "BTC");
Assert.Equal(2, data.Count());
@ -1486,6 +1494,15 @@ namespace BTCPayServer.Tests
info = await client.GetLightningNodeInfo(user.StoreId, "BTC");
Assert.Single(info.NodeURIs);
Assert.NotEqual(0, info.BlockHeight);
// As admin, can use the internal node through our store.
await user.MakeAdmin(true);
await user.RegisterInternalLightningNodeAsync("BTC");
await client.GetLightningNodeInfo(user.StoreId, "BTC");
// But if not admin anymore, nope
await user.MakeAdmin(false);
await AssertAPIError("insufficient-api-permissions", () => client.GetLightningNodeInfo(user.StoreId, "BTC"));
}
}

View File

@ -283,6 +283,16 @@ namespace BTCPayServer.Tests
Assert.False(true, storeController.ModelState.FirstOrDefault().Value.Errors[0].ErrorMessage);
}
public async Task RegisterInternalLightningNodeAsync(string cryptoCode, string storeId = null)
{
var storeController = GetController<StoresController>();
var vm = new LightningNodeViewModel { ConnectionString = "", LightningNodeType = LightningNodeType.Internal, SkipPortTest = true };
await storeController.SetupLightningNode(storeId ?? StoreId,
vm, "save", cryptoCode);
if (storeController.ModelState.ErrorCount != 0)
Assert.False(true, storeController.ModelState.FirstOrDefault().Value.Errors[0].ErrorMessage);
}
public async Task<Coin> ReceiveUTXO(Money value, BTCPayNetwork network)
{
var cashCow = parent.ExplorerNode;

View File

@ -104,12 +104,16 @@ namespace BTCPayServer.Controllers.GreenField
protected override async Task<ILightningClient> GetLightningClient(string cryptoCode, bool doingAdminThings)
{
var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
if (network == null ||
!_lightningNetworkOptions.Value.InternalLightningByCryptoCode.TryGetValue(network.CryptoCode,
out var internalLightningNode) ||
!await CanUseInternalLightning(doingAdminThings))
if (network is null)
throw ErrorCryptoCodeNotFound();
if (!_lightningNetworkOptions.Value.InternalLightningByCryptoCode.TryGetValue(network.CryptoCode,
out var internalLightningNode))
{
return null;
throw ErrorInternalLightningNodeNotConfigured();
}
if (!await CanUseInternalLightning(doingAdminThings))
{
throw ErrorShouldBeAdminForInternalNode();
}
return _lightningClientFactory.Create(internalLightningNode, network);
}

View File

@ -102,16 +102,15 @@ namespace BTCPayServer.Controllers.GreenField
return base.CreateInvoice(cryptoCode, request);
}
protected override async Task<ILightningClient> GetLightningClient(string cryptoCode,
protected override Task<ILightningClient> GetLightningClient(string cryptoCode,
bool doingAdminThings)
{
var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
var store = HttpContext.GetStoreData();
if (network == null || store == null)
if (store == null || network == null)
{
return null;
throw ErrorCryptoCodeNotFound();
}
var id = new PaymentMethodId(cryptoCode, PaymentTypes.LightningLike);
@ -119,19 +118,22 @@ namespace BTCPayServer.Controllers.GreenField
.OfType<LightningSupportedPaymentMethod>()
.FirstOrDefault(d => d.PaymentId == id);
if (existing == null)
return null;
throw ErrorLightningNodeNotConfiguredForStore();
if (existing.GetExternalLightningUrl() is LightningConnectionString connectionString)
{
return _lightningClientFactory.Create(connectionString, network);
return Task.FromResult(_lightningClientFactory.Create(connectionString, network));
}
else if (
await CanUseInternalLightning(doingAdminThings) &&
_lightningNetworkOptions.Value.InternalLightningByCryptoCode.TryGetValue(network.CryptoCode,
out var internalLightningNode))
else if (existing.IsInternalNode &&
_lightningNetworkOptions.Value.InternalLightningByCryptoCode.TryGetValue(network.CryptoCode,
out var internalLightningNode))
{
return _lightningClientFactory.Create(internalLightningNode, network);
if (!User.IsInRole(Roles.ServerAdmin))
{
throw ErrorShouldBeAdminForInternalNode();
}
return Task.FromResult(_lightningClientFactory.Create(internalLightningNode, network));
}
return null;
throw ErrorLightningNodeNotConfiguredForStore();
}
}
}

View File

@ -20,15 +20,8 @@ namespace BTCPayServer.Controllers.GreenField
{
public void OnException(ExceptionContext context)
{
if (context.Exception is NBitcoin.JsonConverters.JsonObjectException jsonObject)
{
context.Result = new ObjectResult(new GreenfieldValidationError(jsonObject.Path, jsonObject.Message));
}
else
{
context.Result = new StatusCodeResult(503);
}
context.ExceptionHandled = true;
context.Result = new ObjectResult(new GreenfieldAPIError("ligthning-node-unavailable", $"The lightning node is unavailable ({context.Exception.GetType().Name}: {context.Exception.Message})")) { StatusCode = 503 };
// Do not mark handled, it is possible filters above have better errors
}
}
public abstract class LightningNodeApiController : Controller
@ -49,10 +42,6 @@ namespace BTCPayServer.Controllers.GreenField
public virtual async Task<IActionResult> GetInfo(string cryptoCode)
{
var lightningClient = await GetLightningClient(cryptoCode, true);
if (lightningClient == null)
{
return NotFound();
}
var info = await lightningClient.GetInfo();
return Ok(new LightningNodeInformationData()
{
@ -64,11 +53,6 @@ namespace BTCPayServer.Controllers.GreenField
public virtual async Task<IActionResult> ConnectToNode(string cryptoCode, ConnectToNodeRequest request)
{
var lightningClient = await GetLightningClient(cryptoCode, true);
if (lightningClient == null)
{
return NotFound();
}
if (request?.NodeURI is null)
{
ModelState.AddModelError(nameof(request.NodeURI), "A valid node info was not provided to connect to");
@ -94,10 +78,6 @@ namespace BTCPayServer.Controllers.GreenField
public virtual async Task<IActionResult> GetChannels(string cryptoCode)
{
var lightningClient = await GetLightningClient(cryptoCode, true);
if (lightningClient == null)
{
return NotFound();
}
var channels = await lightningClient.ListChannels();
return Ok(channels.Select(channel => new LightningChannelData()
@ -198,10 +178,6 @@ namespace BTCPayServer.Controllers.GreenField
{
var lightningClient = await GetLightningClient(cryptoCode, true);
var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
if (lightningClient == null || network == null)
{
return NotFound();
}
if (lightningInvoice?.BOLT11 is null ||
!BOLT11PaymentRequest.TryParse(lightningInvoice.BOLT11, out _, network.NBitcoinNetwork))
@ -248,12 +224,6 @@ namespace BTCPayServer.Controllers.GreenField
public virtual async Task<IActionResult> CreateInvoice(string cryptoCode, CreateLightningInvoiceRequest request)
{
var lightningClient = await GetLightningClient(cryptoCode, false);
if (lightningClient == null)
{
return NotFound();
}
if (request.Amount < LightMoney.Zero)
{
ModelState.AddModelError(nameof(request.Amount), "Amount should be more or equals to 0");
@ -285,6 +255,23 @@ namespace BTCPayServer.Controllers.GreenField
}
}
protected JsonHttpException ErrorLightningNodeNotConfiguredForStore()
{
return new JsonHttpException(this.CreateAPIError(404, "lightning-not-configured", "The lightning node is not set up"));
}
protected JsonHttpException ErrorInternalLightningNodeNotConfigured()
{
return new JsonHttpException(this.CreateAPIError(404, "lightning-not-configured", "The internal lightning node is not set up"));
}
protected JsonHttpException ErrorCryptoCodeNotFound()
{
return new JsonHttpException(this.CreateAPIError(404, "unknown-cryptocode", "This crypto code isn't set up in this BTCPay Server instance"));
}
protected JsonHttpException ErrorShouldBeAdminForInternalNode()
{
return new JsonHttpException(this.CreateAPIError(403, "insufficient-api-permissions", "The user should be admin to use the internal lightning node"));
}
private LightningInvoiceData ToModel(LightningInvoice invoice)
{
return new LightningInvoiceData()

View File

@ -486,9 +486,9 @@ namespace BTCPayServer.Controllers.GreenField
case UnprocessableEntityObjectResult {Value: List<GreenfieldValidationError> validationErrors}:
throw new GreenFieldValidationException(validationErrors.ToArray());
case BadRequestObjectResult {Value: GreenfieldAPIError error}:
throw new GreenFieldAPIException(error);
throw new GreenFieldAPIException(400, error);
case NotFoundResult _:
throw new GreenFieldAPIException(new GreenfieldAPIError("not-found", ""));
throw new GreenFieldAPIException(404, new GreenfieldAPIError("not-found", ""));
default:
return;
}

View File

@ -0,0 +1,21 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Mvc.Infrastructure;
namespace BTCPayServer.Filters
{
public class JsonHttpExceptionFilter : IExceptionFilter
{
public void OnException(ExceptionContext context)
{
if (context.Exception is JsonHttpException ex)
{
context.Result = ex.ActionResult;
context.ExceptionHandled = true;
}
}
}
}

View File

@ -0,0 +1,22 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using BTCPayServer.Client.Models;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Filters;
namespace BTCPayServer.Filters
{
public class JsonObjectExceptionFilter : IExceptionFilter
{
public void OnException(ExceptionContext context)
{
if (context.Exception is NBitcoin.JsonConverters.JsonObjectException jsonObject)
{
context.Result = new ObjectResult(new GreenfieldValidationError(jsonObject.Path, jsonObject.Message)) { StatusCode = 400 };
context.ExceptionHandled = true;
}
}
}
}

View File

@ -118,7 +118,8 @@ namespace BTCPayServer.Hosting
o.ModelBinderProviders.Insert(0, new ModelBinders.DefaultModelBinderProvider());
if (!Configuration.GetOrDefault<bool>("nocsp", false))
o.Filters.Add(new ContentSecurityPolicyAttribute(CSPTemplate.AntiXSS));
})
o.Filters.Add(new JsonHttpExceptionFilter());
})
.ConfigureApiBehaviorOptions(options =>
{
options.InvalidModelStateResponseFactory = context =>

View File

@ -0,0 +1,19 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
namespace BTCPayServer
{
public class JsonHttpException : Exception
{
public JsonHttpException(IActionResult actionResult)
{
ActionResult = actionResult;
}
public IActionResult ActionResult { get; }
}
}