From bde88caf14e30fb5ff8547c629aa6133a7d07338 Mon Sep 17 00:00:00 2001 From: Lucas Bach Bisgaard Date: Tue, 17 Oct 2023 13:24:24 +0200 Subject: [PATCH 1/2] Fixes #14351 - Using Fallback to default langauge on a specific item changes the whole VariationContext (#14620) (cherry picked from commit 738749b705d8ea99fd5c8de88a25cd0888ddf1c2) --- .../Extensions/PublishedElementExtensions.cs | 23 ------------------- .../IPublishedValueFallback.cs | 1 + .../PublishedValueFallback.cs | 1 + 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Extensions/PublishedElementExtensions.cs b/src/Umbraco.Core/Extensions/PublishedElementExtensions.cs index 440962cd76..c85178c85c 100644 --- a/src/Umbraco.Core/Extensions/PublishedElementExtensions.cs +++ b/src/Umbraco.Core/Extensions/PublishedElementExtensions.cs @@ -134,27 +134,6 @@ public static class PublishedElementExtensions #endregion - #region CheckVariation - /// - /// Method to check if VariationContext culture differs from culture parameter, if so it will update the VariationContext for the PublishedValueFallback. - /// - /// The requested PublishedValueFallback. - /// The requested culture. - /// The requested segment. - /// - private static void EventuallyUpdateVariationContext(IPublishedValueFallback publishedValueFallback, string? culture, string? segment) - { - IVariationContextAccessor? variationContextAccessor = publishedValueFallback.VariationContextAccessor; - - //If there is a difference in requested culture and the culture that is set in the VariationContext, it will pick wrong localized content. - //This happens for example using links to localized content in a RichText Editor. - if (!string.IsNullOrEmpty(culture) && variationContextAccessor?.VariationContext?.Culture != culture) - { - variationContextAccessor!.VariationContext = new VariationContext(culture, segment); - } - } - #endregion - #region Value /// @@ -195,8 +174,6 @@ public static class PublishedElementExtensions { IPublishedProperty? property = content.GetProperty(alias); - EventuallyUpdateVariationContext(publishedValueFallback, culture, segment); - // if we have a property, and it has a value, return that value if (property != null && property.HasValue(culture, segment)) { diff --git a/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs b/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs index 111d747ec1..839b73ea51 100644 --- a/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs +++ b/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs @@ -5,6 +5,7 @@ namespace Umbraco.Cms.Core.Models.PublishedContent; /// public interface IPublishedValueFallback { + [Obsolete("Scheduled for removal in v14")] /// /// VariationContextAccessor that is not required to be implemented, therefore throws NotImplementedException as default. /// diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedValueFallback.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedValueFallback.cs index d505c5f2c3..16c648233d 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedValueFallback.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedValueFallback.cs @@ -20,6 +20,7 @@ public class PublishedValueFallback : IPublishedValueFallback _variationContextAccessor = variationContextAccessor; } + [Obsolete("Scheduled for removal in v14")] public IVariationContextAccessor VariationContextAccessor { get { return _variationContextAccessor; } } /// From 92af8ac881638e8a29f7dfc64ef0799a950a3842 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Wed, 1 Nov 2023 11:30:32 +0100 Subject: [PATCH 2/2] Check content permissions before performing action (#15043) * Setting actionContext.Result when authz wasn't successful * Taking into account permissions when it is a new node * Cleanup * Passing nodeId as path when new item --- .../Security/ContentPermissions.cs | 59 ++++++++----------- .../Controllers/ContentController.cs | 23 ++++---- .../Controllers/ContentControllerBase.cs | 1 - .../Filters/ContentSaveValidationAttribute.cs | 1 + 4 files changed, 36 insertions(+), 48 deletions(-) diff --git a/src/Umbraco.Core/Security/ContentPermissions.cs b/src/Umbraco.Core/Security/ContentPermissions.cs index db27d100c6..0ad85971cf 100644 --- a/src/Umbraco.Core/Security/ContentPermissions.cs +++ b/src/Umbraco.Core/Security/ContentPermissions.cs @@ -167,12 +167,7 @@ public class ContentPermissions throw new ArgumentNullException(nameof(user)); } - if (permissionsToCheck == null) - { - permissionsToCheck = Array.Empty(); - } - - bool? hasPathAccess = null; + bool hasPathAccess; entity = null; if (nodeId == Constants.System.Root) @@ -183,19 +178,17 @@ public class ContentPermissions { hasPathAccess = user.HasContentBinAccess(_entityService, _appCaches); } - - if (hasPathAccess.HasValue) + else { - return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; - } + entity = _entityService.Get(nodeId, UmbracoObjectTypes.Document); - entity = _entityService.Get(nodeId, UmbracoObjectTypes.Document); - if (entity == null) - { - return ContentAccess.NotFound; - } + if (entity == null) + { + return ContentAccess.NotFound; + } - hasPathAccess = user.HasContentPathAccess(entity, _entityService, _appCaches); + hasPathAccess = user.HasContentPathAccess(entity, _entityService, _appCaches); + } if (hasPathAccess == false) { @@ -208,7 +201,8 @@ public class ContentPermissions } // get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(entity.Path, user, permissionsToCheck) + // if there is no entity for this id, than just use the id as the path (i.e. -1 or -20) + return CheckPermissionsPath(entity?.Path ?? nodeId.ToString(), user, permissionsToCheck) ? ContentAccess.Granted : ContentAccess.Denied; } @@ -235,12 +229,7 @@ public class ContentPermissions throw new ArgumentNullException(nameof(user)); } - if (permissionsToCheck == null) - { - permissionsToCheck = Array.Empty(); - } - - bool? hasPathAccess = null; + bool hasPathAccess; contentItem = null; if (nodeId == Constants.System.Root) @@ -251,19 +240,17 @@ public class ContentPermissions { hasPathAccess = user.HasContentBinAccess(_entityService, _appCaches); } - - if (hasPathAccess.HasValue) + else { - return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; - } + contentItem = _contentService.GetById(nodeId); - contentItem = _contentService.GetById(nodeId); - if (contentItem == null) - { - return ContentAccess.NotFound; - } + if (contentItem == null) + { + return ContentAccess.NotFound; + } - hasPathAccess = user.HasPathAccess(contentItem, _entityService, _appCaches); + hasPathAccess = user.HasPathAccess(contentItem, _entityService, _appCaches); + } if (hasPathAccess == false) { @@ -276,7 +263,8 @@ public class ContentPermissions } // get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(contentItem.Path, user, permissionsToCheck) + // if there is no content item for this id, than just use the id as the path (i.e. -1 or -20) + return CheckPermissionsPath(contentItem?.Path ?? nodeId.ToString(), user, permissionsToCheck) ? ContentAccess.Granted : ContentAccess.Denied; } @@ -288,8 +276,7 @@ public class ContentPermissions permissionsToCheck = Array.Empty(); } - // get the implicit/inherited permissions for the user for this path, - // if there is no content item for this id, than just use the id as the path (i.e. -1 or -20) + // get the implicit/inherited permissions for the user for this path EntityPermissionSet permission = _userService.GetPermissionsForPath(user, path); var allowed = true; diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 944068114a..9ec67319d9 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -759,6 +759,7 @@ public class ContentController : ContentControllerBase return pagedResult; } + /// /// Creates a blueprint from a content item /// @@ -1057,7 +1058,7 @@ public class ContentController : ContentControllerBase AddDomainWarnings(publishStatus.Content, successfulCultures, globalNotifications); AddPublishStatusNotifications(new[] { publishStatus }, globalNotifications, notifications, successfulCultures); } - break; + break; case ContentSaveAction.PublishWithDescendants: case ContentSaveAction.PublishWithDescendantsNew: { @@ -1074,7 +1075,7 @@ public class ContentController : ContentControllerBase AddDomainWarnings(publishStatus, successfulCultures, globalNotifications); AddPublishStatusNotifications(publishStatus, globalNotifications, notifications, successfulCultures); } - break; + break; case ContentSaveAction.PublishWithDescendantsForce: case ContentSaveAction.PublishWithDescendantsForceNew: { @@ -1090,7 +1091,7 @@ public class ContentController : ContentControllerBase var publishStatus = PublishBranchInternal(contentItem, true, cultureForInvariantErrors, out wasCancelled, out var successfulCultures).ToList(); AddPublishStatusNotifications(publishStatus, globalNotifications, notifications, successfulCultures); } - break; + break; default: throw new ArgumentOutOfRangeException(); } @@ -2745,7 +2746,7 @@ public class ContentController : ContentControllerBase } } } - break; + break; case PublishResultType.SuccessPublish: { // TODO: Here we should have messaging for when there are release dates specified like https://github.com/umbraco/Umbraco-CMS/pull/3507 @@ -2773,7 +2774,7 @@ public class ContentController : ContentControllerBase } } } - break; + break; case PublishResultType.FailedPublishPathNotPublished: { //TODO: This doesn't take into account variations with the successfulCultures param @@ -2782,14 +2783,14 @@ public class ContentController : ContentControllerBase _localizedTextService.Localize(null, "publish"), _localizedTextService.Localize("publish", "contentPublishedFailedByParent", new[] { names }).Trim()); } - break; + break; case PublishResultType.FailedPublishCancelledByEvent: { //TODO: This doesn't take into account variations with the successfulCultures param var names = string.Join(", ", status.Select(x => $"'{x.Content?.Name}'")); AddCancelMessage(display, "publish", "contentPublishedFailedByEvent", new[] { names }); } - break; + break; case PublishResultType.FailedPublishAwaitingRelease: { //TODO: This doesn't take into account variations with the successfulCultures param @@ -2798,7 +2799,7 @@ public class ContentController : ContentControllerBase _localizedTextService.Localize(null, "publish"), _localizedTextService.Localize("publish", "contentPublishedFailedAwaitingRelease", new[] { names }).Trim()); } - break; + break; case PublishResultType.FailedPublishHasExpired: { //TODO: This doesn't take into account variations with the successfulCultures param @@ -2807,7 +2808,7 @@ public class ContentController : ContentControllerBase _localizedTextService.Localize(null, "publish"), _localizedTextService.Localize("publish", "contentPublishedFailedExpired", new[] { names }).Trim()); } - break; + break; case PublishResultType.FailedPublishIsTrashed: { //TODO: This doesn't take into account variations with the successfulCultures param @@ -2816,7 +2817,7 @@ public class ContentController : ContentControllerBase _localizedTextService.Localize(null, "publish"), _localizedTextService.Localize("publish", "contentPublishedFailedIsTrashed", new[] { names }).Trim()); } - break; + break; case PublishResultType.FailedPublishContentInvalid: { if (successfulCultures == null) @@ -2840,7 +2841,7 @@ public class ContentController : ContentControllerBase } } } - break; + break; case PublishResultType.FailedPublishMandatoryCultureMissing: display.AddWarningNotification( _localizedTextService.Localize(null, "publish"), diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs index 36a60843fb..3a20f7a02d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs @@ -79,7 +79,6 @@ public abstract class ContentControllerBase : BackOfficeNotificationsController ModelState.AddModelError("id", $"content with id: {id} was not found"); NotFoundObjectResult errorResponse = NotFound(ModelState); - return errorResponse; } diff --git a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs index fb8629c461..7cf6ba3cf5 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs @@ -278,6 +278,7 @@ internal sealed class ContentSaveValidationAttribute : TypeFilterAttribute if (!authorizationResult.Succeeded) { + actionContext.Result = new ForbidResult(); return false; }