From 0f226b590aa0366d18cb01c49aa3912dec34b611 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 3 Feb 2021 07:58:42 +0100 Subject: [PATCH] updated files before move --- .../HealthChecks/CompilationDebugCheck.cs | 41 ++-- .../HealthChecks/ConfigurationService.cs | 71 ------- .../TrySkipIisCustomErrorsCheck.cs | 80 ------- .../Checks/AbstractSettingsCheck.cs | 151 ++++--------- .../Configuration/NotificationEmailCheck.cs | 40 +++- .../LiveEnvironment/CustomErrorsCheck.cs | 55 ----- .../Checks/LiveEnvironment/TraceCheck.cs | 34 --- .../FolderAndFilePermissionsCheck.cs | 198 ++++++------------ .../Checks/Permissions/PermissionCheckFor.cs | 8 - .../Permissions/PermissionCheckRequirement.cs | 8 - .../Checks/Security/BaseHttpHeaderCheck.cs | 137 +++++------- .../HealthCheck/Checks/Security/HstsCheck.cs | 28 ++- .../Checks/Security/XssProtectionCheck.cs | 30 ++- .../HealthCheck/HealthCheckCollection.cs | 6 +- .../HealthCheck/IConfigurationService.cs | 7 - src/Umbraco.Core/Umbraco.Core.csproj | 9 + 16 files changed, 282 insertions(+), 621 deletions(-) delete mode 100644 src/Umbraco.Core/Configuration/HealthChecks/ConfigurationService.cs delete mode 100644 src/Umbraco.Core/Configuration/HealthChecks/TrySkipIisCustomErrorsCheck.cs delete mode 100644 src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/CustomErrorsCheck.cs delete mode 100644 src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/TraceCheck.cs delete mode 100644 src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckFor.cs delete mode 100644 src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckRequirement.cs delete mode 100644 src/Umbraco.Core/HealthCheck/IConfigurationService.cs diff --git a/src/Umbraco.Core/Configuration/HealthChecks/CompilationDebugCheck.cs b/src/Umbraco.Core/Configuration/HealthChecks/CompilationDebugCheck.cs index a7b99ea205..e134dcd413 100644 --- a/src/Umbraco.Core/Configuration/HealthChecks/CompilationDebugCheck.cs +++ b/src/Umbraco.Core/Configuration/HealthChecks/CompilationDebugCheck.cs @@ -1,30 +1,42 @@ -using System.Collections.Generic; -using Microsoft.Extensions.Logging; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; using Microsoft.Extensions.Options; using Umbraco.Core.Configuration.Models; -using Umbraco.Core.HealthCheck; -using Umbraco.Core.HealthCheck.Checks; using Umbraco.Core.Services; -namespace Umbraco.Core.Configuration.HealthChecks +namespace Umbraco.Core.HealthChecks.Checks.LiveEnvironment { - [HealthCheck("61214FF3-FC57-4B31-B5CF-1D095C977D6D", "Debug Compilation Mode", + /// + /// Health check for the configuration of debug-flag. + /// + [HealthCheck( + "61214FF3-FC57-4B31-B5CF-1D095C977D6D", + "Debug Compilation Mode", Description = "Leaving debug compilation mode enabled can severely slow down a website and take up more memory on the server.", Group = "Live Environment")] public class CompilationDebugCheck : AbstractSettingsCheck { private readonly IOptionsMonitor _hostingSettings; - public CompilationDebugCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory, IOptionsMonitor hostingSettings) - : base(textService, loggerFactory) - { + /// + /// Initializes a new instance of the class. + /// + public CompilationDebugCheck(ILocalizedTextService textService, IOptionsMonitor hostingSettings) + : base(textService) => _hostingSettings = hostingSettings; - } + /// public override string ItemPath => Constants.Configuration.ConfigHostingDebug; + /// + public override string ReadMoreLink => Constants.HealthChecks.DocumentationLinks.LiveEnvironment.CompilationDebugCheck; + + /// public override ValueComparisonType ValueComparisonType => ValueComparisonType.ShouldEqual; + /// public override IEnumerable Values => new List { new AcceptableConfiguration @@ -34,12 +46,13 @@ namespace Umbraco.Core.Configuration.HealthChecks } }; + /// public override string CurrentValue => _hostingSettings.CurrentValue.Debug.ToString(); - public override string CheckSuccessMessage => TextService.Localize("healthcheck/compilationDebugCheckSuccessMessage"); + /// + public override string CheckSuccessMessage => LocalizedTextService.Localize("healthcheck/compilationDebugCheckSuccessMessage"); - public override string CheckErrorMessage => TextService.Localize("healthcheck/compilationDebugCheckErrorMessage"); - - public override string RectifySuccessMessage => TextService.Localize("healthcheck/compilationDebugCheckRectifySuccessMessage"); + /// + public override string CheckErrorMessage => LocalizedTextService.Localize("healthcheck/compilationDebugCheckErrorMessage"); } } diff --git a/src/Umbraco.Core/Configuration/HealthChecks/ConfigurationService.cs b/src/Umbraco.Core/Configuration/HealthChecks/ConfigurationService.cs deleted file mode 100644 index 2459698b7a..0000000000 --- a/src/Umbraco.Core/Configuration/HealthChecks/ConfigurationService.cs +++ /dev/null @@ -1,71 +0,0 @@ -using System; -using Microsoft.Extensions.Logging; -using Umbraco.Core.HealthCheck; -using Umbraco.Core.Services; - -namespace Umbraco.Core.Configuration.HealthChecks -{ - public class ConfigurationService : IConfigurationService - { - private readonly ILocalizedTextService _textService; - private readonly ILogger _logger; - private readonly IConfigManipulator _configManipulator; - - /// - /// - /// - /// - public ConfigurationService(ILocalizedTextService textService, ILogger logger, IConfigManipulator configManipulator) - { - if (textService == null) HandleNullParameter(nameof(textService)); - if (configManipulator == null) HandleNullParameter(nameof(configManipulator)); - if (logger == null) HandleNullParameter(nameof(logger)); - - _configManipulator = configManipulator; - _textService = textService; - _logger = logger; - } - - private void HandleNullParameter(string parameter) - { - _logger.LogError("Error trying to get configuration value", parameter); - throw new ArgumentNullException(parameter); - } - - /// - /// Updates a value in a given configuration file with the given path - /// - /// - /// - /// - public ConfigurationServiceResult UpdateConfigFile(string value, string itemPath) - { - try - { - if (itemPath == null) - { - return new ConfigurationServiceResult - { - Success = false, - Result = _textService.Localize("healthcheck/configurationServiceNodeNotFound", new[] { itemPath, value }) - }; - } - - _configManipulator.SaveConfigValue(itemPath, value); - return new ConfigurationServiceResult - { - Success = true - }; - } - catch (Exception ex) - { - _logger.LogError(ex, "Error trying to update configuration"); - return new ConfigurationServiceResult - { - Success = false, - Result = _textService.Localize("healthcheck/configurationServiceError", new[] { ex.Message }) - }; - } - } - } -} diff --git a/src/Umbraco.Core/Configuration/HealthChecks/TrySkipIisCustomErrorsCheck.cs b/src/Umbraco.Core/Configuration/HealthChecks/TrySkipIisCustomErrorsCheck.cs deleted file mode 100644 index 9710080f35..0000000000 --- a/src/Umbraco.Core/Configuration/HealthChecks/TrySkipIisCustomErrorsCheck.cs +++ /dev/null @@ -1,80 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; -using Umbraco.Core.Configuration.Models; -using Umbraco.Core.Hosting; -using Umbraco.Core.Services; - -namespace Umbraco.Core.HealthCheck.Checks.Configuration -{ - [Obsolete("This is not currently in the appsettings.JSON and so can either be removed, or rewritten in .NET Core fashion")] - [HealthCheck("046A066C-4FB2-4937-B931-069964E16C66", "Try Skip IIS Custom Errors", - Description = "Starting with IIS 7.5, this must be set to true for Umbraco 404 pages to show. Otherwise, IIS will takeover and render its built-in error page.", - Group = "Configuration")] - public class TrySkipIisCustomErrorsCheck : AbstractSettingsCheck - { - private readonly ILocalizedTextService _textService; - private readonly ILoggerFactory _loggerFactory; - private readonly Version _iisVersion; - private readonly GlobalSettings _globalSettings; - - public TrySkipIisCustomErrorsCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory, IOptions globalSettings) - : base(textService, loggerFactory) - { - _textService = textService; - _loggerFactory = loggerFactory; - //TODO: detect if hosted in IIS, and then IIS version if we want to go this route - _iisVersion = new Version("7.5"); - _globalSettings = globalSettings.Value; - } - - public override string ItemPath => "TBC"; - - public override ValueComparisonType ValueComparisonType => ValueComparisonType.ShouldEqual; - - public override string CurrentValue => null; - - public override IEnumerable Values - { - get - { - // beware! 7.5 and 7.5.0 are not the same thing! - var recommendedValue = _iisVersion >= new Version("7.5") - ? bool.TrueString.ToLower() - : bool.FalseString.ToLower(); - return new List { new AcceptableConfiguration { IsRecommended = true, Value = recommendedValue } }; - } - } - - public override string CheckSuccessMessage - { - get - { - return _textService.Localize("healthcheck/trySkipIisCustomErrorsCheckSuccessMessage", - new[] { Values.First(v => v.IsRecommended).Value, _iisVersion.ToString() }); - } - } - - public override string CheckErrorMessage - { - get - { - return _textService.Localize("healthcheck/trySkipIisCustomErrorsCheckErrorMessage", - new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, _iisVersion.ToString() }); - } - } - - public override string RectifySuccessMessage - { - get - { - return _textService.Localize("healthcheck/trySkipIisCustomErrorsCheckRectifySuccessMessage", - new[] { "Not implemented" }); - - //new[] { Values.First(v => v.IsRecommended).Value, _iisVersion.ToString() }); - } - } - } -} diff --git a/src/Umbraco.Core/HealthCheck/Checks/AbstractSettingsCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/AbstractSettingsCheck.cs index 62543dcfbd..0869e7c8ec 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/AbstractSettingsCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/AbstractSettingsCheck.cs @@ -1,15 +1,28 @@ -using System; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; using System.Collections.Generic; using System.Linq; -using Microsoft.Extensions.Logging; +using System.Threading.Tasks; using Umbraco.Core.Services; -namespace Umbraco.Core.HealthCheck.Checks +namespace Umbraco.Core.HealthChecks.Checks { + /// + /// Provides a base class for health checks of configuration values. + /// public abstract class AbstractSettingsCheck : HealthCheck { - protected ILocalizedTextService TextService { get; } - protected ILoggerFactory LoggerFactory { get; } + /// + /// Initializes a new instance of the class. + /// + protected AbstractSettingsCheck(ILocalizedTextService textService) => LocalizedTextService = textService; + + /// + /// Gets the localized text service. + /// + protected ILocalizedTextService LocalizedTextService { get; } /// /// Gets key within the JSON to check, in the colon-delimited format @@ -17,6 +30,11 @@ namespace Umbraco.Core.HealthCheck.Checks /// public abstract string ItemPath { get; } + /// + /// Gets a link to an external resource with more information. + /// + public abstract string ReadMoreLink { get; } + /// /// Gets the values to compare against. /// @@ -27,134 +45,53 @@ namespace Umbraco.Core.HealthCheck.Checks /// public abstract string CurrentValue { get; } - /// - /// Gets the provided value - /// - public string ProvidedValue { get; set; } - /// /// Gets the comparison type for checking the value. /// public abstract ValueComparisonType ValueComparisonType { get; } - protected AbstractSettingsCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory) - { - TextService = textService; - LoggerFactory = loggerFactory; - } - /// /// Gets the message for when the check has succeeded. /// - public virtual string CheckSuccessMessage - { - get - { - return TextService.Localize("healthcheck/checkSuccessMessage", new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }); - } - } + public virtual string CheckSuccessMessage => LocalizedTextService.Localize("healthcheck/checkSuccessMessage", new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }); /// /// Gets the message for when the check has failed. /// - public virtual string CheckErrorMessage - { - get - { - return ValueComparisonType == ValueComparisonType.ShouldEqual - ? TextService.Localize("healthcheck/checkErrorMessageDifferentExpectedValue", - new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }) - : TextService.Localize("healthcheck/checkErrorMessageUnexpectedValue", - new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }); - } - } + public virtual string CheckErrorMessage => + ValueComparisonType == ValueComparisonType.ShouldEqual + ? LocalizedTextService.Localize( + "healthcheck/checkErrorMessageDifferentExpectedValue", + new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }) + : LocalizedTextService.Localize( + "healthcheck/checkErrorMessageUnexpectedValue", + new[] { CurrentValue, Values.First(v => v.IsRecommended).Value, ItemPath }); - /// - /// Gets the rectify success message. - /// - public virtual string RectifySuccessMessage - { - get - { - AcceptableConfiguration recommendedValue = Values.FirstOrDefault(v => v.IsRecommended); - string rectifiedValue = recommendedValue != null ? recommendedValue.Value : ProvidedValue; - return TextService.Localize("healthcheck/rectifySuccessMessage", - new[] - { - CurrentValue, - rectifiedValue, - ItemPath - }); - } - } - - /// - /// Gets a value indicating whether this check can be rectified automatically. - /// - public virtual bool CanRectify => ValueComparisonType == ValueComparisonType.ShouldEqual; - - /// - /// Gets a value indicating whether this check can be rectified automatically if a value is provided. - /// - public virtual bool CanRectifyWithValue => ValueComparisonType == ValueComparisonType.ShouldNotEqual; - - public override IEnumerable GetStatus() + /// + public override Task> GetStatus() { // update the successMessage with the CurrentValue var successMessage = string.Format(CheckSuccessMessage, ItemPath, Values, CurrentValue); bool valueFound = Values.Any(value => string.Equals(CurrentValue, value.Value, StringComparison.InvariantCultureIgnoreCase)); - if (ValueComparisonType == ValueComparisonType.ShouldEqual - && valueFound || ValueComparisonType == ValueComparisonType.ShouldNotEqual - && valueFound == false) + if ((ValueComparisonType == ValueComparisonType.ShouldEqual && valueFound) + || (ValueComparisonType == ValueComparisonType.ShouldNotEqual && valueFound == false)) { - return new[] - { - new HealthCheckStatus(successMessage) + return Task.FromResult(new HealthCheckStatus(successMessage) { - ResultType = StatusResultType.Success - } - }; + ResultType = StatusResultType.Success, + }.Yield()); } - // Declare the action for rectifying the config value - var rectifyAction = new HealthCheckAction("rectify", Id) - { - Name = TextService.Localize("healthcheck/rectifyButton"), - ValueRequired = CanRectifyWithValue - }; - string resultMessage = string.Format(CheckErrorMessage, ItemPath, Values, CurrentValue); - return new[] + return Task.FromResult(new HealthCheckStatus(resultMessage) { - new HealthCheckStatus(resultMessage) - { - ResultType = StatusResultType.Error, - Actions = CanRectify || CanRectifyWithValue ? new[] { rectifyAction } : new HealthCheckAction[0] - } - }; - } - - /// - /// Rectifies this check. - /// - /// - public virtual HealthCheckStatus Rectify(HealthCheckAction action) - { - if (ValueComparisonType == ValueComparisonType.ShouldNotEqual) - { - throw new InvalidOperationException(TextService.Localize("healthcheck/cannotRectifyShouldNotEqual")); - } - - //TODO: show message instead of actually fixing config - string recommendedValue = Values.First(v => v.IsRecommended).Value; - string resultMessage = string.Format(RectifySuccessMessage, ItemPath, Values); - return new HealthCheckStatus(resultMessage) { ResultType = StatusResultType.Success }; + ResultType = StatusResultType.Error, ReadMoreLink = ReadMoreLink + }.Yield()); } + /// public override HealthCheckStatus ExecuteAction(HealthCheckAction action) - { - return Rectify(action); - } + => throw new NotSupportedException("Configuration cannot be automatically fixed."); } } diff --git a/src/Umbraco.Core/HealthCheck/Checks/Configuration/NotificationEmailCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Configuration/NotificationEmailCheck.cs index a6e6a83c47..474a450e82 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Configuration/NotificationEmailCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Configuration/NotificationEmailCheck.cs @@ -1,12 +1,19 @@ -using System.Collections.Generic; -using Microsoft.Extensions.Logging; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; using Microsoft.Extensions.Options; using Umbraco.Core.Configuration.Models; using Umbraco.Core.Services; -namespace Umbraco.Core.HealthCheck.Checks.Configuration +namespace Umbraco.Core.HealthChecks.Checks.Configuration { - [HealthCheck("3E2F7B14-4B41-452B-9A30-E67FBC8E1206", "Notification Email Settings", + /// + /// Health check for the recommended production configuration for Notification Email. + /// + [HealthCheck( + "3E2F7B14-4B41-452B-9A30-E67FBC8E1206", + "Notification Email Settings", Description = "If notifications are used, the 'from' email address should be specified and changed from the default value.", Group = "Configuration")] public class NotificationEmailCheck : AbstractSettingsCheck @@ -14,26 +21,37 @@ namespace Umbraco.Core.HealthCheck.Checks.Configuration private readonly IOptionsMonitor _contentSettings; private const string DefaultFromEmail = "your@email.here"; - public NotificationEmailCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory, IOptionsMonitor contentSettings) - : base(textService, loggerFactory) - { + /// + /// Initializes a new instance of the class. + /// + public NotificationEmailCheck( + ILocalizedTextService textService, + IOptionsMonitor contentSettings) + : base(textService) => _contentSettings = contentSettings; - } + /// public override string ItemPath => Constants.Configuration.ConfigContentNotificationsEmail; + /// public override ValueComparisonType ValueComparisonType => ValueComparisonType.ShouldNotEqual; + /// public override IEnumerable Values => new List { new AcceptableConfiguration { IsRecommended = false, Value = DefaultFromEmail } }; + /// public override string CurrentValue => _contentSettings.CurrentValue.Notifications.Email; - public override string CheckSuccessMessage => TextService.Localize("healthcheck/notificationEmailsCheckSuccessMessage", new[] { CurrentValue }); + /// + public override string CheckSuccessMessage => LocalizedTextService.Localize("healthcheck/notificationEmailsCheckSuccessMessage", new[] { CurrentValue }); - public override string CheckErrorMessage => TextService.Localize("healthcheck/notificationEmailsCheckErrorMessage", new[] { DefaultFromEmail }); - + /// + public override string CheckErrorMessage => LocalizedTextService.Localize("healthcheck/notificationEmailsCheckErrorMessage", new[] { DefaultFromEmail }); + + /// + public override string ReadMoreLink => Constants.HealthChecks.DocumentationLinks.Configuration.NotificationEmailCheck; } } diff --git a/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/CustomErrorsCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/CustomErrorsCheck.cs deleted file mode 100644 index b003506205..0000000000 --- a/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/CustomErrorsCheck.cs +++ /dev/null @@ -1,55 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using Microsoft.Extensions.Logging; -using Umbraco.Core.Services; - -namespace Umbraco.Core.HealthCheck.Checks.LiveEnvironment -{ - [HealthCheck("4090C0A1-2C52-4124-92DD-F028FD066A64", "Custom Errors", - Description = "Leaving custom errors off will display a complete stack trace to your visitors if an exception occurs.", - Group = "Live Environment")] - public class CustomErrorsCheck : AbstractSettingsCheck - { - public CustomErrorsCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory) - : base(textService, loggerFactory) - { } - - public override string ItemPath => Constants.Configuration.ConfigCustomErrorsMode; - - public override ValueComparisonType ValueComparisonType => ValueComparisonType.ShouldEqual; - - public override IEnumerable Values => new List - { - new AcceptableConfiguration { IsRecommended = true, Value = "RemoteOnly" }, - new AcceptableConfiguration { IsRecommended = false, Value = "On" } - }; - - public override string CurrentValue { get; } - - public override string CheckSuccessMessage - { - get - { - return TextService.Localize("healthcheck/customErrorsCheckSuccessMessage", new[] { CurrentValue }); - } - } - - public override string CheckErrorMessage - { - get - { - return TextService.Localize("healthcheck/customErrorsCheckErrorMessage", - new[] { CurrentValue, Values.First(v => v.IsRecommended).Value }); - } - } - - public override string RectifySuccessMessage - { - get - { - return TextService.Localize("healthcheck/customErrorsCheckRectifySuccessMessage", - new[] { Values.First(v => v.IsRecommended).Value }); - } - } - } -} diff --git a/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/TraceCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/TraceCheck.cs deleted file mode 100644 index 03a6ecfde2..0000000000 --- a/src/Umbraco.Core/HealthCheck/Checks/LiveEnvironment/TraceCheck.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System.Collections.Generic; -using Microsoft.Extensions.Logging; -using Umbraco.Core.Services; - -namespace Umbraco.Core.HealthCheck.Checks.LiveEnvironment -{ - [HealthCheck("9BED6EF4-A7F3-457A-8935-B64E9AA8BAB3", "Trace Mode", - Description = "Leaving trace mode enabled can make valuable information about your system available to hackers.", - Group = "Live Environment")] - public class TraceCheck : AbstractSettingsCheck - { - public TraceCheck(ILocalizedTextService textService, ILoggerFactory loggerFactory) - : base(textService, loggerFactory) - { } - - public override string ItemPath => "/configuration/system.web/trace/@enabled"; - - public override ValueComparisonType ValueComparisonType => ValueComparisonType.ShouldEqual; - - public override IEnumerable Values => new List - { - new AcceptableConfiguration { IsRecommended = true, Value = bool.FalseString.ToLower() } - }; - - public override string CurrentValue { get; } - - public override string CheckSuccessMessage => TextService.Localize("healthcheck/traceModeCheckSuccessMessage"); - - public override string CheckErrorMessage => TextService.Localize("healthcheck/traceModeCheckErrorMessage"); - - public override string RectifySuccessMessage => TextService.Localize("healthcheck/traceModeCheckRectifySuccessMessage"); - - } -} diff --git a/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs index 28e1de3996..0bb7c56486 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs @@ -1,14 +1,19 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + using System; using System.Collections.Generic; using System.Linq; -using Microsoft.Extensions.Options; -using Umbraco.Core.Configuration.Models; -using Umbraco.Core.Hosting; +using System.Text; +using System.Threading.Tasks; using Umbraco.Core.Install; using Umbraco.Core.Services; -namespace Umbraco.Core.HealthCheck.Checks.Permissions +namespace Umbraco.Core.HealthChecks.Checks.Permissions { + /// + /// Health check for the folder and file permissions. + /// [HealthCheck( "53DBA282-4A79-4B67-B958-B29EC40FCC23", "Folder & File Permissions", @@ -17,145 +22,80 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions public class FolderAndFilePermissionsCheck : HealthCheck { private readonly ILocalizedTextService _textService; - private readonly IOptionsMonitor _globalSettings; private readonly IFilePermissionHelper _filePermissionHelper; - private readonly IHostingEnvironment _hostingEnvironment; - public FolderAndFilePermissionsCheck(ILocalizedTextService textService, IOptionsMonitor globalSettings, IFilePermissionHelper filePermissionHelper, IHostingEnvironment hostingEnvironment) + /// + /// Initializes a new instance of the class. + /// + public FolderAndFilePermissionsCheck( + ILocalizedTextService textService, + IFilePermissionHelper filePermissionHelper) { _textService = textService; - _globalSettings = globalSettings; _filePermissionHelper = filePermissionHelper; - _hostingEnvironment = hostingEnvironment; } /// /// Get the status for this health check /// - // TODO: This should really just run the IFilePermissionHelper.RunFilePermissionTestSuite and then we'd have a - // IFilePermissions interface resolved as a collection within the IFilePermissionHelper that runs checks against all - // IFilePermissions registered. Then there's no hard coding things done here and the checks here will be consistent - // with the checks run in IFilePermissionHelper.RunFilePermissionTestSuite which occurs on install too. - public override IEnumerable GetStatus() => new[] { CheckFolderPermissions(), CheckFilePermissions() }; + public override Task> GetStatus() + { + _filePermissionHelper.RunFilePermissionTestSuite(out Dictionary> errors); + + return Task.FromResult(errors.Select(x => new HealthCheckStatus(GetMessage(x)) + { + ResultType = x.Value.Any() ? StatusResultType.Error : StatusResultType.Success, + ReadMoreLink = GetReadMoreLink(x), + Description = GetErrorDescription(x) + })); + } + + private string GetErrorDescription(KeyValuePair> status) + { + if (!status.Value.Any()) + { + return null; + } + + var sb = new StringBuilder("The following failed:"); + + sb.AppendLine("
    "); + foreach (var error in status.Value) + { + sb.Append("
  • " + error + "
    • "); + } + + sb.AppendLine("
"); + return sb.ToString(); + } + + private string GetMessage(KeyValuePair> status) + => _textService.Localize("permissions", status.Key); + + private string GetReadMoreLink(KeyValuePair> status) + { + if (!status.Value.Any()) + { + return null; + } + + switch (status.Key) + { + case FilePermissionTest.FileWriting: + return Constants.HealthChecks.DocumentationLinks.FolderAndFilePermissionsCheck.FileWriting; + case FilePermissionTest.FolderCreation: + return Constants.HealthChecks.DocumentationLinks.FolderAndFilePermissionsCheck.FolderCreation; + case FilePermissionTest.FileWritingForPackages: + return Constants.HealthChecks.DocumentationLinks.FolderAndFilePermissionsCheck.FileWritingForPackages; + case FilePermissionTest.MediaFolderCreation: + return Constants.HealthChecks.DocumentationLinks.FolderAndFilePermissionsCheck.MediaFolderCreation; + default: return null; + } + } /// /// Executes the action and returns it's status /// public override HealthCheckStatus ExecuteAction(HealthCheckAction action) => throw new InvalidOperationException("FolderAndFilePermissionsCheck has no executable actions"); - - private HealthCheckStatus CheckFolderPermissions() - { - // Create lists of paths to check along with a flag indicating if modify rights are required - // in ALL circumstances or just some - var pathsToCheck = new Dictionary - { - { Constants.SystemDirectories.Data, PermissionCheckRequirement.Required }, - { Constants.SystemDirectories.Packages, PermissionCheckRequirement.Required}, - { Constants.SystemDirectories.Preview, PermissionCheckRequirement.Required }, - { Constants.SystemDirectories.AppPlugins, PermissionCheckRequirement.Required }, - { Constants.SystemDirectories.Config, PermissionCheckRequirement.Optional }, - { _globalSettings.CurrentValue.UmbracoCssPath, PermissionCheckRequirement.Optional }, - { _globalSettings.CurrentValue.UmbracoMediaPath, PermissionCheckRequirement.Optional }, - { _globalSettings.CurrentValue.UmbracoScriptsPath, PermissionCheckRequirement.Optional }, - { _globalSettings.CurrentValue.UmbracoPath, PermissionCheckRequirement.Optional }, - { Constants.SystemDirectories.MvcViews, 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 - { - { Constants.SystemDirectories.Bin, PermissionCheckRequirement.Optional } - }; - - // Run checks for required and optional paths for modify permission - var requiredPathCheckResult = _filePermissionHelper.EnsureDirectories( - GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out var requiredFailedPaths); - var optionalPathCheckResult = _filePermissionHelper.EnsureDirectories( - GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out var optionalFailedPaths); - - // now check the special folders - var requiredPathCheckResult2 = _filePermissionHelper.EnsureDirectories( - GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Required), out var requiredFailedPaths2, writeCausesRestart: true); - var optionalPathCheckResult2 = _filePermissionHelper.EnsureDirectories( - GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Optional), out var 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); - } - - private HealthCheckStatus CheckFilePermissions() - { - // Create lists of paths to check along with a flag indicating if modify rights are required - // in ALL circumstances or just some - var pathsToCheck = new Dictionary - { - { "~/Web.config", PermissionCheckRequirement.Optional }, - }; - - // Run checks for required and optional paths for modify permission - var requiredPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out IEnumerable requiredFailedPaths); - var optionalPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out IEnumerable optionalFailedPaths); - - return GetStatus(requiredPathCheckResult, requiredFailedPaths, optionalPathCheckResult, optionalFailedPaths, PermissionCheckFor.File); - } - - private string[] GetPathsToCheck( - Dictionary pathsToCheck, - PermissionCheckRequirement requirement) => pathsToCheck - .Where(x => x.Value == requirement) - .Select(x => _hostingEnvironment.MapPathContentRoot(x.Key)) - .OrderBy(x => x) - .ToArray(); - - private HealthCheckStatus GetStatus(bool requiredPathCheckResult, IEnumerable requiredFailedPaths, bool optionalPathCheckResult, IEnumerable optionalFailedPaths, PermissionCheckFor checkingFor) - { - // Return error if any required paths fail the check, or warning if any optional ones do - var resultType = StatusResultType.Success; - var messageKey = string.Format("healthcheck/{0}PermissionsCheckMessage", - checkingFor == PermissionCheckFor.Folder ? "folder" : "file"); - var message = _textService.Localize(messageKey); - if (requiredPathCheckResult == false) - { - resultType = StatusResultType.Error; - messageKey = string.Format("healthcheck/required{0}PermissionFailed", - checkingFor == PermissionCheckFor.Folder ? "Folder" : "File"); - message = GetMessageForPathCheckFailure(messageKey, requiredFailedPaths); - } - else if (optionalPathCheckResult == false) - { - resultType = StatusResultType.Warning; - messageKey = string.Format("healthcheck/optional{0}PermissionFailed", - checkingFor == PermissionCheckFor.Folder ? "Folder" : "File"); - message = GetMessageForPathCheckFailure(messageKey, optionalFailedPaths); - } - - var actions = new List(); - return new HealthCheckStatus(message) - { - ResultType = resultType, - Actions = actions - }; - } - - private string GetMessageForPathCheckFailure(string messageKey, IEnumerable failedPaths) - { - var rootFolder = _hostingEnvironment.MapPathContentRoot("/"); - var failedFolders = failedPaths - .Select(x => ParseFolderFromFullPath(rootFolder, x)); - return _textService.Localize(messageKey, - new[] { string.Join(", ", failedFolders) }); - } - - private string ParseFolderFromFullPath(string rootFolder, string filePath) - { - return filePath.Replace(rootFolder, string.Empty); - } } } diff --git a/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckFor.cs b/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckFor.cs deleted file mode 100644 index bd914d064d..0000000000 --- a/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckFor.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Umbraco.Core.HealthCheck.Checks.Permissions -{ - internal enum PermissionCheckFor - { - Folder, - File - } -} diff --git a/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckRequirement.cs b/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckRequirement.cs deleted file mode 100644 index f77fdbf2e3..0000000000 --- a/src/Umbraco.Core/HealthCheck/Checks/Permissions/PermissionCheckRequirement.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Umbraco.Core.HealthCheck.Checks.Permissions -{ - internal enum PermissionCheckRequirement - { - Required, - Optional - } -} diff --git a/src/Umbraco.Core/HealthCheck/Checks/Security/BaseHttpHeaderCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Security/BaseHttpHeaderCheck.cs index 5bf92342bf..0f188bd390 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Security/BaseHttpHeaderCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Security/BaseHttpHeaderCheck.cs @@ -1,130 +1,131 @@ -using System; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; using System.Collections.Generic; using System.IO; using System.Linq; -using System.Net; +using System.Net.Http; using System.Text.RegularExpressions; -using Microsoft.Extensions.Configuration; +using System.Threading.Tasks; using Umbraco.Core.Services; using Umbraco.Web; -namespace Umbraco.Core.HealthCheck.Checks.Security +namespace Umbraco.Core.HealthChecks.Checks.Security { + /// + /// Provides a base class for health checks of http header values. + /// public abstract class BaseHttpHeaderCheck : HealthCheck { - protected ILocalizedTextService TextService { get; } - - private const string SetHeaderInConfigAction = "setHeaderInConfig"; - private readonly string _header; private readonly string _value; private readonly string _localizedTextPrefix; private readonly bool _metaTagOptionAvailable; private readonly IRequestAccessor _requestAccessor; + private static HttpClient s_httpClient; + /// + /// Initializes a new instance of the class. + /// protected BaseHttpHeaderCheck( IRequestAccessor requestAccessor, ILocalizedTextService textService, - string header, string value, string localizedTextPrefix, bool metaTagOptionAvailable) + string header, + string value, + string localizedTextPrefix, + bool metaTagOptionAvailable) { - TextService = textService ?? throw new ArgumentNullException(nameof(textService)); + LocalizedTextService = textService ?? throw new ArgumentNullException(nameof(textService)); _requestAccessor = requestAccessor; _header = header; _value = value; _localizedTextPrefix = localizedTextPrefix; _metaTagOptionAvailable = metaTagOptionAvailable; - } + private static HttpClient HttpClient => s_httpClient ??= new HttpClient(); + + + /// + /// Gets the localized text service. + /// + protected ILocalizedTextService LocalizedTextService { get; } + + /// + /// Gets a link to an external read more page. + /// + protected abstract string ReadMoreLink { get; } + /// /// Get the status for this health check /// - /// - public override IEnumerable GetStatus() - { - //return the statuses - return new[] { CheckForHeader() }; - } + public override async Task> GetStatus() => + await Task.WhenAll(CheckForHeader()); /// /// Executes the action and returns it's status /// - /// - /// public override HealthCheckStatus ExecuteAction(HealthCheckAction action) - { - switch (action.Alias) - { - case SetHeaderInConfigAction: - return SetHeaderInConfig(); - default: - throw new InvalidOperationException("HTTP Header action requested is either not executable or does not exist"); - } - } + => throw new InvalidOperationException("HTTP Header action requested is either not executable or does not exist"); - protected HealthCheckStatus CheckForHeader() + /// + /// The actual health check method. + /// + protected async Task CheckForHeader() { - var message = string.Empty; + string message; var success = false; // Access the site home page and check for the click-jack protection header or meta tag - var url = _requestAccessor.GetApplicationUrl(); - var request = WebRequest.Create(url); - request.Method = "GET"; + Uri url = _requestAccessor.GetApplicationUrl(); + try { - var response = request.GetResponse(); + using HttpResponseMessage response = await HttpClient.GetAsync(url); // Check first for header - success = HasMatchingHeader(response.Headers.AllKeys); + success = HasMatchingHeader(response.Headers.Select(x => x.Key)); // If not found, and available, check for meta-tag if (success == false && _metaTagOptionAvailable) { - success = DoMetaTagsContainKeyForHeader(response); + success = await DoMetaTagsContainKeyForHeader(response); } message = success - ? TextService.Localize($"healthcheck/{_localizedTextPrefix}CheckHeaderFound") - : TextService.Localize($"healthcheck/{_localizedTextPrefix}CheckHeaderNotFound"); + ? LocalizedTextService.Localize($"healthcheck/{_localizedTextPrefix}CheckHeaderFound") + : LocalizedTextService.Localize($"healthcheck/{_localizedTextPrefix}CheckHeaderNotFound"); } catch (Exception ex) { - message = TextService.Localize("healthcheck/healthCheckInvalidUrl", new[] { url.ToString(), ex.Message }); - } - - var actions = new List(); - if (success == false) - { - actions.Add(new HealthCheckAction(SetHeaderInConfigAction, Id) - { - Name = TextService.Localize("healthcheck/setHeaderInConfig"), - Description = TextService.Localize($"healthcheck/{_localizedTextPrefix}SetHeaderInConfigDescription") - }); + message = LocalizedTextService.Localize("healthcheck/healthCheckInvalidUrl", new[] { url.ToString(), ex.Message }); } return new HealthCheckStatus(message) { ResultType = success ? StatusResultType.Success : StatusResultType.Error, - Actions = actions + ReadMoreLink = success ? null : ReadMoreLink }; } private bool HasMatchingHeader(IEnumerable headerKeys) - { - return headerKeys.Contains(_header, StringComparer.InvariantCultureIgnoreCase); - } + => headerKeys.Contains(_header, StringComparer.InvariantCultureIgnoreCase); - private bool DoMetaTagsContainKeyForHeader(WebResponse response) + private async Task DoMetaTagsContainKeyForHeader(HttpResponseMessage response) { - using (var stream = response.GetResponseStream()) + using (Stream stream = await response.Content.ReadAsStreamAsync()) { - if (stream == null) return false; + if (stream == null) + { + return false; + } + using (var reader = new StreamReader(stream)) { var html = reader.ReadToEnd(); - var metaTags = ParseMetaTags(html); + Dictionary metaTags = ParseMetaTags(html); return HasMatchingHeader(metaTags.Keys); } } @@ -138,27 +139,5 @@ namespace Umbraco.Core.HealthCheck.Checks.Security .Cast() .ToDictionary(m => m.Groups[1].Value, m => m.Groups[2].Value); } - - private HealthCheckStatus SetHeaderInConfig() - { - var errorMessage = string.Empty; - //TODO: edit to show fix suggestion instead of making fix - var success = true; - - if (success) - { - return - new HealthCheckStatus(TextService.Localize(string.Format("healthcheck/{0}SetHeaderInConfigSuccess", _localizedTextPrefix))) - { - ResultType = StatusResultType.Success - }; - } - - return - new HealthCheckStatus(TextService.Localize("healthcheck/setHeaderInConfigError", new[] { errorMessage })) - { - ResultType = StatusResultType.Error - }; - } } } diff --git a/src/Umbraco.Core/HealthCheck/Checks/Security/HstsCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Security/HstsCheck.cs index ee8f733fca..e7bc243d0c 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Security/HstsCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Security/HstsCheck.cs @@ -1,8 +1,14 @@ -using Umbraco.Core.Services; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Core.Services; using Umbraco.Web; -namespace Umbraco.Core.HealthCheck.Checks.Security +namespace Umbraco.Core.HealthChecks.Checks.Security { + /// + /// Health check for the recommended production setup regarding the Strict-Transport-Security header. + /// [HealthCheck( "E2048C48-21C5-4BE1-A80B-8062162DF124", "Cookie hijacking and protocol downgrade attacks Protection (Strict-Transport-Security Header (HSTS))", @@ -10,14 +16,22 @@ namespace Umbraco.Core.HealthCheck.Checks.Security Group = "Security")] public class HstsCheck : BaseHttpHeaderCheck { - // The check is mostly based on the instructions in the OWASP CheatSheet - // (https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTTP_Strict_Transport_Security_Cheat_Sheet.md) - // and the blog post of Troy Hunt (https://www.troyhunt.com/understanding-http-strict-transport/) - // If you want do to it perfectly, you have to submit it https://hstspreload.org/, - // but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites. + /// + /// Initializes a new instance of the class. + /// + /// + /// The check is mostly based on the instructions in the OWASP CheatSheet + /// (https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTTP_Strict_Transport_Security_Cheat_Sheet.md) + /// and the blog post of Troy Hunt (https://www.troyhunt.com/understanding-http-strict-transport/) + /// If you want do to it perfectly, you have to submit it https://hstspreload.org/, + /// but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites. + /// public HstsCheck(IRequestAccessor requestAccessor, ILocalizedTextService textService) : base(requestAccessor, textService, "Strict-Transport-Security", "max-age=10886400", "hSTS", true) { } + + /// + protected override string ReadMoreLink => Constants.HealthChecks.DocumentationLinks.Security.HstsCheck; } } diff --git a/src/Umbraco.Core/HealthCheck/Checks/Security/XssProtectionCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Security/XssProtectionCheck.cs index a5f0f28f22..29d14ee238 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Security/XssProtectionCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Security/XssProtectionCheck.cs @@ -1,8 +1,14 @@ -using Umbraco.Core.Services; +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Core.Services; using Umbraco.Web; -namespace Umbraco.Core.HealthCheck.Checks.Security +namespace Umbraco.Core.HealthChecks.Checks.Security { + /// + /// Health check for the recommended production setup regarding the X-XSS-Protection header. + /// [HealthCheck( "F4D2B02E-28C5-4999-8463-05759FA15C3A", "Cross-site scripting Protection (X-XSS-Protection header)", @@ -10,14 +16,22 @@ namespace Umbraco.Core.HealthCheck.Checks.Security Group = "Security")] public class XssProtectionCheck : BaseHttpHeaderCheck { - // The check is mostly based on the instructions in the OWASP CheatSheet - // (https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet) - // and the blog post of Troy Hunt (https://www.troyhunt.com/understanding-http-strict-transport/) - // If you want do to it perfectly, you have to submit it https://hstspreload.appspot.com/, - // but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites. - public XssProtectionCheck(IRequestAccessor requestAccessor,ILocalizedTextService textService) + /// + /// Initializes a new instance of the class. + /// + /// + /// The check is mostly based on the instructions in the OWASP CheatSheet + /// (https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet) + /// and the blog post of Troy Hunt (https://www.troyhunt.com/understanding-http-strict-transport/) + /// If you want do to it perfectly, you have to submit it https://hstspreload.appspot.com/, + /// but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites. + /// + public XssProtectionCheck(IRequestAccessor requestAccessor, ILocalizedTextService textService) : base(requestAccessor, textService, "X-XSS-Protection", "1; mode=block", "xssProtection", true) { } + + /// + protected override string ReadMoreLink => Constants.HealthChecks.DocumentationLinks.Security.XssProtectionCheck; } } diff --git a/src/Umbraco.Core/HealthCheck/HealthCheckCollection.cs b/src/Umbraco.Core/HealthCheck/HealthCheckCollection.cs index fc8d5dff25..88fadee5ec 100644 --- a/src/Umbraco.Core/HealthCheck/HealthCheckCollection.cs +++ b/src/Umbraco.Core/HealthCheck/HealthCheckCollection.cs @@ -1,11 +1,11 @@ using System.Collections.Generic; using Umbraco.Core.Composing; -namespace Umbraco.Core.HealthCheck +namespace Umbraco.Core.HealthChecks { - public class HealthCheckCollection : BuilderCollectionBase + public class HealthCheckCollection : BuilderCollectionBase { - public HealthCheckCollection(IEnumerable items) + public HealthCheckCollection(IEnumerable items) : base(items) { } } diff --git a/src/Umbraco.Core/HealthCheck/IConfigurationService.cs b/src/Umbraco.Core/HealthCheck/IConfigurationService.cs deleted file mode 100644 index dc513bb765..0000000000 --- a/src/Umbraco.Core/HealthCheck/IConfigurationService.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace Umbraco.Core.HealthCheck -{ - public interface IConfigurationService - { - ConfigurationServiceResult UpdateConfigFile(string value, string itemPath); - } -} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 642aaf8814..c38eca8fbc 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -46,4 +46,13 @@ <_Parameter1>DynamicProxyGenAssembly2 + + + + + + + + +