From 4aa15971380060f37fcc3ab02ace5efce54ee77e Mon Sep 17 00:00:00 2001 From: Jesse Date: Thu, 8 Jun 2023 02:11:18 -0700 Subject: [PATCH 01/31] Fix for escaping markdown characters in property descriptions (#12345) * Added secure to the UMB-XSRF-V cookie when global https is true. * tweaked markdown handling * added in link handling tweak for simpleMarkdown filter * tweaked simple markdown filter to modify all links instead of just the first one * moved transformation of markdown content in description into C# code * Format of white space * Reverted unecessary change. * Removed unwanted framework version lines * Reduce nesting of if statements. * Changed to .Contains for readability. --------- Co-authored-by: Corey Philipp Co-authored-by: jaandrews Co-authored-by: Emma Garland --- .../Mapping/ContentMapDefinition.cs | 49 +++++- .../Security/BackOfficeAntiforgery.cs | 8 +- .../Umbraco.Web.BackOffice.csproj | 3 +- .../property/umbproperty.directive.js | 160 +++++++++--------- .../common/filters/simpleMarkdown.filter.js | 20 --- .../filters/simpleMarkdown.filter.js.js | 20 --- .../components/property/umb-property.html | 4 +- 7 files changed, 135 insertions(+), 129 deletions(-) delete mode 100644 src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js delete mode 100644 src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js.js diff --git a/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs b/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs index 57732a9898..e91384f86a 100644 --- a/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs +++ b/src/Umbraco.Web.BackOffice/Mapping/ContentMapDefinition.cs @@ -1,4 +1,6 @@ using System.Globalization; +using System.Text.RegularExpressions; +using HeyRed.MarkdownSharp; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Actions; @@ -303,6 +305,51 @@ internal class ContentMapDefinition : IMapDefinition { Properties = context.MapEnumerable(source.Properties).WhereNotNull() }; + var markdown = new Markdown(); + var linkCheck = new Regex("]+>", RegexOptions.IgnoreCase); + var evaluator = new MatchEvaluator(AddRelNoReferrer); + foreach (TVariant variant in target.Variants) + { + foreach (Tab tab in variant.Tabs) + { + if (tab.Properties == null) + { + continue; + } + + foreach (ContentPropertyDisplay property in tab.Properties) + { + if (string.IsNullOrEmpty(property.Description)) + { + continue; + } + + var description = markdown.Transform(property.Description); + property.Description = linkCheck.Replace(description, evaluator); + } + } + } + } + + private string AddRelNoReferrer(Match m) + { + string result = m.Value; + if (!result.Contains("rel=", StringComparison.Ordinal)) + { + result = result.Replace(">", " rel=\"noreferrer\">"); + } + + if (!result.Contains("class=", StringComparison.Ordinal)) + { + result = result.Replace(">", " class=\"underline\">"); + } + + if (!result.Contains("target=", StringComparison.Ordinal)) + { + result = result.Replace(">", " target=\"_blank\">"); + } + + return result; } // Umbraco.Code.MapAll -Segment -Language -DisplayName @@ -359,7 +406,7 @@ internal class ContentMapDefinition : IMapDefinition { currentUser = currentIUserBackofficeUser; } - else if(_backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is not null) + else if (_backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is not null) { currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs index 2587ae9a58..a6e4e96d8b 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs @@ -8,7 +8,7 @@ using Umbraco.Cms.Core.Configuration.Models; namespace Umbraco.Cms.Web.BackOffice.Security; /// -/// Antiforgery implementation for the Umbraco back office +/// Anti-forgery implementation for the Umbraco back office /// /// /// This is a wrapper around the global/default .net service. Because this service is a @@ -33,14 +33,12 @@ public class BackOfficeAntiforgery : IBackOfficeAntiforgery { x.HeaderName = Constants.Web.AngularHeadername; x.Cookie.Name = Constants.Web.CsrfValidationCookieName; + x.Cookie.SecurePolicy = globalSettings.CurrentValue.UseHttps ? CookieSecurePolicy.Always : CookieSecurePolicy.SameAsRequest; }); ServiceProvider container = services.BuildServiceProvider(); _internalAntiForgery = container.GetRequiredService(); _globalSettings = globalSettings.CurrentValue; - globalSettings.OnChange(x => - { - _globalSettings = x; - }); + globalSettings.OnChange(x => _globalSettings = x); } /// diff --git a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj index 08e89c375f..00e6c46273 100644 --- a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj +++ b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj @@ -12,6 +12,7 @@ + @@ -32,4 +33,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js index 5a30f81d4b..ccc9f92caf 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js @@ -4,96 +4,96 @@ * @restrict E **/ (function () { - 'use strict'; + 'use strict'; - angular - .module("umbraco.directives") - .component('umbProperty', { - templateUrl: 'views/components/property/umb-property.html', - controller: UmbPropertyController, - controllerAs: 'vm', - transclude: true, - require: { - parentUmbProperty: '?^^umbProperty', - parentForm: '?^^form' - }, - bindings: { - property: "=", + angular + .module("umbraco.directives") + .component('umbProperty', { + templateUrl: 'views/components/property/umb-property.html', + controller: UmbPropertyController, + controllerAs: 'vm', + transclude: true, + require: { + parentUmbProperty: '?^^umbProperty', + parentForm: '?^^form' + }, + bindings: { + property: "=", node: "<", - elementKey: "@", - // optional, if set this will be used for the property alias validation path (hack required because NC changes the actual property.alias :/) - propertyAlias: "@", - showInherit: "<", + elementKey: "@", + // optional, if set this will be used for the property alias validation path (hack required because NC changes the actual property.alias :/) + propertyAlias: "@", + showInherit: "<", inheritsFrom: "<", hideLabel: " s && s.vm && s.vm.constructor.name === "UmbPropertyController"); - vm.parentUmbProperty = found ? found.vm : null; - } - - if (vm.property.description) { - // split by lines containing only '--' - var descriptionParts = vm.property.description.split(/^--$/gim); - if (descriptionParts.length > 1) { - // if more than one part, we have an extended description, - // combine to one extended description, and remove leading linebreak - vm.property.extendedDescription = descriptionParts.splice(1).join("--").substring(1); - vm.property.extendedDescriptionVisible = false; - - // set propertydescription to first part, and remove trailing linebreak - vm.property.description = descriptionParts[0].slice(0, -1); - } - } - } + vm.$onInit = onInit; + vm.setDirty = function () { + // NOTE: We need to use scope because we haven't changd it to vm.propertyForm in the html and that + // might mean a breaking change. + $scope.propertyForm.$setDirty(); } + vm.setPropertyError = function (errorMsg) { + vm.property.propertyErrorMessage = errorMsg; + }; + + vm.propertyActions = []; + vm.setPropertyActions = function (actions) { + vm.propertyActions = actions; + }; + + // returns the validation path for the property to be used as the validation key for server side validation logic + vm.getValidationPath = function () { + + var parentValidationPath = vm.parentUmbProperty ? vm.parentUmbProperty.getValidationPath() : null; + var propAlias = vm.propertyAlias ? vm.propertyAlias : vm.property.alias; + // the elementKey will be empty when this is not a nested property + var valPath = vm.elementKey ? vm.elementKey + "/" + propAlias : propAlias; + return serverValidationManager.createPropertyValidationKey(valPath, parentValidationPath); + } + + function onInit() { + vm.controlLabelTitle = null; + if (Umbraco.Sys.ServerVariables.isDebuggingEnabled) { + userService.getCurrentUser().then(function (u) { + if (u.allowedSections.indexOf("settings") !== -1 ? true : false) { + vm.controlLabelTitle = vm.property.alias; + } + }); + } + + if (!vm.parentUmbProperty) { + // not found, then fallback to searching the scope chain, this may be needed when DOM inheritance isn't maintained but scope + // inheritance is (i.e.infinite editing) + var found = angularHelper.traverseScopeChain($scope, s => s && s.vm && s.vm.constructor.name === "UmbPropertyController"); + vm.parentUmbProperty = found ? found.vm : null; + } + + if (vm.property.description) { + // split by lines containing only '--' + var descriptionParts = vm.property.description.split(/^--$/gim); + if (descriptionParts.length > 1) { + // if more than one part, we have an extended description, + // combine to one extended description, and remove leading linebreak + vm.property.extendedDescription = descriptionParts.splice(1).join("--").substring(1); + vm.property.extendedDescriptionVisible = false; + + // set propertydescription to first part, and remove trailing linebreak + vm.property.description = descriptionParts[0].slice(0, -1); + } + } + } + + } + })(); diff --git a/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js b/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js deleted file mode 100644 index 58d5b0ed91..0000000000 --- a/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js +++ /dev/null @@ -1,20 +0,0 @@ -/** -* @ngdoc filter -* @name umbraco.filters.simpleMarkdown -* @description -* Used when rendering a string as Markdown as HTML (i.e. with ng-bind-html). Allows use of **bold**, *italics*, ![images](url) and [links](url) -**/ -angular.module("umbraco.filters").filter('simpleMarkdown', function () { - return function (text) { - if (!text) { - return ''; - } - - return text - .replace(/\*\*(.*)\*\*/gim, '$1') - .replace(/\*(.*)\*/gim, '$1') - .replace(/!\[(.*?)\]\((.*?)\)/gim, "$1") - .replace(/\[(.*?)\]\((.*?)\)/gim, "$1") - .replace(/\n/g, '
').trim(); - }; -}); diff --git a/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js.js b/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js.js deleted file mode 100644 index d33b96916a..0000000000 --- a/src/Umbraco.Web.UI.Client/src/common/filters/simpleMarkdown.filter.js.js +++ /dev/null @@ -1,20 +0,0 @@ -/** -* @ngdoc filter -* @name umbraco.filters.simpleMarkdown -* @description -* Used when rendering a string as Markdown as HTML (i.e. with ng-bind-html). Allows use of **bold**, *italics*, ![images](url) and [links](url) -**/ -angular.module("umbraco.filters").filter('simpleMarkdown', function () { - return function (text) { - if (!text) { - return ''; - } - - return text - .replace(/\*\*(.*)\*\*/gim, '$1') - .replace(/\*(.*)\*/gim, '$1') - .replace(/!\[(.*?)\]\((.*?)\)/gim, "$1") - .replace(/\[(.*?)\]\((.*?)\)/gim, "$1") - .replace(/\n/g, '
').trim(); - }; -}); diff --git a/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html b/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html index cc41896a0f..2c017afdba 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html @@ -13,12 +13,12 @@ - +
- +