diff --git a/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs b/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs index 444243b0c461..6c31b617a272 100644 --- a/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs +++ b/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Net.Security; using System.Security.Cryptography.X509Certificates; @@ -74,23 +75,36 @@ private static bool ServerCertificateCustomValidation( return sslErrors == SslPolicyErrors.None; } - private HealthCheckStatus? CheckApplicationUrlAvailable() + private bool TryGetApplicationUrl( + [NotNullWhen(true)] out Uri? applicationUrl, + [NotNullWhen(false)] out HealthCheckStatus? unavailableStatus) { - if (_hostingEnvironment.ApplicationMainUrl is not null) + applicationUrl = _hostingEnvironment.ApplicationMainUrl; + if (applicationUrl is not null) { - return null; + unavailableStatus = null; + return true; } - return new HealthCheckStatus( + unavailableStatus = new HealthCheckStatus( _textService.Localize("healthcheck", "httpsCheckNoApplicationUrl")) { ResultType = StatusResultType.Info, }; + return false; } - private async Task CheckForValidCertificate() + /// + /// Checks that the site's HTTPS certificate is valid and not nearing expiry by making a HEAD request to the application URL over HTTPS. + /// + /// + /// Exposed as internal (rather than private) solely to enable direct unit testing of the individual + /// check in isolation; calling would additionally execute the other checks, and with a + /// valid application URL configured that would mean a real outbound HTTPS request on every test run. + /// + internal async Task CheckForValidCertificate() { - if (CheckApplicationUrlAvailable() is HealthCheckStatus unavailable) + if (TryGetApplicationUrl(out Uri? applicationUrl, out HealthCheckStatus? unavailable) is false) { return unavailable; } @@ -99,7 +113,7 @@ private async Task CheckForValidCertificate() StatusResultType result; // Attempt to access the site over HTTPS to see if it HTTPS is supported and a valid certificate has been configured - var urlBuilder = new UriBuilder(_hostingEnvironment.ApplicationMainUrl) { Scheme = Uri.UriSchemeHttps }; + var urlBuilder = new UriBuilder(applicationUrl) { Scheme = Uri.UriSchemeHttps }; Uri url = urlBuilder.Uri; using var request = new HttpRequestMessage(HttpMethod.Head, url); @@ -132,7 +146,7 @@ private async Task CheckForValidCertificate() else if (daysToExpiry < NumberOfDaysForExpiryWarning) { result = StatusResultType.Warning; - message = _textService.Localize("healthcheck", "httpsCheckExpiringCertificate", new[] { daysToExpiry.ToString() }); + message = _textService.Localize("healthcheck", "httpsCheckExpiringCertificate", [daysToExpiry.ToString()]); } else { @@ -143,7 +157,7 @@ private async Task CheckForValidCertificate() else { result = StatusResultType.Error; - message = _textService.Localize("healthcheck", "healthCheckInvalidUrl", new[] { url.AbsoluteUri, response.ReasonPhrase }); + message = _textService.Localize("healthcheck", "healthCheckInvalidUrl", [url.AbsoluteUri, response.ReasonPhrase]); } } catch (Exception ex) @@ -151,12 +165,12 @@ private async Task CheckForValidCertificate() if (ex is WebException exception) { message = exception.Status == WebExceptionStatus.TrustFailure - ? _textService.Localize("healthcheck", "httpsCheckInvalidCertificate", new[] { exception.Message }) - : _textService.Localize("healthcheck", "healthCheckInvalidUrl", new[] { url.AbsoluteUri, exception.Message }); + ? _textService.Localize("healthcheck", "httpsCheckInvalidCertificate", [exception.Message]) + : _textService.Localize("healthcheck", "healthCheckInvalidUrl", [url.AbsoluteUri, exception.Message]); } else { - message = _textService.Localize("healthcheck", "healthCheckInvalidUrl", new[] { url.AbsoluteUri, ex.Message }); + message = _textService.Localize("healthcheck", "healthCheckInvalidUrl", [url.AbsoluteUri, ex.Message]); } result = StatusResultType.Error; @@ -171,18 +185,25 @@ private async Task CheckForValidCertificate() }; } - private Task CheckIfCurrentSchemeIsHttps() + /// + /// Checks whether the application URL's scheme is HTTPS. + /// + /// + /// Exposed as internal (rather than private) solely to enable direct unit testing of the individual + /// check in isolation, so that tests can cover the scheme branches without also running + /// (which would make a real outbound HTTPS request). + /// + internal Task CheckIfCurrentSchemeIsHttps() { - if (CheckApplicationUrlAvailable() is HealthCheckStatus unavailable) + if (TryGetApplicationUrl(out Uri? applicationUrl, out HealthCheckStatus? unavailable) is false) { return Task.FromResult(unavailable); } - Uri uri = _hostingEnvironment.ApplicationMainUrl; - var success = uri.Scheme == Uri.UriSchemeHttps; + var success = applicationUrl.Scheme == Uri.UriSchemeHttps; return Task.FromResult( - new HealthCheckStatus(_textService.Localize("healthcheck", "httpsCheckIsCurrentSchemeHttps", new[] { success ? string.Empty : "not" })) + new HealthCheckStatus(_textService.Localize("healthcheck", "httpsCheckIsCurrentSchemeHttps", [success ? string.Empty : "not"])) { ResultType = success ? StatusResultType.Success : StatusResultType.Error, ReadMoreLink = success @@ -191,26 +212,33 @@ private Task CheckIfCurrentSchemeIsHttps() }); } - private Task CheckHttpsConfigurationSetting() + /// + /// Checks whether the configuration setting agrees with the scheme of the application URL. + /// + /// + /// Exposed as internal (rather than private) solely to enable direct unit testing of the individual + /// check in isolation, so that tests can cover the configuration branches without also running + /// (which would make a real outbound HTTPS request). + /// + internal Task CheckHttpsConfigurationSetting() { - if (CheckApplicationUrlAvailable() is HealthCheckStatus unavailable) + if (TryGetApplicationUrl(out Uri? applicationUrl, out HealthCheckStatus? unavailable) is false) { return Task.FromResult(unavailable); } var httpsSettingEnabled = _globalSettings.CurrentValue.UseHttps; - Uri uri = _hostingEnvironment.ApplicationMainUrl; string resultMessage; StatusResultType resultType; - if (uri.Scheme != Uri.UriSchemeHttps) + if (applicationUrl.Scheme != Uri.UriSchemeHttps) { resultMessage = _textService.Localize("healthcheck", "httpsCheckConfigurationRectifyNotPossible"); resultType = StatusResultType.Info; } else { - resultMessage = _textService.Localize("healthcheck", "httpsCheckConfigurationCheckResult", new[] { httpsSettingEnabled.ToString(), httpsSettingEnabled ? string.Empty : "not" }); + resultMessage = _textService.Localize("healthcheck", "httpsCheckConfigurationCheckResult", [httpsSettingEnabled.ToString(), httpsSettingEnabled ? string.Empty : "not"]); resultType = httpsSettingEnabled ? StatusResultType.Success : StatusResultType.Error; } diff --git a/src/Umbraco.Core/Hosting/IHostingEnvironment.cs b/src/Umbraco.Core/Hosting/IHostingEnvironment.cs index ad507255af65..45f470dcf22d 100644 --- a/src/Umbraco.Core/Hosting/IHostingEnvironment.cs +++ b/src/Umbraco.Core/Hosting/IHostingEnvironment.cs @@ -81,10 +81,22 @@ public interface IHostingEnvironment bool IsHosted { get; } /// - /// Gets the main application url. + /// Gets the main application URL, or if no URL has been configured or detected yet. /// - // TODO (V18): Change to Uri? to reflect that this can be null when ApplicationUrlDetection is None and no explicit URL is configured. - Uri ApplicationMainUrl { get; } + /// + /// + /// The value is populated from when + /// it is set, and otherwise from the first (or every) incoming request depending on + /// . + /// + /// + /// This property can be — for example when + /// is configured and no explicit + /// is provided, or before the first + /// request has been observed. Consumers must therefore null-check before use. + /// + /// + Uri? ApplicationMainUrl { get; } /// /// Maps a virtual path to a physical path to the application's web root diff --git a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs index 4c0600be41d3..193736a452d5 100644 --- a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs +++ b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs @@ -81,9 +81,9 @@ public AspNetCoreHostingEnvironment( private Uri? _applicationMainUrl; /// - public Uri ApplicationMainUrl + public Uri? ApplicationMainUrl { - get => _applicationMainUrl!; + get => _applicationMainUrl; private set => _applicationMainUrl = value; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs new file mode 100644 index 000000000000..7eab87ff418a --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs @@ -0,0 +1,127 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Globalization; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.HealthChecks; +using Umbraco.Cms.Core.HealthChecks.Checks.Security; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.HealthChecks; + +[TestFixture] +public class HttpsCheckTests +{ + private const string HttpsUrl = "https://example.com/"; + private const string HttpUrl = "http://example.com/"; + + [Test] + public async Task GetStatusAsync_WhenApplicationMainUrlIsNull_AllChecksReturnInfo() + { + HttpsCheck check = CreateCheck(applicationMainUrl: null); + + HealthCheckStatus[] statuses = (await check.GetStatusAsync()).ToArray(); + + Assert.That(statuses, Has.Length.EqualTo(3)); + Assert.Multiple(() => + { + foreach (HealthCheckStatus status in statuses) + { + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Info)); + Assert.That(status.Message, Is.EqualTo("httpsCheckNoApplicationUrl")); + } + }); + } + + [Test] + public async Task CheckIfCurrentSchemeIsHttps_WhenUrlIsHttps_ReturnsSuccess() + { + HttpsCheck check = CreateCheck(applicationMainUrl: new Uri(HttpsUrl)); + + HealthCheckStatus status = await check.CheckIfCurrentSchemeIsHttps(); + + Assert.Multiple(() => + { + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Success)); + Assert.That(status.ReadMoreLink, Is.Null.Or.Empty); + }); + } + + [Test] + public async Task CheckIfCurrentSchemeIsHttps_WhenUrlIsHttp_ReturnsError() + { + HttpsCheck check = CreateCheck(applicationMainUrl: new Uri(HttpUrl)); + + HealthCheckStatus status = await check.CheckIfCurrentSchemeIsHttps(); + + Assert.Multiple(() => + { + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Error)); + Assert.That(status.ReadMoreLink, Is.Not.Null.And.Not.Empty); + }); + } + + [Test] + public async Task CheckHttpsConfigurationSetting_WhenUrlIsHttp_ReturnsInfo() + { + HttpsCheck check = CreateCheck(applicationMainUrl: new Uri(HttpUrl), useHttps: false); + + HealthCheckStatus status = await check.CheckHttpsConfigurationSetting(); + + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Info)); + } + + [Test] + public async Task CheckHttpsConfigurationSetting_WhenUrlIsHttpsAndUseHttpsEnabled_ReturnsSuccess() + { + HttpsCheck check = CreateCheck(applicationMainUrl: new Uri(HttpsUrl), useHttps: true); + + HealthCheckStatus status = await check.CheckHttpsConfigurationSetting(); + + Assert.Multiple(() => + { + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Success)); + Assert.That(status.ReadMoreLink, Is.Null.Or.Empty); + }); + } + + [Test] + public async Task CheckHttpsConfigurationSetting_WhenUrlIsHttpsAndUseHttpsDisabled_ReturnsError() + { + HttpsCheck check = CreateCheck(applicationMainUrl: new Uri(HttpsUrl), useHttps: false); + + HealthCheckStatus status = await check.CheckHttpsConfigurationSetting(); + + Assert.Multiple(() => + { + Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Error)); + Assert.That(status.ReadMoreLink, Is.Not.Null.And.Not.Empty); + }); + } + + // ILocalizedTextService.Localize is called via extensions that delegate to the 4-arg interface method. + // We return the alias so tests can assert on which key was used. + private static ILocalizedTextService MockTextService() + { + var mock = new Mock(); + mock.Setup(x => x.Localize( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns((string? _, string? alias, CultureInfo? _, IDictionary _) => alias ?? string.Empty); + return mock.Object; + } + + private static HttpsCheck CreateCheck(Uri? applicationMainUrl, bool useHttps = false) + { + IHostingEnvironment hostingEnvironment = Mock.Of(x => x.ApplicationMainUrl == applicationMainUrl); + IOptionsMonitor globalSettings = + Mock.Of>(m => m.CurrentValue == new GlobalSettings { UseHttps = useHttps }); + return new HttpsCheck(MockTextService(), globalSettings, hostingEnvironment); + } +}