Skip to content

Commit

Permalink
Apps: Allow authenticated, non-owner users permissioned access
Browse files Browse the repository at this point in the history
Fixes btcpayserver#5698. Before this, the app lookup was constrained by the user having at least `CanModifyStoreSettings` permissions. This changes it to require the user being associated with a store, leaving the fine-grained authorization checks up to the individual actions.
  • Loading branch information
dennisreimann committed Jan 25, 2024
1 parent 086f713 commit 4b03115
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 11 deletions.
2 changes: 1 addition & 1 deletion BTCPayServer.Tests/BTCPayServer.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<PackageReference Include="Newtonsoft.Json.Schema" Version="3.0.15" />
<PackageReference Include="Selenium.Support" Version="4.1.1" />
<PackageReference Include="Selenium.WebDriver" Version="4.1.1" />
<PackageReference Include="Selenium.WebDriver.ChromeDriver" Version="119.0.6045.10500" />
<PackageReference Include="Selenium.WebDriver.ChromeDriver" Version="121.0.6167.8500" />
<PackageReference Include="xunit" Version="2.6.6" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.6">
<PrivateAssets>all</PrivateAssets>
Expand Down
32 changes: 32 additions & 0 deletions BTCPayServer.Tests/SeleniumTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2332,10 +2332,27 @@ public async Task CanUsePOSKeypad()
s.Server.ActivateLightning();
await s.StartAsync();
await s.Server.EnsureChannelsSetup();

// Create users
var user = s.RegisterNewUser();
var userAccount = s.AsTestAccount();
s.GoToHome();
s.Logout();
s.GoToRegister();
s.RegisterNewUser(true);

// Setup store and associate user
s.CreateNewStore();
s.GoToStore();
s.AddLightningNode(LightningConnectionType.CLightning, false);
s.GoToStore(StoreNavPages.Users);
s.Driver.FindElement(By.Id("Email")).Clear();
s.Driver.FindElement(By.Id("Email")).SendKeys(user);
new SelectElement(s.Driver.FindElement(By.Id("Role"))).SelectByValue("Guest");
s.Driver.FindElement(By.Id("AddUser")).Click();
Assert.Contains("User added successfully", s.FindAlertMessage().Text);

// Setup POS
s.Driver.FindElement(By.Id("StoreNav-CreatePointOfSale")).Click();
s.Driver.FindElement(By.Id("AppName")).SendKeys(Guid.NewGuid().ToString());
s.Driver.FindElement(By.Id("Create")).Click();
Expand All @@ -2360,6 +2377,8 @@ public async Task CanUsePOSKeypad()
s.Driver.WaitForElement(By.ClassName("keypad"));

// basic checks
var keypadUrl = s.Driver.Url;
s.Driver.FindElement(By.Id("RecentTransactionsToggle"));
Assert.Contains("EUR", s.Driver.FindElement(By.Id("Currency")).Text);
Assert.Contains("0,00", s.Driver.FindElement(By.Id("Amount")).Text);
Assert.Equal("", s.Driver.FindElement(By.Id("Calculation")).Text);
Expand Down Expand Up @@ -2405,6 +2424,19 @@ public async Task CanUsePOSKeypad()
s.Driver.FindElement(By.Id("DetailsToggle")).Click();
s.Driver.WaitForElement(By.Id("PaymentDetails-TotalFiat"));
Assert.Contains("1 222,21 €", s.Driver.FindElement(By.Id("PaymentDetails-TotalFiat")).Text);

// Guest user can access recent transactions
s.GoToHome();
s.Logout();
s.LogIn(user, userAccount.RegisterDetails.Password);
s.GoToUrl(keypadUrl);
s.Driver.FindElement(By.Id("RecentTransactionsToggle"));
s.GoToHome();
s.Logout();

// Unauthenticated user can't access recent transactions
s.GoToUrl(keypadUrl);
s.Driver.ElementDoesNotExist(By.Id("RecentTransactionsToggle"));
}

[Fact]
Expand Down
9 changes: 4 additions & 5 deletions BTCPayServer/Security/CookieAuthorizationHandler.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using BTCPayServer.Abstractions.Constants;
using BTCPayServer.Abstractions.Contracts;
Expand Down Expand Up @@ -74,10 +73,10 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
// resolve from app
if (routeData.Values.TryGetValue("appId", out var vAppId) && vAppId is string appId)
{
app = await _appService.GetAppDataIfOwner(userId, appId);
app = await _appService.GetAppData(userId, appId);
if (storeId == null)
{
storeId = app?.StoreDataId ?? String.Empty;
storeId = app?.StoreDataId ?? string.Empty;
}
else if (app?.StoreDataId != storeId)
{
Expand All @@ -90,7 +89,7 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
paymentRequest = await _paymentRequestRepository.FindPaymentRequest(payReqId, userId);
if (storeId == null)
{
storeId = paymentRequest?.StoreDataId ?? String.Empty;
storeId = paymentRequest?.StoreDataId ?? string.Empty;
}
else if (paymentRequest?.StoreDataId != storeId)
{
Expand All @@ -103,7 +102,7 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
invoice = await _invoiceRepository.GetInvoice(invoiceId);
if (storeId == null)
{
storeId = invoice?.StoreId ?? String.Empty;
storeId = invoice?.StoreId ?? string.Empty;
}
else if (invoice?.StoreId != storeId)
{
Expand Down
24 changes: 20 additions & 4 deletions BTCPayServer/Services/Apps/AppService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,26 @@ public static ViewPointOfSaleViewModel.Item[] Parse(string template, bool includ
return null;
await using var ctx = _ContextFactory.CreateContext();
var app = await ctx.UserStore
.Include(store => store.StoreRole)
.Where(us => us.ApplicationUserId == userId && us.StoreRole.Permissions.Contains(Policies.CanModifyStoreSettings))
.SelectMany(us => us.StoreData.Apps.Where(a => a.Id == appId))
.FirstOrDefaultAsync();
.Include(store => store.StoreRole)
.Where(us => us.ApplicationUserId == userId && us.StoreRole.Permissions.Contains(Policies.CanModifyStoreSettings))
.SelectMany(us => us.StoreData.Apps.Where(a => a.Id == appId))
.FirstOrDefaultAsync();
if (app == null)
return null;
if (type != null && type != app.AppType)
return null;
return app;
}

public async Task<AppData?> GetAppData(string userId, string appId, string? type = null)
{
if (userId == null || appId == null)
return null;
await using var ctx = _ContextFactory.CreateContext();
var app = await ctx.UserStore
.Where(us => us.ApplicationUserId == userId && us.StoreData != null && us.StoreData.UserStores.Any(u => u.ApplicationUserId == userId))
.SelectMany(us => us.StoreData.Apps.Where(a => a.Id == appId))
.FirstOrDefaultAsync();
if (app == null)
return null;
if (type != null && type != app.AppType)
Expand Down
2 changes: 1 addition & 1 deletion BTCPayServer/Views/UIStores/StoreUsers.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
</select>
</div>
<div class="ms-3">
<button type="submit" role="button" class="btn btn-primary">Add User</button>
<button type="submit" role="button" class="btn btn-primary" id="AddUser">Add User</button>
</div>
</div>
</form>
Expand Down

0 comments on commit 4b03115

Please sign in to comment.