From 945050858e605fc78e11da2f6d2810400599bc35 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 18 Jul 2017 09:11:12 +0200 Subject: [PATCH 1/5] Cleanup --- src/Umbraco.Core/Models/Content.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index b3fbe4164b..70cb877183 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -23,7 +23,7 @@ namespace Umbraco.Core.Models private DateTime? _expireDate; private int _writer; private string _nodeName;//NOTE Once localization is introduced this will be the non-localized Node Name. - private bool _permissionsChanged; + /// /// Constructor for creating a Content object /// @@ -32,7 +32,7 @@ namespace Umbraco.Core.Models /// ContentType for the current Content object public Content(string name, IContent parent, IContentType contentType) : this(name, parent, contentType, new PropertyCollection()) - { + { } /// @@ -68,7 +68,7 @@ namespace Umbraco.Core.Models /// Id of the Parent content /// ContentType for the current Content object /// Collection of properties - public Content(string name, int parentId, IContentType contentType, PropertyCollection properties) + public Content(string name, int parentId, IContentType contentType, PropertyCollection properties) : base(name, parentId, contentType, properties) { Mandate.ParameterNotNull(contentType, "contentType"); @@ -94,7 +94,7 @@ namespace Umbraco.Core.Models /// This is used to override the default one from the ContentType. /// /// - /// If no template is explicitly set on the Content object, + /// If no template is explicitly set on the Content object, /// the Default template from the ContentType will be returned. /// [DataMember] @@ -195,7 +195,7 @@ namespace Umbraco.Core.Models get { return _nodeName; } set { SetPropertyValueAndDetectChanges(value, ref _nodeName, Ps.Value.NodeNameSelector); } } - + /// /// Gets the ContentType used by this content object @@ -283,7 +283,7 @@ namespace Umbraco.Core.Models ChangePublishedState(PublishedState.Unpublished); } } - + /// /// Method to call when Entity is being updated /// From 8c3f6dfc251d6dc4bd325866dac369f238711dc5 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 18 Jul 2017 15:09:34 +0200 Subject: [PATCH 2/5] U4-10173 - refactor start nodes --- src/Umbraco.Core/Models/Membership/User.cs | 4 +- src/Umbraco.Core/Models/UserExtensions.cs | 110 ++++++++++++++---- .../Models/UserExtensionsTests.cs | 3 +- src/Umbraco.Web/Editors/ContentController.cs | 13 +-- src/Umbraco.Web/Editors/EntityController.cs | 12 +- src/Umbraco.Web/Editors/MediaController.cs | 15 +-- .../Trees/ContentTreeController.cs | 2 +- src/Umbraco.Web/Trees/MediaTreeController.cs | 2 +- .../BasePages/BasePage.cs | 30 ++--- 9 files changed, 122 insertions(+), 69 deletions(-) diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 26e4f79f69..1e457eecf6 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -308,7 +308,7 @@ namespace Umbraco.Core.Models.Membership [IgnoreDataMember] public int[] AllStartContentIds { - get { return _allStartContentIds ?? (_allStartContentIds = StartContentIds.Concat(Groups.Where(x => x.StartContentId.HasValue).Select(x => x.StartContentId.Value)).Distinct().ToArray()); } + get { return this.GetAllContentStartNodes(ApplicationContext.Current.Services.EntityService); } } /// @@ -317,7 +317,7 @@ namespace Umbraco.Core.Models.Membership [IgnoreDataMember] public int[] AllStartMediaIds { - get { return _allStartMediaIds ?? (_allStartMediaIds = StartMediaIds.Concat(Groups.Where(x => x.StartMediaId.HasValue).Select(x => x.StartMediaId.Value)).Distinct().ToArray()); } + get { return this.GetAllMediaStartNodes(ApplicationContext.Current.Services.EntityService); } } /// diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 58a844ddf1..4f043c0ec7 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -107,19 +107,36 @@ namespace Umbraco.Core.Models } } - /// - /// Checks if the user has access to the content item based on their start noe - /// - /// - /// - /// - internal static bool HasPathAccess(this IUser user, IContent content) + internal static bool HasContentRootAccess(this IUser user, IEntityService entityService) { - if (user == null) throw new ArgumentNullException("user"); - if (content == null) throw new ArgumentNullException("content"); - return HasPathAccess(content.Path, user.AllStartContentIds, Constants.System.RecycleBinContent); + return HasPathAccess(Constants.System.Root.ToInvariantString(), user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); } - + + internal static bool HasContentBinAccess(this IUser user, IEntityService entityService) + { + return HasPathAccess(Constants.System.RecycleBinContent.ToInvariantString(), user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); + } + + internal static bool HasMediaRootAccess(this IUser user, IEntityService entityService) + { + return HasPathAccess(Constants.System.Root.ToInvariantString(), user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + } + + internal static bool HasMediaBinAccess(this IUser user, IEntityService entityService) + { + return HasPathAccess(Constants.System.RecycleBinMedia.ToInvariantString(), user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + } + + internal static bool HasPathAccess(this IUser user, IContent content, IEntityService entityService) + { + return HasPathAccess(content.Path, user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); + } + + internal static bool HasPathAccess(this IUser user, IMedia media, IEntityService entityService) + { + return HasPathAccess(media.Path, user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + } + internal static bool HasPathAccess(string path, int[] startNodeIds, int recycleBinId) { if (string.IsNullOrWhiteSpace(path)) throw new ArgumentException("Value cannot be null or whitespace.", "path"); @@ -151,19 +168,6 @@ namespace Umbraco.Core.Models return false; } - /// - /// Checks if the user has access to the media item based on their start noe - /// - /// - /// - /// - internal static bool HasPathAccess(this IUser user, IMedia media) - { - if (user == null) throw new ArgumentNullException("user"); - if (media == null) throw new ArgumentNullException("media"); - return HasPathAccess(media.Path, user.AllStartMediaIds, Constants.System.RecycleBinMedia); - } - /// /// Determines whether this user is an admin. /// @@ -176,5 +180,63 @@ namespace Umbraco.Core.Models if (user == null) throw new ArgumentNullException("user"); return user.Groups != null && user.Groups.Any(x => x.Alias == Constants.Security.AdminGroupAlias); } + + public static int[] GetAllContentStartNodes(this IUser user, IEntityService entityService) + { + var gsn = user.Groups.Where(x => x.StartContentId.HasValue).Select(x => x.StartContentId.Value).Distinct().ToArray(); + var usn = user.StartContentIds; + return CombineStartNodes(gsn, usn, entityService); + } + + public static int[] GetAllMediaStartNodes(this IUser user, IEntityService entityService) + { + var gsn = user.Groups.Where(x => x.StartMediaId.HasValue).Select(x => x.StartMediaId.Value).Distinct().ToArray(); + var usn = user.StartMediaIds; + return CombineStartNodes(gsn, usn, entityService); + } + + private static bool StartsWithPath(string test, string path) + { + return test.StartsWith(path) && test.Length > path.Length && test[path.Length] == ','; + } + + private static int[] CombineStartNodes(int[] groupSn, int[] userSn, IEntityService entityService) + { + // assume groupSn and userSn each don't contain duplicates + + var asn = groupSn.Concat(userSn).Distinct().ToArray(); + var paths = entityService.GetAll(UmbracoObjectTypes.Document, asn).ToDictionary(x => x.Id, x => x.Path); + + paths[-1] = "-1"; // entityService does not get that one + + var lsn = new List(); + foreach (var sn in groupSn) + { + var snp = paths[sn]; + if (lsn.Any(x => StartsWithPath(snp, paths[x]))) continue; // skip if something above this sn + lsn.RemoveAll(x => StartsWithPath(paths[x], snp)); // remove anything below this sn + lsn.Add(sn); + } + + var usn = new List(); + foreach (var sn in userSn) + { + if (groupSn.Contains(sn)) continue; + + var snp = paths[sn]; + if (usn.Any(x => StartsWithPath(paths[x], snp))) continue; // skip if something below this sn + usn.RemoveAll(x => StartsWithPath(snp, paths[x])); // remove anything above this sn + usn.Add(sn); + } + + foreach (var sn in usn) + { + var snp = paths[sn]; + lsn.RemoveAll(x => StartsWithPath(snp, paths[x]) || StartsWithPath(paths[x], snp)); // remove anything above or below this sn + lsn.Add(sn); + } + + return lsn.ToArray(); + } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/Models/UserExtensionsTests.cs b/src/Umbraco.Tests/Models/UserExtensionsTests.cs index 653b8d5a27..4d3514bf00 100644 --- a/src/Umbraco.Tests/Models/UserExtensionsTests.cs +++ b/src/Umbraco.Tests/Models/UserExtensionsTests.cs @@ -1,5 +1,6 @@ using Moq; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; @@ -23,7 +24,7 @@ namespace Umbraco.Tests.Models contentMock.Setup(c => c.Path).Returns(contentPath); var content = contentMock.Object; - Assert.AreEqual(outcome, user.HasPathAccess(content)); + Assert.AreEqual(outcome, user.HasPathAccess(content, ApplicationContext.Current.Services.EntityService)); } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index b624041992..8034485785 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1059,17 +1059,12 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); } + var entityService = ApplicationContext.Current.Services.EntityService; var hasPathAccess = (nodeId == Constants.System.Root) - ? UserExtensions.HasPathAccess( - Constants.System.Root.ToInvariantString(), - user.AllStartContentIds, - Constants.System.RecycleBinContent) + ? user.HasContentRootAccess(entityService) : (nodeId == Constants.System.RecycleBinContent) - ? UserExtensions.HasPathAccess( - Constants.System.RecycleBinContent.ToInvariantString(), - user.AllStartContentIds, - Constants.System.RecycleBinContent) - : user.HasPathAccess(contentItem); + ? user.HasContentBinAccess(entityService) + : user.HasPathAccess(contentItem, entityService); if (hasPathAccess == false) { diff --git a/src/Umbraco.Web/Editors/EntityController.cs b/src/Umbraco.Web/Editors/EntityController.cs index a2aa8e6b4d..29e8bc9999 100644 --- a/src/Umbraco.Web/Editors/EntityController.cs +++ b/src/Umbraco.Web/Editors/EntityController.cs @@ -551,10 +551,10 @@ namespace Umbraco.Web.Editors switch (type) { case UmbracoEntityTypes.Document: - aids = Security.CurrentUser.AllStartContentIds; + aids = Security.CurrentUser.GetAllContentStartNodes(Services.EntityService); break; case UmbracoEntityTypes.Media: - aids = Security.CurrentUser.AllStartMediaIds; + aids = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService); break; } @@ -695,14 +695,14 @@ namespace Umbraco.Web.Editors type = "media"; AddExamineSearchFrom(searchFrom, sb); - AddExamineUserStartNode(Security.CurrentUser.AllStartMediaIds, sb); + AddExamineUserStartNode(Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService), sb); break; case UmbracoEntityTypes.Document: type = "content"; AddExamineSearchFrom(searchFrom, sb); - AddExamineUserStartNode(Security.CurrentUser.AllStartContentIds, sb); + AddExamineUserStartNode(Security.CurrentUser.GetAllContentStartNodes(Services.EntityService), sb); break; default: @@ -950,10 +950,10 @@ namespace Umbraco.Web.Editors switch (entityType) { case UmbracoEntityTypes.Document: - aids = Security.CurrentUser.AllStartContentIds; + aids = Security.CurrentUser.GetAllContentStartNodes(Services.EntityService); break; case UmbracoEntityTypes.Media: - aids = Security.CurrentUser.AllStartMediaIds; + aids = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService); break; } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index 1603615a58..5905af9532 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -254,7 +254,7 @@ namespace Umbraco.Web.Editors private int[] _userStartNodes; protected int[] UserStartNodes { - get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.AllStartMediaIds); } + get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService)); } } /// @@ -910,17 +910,12 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); } + var entityService = Core.ApplicationContext.Current.Services.EntityService; var hasPathAccess = (nodeId == Constants.System.Root) - ? UserExtensions.HasPathAccess( - Constants.System.Root.ToInvariantString(), - user.AllStartMediaIds, - Constants.System.RecycleBinMedia) + ? user.HasMediaRootAccess(entityService) : (nodeId == Constants.System.RecycleBinMedia) - ? UserExtensions.HasPathAccess( - Constants.System.RecycleBinMedia.ToInvariantString(), - user.AllStartMediaIds, - Constants.System.RecycleBinMedia) - : user.HasPathAccess(media); + ? user.HasMediaBinAccess(entityService) + : user.HasPathAccess(media, entityService); return hasPathAccess; } diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 9c15a8b462..5e982dfd9d 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -215,7 +215,7 @@ namespace Umbraco.Web.Trees { return false; } - return Security.CurrentUser.HasPathAccess(content); + return Security.CurrentUser.HasPathAccess(content, Services.EntityService); } /// diff --git a/src/Umbraco.Web/Trees/MediaTreeController.cs b/src/Umbraco.Web/Trees/MediaTreeController.cs index 61264e8af8..2a25648f77 100644 --- a/src/Umbraco.Web/Trees/MediaTreeController.cs +++ b/src/Umbraco.Web/Trees/MediaTreeController.cs @@ -163,7 +163,7 @@ namespace Umbraco.Web.Trees { return false; } - return Security.CurrentUser.HasPathAccess(media); + return Security.CurrentUser.HasPathAccess(media, Services.EntityService); } } } \ No newline at end of file diff --git a/src/umbraco.businesslogic/BasePages/BasePage.cs b/src/umbraco.businesslogic/BasePages/BasePage.cs index 2fb23571c6..fcfd9ecf44 100644 --- a/src/umbraco.businesslogic/BasePages/BasePage.cs +++ b/src/umbraco.businesslogic/BasePages/BasePage.cs @@ -23,7 +23,7 @@ namespace umbraco.BasePages { /// /// umbraco.BasePages.BasePage is the default page type for the umbraco backend. - /// The basepage keeps track of the current user and the page context. But does not + /// The basepage keeps track of the current user and the page context. But does not /// Restrict access to the page itself. /// The keep the page secure, the umbracoEnsuredPage class should be used instead /// @@ -33,7 +33,7 @@ namespace umbraco.BasePages private User _user; private bool _userisValidated = false; private ClientTools _clientTools; - + /// /// The path to the umbraco root folder /// @@ -56,7 +56,7 @@ namespace umbraco.BasePages protected static ISqlHelper SqlHelper { get { return BusinessLogic.Application.SqlHelper; } - } + } /// /// Returns the current ApplicationContext @@ -83,7 +83,7 @@ namespace umbraco.BasePages } /// - /// Returns the current BasePage for the current request. + /// Returns the current BasePage for the current request. /// This assumes that the current page is a BasePage, otherwise, returns null; /// [Obsolete("Should use the Umbraco.Web.UmbracoContext.Current singleton instead to access common methods and properties")] @@ -93,9 +93,9 @@ namespace umbraco.BasePages { var page = HttpContext.Current.CurrentHandler as BasePage; if (page != null) return page; - //the current handler is not BasePage but people might be expecting this to be the case if they + //the current handler is not BasePage but people might be expecting this to be the case if they // are using this singleton accesor... which is legacy code now and shouldn't be used. When people - // start using Umbraco.Web.UI.Pages.BasePage then this will not be the case! So, we'll just return a + // start using Umbraco.Web.UI.Pages.BasePage then this will not be the case! So, we'll just return a // new instance of BasePage as a hack to make it work. if (HttpContext.Current.Items["umbraco.BasePages.BasePage"] == null) { @@ -186,7 +186,7 @@ namespace umbraco.BasePages var identity = HttpContext.Current.GetCurrentIdentity( //DO NOT AUTO-AUTH UNLESS THE CURRENT HANDLER IS WEBFORMS! // Without this check, anything that is using this legacy API, like ui.Text will - // automatically log the back office user in even if it is a front-end request (if there is + // automatically log the back office user in even if it is a front-end request (if there is // a back office user logged in. This can cause problems becaues the identity is changing mid // request. For example: http://issues.umbraco.org/issue/U4-4010 HttpContext.Current.CurrentHandler is Page); @@ -217,7 +217,7 @@ namespace umbraco.BasePages var identity = HttpContext.Current.GetCurrentIdentity( //DO NOT AUTO-AUTH UNLESS THE CURRENT HANDLER IS WEBFORMS! // Without this check, anything that is using this legacy API, like ui.Text will - // automatically log the back office user in even if it is a front-end request (if there is + // automatically log the back office user in even if it is a front-end request (if there is // a back office user logged in. This can cause problems becaues the identity is changing mid // request. For example: http://issues.umbraco.org/issue/U4-4010 HttpContext.Current.CurrentHandler is Page); @@ -234,7 +234,7 @@ namespace umbraco.BasePages { var ticket = HttpContext.Current.GetUmbracoAuthTicket(); if (ticket.Expired) return 0; - var ticks = ticket.Expiration.Ticks - DateTime.Now.Ticks; + var ticks = ticket.Expiration.Ticks - DateTime.Now.Ticks; return ticks; } @@ -251,7 +251,7 @@ namespace umbraco.BasePages var identity = HttpContext.Current.GetCurrentIdentity( //DO NOT AUTO-AUTH UNLESS THE CURRENT HANDLER IS WEBFORMS! // Without this check, anything that is using this legacy API, like ui.Text will - // automatically log the back office user in even if it is a front-end request (if there is + // automatically log the back office user in even if it is a front-end request (if there is // a back office user logged in. This can cause problems becaues the identity is changing mid // request. For example: http://issues.umbraco.org/issue/U4-4010 HttpContext.Current.CurrentHandler is Page); @@ -279,7 +279,7 @@ namespace umbraco.BasePages public static void RenewLoginTimeout() { - HttpContext.Current.RenewUmbracoAuthTicket(); + HttpContext.Current.RenewUmbracoAuthTicket(); } /// @@ -294,8 +294,8 @@ namespace umbraco.BasePages AllowedApplications = u.GetApplications().Select(x => x.alias).ToArray(), RealName = u.Name, Roles = u.GetGroups(), - StartContentNodes = u.UserEntity.AllStartContentIds, - StartMediaNodes = u.UserEntity.AllStartMediaIds, + StartContentNodes = u.UserEntity.GetAllContentStartNodes(ApplicationContext.Current.Services.EntityService), + StartMediaNodes = u.UserEntity.GetAllMediaStartNodes(ApplicationContext.Current.Services.EntityService), Username = u.LoginName, Culture = ui.Culture(u) @@ -342,7 +342,7 @@ namespace umbraco.BasePages } //[Obsolete("Use ClientTools instead")] - //public void reloadParentNode() + //public void reloadParentNode() //{ // ClientTools.ReloadParentNode(true); //} @@ -379,7 +379,7 @@ namespace umbraco.BasePages { base.OnInit(e); - //This must be set on each page to mitigate CSRF attacks which ensures that this unique token + //This must be set on each page to mitigate CSRF attacks which ensures that this unique token // is added to the viewstate of each request if (umbracoUserContextID.IsNullOrWhiteSpace() == false) { From f28136de9189bcc7ad499367abab96d3eba99666 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 18 Jul 2017 15:54:22 +0200 Subject: [PATCH 3/5] U4-10173 - fix --- src/Umbraco.Core/Models/UserExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 4f043c0ec7..e17757c2c0 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -185,14 +185,14 @@ namespace Umbraco.Core.Models { var gsn = user.Groups.Where(x => x.StartContentId.HasValue).Select(x => x.StartContentId.Value).Distinct().ToArray(); var usn = user.StartContentIds; - return CombineStartNodes(gsn, usn, entityService); + return CombineStartNodes(UmbracoObjectTypes.Document, gsn, usn, entityService); } public static int[] GetAllMediaStartNodes(this IUser user, IEntityService entityService) { var gsn = user.Groups.Where(x => x.StartMediaId.HasValue).Select(x => x.StartMediaId.Value).Distinct().ToArray(); var usn = user.StartMediaIds; - return CombineStartNodes(gsn, usn, entityService); + return CombineStartNodes(UmbracoObjectTypes.Media, gsn, usn, entityService); } private static bool StartsWithPath(string test, string path) @@ -200,12 +200,12 @@ namespace Umbraco.Core.Models return test.StartsWith(path) && test.Length > path.Length && test[path.Length] == ','; } - private static int[] CombineStartNodes(int[] groupSn, int[] userSn, IEntityService entityService) + private static int[] CombineStartNodes(UmbracoObjectTypes objectType, int[] groupSn, int[] userSn, IEntityService entityService) { // assume groupSn and userSn each don't contain duplicates var asn = groupSn.Concat(userSn).Distinct().ToArray(); - var paths = entityService.GetAll(UmbracoObjectTypes.Document, asn).ToDictionary(x => x.Id, x => x.Path); + var paths = entityService.GetAll(objectType, asn).ToDictionary(x => x.Id, x => x.Path); paths[-1] = "-1"; // entityService does not get that one From 38cd3dcf94c8e25c00cb9c2da2292e85780c1695 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 19 Jul 2017 16:15:16 +1000 Subject: [PATCH 4/5] Removes the AllStartNodeIds properties from IUser, updates all unit tests to work with the new methods, ensures the calculation is cached to the user object --- src/Umbraco.Core/Models/Membership/IUser.cs | 10 -- src/Umbraco.Core/Models/Membership/User.cs | 51 ++++---- src/Umbraco.Core/Models/UserExtensions.cs | 71 ++++++++--- .../Models/UserExtensionsTests.cs | 32 +++-- .../Controllers/ContentControllerUnitTests.cs | 110 +++++++++++++----- ...terAllowedOutgoingContentAttributeTests.cs | 49 ++++++-- .../Controllers/MediaControllerUnitTests.cs | 61 +++++++--- src/Umbraco.Web/Editors/ContentController.cs | 11 +- .../Editors/ContentPostValidateAttribute.cs | 13 ++- src/Umbraco.Web/Editors/EntityController.cs | 12 +- src/Umbraco.Web/Editors/MediaController.cs | 17 ++- .../Editors/MediaPostValidateAttribute.cs | 10 +- .../Models/Mapping/UserModelMapper.cs | 8 +- .../Trees/ContentTreeController.cs | 2 +- src/Umbraco.Web/Trees/MediaTreeController.cs | 2 +- ...EnsureUserPermissionForContentAttribute.cs | 4 +- .../EnsureUserPermissionForMediaAttribute.cs | 4 +- .../FilterAllowedOutgoingContentAttribute.cs | 48 ++++++-- .../FilterAllowedOutgoingMediaAttribute.cs | 2 +- .../BasePages/BasePage.cs | 4 +- 20 files changed, 364 insertions(+), 157 deletions(-) diff --git a/src/Umbraco.Core/Models/Membership/IUser.cs b/src/Umbraco.Core/Models/Membership/IUser.cs index 555537d851..b7afdeb624 100644 --- a/src/Umbraco.Core/Models/Membership/IUser.cs +++ b/src/Umbraco.Core/Models/Membership/IUser.cs @@ -46,15 +46,5 @@ namespace Umbraco.Core.Models.Membership /// Will hold the media file system relative path of the users custom avatar if they uploaded one /// string Avatar { get; set; } - - /// - /// Returns all start node Ids assigned to the user based on both the explicit start node ids assigned to the user and any start node Ids assigned to it's user groups - /// - int[] AllStartContentIds { get; } - - /// - /// Returns all start node Ids assigned to the user based on both the explicit start node ids assigned to the user and any start node Ids assigned to it's user groups - /// - int[] AllStartMediaIds { get; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 1e457eecf6..b64fd07452 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -115,10 +115,8 @@ namespace Umbraco.Core.Models.Membership private DateTime _lastPasswordChangedDate; private DateTime _lastLoginDate; private DateTime _lastLockoutDate; - private bool _defaultToLiveEditing; - private int[] _allStartContentIds; - private int[] _allStartMediaIds; + private IDictionary _additionalData; private static readonly Lazy Ps = new Lazy(); @@ -300,25 +298,7 @@ namespace Umbraco.Core.Models.Membership { get { return _avatar; } set { SetPropertyValueAndDetectChanges(value, ref _avatar, Ps.Value.AvatarSelector); } - } - - /// - /// Returns all start node Ids assigned to the user based on both the explicit start node ids assigned to the user and any start node Ids assigned to it's user groups - /// - [IgnoreDataMember] - public int[] AllStartContentIds - { - get { return this.GetAllContentStartNodes(ApplicationContext.Current.Services.EntityService); } - } - - /// - /// Returns all start node Ids assigned to the user based on both the explicit start node ids assigned to the user and any start node Ids assigned to it's user groups - /// - [IgnoreDataMember] - public int[] AllStartMediaIds - { - get { return this.GetAllMediaStartNodes(ApplicationContext.Current.Services.EntityService); } - } + } /// /// Gets or sets the session timeout. @@ -417,18 +397,37 @@ namespace Umbraco.Core.Models.Membership _allowedSections = null; OnPropertyChanged(Ps.Value.UserGroupsSelector); } - } - + } + #endregion + /// + /// This is used as an internal cache for this entity - specifically for calculating start nodes so we don't re-calculated all of the time + /// + [IgnoreDataMember] + [DoNotClone] + internal IDictionary AdditionalData + { + get { return _additionalData ?? (_additionalData = new Dictionary()); } + } + public override object DeepClone() { var clone = (User)base.DeepClone(); + //turn off change tracking + clone.DisableChangeTracking(); //manually clone the start node props clone._startContentIds = _startContentIds.ToArray(); clone._startMediaIds = _startMediaIds.ToArray(); - //turn off change tracking - clone.DisableChangeTracking(); + //This ensures that any value in the dictionary that is deep cloneable is cloned too + foreach (var key in clone.AdditionalData.Keys.ToArray()) + { + var deepCloneable = clone.AdditionalData[key] as IDeepCloneable; + if (deepCloneable != null) + { + clone.AdditionalData[key] = deepCloneable.DeepClone(); + } + } //need to create new collections otherwise they'll get copied by ref clone._userGroups = new HashSet(_userGroups); clone._allowedSections = _allowedSections != null ? new List(_allowedSections) : null; diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 4f043c0ec7..db57d03828 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -109,32 +109,32 @@ namespace Umbraco.Core.Models internal static bool HasContentRootAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.Root.ToInvariantString(), user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); + return HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasContentBinAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.RecycleBinContent.ToInvariantString(), user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); + return HasPathAccess(Constants.System.RecycleBinContent.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasMediaRootAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.Root.ToInvariantString(), user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + return HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool HasMediaBinAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.RecycleBinMedia.ToInvariantString(), user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + return HasPathAccess(Constants.System.RecycleBinMedia.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool HasPathAccess(this IUser user, IContent content, IEntityService entityService) { - return HasPathAccess(content.Path, user.GetAllContentStartNodes(entityService), Constants.System.RecycleBinContent); + return HasPathAccess(content.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasPathAccess(this IUser user, IMedia media, IEntityService entityService) { - return HasPathAccess(media.Path, user.GetAllMediaStartNodes(entityService), Constants.System.RecycleBinMedia); + return HasPathAccess(media.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool HasPathAccess(string path, int[] startNodeIds, int recycleBinId) @@ -181,37 +181,79 @@ namespace Umbraco.Core.Models return user.Groups != null && user.Groups.Any(x => x.Alias == Constants.Security.AdminGroupAlias); } - public static int[] GetAllContentStartNodes(this IUser user, IEntityService entityService) + public static int[] CalculateContentStartNodeIds(this IUser user, IEntityService entityService) { + const string cacheKey = "AllContentStartNodes"; + //try to look them up from cache so we don't recalculate + var valuesInUserCache = FromUserCache(user, cacheKey); + if (valuesInUserCache != null) return valuesInUserCache; + var gsn = user.Groups.Where(x => x.StartContentId.HasValue).Select(x => x.StartContentId.Value).Distinct().ToArray(); var usn = user.StartContentIds; - return CombineStartNodes(gsn, usn, entityService); + var vals = CombineStartNodes(gsn, usn, entityService); + ToUserCache(user, cacheKey, vals); + return vals; } - public static int[] GetAllMediaStartNodes(this IUser user, IEntityService entityService) + public static int[] CalculateMediaStartNodeIds(this IUser user, IEntityService entityService) { + const string cacheKey = "AllMediaStartNodes"; + //try to look them up from cache so we don't recalculate + var valuesInUserCache = FromUserCache(user, cacheKey); + if (valuesInUserCache != null) return valuesInUserCache; + var gsn = user.Groups.Where(x => x.StartMediaId.HasValue).Select(x => x.StartMediaId.Value).Distinct().ToArray(); var usn = user.StartMediaIds; - return CombineStartNodes(gsn, usn, entityService); + var vals = CombineStartNodes(gsn, usn, entityService); + ToUserCache(user, cacheKey, vals); + return vals; } + private static int[] FromUserCache(IUser user, string cacheKey) + { + var entityUser = user as User; + if (entityUser != null) + { + object allContentStartNodes; + if (entityUser.AdditionalData.TryGetValue(cacheKey, out allContentStartNodes)) + { + var asArray = allContentStartNodes as int[]; + if (asArray != null) return asArray; + } + } + return null; + } + + private static void ToUserCache(IUser user, string cacheKey, int[] vals) + { + var entityUser = user as User; + if (entityUser != null) + { + entityUser.AdditionalData[cacheKey] = vals; + } + } + private static bool StartsWithPath(string test, string path) { return test.StartsWith(path) && test.Length > path.Length && test[path.Length] == ','; } - - private static int[] CombineStartNodes(int[] groupSn, int[] userSn, IEntityService entityService) + + //TODO: Unit test this + internal static int[] CombineStartNodes(int[] groupSn, int[] userSn, IEntityService entityService) { // assume groupSn and userSn each don't contain duplicates - var asn = groupSn.Concat(userSn).Distinct().ToArray(); + var asn = groupSn.Concat(userSn).Distinct().ToArray(); + + //TODO: Change this to a more optimal lookup just to retrieve paths var paths = entityService.GetAll(UmbracoObjectTypes.Document, asn).ToDictionary(x => x.Id, x => x.Path); paths[-1] = "-1"; // entityService does not get that one var lsn = new List(); foreach (var sn in groupSn) - { + { + //TODO: Change this to TryGetValue var snp = paths[sn]; if (lsn.Any(x => StartsWithPath(snp, paths[x]))) continue; // skip if something above this sn lsn.RemoveAll(x => StartsWithPath(paths[x], snp)); // remove anything below this sn @@ -223,6 +265,7 @@ namespace Umbraco.Core.Models { if (groupSn.Contains(sn)) continue; + //TODO: Change this to TryGetValue var snp = paths[sn]; if (usn.Any(x => StartsWithPath(paths[x], snp))) continue; // skip if something below this sn usn.RemoveAll(x => StartsWithPath(snp, paths[x])); // remove anything above this sn diff --git a/src/Umbraco.Tests/Models/UserExtensionsTests.cs b/src/Umbraco.Tests/Models/UserExtensionsTests.cs index 4d3514bf00..a4284823f0 100644 --- a/src/Umbraco.Tests/Models/UserExtensionsTests.cs +++ b/src/Umbraco.Tests/Models/UserExtensionsTests.cs @@ -2,29 +2,37 @@ using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; namespace Umbraco.Tests.Models { [TestFixture] public class UserExtensionsTests { - [TestCase(2, "-1,1,2,3,4,5", true)] - [TestCase(6, "-1,1,2,3,4,5", false)] - [TestCase(-1, "-1,1,2,3,4,5", true)] - [TestCase(5, "-1,1,2,3,4,5", true)] - [TestCase(-1, "-1,-20,1,2,3,4,5", true)] - [TestCase(1, "-1,-20,1,2,3,4,5", false)] - public void Determines_Path_Based_Access_To_Content(int userId, string contentPath, bool outcome) + [TestCase(2, "-1,1,2", "-1,1,2,3,4,5", true)] + [TestCase(6, "-1,1,2,3,4,5,6", "-1,1,2,3,4,5", false)] + [TestCase(-1, "-1", "-1,1,2,3,4,5", true)] + [TestCase(5, "-1,1,2,3,4,5", "-1,1,2,3,4,5", true)] + [TestCase(-1, "-1", "-1,-20,1,2,3,4,5", true)] + [TestCase(1, "-1,-20,1", "-1,-20,1,2,3,4,5", false)] + public void Determines_Path_Based_Access_To_Content(int startNodeId, string startNodePath, string contentPath, bool outcome) { var userMock = new Mock(); - userMock.Setup(u => u.AllStartContentIds).Returns(new[]{ userId }); + userMock.Setup(u => u.StartContentIds).Returns(new[]{ startNodeId }); var user = userMock.Object; - var contentMock = new Mock(); - contentMock.Setup(c => c.Path).Returns(contentPath); - var content = contentMock.Object; + var content = Mock.Of(c => c.Path == contentPath && c.Id == 5); - Assert.AreEqual(outcome, user.HasPathAccess(content, ApplicationContext.Current.Services.EntityService)); + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] + { + Mock.Of(entity => entity.Id == startNodeId && entity.Path == startNodePath) + }); + var entityService = entityServiceMock.Object; + + Assert.AreEqual(outcome, user.HasPathAccess(content, entityService)); } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs index 23177cf419..e80e0743d5 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs @@ -3,6 +3,7 @@ using System.Web.Http; using Moq; using NUnit.Framework; using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Web.Editors; @@ -18,7 +19,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var contentMock = new Mock(); contentMock.Setup(c => c.Path).Returns("-1,1234,5678"); @@ -26,9 +26,13 @@ namespace Umbraco.Tests.Web.Controllers var contentServiceMock = new Mock(); contentServiceMock.Setup(x => x.GetById(1234)).Returns(content); var contentService = contentServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, null, contentService, 1234); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234); //assert Assert.IsTrue(result); @@ -40,7 +44,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var contentMock = new Mock(); contentMock.Setup(c => c.Path).Returns("-1,1234,5678"); @@ -53,9 +56,11 @@ namespace Umbraco.Tests.Web.Controllers var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; - + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act/assert - Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F' })); + Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F' })); } [Test] @@ -64,7 +69,7 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new[]{ 9876 }); + userMock.Setup(u => u.StartContentIds).Returns(new[]{ 9876 }); var user = userMock.Object; var contentMock = new Mock(); contentMock.Setup(c => c.Path).Returns("-1,1234,5678"); @@ -77,9 +82,13 @@ namespace Umbraco.Tests.Web.Controllers var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 9876 && entity.Path == "-1,9876") }); + var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F'}); //assert Assert.IsFalse(result); @@ -91,7 +100,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var contentMock = new Mock(); contentMock.Setup(c => c.Path).Returns("-1,1234,5678"); @@ -107,9 +115,11 @@ namespace Umbraco.Tests.Web.Controllers var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F'}); //assert Assert.IsFalse(result); @@ -121,7 +131,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var contentMock = new Mock(); contentMock.Setup(c => c.Path).Returns("-1,1234,5678"); @@ -129,17 +138,19 @@ namespace Umbraco.Tests.Web.Controllers var contentServiceMock = new Mock(); contentServiceMock.Setup(x => x.GetById(1234)).Returns(content); var contentService = contentServiceMock.Object; - var userServiceMock = new Mock(); var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A", "F", "C" }) }; var permissionSet = new EntityPermissionSet(1234, permissions); + var userServiceMock = new Mock(); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F'}); //assert Assert.IsTrue(result); @@ -151,11 +162,16 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; - + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act - var result = ContentController.CheckPermissions(new Dictionary(), user, null, null, -1); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1); //assert Assert.IsTrue(result); @@ -167,11 +183,16 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; - + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act - var result = ContentController.CheckPermissions(new Dictionary(), user, null, null, -20); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20); //assert Assert.IsTrue(result); @@ -183,11 +204,19 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new []{ 1234 }); + userMock.Setup(u => u.StartContentIds).Returns(new []{ 1234 }); var user = userMock.Object; + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); + var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, null, null, -20); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20); //assert Assert.IsFalse(result); @@ -199,11 +228,19 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new []{ 1234 }); + userMock.Setup(u => u.StartContentIds).Returns(new []{ 1234 }); var user = userMock.Object; + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); + var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, null, null, -1); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1); //assert Assert.IsFalse(result); @@ -215,7 +252,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var userServiceMock = new Mock(); @@ -225,10 +261,15 @@ namespace Umbraco.Tests.Web.Controllers }; var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1")).Returns(permissionSet); + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, new[] { 'A'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1, new[] { 'A'}); //assert Assert.IsTrue(result); @@ -240,7 +281,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var userServiceMock = new Mock(); @@ -251,9 +291,13 @@ namespace Umbraco.Tests.Web.Controllers var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, new[] { 'B'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1, new[] { 'B'}); //assert Assert.IsFalse(result); @@ -265,7 +309,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var userServiceMock = new Mock(); @@ -277,9 +320,13 @@ namespace Umbraco.Tests.Web.Controllers userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-20")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, new[] { 'A'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20, new[] { 'A'}); //assert Assert.IsTrue(result); @@ -291,7 +338,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartContentIds).Returns(new int[0]); var user = userMock.Object; var userServiceMock = new Mock(); @@ -302,9 +348,13 @@ namespace Umbraco.Tests.Web.Controllers var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-20")).Returns(permissionSet); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + var contentServiceMock = new Mock(); + var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, new[] { 'B'}); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20, new[] { 'B'}); //assert Assert.IsFalse(result); diff --git a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs index 5afce04a1a..3bc39ffb29 100644 --- a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs @@ -6,6 +6,8 @@ using System.Net.Http.Headers; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; @@ -19,7 +21,12 @@ namespace Umbraco.Tests.Web.Controllers [Test] public void GetValueFromResponse_Already_EnumerableContent() { - var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable)); + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + + var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), userService, entityService); var val = new List() {new ContentItemBasic()}; var result = att.GetValueFromResponse( new ObjectContent(typeof (IEnumerable), @@ -34,7 +41,12 @@ namespace Umbraco.Tests.Web.Controllers [Test] public void GetValueFromResponse_From_Property() { - var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), "MyList"); + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + + var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), "MyList", userService, entityService); var val = new List() { new ContentItemBasic() }; var container = new MyTestClass() {MyList = val}; @@ -51,7 +63,12 @@ namespace Umbraco.Tests.Web.Controllers [Test] public void GetValueFromResponse_Returns_Null_Not_Found_Property() { - var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), "DontFind"); + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + + var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), "DontFind", userService, entityService); var val = new List() { new ContentItemBasic() }; var container = new MyTestClass() { MyList = val }; @@ -68,7 +85,18 @@ namespace Umbraco.Tests.Web.Controllers [Test] public void Filter_On_Start_Node() { - var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable)); + var userMock = new Mock(); + userMock.Setup(u => u.Id).Returns(9); + userMock.Setup(u => u.StartContentIds).Returns(new[] { 5 }); + var user = userMock.Object; + var userServiceMock = new Mock(); + var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 5 && entity.Path == "-1,5") }); + var entityService = entityServiceMock.Object; + + var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), userService, entityService); var list = new List(); var path = ""; for (var i = 0; i < 10; i++) @@ -80,11 +108,6 @@ namespace Umbraco.Tests.Web.Controllers path += i.ToInvariantString(); list.Add(new ContentItemBasic { Id = i, Name = "Test" + i, ParentId = i, Path = path }); } - - var userMock = new Mock(); - userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartContentIds).Returns(new []{ 5 }); - var user = userMock.Object; att.FilterBasedOnStartNode(list, user); @@ -94,8 +117,7 @@ namespace Umbraco.Tests.Web.Controllers [Test] public void Filter_On_Permissions() - { - var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable)); + { var list = new List(); for (var i = 0; i < 10; i++) { @@ -119,8 +141,11 @@ namespace Umbraco.Tests.Web.Controllers }; userServiceMock.Setup(x => x.GetPermissions(user, ids)).Returns(permissions); var userService = userServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; - att.FilterBasedOnPermissions(list, user, userService); + var att = new FilterAllowedOutgoingContentAttribute(typeof(IEnumerable), userService, entityService); + att.FilterBasedOnPermissions(list, user); Assert.AreEqual(3, list.Count); Assert.AreEqual(1, list.ElementAt(0).Id); diff --git a/src/Umbraco.Tests/Web/Controllers/MediaControllerUnitTests.cs b/src/Umbraco.Tests/Web/Controllers/MediaControllerUnitTests.cs index 9cb0578fc9..553a37adc9 100644 --- a/src/Umbraco.Tests/Web/Controllers/MediaControllerUnitTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/MediaControllerUnitTests.cs @@ -3,6 +3,7 @@ using System.Web.Http; using Moq; using NUnit.Framework; using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Web.Editors; @@ -18,17 +19,18 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartMediaIds).Returns(new int[0]); var user = userMock.Object; var mediaMock = new Mock(); mediaMock.Setup(m => m.Path).Returns("-1,1234,5678"); var media = mediaMock.Object; - var mediaServiceMock = new Mock(); + var mediaServiceMock = new Mock(); mediaServiceMock.Setup(x => x.GetById(1234)).Returns(media); var mediaService = mediaServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, 1234); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234); //assert Assert.IsTrue(result); @@ -40,7 +42,6 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartMediaIds).Returns(new int[0]); var user = userMock.Object; var mediaMock = new Mock(); mediaMock.Setup(m => m.Path).Returns("-1,1234,5678"); @@ -48,9 +49,11 @@ namespace Umbraco.Tests.Web.Controllers var mediaServiceMock = new Mock(); mediaServiceMock.Setup(x => x.GetById(0)).Returns(media); var mediaService = mediaServiceMock.Object; - + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act/assert - Assert.Throws(() => MediaController.CheckPermissions(new Dictionary(), user, mediaService, 1234)); + Assert.Throws(() => MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234)); } [Test] @@ -59,7 +62,7 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(9); - userMock.Setup(u => u.AllStartMediaIds).Returns(new[]{ 9876 }); + userMock.Setup(u => u.StartMediaIds).Returns(new[]{ 9876 }); var user = userMock.Object; var mediaMock = new Mock(); mediaMock.Setup(m => m.Path).Returns("-1,1234,5678"); @@ -67,9 +70,13 @@ namespace Umbraco.Tests.Web.Controllers var mediaServiceMock = new Mock(); mediaServiceMock.Setup(x => x.GetById(1234)).Returns(media); var mediaService = mediaServiceMock.Object; - + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] {Mock.Of(entity => entity.Id == 9876 && entity.Path == "-1,9876")}); + var entityService = entityServiceMock.Object; + //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, 1234); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234); //assert Assert.IsFalse(result); @@ -81,11 +88,14 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartMediaIds).Returns(new int[]{}); var user = userMock.Object; - + var mediaServiceMock = new Mock(); + var mediaService = mediaServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; + //act - var result = MediaController.CheckPermissions(new Dictionary(), user, null, -1); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -1); //assert Assert.IsTrue(result); @@ -97,11 +107,17 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartMediaIds).Returns(new[]{ 1234 }); + userMock.Setup(u => u.StartMediaIds).Returns(new[]{ 1234 }); var user = userMock.Object; + var mediaServiceMock = new Mock(); + var mediaService = mediaServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); + var entityService = entityServiceMock.Object; //act - var result = MediaController.CheckPermissions(new Dictionary(), user, null, -1); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -1); //assert Assert.IsFalse(result); @@ -113,11 +129,14 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartMediaIds).Returns(new int[]{}); var user = userMock.Object; + var mediaServiceMock = new Mock(); + var mediaService = mediaServiceMock.Object; + var entityServiceMock = new Mock(); + var entityService = entityServiceMock.Object; //act - var result = MediaController.CheckPermissions(new Dictionary(), user, null, -21); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -21); //assert Assert.IsTrue(result); @@ -129,11 +148,17 @@ namespace Umbraco.Tests.Web.Controllers //arrange var userMock = new Mock(); userMock.Setup(u => u.Id).Returns(0); - userMock.Setup(u => u.AllStartMediaIds).Returns(new[]{ 1234 }); + userMock.Setup(u => u.StartMediaIds).Returns(new[]{ 1234 }); var user = userMock.Object; + var mediaServiceMock = new Mock(); + var mediaService = mediaServiceMock.Object; + var entityServiceMock = new Mock(); + entityServiceMock.Setup(x => x.GetAll(It.IsAny(), It.IsAny())) + .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); + var entityService = entityServiceMock.Object; //act - var result = MediaController.CheckPermissions(new Dictionary(), user, null, -21); + var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -21); //assert Assert.IsFalse(result); diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 8034485785..160ac7e5fd 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1023,7 +1023,6 @@ namespace Umbraco.Web.Editors } - /// /// Performs a permissions check for the user to check if it has access to the node based on /// start node and/or permissions for the node @@ -1032,6 +1031,7 @@ namespace Umbraco.Web.Editors /// /// /// + /// /// The content to lookup, if the contentItem is not specified /// /// Specifies the already resolved content item to check against @@ -1041,10 +1041,16 @@ namespace Umbraco.Web.Editors IUser user, IUserService userService, IContentService contentService, + IEntityService entityService, int nodeId, char[] permissionsToCheck = null, IContent contentItem = null) { + if (storage == null) throw new ArgumentNullException("storage"); + if (user == null) throw new ArgumentNullException("user"); + if (userService == null) throw new ArgumentNullException("userService"); + if (contentService == null) throw new ArgumentNullException("contentService"); + if (entityService == null) throw new ArgumentNullException("entityService"); if (contentItem == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinContent) { @@ -1058,8 +1064,7 @@ namespace Umbraco.Web.Editors { throw new HttpResponseException(HttpStatusCode.NotFound); } - - var entityService = ApplicationContext.Current.Services.EntityService; + var hasPathAccess = (nodeId == Constants.System.Root) ? user.HasContentRootAccess(entityService) : (nodeId == Constants.System.RecycleBinContent) diff --git a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs index 62898e7889..71715e339a 100644 --- a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs @@ -22,18 +22,21 @@ namespace Umbraco.Web.Editors private readonly IContentService _contentService; private readonly WebSecurity _security; private readonly IUserService _userService; + private readonly IEntityService _entityService; public ContentPostValidateAttribute() { } - public ContentPostValidateAttribute(IContentService contentService, IUserService userService, WebSecurity security) + public ContentPostValidateAttribute(IContentService contentService, IUserService userService, IEntityService entityService, WebSecurity security) { if (contentService == null) throw new ArgumentNullException("contentService"); if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); if (security == null) throw new ArgumentNullException("security"); _contentService = contentService; _userService = userService; + _entityService = entityService; _security = security; } @@ -52,6 +55,11 @@ namespace Umbraco.Web.Editors get { return _userService ?? ApplicationContext.Current.Services.UserService; } } + private IEntityService EntityService + { + get { return _entityService ?? ApplicationContext.Current.Services.EntityService; } + } + public override bool AllowMultiple { get { return true; } @@ -139,7 +147,8 @@ namespace Umbraco.Web.Editors actionContext.Request.Properties, Security.CurrentUser, UserService, - ContentService, + ContentService, + EntityService, contentIdToCheck, permissionToCheck.ToArray(), contentToCheck) == false) diff --git a/src/Umbraco.Web/Editors/EntityController.cs b/src/Umbraco.Web/Editors/EntityController.cs index 29e8bc9999..b22ce81784 100644 --- a/src/Umbraco.Web/Editors/EntityController.cs +++ b/src/Umbraco.Web/Editors/EntityController.cs @@ -551,10 +551,10 @@ namespace Umbraco.Web.Editors switch (type) { case UmbracoEntityTypes.Document: - aids = Security.CurrentUser.GetAllContentStartNodes(Services.EntityService); + aids = Security.CurrentUser.CalculateContentStartNodeIds(Services.EntityService); break; case UmbracoEntityTypes.Media: - aids = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService); + aids = Security.CurrentUser.CalculateMediaStartNodeIds(Services.EntityService); break; } @@ -695,14 +695,14 @@ namespace Umbraco.Web.Editors type = "media"; AddExamineSearchFrom(searchFrom, sb); - AddExamineUserStartNode(Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService), sb); + AddExamineUserStartNode(Security.CurrentUser.CalculateMediaStartNodeIds(Services.EntityService), sb); break; case UmbracoEntityTypes.Document: type = "content"; AddExamineSearchFrom(searchFrom, sb); - AddExamineUserStartNode(Security.CurrentUser.GetAllContentStartNodes(Services.EntityService), sb); + AddExamineUserStartNode(Security.CurrentUser.CalculateContentStartNodeIds(Services.EntityService), sb); break; default: @@ -950,10 +950,10 @@ namespace Umbraco.Web.Editors switch (entityType) { case UmbracoEntityTypes.Document: - aids = Security.CurrentUser.GetAllContentStartNodes(Services.EntityService); + aids = Security.CurrentUser.CalculateContentStartNodeIds(Services.EntityService); break; case UmbracoEntityTypes.Media: - aids = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService); + aids = Security.CurrentUser.CalculateMediaStartNodeIds(Services.EntityService); break; } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index 5905af9532..0b1a697eae 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -254,7 +254,7 @@ namespace Umbraco.Web.Editors private int[] _userStartNodes; protected int[] UserStartNodes { - get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.GetAllMediaStartNodes(Services.EntityService)); } + get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.CalculateMediaStartNodeIds(Services.EntityService)); } } /// @@ -680,7 +680,9 @@ namespace Umbraco.Web.Editors if (CheckPermissions( new Dictionary(), Security.CurrentUser, - Services.MediaService, parentId) == false) + Services.MediaService, + Services.EntityService, + parentId) == false) { return Request.CreateResponse( HttpStatusCode.Forbidden, @@ -892,11 +894,17 @@ namespace Umbraco.Web.Editors /// The storage to add the content item to so it can be reused /// /// + /// /// The content to lookup, if the contentItem is not specified /// Specifies the already resolved content item to check against, setting this ignores the nodeId /// - internal static bool CheckPermissions(IDictionary storage, IUser user, IMediaService mediaService, int nodeId, IMedia media = null) + internal static bool CheckPermissions(IDictionary storage, IUser user, IMediaService mediaService, IEntityService entityService, int nodeId, IMedia media = null) { + if (storage == null) throw new ArgumentNullException("storage"); + if (user == null) throw new ArgumentNullException("user"); + if (mediaService == null) throw new ArgumentNullException("mediaService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + if (media == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) { media = mediaService.GetById(nodeId); @@ -908,9 +916,8 @@ namespace Umbraco.Web.Editors if (media == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) { throw new HttpResponseException(HttpStatusCode.NotFound); - } + } - var entityService = Core.ApplicationContext.Current.Services.EntityService; var hasPathAccess = (nodeId == Constants.System.Root) ? user.HasMediaRootAccess(entityService) : (nodeId == Constants.System.RecycleBinMedia) diff --git a/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs b/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs index ed26fcbc57..1d38c4fce4 100644 --- a/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs @@ -19,17 +19,19 @@ namespace Umbraco.Web.Editors internal sealed class MediaPostValidateAttribute : ActionFilterAttribute { private readonly IMediaService _mediaService; + private readonly IEntityService _entityService; private readonly WebSecurity _security; public MediaPostValidateAttribute() { } - public MediaPostValidateAttribute(IMediaService mediaService, WebSecurity security) + public MediaPostValidateAttribute(IMediaService mediaService, IEntityService entityService, WebSecurity security) { if (mediaService == null) throw new ArgumentNullException("mediaService"); if (security == null) throw new ArgumentNullException("security"); _mediaService = mediaService; + _entityService = entityService; _security = security; } @@ -38,6 +40,11 @@ namespace Umbraco.Web.Editors get { return _mediaService ?? ApplicationContext.Current.Services.MediaService; } } + private IEntityService EntityService + { + get { return _entityService ?? ApplicationContext.Current.Services.EntityService; } + } + private WebSecurity Security { get { return _security ?? UmbracoContext.Current.Security; } @@ -81,6 +88,7 @@ namespace Umbraco.Web.Editors actionContext.Request.Properties, Security.CurrentUser, MediaService, + EntityService, contentIdToCheck, contentToCheck) == false) { diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index f76bdb57fa..30bf7df07e 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -309,8 +309,8 @@ namespace Umbraco.Web.Models.Mapping config.CreateMap() .ForMember(detail => detail.Avatars, opt => opt.MapFrom(user => user.GetCurrentUserAvatarUrls(applicationContext.Services.UserService, applicationContext.ApplicationCache.RuntimeCache))) .ForMember(detail => detail.UserId, opt => opt.MapFrom(user => GetIntId(user.Id))) - .ForMember(detail => detail.StartContentIds, opt => opt.MapFrom(user => user.AllStartContentIds)) - .ForMember(detail => detail.StartMediaIds, opt => opt.MapFrom(user => user.AllStartMediaIds)) + .ForMember(detail => detail.StartContentIds, opt => opt.MapFrom(user => user.CalculateContentStartNodeIds(applicationContext.Services.EntityService))) + .ForMember(detail => detail.StartMediaIds, opt => opt.MapFrom(user => user.CalculateMediaStartNodeIds(applicationContext.Services.EntityService))) .ForMember(detail => detail.Culture, opt => opt.MapFrom(user => user.GetUserCulture(applicationContext.Services.TextService))) .ForMember( detail => detail.EmailHash, @@ -326,8 +326,8 @@ namespace Umbraco.Web.Models.Mapping .ForMember(detail => detail.AllowedApplications, opt => opt.MapFrom(user => user.AllowedSections)) .ForMember(detail => detail.RealName, opt => opt.MapFrom(user => user.Name)) .ForMember(detail => detail.Roles, opt => opt.MapFrom(user => user.Groups.ToArray())) - .ForMember(detail => detail.StartContentNodes, opt => opt.MapFrom(user => user.AllStartContentIds)) - .ForMember(detail => detail.StartMediaNodes, opt => opt.MapFrom(user => user.AllStartMediaIds)) + .ForMember(detail => detail.StartContentNodes, opt => opt.MapFrom(user => user.CalculateContentStartNodeIds(applicationContext.Services.EntityService))) + .ForMember(detail => detail.StartMediaNodes, opt => opt.MapFrom(user => user.CalculateMediaStartNodeIds(applicationContext.Services.EntityService))) .ForMember(detail => detail.Username, opt => opt.MapFrom(user => user.Username)) .ForMember(detail => detail.Culture, opt => opt.MapFrom(user => user.GetUserCulture(applicationContext.Services.TextService))) .ForMember(detail => detail.SessionId, opt => opt.MapFrom(user => user.SecurityStamp.IsNullOrWhiteSpace() ? Guid.NewGuid().ToString("N") : user.SecurityStamp)); diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 5e982dfd9d..2dde698a28 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -65,7 +65,7 @@ namespace Umbraco.Web.Trees private int[] _userStartNodes; protected override int[] UserStartNodes { - get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.AllStartContentIds); } + get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.CalculateContentStartNodeIds(Services.EntityService)); } } /// diff --git a/src/Umbraco.Web/Trees/MediaTreeController.cs b/src/Umbraco.Web/Trees/MediaTreeController.cs index 2a25648f77..f3d583f1e3 100644 --- a/src/Umbraco.Web/Trees/MediaTreeController.cs +++ b/src/Umbraco.Web/Trees/MediaTreeController.cs @@ -56,7 +56,7 @@ namespace Umbraco.Web.Trees private int[] _userStartNodes; protected override int[] UserStartNodes { - get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.AllStartMediaIds); } + get { return _userStartNodes ?? (_userStartNodes = Security.CurrentUser.CalculateMediaStartNodeIds(Services.EntityService)); } } /// diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs index 9cebc4b95a..129bac1c4d 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs @@ -102,7 +102,9 @@ namespace Umbraco.Web.WebApi.Filters actionContext.Request.Properties, UmbracoContext.Current.Security.CurrentUser, ApplicationContext.Current.Services.UserService, - ApplicationContext.Current.Services.ContentService, nodeId, _permissionToCheck.HasValue ? new[]{_permissionToCheck.Value}: null)) + ApplicationContext.Current.Services.ContentService, + ApplicationContext.Current.Services.EntityService, + nodeId, _permissionToCheck.HasValue ? new[]{_permissionToCheck.Value}: null)) { base.OnActionExecuting(actionContext); } diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs index e126d42c21..1289f25063 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs @@ -123,7 +123,9 @@ namespace Umbraco.Web.WebApi.Filters if (MediaController.CheckPermissions( actionContext.Request.Properties, UmbracoContext.Current.Security.CurrentUser, - ApplicationContext.Current.Services.MediaService, nodeId)) + ApplicationContext.Current.Services.MediaService, + ApplicationContext.Current.Services.EntityService, + nodeId)) { base.OnActionExecuting(actionContext); } diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs index 6091392321..331839780a 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs @@ -18,36 +18,70 @@ namespace Umbraco.Web.WebApi.Filters /// internal sealed class FilterAllowedOutgoingContentAttribute : FilterAllowedOutgoingMediaAttribute { + private readonly IUserService _userService; + private readonly IEntityService _entityService; private readonly char _permissionToCheck; public FilterAllowedOutgoingContentAttribute(Type outgoingType) - : base(outgoingType) + : this(outgoingType, ApplicationContext.Current.Services.UserService, ApplicationContext.Current.Services.EntityService) { _permissionToCheck = ActionBrowse.Instance.Letter; } public FilterAllowedOutgoingContentAttribute(Type outgoingType, char permissionToCheck) - : base(outgoingType) + : this(outgoingType, ApplicationContext.Current.Services.UserService, ApplicationContext.Current.Services.EntityService) { _permissionToCheck = permissionToCheck; } public FilterAllowedOutgoingContentAttribute(Type outgoingType, string propertyName) - : base(outgoingType, propertyName) + : this(outgoingType, propertyName, ApplicationContext.Current.Services.UserService, ApplicationContext.Current.Services.EntityService) { _permissionToCheck = ActionBrowse.Instance.Letter; } + + public FilterAllowedOutgoingContentAttribute(Type outgoingType, IUserService userService, IEntityService entityService) + : base(outgoingType) + { + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + _userService = userService; + _entityService = entityService; + _permissionToCheck = ActionBrowse.Instance.Letter; + } + + public FilterAllowedOutgoingContentAttribute(Type outgoingType, char permissionToCheck, IUserService userService, IEntityService entityService) + : base(outgoingType) + { + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + _userService = userService; + _entityService = entityService; + _permissionToCheck = permissionToCheck; + } + + public FilterAllowedOutgoingContentAttribute(Type outgoingType, string propertyName, IUserService userService, IEntityService entityService) + : base(outgoingType, propertyName) + { + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + _userService = userService; + _entityService = entityService; + _permissionToCheck = ActionBrowse.Instance.Letter; + } + + protected override void FilterItems(IUser user, IList items) { base.FilterItems(user, items); - FilterBasedOnPermissions(items, user, ApplicationContext.Current.Services.UserService); + FilterBasedOnPermissions(items, user); } protected override int[] GetUserStartNodes(IUser user) { - return user.AllStartContentIds; + return user.CalculateContentStartNodeIds(_entityService); } protected override int RecycleBinId @@ -55,7 +89,7 @@ namespace Umbraco.Web.WebApi.Filters get { return Constants.System.RecycleBinContent; } } - internal void FilterBasedOnPermissions(IList items, IUser user, IUserService userService) + internal void FilterBasedOnPermissions(IList items, IUser user) { var length = items.Count; @@ -67,7 +101,7 @@ namespace Umbraco.Web.WebApi.Filters ids.Add(((dynamic)items[i]).Id); } //get all the permissions for these nodes in one call - var permissions = userService.GetPermissions(user, ids.ToArray()).ToArray(); + var permissions = _userService.GetPermissions(user, ids.ToArray()).ToArray(); var toRemove = new List(); foreach (dynamic item in items) { diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs index 1ceeb133f3..beb67f3395 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs @@ -41,7 +41,7 @@ namespace Umbraco.Web.WebApi.Filters protected virtual int[] GetUserStartNodes(IUser user) { - return user.AllStartMediaIds; + return user.CalculateMediaStartNodeIds(ApplicationContext.Current.Services.EntityService); } protected virtual int RecycleBinId diff --git a/src/umbraco.businesslogic/BasePages/BasePage.cs b/src/umbraco.businesslogic/BasePages/BasePage.cs index fcfd9ecf44..916f1e8d9f 100644 --- a/src/umbraco.businesslogic/BasePages/BasePage.cs +++ b/src/umbraco.businesslogic/BasePages/BasePage.cs @@ -294,8 +294,8 @@ namespace umbraco.BasePages AllowedApplications = u.GetApplications().Select(x => x.alias).ToArray(), RealName = u.Name, Roles = u.GetGroups(), - StartContentNodes = u.UserEntity.GetAllContentStartNodes(ApplicationContext.Current.Services.EntityService), - StartMediaNodes = u.UserEntity.GetAllMediaStartNodes(ApplicationContext.Current.Services.EntityService), + StartContentNodes = u.UserEntity.CalculateContentStartNodeIds(ApplicationContext.Current.Services.EntityService), + StartMediaNodes = u.UserEntity.CalculateMediaStartNodeIds(ApplicationContext.Current.Services.EntityService), Username = u.LoginName, Culture = ui.Culture(u) From 2851216b887724e35179a20b42268bdf3d80eea4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 19 Jul 2017 10:49:10 +0200 Subject: [PATCH 5/5] U4-10173 - fix --- src/SolutionInfo.cs | 2 +- .../Configuration/UmbracoVersion.cs | 2 +- src/Umbraco.Core/Models/UserExtensions.cs | 20 ++++++++++--------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/SolutionInfo.cs b/src/SolutionInfo.cs index 9f58f72a6d..590e92b53d 100644 --- a/src/SolutionInfo.cs +++ b/src/SolutionInfo.cs @@ -12,4 +12,4 @@ using System.Resources; [assembly: AssemblyVersion("1.0.*")] [assembly: AssemblyFileVersion("7.7.0")] -[assembly: AssemblyInformationalVersion("7.7.0-alpha001")] \ No newline at end of file +[assembly: AssemblyInformationalVersion("7.7.0-alpha002")] \ No newline at end of file diff --git a/src/Umbraco.Core/Configuration/UmbracoVersion.cs b/src/Umbraco.Core/Configuration/UmbracoVersion.cs index 802c83c818..2c5ea65487 100644 --- a/src/Umbraco.Core/Configuration/UmbracoVersion.cs +++ b/src/Umbraco.Core/Configuration/UmbracoVersion.cs @@ -24,7 +24,7 @@ namespace Umbraco.Core.Configuration /// Gets the version comment (like beta or RC). /// /// The version comment. - public static string CurrentComment { get { return "alpha001"; } } + public static string CurrentComment { get { return "alpha002"; } } // Get the version of the umbraco.dll by looking at a class in that dll // Had to do it like this due to medium trust issues, see: http://haacked.com/archive/2010/11/04/assembly-location-and-medium-trust.aspx diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 76255a73b6..73545a41ee 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -81,7 +81,7 @@ namespace Umbraco.Core.Models /// /// /// - /// + /// public static CultureInfo GetUserCulture(this IUser user, ILocalizedTextService textService) { if (user == null) throw new ArgumentNullException("user"); @@ -94,7 +94,7 @@ namespace Umbraco.Core.Models try { var culture = CultureInfo.GetCultureInfo(userLanguage.Replace("_", "-")); - //TODO: This is a hack because we store the user language as 2 chars instead of the full culture + //TODO: This is a hack because we store the user language as 2 chars instead of the full culture // which is actually stored in the language files (which are also named with 2 chars!) so we need to attempt // to convert to a supported full culture var result = textService.ConvertToSupportedCultureWithRegionCode(culture); @@ -153,11 +153,11 @@ namespace Umbraco.Core.Models if (formattedPath.Contains(formattedRecycleBinId)) { return false; - } + } //check for normal paths foreach (var startNodeId in startNodeIds) - { + { var formattedStartNodeId = "," + startNodeId.ToInvariantString() + ","; var hasAccess = formattedPath.Contains(formattedStartNodeId); @@ -253,8 +253,9 @@ namespace Umbraco.Core.Models var lsn = new List(); foreach (var sn in groupSn) { - //TODO: Change this to TryGetValue - var snp = paths[sn]; + string snp; + if (paths.TryGetValue(sn, out snp) == false) continue; // ignore + if (lsn.Any(x => StartsWithPath(snp, paths[x]))) continue; // skip if something above this sn lsn.RemoveAll(x => StartsWithPath(paths[x], snp)); // remove anything below this sn lsn.Add(sn); @@ -265,8 +266,9 @@ namespace Umbraco.Core.Models { if (groupSn.Contains(sn)) continue; - //TODO: Change this to TryGetValue - var snp = paths[sn]; + string snp; + if (paths.TryGetValue(sn, out snp) == false) continue; // ignore + if (usn.Any(x => StartsWithPath(paths[x], snp))) continue; // skip if something below this sn usn.RemoveAll(x => StartsWithPath(snp, paths[x])); // remove anything above this sn usn.Add(sn); @@ -274,7 +276,7 @@ namespace Umbraco.Core.Models foreach (var sn in usn) { - var snp = paths[sn]; + var snp = paths[sn]; // has to be here now lsn.RemoveAll(x => StartsWithPath(snp, paths[x]) || StartsWithPath(paths[x], snp)); // remove anything above or below this sn lsn.Add(sn); }