diff --git a/build/azure-pipelines.yml b/build/azure-pipelines.yml
index 8d3f9edc0f..aa0ddf1db6 100644
--- a/build/azure-pipelines.yml
+++ b/build/azure-pipelines.yml
@@ -71,7 +71,7 @@ stages:
- job: A
displayName: Build Umbraco CMS
pool:
- vmImage: 'ubuntu-latest'
+ vmImage: 'windows-latest'
steps:
- checkout: self
submodules: false
diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs
index 4e2faeb1c7..fc226c5589 100644
--- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs
+++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs
@@ -20,6 +20,8 @@ public class
{
private readonly ContentPermissions _contentPermissions;
+ protected override UmbracoObjectTypes KeyParsingFilterType => UmbracoObjectTypes.Document;
+
///
/// Initializes a new instance of the class.
///
@@ -48,7 +50,11 @@ public class
return Task.FromResult(true);
}
- var argument = routeVal.ToString();
+ // Handle case where the incoming querystring could contain more than one value (e.g. ?id=1000&id=1001).
+ // It's the first one that'll be processed by the protected method so we should verify that.
+ var argument = routeVal.Count == 1
+ ? routeVal.ToString()
+ : routeVal.FirstOrDefault()?.ToString() ?? string.Empty;
if (!TryParseNodeId(argument, out nodeId))
{
diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs
index 7b662e5fc0..e8a8f36aca 100644
--- a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs
+++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs
@@ -18,6 +18,8 @@ public class MediaPermissionsQueryStringHandler : PermissionsQueryStringHandler<
{
private readonly MediaPermissions _mediaPermissions;
+ protected override UmbracoObjectTypes KeyParsingFilterType => UmbracoObjectTypes.Media;
+
///
/// Initializes a new instance of the class.
///
@@ -44,7 +46,11 @@ public class MediaPermissionsQueryStringHandler : PermissionsQueryStringHandler<
return Task.FromResult(true);
}
- var argument = routeVal.ToString();
+ // Handle case where the incoming querystring could contain more than one value (e.g. ?id=1000&id=1001).
+ // It's the first one that'll be processed by the protected method so we should verify that.
+ var argument = routeVal.Count == 1
+ ? routeVal.ToString()
+ : routeVal.FirstOrDefault()?.ToString() ?? string.Empty;
if (!TryParseNodeId(argument, out var nodeId))
{
diff --git a/src/Umbraco.Web.BackOffice/Authorization/PermissionsQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/PermissionsQueryStringHandler.cs
index 5367208c79..44b7c5ba8d 100644
--- a/src/Umbraco.Web.BackOffice/Authorization/PermissionsQueryStringHandler.cs
+++ b/src/Umbraco.Web.BackOffice/Authorization/PermissionsQueryStringHandler.cs
@@ -49,12 +49,18 @@ public abstract class PermissionsQueryStringHandler : MustSatisfyRequirementA
///
protected IEntityService EntityService { get; set; }
+ ///
+ /// Defaults to Unknown so all types are allowed, since Keys are unique across all node types this works,
+ /// but it if you are certain you are looking for a specific type this should be overwritten for DB query performance.
+ ///
+ protected virtual UmbracoObjectTypes KeyParsingFilterType => UmbracoObjectTypes.Unknown;
+
///
/// Attempts to parse a node ID from a string representation found in a querystring value.
///
/// Querystring value.
/// Output parsed Id.
- /// True of node ID could be parased, false it not.
+ /// True of node ID could be parsed, false it not.
protected bool TryParseNodeId(string argument, out int nodeId)
{
// If the argument is an int, it will parse and can be assigned to nodeId.
@@ -75,7 +81,7 @@ public abstract class PermissionsQueryStringHandler : MustSatisfyRequirementA
if (Guid.TryParse(argument, out Guid key))
{
- nodeId = EntityService.GetId(key, UmbracoObjectTypes.Document).Result;
+ nodeId = EntityService.GetId(key, KeyParsingFilterType).Result;
return true;
}
diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs
index a1c5e55ac8..2f71f5ef3f 100644
--- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs
+++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs
@@ -256,6 +256,7 @@ public class ContentController : ContentControllerBase
/// Permission check is done for letter 'R' which is for which the user must have access to
/// update
///
+ [HttpPost]
public async Task?>> PostSaveUserGroupPermissions(
UserGroupPermissionsSave saveModel)
{
@@ -902,6 +903,7 @@ public class ContentController : ContentControllerBase
[Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)]
[FileUploadCleanupFilter]
[ContentSaveValidation(skipUserAccessValidation:true)] // skip user access validation because we "only" require Settings access to create new blueprints from scratch
+ [HttpPost]
public async Task?>?> PostSaveBlueprint(
[ModelBinder(typeof(BlueprintItemBinder))] ContentItemSave contentItem)
{
@@ -939,6 +941,7 @@ public class ContentController : ContentControllerBase
[FileUploadCleanupFilter]
[ContentSaveValidation]
[OutgoingEditorModelEvent]
+ [HttpPost]
public async Task?>> PostSave(
[ModelBinder(typeof(ContentItemBinder))] ContentItemSave contentItem)
{
@@ -2124,6 +2127,7 @@ public class ContentController : ContentControllerBase
/// does not have Publish access to this node.
///
[Authorize(Policy = AuthorizationPolicies.ContentPermissionPublishById)]
+ [HttpPost]
public IActionResult PostPublishById(int id)
{
IContent? foundContent = GetObjectFromRequest(() => _contentService.GetById(id));
@@ -2155,6 +2159,7 @@ public class ContentController : ContentControllerBase
/// does not have Publish access to this node.
///
[Authorize(Policy = AuthorizationPolicies.ContentPermissionPublishById)]
+ [HttpPost]
public IActionResult PostPublishByIdAndCulture(PublishContent model)
{
var languageCount = _allLangs.Value.Count();
@@ -2278,6 +2283,7 @@ public class ContentController : ContentControllerBase
///
///
///
+ [HttpPost]
public async Task PostSort(ContentSortOrder sorted)
{
if (sorted == null)
@@ -2329,6 +2335,7 @@ public class ContentController : ContentControllerBase
///
///
///
+ [HttpPost]
public async Task PostMove(MoveOrCopy move)
{
// Authorize...
@@ -2368,6 +2375,7 @@ public class ContentController : ContentControllerBase
///
///
///
+ [HttpPost]
public async Task?> PostCopy(MoveOrCopy copy)
{
// Authorize...
@@ -2407,6 +2415,7 @@ public class ContentController : ContentControllerBase
/// The content and variants to unpublish
///
[OutgoingEditorModelEvent]
+ [HttpPost]
public async Task> PostUnpublish(UnpublishContent model)
{
IContent? foundContent = _contentService.GetById(model.Id);
@@ -3131,6 +3140,7 @@ public class ContentController : ContentControllerBase
return notifications;
}
+ [HttpPost]
public IActionResult PostNotificationOptions(
int contentId,
[FromQuery(Name = "notifyOptions[]")] string[] notifyOptions)
diff --git a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs
index e6a131950e..404713a2b9 100644
--- a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs
+++ b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs
@@ -386,6 +386,7 @@ public class MediaController : ContentControllerBase
///
///
///
+ [HttpPost]
public async Task PostMove(MoveOrCopy move)
{
// Authorize...
@@ -436,6 +437,7 @@ public class MediaController : ContentControllerBase
[FileUploadCleanupFilter]
[MediaItemSaveValidation]
[OutgoingEditorModelEvent]
+ [HttpPost]
public ActionResult? PostSave(
[ModelBinder(typeof(MediaItemBinder))] MediaItemSave contentItem)
{
@@ -551,6 +553,7 @@ public class MediaController : ContentControllerBase
///
///
///
+ [HttpPost]
public async Task PostSort(ContentSortOrder sorted)
{
if (sorted == null)
@@ -595,6 +598,7 @@ public class MediaController : ContentControllerBase
}
}
+ [HttpPost]
public async Task> PostAddFolder(PostedFolder folder)
{
ActionResult? parentIdResult = await GetParentIdAsIntAsync(folder.ParentId, true);
@@ -628,6 +632,7 @@ public class MediaController : ContentControllerBase
///
/// We cannot validate this request with attributes (nicely) due to the nature of the multi-part for data.
///
+ [HttpPost]
public async Task PostAddFile([FromForm] string path, [FromForm] string currentFolder,
[FromForm] string contentTypeAlias, List file)
{
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandlerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandlerTests.cs
index c562eb67ea..f7782e8d99 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandlerTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandlerTests.cs
@@ -1,9 +1,7 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
-using System.Collections.Generic;
using System.Security.Claims;
-using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;
@@ -34,7 +32,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Id_From_Requirement_With_Permission_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext(NodeId);
- var mockHttpContextAccessor = CreateMockHttpContextAccessor();
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue();
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -46,7 +44,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Id_From_Requirement_Without_Permission_Is_Not_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext(NodeId);
- var mockHttpContextAccessor = CreateMockHttpContextAccessor();
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue();
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "B" });
await sut.HandleAsync(authHandlerContext);
@@ -59,7 +57,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Id_Missing_From_Requirement_And_QueryString_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor("xxx");
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue("xxx");
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -71,7 +69,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Integer_Id_From_QueryString_With_Permission_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: NodeId.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: NodeId.ToString());
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -84,7 +82,21 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Integer_Id_From_QueryString_Without_Permission_Is_Not_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: NodeId.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: NodeId.ToString());
+ var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "B" });
+
+ await sut.HandleAsync(authHandlerContext);
+
+ Assert.IsFalse(authHandlerContext.HasSucceeded);
+ AssertContentCached(mockHttpContextAccessor);
+ }
+
+ [Test]
+ public async Task Node_Integer_Id_From_QueryString_Without_Permission_Is_Not_Authorized_Even_When_Additional_Parameter_For_Id_With_Permission_Is_Provided()
+ {
+ // Provides initially failing test and verifies fix for advisory https://github.com/umbraco/Umbraco-CMS/security/advisories/GHSA-wx5h-wqfq-v698
+ var authHandlerContext = CreateAuthorizationHandlerContext();
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValues(queryStringValues: [NodeId.ToString(), 1001.ToString()]);
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "B" });
await sut.HandleAsync(authHandlerContext);
@@ -97,7 +109,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Udi_Id_From_QueryString_With_Permission_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: s_nodeUdi.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: s_nodeUdi.ToString());
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -110,7 +122,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Udi_Id_From_QueryString_Without_Permission_Is_Not_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: s_nodeUdi.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: s_nodeUdi.ToString());
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "B" });
await sut.HandleAsync(authHandlerContext);
@@ -123,7 +135,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Guid_Id_From_QueryString_With_Permission_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: s_nodeGuid.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: s_nodeGuid.ToString());
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -136,7 +148,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Guid_Id_From_QueryString_Without_Permission_Is_Not_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: s_nodeGuid.ToString());
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: s_nodeGuid.ToString());
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "B" });
await sut.HandleAsync(authHandlerContext);
@@ -149,7 +161,7 @@ public class ContentPermissionsQueryStringHandlerTests
public async Task Node_Invalid_Id_From_QueryString_Is_Authorized()
{
var authHandlerContext = CreateAuthorizationHandlerContext();
- var mockHttpContextAccessor = CreateMockHttpContextAccessor(queryStringValue: "invalid");
+ var mockHttpContextAccessor = CreateMockHttpContextAccessorWithQueryStringValue(queryStringValue: "invalid");
var sut = CreateHandler(mockHttpContextAccessor.Object, NodeId, new[] { "A" });
await sut.HandleAsync(authHandlerContext);
@@ -168,14 +180,20 @@ public class ContentPermissionsQueryStringHandlerTests
return new AuthorizationHandlerContext(new List { requirement }, user, resource);
}
- private static Mock CreateMockHttpContextAccessor(
+ private static Mock CreateMockHttpContextAccessorWithQueryStringValue(
string queryStringName = QueryStringName,
string queryStringValue = "")
+ => CreateMockHttpContextAccessorWithQueryStringValues(queryStringName, [queryStringValue]);
+
+ private static Mock CreateMockHttpContextAccessorWithQueryStringValues(
+ string queryStringName = QueryStringName,
+ string[]? queryStringValues = null)
{
+ queryStringValues ??= [];
var mockHttpContextAccessor = new Mock();
var mockHttpContext = new Mock();
var mockHttpRequest = new Mock();
- var queryParams = new Dictionary { { queryStringName, queryStringValue } };
+ var queryParams = new Dictionary { { queryStringName, new StringValues(queryStringValues) } };
mockHttpRequest.SetupGet(x => x.Query).Returns(new QueryCollection(queryParams));
mockHttpContext.SetupGet(x => x.Request).Returns(mockHttpRequest.Object);
mockHttpContext.SetupGet(x => x.Items).Returns(new Dictionary