From 550568dda6543eca60b1d0b09d770a0bebeb4395 Mon Sep 17 00:00:00 2001 From: elitsa Date: Tue, 28 Jan 2020 13:48:44 +0100 Subject: [PATCH] Creating an interface for FilePermissionHelper, making the implementation (in Umbraco.Web) singleton and adding the interface to Umbraco.Abstractions. Changing static methods in FilePermissionHelper to instance ones and fixing old way of referencing those methods. Removing the usage of Current in FolderAndFilePermissionsCheck. Fixing tests - mocking the newly created IFilePermissionHelper --- .../FolderAndFilePermissionsCheck.cs | 30 +++++++------- .../Install/IFilePermissionHelper.cs | 16 ++++++++ .../PublishedContent/NuCacheChildrenTests.cs | 6 ++- .../PublishedContent/NuCacheTests.cs | 6 ++- .../Scoping/ScopedNuCacheTests.cs | 5 ++- .../ContentTypeServiceVariantsTests.cs | 6 ++- .../Install/FilePermissionHelper.cs | 39 ++++++++++--------- .../InstallSteps/FilePermissionsStep.cs | 8 +++- .../NuCache/PublishedSnapshotService.cs | 8 +++- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 3 ++ 10 files changed, 87 insertions(+), 40 deletions(-) rename src/{Umbraco.Web => Umbraco.Abstractions}/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs (86%) create mode 100644 src/Umbraco.Abstractions/Install/IFilePermissionHelper.cs diff --git a/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs b/src/Umbraco.Abstractions/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs similarity index 86% rename from src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs rename to src/Umbraco.Abstractions/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs index f11cd9b553..6aeb83d61c 100644 --- a/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs +++ b/src/Umbraco.Abstractions/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs @@ -2,10 +2,10 @@ using System.Collections.Generic; using System.Linq; using Umbraco.Core; -using Umbraco.Web.Composing; -using Umbraco.Core.Services; -using Umbraco.Web.Install; using Umbraco.Core.Configuration; +using Umbraco.Core.Install; +using Umbraco.Core.IO; +using Umbraco.Core.Services; namespace Umbraco.Web.HealthCheck.Checks.Permissions { @@ -30,11 +30,15 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions { private readonly ILocalizedTextService _textService; private readonly IGlobalSettings _globalSettings; + private readonly IFilePermissionHelper _filePermissionHelper; + private readonly IIOHelper _ioHelper; - public FolderAndFilePermissionsCheck(ILocalizedTextService textService, IGlobalSettings globalSettings) + public FolderAndFilePermissionsCheck(ILocalizedTextService textService, IGlobalSettings globalSettings, IFilePermissionHelper filePermissionHelper, IIOHelper ioHelper) { _textService = textService; _globalSettings = globalSettings; + _filePermissionHelper = filePermissionHelper; + _ioHelper = ioHelper; } /// @@ -84,15 +88,15 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions }; // Run checks for required and optional paths for modify permission - var requiredPathCheckResult = FilePermissionHelper.EnsureDirectories( + var requiredPathCheckResult = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out var requiredFailedPaths); - var optionalPathCheckResult = FilePermissionHelper.EnsureDirectories( + var optionalPathCheckResult = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out var optionalFailedPaths); //now check the special folders - var requiredPathCheckResult2 = FilePermissionHelper.EnsureDirectories( + var requiredPathCheckResult2 = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Required), out var requiredFailedPaths2, writeCausesRestart:true); - var optionalPathCheckResult2 = FilePermissionHelper.EnsureDirectories( + var optionalPathCheckResult2 = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Optional), out var optionalFailedPaths2, writeCausesRestart: true); requiredPathCheckResult = requiredPathCheckResult && requiredPathCheckResult2; @@ -117,18 +121,18 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions // Run checks for required and optional paths for modify permission IEnumerable requiredFailedPaths; IEnumerable optionalFailedPaths; - var requiredPathCheckResult = FilePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out requiredFailedPaths); - var optionalPathCheckResult = FilePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out optionalFailedPaths); + var requiredPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out requiredFailedPaths); + var optionalPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out optionalFailedPaths); return GetStatus(requiredPathCheckResult, requiredFailedPaths, optionalPathCheckResult, optionalFailedPaths, PermissionCheckFor.File); } - private static string[] GetPathsToCheck(Dictionary pathsToCheck, + private string[] GetPathsToCheck(Dictionary pathsToCheck, PermissionCheckRequirement requirement) { return pathsToCheck .Where(x => x.Value == requirement) - .Select(x => Current.IOHelper.MapPath(x.Key)) + .Select(x => _ioHelper.MapPath(x.Key)) .OrderBy(x => x) .ToArray(); } @@ -168,7 +172,7 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions private string GetMessageForPathCheckFailure(string messageKey, IEnumerable failedPaths) { - var rootFolder = Current.IOHelper.MapPath("/"); + var rootFolder = _ioHelper.MapPath("/"); var failedFolders = failedPaths .Select(x => ParseFolderFromFullPath(rootFolder, x)); return _textService.Localize(messageKey, diff --git a/src/Umbraco.Abstractions/Install/IFilePermissionHelper.cs b/src/Umbraco.Abstractions/Install/IFilePermissionHelper.cs new file mode 100644 index 0000000000..90f550f2c9 --- /dev/null +++ b/src/Umbraco.Abstractions/Install/IFilePermissionHelper.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; + +namespace Umbraco.Core.Install +{ + public interface IFilePermissionHelper + { + bool RunFilePermissionTestSuite(out Dictionary> report); + bool EnsureDirectories(string[] dirs, out IEnumerable errors, bool writeCausesRestart = false); + bool EnsureFiles(string[] files, out IEnumerable errors); + bool EnsureCanCreateSubDirectory(string dir, out IEnumerable errors); + bool EnsureCanCreateSubDirectories(IEnumerable dirs, out IEnumerable errors); + bool TestPublishedSnapshotService(out IEnumerable errors); + bool TryCreateDirectory(string dir); + bool TryAccessDirectory(string dir, bool canWrite); + } +} diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs index 5b2838df39..42e446c4d3 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Events; using Umbraco.Core.Hosting; +using Umbraco.Core.Install; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -143,6 +144,8 @@ namespace Umbraco.Tests.PublishedContent var typeFinder = new TypeFinder(Mock.Of()); + var filePermissionHelper = Mock.Of(); + // at last, create the complete NuCache snapshot service! var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true }; _snapshotService = new PublishedSnapshotService(options, @@ -165,7 +168,8 @@ namespace Umbraco.Tests.PublishedContent new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider(TestHelper.ShortStringHelper) }), typeFinder, hostingEnvironment, - new MockShortStringHelper()); + new MockShortStringHelper(), + filePermissionHelper); // invariant is the current default _variationAccesor.VariationContext = new VariationContext(); diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs index 8bf976d24e..08e68ce652 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs @@ -8,6 +8,7 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Events; +using Umbraco.Core.Install; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -185,6 +186,8 @@ namespace Umbraco.Tests.PublishedContent var typeFinder = new TypeFinder(Mock.Of()); + var filePermissionHelper = Mock.Of(); + // at last, create the complete NuCache snapshot service! var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true }; _snapshotService = new PublishedSnapshotService(options, @@ -207,7 +210,8 @@ namespace Umbraco.Tests.PublishedContent new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider(TestHelper.ShortStringHelper) }), typeFinder, TestHelper.GetHostingEnvironment(), - new MockShortStringHelper()); + new MockShortStringHelper(), + filePermissionHelper); // invariant is the current default _variationAccesor.VariationContext = new VariationContext(); diff --git a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs index 312cc9969b..eba7187147 100644 --- a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs @@ -10,6 +10,7 @@ using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Events; +using Umbraco.Core.Install; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -83,6 +84,7 @@ namespace Umbraco.Tests.Scoping var mediaRepository = Mock.Of(); var memberRepository = Mock.Of(); var hostingEnvironment = TestHelper.GetHostingEnvironment(); + var filePermissionHelper = Mock.Of(); var typeFinder = new TypeFinder(Mock.Of()); @@ -105,7 +107,8 @@ namespace Umbraco.Tests.Scoping new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider(ShortStringHelper) }), typeFinder, hostingEnvironment, - new MockShortStringHelper()); + new MockShortStringHelper(), + filePermissionHelper); } protected UmbracoContext GetUmbracoContextNu(string url, int templateId = 1234, RouteData routeData = null, bool setSingleton = false, IUmbracoSettingsSection umbracoSettings = null, IEnumerable urlProviders = null) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 6396564a3e..bbd428b918 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Hosting; +using Umbraco.Core.Install; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -55,7 +56,7 @@ namespace Umbraco.Tests.Services var mediaRepository = Mock.Of(); var memberRepository = Mock.Of(); var hostingEnvironment = Mock.Of(); - + var filePermissionHelper = Mock.Of(); var typeFinder = new TypeFinder(Mock.Of()); @@ -78,7 +79,8 @@ namespace Umbraco.Tests.Services new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider(ShortStringHelper) }), typeFinder, hostingEnvironment, - new MockShortStringHelper()); + new MockShortStringHelper(), + filePermissionHelper); } public class LocalServerMessenger : ServerMessengerBase diff --git a/src/Umbraco.Web/Install/FilePermissionHelper.cs b/src/Umbraco.Web/Install/FilePermissionHelper.cs index 877c2488f6..cd307ec6a6 100644 --- a/src/Umbraco.Web/Install/FilePermissionHelper.cs +++ b/src/Umbraco.Web/Install/FilePermissionHelper.cs @@ -4,33 +4,34 @@ using System.Linq; using System.IO; using System.Security.AccessControl; using Umbraco.Core; +using Umbraco.Core.Install; using Umbraco.Core.IO; using Umbraco.Web.Composing; namespace Umbraco.Web.Install { - internal class FilePermissionHelper + internal class FilePermissionHelper: IFilePermissionHelper { // ensure that these directories exist and Umbraco can write to them - private static readonly string[] PermissionDirs = { Current.Configs.Global().UmbracoCssPath, Constants.SystemDirectories.Config, Constants.SystemDirectories.Data, Current.Configs.Global().UmbracoMediaPath, Constants.SystemDirectories.Preview }; - private static readonly string[] PackagesPermissionsDirs = { Constants.SystemDirectories.Bin, Current.Configs.Global().UmbracoPath, Constants.SystemDirectories.Packages }; + private readonly string[] _permissionDirs = { Current.Configs.Global().UmbracoCssPath, Constants.SystemDirectories.Config, Constants.SystemDirectories.Data, Current.Configs.Global().UmbracoMediaPath, Constants.SystemDirectories.Preview }; + private readonly string[] _packagesPermissionsDirs = { Constants.SystemDirectories.Bin, Current.Configs.Global().UmbracoPath, Constants.SystemDirectories.Packages }; // ensure Umbraco can write to these files (the directories must exist) - private static readonly string[] PermissionFiles = { }; + private readonly string[] _permissionFiles = { }; - public static bool RunFilePermissionTestSuite(out Dictionary> report) + public bool RunFilePermissionTestSuite(out Dictionary> report) { report = new Dictionary>(); using (ChangesMonitor.Suspended()) // hack: ensure this does not trigger a restart { - if (EnsureDirectories(PermissionDirs, out var errors) == false) + if (EnsureDirectories(_permissionDirs, out var errors) == false) report["Folder creation failed"] = errors.ToList(); - if (EnsureDirectories(PackagesPermissionsDirs, out errors) == false) + if (EnsureDirectories(_packagesPermissionsDirs, out errors) == false) report["File writing for packages failed"] = errors.ToList(); - if (EnsureFiles(PermissionFiles, out errors) == false) + if (EnsureFiles(_permissionFiles, out errors) == false) report["File writing failed"] = errors.ToList(); if (TestPublishedSnapshotService(out errors) == false) @@ -54,7 +55,7 @@ namespace Umbraco.Web.Install /// reliable but we cannot write a file since it will cause an app domain restart. /// /// - public static bool EnsureDirectories(string[] dirs, out IEnumerable errors, bool writeCausesRestart = false) + public bool EnsureDirectories(string[] dirs, out IEnumerable errors, bool writeCausesRestart = false) { List temp = null; var success = true; @@ -74,7 +75,7 @@ namespace Umbraco.Web.Install return success; } - public static bool EnsureFiles(string[] files, out IEnumerable errors) + public bool EnsureFiles(string[] files, out IEnumerable errors) { List temp = null; var success = true; @@ -92,12 +93,12 @@ namespace Umbraco.Web.Install return success; } - public static bool EnsureCanCreateSubDirectory(string dir, out IEnumerable errors) + public bool EnsureCanCreateSubDirectory(string dir, out IEnumerable errors) { return EnsureCanCreateSubDirectories(new[] { dir }, out errors); } - public static bool EnsureCanCreateSubDirectories(IEnumerable dirs, out IEnumerable errors) + public bool EnsureCanCreateSubDirectories(IEnumerable dirs, out IEnumerable errors) { List temp = null; var success = true; @@ -115,7 +116,7 @@ namespace Umbraco.Web.Install return success; } - public static bool TestPublishedSnapshotService(out IEnumerable errors) + public bool TestPublishedSnapshotService(out IEnumerable errors) { var publishedSnapshotService = Current.PublishedSnapshotService; return publishedSnapshotService.EnsureEnvironment(out errors); @@ -124,7 +125,7 @@ namespace Umbraco.Web.Install // tries to create a sub-directory // if successful, the sub-directory is deleted // creates the directory if needed - does not delete it - private static bool TryCreateSubDirectory(string dir) + private bool TryCreateSubDirectory(string dir) { try { @@ -142,7 +143,7 @@ namespace Umbraco.Web.Install // tries to create a file // if successful, the file is deleted // creates the directory if needed - does not delete it - public static bool TryCreateDirectory(string dir) + public bool TryCreateDirectory(string dir) { try { @@ -170,7 +171,7 @@ namespace Umbraco.Web.Install // use the ACL APIs to avoid creating files // // if the directory does not exist, do nothing & success - public static bool TryAccessDirectory(string dir, bool canWrite) + public bool TryAccessDirectory(string dir, bool canWrite) { try { @@ -197,7 +198,7 @@ namespace Umbraco.Web.Install } } - private static bool HasWritePermission(string path) + private bool HasWritePermission(string path) { var writeAllow = false; var writeDeny = false; @@ -235,7 +236,7 @@ namespace Umbraco.Web.Install // tries to write into a file // fails if the directory does not exist - private static bool TryWriteFile(string file) + private bool TryWriteFile(string file) { try { @@ -249,7 +250,7 @@ namespace Umbraco.Web.Install } } - private static string CreateRandomName() + private string CreateRandomName() { return "umbraco-test." + Guid.NewGuid().ToString("N").Substring(0, 8); } diff --git a/src/Umbraco.Web/Install/InstallSteps/FilePermissionsStep.cs b/src/Umbraco.Web/Install/InstallSteps/FilePermissionsStep.cs index 2c236e6592..b4be0b0a21 100644 --- a/src/Umbraco.Web/Install/InstallSteps/FilePermissionsStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/FilePermissionsStep.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; using System.Threading.Tasks; using Umbraco.Core; +using Umbraco.Core.Install; using Umbraco.Core.IO; using Umbraco.Web.Install.Models; @@ -13,11 +14,16 @@ namespace Umbraco.Web.Install.InstallSteps PerformsAppRestart = true)] internal class FilePermissionsStep : InstallSetupStep { + private readonly IFilePermissionHelper _filePermissionHelper; + public FilePermissionsStep(IFilePermissionHelper filePermissionHelper) + { + _filePermissionHelper = filePermissionHelper; + } public override Task ExecuteAsync(object model) { // validate file permissions Dictionary> report; - var permissionsOk = FilePermissionHelper.RunFilePermissionTestSuite(out report); + var permissionsOk = _filePermissionHelper.RunFilePermissionTestSuite(out report); if (permissionsOk == false) throw new InstallException("Permission check failed", "permissionsreport", new { errors = report }); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index dabf195284..67e334ec83 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Hosting; +using Umbraco.Core.Install; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; @@ -51,6 +52,7 @@ namespace Umbraco.Web.PublishedCache.NuCache private readonly ITypeFinder _typeFinder; private readonly IHostingEnvironment _hostingEnvironment; private readonly IShortStringHelper _shortStringHelper; + private readonly IFilePermissionHelper _filePermissionHelper; // volatile because we read it with no lock private volatile bool _isReady; @@ -87,7 +89,8 @@ namespace Umbraco.Web.PublishedCache.NuCache UrlSegmentProviderCollection urlSegmentProviders, ITypeFinder typeFinder, IHostingEnvironment hostingEnvironment, - IShortStringHelper shortStringHelper) + IShortStringHelper shortStringHelper, + IFilePermissionHelper filePermissionHelper) : base(publishedSnapshotAccessor, variationContextAccessor) { //if (Interlocked.Increment(ref _singletonCheck) > 1) @@ -107,6 +110,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _typeFinder = typeFinder; _hostingEnvironment = hostingEnvironment; _shortStringHelper = shortStringHelper; + _filePermissionHelper = filePermissionHelper; // we need an Xml serializer here so that the member cache can support XPath, // for members this is done by navigating the serialized-to-xml member @@ -360,7 +364,7 @@ namespace Umbraco.Web.PublishedCache.NuCache public override bool EnsureEnvironment(out IEnumerable errors) { // must have app_data and be able to write files into it - var ok = FilePermissionHelper.TryCreateDirectory(GetLocalFilesPath()); + var ok = _filePermissionHelper.TryCreateDirectory(GetLocalFilesPath()); errors = ok ? Enumerable.Empty() : new[] { "NuCache local files." }; return ok; } diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 4cb256e44a..8de0a0e995 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -10,6 +10,7 @@ using Umbraco.Core.Dashboards; using Umbraco.Core.Dictionary; using Umbraco.Core.Events; using Umbraco.Core.Hosting; +using Umbraco.Core.Install; using Umbraco.Core.Migrations.PostMigrations; using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.PublishedContent; @@ -27,6 +28,7 @@ using Umbraco.Web.Editors; using Umbraco.Web.Features; using Umbraco.Web.HealthCheck; using Umbraco.Web.Hosting; +using Umbraco.Web.Install; using Umbraco.Web.Macros; using Umbraco.Web.Media.EmbedProviders; using Umbraco.Web.Models.PublishedContent; @@ -63,6 +65,7 @@ namespace Umbraco.Web.Runtime composition.Register(); composition.Register(); composition.Register(); + composition.Register(Lifetime.Singleton); composition.RegisterUnique(); // required for hybrid accessors