yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Uses correct preview cookie same site and secure settings to allow preview mode to flow between links in the preview frame (#18640)

Browse Source 
* Uses correct preview cookie same site and secure settings to allow preview mode to flow between links in the preview frame.

* Fixed comment.
This commit is contained in:
Andy Butland
2025-03-21 15:20:08 +01:00
committed by GitHub
parent 45b0e43b89
commit 394210a8f7
7 changed files with 235 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/Umbraco.Core/Services/PreviewService.cs
View File

@@ -1,4 +1,4 @@
using System.Security.Claims;
using System.Security.Claims;
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Models.Membership;
@@ -34,7 +34,7 @@ public class PreviewService : IPreviewService
if (attempt.Success)
{
_cookieManager.SetCookieValue(Constants.Web.PreviewCookieName, attempt.Result!, true);
_cookieManager.SetCookieValue(Constants.Web.PreviewCookieName, attempt.Result!, true, true, "None");
}
return attempt.Success;

46
src/Umbraco.Core/Web/ICookieManager.cs
View File

@@ -1,15 +1,61 @@
namespace Umbraco.Cms.Core.Web;
/// <summary>
/// Defines cookie related operations.
/// </summary>
public interface ICookieManager
{
/// <summary>
/// Expires the cookie with the specified name.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
void ExpireCookie(string cookieName);
/// <summary>
/// Gets the value of the cookie with the specified name.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
string? GetCookieValue(string cookieName);
/// <summary>
/// Sets the value of a cookie with the specified name.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
/// <param name="value">The cookie value.</param>
[Obsolete("Use overload with the httpOnly parameter instead. Scheduled for removal in V16.")]
void SetCookieValue(string cookieName, string value) => SetCookieValue(cookieName, value, false);
/// <summary>
/// Sets the value of a cookie with the specified name.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
/// <param name="value">The cookie value.</param>
/// <param name="httpOnly">Indicates whether the created cookie should be marked as HTTP only.</param>
[Obsolete("Use overload with the secure and sameSiteMode parameters instead. Scheduled for removal in V17.")]
void SetCookieValue(string cookieName, string value, bool httpOnly);
/// <summary>
/// Sets the value of a cookie with the specified name.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
/// <param name="value">The cookie value.</param>
/// <param name="httpOnly">Indicates whether the created cookie should be marked as HTTP only.</param>
/// <param name="secure">Indicates whether the created cookie should be marked as secure.</param>
/// <param name="sameSiteMode">Indicates the created cookie's same site status.</param>
/// <remarks>
/// The value provided by <paramref name="sameSiteMode"/> should match the enum values available from
/// Microsoft.AspNetCore.Http.SameSiteMode.
/// This hasn't been used as the parameter directly to avoid a dependency on Microsoft.AspNetCore.Http in
/// the core project.
/// </remarks>
void SetCookieValue(string cookieName, string value, bool httpOnly, bool secure, string sameSiteMode)
#pragma warning disable CS0618 // Type or member is obsolete
=> SetCookieValue(cookieName, value, httpOnly);
#pragma warning restore CS0618 // Type or member is obsolete
/// <summary>
/// Determines whether a cookie with the specified name exists.
/// </summary>
/// <param name="cookieName">The cookie name.</param>
bool HasCookie(string cookieName);
}

2
src/Umbraco.Infrastructure/Install/InstallHelper.cs
View File

@@ -62,7 +62,7 @@ namespace Umbraco.Cms.Infrastructure.Install
{
installId = Guid.NewGuid();
_cookieManager.SetCookieValue(Constants.Web.InstallerCookieName, installId.ToString(), false);
_cookieManager.SetCookieValue(Constants.Web.InstallerCookieName, installId.ToString(), false, false, "Unspecified");
}
var dbProvider = string.Empty;

45
src/Umbraco.Web.Common/AspNetCore/AspNetCoreCookieManager.cs
View File

@@ -3,13 +3,21 @@ using Umbraco.Cms.Core.Web;
namespace Umbraco.Cms.Web.Common.AspNetCore;
/// <summary>
/// An <see cref="ICookieManager"/> implementation for ASP.NET Core.
/// </summary>
public class AspNetCoreCookieManager : ICookieManager
{
private readonly IHttpContextAccessor _httpContextAccessor;
/// <summary>
/// Initializes a new instance of the <see cref="AspNetCoreCookieManager"/> class.
/// </summary>
/// <param name="httpContextAccessor">The <see href="IHttpContextAccessor" />.</param>
public AspNetCoreCookieManager(IHttpContextAccessor httpContextAccessor) =>
_httpContextAccessor = httpContextAccessor;
/// <inheritdoc/>
public void ExpireCookie(string cookieName)
{
HttpContext? httpContext = _httpContextAccessor.HttpContext;
@@ -21,14 +29,43 @@ public class AspNetCoreCookieManager : ICookieManager
var cookieValue = httpContext.Request.Cookies[cookieName];
httpContext.Response.Cookies.Append(cookieName, cookieValue ?? string.Empty,
new CookieOptions { Expires = DateTime.Now.AddYears(-1) });
httpContext.Response.Cookies.Append(
cookieName,
cookieValue ?? string.Empty,
new CookieOptions
{
Expires = DateTime.Now.AddYears(-1),
});
}
/// <inheritdoc/>
public string? GetCookieValue(string cookieName) => _httpContextAccessor.HttpContext?.Request.Cookies[cookieName];
/// <inheritdoc/>
[Obsolete("Use overload with the secure and sameSiteMode parameters instead. Scheduled for removal in V17.")]
public void SetCookieValue(string cookieName, string value, bool httpOnly) =>
_httpContextAccessor.HttpContext?.Response.Cookies.Append(cookieName, value, new CookieOptions { HttpOnly = httpOnly });
SetCookieValue(cookieName, value, httpOnly, false, SameSiteMode.Unspecified.ToString());
public bool HasCookie(string cookieName) => !(GetCookieValue(cookieName) is null);
/// <inheritdoc/>
public void SetCookieValue(string cookieName, string value, bool httpOnly, bool secure, string sameSiteMode)
{
SameSiteMode sameSiteModeValue = ParseSameSiteMode(sameSiteMode);
_httpContextAccessor.HttpContext?.Response.Cookies.Append(
cookieName,
value,
new CookieOptions
{
HttpOnly = httpOnly,
SameSite = sameSiteModeValue,
Secure = secure,
});
}
private static SameSiteMode ParseSameSiteMode(string sameSiteMode) =>
Enum.TryParse(sameSiteMode, ignoreCase: true, out SameSiteMode result)
? result
: throw new ArgumentException($"The provided {nameof(sameSiteMode)} value could not be parsed into as SameSiteMode value.", nameof(sameSiteMode));
/// <inheritdoc/>
public bool HasCookie(string cookieName) => GetCookieValue(cookieName) is not null;
}

2
src/Umbraco.Web.Common/Profiler/WebProfiler.cs
View File

@@ -109,7 +109,7 @@ public class WebProfiler : IProfiler
&& !location.Contains("://"))
{
MiniProfilerContext.Value.Root.Name = "Before Redirect";
cookieManager.SetCookieValue(WebProfileCookieKey, MiniProfilerContext.Value.ToJson(), false);
cookieManager.SetCookieValue(WebProfileCookieKey, MiniProfilerContext.Value.ToJson(), false, false, "Unspecified");
}
}
}

53
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/PreviewServiceTests.cs Normal file
View File

@@ -0,0 +1,53 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Preview;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services;
[TestFixture]
public class PreviewServiceTests
{
[Test]
public async Task TryEnterPreviewAsync_Sets_Expected_Cookie_On_Successful_Token_Generation()
{
var userKey = Guid.NewGuid();
const string Token = "token";
var cookieManagerMock = new Mock<ICookieManager>();
var previewTokenGeneratorMock = new Mock<IPreviewTokenGenerator>();
previewTokenGeneratorMock
.Setup(x => x.GenerateTokenAsync(It.Is<Guid>(y => y == userKey)))
.ReturnsAsync(Attempt<string?>.Succeed(Token));
var previewService = new PreviewService(
cookieManagerMock.Object,
previewTokenGeneratorMock.Object,
Mock.Of<IServiceScopeFactory>(),
Mock.Of<IRequestCache>());
var user = new UserBuilder()
.WithKey(userKey)
.Build();
var result = await previewService.TryEnterPreviewAsync(user);
cookieManagerMock
.Verify(x => x.SetCookieValue(
It.Is<string>(y => y == Constants.Web.PreviewCookieName),
It.Is<string>(y => y == Token),
It.Is<bool>(y => y == true),
It.Is<bool>(y => y == true),
It.Is<string>(y => y == SameSiteMode.None.ToString())));
Assert.IsTrue(result);
}
}

91
tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/AspNetCore/AspNetCoreCookieManagerTests.cs Normal file
View File

@@ -0,0 +1,91 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;
using Microsoft.Net.Http.Headers;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Web.Common.AspNetCore;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Extensions;
[TestFixture]
public class AspNetCoreCookieManagerTests
{
private const string CookieName = "testCookie";
private const string CookieValue = "testValue";
[Test]
public void Can_Set_Cookie()
{
var httpContext = new DefaultHttpContext();
var cookieManager = CreateCookieManager(httpContext);
cookieManager.SetCookieValue(CookieName, CookieValue, true, true, "Strict");
Assert.AreEqual(GetExpectedCookie(), httpContext.Response.Headers.SetCookie);
}
[Test]
public void Set_Cookie_With_Invalid_Same_Site_Value_Throws_Expected_Exception()
{
var httpContext = new DefaultHttpContext();
var cookieManager = CreateCookieManager(httpContext);
Assert.Throws<ArgumentException>(() => cookieManager.SetCookieValue(CookieName, CookieValue, true, true, "invalid"));
}
[Test]
public void Can_Get_Cookie()
{
var httpContext = new DefaultHttpContext();
AddCookieToRequest(httpContext);
var cookieManager = CreateCookieManager(httpContext);
var result = cookieManager.GetCookieValue(CookieName);
Assert.AreEqual(CookieValue, result);
}
[Test]
public void Can_Verify_Cookie_Exists()
{
var httpContext = new DefaultHttpContext();
AddCookieToRequest(httpContext);
var cookieManager = CreateCookieManager(httpContext);
var result = cookieManager.HasCookie(CookieName);
Assert.IsTrue(result);
}
[Test]
public void Can_Expire_Cookie()
{
var httpContext = new DefaultHttpContext();
AddCookieToRequest(httpContext);
var cookieManager = CreateCookieManager(httpContext);
cookieManager.SetCookieValue(CookieName, CookieValue, true, true, "Strict");
cookieManager.ExpireCookie(CookieName);
var setCookieHeader = httpContext.Response.Headers.SetCookie.ToString();
Assert.IsTrue(setCookieHeader.StartsWith(GetExpectedCookie()));
Assert.IsTrue(setCookieHeader.Contains($"expires="));
}
private static AspNetCoreCookieManager CreateCookieManager(DefaultHttpContext httpContext)
{
var httpContextAccessor = Mock.Of<IHttpContextAccessor>(x => x.HttpContext == httpContext);
return new AspNetCoreCookieManager(httpContextAccessor);
}
private static void AddCookieToRequest(DefaultHttpContext httpContext)
{
var cookie = new StringValues(CookieName + "=" + CookieValue);
httpContext.Request.Headers.Append(HeaderNames.Cookie, cookie);
}
private static string GetExpectedCookie() => $"testCookie={CookieValue}; path=/; secure; samesite=strict; httponly";
}