From 73f31dd800290533414efefc60261c4a3cc186cf Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 4 Dec 2018 11:34:15 +0100 Subject: [PATCH] Refactor manifest dashboards, security --- .../Configuration/Dashboard/AccessElement.cs | 24 +-- .../Configuration/Dashboard/AccessItem.cs | 15 -- .../Configuration/Dashboard/AccessRule.cs | 14 ++ .../Configuration/Dashboard/AccessRuleType.cs | 28 +++ .../Configuration/Dashboard/AccessType.cs | 9 - .../Configuration/Dashboard/ControlElement.cs | 17 +- .../Configuration/Dashboard/IAccess.cs | 2 +- .../Configuration/Dashboard/IAccessItem.cs | 15 -- .../Configuration/Dashboard/IAccessRule.cs | 18 ++ .../Manifest/DashboardAccessRuleConverter.cs | 45 +++++ .../Manifest/ManifestDashboardDefinition.cs | 36 ++++ .../Manifest/ManifestDashboardSection.cs | 33 ---- src/Umbraco.Core/Manifest/ManifestParser.cs | 12 +- src/Umbraco.Core/Manifest/PackageManifest.cs | 13 +- src/Umbraco.Core/Umbraco.Core.csproj | 9 +- .../DashboardSettingsTests.cs | 12 +- .../Manifest/ManifestParserTests.cs | 49 +++++ .../Editors/DashboardController.cs | 8 +- src/Umbraco.Web/Editors/DashboardHelper.cs | 176 ------------------ src/Umbraco.Web/Editors/DashboardSecurity.cs | 133 +++++++------ src/Umbraco.Web/Editors/Dashboards.cs | 159 ++++++++++++++++ src/Umbraco.Web/Editors/SectionController.cs | 27 +-- .../Runtime/WebRuntimeComponent.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 24 files changed, 462 insertions(+), 396 deletions(-) delete mode 100644 src/Umbraco.Core/Configuration/Dashboard/AccessItem.cs create mode 100644 src/Umbraco.Core/Configuration/Dashboard/AccessRule.cs create mode 100644 src/Umbraco.Core/Configuration/Dashboard/AccessRuleType.cs delete mode 100644 src/Umbraco.Core/Configuration/Dashboard/AccessType.cs delete mode 100644 src/Umbraco.Core/Configuration/Dashboard/IAccessItem.cs create mode 100644 src/Umbraco.Core/Configuration/Dashboard/IAccessRule.cs create mode 100644 src/Umbraco.Core/Manifest/DashboardAccessRuleConverter.cs create mode 100644 src/Umbraco.Core/Manifest/ManifestDashboardDefinition.cs delete mode 100644 src/Umbraco.Core/Manifest/ManifestDashboardSection.cs delete mode 100644 src/Umbraco.Web/Editors/DashboardHelper.cs create mode 100644 src/Umbraco.Web/Editors/Dashboards.cs diff --git a/src/Umbraco.Core/Configuration/Dashboard/AccessElement.cs b/src/Umbraco.Core/Configuration/Dashboard/AccessElement.cs index 1642f23fc5..01538c8e0b 100644 --- a/src/Umbraco.Core/Configuration/Dashboard/AccessElement.cs +++ b/src/Umbraco.Core/Configuration/Dashboard/AccessElement.cs @@ -7,26 +7,22 @@ namespace Umbraco.Core.Configuration.Dashboard internal class AccessElement : RawXmlConfigurationElement, IAccess { public AccessElement() - { - - } + { } public AccessElement(XElement rawXml) - :base(rawXml) - { - } + : base(rawXml) + { } - public IEnumerable Rules + public IEnumerable Rules { get { - var result = new List(); - if (RawXml != null) - { - result.AddRange(RawXml.Elements("deny").Select(x => new AccessItem {Action = AccessType.Deny, Value = x.Value })); - result.AddRange(RawXml.Elements("grant").Select(x => new AccessItem { Action = AccessType.Grant, Value = x.Value })); - result.AddRange(RawXml.Elements("grantBySection").Select(x => new AccessItem { Action = AccessType.GrantBySection, Value = x.Value })); - } + var result = new List(); + if (RawXml == null) return result; + + result.AddRange(RawXml.Elements("deny").Select(x => new AccessRule {Type = AccessRuleType.Deny, Value = x.Value })); + result.AddRange(RawXml.Elements("grant").Select(x => new AccessRule { Type = AccessRuleType.Grant, Value = x.Value })); + result.AddRange(RawXml.Elements("grantBySection").Select(x => new AccessRule { Type = AccessRuleType.GrantBySection, Value = x.Value })); return result; } } diff --git a/src/Umbraco.Core/Configuration/Dashboard/AccessItem.cs b/src/Umbraco.Core/Configuration/Dashboard/AccessItem.cs deleted file mode 100644 index 37cf491536..0000000000 --- a/src/Umbraco.Core/Configuration/Dashboard/AccessItem.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace Umbraco.Core.Configuration.Dashboard -{ - internal class AccessItem : IAccessItem - { - /// - /// This can be grant, deny or grantBySection - /// - public AccessType Action { get; set; } - - /// - /// The value of the action - /// - public string Value { get; set; } - } -} diff --git a/src/Umbraco.Core/Configuration/Dashboard/AccessRule.cs b/src/Umbraco.Core/Configuration/Dashboard/AccessRule.cs new file mode 100644 index 0000000000..fe6840ff64 --- /dev/null +++ b/src/Umbraco.Core/Configuration/Dashboard/AccessRule.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Core.Configuration.Dashboard +{ + /// + /// Implements . + /// + internal class AccessRule : IAccessRule + { + /// + public AccessRuleType Type { get; set; } + + /// + public string Value { get; set; } + } +} diff --git a/src/Umbraco.Core/Configuration/Dashboard/AccessRuleType.cs b/src/Umbraco.Core/Configuration/Dashboard/AccessRuleType.cs new file mode 100644 index 0000000000..cb9ce983fe --- /dev/null +++ b/src/Umbraco.Core/Configuration/Dashboard/AccessRuleType.cs @@ -0,0 +1,28 @@ +namespace Umbraco.Core.Configuration.Dashboard +{ + /// + /// Defines dashboard access rules type. + /// + public enum AccessRuleType + { + /// + /// Unknown (default value). + /// + Unknown = 0, + + /// + /// Grant access to the dashboard if user belongs to the specified user group. + /// + Grant, + + /// + /// Deny access to the dashboard if user belongs to the specified user group. + /// + Deny, + + /// + /// Grant access to the dashboard if user has access to the specified section. + /// + GrantBySection + } +} diff --git a/src/Umbraco.Core/Configuration/Dashboard/AccessType.cs b/src/Umbraco.Core/Configuration/Dashboard/AccessType.cs deleted file mode 100644 index d72cac15d0..0000000000 --- a/src/Umbraco.Core/Configuration/Dashboard/AccessType.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Umbraco.Core.Configuration.Dashboard -{ - public enum AccessType - { - Grant, - Deny, - GrantBySection - } -} diff --git a/src/Umbraco.Core/Configuration/Dashboard/ControlElement.cs b/src/Umbraco.Core/Configuration/Dashboard/ControlElement.cs index 0093c57778..20dac7460e 100644 --- a/src/Umbraco.Core/Configuration/Dashboard/ControlElement.cs +++ b/src/Umbraco.Core/Configuration/Dashboard/ControlElement.cs @@ -1,5 +1,4 @@ -using System; -using System.Configuration; +using System.Configuration; using System.Linq; using System.Xml.Linq; @@ -8,26 +7,21 @@ namespace Umbraco.Core.Configuration.Dashboard internal class ControlElement : RawXmlConfigurationElement, IDashboardControl { - public string PanelCaption { get { - return RawXml.Attribute("panelCaption") == null - ? "" - : RawXml.Attribute("panelCaption").Value; + var panelCaption = RawXml.Attribute("panelCaption"); + return panelCaption == null ? "" : panelCaption.Value; } } + public AccessElement Access { get { var access = RawXml.Element("access"); - if (access == null) - { - return new AccessElement(); - } - return new AccessElement(access); + return access == null ? new AccessElement() : new AccessElement(access); } } @@ -45,7 +39,6 @@ namespace Umbraco.Core.Configuration.Dashboard } } - IAccess IDashboardControl.AccessRights => Access; } } diff --git a/src/Umbraco.Core/Configuration/Dashboard/IAccess.cs b/src/Umbraco.Core/Configuration/Dashboard/IAccess.cs index b7d8540a79..8ac1b18cca 100644 --- a/src/Umbraco.Core/Configuration/Dashboard/IAccess.cs +++ b/src/Umbraco.Core/Configuration/Dashboard/IAccess.cs @@ -4,6 +4,6 @@ namespace Umbraco.Core.Configuration.Dashboard { public interface IAccess { - IEnumerable Rules { get; } + IEnumerable Rules { get; } } } diff --git a/src/Umbraco.Core/Configuration/Dashboard/IAccessItem.cs b/src/Umbraco.Core/Configuration/Dashboard/IAccessItem.cs deleted file mode 100644 index 8b18d50bb3..0000000000 --- a/src/Umbraco.Core/Configuration/Dashboard/IAccessItem.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace Umbraco.Core.Configuration.Dashboard -{ - public interface IAccessItem - { - /// - /// This can be grant, deny or grantBySection - /// - AccessType Action { get; set; } - - /// - /// The value of the action - /// - string Value { get; set; } - } -} diff --git a/src/Umbraco.Core/Configuration/Dashboard/IAccessRule.cs b/src/Umbraco.Core/Configuration/Dashboard/IAccessRule.cs new file mode 100644 index 0000000000..8b51b1b73a --- /dev/null +++ b/src/Umbraco.Core/Configuration/Dashboard/IAccessRule.cs @@ -0,0 +1,18 @@ +namespace Umbraco.Core.Configuration.Dashboard +{ + /// + /// Represents an access rule. + /// + public interface IAccessRule + { + /// + /// Gets or sets the rule type. + /// + AccessRuleType Type { get; set; } + + /// + /// Gets or sets the value for the rule. + /// + string Value { get; set; } + } +} diff --git a/src/Umbraco.Core/Manifest/DashboardAccessRuleConverter.cs b/src/Umbraco.Core/Manifest/DashboardAccessRuleConverter.cs new file mode 100644 index 0000000000..c627728a32 --- /dev/null +++ b/src/Umbraco.Core/Manifest/DashboardAccessRuleConverter.cs @@ -0,0 +1,45 @@ +using System; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Umbraco.Core.Configuration.Dashboard; +using Umbraco.Core.Serialization; + +namespace Umbraco.Core.Manifest +{ + /// + /// Implements a json read converter for . + /// + internal class DashboardAccessRuleConverter : JsonReadConverter + { + /// + protected override IAccessRule Create(Type objectType, string path, JObject jObject) + { + return new AccessRule(); + } + + /// + protected override void Deserialize(JObject jobject, IAccessRule target, JsonSerializer serializer) + { + // see Create above, target is either DataEditor (parameter) or ConfiguredDataEditor (property) + + if (!(target is AccessRule accessRule)) + throw new Exception("panic."); + + GetRule(accessRule, jobject, "grant", AccessRuleType.Grant); + GetRule(accessRule, jobject, "deny", AccessRuleType.Deny); + GetRule(accessRule, jobject, "grantBySection", AccessRuleType.GrantBySection); + + if (accessRule.Type == AccessRuleType.Unknown) throw new InvalidOperationException("Rule is not defined."); + } + + private void GetRule(AccessRule rule, JObject jobject, string name, AccessRuleType type) + { + var token = jobject[name]; + if (token == null) return; + if (rule.Type != AccessRuleType.Unknown) throw new InvalidOperationException("Multiple definition of a rule."); + if (token.Type != JTokenType.String) throw new InvalidOperationException("Rule value is not a string."); + rule.Type = type; + rule.Value = token.Value(); + } + } +} diff --git a/src/Umbraco.Core/Manifest/ManifestDashboardDefinition.cs b/src/Umbraco.Core/Manifest/ManifestDashboardDefinition.cs new file mode 100644 index 0000000000..83f047b264 --- /dev/null +++ b/src/Umbraco.Core/Manifest/ManifestDashboardDefinition.cs @@ -0,0 +1,36 @@ +using System; +using System.ComponentModel; +using Newtonsoft.Json; +using Umbraco.Core.Configuration.Dashboard; +using Umbraco.Core.IO; + +namespace Umbraco.Core.Manifest +{ + public class ManifestDashboardDefinition + { + private string _view; + + [JsonProperty("name", Required = Required.Always)] + public string Name { get; set; } + + [JsonProperty("alias", Required = Required.Always)] + public string Alias { get; set; } + + [JsonProperty("weight", DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] + [DefaultValue(100)] + public int Weight { get; set; } + + [JsonProperty("view", Required = Required.Always)] + public string View + { + get => _view; + set => _view = IOHelper.ResolveVirtualUrl(value); + } + + [JsonProperty("sections")] + public string[] Sections { get; set; } = Array.Empty(); + + [JsonProperty("access")] + public IAccessRule[] AccessRules { get; set; } = Array.Empty(); + } +} diff --git a/src/Umbraco.Core/Manifest/ManifestDashboardSection.cs b/src/Umbraco.Core/Manifest/ManifestDashboardSection.cs deleted file mode 100644 index c09cd6bb45..0000000000 --- a/src/Umbraco.Core/Manifest/ManifestDashboardSection.cs +++ /dev/null @@ -1,33 +0,0 @@ -using System.Collections.Generic; -using Newtonsoft.Json; - -namespace Umbraco.Core.Manifest -{ - public class ManifestDashboard - { - public ManifestDashboard() - { - Name = string.Empty; - Alias = string.Empty; - Weight = int.MaxValue; //default so we can check if this value has been explicitly set - View = string.Empty; - Sections = new List(); - } - - [JsonProperty("name")] - public string Name { get; set; } - - [JsonProperty("aias")] - public string Alias { get; set; } - - [JsonProperty("weight")] - public int Weight { get; set; } - - [JsonProperty("view")] - public string View { get; set; } - - [JsonProperty("sections")] - public List Sections { get; set; } - - } -} diff --git a/src/Umbraco.Core/Manifest/ManifestParser.cs b/src/Umbraco.Core/Manifest/ManifestParser.cs index 9a6b6ae343..fe021fae5b 100644 --- a/src/Umbraco.Core/Manifest/ManifestParser.cs +++ b/src/Umbraco.Core/Manifest/ManifestParser.cs @@ -100,7 +100,7 @@ namespace Umbraco.Core.Manifest var parameterEditors = new List(); var gridEditors = new List(); var contentApps = new List(); - var dashboards = new List(); + var dashboards = new List(); foreach (var manifest in manifests) { @@ -110,7 +110,7 @@ namespace Umbraco.Core.Manifest if (manifest.ParameterEditors != null) parameterEditors.AddRange(manifest.ParameterEditors); if (manifest.GridEditors != null) gridEditors.AddRange(manifest.GridEditors); if (manifest.ContentApps != null) contentApps.AddRange(manifest.ContentApps); - if (manifest.Dashboards != null) dashboards.AddRange(manifest.Dashboards); + if (manifest.Dashboards != null) dashboards.AddRange(manifest.Dashboards); } return new PackageManifest @@ -132,7 +132,6 @@ namespace Umbraco.Core.Manifest return new string[0]; return Directory.GetFiles(_path, "package.manifest", SearchOption.AllDirectories); } - private static string TrimPreamble(string text) { @@ -154,8 +153,8 @@ namespace Umbraco.Core.Manifest var manifest = JsonConvert.DeserializeObject(text, new DataEditorConverter(_logger), new ValueValidatorConverter(_validators), - //TODO: DO i need a dashboard one? - new ContentAppDefinitionConverter()); + new ContentAppDefinitionConverter(), + new DashboardAccessRuleConverter()); // scripts and stylesheets are raw string, must process here for (var i = 0; i < manifest.Scripts.Length; i++) @@ -169,8 +168,6 @@ namespace Umbraco.Core.Manifest if (ppEditors.Count > 0) manifest.ParameterEditors = manifest.ParameterEditors.Union(ppEditors).ToArray(); - //TODO: Do we need to deal with dashboards or are they auto parsed? - return manifest; } @@ -179,6 +176,5 @@ namespace Umbraco.Core.Manifest { return JsonConvert.DeserializeObject>(text); } - } } diff --git a/src/Umbraco.Core/Manifest/PackageManifest.cs b/src/Umbraco.Core/Manifest/PackageManifest.cs index ac472e0eec..95a5c01b6a 100644 --- a/src/Umbraco.Core/Manifest/PackageManifest.cs +++ b/src/Umbraco.Core/Manifest/PackageManifest.cs @@ -1,7 +1,5 @@ -using System.Collections.Generic; -using System; +using System; using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.PropertyEditors; @@ -23,19 +21,14 @@ namespace Umbraco.Core.Manifest [JsonProperty("parameterEditors")] public IDataEditor[] ParameterEditors { get; set; } = Array.Empty(); - + [JsonProperty("gridEditors")] public GridEditor[] GridEditors { get; set; } = Array.Empty(); [JsonProperty("contentApps")] public IContentAppDefinition[] ContentApps { get; set; } = Array.Empty(); - /// - /// The dictionary of dashboards - /// [JsonProperty("dashboards")] - public ManifestDashboard[] Dashboards { get; set; } = Array.Empty(); - - + public ManifestDashboardDefinition[] Dashboards { get; set; } = Array.Empty(); } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index d3a547f436..047f3f8cdd 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -194,8 +194,8 @@ - - + + @@ -203,7 +203,7 @@ - + @@ -334,8 +334,9 @@ + - + diff --git a/src/Umbraco.Tests/Configurations/DashboardSettings/DashboardSettingsTests.cs b/src/Umbraco.Tests/Configurations/DashboardSettings/DashboardSettingsTests.cs index f1892bdaf4..920de683b4 100644 --- a/src/Umbraco.Tests/Configurations/DashboardSettings/DashboardSettingsTests.cs +++ b/src/Umbraco.Tests/Configurations/DashboardSettings/DashboardSettingsTests.cs @@ -56,11 +56,11 @@ namespace Umbraco.Tests.Configurations.DashboardSettings Assert.AreEqual(3, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.Count()); Assert.AreEqual("translator", SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(0).Value); - Assert.AreEqual(AccessType.Deny, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(0).Action); + Assert.AreEqual(AccessRuleType.Deny, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(0).Type); Assert.AreEqual("hello", SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(1).Value); - Assert.AreEqual(AccessType.Grant, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(1).Action); + Assert.AreEqual(AccessRuleType.Grant, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(1).Type); Assert.AreEqual("world", SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(2).Value); - Assert.AreEqual(AccessType.GrantBySection, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(2).Action); + Assert.AreEqual(AccessRuleType.GrantBySection, SettingsSection.Sections.ElementAt(3).AccessRights.Rules.ElementAt(2).Type); } [Test] @@ -94,7 +94,7 @@ namespace Umbraco.Tests.Configurations.DashboardSettings public void Test_Tab_Access() { Assert.AreEqual(1, SettingsSection.Sections.ElementAt(2).Tabs.ElementAt(1).AccessRights.Rules.Count()); - Assert.AreEqual(AccessType.Grant, SettingsSection.Sections.ElementAt(2).Tabs.ElementAt(1).AccessRights.Rules.ElementAt(0).Action); + Assert.AreEqual(AccessRuleType.Grant, SettingsSection.Sections.ElementAt(2).Tabs.ElementAt(1).AccessRights.Rules.ElementAt(0).Type); Assert.AreEqual("admin", SettingsSection.Sections.ElementAt(2).Tabs.ElementAt(1).AccessRights.Rules.ElementAt(0).Value); } @@ -114,9 +114,9 @@ namespace Umbraco.Tests.Configurations.DashboardSettings public void Test_Control_Access() { Assert.AreEqual(2, SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.Count()); - Assert.AreEqual(AccessType.Deny, SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(0).Action); + Assert.AreEqual(AccessRuleType.Deny, SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(0).Type); Assert.AreEqual("editor", SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(0).Value); - Assert.AreEqual(AccessType.Deny, SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(1).Action); + Assert.AreEqual(AccessRuleType.Deny, SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(1).Type); Assert.AreEqual("writer", SettingsSection.Sections.ElementAt(3).Tabs.ElementAt(0).Controls.ElementAt(1).AccessRights.Rules.ElementAt(1).Value); } } diff --git a/src/Umbraco.Tests/Manifest/ManifestParserTests.cs b/src/Umbraco.Tests/Manifest/ManifestParserTests.cs index 5145b848ed..19aaf79581 100644 --- a/src/Umbraco.Tests/Manifest/ManifestParserTests.cs +++ b/src/Umbraco.Tests/Manifest/ManifestParserTests.cs @@ -8,6 +8,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Umbraco.Core.Cache; using Umbraco.Core.Composing; +using Umbraco.Core.Configuration.Dashboard; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; using Umbraco.Core.PropertyEditors; @@ -383,5 +384,53 @@ javascript: ['~/test.js',/*** some note about stuff asd09823-4**09234*/ '~/test2 Assert.AreEqual("icon-bar", app1.Icon); Assert.AreEqual("/App_Plugins/MyPackage/ContentApps/MyApp2.html", app1.View); } + + [Test] + public void CanParseManifest_Dashboards() + { + const string json = @"{'dashboards': [ + { + 'name': 'First One', + 'alias': 'something', + 'view': '~/App_Plugins/MyPackage/Dashboards/one.html', + 'sections': [ 'content' ], + 'access': [ {'grant':'user'}, {'deny':'foo'} ] + + }, + { + 'name': 'Second-One', + 'alias': 'something.else', + 'weight': -1, + 'view': '~/App_Plugins/MyPackage/Dashboards/two.html', + 'sections': [ 'forms' ], + } +]}"; + + var manifest = _parser.ParseManifest(json); + Assert.AreEqual(2, manifest.Dashboards.Length); + + Assert.IsInstanceOf(manifest.Dashboards[0]); + var db0 = manifest.Dashboards[0]; + Assert.AreEqual("something", db0.Alias); + Assert.AreEqual("First One", db0.Name); + Assert.AreEqual(100, db0.Weight); + Assert.AreEqual("/App_Plugins/MyPackage/Dashboards/one.html", db0.View); + Assert.AreEqual(1, db0.Sections.Length); + Assert.AreEqual("content", db0.Sections[0]); + Assert.AreEqual(2, db0.AccessRules.Length); + Assert.AreEqual(AccessRuleType.Grant, db0.AccessRules[0].Type); + Assert.AreEqual("user", db0.AccessRules[0].Value); + Assert.AreEqual(AccessRuleType.Deny, db0.AccessRules[1].Type); + Assert.AreEqual("foo", db0.AccessRules[1].Value); + + Assert.IsInstanceOf(manifest.Dashboards[1]); + var db1 = manifest.Dashboards[1]; + Assert.AreEqual("something.else", db1.Alias); + Assert.AreEqual("Second-One", db1.Name); + Assert.AreEqual(-1, db1.Weight); + Assert.AreEqual("/App_Plugins/MyPackage/Dashboards/two.html", db1.View); + Assert.AreEqual(1, db1.Sections.Length); + Assert.AreEqual("forms", db1.Sections[0]); + } } } diff --git a/src/Umbraco.Web/Editors/DashboardController.cs b/src/Umbraco.Web/Editors/DashboardController.cs index 186e0e0a13..72b7acc9e7 100644 --- a/src/Umbraco.Web/Editors/DashboardController.cs +++ b/src/Umbraco.Web/Editors/DashboardController.cs @@ -25,11 +25,11 @@ namespace Umbraco.Web.Editors [WebApi.UmbracoAuthorize] public class DashboardController : UmbracoApiController { - private readonly DashboardHelper _dashboardHelper; + private readonly Dashboards _dashboards; - public DashboardController(DashboardHelper dashboardHelper) + public DashboardController(Dashboards dashboards) { - _dashboardHelper = dashboardHelper; + _dashboards = dashboards; } //we have just one instance of HttpClient shared for the entire application @@ -120,7 +120,7 @@ namespace Umbraco.Web.Editors [ValidateAngularAntiForgeryToken] public IEnumerable> GetDashboard(string section) { - return _dashboardHelper.GetDashboard(section, Security.CurrentUser); + return _dashboards.GetDashboards(section, Security.CurrentUser); } } } diff --git a/src/Umbraco.Web/Editors/DashboardHelper.cs b/src/Umbraco.Web/Editors/DashboardHelper.cs deleted file mode 100644 index 365e0c6f9a..0000000000 --- a/src/Umbraco.Web/Editors/DashboardHelper.cs +++ /dev/null @@ -1,176 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using Umbraco.Core; -using Umbraco.Core.Cache; -using Umbraco.Core.Configuration.Dashboard; -using Umbraco.Core.IO; -using Umbraco.Core.Manifest; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Services; -using Umbraco.Web.Models.ContentEditing; - -namespace Umbraco.Web.Editors -{ - public class DashboardHelper - { - private readonly ISectionService _sectionService; - private readonly IDashboardSection _dashboardSection; - private readonly ManifestParser _manifestParser; - - public DashboardHelper(ISectionService sectionService, IDashboardSection dashboardSection, ManifestParser manifestParser) - { - _sectionService = sectionService ?? throw new ArgumentNullException(nameof(sectionService)); - _dashboardSection = dashboardSection; - _manifestParser = manifestParser; - } - - /// - /// Returns the dashboard models per section for the current user and it's access - /// - /// - /// - public IDictionary>> GetDashboards(IUser currentUser) - { - var result = new Dictionary>>(); - foreach (var section in _sectionService.GetSections()) - { - result[section.Alias] = GetDashboard(section.Alias, currentUser); - } - return result; - } - - /// - /// Returns the dashboard model for the given section based on the current user and it's access - /// - /// - /// - /// - public IEnumerable> GetDashboard(string section, IUser currentUser) - { - var configDashboards = GetDashboardsFromConfig(1, section, currentUser); - var pluginDashboards = GetDashboardsFromPlugins(configDashboards.Count + 1, section, currentUser); - - //now we need to merge them, the plugin ones would replace anything matched in the config one where the tab alias matches - var added = new List>(); //to track the ones we'll add - foreach (var configDashboard in configDashboards) - { - var matched = pluginDashboards.Where(x => string.Equals(x.Alias, configDashboard.Alias, StringComparison.InvariantCultureIgnoreCase)).ToList(); - foreach (var tab in matched) - { - configDashboard.Label = tab.Label; //overwrite - configDashboard.Properties = configDashboard.Properties.Concat(tab.Properties).ToList(); //combine - added.Add(tab); //track this - } - } - - //now add the plugin dashboards to the config dashboards that have not already been added - var toAdd = pluginDashboards.Where(pluginDashboard => added.Contains(pluginDashboard) == false).ToList(); - configDashboards.AddRange(toAdd); - - //last thing is to re-sort and ID the tabs - configDashboards.Sort((tab, tab1) => tab.Id > tab1.Id ? 1 : 0); - for (var index = 0; index < configDashboards.Count; index++) - { - var tab = configDashboards[index]; - tab.Id = (index + 1); - if (tab.Id == 1) - tab.IsActive = true; - } - - return configDashboards; - } - - private List> GetDashboardsFromConfig(int startTabId, string section, IUser currentUser) - { - var tabs = new List>(); - var i = startTabId; - - //disable packages section dashboard - if (section == "packages") return tabs; - - foreach (var dashboardSection in _dashboardSection.Sections.Where(x => x.Areas.Contains(section))) - { - //we need to validate access to this section - if (DashboardSecurity.AuthorizeAccess(dashboardSection, currentUser, _sectionService) == false) - continue; - - //User is authorized - foreach (var tab in dashboardSection.Tabs) - { - //we need to validate access to this tab - if (DashboardSecurity.AuthorizeAccess(tab, currentUser, _sectionService) == false) - continue; - - var dashboardControls = new List(); - - foreach (var control in tab.Controls) - { - if (DashboardSecurity.AuthorizeAccess(control, currentUser, _sectionService) == false) - continue; - - var dashboardControl = new DashboardControl(); - var controlPath = control.ControlPath.Trim(); - dashboardControl.Caption = control.PanelCaption; - dashboardControl.Path = IOHelper.FindFile(controlPath); - if (controlPath.ToLowerInvariant().EndsWith(".ascx".ToLowerInvariant())) - throw new NotSupportedException("Legacy UserControl (.ascx) dashboards are no longer supported"); - - dashboardControls.Add(dashboardControl); - } - - tabs.Add(new Tab - { - Id = i, - Alias = tab.Caption.ToSafeAlias(), - Label = tab.Caption, - Properties = dashboardControls - }); - - i++; - } - } - - //In case there are no tabs or a user doesn't have access the empty tabs list is returned - return tabs; - } - - private List> GetDashboardsFromPlugins(int startTabId, string section, IUser currentUser) - { - //TODO: Need to integrate the security with the manifest dashboards - - var appPlugins = new DirectoryInfo(IOHelper.MapPath(SystemDirectories.AppPlugins)); - - var tabs = new List>(); - var i = startTabId; - - foreach (var dashboard in _manifestParser.Manifest.Dashboards.Where(x => x.Sections.InvariantContains(section))) - { - var dashboardControls = new List(); - var view = dashboard.View.Trim(); - var dashboardControl = new DashboardControl - { - Path = IOHelper.FindFile(view) - }; - - if (view.ToLowerInvariant().EndsWith(".ascx".ToLowerInvariant())) - throw new NotSupportedException("Legacy UserControl (.ascx) dashboards are no longer supported"); - - dashboardControls.Add(dashboardControl); - - tabs.Add(new Tab - { - //assign the Id to the value of the index if one was defined, then we'll use the Id to sort later - Id = dashboard.Weight == int.MaxValue ? i : dashboard.Weight, - Alias = dashboard.Alias.ToSafeAlias(), - Label = dashboard.Name, - Properties = dashboardControls - }); - - i++; - } - return tabs; - } - } -} diff --git a/src/Umbraco.Web/Editors/DashboardSecurity.cs b/src/Umbraco.Web/Editors/DashboardSecurity.cs index 87ed5af196..1481606c7e 100644 --- a/src/Umbraco.Web/Editors/DashboardSecurity.cs +++ b/src/Umbraco.Web/Editors/DashboardSecurity.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Globalization; using System.Linq; using Umbraco.Core; @@ -17,105 +18,97 @@ namespace Umbraco.Web.Editors public static bool AuthorizeAccess(ISection dashboardSection, IUser user, ISectionService sectionService) { - if (user.Id.ToString(CultureInfo.InvariantCulture) == 0.ToInvariantString()) - { - return true; - } - - var denyTypes = dashboardSection.AccessRights.Rules.Where(x => x.Action == AccessType.Deny).ToArray(); - var grantedTypes = dashboardSection.AccessRights.Rules.Where(x => x.Action == AccessType.Grant).ToArray(); - var grantedBySectionTypes = dashboardSection.AccessRights.Rules.Where(x => x.Action == AccessType.GrantBySection).ToArray(); - - return CheckUserAccessByRules(user, sectionService, denyTypes, grantedTypes, grantedBySectionTypes); + return CheckUserAccessByRules(user, sectionService, dashboardSection.AccessRights.Rules); } public static bool AuthorizeAccess(IDashboardTab dashboardTab, IUser user, ISectionService sectionService) { - if (user.Id.ToString(CultureInfo.InvariantCulture) == Constants.System.Root.ToInvariantString()) - { - return true; - } - - var denyTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.Deny).ToArray(); - var grantedTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.Grant).ToArray(); - var grantedBySectionTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.GrantBySection).ToArray(); - - return CheckUserAccessByRules(user, sectionService, denyTypes, grantedTypes, grantedBySectionTypes); + return CheckUserAccessByRules(user, sectionService, dashboardTab.AccessRights.Rules); } - public static bool AuthorizeAccess(IDashboardControl dashboardTab, IUser user, ISectionService sectionService) + public static bool AuthorizeAccess(IDashboardControl dashboardControl, IUser user, ISectionService sectionService) { - if (user.Id.ToString(CultureInfo.InvariantCulture) == Constants.System.Root.ToInvariantString()) - { - return true; - } - - var denyTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.Deny).ToArray(); - var grantedTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.Grant).ToArray(); - var grantedBySectionTypes = dashboardTab.AccessRights.Rules.Where(x => x.Action == AccessType.GrantBySection).ToArray(); - - return CheckUserAccessByRules(user, sectionService, denyTypes, grantedTypes, grantedBySectionTypes); + return CheckUserAccessByRules(user, sectionService, dashboardControl.AccessRights.Rules); } - public static bool CheckUserAccessByRules(IUser user, ISectionService sectionService, IAccessItem[] denyTypes, IAccessItem[] grantedTypes, IAccessItem[] grantedBySectionTypes) + private static (IAccessRule[], IAccessRule[], IAccessRule[]) GroupRules(IEnumerable rules) { - var allowedSoFar = false; + IAccessRule[] denyRules = null, grantRules = null, grantBySectionRules = null; - // if there's no grantBySection or grant rules defined - we allow access so far and skip to checking deny rules - if (grantedBySectionTypes.Any() == false && grantedTypes.Any() == false) + var groupedRules = rules.GroupBy(x => x.Type); + foreach (var group in groupedRules) { - allowedSoFar = true; + var a = group.ToArray(); + switch (group.Key) + { + case AccessRuleType.Deny: + denyRules = a; + break; + case AccessRuleType.Grant: + grantRules = a; + break; + case AccessRuleType.GrantBySection: + grantBySectionRules = a; + break; + default: + throw new Exception("panic"); + } } - // else we check the rules and only allow if one matches - else + + return (denyRules ?? Array.Empty(), grantRules ?? Array.Empty(), grantBySectionRules ?? Array.Empty()); + } + + public static bool CheckUserAccessByRules(IUser user, ISectionService sectionService, IEnumerable rules) + { + if (user.Id == Constants.Security.SuperUserId) + return true; + + var (denyRules, grantRules, grantBySectionRules) = GroupRules(rules); + + var hasAccess = true; + string[] assignedUserGroups = null; + + // if there are no grant rules, then access is granted by default, unless denied + // otherwise, grant rules determine if access can be granted at all + if (grantBySectionRules.Length > 0 || grantRules.Length > 0) { + hasAccess = false; + // check if this item has any grant-by-section arguments. // if so check if the user has access to any of the sections approved, if so they will be allowed to see it (so far) - if (grantedBySectionTypes.Any()) + if (grantBySectionRules.Length > 0) { - var allowedApps = sectionService.GetAllowedSections(Convert.ToInt32(user.Id)) - .Select(x => x.Alias) - .ToArray(); + var allowedSections = sectionService.GetAllowedSections(user.Id).Select(x => x.Alias).ToArray(); + var wantedSections = grantBySectionRules.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); - var allApprovedSections = grantedBySectionTypes.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); - if (allApprovedSections.Any(allowedApps.Contains)) - { - allowedSoFar = true; - } + if (wantedSections.Intersect(allowedSections).Any()) + hasAccess = true; } // if not already granted access, check if this item as any grant arguments. // if so check if the user is in one of the user groups approved, if so they will be allowed to see it (so far) - if (allowedSoFar == false && grantedTypes.Any()) + if (hasAccess == false && grantRules.Any()) { - var allApprovedUserTypes = grantedTypes.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); - foreach (var userGroup in user.Groups) - { - if (allApprovedUserTypes.InvariantContains(userGroup.Alias)) - { - allowedSoFar = true; - break; - } - } + assignedUserGroups = user.Groups.Select(x => x.Alias).ToArray(); + var wantedUserGroups = grantRules.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); + + if (wantedUserGroups.Intersect(assignedUserGroups).Any()) + hasAccess = true; } } + if (!hasAccess || denyRules.Length == 0) + return false; + // check if this item has any deny arguments, if so check if the user is in one of the denied user groups, if so they will // be denied to see it no matter what - if (denyTypes.Any()) - { - var allDeniedUserTypes = denyTypes.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); - foreach (var userGroup in user.Groups) - { - if (allDeniedUserTypes.InvariantContains(userGroup.Alias)) - { - allowedSoFar = false; - break; - } - } - } + assignedUserGroups = assignedUserGroups ?? user.Groups.Select(x => x.Alias).ToArray(); + var deniedUserGroups = denyRules.SelectMany(g => g.Value.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)).ToArray(); - return allowedSoFar; + if (deniedUserGroups.Intersect(assignedUserGroups).Any()) + hasAccess = false; + + return hasAccess; } } } diff --git a/src/Umbraco.Web/Editors/Dashboards.cs b/src/Umbraco.Web/Editors/Dashboards.cs new file mode 100644 index 0000000000..4bf2d81045 --- /dev/null +++ b/src/Umbraco.Web/Editors/Dashboards.cs @@ -0,0 +1,159 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core; +using Umbraco.Core.Configuration.Dashboard; +using Umbraco.Core.IO; +using Umbraco.Core.Manifest; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; +using Umbraco.Web.Models.ContentEditing; + +namespace Umbraco.Web.Editors +{ + public class Dashboards + { + private readonly ISectionService _sectionService; + private readonly IDashboardSection _dashboardSection; + private readonly ManifestParser _manifestParser; + + public Dashboards(ISectionService sectionService, IDashboardSection dashboardSection, ManifestParser manifestParser) + { + _sectionService = sectionService ?? throw new ArgumentNullException(nameof(sectionService)); + _dashboardSection = dashboardSection; + _manifestParser = manifestParser; + } + + /// + /// Gets all dashboards, organized by section, for a user. + /// + public IDictionary>> GetDashboards(IUser currentUser) + { + return _sectionService.GetSections().ToDictionary(x => x.Alias, x => GetDashboards(x.Alias, currentUser)); + } + + /// + /// Returns dashboards for a specific section, for a user. + /// + public IEnumerable> GetDashboards(string section, IUser currentUser) + { + var tabId = 1; + var configDashboards = GetDashboardsFromConfig(ref tabId, section, currentUser); + var pluginDashboards = GetDashboardsFromPlugins(ref tabId, section, currentUser); + + // merge dashboards + // both collections contain tab.alias -> controls + var dashboards = configDashboards; + + // until now, it was fine to have duplicate tab.aliases in configDashboard + // so... the rule should be - just merge whatever we get, don't be clever + dashboards.AddRange(pluginDashboards); + + // re-sort by id + dashboards.Sort((tab1, tab2) => tab1.Id > tab2.Id ? 1 : 0); + + // re-assign ids (why?) + var i = 1; + foreach (var tab in dashboards) + { + tab.Id = i++; + tab.IsActive = tab.Id == 1; + } + + return configDashboards; + } + + // note: + // in dashboard.config we have 'sections' which define 'tabs' for 'areas' + // and 'areas' are the true UI sections - and each tab can have more than + // one control + // in a manifest, we directly have 'dashboards' which map to a unique + // control in a tab + + // gets all tabs & controls from the config file + private List> GetDashboardsFromConfig(ref int tabId, string section, IUser currentUser) + { + var tabs = new List>(); + + // disable packages section dashboard + if (section == "packages") return tabs; + + foreach (var dashboardSection in _dashboardSection.Sections.Where(x => x.Areas.InvariantContains(section))) + { + // validate access to this section + if (!DashboardSecurity.AuthorizeAccess(dashboardSection, currentUser, _sectionService)) + continue; + + foreach (var tab in dashboardSection.Tabs) + { + // validate access to this tab + if (!DashboardSecurity.AuthorizeAccess(tab, currentUser, _sectionService)) + continue; + + var dashboardControls = new List(); + + foreach (var control in tab.Controls) + { + // validate access to this control + if (!DashboardSecurity.AuthorizeAccess(control, currentUser, _sectionService)) + continue; + + // create and add control + var dashboardControl = new DashboardControl + { + Caption = control.PanelCaption, + Path = IOHelper.FindFile(control.ControlPath.Trim()) + }; + + if (dashboardControl.Path.InvariantEndsWith(".ascx")) + throw new NotSupportedException("Legacy UserControl (.ascx) dashboards are no longer supported."); + + dashboardControls.Add(dashboardControl); + } + + // create and add tab + tabs.Add(new Tab + { + Id = tabId++, + Alias = tab.Caption.ToSafeAlias(), + Label = tab.Caption, + Properties = dashboardControls + }); + } + } + + return tabs; + } + + private List> GetDashboardsFromPlugins(ref int tabId, string section, IUser currentUser) + { + var tabs = new List>(); + + foreach (var dashboard in _manifestParser.Manifest.Dashboards.Where(x => x.Sections.InvariantContains(section)).OrderBy(x => x.Weight)) + { + // validate access + if (!DashboardSecurity.CheckUserAccessByRules(currentUser, _sectionService, dashboard.AccessRules)) + continue; + + var dashboardControl = new DashboardControl + { + Caption = "", + Path = IOHelper.FindFile(dashboard.View.Trim()) + }; + + if (dashboardControl.Path.InvariantEndsWith(".ascx")) + throw new NotSupportedException("Legacy UserControl (.ascx) dashboards are no longer supported."); + + tabs.Add(new Tab + { + Id = tabId++, + Alias = dashboard.Alias.ToSafeAlias(), + Label = dashboard.Name, + Properties = new[] { dashboardControl } + }); + } + + return tabs; + } + } +} diff --git a/src/Umbraco.Web/Editors/SectionController.cs b/src/Umbraco.Web/Editors/SectionController.cs index b7db641946..bcf451a5bb 100644 --- a/src/Umbraco.Web/Editors/SectionController.cs +++ b/src/Umbraco.Web/Editors/SectionController.cs @@ -17,16 +17,15 @@ namespace Umbraco.Web.Editors [PluginController("UmbracoApi")] public class SectionController : UmbracoAuthorizedJsonController { - private readonly DashboardHelper _dashboardHelper; + private readonly Dashboards _dashboards; - public SectionController(DashboardHelper dashboardHelper) + public SectionController(Dashboards dashboards) { - _dashboardHelper = dashboardHelper; + _dashboards = dashboards; } public IEnumerable
GetSections() { - var sections = Services.SectionService.GetAllowedSections(Security.GetUserId().ResultOr(0)); var sectionModels = sections.Select(Mapper.Map).ToArray(); @@ -39,24 +38,18 @@ namespace Umbraco.Web.Editors Current.Container.InjectProperties(appTreeController); appTreeController.ControllerContext = ControllerContext; - var dashboards = _dashboardHelper.GetDashboards(Security.CurrentUser); + var dashboards = _dashboards.GetDashboards(Security.CurrentUser); + //now we can add metadata for each section so that the UI knows if there's actually anything at all to render for //a dashboard for a given section, then the UI can deal with it accordingly (i.e. redirect to the first tree) foreach (var section in sectionModels) { - var hasDashboards = false; - if (dashboards.TryGetValue(section.Alias, out var dashboardsForSection)) - { - if (dashboardsForSection.Any()) - hasDashboards = true; - } + var hasDashboards = dashboards.TryGetValue(section.Alias, out var dashboardsForSection) && dashboardsForSection.Any(); + if (hasDashboards) continue; - if (hasDashboards == false) - { - //get the first tree in the section and get it's root node route path - var sectionRoot = appTreeController.GetApplicationTrees(section.Alias, null, null).Result; - section.RoutePath = GetRoutePathForFirstTree(sectionRoot); - } + // get the first tree in the section and get its root node route path + var sectionRoot = appTreeController.GetApplicationTrees(section.Alias, null, null).Result; + section.RoutePath = GetRoutePathForFirstTree(sectionRoot); } return sectionModels; diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index f8963f2f01..97cebdb076 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -122,7 +122,7 @@ namespace Umbraco.Web.Runtime composition.Container.EnableMvc(); // does container.EnablePerWebRequestScope() composition.Container.ScopeManagerProvider = smp; // reverts - we will do it last (in WebRuntime) - composition.Container.RegisterSingleton(); + composition.Container.RegisterSingleton(); composition.Container.RegisterUmbracoControllers(typeLoader, GetType().Assembly); composition.Container.EnableWebApi(GlobalConfiguration.Configuration); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 32134e7a45..c17536ab4d 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -190,7 +190,7 @@ - +