Merge pull request #695 from immense/tech/refactor-authcomponentbase

Refactor AuthComponentBase.

Server/Auth/OrganizationAdminRequirementHandler.cs

@@ -1,29 +1,33 @@
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Identity;
+using Remotely.Server.Services;
 using Remotely.Shared.Entities;
 using System.Threading.Tasks;
 
 namespace Remotely.Server.Auth;
 
 public class OrganizationAdminRequirementHandler : AuthorizationHandler<OrganizationAdminRequirement>
 {
-    private readonly UserManager<RemotelyUser> _userManager;
+    private readonly IDataService _dataService;
 
-    public OrganizationAdminRequirementHandler(UserManager<RemotelyUser> userManager)
+    public OrganizationAdminRequirementHandler(IDataService dataService)
     {
-        _userManager = userManager;
+        _dataService = dataService;
     }
 
     protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, OrganizationAdminRequirement requirement)
     {
-        if (context.User.Identity?.IsAuthenticated != true)
+        if (context.User.Identity?.IsAuthenticated != true ||
+            string.IsNullOrWhiteSpace(context.User.Identity.Name))
         {
             context.Fail();
             return;
         }
 
-        var user = await _userManager.GetUserAsync(context.User);
-        if (user?.IsAdministrator != true)
+        var userResult = await _dataService.GetUserByName(context.User.Identity.Name);
+
+        if (!userResult.IsSuccess ||
+            !userResult.Value.IsAdministrator)
         {
             context.Fail();
             return;

Server/Auth/ServerAdminRequirementHandler.cs

@@ -2,35 +2,39 @@
 
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Identity;
+using Remotely.Server.Services;
 using Remotely.Shared.Entities;
 using System.Threading.Tasks;
 
 namespace Remotely.Server.Auth;
 
 public class ServerAdminRequirementHandler : AuthorizationHandler<ServerAdminRequirement>
 {
-    private readonly UserManager<RemotelyUser> _userManager;
+    private readonly IDataService _dataService;
 
-    public ServerAdminRequirementHandler(UserManager<RemotelyUser> userManager)
+    public ServerAdminRequirementHandler(IDataService dataService)
     {
-        _userManager = userManager;
+        _dataService = dataService;
     }
 
     protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, ServerAdminRequirement requirement)
     {
-        if (context.User.Identity?.IsAuthenticated != true)
+        if (context.User.Identity?.IsAuthenticated != true ||
+            string.IsNullOrWhiteSpace(context.User.Identity.Name))
         {
             context.Fail();
             return;
         }
 
-        var user = await _userManager.GetUserAsync(context.User);
-        if (user?.IsServerAdmin != true)
+        var userResult = await _dataService.GetUserByName(context.User.Identity.Name);
+
+        if (!userResult.IsSuccess ||
+            !userResult.Value.IsServerAdmin)
         {
             context.Fail();
             return;
         }
-        
+
         context.Succeed(requirement);
     }
 }

Server/Auth/TwoFactorRequiredHandler.cs

@@ -11,21 +11,25 @@ namespace Remotely.Server.Auth;
 
 public class TwoFactorRequiredHandler : AuthorizationHandler<TwoFactorRequiredRequirement>
 {
-    private readonly UserManager<RemotelyUser> _userManager;
+    private readonly IDataService _dataService;
     private readonly IApplicationConfig _appConfig;
 
-    public TwoFactorRequiredHandler(UserManager<RemotelyUser> userManager, IApplicationConfig appConfig)
+    public TwoFactorRequiredHandler(IDataService dataService, IApplicationConfig appConfig)
     {
-        _userManager = userManager;
+        _dataService = dataService;
         _appConfig = appConfig;
     }
 
     protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, TwoFactorRequiredRequirement requirement)
     {
-        if (context.User.Identity?.IsAuthenticated == true && _appConfig.Require2FA)
+        if (context.User.Identity?.IsAuthenticated == true &&
+            context.User.Identity.Name is not null &&
+            _appConfig.Require2FA)
         {
-            var user = await _userManager.GetUserAsync(context.User);
-            if (user?.TwoFactorEnabled != true)
+            var userResult = await _dataService.GetUserByName(context.User.Identity.Name);
+
+            if (!userResult.IsSuccess ||
+                !userResult.Value.TwoFactorEnabled)
             {
                 context.Fail();
                 return;

Server/Components/AlertsFrame.razor

@@ -63,10 +63,8 @@
     {
         await base.OnInitializedAsync();
 
-        if (IsAuthenticated)
-        {
-            GetAlerts();
-        }
+        EnsureUserSet();
+        GetAlerts();
     }
 
     private async Task ClearAlert(Alert alert)
@@ -77,12 +75,14 @@
 
     private async Task ClearAllAlerts()
     {
+        EnsureUserSet();
         await DataService.DeleteAllAlerts(User.OrganizationID, User.UserName);
         _alerts.Clear();
     }
 
     private void GetAlerts()
     {
+        EnsureUserSet();
         _alerts.Clear();
         var alerts = DataService.GetAlerts(User.Id);
         if (alerts.Any())

Server/Components/AuthComponentBase.cs

@@ -4,6 +4,7 @@
 using Remotely.Server.Services;
 using Remotely.Shared.Entities;
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -12,62 +13,34 @@ namespace Remotely.Server.Components;
 [Authorize]
 public class AuthComponentBase : ComponentBase
 {
-    private readonly ManualResetEventSlim _initSignal = new();
-    private RemotelyUser? _user;
-    private string? _userName;
+    [Inject]
+    protected IAuthService AuthService { get; set; } = null!;
 
-    public bool IsAuthenticated { get; private set; }
+    protected RemotelyUser? User { get; private set; }
 
-    public bool IsUserSet => _user is not null;
+    protected string? UserName => User?.UserName;
 
-    public RemotelyUser User
+    [MemberNotNull(nameof(User), nameof(UserName))]
+    protected void EnsureUserSet()
     {
-        get
+        if (User is null)
         {
-            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.");
+            throw new InvalidOperationException("User has not been set.");
         }
-        private set => _user = value;
-    }
 
-    public string UserName
-    {
-        get
+        if (UserName is null)
         {
-            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.");
+            throw new InvalidOperationException("UserName has not been set.");
         }
-        private set => _userName = value;
     }
 
-    [Inject]
-    protected IAuthService AuthService { get; set; } = null!;
-
-    [Inject]
-    private ILogger<AuthComponentBase> 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;
+            User = userResult.Value;
         }
-        _initSignal.Set();
         await base.OnInitializedAsync();
     }
 }

Server/Components/AuthorizedIndex.razor

@@ -18,23 +18,27 @@
 
 @code {
 
-    protected override void OnAfterRender(bool firstRender)
+    protected override async Task OnAfterRenderAsync(bool firstRender)
     {
-        if (AppConfig.Require2FA && !User.TwoFactorEnabled)
+        if (User is not null &&
+            AppConfig.Require2FA && 
+            !User.TwoFactorEnabled)
         {
             NavManager.NavigateTo("/TwoFactorRequired");
         }
-        base.OnAfterRender(firstRender);
+        await base.OnAfterRenderAsync(firstRender);
     }
 
     protected override async Task OnInitializedAsync()
     {
         await base.OnInitializedAsync();
 
+        var isAuthenticated = await AuthService.IsAuthenticated();
+        var userResult = await AuthService.GetUser();
         // 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)
+        if (isAuthenticated == true && !userResult.IsSuccess)
         {
             await SignInManager.SignOutAsync();
             NavManager.NavigateTo("/");

Server/Components/Devices/DeviceCard.razor.cs

@@ -79,6 +79,7 @@ public void Dispose()
     protected override async Task OnInitializedAsync()
     {
         await base.OnInitializedAsync();
+        EnsureUserSet();
         _theme = await AppState.GetEffectiveTheme();
         _currentVersion = UpgradeService.GetCurrentVersion();
         _deviceGroups = DataService.GetDeviceGroups(UserName);
@@ -180,6 +181,7 @@ private void HandleHeaderClick()
     }
     private async Task HandleValidSubmit()
     {
+        EnsureUserSet();
         if (!DataService.DoesUserHaveAccessToDevice(Device.ID, User))
         {
             ToastService.ShowToast("Unauthorized.", classString: "bg-warning");
@@ -199,6 +201,8 @@ await DataService.UpdateDevice(Device.ID,
 
     private async Task OnFileInputChanged(InputFileChangeEventArgs args)
     {
+        EnsureUserSet();
+
         ToastService.ShowToast("File upload started.");
 
         var fileId = await DataService.AddSharedFile(args.File, User.OrganizationID, OnFileInputProgress);

Server/Components/Devices/DevicesFrame.razor.cs

@@ -74,6 +74,8 @@ protected override async Task OnInitializedAsync()
     {
         await base.OnInitializedAsync();
 
+        EnsureUserSet();
+
         CircuitConnection.MessageReceived += CircuitConnection_MessageReceived;
         AppState.PropertyChanged += AppState_PropertyChanged;
 
@@ -283,6 +285,8 @@ private void HandleRefreshClicked()
 
     private void LoadDevices()
     {
+        EnsureUserSet();
+
         lock (_devicesLock)
         {
             _allDevices.Clear();
@@ -346,6 +350,8 @@ private void ToggleSortDirection()
 
     private async Task WakeDevices()
     {
+        EnsureUserSet();
+
         var offlineDevices = DataService
            .GetDevicesForUser(UserName)
            .Where(x => !x.IsOnline);

Server/Components/Devices/Terminal.razor.cs

@@ -64,6 +64,8 @@ private string InputText
     private EventCallback<SavedScript> RunQuickScript =>
         EventCallback.Factory.Create<SavedScript>(this, async script =>
         {
+            EnsureUserSet();
+
             var scriptRun = new ScriptRun
             {
                 OrganizationID = User.OrganizationID,
@@ -285,6 +287,8 @@ private async Task ShowAllCompletions()
 
     private async Task ShowQuickScripts()
     {
+        EnsureUserSet();
+
         var quickScripts = await DataService.GetQuickScripts(User.Id);
         if (quickScripts?.Any() != true)
         {
@@ -344,6 +348,8 @@ private void ToggleTerminalOpen()
     }
     private bool TryMatchShellShortcuts()
     {
+        EnsureUserSet();
+
         var currentText = InputText?.Trim()?.ToLower();
 
         if (string.IsNullOrWhiteSpace(currentText))

Server/Components/Scripts/RunScript.razor.cs

@@ -63,7 +63,7 @@ protected override void OnAfterRender(bool firstRender)
     protected override async Task OnInitializedAsync()
     {
         await base.OnInitializedAsync();
-
+        EnsureUserSet();
         _deviceGroups = DataService.GetDeviceGroups(UserName);
         _devices = DataService
             .GetDevicesForUser(UserName)
@@ -107,6 +107,8 @@ private void DeviceSelectedChanged(ChangeEventArgs args, Device device)
 
     private async Task ExecuteScript()
     {
+        EnsureUserSet();
+
         if (_selectedScript is null)
         {
             ToastService.ShowToast("You must select a script.", classString: "bg-warning");
@@ -163,6 +165,8 @@ private async Task ExecuteScript()
 
     private async Task ScriptSelected(ScriptTreeNode viewModel)
     {
+        EnsureUserSet();
+
         if (viewModel.Script is not null)
         {
             var scriptResult = await DataService.GetSavedScript(User.Id, viewModel.Script.Id);