Fix: Do not expose xpub without modify store permission (#6212)

This commit is contained in:
Nicolas Dorier 2024-09-27 15:27:04 +09:00 committed by GitHub
parent 272cc3d3c9
commit 9ba4b030ed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 185 additions and 169 deletions

View File

@ -46,9 +46,15 @@ public partial class BTCPayServerClient
return await SendHttpRequest<InvoiceData>($"api/v1/stores/{storeId}/invoices/{invoiceId}", null, HttpMethod.Get, token);
}
public virtual async Task<InvoicePaymentMethodDataModel[]> GetInvoicePaymentMethods(string storeId, string invoiceId,
bool onlyAccountedPayments = true, bool includeSensitive = false,
CancellationToken token = default)
{
return await SendHttpRequest<InvoicePaymentMethodDataModel[]>($"api/v1/stores/{storeId}/invoices/{invoiceId}/payment-methods", null, HttpMethod.Get, token);
var queryPayload = new Dictionary<string, object>
{
{ nameof(onlyAccountedPayments), onlyAccountedPayments },
{ nameof(includeSensitive), includeSensitive }
};
return await SendHttpRequest<InvoicePaymentMethodDataModel[]>($"api/v1/stores/{storeId}/invoices/{invoiceId}/payment-methods", queryPayload, HttpMethod.Get, token);
}
public virtual async Task ArchiveInvoice(string storeId, string invoiceId,

View File

@ -2421,6 +2421,14 @@ namespace BTCPayServer.Tests
invoice = await client.CreateInvoice(user.StoreId, new CreateInvoiceRequest { Amount = 5000.0m, Currency = "USD" });
methods = await client.GetInvoicePaymentMethods(user.StoreId, invoice.Id);
method = methods.First();
Assert.Equal(JTokenType.Null, method.AdditionalData["accountDerivation"].Type);
Assert.NotNull(method.AdditionalData["keyPath"]);
methods = await client.GetInvoicePaymentMethods(user.StoreId, invoice.Id, includeSensitive: true);
method = methods.First();
Assert.Equal(JTokenType.String, method.AdditionalData["accountDerivation"].Type);
var clientViewOnly = await user.CreateClient(Policies.CanViewInvoices);
await AssertApiError(403, "missing-permission", () => clientViewOnly.GetInvoicePaymentMethods(user.StoreId, invoice.Id, includeSensitive: true));
await tester.WaitForEvent<NewOnChainTransactionEvent>(async () =>
{

View File

@ -1,6 +1,7 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
@ -14,6 +15,7 @@ using BTCPayServer.Payments;
using BTCPayServer.Payments.Bitcoin;
using BTCPayServer.Payouts;
using BTCPayServer.Rating;
using BTCPayServer.Security;
using BTCPayServer.Security.Greenfield;
using BTCPayServer.Services;
using BTCPayServer.Services.Invoices;
@ -25,6 +27,7 @@ using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using NBitcoin;
using NBitpayClient;
using Newtonsoft.Json.Linq;
using CreateInvoiceRequest = BTCPayServer.Client.Models.CreateInvoiceRequest;
using InvoiceData = BTCPayServer.Client.Models.InvoiceData;
@ -96,11 +99,7 @@ namespace BTCPayServer.Controllers.Greenfield
[FromQuery] int? take = null
)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return StoreNotFound();
}
var store = HttpContext.GetStoreData()!;
if (startDate is DateTimeOffset s &&
endDate is DateTimeOffset e &&
s > e)
@ -133,17 +132,9 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpGet("~/api/v1/stores/{storeId}/invoices/{invoiceId}")]
public async Task<IActionResult> GetInvoice(string storeId, string invoiceId)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice?.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
return Ok(ToModel(invoice));
}
@ -153,16 +144,9 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpDelete("~/api/v1/stores/{storeId}/invoices/{invoiceId}")]
public async Task<IActionResult> ArchiveInvoice(string storeId, string invoiceId)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice?.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
await _invoiceRepository.ToggleInvoiceArchival(invoiceId, true, storeId);
return Ok();
}
@ -172,19 +156,10 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpPut("~/api/v1/stores/{storeId}/invoices/{invoiceId}")]
public async Task<IActionResult> UpdateInvoice(string storeId, string invoiceId, UpdateInvoiceRequest request)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var result = await _invoiceRepository.UpdateInvoiceMetadata(invoiceId, storeId, request.Metadata);
if (result != null)
{
return Ok(ToModel(result));
}
return InvoiceNotFound();
if (!BelongsToThisStore(result))
return InvoiceNotFound();
return Ok(ToModel(result));
}
[Authorize(Policy = Policies.CanCreateInvoice,
@ -192,12 +167,7 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpPost("~/api/v1/stores/{storeId}/invoices")]
public async Task<IActionResult> CreateInvoice(string storeId, CreateInvoiceRequest request)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return StoreNotFound();
}
var store = HttpContext.GetStoreData()!;
if (request.Amount < 0.0m)
{
ModelState.AddModelError(nameof(request.Amount), "The amount should be 0 or more.");
@ -271,17 +241,9 @@ namespace BTCPayServer.Controllers.Greenfield
public async Task<IActionResult> MarkInvoiceStatus(string storeId, string invoiceId,
MarkInvoiceStatusRequest request)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
if (!await _invoiceRepository.MarkInvoiceStatus(invoice.Id, request.Status))
{
@ -300,17 +262,9 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpPost("~/api/v1/stores/{storeId}/invoices/{invoiceId}/unarchive")]
public async Task<IActionResult> UnarchiveInvoice(string storeId, string invoiceId)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
if (!invoice.Archived)
{
@ -328,21 +282,23 @@ namespace BTCPayServer.Controllers.Greenfield
[Authorize(Policy = Policies.CanViewInvoices,
AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
[HttpGet("~/api/v1/stores/{storeId}/invoices/{invoiceId}/payment-methods")]
public async Task<IActionResult> GetInvoicePaymentMethods(string storeId, string invoiceId, bool onlyAccountedPayments = true)
public async Task<IActionResult> GetInvoicePaymentMethods(string storeId, string invoiceId, bool onlyAccountedPayments = true, bool includeSensitive = false)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice?.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
return Ok(ToPaymentMethodModels(invoice, onlyAccountedPayments));
if (includeSensitive && !await _authorizationService.CanModifyStore(User))
return this.CreateAPIPermissionError(Policies.CanModifyStoreSettings);
return Ok(ToPaymentMethodModels(invoice, onlyAccountedPayments, includeSensitive));
}
bool BelongsToThisStore([NotNullWhen(true)] InvoiceEntity invoice) => BelongsToThisStore(invoice, out _);
private bool BelongsToThisStore([NotNullWhen(true)] InvoiceEntity invoice, [MaybeNullWhen(false)] out Data.StoreData store)
{
store = this.HttpContext.GetStoreData();
return invoice?.StoreId is not null && store.Id == invoice.StoreId;
}
[Authorize(Policy = Policies.CanViewInvoices,
@ -350,17 +306,9 @@ namespace BTCPayServer.Controllers.Greenfield
[HttpPost("~/api/v1/stores/{storeId}/invoices/{invoiceId}/payment-methods/{paymentMethod}/activate")]
public async Task<IActionResult> ActivateInvoicePaymentMethod(string storeId, string invoiceId, string paymentMethod)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return InvoiceNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice?.StoreId != store.Id)
{
if (!BelongsToThisStore(invoice))
return InvoiceNotFound();
}
if (PaymentMethodId.TryParse(paymentMethod, out var paymentMethodId))
{
@ -381,22 +329,9 @@ namespace BTCPayServer.Controllers.Greenfield
CancellationToken cancellationToken = default
)
{
var store = HttpContext.GetStoreData();
if (store == null)
{
return StoreNotFound();
}
var invoice = await _invoiceRepository.GetInvoice(invoiceId, true);
if (invoice == null)
{
if (!BelongsToThisStore(invoice, out var store))
return InvoiceNotFound();
}
if (invoice.StoreId != store.Id)
{
return InvoiceNotFound();
}
if (!invoice.GetInvoiceState().CanRefund())
{
return this.CreateAPIError("non-refundable", "Cannot refund this invoice");
@ -588,12 +523,8 @@ namespace BTCPayServer.Controllers.Greenfield
{
return this.CreateAPIError(404, "invoice-not-found", "The invoice was not found");
}
private IActionResult StoreNotFound()
{
return this.CreateAPIError(404, "store-not-found", "The store was not found");
}
private InvoicePaymentMethodDataModel[] ToPaymentMethodModels(InvoiceEntity entity, bool includeAccountedPaymentOnly)
private InvoicePaymentMethodDataModel[] ToPaymentMethodModels(InvoiceEntity entity, bool includeAccountedPaymentOnly, bool includeSensitive)
{
return entity.GetPaymentPrompts().Select(
prompt =>
@ -606,7 +537,12 @@ namespace BTCPayServer.Controllers.Greenfield
var details = prompt.Details;
if (handler is not null && prompt.Activated)
details = JToken.FromObject(handler.ParsePaymentPromptDetails(details), handler.Serializer.ForAPI());
{
var detailsObj = handler.ParsePaymentPromptDetails(details);
if (!includeSensitive)
handler.StripDetailsForNonOwner(detailsObj);
details = JToken.FromObject(detailsObj, handler.Serializer.ForAPI());
}
return new InvoicePaymentMethodDataModel
{
Activated = prompt.Activated,
@ -621,7 +557,7 @@ namespace BTCPayServer.Controllers.Greenfield
PaymentMethodFee = accounting?.PaymentMethodFee ?? 0m,
PaymentLink = (prompt.Activated ? paymentLinkExtension?.GetPaymentLink(prompt, Url) : null) ?? string.Empty,
Payments = payments.Select(paymentEntity => ToPaymentModel(entity, paymentEntity)).ToList(),
AdditionalData = prompt.Details
AdditionalData = details
};
}).ToArray();
}

View File

@ -145,9 +145,7 @@ namespace BTCPayServer.Controllers.Greenfield
if (includeConfig is true)
{
var canModifyStore = (await _authorizationService.AuthorizeAsync(User, null,
new PolicyRequirement(Policies.CanModifyStoreSettings))).Succeeded;
if (!canModifyStore)
if (!await _authorizationService.CanModifyStore(User))
return this.CreateAPIPermissionError(Policies.CanModifyStoreSettings);
}

View File

@ -831,10 +831,12 @@ namespace BTCPayServer.Controllers.Greenfield
}
public override async Task<InvoicePaymentMethodDataModel[]> GetInvoicePaymentMethods(string storeId,
string invoiceId, CancellationToken token = default)
string invoiceId,
bool onlyAccountedPayments = true, bool includeSensitive = false,
CancellationToken token = default)
{
return GetFromActionResult<InvoicePaymentMethodDataModel[]>(
await GetController<GreenfieldInvoiceController>().GetInvoicePaymentMethods(storeId, invoiceId));
await GetController<GreenfieldInvoiceController>().GetInvoicePaymentMethods(storeId, invoiceId, onlyAccountedPayments, includeSensitive));
}
public override async Task ArchiveInvoice(string storeId, string invoiceId, CancellationToken token = default)

View File

@ -2,6 +2,7 @@ using System.Security.Claims;
using System.Threading.Tasks;
using BTCPayServer.Abstractions.Constants;
using BTCPayServer.Client;
using BTCPayServer.Security;
using BTCPayServer.Security.Bitpay;
using BTCPayServer.Security.Greenfield;
using BTCPayServer.Services;
@ -12,6 +13,11 @@ namespace BTCPayServer
{
public static class AuthorizationExtensions
{
public static async Task<bool> CanModifyStore(this IAuthorizationService authorizationService, ClaimsPrincipal user)
{
return (await authorizationService.AuthorizeAsync(user, null,
new PolicyRequirement(Policies.CanModifyStoreSettings))).Succeeded;
}
public static async Task<(bool HotWallet, bool RPCImport)> CanUseHotWallet(
this IAuthorizationService authorizationService,
PoliciesSettings policiesSettings,

View File

@ -89,7 +89,10 @@ namespace BTCPayServer.Payments.Bitcoin
{
return ParsePaymentMethodConfig(config);
}
public void StripDetailsForNonOwner(object details)
{
((BitcoinPaymentPromptDetails)details).AccountDerivation = null;
}
public async Task AfterSavingInvoice(PaymentMethodContext paymentMethodContext)
{
var paymentPrompt = paymentMethodContext.Prompt;

View File

@ -14,6 +14,9 @@ namespace BTCPayServer.Payments.Bitcoin
[JsonConverter(typeof(StringEnumConverter))]
public NetworkFeeMode FeeMode { get; set; }
/// <summary>
/// The fee rate charged to the user as `PaymentMethodFee`.
/// </summary>
[JsonConverter(typeof(NBitcoin.JsonConverters.FeeRateJsonConverter))]
public FeeRate PaymentMethodFeeRate
{
@ -21,6 +24,10 @@ namespace BTCPayServer.Payments.Bitcoin
set;
}
public bool PayjoinEnabled { get; set; }
/// <summary>
/// The recommended fee rate for this payment method.
/// </summary>
[JsonConverter(typeof(NBitcoin.JsonConverters.FeeRateJsonConverter))]
public FeeRate RecommendedFeeRate { get; set; }
[JsonConverter(typeof(NBitcoin.JsonConverters.KeyPathJsonConverter))]

View File

@ -67,6 +67,12 @@ namespace BTCPayServer.Payments
/// <param name="details"></param>
/// <returns></returns>
object ParsePaymentPromptDetails(JToken details);
/// <summary>
/// Remove properties from the details which shouldn't appear to non-store owner.
/// </summary>
/// <param name="details">Prompt details</param>
void StripDetailsForNonOwner(object details) { }
/// <summary>
/// Parse the configuration of the payment method in the store
/// </summary>

View File

@ -413,6 +413,16 @@
"type": "boolean",
"default": true
}
},
{
"name": "includeSensitive",
"in": "query",
"required": false,
"description": "If `true`, `additionalData` might include sensitive data (such as xpub). Requires the permission `btcpay.store.canmodifystoresettings`.",
"schema": {
"type": "boolean",
"default": false
}
}
],
"description": "View information about the specified invoice's payment methods",
@ -644,10 +654,10 @@
]
}
},
"/api/v1/stores/{storeId}/invoices/{invoiceId}/refund": {
"/api/v1/stores/{storeId}/invoices/{invoiceId}/refund": {
"post": {
"tags": [
"Invoices"
"Invoices"
],
"summary": "Refund invoice",
"parameters": [
@ -668,69 +678,69 @@
"schema": {
"type": "string"
}
}
],
"description": "Refund invoice",
"operationId": "Invoices_Refund",
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"type": "object",
"additionalProperties": false,
"properties": {
"name": {
"type": "string",
"description": "Name of the pull payment (Default: 'Refund' followed by the invoice id)",
"nullable": true
},
"description": {
"type": "string",
"description": "Description of the pull payment"
},
"payoutMethodId": {
"$ref": "#/components/schemas/PayoutMethodId"
},
"refundVariant": {
"type": "string",
"description": "* `RateThen`: Refund the crypto currency price, at the rate the invoice got paid.\r\n* `CurrentRate`: Refund the crypto currency price, at the current rate.\r\n*`Fiat`: Refund the invoice currency, at the rate when the refund will be sent.\r\n*`OverpaidAmount`: Refund the crypto currency amount that was overpaid.\r\n*`Custom`: Specify the amount, currency, and rate of the refund. (see `customAmount` and `customCurrency`)",
"x-enumNames": [
"RateThen",
"CurrentRate",
"Fiat",
"Custom"
],
"enum": [
"RateThen",
"CurrentRate",
"OverpaidAmount",
"Fiat",
"Custom"
]
},
"subtractPercentage": {
"type": "string",
"format": "decimal",
"description": "Optional percentage by which to reduce the refund, e.g. as processing charge or to compensate for the mining fee.",
"example": "2.1"
},
"customAmount": {
"type": "string",
"format": "decimal",
"description": "The amount to refund if the `refundVariant` is `Custom`.",
"example": "5.00"
},
"customCurrency": {
"type": "string",
"description": "The currency to refund if the `refundVariant` is `Custom`",
"example": "USD"
}
}
}
}
],
"description": "Refund invoice",
"operationId": "Invoices_Refund",
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"type": "object",
"additionalProperties": false,
"properties": {
"name": {
"type": "string",
"description": "Name of the pull payment (Default: 'Refund' followed by the invoice id)",
"nullable": true
},
"description": {
"type": "string",
"description": "Description of the pull payment"
},
"payoutMethodId": {
"$ref": "#/components/schemas/PayoutMethodId"
},
"refundVariant": {
"type": "string",
"description": "* `RateThen`: Refund the crypto currency price, at the rate the invoice got paid.\r\n* `CurrentRate`: Refund the crypto currency price, at the current rate.\r\n*`Fiat`: Refund the invoice currency, at the rate when the refund will be sent.\r\n*`OverpaidAmount`: Refund the crypto currency amount that was overpaid.\r\n*`Custom`: Specify the amount, currency, and rate of the refund. (see `customAmount` and `customCurrency`)",
"x-enumNames": [
"RateThen",
"CurrentRate",
"Fiat",
"Custom"
],
"enum": [
"RateThen",
"CurrentRate",
"OverpaidAmount",
"Fiat",
"Custom"
]
},
"subtractPercentage": {
"type": "string",
"format": "decimal",
"description": "Optional percentage by which to reduce the refund, e.g. as processing charge or to compensate for the mining fee.",
"example": "2.1"
},
"customAmount": {
"type": "string",
"format": "decimal",
"description": "The amount to refund if the `refundVariant` is `Custom`.",
"example": "5.00"
},
"customCurrency": {
"type": "string",
"description": "The currency to refund if the `refundVariant` is `Custom`",
"example": "USD"
}
}
}
}
}
},
},
"responses": {
"200": {
"description": "Pull payment for refunding the invoice",
@ -1329,6 +1339,7 @@
"anyOf": [
{
"type": "object",
"title": "*-LNURL",
"description": "LNURL Pay information",
"properties": {
"providedComment": {
@ -1345,6 +1356,39 @@
}
}
},
{
"type": "object",
"title": "*-CHAIN",
"description": "Bitcoin On-Chain payment information",
"properties": {
"keyPath": {
"type": "string",
"description": "The key path relative to the account derviation key.",
"example": "0/1"
},
"payjoinEnabled": {
"type": "boolean",
"description": "If the payjoin feature is enabled for this payment method."
},
"accountDerivation": {
"type": "string",
"description": "The derivation scheme used to derive addresses (null if `includeSensitive` is `false`)",
"example": "xpub6DVMcQAQCtGbNDTEjQGtR1GRoTKw7AzP6bVivX4gFnewcnRk1r1tbczpfsaYjKKVrmtyiwYqAEnALYzZ8yoTArVsKfZekmwLFqQp4MRgPhy"
},
"recommendedFeeRate": {
"type": "string",
"format": "decimal",
"description": "The recommended fee rate for this payment method.",
"example": "4.107"
},
"paymentMethodFeeRate": {
"type": "string",
"format": "decimal",
"description": "The fee rate charged to the user as `PaymentMethodFee`.",
"example": "3.975"
}
}
},
{
"type": "object",
"description": "No additional information"