From 9507888900551c07d59e161553d6422add38426f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20B=C3=BCchel?= Date: Thu, 1 Jun 2023 18:18:01 +0200 Subject: [PATCH] SEBWIN-674: Extended unit test coverage for third-party application logic. --- .../ApplicationFactoryTests.cs | 36 +++- .../ExternalApplicationTests.cs | 167 ++++++++++++++++++ ...eExamBrowser.Applications.UnitTests.csproj | 5 + .../ApplicationFactory.cs | 25 ++- .../ExternalApplication.cs | 45 ++--- .../ExternalApplicationInstance.cs | 26 +-- .../ExternalApplicationWindow.cs | 2 +- .../SafeExamBrowser.Applications.csproj | 4 + SafeExamBrowser.Client/CompositionRoot.cs | 2 +- .../Registry/RegistryValue.cs | 1 + 10 files changed, 257 insertions(+), 56 deletions(-) create mode 100644 SafeExamBrowser.Applications.UnitTests/ExternalApplicationTests.cs diff --git a/SafeExamBrowser.Applications.UnitTests/ApplicationFactoryTests.cs b/SafeExamBrowser.Applications.UnitTests/ApplicationFactoryTests.cs index 3215b9ba..08e5effd 100644 --- a/SafeExamBrowser.Applications.UnitTests/ApplicationFactoryTests.cs +++ b/SafeExamBrowser.Applications.UnitTests/ApplicationFactoryTests.cs @@ -13,6 +13,7 @@ using SafeExamBrowser.Applications.Contracts; using SafeExamBrowser.Logging.Contracts; using SafeExamBrowser.Monitoring.Contracts.Applications; using SafeExamBrowser.Settings.Applications; +using SafeExamBrowser.SystemComponents.Contracts.Registry; using SafeExamBrowser.WindowsApi.Contracts; namespace SafeExamBrowser.Applications.UnitTests @@ -24,6 +25,7 @@ namespace SafeExamBrowser.Applications.UnitTests private Mock logger; private Mock nativeMethods; private Mock processFactory; + private Mock registry; private ApplicationFactory sut; @@ -34,8 +36,9 @@ namespace SafeExamBrowser.Applications.UnitTests logger = new Mock(); nativeMethods = new Mock(); processFactory = new Mock(); + registry = new Mock(); - sut = new ApplicationFactory(applicationMonitor.Object, logger.Object, nativeMethods.Object, processFactory.Object); + sut = new ApplicationFactory(applicationMonitor.Object, logger.Object, nativeMethods.Object, processFactory.Object, registry.Object); } [TestMethod] @@ -54,12 +57,32 @@ namespace SafeExamBrowser.Applications.UnitTests Assert.IsInstanceOfType(application); } + [TestMethod] + public void MustCorrectlyReadPathFromRegistry() + { + var o = default(object); + var settings = new WhitelistApplication + { + DisplayName = "Windows Command Prompt", + ExecutableName = "cmd.exe", + }; + + var result = sut.TryCreate(settings, out var application); + + registry.Verify(r => r.TryRead(It.Is(s => s.Contains(RegistryValue.MachineHive.AppPaths_Key)), It.Is(s => s == "Path"), out o), Times.Once); + + Assert.AreEqual(FactoryResult.Success, result); + Assert.IsNotNull(application); + Assert.IsInstanceOfType(application); + } + [TestMethod] public void MustIndicateIfApplicationNotFound() { var settings = new WhitelistApplication { - ExecutableName = "some_random_application_which_does_not_exist_on_a_normal_system.exe" + ExecutableName = "some_random_application_which_does_not_exist_on_a_normal_system.exe", + ExecutablePath = "Some/Path/Which/Does/Not/Exist" }; var result = sut.TryCreate(settings, out var application); @@ -71,13 +94,10 @@ namespace SafeExamBrowser.Applications.UnitTests [TestMethod] public void MustFailGracefullyAndIndicateThatErrorOccurred() { - var settings = new WhitelistApplication - { - DisplayName = "Windows Command Prompt", - ExecutableName = "cmd.exe", - }; + var o = default(object); + var settings = new WhitelistApplication(); - logger.Setup(l => l.CloneFor(It.IsAny())).Throws(); + registry.Setup(r => r.TryRead(It.IsAny(), It.IsAny(), out o)).Throws(); var result = sut.TryCreate(settings, out var application); diff --git a/SafeExamBrowser.Applications.UnitTests/ExternalApplicationTests.cs b/SafeExamBrowser.Applications.UnitTests/ExternalApplicationTests.cs new file mode 100644 index 00000000..51e85bc8 --- /dev/null +++ b/SafeExamBrowser.Applications.UnitTests/ExternalApplicationTests.cs @@ -0,0 +1,167 @@ +/* + * Copyright (c) 2023 ETH Zürich, Educational Development and Technology (LET) + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using SafeExamBrowser.Core.Contracts.Resources.Icons; +using SafeExamBrowser.Logging.Contracts; +using SafeExamBrowser.Monitoring.Contracts.Applications; +using SafeExamBrowser.Monitoring.Contracts.Applications.Events; +using SafeExamBrowser.Settings.Applications; +using SafeExamBrowser.WindowsApi.Contracts; + +namespace SafeExamBrowser.Applications.UnitTests +{ + [TestClass] + public class ExternalApplicationTests + { + private Mock applicationMonitor; + private string executablePath; + private Mock logger; + private Mock nativeMethods; + private Mock processFactory; + private WhitelistApplication settings; + + private ExternalApplication sut; + + [TestInitialize] + public void Initialize() + { + applicationMonitor = new Mock(); + executablePath = @"C:\Some\Random\Path\Application.exe"; + logger = new Mock(); + nativeMethods = new Mock(); + processFactory = new Mock(); + settings = new WhitelistApplication(); + + logger.Setup(l => l.CloneFor(It.IsAny())).Returns(new Mock().Object); + + sut = new ExternalApplication(applicationMonitor.Object, executablePath, logger.Object, nativeMethods.Object, processFactory.Object, settings, 1); + } + + [TestMethod] + public void GetWindows_MustCorrectlyReturnOpenWindows() + { + var sync = new AutoResetEvent(false); + var process1 = new Mock(); + var process2 = new Mock(); + var openWindows = new List { new IntPtr(123), new IntPtr(234), new IntPtr(456), new IntPtr(345), new IntPtr(567), new IntPtr(789), }; + + nativeMethods.Setup(n => n.GetOpenWindows()).Returns(openWindows); + nativeMethods.Setup(n => n.GetProcessIdFor(It.Is(p => p == new IntPtr(234)))).Returns(1234); + nativeMethods.Setup(n => n.GetProcessIdFor(It.Is(p => p == new IntPtr(345)))).Returns(1234); + nativeMethods.Setup(n => n.GetProcessIdFor(It.Is(p => p == new IntPtr(567)))).Returns(5678); + process1.Setup(p => p.TryClose(It.IsAny())).Returns(false); + process1.Setup(p => p.TryKill(It.IsAny())).Returns(true); + process1.SetupGet(p => p.Id).Returns(1234); + process2.Setup(p => p.TryClose(It.IsAny())).Returns(true); + process2.SetupGet(p => p.Id).Returns(5678); + processFactory.Setup(f => f.StartNew(It.IsAny(), It.IsAny())).Returns(process1.Object); + + sut.WindowsChanged += () => sync.Set(); + sut.Initialize(); + sut.Start(); + + applicationMonitor.Raise(m => m.InstanceStarted += null, sut.Id, process2.Object); + + sync.WaitOne(); + sync.WaitOne(); + + var windows = sut.GetWindows(); + + Assert.AreEqual(3, windows.Count()); + Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(234))); + Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(345))); + Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(567))); + } + + [TestMethod] + public void Initialize_MustInitializeCorrectly() + { + settings.AutoStart = new Random().Next(2) == 1; + + sut.Initialize(); + + applicationMonitor.VerifyAdd(a => a.InstanceStarted += It.IsAny(), Times.Once); + + Assert.AreEqual(settings.AutoStart, sut.AutoStart); + Assert.AreEqual(executablePath, (sut.Icon as EmbeddedIconResource).FilePath); + Assert.AreEqual(settings.Id, settings.Id); + Assert.AreEqual(settings.DisplayName, sut.Name); + Assert.AreEqual(settings.Description ?? settings.DisplayName, sut.Tooltip); + } + + [TestMethod] + public void Start_MustCreateInstanceCorrectly() + { + settings.Arguments.Add("some_parameter"); + settings.Arguments.Add("another_parameter"); + settings.Arguments.Add("yet another parameter"); + + sut.Start(); + + processFactory.Verify(f => f.StartNew(executablePath, It.Is(args => args.All(a => settings.Arguments.Contains(a)))), Times.Once); + } + + [TestMethod] + public void Start_MustHandleFailureGracefully() + { + processFactory.Setup(f => f.StartNew(It.IsAny(), It.IsAny())).Throws(); + + sut.Start(); + + logger.Verify(l => l.Error(It.IsAny(), It.IsAny()), Times.AtLeastOnce); + processFactory.Verify(f => f.StartNew(It.IsAny(), It.IsAny()), Times.Once); + } + + [TestMethod] + public void Terminate_MustStopAllInstancesCorrectly() + { + var process1 = new Mock(); + var process2 = new Mock(); + + process1.Setup(p => p.TryClose(It.IsAny())).Returns(false); + process1.Setup(p => p.TryKill(It.IsAny())).Returns(true); + process1.SetupGet(p => p.Id).Returns(1234); + process2.Setup(p => p.TryClose(It.IsAny())).Returns(true); + process2.SetupGet(p => p.Id).Returns(5678); + processFactory.Setup(f => f.StartNew(It.IsAny(), It.IsAny())).Returns(process1.Object); + + sut.Initialize(); + sut.Start(); + + applicationMonitor.Raise(m => m.InstanceStarted += null, sut.Id, process2.Object); + sut.Terminate(); + + process1.Verify(p => p.TryClose(It.IsAny()), Times.AtLeastOnce); + process1.Verify(p => p.TryKill(It.IsAny()), Times.Once); + process2.Verify(p => p.TryClose(It.IsAny()), Times.Once); + process2.Verify(p => p.TryKill(It.IsAny()), Times.Never); + } + + [TestMethod] + public void Terminate_MustHandleFailureGracefully() + { + var process = new Mock(); + + process.Setup(p => p.TryClose(It.IsAny())).Throws(); + processFactory.Setup(f => f.StartNew(It.IsAny(), It.IsAny())).Returns(process.Object); + + sut.Initialize(); + sut.Start(); + sut.Terminate(); + + process.Verify(p => p.TryClose(It.IsAny()), Times.AtLeastOnce); + process.Verify(p => p.TryKill(It.IsAny()), Times.Never); + } + } +} diff --git a/SafeExamBrowser.Applications.UnitTests/SafeExamBrowser.Applications.UnitTests.csproj b/SafeExamBrowser.Applications.UnitTests/SafeExamBrowser.Applications.UnitTests.csproj index 65f35445..ad9cf941 100644 --- a/SafeExamBrowser.Applications.UnitTests/SafeExamBrowser.Applications.UnitTests.csproj +++ b/SafeExamBrowser.Applications.UnitTests/SafeExamBrowser.Applications.UnitTests.csproj @@ -83,6 +83,7 @@ + @@ -113,6 +114,10 @@ {30b2d907-5861-4f39-abad-c4abf1b3470e} SafeExamBrowser.Settings + + {903129c6-e236-493b-9ad6-c6a57f647a3a} + SafeExamBrowser.SystemComponents.Contracts + {7016f080-9aa5-41b2-a225-385ad877c171} SafeExamBrowser.WindowsApi.Contracts diff --git a/SafeExamBrowser.Applications/ApplicationFactory.cs b/SafeExamBrowser.Applications/ApplicationFactory.cs index 69cef224..1e74de1e 100644 --- a/SafeExamBrowser.Applications/ApplicationFactory.cs +++ b/SafeExamBrowser.Applications/ApplicationFactory.cs @@ -9,11 +9,11 @@ using System; using System.Collections.Generic; using System.IO; -using Microsoft.Win32; using SafeExamBrowser.Applications.Contracts; using SafeExamBrowser.Logging.Contracts; using SafeExamBrowser.Monitoring.Contracts.Applications; using SafeExamBrowser.Settings.Applications; +using SafeExamBrowser.SystemComponents.Contracts.Registry; using SafeExamBrowser.WindowsApi.Contracts; namespace SafeExamBrowser.Applications @@ -24,17 +24,20 @@ namespace SafeExamBrowser.Applications private readonly IModuleLogger logger; private readonly INativeMethods nativeMethods; private readonly IProcessFactory processFactory; + private readonly IRegistry registry; public ApplicationFactory( IApplicationMonitor applicationMonitor, IModuleLogger logger, INativeMethods nativeMethods, - IProcessFactory processFactory) + IProcessFactory processFactory, + IRegistry registry) { this.applicationMonitor = applicationMonitor; this.logger = logger; this.nativeMethods = nativeMethods; this.processFactory = processFactory; + this.registry = registry; } public FactoryResult TryCreate(WhitelistApplication settings, out IApplication application) @@ -71,8 +74,10 @@ namespace SafeExamBrowser.Applications private IApplication BuildApplication(string executablePath, WhitelistApplication settings) { + const int ONE_SECOND = 1000; + var applicationLogger = logger.CloneFor(settings.DisplayName); - var application = new ExternalApplication(applicationMonitor, executablePath, applicationLogger, nativeMethods, processFactory, settings); + var application = new ExternalApplication(applicationMonitor, executablePath, applicationLogger, nativeMethods, processFactory, settings, ONE_SECOND); return application; } @@ -131,19 +136,9 @@ namespace SafeExamBrowser.Applications private string QueryPathFromRegistry(WhitelistApplication settings) { - try + if (registry.TryRead($@"{RegistryValue.MachineHive.AppPaths_Key}\{settings.ExecutableName}", "Path", out var value)) { - using (var key = Registry.LocalMachine.OpenSubKey($@"SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{settings.ExecutableName}")) - { - if (key != null) - { - return key.GetValue("Path") as string; - } - } - } - catch (Exception e) - { - logger.Error($"Failed to query path in registry for '{settings.ExecutableName}'!", e); + return value as string; } return default; diff --git a/SafeExamBrowser.Applications/ExternalApplication.cs b/SafeExamBrowser.Applications/ExternalApplication.cs index 22f49cb3..95bd0849 100644 --- a/SafeExamBrowser.Applications/ExternalApplication.cs +++ b/SafeExamBrowser.Applications/ExternalApplication.cs @@ -23,13 +23,14 @@ namespace SafeExamBrowser.Applications { private readonly object @lock = new object(); - private IApplicationMonitor applicationMonitor; - private string executablePath; - private IModuleLogger logger; - private INativeMethods nativeMethods; - private IList instances; - private IProcessFactory processFactory; - private WhitelistApplication settings; + private readonly IApplicationMonitor applicationMonitor; + private readonly string executablePath; + private readonly IList instances; + private readonly IModuleLogger logger; + private readonly INativeMethods nativeMethods; + private readonly IProcessFactory processFactory; + private readonly WhitelistApplication settings; + private readonly int windowMonitoringInterval; public bool AutoStart { get; private set; } public IconResource Icon { get; private set; } @@ -45,7 +46,8 @@ namespace SafeExamBrowser.Applications IModuleLogger logger, INativeMethods nativeMethods, IProcessFactory processFactory, - WhitelistApplication settings) + WhitelistApplication settings, + int windowMonitoringInterval_ms) { this.applicationMonitor = applicationMonitor; this.executablePath = executablePath; @@ -54,6 +56,7 @@ namespace SafeExamBrowser.Applications this.instances = new List(); this.processFactory = processFactory; this.settings = settings; + this.windowMonitoringInterval = windowMonitoringInterval_ms; } public IEnumerable GetWindows() @@ -89,18 +92,6 @@ namespace SafeExamBrowser.Applications } } - private string[] BuildArguments() - { - var arguments = new List(); - - foreach (var argument in settings.Arguments) - { - arguments.Add(Environment.ExpandEnvironmentVariables(argument)); - } - - return arguments.ToArray(); - } - public void Terminate() { applicationMonitor.InstanceStarted -= ApplicationMonitor_InstanceStarted; @@ -153,12 +144,24 @@ namespace SafeExamBrowser.Applications WindowsChanged?.Invoke(); } + private string[] BuildArguments() + { + var arguments = new List(); + + foreach (var argument in settings.Arguments) + { + arguments.Add(Environment.ExpandEnvironmentVariables(argument)); + } + + return arguments.ToArray(); + } + private void InitializeInstance(IProcess process) { lock (@lock) { var instanceLogger = logger.CloneFor($"{Name} ({process.Id})"); - var instance = new ExternalApplicationInstance(Icon, instanceLogger, nativeMethods, process); + var instance = new ExternalApplicationInstance(Icon, instanceLogger, nativeMethods, process, windowMonitoringInterval); instance.Terminated += Instance_Terminated; instance.WindowsChanged += () => WindowsChanged?.Invoke(); diff --git a/SafeExamBrowser.Applications/ExternalApplicationInstance.cs b/SafeExamBrowser.Applications/ExternalApplicationInstance.cs index f1ea33b6..a83288a7 100644 --- a/SafeExamBrowser.Applications/ExternalApplicationInstance.cs +++ b/SafeExamBrowser.Applications/ExternalApplicationInstance.cs @@ -12,8 +12,8 @@ using System.Linq; using System.Timers; using SafeExamBrowser.Applications.Contracts; using SafeExamBrowser.Applications.Contracts.Events; -using SafeExamBrowser.Core.Contracts.Resources.Icons; using SafeExamBrowser.Applications.Events; +using SafeExamBrowser.Core.Contracts.Resources.Icons; using SafeExamBrowser.Logging.Contracts; using SafeExamBrowser.WindowsApi.Contracts; @@ -23,24 +23,32 @@ namespace SafeExamBrowser.Applications { private readonly object @lock = new object(); - private IconResource icon; - private ILogger logger; - private INativeMethods nativeMethods; - private IProcess process; + private readonly IconResource icon; + private readonly ILogger logger; + private readonly INativeMethods nativeMethods; + private readonly IProcess process; + private readonly int windowMonitoringInterval; + private readonly IList windows; + private Timer timer; - private IList windows; internal int Id { get; private set; } internal event InstanceTerminatedEventHandler Terminated; internal event WindowsChangedEventHandler WindowsChanged; - internal ExternalApplicationInstance(IconResource icon, ILogger logger, INativeMethods nativeMethods, IProcess process) + internal ExternalApplicationInstance( + IconResource icon, + ILogger logger, + INativeMethods nativeMethods, + IProcess process, + int windowMonitoringInterval_ms) { this.icon = icon; this.logger = logger; this.nativeMethods = nativeMethods; this.process = process; + this.windowMonitoringInterval = windowMonitoringInterval_ms; this.windows = new List(); } @@ -145,11 +153,9 @@ namespace SafeExamBrowser.Applications private void InitializeEvents() { - const int ONE_SECOND = 1000; - process.Terminated += Process_Terminated; - timer = new Timer(ONE_SECOND); + timer = new Timer(windowMonitoringInterval); timer.Elapsed += Timer_Elapsed; timer.Start(); } diff --git a/SafeExamBrowser.Applications/ExternalApplicationWindow.cs b/SafeExamBrowser.Applications/ExternalApplicationWindow.cs index 140ac74c..df1e30ce 100644 --- a/SafeExamBrowser.Applications/ExternalApplicationWindow.cs +++ b/SafeExamBrowser.Applications/ExternalApplicationWindow.cs @@ -16,7 +16,7 @@ namespace SafeExamBrowser.Applications { internal class ExternalApplicationWindow : IApplicationWindow { - private INativeMethods nativeMethods; + private readonly INativeMethods nativeMethods; public IntPtr Handle { get; } public IconResource Icon { get; private set; } diff --git a/SafeExamBrowser.Applications/SafeExamBrowser.Applications.csproj b/SafeExamBrowser.Applications/SafeExamBrowser.Applications.csproj index 1c748536..f06fad4e 100644 --- a/SafeExamBrowser.Applications/SafeExamBrowser.Applications.csproj +++ b/SafeExamBrowser.Applications/SafeExamBrowser.Applications.csproj @@ -82,6 +82,10 @@ {30b2d907-5861-4f39-abad-c4abf1b3470e} SafeExamBrowser.Settings + + {903129c6-e236-493b-9ad6-c6a57f647a3a} + SafeExamBrowser.SystemComponents.Contracts + {7016f080-9aa5-41b2-a225-385ad877c171} SafeExamBrowser.WindowsApi.Contracts diff --git a/SafeExamBrowser.Client/CompositionRoot.cs b/SafeExamBrowser.Client/CompositionRoot.cs index 52543c1f..a4b06926 100644 --- a/SafeExamBrowser.Client/CompositionRoot.cs +++ b/SafeExamBrowser.Client/CompositionRoot.cs @@ -109,7 +109,7 @@ namespace SafeExamBrowser.Client var processFactory = new ProcessFactory(ModuleLogger(nameof(ProcessFactory))); var applicationMonitor = new ApplicationMonitor(TWO_SECONDS, ModuleLogger(nameof(ApplicationMonitor)), nativeMethods, processFactory); - var applicationFactory = new ApplicationFactory(applicationMonitor, ModuleLogger(nameof(ApplicationFactory)), nativeMethods, processFactory); + var applicationFactory = new ApplicationFactory(applicationMonitor, ModuleLogger(nameof(ApplicationFactory)), nativeMethods, processFactory, new Registry(ModuleLogger(nameof(Registry)))); var displayMonitor = new DisplayMonitor(ModuleLogger(nameof(DisplayMonitor)), nativeMethods, systemInfo); var explorerShell = new ExplorerShell(ModuleLogger(nameof(ExplorerShell)), nativeMethods); var fileSystemDialog = BuildFileSystemDialog(); diff --git a/SafeExamBrowser.SystemComponents.Contracts/Registry/RegistryValue.cs b/SafeExamBrowser.SystemComponents.Contracts/Registry/RegistryValue.cs index ff2548da..7ca03fb2 100644 --- a/SafeExamBrowser.SystemComponents.Contracts/Registry/RegistryValue.cs +++ b/SafeExamBrowser.SystemComponents.Contracts/Registry/RegistryValue.cs @@ -19,6 +19,7 @@ namespace SafeExamBrowser.SystemComponents.Contracts.Registry /// public static class MachineHive { + public const string AppPaths_Key = @"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths"; public const string EaseOfAccess_Key = @"HKEY_LOCAL_MACHINE\Software\Microsoft\Windows NT\CurrentVersion\Image File Execution Options\Utilman.exe"; public const string EaseOfAccess_Name = "Debugger"; }