From 213d8c00aa6655b2b5c1514c337221898b3c2ed9 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 4 Oct 2021 09:45:33 +0200 Subject: [PATCH 1/5] Remove inherited property group id/key when local properties are added (#11231) * Remove inherited property group id/key when local properties are added * Rebind saved content type values * Remove inherited from save group (cherry picked from commit 20b9db87d091e7d53f9f25d4d11744afcffa2584) --- .../services/contenttypehelper.service.js | 39 +++++++++++++++++ .../services/umbdataformatter.service.js | 40 +++++++++-------- .../views/documenttypes/edit.controller.js | 32 ++------------ .../src/views/mediatypes/edit.controller.js | 35 ++------------- .../src/views/membertypes/edit.controller.js | 43 +++---------------- 5 files changed, 73 insertions(+), 116 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js index d9157093a7b9..cceb67689fed 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js @@ -439,6 +439,45 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter, $inje array.push(placeholder); + }, + + rebindSavedContentType: function (contentType, savedContentType) { + // The saved content type might have updated values (eg. new IDs/keys), so make sure the view model is updated + contentType.ModelState = savedContentType.ModelState; + contentType.id = savedContentType.id; + contentType.groups.forEach(function (group) { + if (!group.alias) return; + + var k = 0; + while (k < savedContentType.groups.length && savedContentType.groups[k].alias != group.alias) + k++; + + if (k == savedContentType.groups.length) { + group.id = 0; + return; + } + + var savedGroup = savedContentType.groups[k]; + group.id = savedGroup.id; + group.key = savedGroup.key; + group.contentTypeId = savedGroup.contentTypeId; + + group.properties.forEach(function (property) { + if (property.id || !property.alias) return; + + k = 0; + while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) + k++; + + if (k == savedGroup.properties.length) { + property.id = 0; + return; + } + + var savedProperty = savedGroup.properties[k]; + property.id = savedProperty.id; + }); + }); } }; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index 226fabeae41d..7270ae5bbf06 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -37,7 +37,7 @@ return { - formatChangePasswordModel: function(model) { + formatChangePasswordModel: function (model) { if (!model) { return null; } @@ -59,26 +59,23 @@ }, formatContentTypePostData: function (displayModel, action) { - - //create the save model from the display model + // Create the save model from the display model var saveModel = _.pick(displayModel, 'compositeContentTypes', 'isContainer', 'allowAsRoot', 'allowedTemplates', 'allowedContentTypes', 'alias', 'description', 'thumbnail', 'name', 'id', 'icon', 'trashed', 'key', 'parentId', 'alias', 'path', 'allowCultureVariant', 'allowSegmentVariant', 'isElement'); - // TODO: Map these saveModel.allowedTemplates = _.map(displayModel.allowedTemplates, function (t) { return t.alias; }); saveModel.defaultTemplate = displayModel.defaultTemplate ? displayModel.defaultTemplate.alias : null; var realGroups = _.reject(displayModel.groups, function (g) { - //do not include these tabs + // Do not include groups with init state return g.tabState === "init"; }); saveModel.groups = _.map(realGroups, function (g) { - - var saveGroup = _.pick(g, 'inherited', 'id', 'sortOrder', 'name', 'key', 'alias', 'type'); + var saveGroup = _.pick(g, 'id', 'sortOrder', 'name', 'key', 'alias', 'type'); var realProperties = _.reject(g.properties, function (p) { - //do not include these properties + // Do not include properties with init state or inherited from a composition return p.propertyState === "init" || p.inherited === true; }); @@ -89,16 +86,21 @@ saveGroup.properties = saveProperties; - //if this is an inherited group and there are not non-inherited properties on it, then don't send up the data - if (saveGroup.inherited === true && saveProperties.length === 0) { - return null; + if (g.inherited === true) { + if (saveProperties.length === 0) { + // All properties are inherited from the compositions, no need to save this group + return null; + } else if (g.contentTypeId != saveModel.id) { + // We have local properties, but the group id is not local, ensure a new id/key is generated on save + saveGroup = _.omit(saveGroup, 'id', 'key'); + } } return saveGroup; }); - //we don't want any null groups saveModel.groups = _.reject(saveModel.groups, function (g) { + // Do not include empty/null groups return !g; }); @@ -127,17 +129,17 @@ }, /** formats the display model used to display the dictionary to the model used to save the dictionary */ - formatDictionaryPostData : function(dictionary, nameIsDirty) { + formatDictionaryPostData: function (dictionary, nameIsDirty) { var saveModel = { parentId: dictionary.parentId, id: dictionary.id, name: dictionary.name, nameIsDirty: nameIsDirty, translations: [], - key : dictionary.key + key: dictionary.key }; - for(var i = 0; i < dictionary.translations.length; i++) { + for (var i = 0; i < dictionary.translations.length; i++) { saveModel.translations.push({ isoCode: dictionary.translations[i].isoCode, languageId: dictionary.translations[i].languageId, @@ -362,7 +364,7 @@ parentId: displayModel.parentId, //set the action on the save model action: action, - variants: _.map(displayModel.variants, function(v) { + variants: _.map(displayModel.variants, function (v) { return { name: v.name || "", //if its null/empty,we must pass up an empty string else we get json converter errors properties: getContentProperties(v.tabs), @@ -392,7 +394,7 @@ * @param {} displayModel * @returns {} */ - formatContentGetData: function(displayModel) { + formatContentGetData: function (displayModel) { // We need to check for invariant properties among the variant variants, // as the value of an invariant property is shared between different variants. @@ -458,12 +460,12 @@ * Formats the display model used to display the relation type to a model used to save the relation type. * @param {Object} relationType */ - formatRelationTypePostData : function(relationType) { + formatRelationTypePostData: function (relationType) { var saveModel = { id: relationType.id, name: relationType.name, alias: relationType.alias, - key : relationType.key, + key: relationType.key, isBidirectional: relationType.isBidirectional, parentObjectType: relationType.parentObjectType, childObjectType: relationType.childObjectType diff --git a/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js index 3672af900c4d..a5e49163f9e5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js @@ -324,35 +324,9 @@ scope: $scope, content: vm.contentType, infiniteMode: infiniteMode, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.ModelState = savedContentType.ModelState; - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { // allow UI to access server validation state diff --git a/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js index ecf2aec30c0b..32fac36baaee 100644 --- a/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js @@ -295,38 +295,9 @@ saveMethod: mediaTypeResource.save, scope: $scope, content: vm.contentType, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { //success diff --git a/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js index 53bb4adb9b99..2eddc71924be 100644 --- a/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js @@ -175,11 +175,11 @@ //we are creating so get an empty data type item memberTypeResource.getScaffold(memberTypeId) - .then(function (dt) { - init(dt); + .then(function (dt) { + init(dt); - vm.page.loading = false; - }); + vm.page.loading = false; + }); } else { loadMemberType(); @@ -215,38 +215,9 @@ saveMethod: memberTypeResource.save, scope: $scope, content: vm.contentType, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { //success From 8acf7e147e9ccfcec2c45aa9d6057cab4368cec0 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 22 Oct 2021 09:58:47 +0200 Subject: [PATCH 2/5] Changes to GetReducedEventList (#11444) * Instead of only using first event, we combine events of same type into a single event with multiple arguments * Added generic method to DRY up grouping logic. * Renamed method to better reflect new functionality. Co-authored-by: Andy Butland --- .../Cache/DistributedCacheBinderTests.cs | 76 +++++++++++++-- .../Cache/DistributedCacheBinder.cs | 92 +++++++++++++------ 2 files changed, 129 insertions(+), 39 deletions(-) diff --git a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs index 00a33c0b6c52..7037e1bea897 100644 --- a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using Moq; @@ -10,6 +11,7 @@ using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Core.Services.Changes; +using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Tests.Testing; using Umbraco.Tests.Testing.Objects.Accessors; using Umbraco.Web; @@ -168,17 +170,73 @@ public void CanHandleEvent() } [Test] - public void OnlyHandlesOnContentTypeEvent() + public void GroupsContentTypeEvents() { - var definitions = new IEventDefinition[] + var num = 30; + var contentTypes = Enumerable.Repeat(MockedContentTypes.CreateBasicContentType(), num); + var mediaTypes = Enumerable.Repeat(MockedContentTypes.CreateImageMediaType(), num); + var memberTypes = Enumerable.Repeat(MockedContentTypes.CreateSimpleMemberType(), num); + var definitionsContent = contentTypes.SelectMany(x => new IEventDefinition[] { - new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(Enumerable.Empty>()), "Changed"), - new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(Enumerable.Empty()), "Saved"), - new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(Enumerable.Empty>()), "Changed"), - new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(Enumerable.Empty()), "Saved"), - }; - var result = DistributedCacheBinder.GetReducedEventList(definitions); - Assert.AreEqual(1, result.Count()); + new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(x), "Saved"), + }); + + var definitionsMedia = mediaTypes.SelectMany(x => new IEventDefinition[] + { + new EventDefinition.EventArgs>(null, Current.Services.MediaTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.MediaTypeService, new SaveEventArgs(x), "Saved"), + }); + var definitionsMember = memberTypes.SelectMany(x => new IEventDefinition[] + { + new EventDefinition.EventArgs>(null, Current.Services.MemberTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.MemberTypeService, new SaveEventArgs(x), "Saved"), + }); + + var definitions = new List(); + definitions.AddRange(definitionsContent); + definitions.AddRange(definitionsMedia); + definitions.AddRange(definitionsMember); + + var result = DistributedCacheBinder.GetGroupedEventList(definitions); + + Assert.Multiple(() => + { + Assert.AreEqual(num * 6, definitions.Count(), "Precondition is we have many definitions"); + Assert.AreEqual(6, result.Count(), "Unexpected number of reduced definitions"); + foreach (var eventDefinition in result) + { + if (eventDefinition.Args is SaveEventArgs saveContentEventArgs) + { + Assert.AreEqual(num, saveContentEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeContentEventArgs) + { + Assert.AreEqual(num, changeContentEventArgs.Changes.Count()); + } + + if (eventDefinition.Args is SaveEventArgs saveMediaEventArgs) + { + Assert.AreEqual(num, saveMediaEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeMediaEventArgs) + { + Assert.AreEqual(num, changeMediaEventArgs.Changes.Count()); + } + + if (eventDefinition.Args is SaveEventArgs saveMemberEventArgs) + { + Assert.AreEqual(num, saveMemberEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeMemberEventArgs) + { + Assert.AreEqual(num, changeMemberEventArgs.Changes.Count()); + } + } + }); } } } diff --git a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs index e3a5a01d54d8..bfb1a01a6919 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs @@ -6,6 +6,9 @@ using Umbraco.Core; using Umbraco.Core.Events; using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Core.Services.Changes; namespace Umbraco.Web.Cache { @@ -66,10 +69,9 @@ public void HandleEvents(IEnumerable events) using (_umbracoContextFactory.EnsureUmbracoContext()) { // When it comes to content types types, a change to any single one will trigger a reload of the content and media caches. - // As far as I (AB) can tell, there's no type specific logic here, they all clear caches for all content types, and trigger a reload of all content and media. - // We also have events registered for Changed and Saved, which do the same thing, so really only need one of these. - // Hence if we have more than one document or media types, we can and should only handle one of the events for one, to avoid repeated cache reloads. - foreach (var e in GetReducedEventList(events)) + // We can reduce the impact of that by grouping the events to invoke just one per type, providing a collection of the individual arguments. + var groupedEvents = GetGroupedEventList(events); + foreach (var e in groupedEvents) { var handler = FindHandler(e); if (handler == null) @@ -86,47 +88,77 @@ public void HandleEvents(IEnumerable events) } // Internal for tests - internal static IEnumerable GetReducedEventList(IEnumerable events) + internal static IEnumerable GetGroupedEventList(IEnumerable events) { - var reducedEvents = new List(); + var groupedEvents = new List(); - var gotDoumentType = false; - var gotMediaType = false; - var gotMemberType = false; + var grouped = events.GroupBy(x => x.GetType()); - foreach (var evt in events) + foreach (var group in grouped) { - if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.ContentTypeService))) + if (group.Key == typeof(EventDefinition>)) { - if (gotDoumentType == false) - { - reducedEvents.Add(evt); - gotDoumentType = true; - } + GroupSaveEvents(groupedEvents, group); } - else if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.MediaTypeService))) + else if (group.Key == typeof(EventDefinition.EventArgs>)) { - if (gotMediaType == false) - { - reducedEvents.Add(evt); - gotMediaType = true; - } + GroupChangeEvents(groupedEvents, group); } - else if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.MemberTypeService))) + else if (group.Key == typeof(EventDefinition>)) { - if (gotMemberType == false) - { - reducedEvents.Add(evt); - gotMemberType = true; - } + GroupSaveEvents(groupedEvents, group); + } + else if (group.Key == typeof(EventDefinition.EventArgs>)) + { + GroupChangeEvents(groupedEvents, group); + } + else if (group.Key == typeof(EventDefinition>)) + { + GroupSaveEvents(groupedEvents, group); + } + else if (group.Key == typeof(EventDefinition.EventArgs>)) + { + GroupChangeEvents(groupedEvents, group); } else { - reducedEvents.Add(evt); + groupedEvents.AddRange(group); } } - return reducedEvents; + return groupedEvents; + } + + private static void GroupSaveEvents(List groupedEvents, IGrouping group) + where TService : IContentTypeBaseService + where TType : IContentTypeBase + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + groupedEvents.Add(new EventDefinition>( + null, + (TService)groupedGroup.Key.Sender, + new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), + groupedGroup.Key.EventName)); + } + } + + private static void GroupChangeEvents(List groupedEvents, IGrouping group) + where TService : IContentTypeBaseService + where TType : class, IContentTypeComposition + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + groupedEvents.Add(new EventDefinition.EventArgs>( + null, + (TService)groupedGroup.Key.Sender, + new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), + groupedGroup.Key.EventName)); + } } } } From da48f8435a914cf809a05b0305c154d55715e5a3 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Tue, 19 Oct 2021 13:30:03 +0200 Subject: [PATCH 3/5] Merge pull request #11360 from umbraco/v8/bugfix/11057-mandatory-image-not-validating-after-first-time-failure Fixes 11057: Mandatory Image not validating after first time failure (cherry picked from commit 5cc70d2160d2cb917d5d9983a715474619cb87e1) --- .../mediapicker3/umb-media-picker3-property-editor.html | 4 ++++ .../mediapicker3/umbMediaPicker3PropertyEditor.component.js | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html index daf9566e2d4d..f40c6bc437e5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html @@ -69,4 +69,8 @@ + + + + diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js index 02b9f0b92813..ff0b0030e6b5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js @@ -137,6 +137,10 @@ if (vm.propertyForm) { vm.propertyForm.$setDirty(); } + + if (vm.modelValueForm) { + vm.modelValueForm.modelValue.$setDirty(); + } } function addMediaAt(createIndex, $event) { From 23ce69db2fe3b0677a0d8520aa48efce3b1773a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Tue, 2 Nov 2021 13:21:44 +0100 Subject: [PATCH 4/5] Additional optional sanitization of scripting in TinyMCE (#10653) (cherry picked from commit f68dba7bcb16308af17c5385b8e586165e44b578) --- .../Configuration/GlobalSettings.cs | 21 ++++++ .../Configuration/IGlobalSettings.cs | 5 ++ src/Umbraco.Core/Constants-AppSettings.cs | 7 +- .../src/common/services/tinymce.service.js | 67 +++++++++++++++++++ src/Umbraco.Web.UI/web.Template.config | 1 + .../Editors/BackOfficeServerVariables.cs | 1 + 6 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index c844abe75e49..41e8f633c99b 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -395,6 +395,27 @@ public bool UseHttps } } + /// + /// Returns true if TinyMCE scripting sanitization should be applied + /// + /// + /// The default value is false + /// + public bool SanitizeTinyMce + { + get + { + try + { + return bool.Parse(ConfigurationManager.AppSettings[Constants.AppSettings.SanitizeTinyMce]); + } + catch + { + return false; + } + } + } + /// /// An int value representing the time in milliseconds to lock the database for a write operation /// diff --git a/src/Umbraco.Core/Configuration/IGlobalSettings.cs b/src/Umbraco.Core/Configuration/IGlobalSettings.cs index 483829f85ff3..2ebab722f001 100644 --- a/src/Umbraco.Core/Configuration/IGlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/IGlobalSettings.cs @@ -77,5 +77,10 @@ public interface IGlobalSettings /// Gets the write lock timeout. /// int SqlWriteLockTimeOut { get; } + + /// + /// Returns true if TinyMCE scripting sanitization should be applied + /// + bool SanitizeTinyMce { get; } } } diff --git a/src/Umbraco.Core/Constants-AppSettings.cs b/src/Umbraco.Core/Constants-AppSettings.cs index 99ea26b4d698..de7799c1655c 100644 --- a/src/Umbraco.Core/Constants-AppSettings.cs +++ b/src/Umbraco.Core/Constants-AppSettings.cs @@ -109,7 +109,7 @@ public static class AppSettings /// A true or false indicating whether umbraco should force a secure (https) connection to the backoffice. /// public const string UseHttps = "Umbraco.Core.UseHttps"; - + /// /// A true/false value indicating whether the content dashboard should be visible for all user groups. /// @@ -155,6 +155,11 @@ public static class Debug /// An int value representing the time in milliseconds to lock the database for a write operation /// public const string SqlWriteLockTimeOut = "Umbraco.Core.SqlWriteLockTimeOut"; + + /// + /// Returns true if TinyMCE scripting sanitization should be applied + /// + public const string SanitizeTinyMce = "Umbraco.Web.SanitizeTinyMce"; } } } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js index 46c047448f88..fe3219e2b7aa 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js @@ -1491,6 +1491,19 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); } + + if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){ + /** prevent injecting arbitrary JavaScript execution in on-attributes. */ + const allNodes = Array.prototype.slice.call(args.editor.dom.doc.getElementsByTagName("*")); + allNodes.forEach(node => { + for (var i = 0; i < node.attributes.length; i++) { + if(node.attributes[i].name.indexOf("on") === 0) { + node.removeAttribute(node.attributes[i].name) + } + } + }); + } + }); args.editor.on('init', function (e) { @@ -1502,6 +1515,60 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s //enable browser based spell checking args.editor.getBody().setAttribute('spellcheck', true); + + /** Setup sanitization for preventing injecting arbitrary JavaScript execution in attributes: + * https://github.com/advisories/GHSA-w7jx-j77m-wp65 + * https://github.com/advisories/GHSA-5vm8-hhgr-jcjp + */ + const uriAttributesToSanitize = ['src', 'href', 'data', 'background', 'action', 'formaction', 'poster', 'xlink:href']; + const parseUri = function() { + // Encapsulated JS logic. + const safeSvgDataUrlElements = [ 'img', 'video' ]; + const scriptUriRegExp = /((java|vb)script|mhtml):/i; + const trimRegExp = /[\s\u0000-\u001F]+/g; + const isInvalidUri = (uri, tagName) => { + if (/^data:image\//i.test(uri)) { + return safeSvgDataUrlElements.indexOf(tagName) !== -1 && /^data:image\/svg\+xml/i.test(uri); + } else { + return /^data:/i.test(uri); + } + }; + + return function parseUri(uri, tagName) { + uri = uri.replace(trimRegExp, ''); + try { + // Might throw malformed URI sequence + uri = decodeURIComponent(uri); + } catch (ex) { + // Fallback to non UTF-8 decoder + uri = unescape(uri); + } + + if (scriptUriRegExp.test(uri)) { + return; + } + + if (isInvalidUri(uri, tagName)) { + return; + } + + return uri; + } + }(); + + if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){ + args.editor.serializer.addAttributeFilter(uriAttributesToSanitize, function (nodes) { + nodes.forEach(function(node) { + node.attributes.forEach(function(attr) { + const attrName = attr.name.toLowerCase(); + if(uriAttributesToSanitize.indexOf(attrName) !== -1) { + attr.value = parseUri(attr.value, node.name); + } + }); + }); + }); + } + //start watching the value startWatch(); }); diff --git a/src/Umbraco.Web.UI/web.Template.config b/src/Umbraco.Web.UI/web.Template.config index e61c6585ad43..f19ab5d3b638 100644 --- a/src/Umbraco.Web.UI/web.Template.config +++ b/src/Umbraco.Web.UI/web.Template.config @@ -51,6 +51,7 @@ + + + +
+ + +
+
+ + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +