From a0807cf26bdb05d1a3a6c5494ce1cd446f8fe254 Mon Sep 17 00:00:00 2001 From: Jared Goodwin Date: Thu, 27 Jul 2023 09:49:23 -0700 Subject: [PATCH] Fix AuthComponentBase and tests. --- Server/Components/AuthComponentBase.cs | 58 ++++++++++++++----- Server/Components/AuthorizedIndex.razor | 5 +- .../Scripts/ScriptSchedules.razor.cs | 8 +-- Server/Components/TabControl/TabHeader.razor | 8 +-- Server/Hubs/CircuitConnection.cs | 21 ++++++- Server/Pages/DeviceDetails.razor.cs | 5 -- Server/Services/DataService.cs | 1 + Tests/Server.Tests/DataServiceTests.cs | 6 +- 8 files changed, 77 insertions(+), 35 deletions(-) diff --git a/Server/Components/AuthComponentBase.cs b/Server/Components/AuthComponentBase.cs index 371392a6c..42b922ed1 100644 --- a/Server/Components/AuthComponentBase.cs +++ b/Server/Components/AuthComponentBase.cs @@ -1,47 +1,73 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Components; -using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; using Remotely.Server.Services; using Remotely.Shared.Entities; using System; -using System.Diagnostics.CodeAnalysis; +using System.Threading; using System.Threading.Tasks; namespace Remotely.Server.Components; +[Authorize] public class AuthComponentBase : ComponentBase { + private readonly ManualResetEventSlim _initSignal = new(); private RemotelyUser? _user; private string? _userName; - protected override async Task OnInitializedAsync() - { - IsAuthenticated = await AuthService.IsAuthenticated(); - var userResult = await AuthService.GetUser(); - if (userResult.IsSuccess) - { - _user = userResult.Value; - _userName = userResult.Value.UserName ?? string.Empty; - } - await base.OnInitializedAsync(); - } - public bool IsAuthenticated { get; private set; } public bool IsUserSet => _user is not null; public RemotelyUser User { - get => _user ?? throw new InvalidOperationException("User has not been resolved yet."); + get + { + if (_initSignal.Wait(TimeSpan.FromSeconds(5)) && _user is not null) + { + return _user; + } + // This should never happen, since AuthBasedComponent is only + // used on components that require authentication. This was easier + // than making this explicitly nullable and refactoring everywhere. + Logger.LogError("Failed to resolve user."); + throw new InvalidOperationException("Failed to resolve user."); + } private set => _user = value; } public string UserName { - get => _userName ?? throw new InvalidOperationException("User has not been resolved yet."); + get + { + if (_initSignal.Wait(TimeSpan.FromSeconds(5)) && _userName is not null) + { + return _userName; + } + Logger.LogError("Failed to resolve user."); + throw new InvalidOperationException("Failed to resolve user."); + } private set => _userName = value; } [Inject] protected IAuthService AuthService { get; set; } = null!; + + [Inject] + private ILogger Logger { get; init; } = null!; + + + protected override async Task OnInitializedAsync() + { + IsAuthenticated = await AuthService.IsAuthenticated(); + var userResult = await AuthService.GetUser(); + if (userResult.IsSuccess) + { + _user = userResult.Value; + _userName = userResult.Value.UserName ?? string.Empty; + } + _initSignal.Set(); + await base.OnInitializedAsync(); + } } diff --git a/Server/Components/AuthorizedIndex.razor b/Server/Components/AuthorizedIndex.razor index b8a8fa586..ba0e9936c 100644 --- a/Server/Components/AuthorizedIndex.razor +++ b/Server/Components/AuthorizedIndex.razor @@ -20,7 +20,7 @@ protected override void OnAfterRender(bool firstRender) { - if (AppConfig.Require2FA && IsUserSet && !User.TwoFactorEnabled) + if (AppConfig.Require2FA && !User.TwoFactorEnabled) { NavManager.NavigateTo("/TwoFactorRequired"); } @@ -31,6 +31,9 @@ { await base.OnInitializedAsync(); + // This handles a weird edge case when the user has been + // deleted but still has an authentication cookie in their + // browser. if (IsAuthenticated == true && !IsUserSet) { await SignInManager.SignOutAsync(); diff --git a/Server/Components/Scripts/ScriptSchedules.razor.cs b/Server/Components/Scripts/ScriptSchedules.razor.cs index dacdb86e6..c7a2b9427 100644 --- a/Server/Components/Scripts/ScriptSchedules.razor.cs +++ b/Server/Components/Scripts/ScriptSchedules.razor.cs @@ -50,12 +50,12 @@ public partial class ScriptSchedules : AuthComponentBase private IToastService ToastService { get; set; } = null!; private bool CanModifySchedule => - _selectedSchedule.CreatorId == User?.Id || - User?.IsAdministrator == true; + _selectedSchedule.CreatorId == User.Id || + User.IsAdministrator; private bool CanDeleteSchedule => - _selectedSchedule.CreatorId == User?.Id || - User?.IsAdministrator == true; + _selectedSchedule.CreatorId == User.Id || + User.IsAdministrator; protected override async Task OnInitializedAsync() { diff --git a/Server/Components/TabControl/TabHeader.razor b/Server/Components/TabControl/TabHeader.razor index 80238cd3c..a21545c7f 100644 --- a/Server/Components/TabControl/TabHeader.razor +++ b/Server/Components/TabControl/TabHeader.razor @@ -32,13 +32,13 @@ throw new Exception("TabHeader must be contained in a TabControl."); } - if (!string.IsNullOrWhiteSpace(NavigationUri)) + if (Parent.ActiveTab == Name) { OnActivated?.Invoke(); } } - - private void SetActiveTab() + + private async Task SetActiveTab() { if (!string.IsNullOrWhiteSpace(NavigationUri)) { @@ -47,7 +47,7 @@ else { Parent?.SetActiveTab(this); - StateHasChanged(); + await InvokeAsync(StateHasChanged); OnActivated?.Invoke(); } } diff --git a/Server/Hubs/CircuitConnection.cs b/Server/Hubs/CircuitConnection.cs index 1bc1bc609..b37b564dc 100644 --- a/Server/Hubs/CircuitConnection.cs +++ b/Server/Hubs/CircuitConnection.cs @@ -85,6 +85,7 @@ public class CircuitConnection : CircuitHandler, ICircuitConnection private readonly ILogger _logger; private readonly IAgentHubSessionCache _agentSessionCache; private readonly IToastService _toastService; + private readonly ManualResetEventSlim _initSignal = new(); private RemotelyUser? _user; public CircuitConnection( @@ -120,8 +121,23 @@ public CircuitConnection( public RemotelyUser User { - get => _user ?? throw new InvalidOperationException("User has not been resolved yet."); - internal set => _user = value; + get + { + if (_initSignal.Wait(TimeSpan.FromSeconds(5)) && _user is not null) + { + return _user; + } + _logger.LogError("Failed to resolve user."); + throw new InvalidOperationException("Failed to resolve user."); + } + internal set + { + _user = value; + if (_user is not null) + { + _initSignal.Set(); + } + } } @@ -223,6 +239,7 @@ public override async Task OnCircuitOpenedAsync(Circuit circuit, CancellationTok } _user = userResult.Value; _circuitManager.TryAddConnection(ConnectionId, this); + _initSignal.Set(); } await base.OnCircuitOpenedAsync(circuit, cancellationToken); } diff --git a/Server/Pages/DeviceDetails.razor.cs b/Server/Pages/DeviceDetails.razor.cs index c3668ea4f..3de0b5a5c 100644 --- a/Server/Pages/DeviceDetails.razor.cs +++ b/Server/Pages/DeviceDetails.razor.cs @@ -171,11 +171,6 @@ private string GetTrimmedText(string source, int stringLength) return source[0..25] + "..."; } - private string GetTrimmedText(string[] source, int stringLength) - { - return GetTrimmedText(string.Join("", source), stringLength); - } - private Task HandleValidSubmit() { if (_device is null) diff --git a/Server/Services/DataService.cs b/Server/Services/DataService.cs index 3d1123618..9d3106fc9 100644 --- a/Server/Services/DataService.cs +++ b/Server/Services/DataService.cs @@ -1566,6 +1566,7 @@ public async Task> GetPendingScriptRuns(string deviceId) .AsNoTracking() .Include(x => x.ScriptRuns) .ThenInclude(x => x.Results) + .Include(x => x.ScriptResults) .FirstOrDefaultAsync(x => x.ID == deviceId); if (device is null) diff --git a/Tests/Server.Tests/DataServiceTests.cs b/Tests/Server.Tests/DataServiceTests.cs index 245ed3fe4..724b8fcc1 100644 --- a/Tests/Server.Tests/DataServiceTests.cs +++ b/Tests/Server.Tests/DataServiceTests.cs @@ -66,12 +66,12 @@ public async Task CreateDevice() }; // First call should create and return device. - var savedDevice = await _dataService.CreateDevice(deviceOptions); + var savedDevice = (await _dataService.CreateDevice(deviceOptions)).Value!; Assert.IsInstanceOfType(savedDevice, typeof(Device)); - // Second call with same DeviceUuid should return null; + // Second call with same DeviceUuid should fail. var secondSave = await _dataService.CreateDevice(deviceOptions); - Assert.IsNull(secondSave); + Assert.IsFalse(secondSave.IsSuccess); } [TestMethod]