From 7bdb136c276f53a4c24b01d98f8bcb2de6f52254 Mon Sep 17 00:00:00 2001 From: Kukks Date: Tue, 30 Jul 2024 15:56:45 +0200 Subject: [PATCH] switch to a long for device identifier --- BTCPayApp.CommonServer/IBTCPayAppHubClient.cs | 2 +- BTCPayServer/App/API/VSSController.cs | 93 ++++++++++--------- BTCPayServer/App/BTCPayAppHub.cs | 2 +- BTCPayServer/App/BTCPayAppState.cs | 28 ++++-- 4 files changed, 70 insertions(+), 55 deletions(-) diff --git a/BTCPayApp.CommonServer/IBTCPayAppHubClient.cs b/BTCPayApp.CommonServer/IBTCPayAppHubClient.cs index bae9993bf..989fd1e5e 100644 --- a/BTCPayApp.CommonServer/IBTCPayAppHubClient.cs +++ b/BTCPayApp.CommonServer/IBTCPayAppHubClient.cs @@ -27,7 +27,7 @@ public interface IBTCPayAppHubClient //methods available on the hub in the server public interface IBTCPayAppHubServer { - Task DeviceMasterSignal(string deviceIdentifier, bool active); + Task DeviceMasterSignal(long deviceIdentifier, bool active); Task> Pair(PairRequest request); Task Handshake(AppHandshake request); diff --git a/BTCPayServer/App/API/VSSController.cs b/BTCPayServer/App/API/VSSController.cs index 584e18f0c..8b698437b 100644 --- a/BTCPayServer/App/API/VSSController.cs +++ b/BTCPayServer/App/API/VSSController.cs @@ -15,6 +15,8 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.Extensions.Logging; using VSSProto; namespace BTCPayServer.App.API; @@ -29,20 +31,21 @@ public class VSSController : Controller, IVSSAPI private readonly ApplicationDbContextFactory _dbContextFactory; private readonly UserManager _userManager; private readonly BTCPayAppState _appState; + private readonly ILogger _logger; public VSSController(ApplicationDbContextFactory dbContextFactory, - UserManager userManager, BTCPayAppState appState) + UserManager userManager, BTCPayAppState appState, ILogger logger) { _dbContextFactory = dbContextFactory; _userManager = userManager; _appState = appState; + _logger = logger; } [HttpPost(HttpVSSAPIClient.GET_OBJECT)] [MediaTypeConstraint("application/octet-stream")] public async Task GetObjectAsync(GetObjectRequest request, CancellationToken cancellationToken) { - var userId = _userManager.GetUserId(User); await using var dbContext = _dbContextFactory.CreateContext(); var store = await dbContext.AppStorageItems.SingleOrDefaultAsync(data => @@ -74,79 +77,79 @@ public class VSSController : Controller, IVSSAPI private bool VerifyGlobalVersion(long globalVersion) { - var userId = _userManager.GetUserId(User); - var conn = _appState.Connections.SingleOrDefault(pair => pair.Value.Master && pair.Value.UserId == userId); + var conn = _appState.Connections.SingleOrDefault(pair => pair.Value.Master && pair.Value.UserId == userId); if (conn.Key == null) { return false; } - // This has a high collision rate, but we're not expecting something insane here since we have auth and other checks in place. - return globalVersion == conn.Value.DeviceIdentifier.GetHashCode(); + + return globalVersion == conn.Value.DeviceIdentifier; } [HttpPost(HttpVSSAPIClient.PUT_OBJECTS)] [MediaTypeConstraint("application/octet-stream")] public async Task PutObjectAsync(PutObjectRequest request, CancellationToken cancellationToken) { - if (!VerifyGlobalVersion(request.GlobalVersion)) return SetResult(BadRequest(new ErrorResponse() { ErrorCode = ErrorCode.ConflictException, Message = "Global version mismatch" })); - + var userId = _userManager.GetUserId(User); - + await using var dbContext = _dbContextFactory.CreateContext(); - - await using var dbContextTransaction = await dbContext.Database.BeginTransactionAsync(cancellationToken); - try + return await dbContext.Database.CreateExecutionStrategy().ExecuteAsync(async async => { - if (request.TransactionItems.Any()) + await using var dbContextTransaction = await dbContext.Database.BeginTransactionAsync(cancellationToken); + try { - var items = request.TransactionItems.Select(data => new AppStorageItemData() + if (request.TransactionItems.Any()) { - Key = data.Key, Value = data.Value.ToByteArray(), UserId = userId, Version = data.Version - }); - await dbContext.AppStorageItems.AddRangeAsync(items, cancellationToken); + var items = request.TransactionItems.Select(data => new AppStorageItemData() + { + Key = data.Key, Value = data.Value.ToByteArray(), UserId = userId, Version = data.Version + }); + await dbContext.AppStorageItems.AddRangeAsync(items, cancellationToken); + } + + if (request.DeleteItems.Any()) + { + var deleteQuery = request.DeleteItems.Aggregate( + dbContext.AppStorageItems.Where(data => data.UserId == userId), + (current, key) => current.Where(data => data.Key == key.Key && data.Version == key.Version)); + await deleteQuery.ExecuteDeleteAsync(cancellationToken: cancellationToken); + } + + await dbContext.SaveChangesAsync(cancellationToken); + await dbContextTransaction.CommitAsync(cancellationToken); + + return new PutObjectResponse(); } - - if (request.DeleteItems.Any()) + catch (Exception e) { - var deleteQuery = request.DeleteItems.Aggregate( - dbContext.AppStorageItems.Where(data => data.UserId == userId), - (current, key) => current.Where(data => data.Key == key.Key && data.Version == key.Version)); - await deleteQuery.ExecuteDeleteAsync(cancellationToken: cancellationToken); + _logger.LogError(e, "Error while processing transaction"); + await dbContextTransaction.RollbackAsync(cancellationToken); + return SetResult(BadRequest(new ErrorResponse() + { + ErrorCode = ErrorCode.ConflictException, Message = e.Message + })); } - - await dbContext.SaveChangesAsync(cancellationToken); - await dbContextTransaction.CommitAsync(cancellationToken); - } - catch (Exception e) - { - await dbContextTransaction.RollbackAsync(cancellationToken); - return SetResult(BadRequest(new ErrorResponse() - { - ErrorCode = ErrorCode.ConflictException, Message = e.Message - })); - } - - - return new PutObjectResponse(); + }, cancellationToken); } [HttpPost(HttpVSSAPIClient.DELETE_OBJECT)] [MediaTypeConstraint("application/octet-stream")] - public async Task DeleteObjectAsync(DeleteObjectRequest request, CancellationToken cancellationToken) + public async Task DeleteObjectAsync(DeleteObjectRequest request, + CancellationToken cancellationToken) { - - var userId = _userManager.GetUserId(User); await using var dbContext = _dbContextFactory.CreateContext(); var store = await dbContext.AppStorageItems .Where(data => data.Key == request.KeyValue.Key && data.UserId == userId && - data.Version == request.KeyValue.Version).ExecuteDeleteAsync(cancellationToken: cancellationToken); + data.Version == request.KeyValue.Version) + .ExecuteDeleteAsync(cancellationToken: cancellationToken); return store == 0 ? SetResult( new NotFoundObjectResult(new ErrorResponse() @@ -157,13 +160,15 @@ public class VSSController : Controller, IVSSAPI } [HttpPost(HttpVSSAPIClient.LIST_KEY_VERSIONS)] - public async Task ListKeyVersionsAsync(ListKeyVersionsRequest request, CancellationToken cancellationToken) + public async Task ListKeyVersionsAsync(ListKeyVersionsRequest request, + CancellationToken cancellationToken) { var userId = _userManager.GetUserId(User); await using var dbContext = _dbContextFactory.CreateContext(); var items = await dbContext.AppStorageItems .Where(data => data.UserId == userId) - .Select(data => new KeyValue() {Key = data.Key, Version = data.Version}).ToListAsync(cancellationToken: cancellationToken); + .Select(data => new KeyValue() {Key = data.Key, Version = data.Version}) + .ToListAsync(cancellationToken: cancellationToken); return new ListKeyVersionsResponse {KeyVersions = {items}}; } } diff --git a/BTCPayServer/App/BTCPayAppHub.cs b/BTCPayServer/App/BTCPayAppHub.cs index 852be0953..4655f39e1 100644 --- a/BTCPayServer/App/BTCPayAppHub.cs +++ b/BTCPayServer/App/BTCPayAppHub.cs @@ -300,7 +300,7 @@ public class BTCPayAppHub : Hub, IBTCPayAppHubServer await _appState.InvoiceUpdate(identifier, lightningInvoice); } - public async Task DeviceMasterSignal(string deviceIdentifier, bool active) + public async Task DeviceMasterSignal(long deviceIdentifier, bool active) { return await _appState.DeviceMasterSignal(Context.ConnectionId,deviceIdentifier,active); } diff --git a/BTCPayServer/App/BTCPayAppState.cs b/BTCPayServer/App/BTCPayAppState.cs index d08381731..aae93f114 100644 --- a/BTCPayServer/App/BTCPayAppState.cs +++ b/BTCPayServer/App/BTCPayAppState.cs @@ -26,7 +26,7 @@ namespace BTCPayServer.Controllers; public record ConnectedInstance( string UserId, - string DeviceIdentifier, + long? DeviceIdentifier, bool Master, // string ProvidedAccessKey, HashSet Groups); @@ -45,7 +45,7 @@ public class BTCPayAppState : IHostedService private DerivationSchemeParser _derivationSchemeParser; - public ConcurrentDictionary Connections { get; set; } + public ConcurrentDictionary Connections { get; set; } = new(); private CancellationTokenSource? _cts; @@ -266,24 +266,29 @@ public class BTCPayAppState : IHostedService } - public async Task DeviceMasterSignal(string contextConnectionId, string deviceIdentifier, bool active) + public async Task DeviceMasterSignal(string contextConnectionId, long deviceIdentifier, bool active) { if (!Connections.TryGetValue(contextConnectionId, out var connectedInstance)) { + _logger.LogWarning("DeviceMasterSignal called on non existing connection"); return false; } - if(!string.IsNullOrEmpty(connectedInstance.DeviceIdentifier) && connectedInstance.DeviceIdentifier != deviceIdentifier) + if(connectedInstance.DeviceIdentifier != null && connectedInstance.DeviceIdentifier != deviceIdentifier) { + _logger.LogWarning("DeviceMasterSignal called with different device identifier"); return false; } - if(string.IsNullOrEmpty(connectedInstance.DeviceIdentifier)) + if(connectedInstance.DeviceIdentifier == null) { - Connections[contextConnectionId] = connectedInstance with {DeviceIdentifier = deviceIdentifier}; + _logger.LogInformation("DeviceMasterSignal called with device identifier {deviceIdentifier}", deviceIdentifier); + connectedInstance = connectedInstance with {DeviceIdentifier = deviceIdentifier}; + Connections[contextConnectionId] = connectedInstance; } if(connectedInstance.Master == active) { + _logger.LogInformation("DeviceMasterSignal called with same active state"); return true; } else if (active) @@ -291,17 +296,22 @@ public class BTCPayAppState : IHostedService //check if there is any other master connection with the same user id if (Connections.Values.Any(c => c.UserId == connectedInstance.UserId && c.Master)) { + _logger.LogWarning("DeviceMasterSignal called with active state but there is already a master connection"); return false; } else { - Connections[contextConnectionId] = connectedInstance with {Master = true}; + _logger.LogInformation("DeviceMasterSignal called with active state"); + connectedInstance = connectedInstance with {Master = true}; + Connections[contextConnectionId] = connectedInstance; return true; } } else { - Connections[contextConnectionId] = connectedInstance with {Master = false}; + _logger.LogInformation("DeviceMasterSignal called with inactive state"); + connectedInstance = connectedInstance with {Master = false}; + Connections[contextConnectionId] = connectedInstance; return true; } @@ -319,7 +329,7 @@ public class BTCPayAppState : IHostedService public async Task Connected(string contextConnectionId, string userId) { - Connections.TryAdd(contextConnectionId, new ConnectedInstance(userId, string.Empty, false, new HashSet())); + Connections.TryAdd(contextConnectionId, new ConnectedInstance(userId, null, false, new HashSet())); if (_nodeInfo.Length > 0) await _hubContext.Clients.Client(contextConnectionId).NotifyServerNode(_nodeInfo);