Skip to content

Commit 99a82e7

Browse files
rockstardevrockstardev
committed
fix: bind protocol OTP redemption to the store it was created for
- Reject anonymous protocol requests whose route store id differs from the store the OTP was created for, before any processing - Pass the OTP-resolved store id to Boltz setup instead of the route value; drop the redundant [FromRoute] StoreId property from ProtocolController - Enforce OTP expiry at redemption time in SamRockProtocolHostedService.TryGet instead of only in the periodic sweep - Add controller-level and HTTP-level regression tests; retry on 429 in the OTP warm-up loops since the endpoints share a rate-limit zone - Version bumped 1.1.0 -> 1.1.1 (patch: authorization hardening)
1 parent 1068af2 commit 99a82e7

7 files changed

Lines changed: 382 additions & 10 deletions

File tree

‎Plugins/SamRockProtocol/Controllers/ProtocolController.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ public class ProtocolController(
3737
BoltzWrapper boltzWrapper)
3838
: Controller
3939
{
40-
[FromRoute]
41-
public string StoreId { get; set; }
42-
4340
[AllowAnonymous]
4441
[IgnoreAntiforgeryToken] // dart/dio update now causes Form[""] to break, so this is needed
4542
[RateLimitsFilter("SamRockProtocol", Scope = RateLimitsScope.RemoteAddress)]
@@ -50,6 +47,11 @@ public async Task<IActionResult> SamRockProtocol()
5047
if (string.IsNullOrEmpty(otp) || !samrockProtocolService.TryGet(otp, out var importWalletModel))
5148
return NotFound(new SamRockProtocolResponse(false, "OTP not found or expired.", null));
5249

50+
// An OTP may only be redeemed on the route of the store it was created for.
51+
var routeStoreId = RouteData.Values["storeId"] as string;
52+
if (!string.Equals(routeStoreId, importWalletModel.StoreId, StringComparison.Ordinal))
53+
return NotFound(new SamRockProtocolResponse(false, "OTP not found or expired.", null));
54+
5355
var storeData = await storeRepository.FindStore(importWalletModel.StoreId);
5456
if (storeData == null)
5557
return NotFound(new SamRockProtocolResponse(false, "Store not found.", null));
@@ -216,7 +218,7 @@ private async Task<IActionResult> processSamRockProtocolRequest(SamRockProtocolR
216218
}
217219
else
218220
{
219-
await boltzWrapper.SetBoltz(StoreId, DescriptorParser.NormalizeDescriptor(setupModel.BTCLN.LBTC.Descriptor), result);
221+
await boltzWrapper.SetBoltz(storeData.Id, DescriptorParser.NormalizeDescriptor(setupModel.BTCLN.LBTC.Descriptor), result);
220222
}
221223
}
222224
else

‎Plugins/SamRockProtocol/SamRockProtocol.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<Description>Get paid online, receive funds on your phone.
1212
SamRock Protocol links your store to self-custodial wallet via a simple QR scan. Try it with Aqua Wallet!
1313
</Description>
14-
<Version>1.1.0</Version>
14+
<Version>1.1.1</Version>
1515
</PropertyGroup>
1616

1717
<!-- Plugin development properties -->

‎Plugins/SamRockProtocol/Services/SamRockProtocolHostedService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ public bool TryGet(string otp, out ImportWalletsViewModel model)
6666
{
6767
if (_samrockImportDictionary.TryGetValue(otp, out var value))
6868
{
69+
if (value.Expires <= DateTimeOffset.UtcNow)
70+
{
71+
_samrockImportDictionary.Remove(otp);
72+
model = null;
73+
return false;
74+
}
75+
6976
model = value;
7077
return true;
7178
}

‎SamRockProtocol.Tests/SamRockProtocolCrossTenantTest.cs‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
using System;
2+
using System.Net;
3+
using System.Net.Http;
4+
using System.Net.Http.Headers;
5+
using System.Text;
6+
using System.Text.Json;
7+
using System.Threading.Tasks;
8+
using BTCPayServer.Tests;
9+
using Xunit;
10+
using Xunit.Abstractions;
11+
12+
namespace BTCPayServer.Plugins.Tests;
13+
14+
/// <summary>
15+
/// HTTP-level test that POST /plugins/{storeId}/samrock/protocol only honors
16+
/// an OTP on the route of the store the OTP was created for, and that the
17+
/// legitimate same-store flow keeps working.
18+
/// </summary>
19+
[Collection("Plugin Tests")]
20+
[Trait("Category", "PlaywrightUITest")]
21+
public class SamRockProtocolCrossTenantTest : UnitTestBase
22+
{
23+
private readonly SharedPluginTestFixture _fixture;
24+
private readonly ITestOutputHelper _helper;
25+
26+
public SamRockProtocolCrossTenantTest(SharedPluginTestFixture fixture, ITestOutputHelper helper) : base(helper)
27+
{
28+
_fixture = fixture;
29+
_helper = helper;
30+
if (_fixture.ServerTester == null) _fixture.Initialize(this);
31+
ServerTester = _fixture.ServerTester;
32+
}
33+
34+
public ServerTester ServerTester { get; }
35+
36+
// AQUA-shape Liquid descriptor, same reference value as the happy-path test.
37+
private const string LiquidDescriptor =
38+
"ct(slip77(c82e173e7eb01dd024136f0c956a2ec078ff04c6abf5611c5db41e16d1326403),elsh(wpkh([e17c2d80/49'/1776'/0']xpub6BemYiVNp19a2CyepSKDsDp2LgfvzZHvmepc5yM656fFDf93qcZ8UpgNwK9EwNbBimkr4mjNbK7anPqKS9M3pa9sGtve9seQaHuQJjJU6ps/0/*)))#ugh3xr7l";
39+
40+
[Fact]
41+
public async Task SamRockProtocol_OtpFromOtherStore_IsRejected()
42+
{
43+
var userA = ServerTester.NewAccount();
44+
await userA.GrantAccessAsync();
45+
var storeIdA = userA.StoreId;
46+
47+
var userB = ServerTester.NewAccount();
48+
await userB.GrantAccessAsync();
49+
var storeIdB = userB.StoreId;
50+
51+
Assert.NotEqual(storeIdA, storeIdB);
52+
53+
// 1. User A mints an OTP for their own store, Lightning selected.
54+
using var clientA = new HttpClient { BaseAddress = ServerTester.PayTester.ServerUri };
55+
var basic = Convert.ToBase64String(Encoding.UTF8.GetBytes(
56+
$"{userA.RegisterDetails.Email}:{userA.RegisterDetails.Password}"));
57+
clientA.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", basic);
58+
59+
// Same warm-up retry as SamRockProtocolHappyPathTest: plugin controller
60+
// routes can race ServerTester boot. The OTP endpoints also share a
61+
// rate-limit zone (12r/min, burst=3, per remote address) with the
62+
// happy-path test, so retry on 429 as well.
63+
string otp = null;
64+
const int otpMaxAttempts = 20;
65+
const int otpDelayMs = 500;
66+
const int rateLimitDelayMs = 6000;
67+
for (var attempt = 1; attempt <= otpMaxAttempts; attempt++)
68+
{
69+
var otpReq = new StringContent(
70+
JsonSerializer.Serialize(new { btc = false, btcln = true, lbtc = false }),
71+
Encoding.UTF8, "application/json");
72+
var otpResp = await clientA.PostAsync($"api/v1/stores/{storeIdA}/samrock/otps", otpReq);
73+
var otpRespBody = await otpResp.Content.ReadAsStringAsync();
74+
_helper.WriteLine($"OTP create attempt {attempt}/{otpMaxAttempts} ({(int)otpResp.StatusCode}): {otpRespBody}");
75+
if (otpResp.StatusCode == HttpStatusCode.NotFound)
76+
{
77+
await Task.Delay(otpDelayMs);
78+
continue;
79+
}
80+
if (otpResp.StatusCode == HttpStatusCode.TooManyRequests)
81+
{
82+
await Task.Delay(rateLimitDelayMs);
83+
continue;
84+
}
85+
Assert.True(otpResp.IsSuccessStatusCode,
86+
$"OTP create expected 2xx, got {(int)otpResp.StatusCode}: {otpRespBody}");
87+
using var otpDoc = JsonDocument.Parse(otpRespBody);
88+
foreach (var name in new[] { "otp", "Otp", "OTP" })
89+
{
90+
if (otpDoc.RootElement.TryGetProperty(name, out var v) && v.ValueKind == JsonValueKind.String)
91+
{
92+
otp = v.GetString();
93+
break;
94+
}
95+
}
96+
break;
97+
}
98+
Assert.False(string.IsNullOrEmpty(otp), "OTP not found in create response");
99+
100+
// Raw JSON string: the request model binds "BTC-LN" via an explicit
101+
// [JsonProperty] name that anonymous C# types cannot express.
102+
var payloadJson = "{\"Version\":\"1.0\",\"BTC-LN\":{\"Type\":\"Boltz\",\"LBTC\":{\"Descriptor\":\"" +
103+
LiquidDescriptor + "\"}}}";
104+
105+
// 2. POST the OTP to store B's protocol route: must be rejected with
106+
// 404 before any processing.
107+
using var anon = new HttpClient { BaseAddress = ServerTester.PayTester.ServerUri };
108+
var (crossResp, crossBody) = await PostProtocolWithRetry(anon, storeIdB, otp, payloadJson, "Cross-store");
109+
110+
Assert.True(crossResp.StatusCode == HttpStatusCode.NotFound,
111+
$"An OTP minted for store {storeIdA} must not be accepted on the route of store " +
112+
$"{storeIdB} (HTTP {(int)crossResp.StatusCode}). Response: {crossBody}");
113+
114+
// 3. The rejected request must not have consumed the OTP: status stays
115+
// "pending".
116+
var statusResp = await clientA.GetAsync($"api/v1/stores/{storeIdA}/samrock/otps/{otp}");
117+
var statusBody = await statusResp.Content.ReadAsStringAsync();
118+
_helper.WriteLine($"OTP status after cross-store POST ({(int)statusResp.StatusCode}): {statusBody}");
119+
Assert.True(statusResp.IsSuccessStatusCode,
120+
$"OTP status expected 2xx, got {(int)statusResp.StatusCode}: {statusBody}");
121+
using (var statusDoc = JsonDocument.Parse(statusBody))
122+
{
123+
string status = null;
124+
foreach (var name in new[] { "status", "Status" })
125+
{
126+
if (statusDoc.RootElement.TryGetProperty(name, out var v) && v.ValueKind == JsonValueKind.String)
127+
{
128+
status = v.GetString();
129+
break;
130+
}
131+
}
132+
Assert.True(string.Equals(status, "pending", StringComparison.OrdinalIgnoreCase),
133+
$"OTP was consumed by the cross-store request (status={status}).");
134+
}
135+
136+
// 4. Legitimate-flow regression: the same OTP must still work on its
137+
// own store's route. (HTTP stays 200 even in EnableBoltzSupport=false
138+
// builds - the inner BTC_LN result reports "Boltz support is not
139+
// enabled in this build." but the action returns Ok.)
140+
var (legitResp, legitBody) = await PostProtocolWithRetry(anon, storeIdA, otp, payloadJson, "Same-store");
141+
Assert.True(legitResp.IsSuccessStatusCode,
142+
$"Same-store protocol POST expected 2xx, got {(int)legitResp.StatusCode}: {legitBody}");
143+
}
144+
145+
// The protocol endpoint is rate limited per remote address; retry on 429
146+
// until the token bucket refills.
147+
private async Task<(HttpResponseMessage resp, string body)> PostProtocolWithRetry(
148+
HttpClient client, string storeId, string otp, string payloadJson, string label)
149+
{
150+
const int maxAttempts = 10;
151+
const int rateLimitDelayMs = 6000;
152+
for (var attempt = 1; ; attempt++)
153+
{
154+
var resp = await client.PostAsync(
155+
$"plugins/{storeId}/samrock/protocol?otp={otp}",
156+
new StringContent(payloadJson, Encoding.UTF8, "application/json"));
157+
var body = await resp.Content.ReadAsStringAsync();
158+
_helper.WriteLine($"{label} protocol POST attempt {attempt} ({(int)resp.StatusCode}): {body}");
159+
if (resp.StatusCode != HttpStatusCode.TooManyRequests || attempt >= maxAttempts)
160+
return (resp, body);
161+
await Task.Delay(rateLimitDelayMs);
162+
}
163+
}
164+
}

‎SamRockProtocol.Tests/SamRockProtocolHappyPathTest.cs‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ public async Task SamRockProtocol_AcceptsAquaDescriptors()
8181
otpResp = await client.PostAsync($"api/v1/stores/{storeId}/samrock/otps", otpReq);
8282
otpRespBody = await otpResp.Content.ReadAsStringAsync();
8383
_helper.WriteLine($"OTP create attempt {attempt}/{otpMaxAttempts} ({(int)otpResp.StatusCode}): {otpRespBody}");
84+
// 429: the OTP endpoints share a rate-limit zone (12r/min, burst=3,
85+
// per remote address) with the cross-tenant test; wait for the
86+
// token bucket to refill.
87+
if (otpResp.StatusCode == System.Net.HttpStatusCode.TooManyRequests)
88+
{
89+
await Task.Delay(6000);
90+
continue;
91+
}
8492
if (otpResp.StatusCode != System.Net.HttpStatusCode.NotFound)
8593
break;
8694
await Task.Delay(otpDelayMs);
@@ -101,7 +109,9 @@ public async Task SamRockProtocol_AcceptsAquaDescriptors()
101109
}
102110
Assert.False(string.IsNullOrEmpty(otp), $"OTP not found in response: {otpRespBody}");
103111

104-
// Protocol POST (anonymous, OTP-gated)
112+
// Protocol POST (anonymous, OTP-gated). Retries on 429: the endpoint
113+
// shares the SamRockProtocol rate-limit zone (12r/min, burst=3, per
114+
// remote address) with the cross-tenant test.
105115
using var anon = new HttpClient { BaseAddress = ServerTester.PayTester.ServerUri };
106116
var payload = new
107117
{
@@ -110,10 +120,18 @@ public async Task SamRockProtocol_AcceptsAquaDescriptors()
110120
LBTC = new { Descriptor = LbtcDescriptor }
111121
};
112122
var json = JsonSerializer.Serialize(payload);
113-
var content = new StringContent(json, Encoding.UTF8, "application/json");
114-
var response = await anon.PostAsync($"plugins/{storeId}/samrock/protocol?otp={otp}", content);
115-
var body = await response.Content.ReadAsStringAsync();
116-
_helper.WriteLine($"Protocol POST response ({(int)response.StatusCode}): {body}");
123+
HttpResponseMessage response = null;
124+
string body = null;
125+
for (var attempt = 1; ; attempt++)
126+
{
127+
var content = new StringContent(json, Encoding.UTF8, "application/json");
128+
response = await anon.PostAsync($"plugins/{storeId}/samrock/protocol?otp={otp}", content);
129+
body = await response.Content.ReadAsStringAsync();
130+
_helper.WriteLine($"Protocol POST attempt {attempt} ({(int)response.StatusCode}): {body}");
131+
if (response.StatusCode != System.Net.HttpStatusCode.TooManyRequests || attempt >= 10)
132+
break;
133+
await Task.Delay(6000);
134+
}
117135

118136
Assert.True(response.IsSuccessStatusCode,
119137
$"Protocol POST expected 2xx, got {(int)response.StatusCode}: {body}");

0 commit comments

Comments
 (0)