Apps: Allow authenticated, non-owner users permissioned access (#5702)

Fixes #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.
This commit is contained in:
d11n 2024-01-25 13:00:33 +01:00 committed by GitHub
parent 086f713752
commit b96cfcd14d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 58 additions and 11 deletions

View file

@ -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>

View file

@ -2332,10 +2332,27 @@ namespace BTCPayServer.Tests
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();
@ -2360,6 +2377,8 @@ namespace BTCPayServer.Tests
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);
@ -2405,6 +2424,19 @@ namespace BTCPayServer.Tests
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]

View file

@ -1,5 +1,4 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using BTCPayServer.Abstractions.Constants;
using BTCPayServer.Abstractions.Contracts;
@ -74,10 +73,10 @@ namespace BTCPayServer.Security
// 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)
{
@ -90,7 +89,7 @@ namespace BTCPayServer.Security
paymentRequest = await _paymentRequestRepository.FindPaymentRequest(payReqId, userId);
if (storeId == null)
{
storeId = paymentRequest?.StoreDataId ?? String.Empty;
storeId = paymentRequest?.StoreDataId ?? string.Empty;
}
else if (paymentRequest?.StoreDataId != storeId)
{
@ -103,7 +102,7 @@ namespace BTCPayServer.Security
invoice = await _invoiceRepository.GetInvoice(invoiceId);
if (storeId == null)
{
storeId = invoice?.StoreId ?? String.Empty;
storeId = invoice?.StoreId ?? string.Empty;
}
else if (invoice?.StoreId != storeId)
{

View file

@ -371,10 +371,26 @@ namespace BTCPayServer.Services.Apps
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)

View file

@ -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>