Skip to content

Commit

Permalink
[v9] Fix the basehttpheader health check so that it's checking the ro…
Browse files Browse the repository at this point in the history
…ot of the domain instead of the /umbraco path (umbraco#11535)

* Fix the basehttpheader health check so that it's checking the root of the domain instead of the /umbraco path.

* Remove unused value from security health checks (it was used in v8 for fixing)
  • Loading branch information
Jeavon authored Oct 29, 2021
1 parent dfdb498 commit acaece6
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using System;
Expand All @@ -20,8 +20,8 @@ namespace Umbraco.Cms.Core.HealthChecks.Checks.Security
public abstract class BaseHttpHeaderCheck : HealthCheck
{
private readonly IHostingEnvironment _hostingEnvironment;
private readonly ILocalizedTextService _textService;
private readonly string _header;
private readonly string _value;
private readonly string _localizedTextPrefix;
private readonly bool _metaTagOptionAvailable;
private static HttpClient s_httpClient;
Expand All @@ -33,26 +33,18 @@ protected BaseHttpHeaderCheck(
IHostingEnvironment hostingEnvironment,
ILocalizedTextService textService,
string header,
string value,
string localizedTextPrefix,
bool metaTagOptionAvailable)
{
LocalizedTextService = textService ?? throw new ArgumentNullException(nameof(textService));
_textService = textService ?? throw new ArgumentNullException(nameof(textService));
_hostingEnvironment = hostingEnvironment;
_header = header;
_value = value;
_localizedTextPrefix = localizedTextPrefix;
_metaTagOptionAvailable = metaTagOptionAvailable;
}

private static HttpClient HttpClient => s_httpClient ??= new HttpClient();


/// <summary>
/// Gets the localized text service.
/// </summary>
protected ILocalizedTextService LocalizedTextService { get; }

/// <summary>
/// Gets a link to an external read more page.
/// </summary>
Expand All @@ -79,7 +71,7 @@ protected async Task<HealthCheckStatus> CheckForHeader()
var success = false;

// Access the site home page and check for the click-jack protection header or meta tag
Uri url = _hostingEnvironment.ApplicationMainUrl;
var url = _hostingEnvironment.ApplicationMainUrl.GetLeftPart(UriPartial.Authority);

try
{
Expand All @@ -95,12 +87,12 @@ protected async Task<HealthCheckStatus> CheckForHeader()
}

message = success
? LocalizedTextService.Localize($"healthcheck", $"{_localizedTextPrefix}CheckHeaderFound")
: LocalizedTextService.Localize($"healthcheck", $"{_localizedTextPrefix}CheckHeaderNotFound");
? _textService.Localize($"healthcheck", $"{_localizedTextPrefix}CheckHeaderFound")
: _textService.Localize($"healthcheck", $"{_localizedTextPrefix}CheckHeaderNotFound");
}
catch (Exception ex)
{
message = LocalizedTextService.Localize("healthcheck","healthCheckInvalidUrl", new[] { url.ToString(), ex.Message });
message = _textService.Localize("healthcheck","healthCheckInvalidUrl", new[] { url.ToString(), ex.Message });
}

return
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Umbraco.Cms.Core.Hosting;
Expand All @@ -20,7 +20,7 @@ public class ClickJackingCheck : BaseHttpHeaderCheck
/// Initializes a new instance of the <see cref="ClickJackingCheck"/> class.
/// </summary>
public ClickJackingCheck(IHostingEnvironment hostingEnvironment, ILocalizedTextService textService)
: base(hostingEnvironment, textService, "X-Frame-Options", "sameorigin", "clickJacking", true)
: base(hostingEnvironment, textService, "X-Frame-Options", "clickJacking", true)
{
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using System;
Expand Down Expand Up @@ -53,7 +53,7 @@ private async Task<HealthCheckStatus> CheckForHeaders()
{
string message;
var success = false;
var url = _hostingEnvironment.ApplicationMainUrl.GetLeftPart(UriPartial.Authority);;
var url = _hostingEnvironment.ApplicationMainUrl.GetLeftPart(UriPartial.Authority);

// Access the site home page and check for the headers
var request = new HttpRequestMessage(HttpMethod.Head, url);
Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/HealthChecks/Checks/Security/HstsCheck.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Umbraco.Cms.Core.Hosting;
Expand Down Expand Up @@ -27,7 +27,7 @@ public class HstsCheck : BaseHttpHeaderCheck
/// but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites.
/// </remarks>
public HstsCheck(IHostingEnvironment hostingEnvironment, ILocalizedTextService textService)
: base(hostingEnvironment, textService, "Strict-Transport-Security", "max-age=10886400", "hSTS", true)
: base(hostingEnvironment, textService, "Strict-Transport-Security", "hSTS", true)
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/HealthChecks/Checks/Security/NoSniffCheck.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Umbraco.Cms.Core.Hosting;
Expand All @@ -20,7 +20,7 @@ public class NoSniffCheck : BaseHttpHeaderCheck
/// Initializes a new instance of the <see cref="NoSniffCheck"/> class.
/// </summary>
public NoSniffCheck(IHostingEnvironment hostingEnvironment, ILocalizedTextService textService)
: base(hostingEnvironment, textService, "X-Content-Type-Options", "nosniff", "noSniff", false)
: base(hostingEnvironment, textService, "X-Content-Type-Options", "noSniff", false)
{
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Umbraco.Cms.Core.Hosting;
Expand Down Expand Up @@ -27,7 +27,7 @@ public class XssProtectionCheck : BaseHttpHeaderCheck
/// but then you should include subdomains and I wouldn't suggest to do that for Umbraco-sites.
/// </remarks>
public XssProtectionCheck(IHostingEnvironment hostingEnvironment, ILocalizedTextService textService)
: base(hostingEnvironment, textService, "X-XSS-Protection", "1; mode=block", "xssProtection", true)
: base(hostingEnvironment, textService, "X-XSS-Protection", "xssProtection", true)
{
}

Expand Down

0 comments on commit acaece6

Please sign in to comment.