From 5f5f71bf3721a86e4f3f50c313a523ee863fce00 Mon Sep 17 00:00:00 2001 From: Andrew Camilleri Date: Thu, 23 Dec 2021 05:32:08 +0100 Subject: [PATCH] Normalize greenfield responses for 404s (#3220) --- .../GreenField/ApiKeysController.cs | 8 +++--- .../GreenField/LightningNodeApiController.cs | 23 +--------------- .../GreenField/LocalBTCPayServerClient.cs | 6 ++--- .../GreenField/NotificationsController.cs | 8 ++++-- .../GreenField/PaymentRequestsController.cs | 15 +++++++---- .../GreenField/PullPaymentController.cs | 2 -- .../StoreLNURLPayPaymentMethodsController.cs | 27 +++++++------------ ...ymentMethodsController.WalletGeneration.cs | 7 ++--- .../StoreOnChainWalletsController.cs | 8 +++--- .../GreenField/StoresController.cs | 15 +++++++---- 10 files changed, 49 insertions(+), 70 deletions(-) diff --git a/BTCPayServer/Controllers/GreenField/ApiKeysController.cs b/BTCPayServer/Controllers/GreenField/ApiKeysController.cs index b7db9759b..3a1c770a0 100644 --- a/BTCPayServer/Controllers/GreenField/ApiKeysController.cs +++ b/BTCPayServer/Controllers/GreenField/ApiKeysController.cs @@ -30,11 +30,12 @@ namespace BTCPayServer.Controllers.GreenField } [HttpGet("~/api/v1/api-keys/current")] - public async Task> GetKey() + public async Task GetKey() { if (!ControllerContext.HttpContext.GetAPIKey(out var apiKey)) { - return NotFound(); + return + this.CreateAPIError(404, "api-key-not-found", "The api key was not present."); } var data = await _apiKeyRepository.GetKey(apiKey); return Ok(FromModel(data)); @@ -44,8 +45,7 @@ namespace BTCPayServer.Controllers.GreenField [Authorize(Policy = Policies.Unrestricted, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] public async Task CreateKey(CreateApiKeyRequest request) { - if (request is null) - return NotFound(); + request ??= new CreateApiKeyRequest(); request.Permissions ??= System.Array.Empty(); var key = new APIKeyData() { diff --git a/BTCPayServer/Controllers/GreenField/LightningNodeApiController.cs b/BTCPayServer/Controllers/GreenField/LightningNodeApiController.cs index 325f13886..d6c52b3ad 100644 --- a/BTCPayServer/Controllers/GreenField/LightningNodeApiController.cs +++ b/BTCPayServer/Controllers/GreenField/LightningNodeApiController.cs @@ -29,7 +29,6 @@ namespace BTCPayServer.Controllers.GreenField private readonly BTCPayNetworkProvider _btcPayNetworkProvider; private readonly ISettingsRepository _settingsRepository; private readonly IAuthorizationService _authorizationService; - protected LightningNodeApiController(BTCPayNetworkProvider btcPayNetworkProvider, ISettingsRepository settingsRepository, IAuthorizationService authorizationService) @@ -95,11 +94,6 @@ namespace BTCPayServer.Controllers.GreenField public virtual async Task OpenChannel(string cryptoCode, OpenLightningChannelRequest request) { var lightningClient = await GetLightningClient(cryptoCode, true); - if (lightningClient == null) - { - return NotFound(); - } - if (request?.NodeURI is null) { ModelState.AddModelError(nameof(request.NodeURI), @@ -166,11 +160,6 @@ namespace BTCPayServer.Controllers.GreenField public virtual async Task GetDepositAddress(string cryptoCode) { var lightningClient = await GetLightningClient(cryptoCode, true); - if (lightningClient == null) - { - return NotFound(); - } - return Ok(new JValue((await lightningClient.GetDepositAddress()).ToString())); } @@ -207,18 +196,8 @@ namespace BTCPayServer.Controllers.GreenField public virtual async Task GetInvoice(string cryptoCode, string id) { var lightningClient = await GetLightningClient(cryptoCode, false); - - if (lightningClient == null) - { - return NotFound(); - } - var inv = await lightningClient.GetInvoice(id); - if (inv == null) - { - return this.CreateAPIError(404, "invoice-not-found", "Impossible to find a lightning invoice with this id"); - } - return Ok(ToModel(inv)); + return inv == null ? this.CreateAPIError(404, "invoice-not-found", "Impossible to find a lightning invoice with this id") : Ok(ToModel(inv)); } public virtual async Task CreateInvoice(string cryptoCode, CreateLightningInvoiceRequest request) diff --git a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs index 53b0a5075..3c7cd6c96 100644 --- a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs +++ b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs @@ -568,7 +568,7 @@ namespace BTCPayServer.Controllers.GreenField public override async Task GetPaymentRequest(string storeId, string paymentRequestId, CancellationToken token = default) { - return GetFromActionResult(await _paymentRequestController.GetPaymentRequest(storeId, paymentRequestId)); + return GetFromActionResult(await _paymentRequestController.GetPaymentRequest(storeId, paymentRequestId)); } public override async Task ArchivePaymentRequest(string storeId, string paymentRequestId, @@ -594,7 +594,7 @@ namespace BTCPayServer.Controllers.GreenField public override async Task GetCurrentAPIKeyInfo(CancellationToken token = default) { - return GetFromActionResult(await _apiKeysController.GetKey()); + return GetFromActionResult(await _apiKeysController.GetKey()); } public override async Task CreateAPIKey(CreateApiKeyRequest request, @@ -734,7 +734,7 @@ namespace BTCPayServer.Controllers.GreenField public override async Task GetStore(string storeId, CancellationToken token = default) { - return GetFromActionResult(await _storesController.GetStore(storeId)); + return GetFromActionResult(_storesController.GetStore(storeId)); } public override async Task RemoveStore(string storeId, CancellationToken token = default) diff --git a/BTCPayServer/Controllers/GreenField/NotificationsController.cs b/BTCPayServer/Controllers/GreenField/NotificationsController.cs index d98cc2116..f94ae17d2 100644 --- a/BTCPayServer/Controllers/GreenField/NotificationsController.cs +++ b/BTCPayServer/Controllers/GreenField/NotificationsController.cs @@ -55,7 +55,7 @@ namespace BTCPayServer.Controllers.GreenField if (items.Count == 0) { - return NotFound(); + return NotificationNotFound(); } return Ok(ToModel(items.Items.First())); @@ -71,7 +71,7 @@ namespace BTCPayServer.Controllers.GreenField if (items.Count == 0) { - return NotFound(); + return NotificationNotFound(); } return Ok(ToModel(items.First())); @@ -101,5 +101,9 @@ namespace BTCPayServer.Controllers.GreenField Link = string.IsNullOrEmpty(entity.ActionLink) ? null : new Uri(entity.ActionLink) }; } + private IActionResult NotificationNotFound() + { + return this.CreateAPIError(404, "notification-not-found", "The notification was not found"); + } } } diff --git a/BTCPayServer/Controllers/GreenField/PaymentRequestsController.cs b/BTCPayServer/Controllers/GreenField/PaymentRequestsController.cs index 1d99be93b..3f70d16d6 100644 --- a/BTCPayServer/Controllers/GreenField/PaymentRequestsController.cs +++ b/BTCPayServer/Controllers/GreenField/PaymentRequestsController.cs @@ -42,14 +42,14 @@ namespace BTCPayServer.Controllers.GreenField [Authorize(Policy = Policies.CanViewPaymentRequests, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpGet("~/api/v1/stores/{storeId}/payment-requests/{paymentRequestId}")] - public async Task> GetPaymentRequest(string storeId, string paymentRequestId) + public async Task GetPaymentRequest(string storeId, string paymentRequestId) { var pr = await _paymentRequestRepository.FindPaymentRequests( new PaymentRequestQuery() { StoreId = storeId, Ids = new[] { paymentRequestId } }); if (pr.Total == 0) { - return NotFound(); + return PaymentRequestNotFound(); } return Ok(FromModel(pr.Items.First())); @@ -58,13 +58,13 @@ namespace BTCPayServer.Controllers.GreenField [Authorize(Policy = Policies.CanModifyPaymentRequests, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpDelete("~/api/v1/stores/{storeId}/payment-requests/{paymentRequestId}")] - public async Task ArchivePaymentRequest(string storeId, string paymentRequestId) + public async Task ArchivePaymentRequest(string storeId, string paymentRequestId) { var pr = await _paymentRequestRepository.FindPaymentRequests( new PaymentRequestQuery() { StoreId = storeId, Ids = new[] { paymentRequestId }, IncludeArchived = false }); if (pr.Total == 0) { - return NotFound(); + return PaymentRequestNotFound(); } var updatedPr = pr.Items.First(); @@ -112,7 +112,7 @@ namespace BTCPayServer.Controllers.GreenField new PaymentRequestQuery() { StoreId = storeId, Ids = new[] { paymentRequestId } }); if (pr.Total == 0) { - return NotFound(); + return PaymentRequestNotFound(); } var updatedPr = pr.Items.First(); @@ -164,5 +164,10 @@ namespace BTCPayServer.Controllers.GreenField CustomCSSLink = blob.CustomCSSLink }; } + + private IActionResult PaymentRequestNotFound() + { + return this.CreateAPIError(404, "payment-request-not-found", "The payment request was not found"); + } } } diff --git a/BTCPayServer/Controllers/GreenField/PullPaymentController.cs b/BTCPayServer/Controllers/GreenField/PullPaymentController.cs index dd762a22b..48b46f857 100644 --- a/BTCPayServer/Controllers/GreenField/PullPaymentController.cs +++ b/BTCPayServer/Controllers/GreenField/PullPaymentController.cs @@ -227,8 +227,6 @@ namespace BTCPayServer.Controllers.GreenField [AllowAnonymous] public async Task CreatePayout(string pullPaymentId, CreatePayoutRequest request) { - if (request is null) - return NotFound(); if (!PaymentMethodId.TryParse(request?.PaymentMethod, out var paymentMethodId)) { ModelState.AddModelError(nameof(request.PaymentMethod), "Invalid payment method"); diff --git a/BTCPayServer/Controllers/GreenField/StoreLNURLPayPaymentMethodsController.cs b/BTCPayServer/Controllers/GreenField/StoreLNURLPayPaymentMethodsController.cs index 64a821745..02d19eac7 100644 --- a/BTCPayServer/Controllers/GreenField/StoreLNURLPayPaymentMethodsController.cs +++ b/BTCPayServer/Controllers/GreenField/StoreLNURLPayPaymentMethodsController.cs @@ -77,11 +77,7 @@ namespace BTCPayServer.Controllers.GreenField [HttpGet("~/api/v1/stores/{storeId}/payment-methods/LNURLPay/{cryptoCode}")] public IActionResult GetLNURLPayPaymentMethod(string storeId, string cryptoCode) { - if (!GetNetwork(cryptoCode, out BTCPayNetwork _)) - { - return NotFound(); - } - + AssertCryptoCodeWallet(cryptoCode, out _); var method = GetExistingLNURLPayPaymentMethod(cryptoCode); if (method is null) { @@ -97,10 +93,8 @@ namespace BTCPayServer.Controllers.GreenField string storeId, string cryptoCode) { - if (!GetNetwork(cryptoCode, out BTCPayNetwork _)) - { - return NotFound(); - } + + AssertCryptoCodeWallet(cryptoCode, out _); var id = new PaymentMethodId(cryptoCode, PaymentTypes.LNURLPay); var store = Store; @@ -115,11 +109,8 @@ namespace BTCPayServer.Controllers.GreenField [FromBody] LNURLPayPaymentMethodData paymentMethodData) { var paymentMethodId = new PaymentMethodId(cryptoCode, PaymentTypes.LNURLPay); - - if (!GetNetwork(cryptoCode, out var network)) - { - return NotFound(); - } + + AssertCryptoCodeWallet(cryptoCode, out _); var lnMethod = StoreLightningNetworkPaymentMethodsController.GetExistingLightningLikePaymentMethod(_btcPayNetworkProvider, cryptoCode, Store); @@ -169,12 +160,12 @@ namespace BTCPayServer.Controllers.GreenField paymentMethod.UseBech32Scheme, paymentMethod.EnableForStandardInvoices ); } - - private bool GetNetwork(string cryptoCode, [MaybeNullWhen(false)] out BTCPayNetwork network) + private void AssertCryptoCodeWallet(string cryptoCode, out BTCPayNetwork network) { network = _btcPayNetworkProvider.GetNetwork(cryptoCode); - network = network?.SupportLightning is true ? network : null; - return network != null; + if (network is null) + throw new JsonHttpException(this.CreateAPIError(404, "unknown-cryptocode", "This crypto code isn't set up in this BTCPay Server instance")); } + } } diff --git a/BTCPayServer/Controllers/GreenField/StoreOnChainPaymentMethodsController.WalletGeneration.cs b/BTCPayServer/Controllers/GreenField/StoreOnChainPaymentMethodsController.WalletGeneration.cs index c4d4536f9..ed9e7456c 100644 --- a/BTCPayServer/Controllers/GreenField/StoreOnChainPaymentMethodsController.WalletGeneration.cs +++ b/BTCPayServer/Controllers/GreenField/StoreOnChainPaymentMethodsController.WalletGeneration.cs @@ -18,11 +18,8 @@ namespace BTCPayServer.Controllers.GreenField public async Task GenerateOnChainWallet(string storeId, string cryptoCode, GenerateWalletRequest request) { - var network = _btcPayNetworkProvider.GetNetwork(cryptoCode); - if (network is null) - { - return NotFound(); - } + + AssertCryptoCodeWallet(cryptoCode, out var network, out var wallet); if (!_walletProvider.IsAvailable(network)) { diff --git a/BTCPayServer/Controllers/GreenField/StoreOnChainWalletsController.cs b/BTCPayServer/Controllers/GreenField/StoreOnChainWalletsController.cs index d75a39ddb..fc8516e23 100644 --- a/BTCPayServer/Controllers/GreenField/StoreOnChainWalletsController.cs +++ b/BTCPayServer/Controllers/GreenField/StoreOnChainWalletsController.cs @@ -151,7 +151,8 @@ namespace BTCPayServer.Controllers.GreenField var addr = await _walletReceiveService.UnReserveAddress(new WalletId(storeId, cryptoCode)); if (addr is null) { - return NotFound(); + return this.CreateAPIError("no-reserved-address", + $"There was no reserved address for {cryptoCode} on this store."); } return Ok(); } @@ -210,7 +211,7 @@ namespace BTCPayServer.Controllers.GreenField var tx = await wallet.FetchTransaction(derivationScheme.AccountDerivation, uint256.Parse(transactionId)); if (tx is null) { - return NotFound(); + return this.CreateAPIError(404, "transaction-not-found", "The transaction was not found."); } var walletId = new WalletId(storeId, cryptoCode); @@ -523,8 +524,7 @@ namespace BTCPayServer.Controllers.GreenField network = _btcPayNetworkProvider.GetNetwork(cryptoCode); if (network is null) { - actionResult = NotFound(); - return true; + throw new JsonHttpException(this.CreateAPIError(404, "unknown-cryptocode", "This crypto code isn't set up in this BTCPay Server instance")); } diff --git a/BTCPayServer/Controllers/GreenField/StoresController.cs b/BTCPayServer/Controllers/GreenField/StoresController.cs index 72de55719..0c1943201 100644 --- a/BTCPayServer/Controllers/GreenField/StoresController.cs +++ b/BTCPayServer/Controllers/GreenField/StoresController.cs @@ -42,14 +42,14 @@ namespace BTCPayServer.Controllers.GreenField [Authorize(Policy = Policies.CanViewStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpGet("~/api/v1/stores/{storeId}")] - public Task> GetStore(string storeId) + public IActionResult GetStore(string storeId) { var store = HttpContext.GetStoreData(); if (store == null) { - return Task.FromResult>(NotFound()); + return StoreNotFound(); } - return Task.FromResult>(Ok(FromModel(store))); + return Ok(FromModel(store)); } [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] @@ -59,7 +59,7 @@ namespace BTCPayServer.Controllers.GreenField var store = HttpContext.GetStoreData(); if (store == null) { - return NotFound(); + return StoreNotFound(); } if (!_storeRepository.CanDeleteStores()) @@ -96,7 +96,7 @@ namespace BTCPayServer.Controllers.GreenField var store = HttpContext.GetStoreData(); if (store == null) { - return NotFound(); + return StoreNotFound(); } var validationResult = Validate(request); if (validationResult != null) @@ -215,5 +215,10 @@ namespace BTCPayServer.Controllers.GreenField return !ModelState.IsValid ? this.CreateValidationError(ModelState) : null; } + + private IActionResult StoreNotFound() + { + return this.CreateAPIError(404, "store-not-found", "The store was not found"); + } } }