Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 50 additions & 22 deletions src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<HealthCheckStatus> CheckForValidCertificate()
/// <summary>
/// Checks that the site's HTTPS certificate is valid and not nearing expiry by making a HEAD request to the application URL over HTTPS.
/// </summary>
/// <remarks>
/// Exposed as <c>internal</c> (rather than <c>private</c>) solely to enable direct unit testing of the individual
/// check in isolation; calling <see cref="GetStatusAsync" /> 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.
/// </remarks>
internal async Task<HealthCheckStatus> CheckForValidCertificate()
{
if (CheckApplicationUrlAvailable() is HealthCheckStatus unavailable)
if (TryGetApplicationUrl(out Uri? applicationUrl, out HealthCheckStatus? unavailable) is false)
{
return unavailable;
}
Expand All @@ -99,7 +113,7 @@ private async Task<HealthCheckStatus> 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);
Expand Down Expand Up @@ -132,7 +146,7 @@ private async Task<HealthCheckStatus> CheckForValidCertificate()
else if (daysToExpiry < NumberOfDaysForExpiryWarning)
{
result = StatusResultType.Warning;
message = _textService.Localize("healthcheck", "httpsCheckExpiringCertificate", new[] { daysToExpiry.ToString() });
message = _textService.Localize("healthcheck", "httpsCheckExpiringCertificate", [daysToExpiry.ToString()]);
}
else
{
Expand All @@ -143,20 +157,20 @@ private async Task<HealthCheckStatus> 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)
{
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;
Expand All @@ -171,18 +185,25 @@ private async Task<HealthCheckStatus> CheckForValidCertificate()
};
}

private Task<HealthCheckStatus> CheckIfCurrentSchemeIsHttps()
/// <summary>
/// Checks whether the application URL's scheme is HTTPS.
/// </summary>
/// <remarks>
/// Exposed as <c>internal</c> (rather than <c>private</c>) solely to enable direct unit testing of the individual
/// check in isolation, so that tests can cover the scheme branches without also running <see cref="CheckForValidCertificate" />
/// (which would make a real outbound HTTPS request).
/// </remarks>
internal Task<HealthCheckStatus> 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
Expand All @@ -191,26 +212,33 @@ private Task<HealthCheckStatus> CheckIfCurrentSchemeIsHttps()
});
}

private Task<HealthCheckStatus> CheckHttpsConfigurationSetting()
/// <summary>
/// Checks whether the <see cref="GlobalSettings.UseHttps" /> configuration setting agrees with the scheme of the application URL.
/// </summary>
/// <remarks>
/// Exposed as <c>internal</c> (rather than <c>private</c>) solely to enable direct unit testing of the individual
/// check in isolation, so that tests can cover the configuration branches without also running <see cref="CheckForValidCertificate" />
/// (which would make a real outbound HTTPS request).
/// </remarks>
internal Task<HealthCheckStatus> 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;
}

Expand Down
18 changes: 15 additions & 3 deletions src/Umbraco.Core/Hosting/IHostingEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,22 @@ public interface IHostingEnvironment
bool IsHosted { get; }

/// <summary>
/// Gets the main application url.
/// Gets the main application URL, or <see langword="null" /> if no URL has been configured or detected yet.
/// </summary>
// 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; }
/// <remarks>
/// <para>
/// The value is populated from <see cref="Configuration.Models.WebRoutingSettings.UmbracoApplicationUrl" /> when
/// it is set, and otherwise from the first (or every) incoming request depending on
/// <see cref="Configuration.Models.WebRoutingSettings.ApplicationUrlDetection" />.
/// </para>
/// <para>
/// This property can be <see langword="null" /> — for example when
/// <see cref="Configuration.Models.ApplicationUrlDetection.None" /> is configured and no explicit
/// <see cref="Configuration.Models.WebRoutingSettings.UmbracoApplicationUrl" /> is provided, or before the first
/// request has been observed. Consumers must therefore null-check before use.
/// </para>
/// </remarks>
Uri? ApplicationMainUrl { get; }

/// <summary>
/// Maps a virtual path to a physical path to the application's web root
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ public AspNetCoreHostingEnvironment(
private Uri? _applicationMainUrl;

/// <inheritdoc />
public Uri ApplicationMainUrl
public Uri? ApplicationMainUrl
{
get => _applicationMainUrl!;
get => _applicationMainUrl;
private set => _applicationMainUrl = value;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
}

Check warning on line 52 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

❌ New issue: Code Duplication

The module contains 4 functions with similar structure: CheckHttpsConfigurationSetting_WhenUrlIsHttpsAndUseHttpsDisabled_ReturnsError,CheckHttpsConfigurationSetting_WhenUrlIsHttpsAndUseHttpsEnabled_ReturnsSuccess,CheckIfCurrentSchemeIsHttps_WhenUrlIsHttp_ReturnsError,CheckIfCurrentSchemeIsHttps_WhenUrlIsHttps_ReturnsSuccess. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

[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<ILocalizedTextService>();
mock.Setup(x => x.Localize(
It.IsAny<string?>(),
It.IsAny<string?>(),
It.IsAny<CultureInfo?>(),
It.IsAny<IDictionary<string, string?>>()))
.Returns((string? _, string? alias, CultureInfo? _, IDictionary<string, string?> _) => alias ?? string.Empty);
return mock.Object;
}

private static HttpsCheck CreateCheck(Uri? applicationMainUrl, bool useHttps = false)
{
IHostingEnvironment hostingEnvironment = Mock.Of<IHostingEnvironment>(x => x.ApplicationMainUrl == applicationMainUrl);
IOptionsMonitor<GlobalSettings> globalSettings =
Mock.Of<IOptionsMonitor<GlobalSettings>>(m => m.CurrentValue == new GlobalSettings { UseHttps = useHttps });
return new HttpsCheck(MockTextService(), globalSettings, hostingEnvironment);
}
}
Loading