diff --git a/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs b/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs index 36e774287b..ff8c2646cc 100644 --- a/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs +++ b/src/Umbraco.Web/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs @@ -59,12 +59,10 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions // in ALL circumstances or just some var pathsToCheck = new Dictionary { - { SystemDirectories.AppCode, PermissionCheckRequirement.Optional }, { SystemDirectories.Data, PermissionCheckRequirement.Required }, { SystemDirectories.Packages, PermissionCheckRequirement.Required}, { SystemDirectories.Preview, PermissionCheckRequirement.Required }, { SystemDirectories.AppPlugins, PermissionCheckRequirement.Required }, - { SystemDirectories.Bin, PermissionCheckRequirement.Optional }, { SystemDirectories.Config, PermissionCheckRequirement.Optional }, { SystemDirectories.Css, PermissionCheckRequirement.Optional }, { SystemDirectories.Masterpages, PermissionCheckRequirement.Optional }, @@ -77,12 +75,33 @@ namespace Umbraco.Web.HealthCheck.Checks.Permissions { SystemDirectories.Xslt, PermissionCheckRequirement.Optional }, }; + //These are special paths to check that will restart an app domain if a file is written to them, + //so these need to be tested differently + var pathsToCheckWithRestarts = new Dictionary + { + { SystemDirectories.AppCode, PermissionCheckRequirement.Optional }, + { SystemDirectories.Bin, PermissionCheckRequirement.Optional } + }; + // Run checks for required and optional paths for modify permission List requiredFailedPaths; List optionalFailedPaths; var requiredPathCheckResult = FilePermissionHelper.TestDirectories(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out requiredFailedPaths); var optionalPathCheckResult = FilePermissionHelper.TestDirectories(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out optionalFailedPaths); + //now check the special folders + List requiredFailedPaths2; + List optionalFailedPaths2; + var requiredPathCheckResult2 = FilePermissionHelper.TestDirectories(GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Required), out requiredFailedPaths2, writeCausesRestart:true); + var optionalPathCheckResult2 = FilePermissionHelper.TestDirectories(GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Optional), out optionalFailedPaths2, writeCausesRestart: true); + + requiredPathCheckResult = requiredPathCheckResult && requiredPathCheckResult2; + optionalPathCheckResult = optionalPathCheckResult && optionalPathCheckResult2; + + //combine the paths + requiredFailedPaths = requiredFailedPaths.Concat(requiredFailedPaths2).ToList(); + optionalFailedPaths = requiredFailedPaths.Concat(optionalFailedPaths2).ToList(); + return GetStatus(requiredPathCheckResult, requiredFailedPaths, optionalPathCheckResult, optionalFailedPaths, PermissionCheckFor.Folder); } diff --git a/src/Umbraco.Web/Install/FilePermissionHelper.cs b/src/Umbraco.Web/Install/FilePermissionHelper.cs index 87491b93a8..a91bd6d306 100644 --- a/src/Umbraco.Web/Install/FilePermissionHelper.cs +++ b/src/Umbraco.Web/Install/FilePermissionHelper.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Web; using System.IO; +using System.Security.AccessControl; using Umbraco.Core.IO; using umbraco; @@ -37,8 +38,19 @@ namespace Umbraco.Web.Install return errorReport.Any() == false; } - - public static bool TestDirectories(string[] directories, out List errorReport) + + /// + /// This will test the directories for write access + /// + /// + /// + /// + /// If this is false, the easiest way to test for write access is to write a temp file, however some folder will cause + /// an App Domain restart if a file is written to the folder, so in that case we need to use the ACL APIs which aren't as + /// reliable but we cannot write a file since it will cause an app domain restart. + /// + /// + public static bool TestDirectories(string[] directories, out List errorReport, bool writeCausesRestart = false) { errorReport = new List(); bool succes = true; @@ -46,7 +58,11 @@ namespace Umbraco.Web.Install { if (Directory.Exists(dir) == false) continue; - bool result = SaveAndDeleteFile(IOHelper.MapPath(dir + "/configWizardPermissionTest.txt")); + var folder = IOHelper.MapPath(dir); + + var result = writeCausesRestart + ? HasWritePermissionOnDir(folder) + : SaveAndDeleteFile(Path.Combine(folder, "configWizardPermissionTest.txt")); if (result == false) { @@ -131,7 +147,42 @@ namespace Umbraco.Web.Install { return false; } + } + private static bool HasWritePermissionOnDir(string path) + { + var writeAllow = false; + var writeDeny = false; + var accessControlList = Directory.GetAccessControl(path); + if (accessControlList == null) + return false; + AuthorizationRuleCollection accessRules; + try + { + accessRules = accessControlList.GetAccessRules(true, true, typeof(System.Security.Principal.SecurityIdentifier)); + if (accessRules == null) + return false; + } + catch (Exception e) + { + //This is not 100% accurate btw because it could turn out that the current user doesn't + //have access to read the current permissions but does have write access. + //I think this is an edge case however + return false; + } + + foreach (FileSystemAccessRule rule in accessRules) + { + if ((FileSystemRights.Write & rule.FileSystemRights) != FileSystemRights.Write) + continue; + + if (rule.AccessControlType == AccessControlType.Allow) + writeAllow = true; + else if (rule.AccessControlType == AccessControlType.Deny) + writeDeny = true; + } + + return writeAllow && writeDeny == false; } private static bool OpenFileForWrite(string file)