From 868edee5e647f6210e0a2507e615a93a2671f251 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 9 Jul 2013 11:47:46 +1000 Subject: [PATCH] Fixes: #U4-2161 - permissions inheritance. Fixes up how user permissions cache is invalidated and looked up (it is now lazy and not forced). User permissions are also cached as a low priority for now and only for 20 mins based on #U4-2474. --- src/Umbraco.Core/Cache/CacheKeys.cs | 1 + src/Umbraco.Core/Models/Content.cs | 19 +++++- .../Repositories/ContentRepository.cs | 13 +++- .../Cache/CacheRefresherEventHandler.cs | 49 ++++++++++++-- src/Umbraco.Web/Cache/DistributedCache.cs | 1 + .../Cache/DistributedCacheExtensions.cs | 17 +++++ src/Umbraco.Web/Cache/UserCacheRefresher.cs | 2 + .../Cache/UserPermissionsCacheRefresher.cs | 47 ++++++++++++++ .../Publishing/UpdateCacheAfterPublish.cs | 2 + .../Publishing/UpdateCacheAfterUnPublish.cs | 2 + src/Umbraco.Web/Umbraco.Web.csproj | 1 + .../umbraco/dialogs/cruds.aspx.cs | 14 ---- src/umbraco.businesslogic/User.cs | 64 ++++++++++--------- 13 files changed, 179 insertions(+), 53 deletions(-) create mode 100644 src/Umbraco.Web/Cache/UserPermissionsCacheRefresher.cs diff --git a/src/Umbraco.Core/Cache/CacheKeys.cs b/src/Umbraco.Core/Cache/CacheKeys.cs index 1c89d7662f..54f9ab4832 100644 --- a/src/Umbraco.Core/Cache/CacheKeys.cs +++ b/src/Umbraco.Core/Cache/CacheKeys.cs @@ -31,6 +31,7 @@ namespace Umbraco.Core.Cache public const string UserContextCacheKey = "UmbracoUserContext"; public const string UserContextTimeoutCacheKey = "UmbracoUserContextTimeout"; public const string UserCacheKey = "UmbracoUser"; + public const string UserPermissionsCacheKey = "UmbracoUserPermissions"; public const string ContentTypeCacheKey = "UmbracoContentType"; diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 41ceaf9b90..0e5c3340fa 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -22,7 +22,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 /// @@ -82,6 +82,7 @@ namespace Umbraco.Core.Models private static readonly PropertyInfo ExpireDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ExpireDate); private static readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); private static readonly PropertyInfo NodeNameSelector = ExpressionHelper.GetPropertyInfo(x => x.NodeName); + private static readonly PropertyInfo PermissionsChangedSelector = ExpressionHelper.GetPropertyInfo(x => x.PermissionsChanged); /// /// Gets or sets the template used by the Content. @@ -243,6 +244,22 @@ namespace Umbraco.Core.Models } } + /// + /// Used internally to track if permissions have been changed during the saving process for this entity + /// + internal bool PermissionsChanged + { + get { return _permissionsChanged; } + set + { + SetPropertyValueAndDetectChanges(o => + { + _permissionsChanged = value; + return _permissionsChanged; + }, _permissionsChanged, PermissionsChangedSelector); + } + } + /// /// Gets the ContentType used by this content object /// diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 93179f7e91..755fc5f04a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -225,9 +225,14 @@ namespace Umbraco.Core.Persistence.Repositories // user's default permissions. if (parentPermissions.Any()) { - var userPermissions = parentPermissions.ToDictionary( - permissionDto => (object) permissionDto.UserId, permissionDto => permissionDto.Permission); + var userPermissions = parentPermissions.Select( + permissionDto => new KeyValuePair( + permissionDto.UserId, + permissionDto.Permission)); AssignEntityPermissions(entity, userPermissions); + //flag the entity's permissions changed flag so we can track those changes. + //Currently only used for the cache refreshers to detect if we should refresh all user permissions cache. + ((Content) entity).PermissionsChanged = true; } //Create the Content specific data - cmsContent @@ -296,6 +301,10 @@ namespace Umbraco.Core.Persistence.Repositories "SELECT coalesce(max(sortOrder),0) FROM umbracoNode WHERE parentid = @ParentId AND nodeObjectType = @NodeObjectType", new {ParentId = entity.ParentId, NodeObjectType = NodeObjectTypeId}); entity.SortOrder = maxSortOrder + 1; + + //Question: If we move a node, should we update permissions to inherit from the new parent if the parent has permissions assigned? + // if we do that, then we'd need to propogate permissions all the way downward which might not be ideal for many people. + // Gonna just leave it as is for now, and not re-propogate permissions. } var factory = new ContentFactory(NodeObjectTypeId, entity.Id); diff --git a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs index 435fb9c7ef..adec7b6f85 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs @@ -10,6 +10,7 @@ using umbraco.cms.businesslogic; using umbraco.cms.businesslogic.member; using System.Linq; using umbraco.cms.businesslogic.web; +using Content = Umbraco.Core.Models.Content; using Macro = umbraco.cms.businesslogic.macro.Macro; using Template = umbraco.cms.businesslogic.template.Template; @@ -123,8 +124,46 @@ namespace Umbraco.Web.Cache MediaService.Deleting += MediaServiceDeleting; MediaService.Moving += MediaServiceMoving; MediaService.Trashing += MediaServiceTrashing; + + ContentService.Created += ContentServiceCreated; + ContentService.Copied += ContentServiceCopied; } + #region Content service event handlers + + /// + /// When an entity is copied new permissions may be assigned to it based on it's parent, if that is the + /// case then we need to clear all user permissions cache. + /// + /// + /// + static void ContentServiceCopied(IContentService sender, Core.Events.CopyEventArgs e) + { + //check if permissions have changed + var permissionsChanged = ((Content)e.Copy).WasPropertyDirty("PermissionsChanged"); + if (permissionsChanged) + { + DistributedCache.Instance.RefreshAllUserPermissionsCache(); + } + } + + /// + /// When an entity is created new permissions may be assigned to it based on it's parent, if that is the + /// case then we need to clear all user permissions cache. + /// + /// + /// + static void ContentServiceCreated(IContentService sender, Core.Events.NewEventArgs e) + { + //check if permissions have changed + var permissionsChanged = ((Content)e.Entity).WasPropertyDirty("PermissionsChanged"); + if (permissionsChanged) + { + DistributedCache.Instance.RefreshAllUserPermissionsCache(); + } + } + #endregion + #region ApplicationTree event handlers static void ApplicationTreeNew(ApplicationTree sender, System.EventArgs e) { @@ -396,15 +435,15 @@ namespace Umbraco.Web.Cache { if (sender.User != null) { - DistributedCache.Instance.RefreshUserCache(sender.User.Id); + DistributedCache.Instance.RefreshUserPermissionsCache(sender.User.Id); } - if (sender.UserId > -1) + else if (sender.UserId > -1) { - DistributedCache.Instance.RefreshUserCache(sender.UserId); + DistributedCache.Instance.RefreshUserPermissionsCache(sender.UserId); } - if (sender.NodeIds.Any()) + else if (sender.NodeIds.Any()) { - DistributedCache.Instance.RefreshAllUserCache(); + DistributedCache.Instance.RefreshAllUserPermissionsCache(); } } diff --git a/src/Umbraco.Web/Cache/DistributedCache.cs b/src/Umbraco.Web/Cache/DistributedCache.cs index a0ce50d4c7..1f7307b289 100644 --- a/src/Umbraco.Web/Cache/DistributedCache.cs +++ b/src/Umbraco.Web/Cache/DistributedCache.cs @@ -43,6 +43,7 @@ namespace Umbraco.Web.Cache public const string MediaCacheRefresherId = "B29286DD-2D40-4DDB-B325-681226589FEC"; public const string MacroCacheRefresherId = "7B1E683C-5F34-43dd-803D-9699EA1E98CA"; public const string UserCacheRefresherId = "E057AF6D-2EE6-41F4-8045-3694010F0AA6"; + public const string UserPermissionsCacheRefresherId = "840AB9C5-5C0B-48DB-A77E-29FE4B80CD3A"; public const string UserTypeCacheRefresherId = "7E707E21-0195-4522-9A3C-658CC1761BD4"; public const string ContentTypeCacheRefresherId = "6902E22C-9C10-483C-91F3-66B7CAE9E2F5"; public const string LanguageCacheRefresherId = "3E0F95D8-0BE5-44B8-8394-2B8750B62654"; diff --git a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs index 5e46927d57..274705439e 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs @@ -61,6 +61,23 @@ namespace Umbraco.Web.Cache } #endregion + #region User permissions cache + public static void RemoveUserPermissionsCache(this DistributedCache dc, int userId) + { + dc.Remove(new Guid(DistributedCache.UserPermissionsCacheRefresherId), userId); + } + + public static void RefreshUserPermissionsCache(this DistributedCache dc, int userId) + { + dc.Refresh(new Guid(DistributedCache.UserPermissionsCacheRefresherId), userId); + } + + public static void RefreshAllUserPermissionsCache(this DistributedCache dc) + { + dc.RefreshAll(new Guid(DistributedCache.UserPermissionsCacheRefresherId)); + } + #endregion + #region Template cache /// /// Refreshes the cache amongst servers for a template diff --git a/src/Umbraco.Web/Cache/UserCacheRefresher.cs b/src/Umbraco.Web/Cache/UserCacheRefresher.cs index 0e4e32bc48..c80e8ed9b7 100644 --- a/src/Umbraco.Web/Cache/UserCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UserCacheRefresher.cs @@ -28,6 +28,7 @@ namespace Umbraco.Web.Cache public override void RefreshAll() { ApplicationContext.Current.ApplicationCache.ClearCacheByKeySearch(CacheKeys.UserCacheKey); + ApplicationContext.Current.ApplicationCache.ClearCacheByKeySearch(CacheKeys.UserPermissionsCacheKey); ApplicationContext.Current.ApplicationCache.ClearCacheByKeySearch(CacheKeys.UserContextCacheKey); } @@ -39,6 +40,7 @@ namespace Umbraco.Web.Cache public override void Remove(int id) { ApplicationContext.Current.ApplicationCache.ClearCacheItem(string.Format("{0}{1}", CacheKeys.UserCacheKey, id)); + ApplicationContext.Current.ApplicationCache.ClearCacheItem(string.Format("{0}{1}", CacheKeys.UserPermissionsCacheKey, id)); //we need to clear all UserContextCacheKey since we cannot invalidate based on ID since the cache is done so based //on the current contextId stored in the database diff --git a/src/Umbraco.Web/Cache/UserPermissionsCacheRefresher.cs b/src/Umbraco.Web/Cache/UserPermissionsCacheRefresher.cs new file mode 100644 index 0000000000..fdafdedb09 --- /dev/null +++ b/src/Umbraco.Web/Cache/UserPermissionsCacheRefresher.cs @@ -0,0 +1,47 @@ +using System; +using Umbraco.Core; +using Umbraco.Core.Cache; + +namespace Umbraco.Web.Cache +{ + /// + /// Used only to invalidate the user permissions cache + /// + /// + /// The UserCacheRefresher will also clear a user's permissions cache, this refresher is for invalidating only permissions + /// for users/content, not the users themselves. + /// + public sealed class UserPermissionsCacheRefresher : CacheRefresherBase + { + protected override UserPermissionsCacheRefresher Instance + { + get { return this; } + } + + public override Guid UniqueIdentifier + { + get { return Guid.Parse(DistributedCache.UserPermissionsCacheRefresherId); } + } + + + public override string Name + { + get { return "User permissions cache refresher"; } + } + + public override void RefreshAll() + { + ApplicationContext.Current.ApplicationCache.ClearCacheByKeySearch(CacheKeys.UserPermissionsCacheKey); + } + + public override void Refresh(int id) + { + Remove(id); + } + + public override void Remove(int id) + { + ApplicationContext.Current.ApplicationCache.ClearCacheItem(string.Format("{0}{1}", CacheKeys.UserPermissionsCacheKey, id)); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterPublish.cs b/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterPublish.cs index f76198d2c3..1371c6fdb9 100644 --- a/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterPublish.cs +++ b/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterPublish.cs @@ -8,6 +8,8 @@ using Umbraco.Web.Cache; namespace Umbraco.Web.Strategies.Publishing { + //TODO: I think we should move this logic into the CacheRefresherEventHandler since all other handlers are registered there for invalidating cache. + /// /// Represents the UpdateCacheAfterPublish class, which subscribes to the Published event /// of the class and is responsible for doing the actual diff --git a/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterUnPublish.cs b/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterUnPublish.cs index 7af7724c7e..39ca0beda3 100644 --- a/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterUnPublish.cs +++ b/src/Umbraco.Web/Strategies/Publishing/UpdateCacheAfterUnPublish.cs @@ -10,6 +10,8 @@ using Umbraco.Web.Cache; namespace Umbraco.Web.Strategies.Publishing { + //TODO: I think we should move this logic into the CacheRefresherEventHandler since all other handlers are registered there for invalidating cache. + /// /// Represents the UpdateCacheAfterUnPublish class, which subscribes to the UnPublished event /// of the class and is responsible for doing the actual diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index d99c76a624..7d0761cf1e 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -286,6 +286,7 @@ + diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs index cf570d93c1..7a96b7ae3e 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs @@ -35,13 +35,8 @@ namespace umbraco.dialogs // Put user code to initialize the page here } - #region Web Form Designer generated code override protected void OnInit(EventArgs e) { - // - // CODEGEN: This call is required by the ASP.NET Web Form Designer. - // - InitializeComponent(); base.OnInit(e); node = new cms.businesslogic.CMSNode(int.Parse(helper.Request("id"))); @@ -99,15 +94,6 @@ namespace umbraco.dialogs PlaceHolder1.Controls.Add(ht); } - /// - /// Required method for Designer support - do not modify - /// the contents of this method with the code editor. - /// - private void InitializeComponent() - { - - } - #endregion protected void Button1_Click(object sender, System.EventArgs e) { diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index 3ee8608da0..5da83d2a6d 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Web.Caching; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Logging; @@ -28,9 +29,6 @@ namespace umbraco.BusinessLogic private bool _userDisabled; private bool _defaultToLiveEditing; - private Hashtable _cruds = new Hashtable(); - private bool _crudsInitialized = false; - private Hashtable _notifications = new Hashtable(); private bool _notificationsInitialized = false; @@ -670,19 +668,42 @@ namespace umbraco.BusinessLogic setupUser(_id); string defaultPermissions = UserType.DefaultPermissions; - if (!_crudsInitialized) - initCruds(); + //get the cached permissions for the user + var cachedPermissions = ApplicationContext.Current.ApplicationCache.GetCacheItem( + string.Format("{0}{1}", CacheKeys.UserPermissionsCacheKey, _id), + //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average + CacheItemPriority.BelowNormal, + null, + //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, + // then it will refresh from the database. + new TimeSpan(0, 20, 0), + () => + { + var cruds = new Hashtable(); + using (var dr = SqlHelper.ExecuteReader("select * from umbracoUser2NodePermission where userId = @userId order by nodeId", SqlHelper.CreateParameter("@userId", this.Id))) + { + while (dr.Read()) + { + if (!cruds.ContainsKey(dr.GetInt("nodeId"))) + { + cruds.Add(dr.GetInt("nodeId"), string.Empty); + } + cruds[dr.GetInt("nodeId")] += dr.GetString("permission"); + } + } + return cruds; + }); // NH 4.7.1 changing default permission behavior to default to User Type permissions IF no specific permissions has been // set for the current node - int nodeId = Path.Contains(",") ? int.Parse(Path.Substring(Path.LastIndexOf(",")+1)) : int.Parse(Path); - if (_cruds.ContainsKey(nodeId)) + var nodeId = Path.Contains(",") ? int.Parse(Path.Substring(Path.LastIndexOf(",", StringComparison.Ordinal)+1)) : int.Parse(Path); + if (cachedPermissions.ContainsKey(nodeId)) { - return _cruds[int.Parse(Path.Substring(Path.LastIndexOf(",")+1))].ToString(); + return cachedPermissions[int.Parse(Path.Substring(Path.LastIndexOf(",", StringComparison.Ordinal) + 1))].ToString(); } // exception to everything. If default cruds is empty and we're on root node; allow browse of root node - if (String.IsNullOrEmpty(defaultPermissions) && Path == "-1") + if (string.IsNullOrEmpty(defaultPermissions) && Path == "-1") defaultPermissions = "F"; // else return default user type cruds @@ -691,29 +712,10 @@ namespace umbraco.BusinessLogic /// /// Initializes the user node permissions - /// + /// + [Obsolete("This method doesn't do anything whatsoever and will be removed in future versions")] public void initCruds() - { - if (!_isInitialized) - setupUser(_id); - - // clear cruds - System.Web.HttpContext.Current.Application.Lock(); - _cruds.Clear(); - System.Web.HttpContext.Current.Application.UnLock(); - - using (IRecordsReader dr = SqlHelper.ExecuteReader("select * from umbracoUser2NodePermission where userId = @userId order by nodeId", SqlHelper.CreateParameter("@userId", this.Id))) - { - // int currentId = -1; - while (dr.Read()) - { - if (!_cruds.ContainsKey(dr.GetInt("nodeId"))) - _cruds.Add(dr.GetInt("nodeId"), String.Empty); - - _cruds[dr.GetInt("nodeId")] += dr.GetString("permission"); - } - } - _crudsInitialized = true; + { } ///