From 1ead3f9b6aae6b5ef55b4bd0f749ce4f2aa00e99 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2013 16:51:08 +1100 Subject: [PATCH] Updated the content display model to pass in a list of the allowed actions (using the letter of the permission). We then dynamically generate the action buttons on the content editor based on what the user is actually allowed to do and the current state of the content. Fixed up some content saving validation for Create + Publish at the same time, since we never allowed that behavior before we now have to check for both permissions during this one execution. Updated the Unpublish method to check for publish permissions - unpublish is an action but it is not permission assignable, you can only unpublish if you can publish. Fixes the user session timeout display timer. --- .../ContentControllerUnitTests.cs | 16 +-- .../src/common/directives/hotkey.directive.js | 8 +- .../views/common/dialogs/user.controller.js | 35 ++++-- .../src/views/common/dialogs/user.html | 2 +- .../views/content/content.edit.controller.js | 113 ++++++++++++++++-- .../src/views/content/edit.html | 34 +++--- src/Umbraco.Web/Editors/ContentController.cs | 29 +++-- .../Editors/ContentPostValidateAttribute.cs | 32 +++-- .../ContentEditing/ContentItemDisplay.cs | 11 +- .../Models/Mapping/ContentModelMapper.cs | 59 +++++++++ ...EnsureUserPermissionForContentAttribute.cs | 2 +- src/umbraco.cms/Actions/ActionNewFolder.cs | 1 + 12 files changed, 276 insertions(+), 66 deletions(-) diff --git a/src/Umbraco.Tests/Controllers/WebApiEditors/ContentControllerUnitTests.cs b/src/Umbraco.Tests/Controllers/WebApiEditors/ContentControllerUnitTests.cs index 45dd854f74..3420a17dc2 100644 --- a/src/Umbraco.Tests/Controllers/WebApiEditors/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests/Controllers/WebApiEditors/ContentControllerUnitTests.cs @@ -54,7 +54,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act/assert - Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F')); + Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F' })); } [Test] @@ -77,7 +77,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); //assert Assert.IsFalse(result); @@ -106,7 +106,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); //assert Assert.IsFalse(result); @@ -135,7 +135,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, new[] { 'F'}); //assert Assert.IsTrue(result); @@ -223,7 +223,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, 'A'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, new[] { 'A'}); //assert Assert.IsTrue(result); @@ -247,7 +247,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, 'B'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -1, new[] { 'B'}); //assert Assert.IsFalse(result); @@ -271,7 +271,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, 'A'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, new[] { 'A'}); //assert Assert.IsTrue(result); @@ -295,7 +295,7 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, 'B'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, null, -20, new[] { 'B'}); //assert Assert.IsFalse(result); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/hotkey.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/hotkey.directive.js index 2f66acc36c..f5277e98e0 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/hotkey.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/hotkey.directive.js @@ -6,7 +6,13 @@ angular.module("umbraco.directives") .directive('hotkey', function ($window, keyboardService, $log) { return function (scope, el, attrs) { - var keyCombo = attrs["hotkey"]; + + //support data binding + + var keyCombo = scope.$eval(attrs["hotkey"]); + if (!keyCombo) { + keyCombo = attrs["hotkey"]; + } keyboardService.bind(keyCombo, function() { var element = $(el); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.controller.js index 877ad7d351..829af7e28f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.controller.js @@ -5,7 +5,8 @@ angular.module("umbraco") $scope.history = historyService.current; $scope.logout = function () { - userService.logout().then(function() { + userService.logout().then(function () { + $scope.remainingAuthSeconds = 0; $scope.hide(); }); }; @@ -16,14 +17,28 @@ angular.module("umbraco") }; //Manually update the remaining timeout seconds - function updateTimeout() { - $timeout(function () { - $scope.user = userService.getCurrentUser(); - //manually execute the digest against this scope only - $scope.$digest(); - updateTimeout(); //keep going (recurse) - }, 1000, false); // 1 second, do NOT execute a global digest - } - updateTimeout(); + function updateTimeout() { + $timeout(function () { + if ($scope.remainingAuthSeconds > 0) { + $scope.remainingAuthSeconds--; + $scope.$digest(); + //recurse + updateTimeout(); + } + + }, 1000, false); // 1 second, do NOT execute a global digest + } + + //get the user + userService.getCurrentUser().then(function (user) { + $scope.user = user; + if ($scope.user) { + $scope.remainingAuthSeconds = $scope.user.remainingAuthSeconds; + //set the timer + updateTimeout(); + } + }); + + }); \ No newline at end of file diff --git a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.html b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.html index 29d70d58d9..ed112429f7 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/user.html @@ -12,7 +12,7 @@

- : {{user.remainingAuthSeconds | timespan}} + : {{remainingAuthSeconds | timespan}}

diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js index 7fd478cdc9..e2bcb705c4 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js @@ -7,17 +7,94 @@ * The controller for the content editor */ function ContentEditController($scope, $routeParams, $q, $timeout, $window, contentResource, navigationService, localizationService, notificationsService, angularHelper, serverValidationManager, contentEditingHelper, fileManager, formHelper) { - - contentResource.checkPermission("P", $routeParams.id).then(function(hasPermission){ - var key = "buttons_sendToPublish" - if(hasPermission){ - key = "buttons_saveAndPublish"; + + $scope.defaultButton = null; + $scope.subButtons = []; + + //This sets up the action buttons based on what permissions the user has. + //The allowedActions parameter contains a list of chars, each represents a button by permission so + //here we'll build the buttons according to the chars of the user. + function configureButtons(content) { + //reset + $scope.subButtons = []; + + //This is the ideal button order but depends on circumstance, we'll use this array to create the button list + // Publish, SendToPublish, Save + var buttonOrder = ["U", "H", "A"]; + + //Create the first button (primary button) + //We cannot have the Save or SaveAndPublish buttons if they don't have create permissions when we are creating a new item. + if (!$routeParams.create || _.contains(content.allowedActions, "C")) { + for (var b in buttonOrder) { + if (_.contains(content.allowedActions, buttonOrder[b])) { + $scope.defaultButton = createButtonDefinition(buttonOrder[b]); + break; + } + } } - localizationService.localize(key).then(function(label){ - $scope.publishButtonLabel = label; - }); - }); + //Now we need to make the drop down button list, this is also slightly tricky because: + //We cannot have any buttons if there's no default button above. + //We cannot have the unpublish button (Z) when there's no publish permission. + //We cannot have the unpublish button (Z) when the item is not published. + if ($scope.defaultButton) { + + //get the last index of the button order + var lastIndex = _.indexOf(buttonOrder, $scope.defaultButton.letter); + //add the remaining + for (var i = lastIndex + 1; i < buttonOrder.length; i++) { + if (_.contains(content.allowedActions, buttonOrder[i])) { + $scope.subButtons.push(createButtonDefinition(buttonOrder[i])); + } + } + + //if we are not creating, then we should add unpublish too, + // so long as it's already published and if the user has access to publish + if (!$routeParams.create) { + if (content.publishDate && _.contains(content.allowedActions,"U")) { + $scope.subButtons.push(createButtonDefinition("Z")); + } + } + } + } + + function createButtonDefinition(ch) { + switch (ch) { + case "U": + //publish action + return { + letter: ch, + labelKey: "buttons_saveAndPublish", + handler: $scope.saveAndPublish, + hotKey: "ctrl+p" + }; + case "H": + //send to publish + return { + letter: ch, + labelKey: "buttons_saveToPublish", + handler: $scope.saveAndPublish, + hotKey: "ctrl+t" + }; + case "A": + //save + return { + letter: ch, + labelKey: "buttons_save", + handler: $scope.save, + hotKey: "ctrl+s" + }; + case "Z": + //unpublish + return { + letter: ch, + labelKey: "content_unPublish", + handler: $scope.unPublish + }; + default: + return null; + } + } if ($routeParams.create) { @@ -25,7 +102,8 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont contentResource.getScaffold($routeParams.id, $routeParams.doctype) .then(function(data) { $scope.loaded = true; - $scope.content = data; + $scope.content = data; + configureButtons($scope.content); }); } else { @@ -34,6 +112,7 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont .then(function(data) { $scope.loaded = true; $scope.content = data; + configureButtons($scope.content); //just get the cached version, no need to force a reload navigationService.syncPath(data.path.split(","), false); @@ -64,6 +143,8 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont rebindCallback: contentEditingHelper.reBindChangedProperties($scope.content, data) }); + configureButtons(data); + navigationService.syncPath(data.path.split(","), true); }); } @@ -85,6 +166,8 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont rebindCallback: contentEditingHelper.reBindChangedProperties($scope.content, data) }); + configureButtons(data); + navigationService.syncPath(data.path.split(","), true); }, function(err) { @@ -126,6 +209,8 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont rebindCallback: contentEditingHelper.reBindChangedProperties($scope.content, data) }); + configureButtons(data); + //fetch tree navigationService.syncPath(data.path.split(","), true); @@ -147,6 +232,14 @@ function ContentEditController($scope, $routeParams, $q, $timeout, $window, cont return deferred.promise; }; + /** this method is called for all action buttons and then we proxy based on the btn definition */ + $scope.performAction = function(btn) { + if (!btn || !angular.isFunction(btn.handler)) { + throw "btn.handler must be a function reference"; + } + btn.handler.apply(this); + }; + } angular.module("umbraco").controller("Umbraco.Editors.Content.EditController", ContentEditController); diff --git a/src/Umbraco.Web.UI.Client/src/views/content/edit.html b/src/Umbraco.Web.UI.Client/src/views/content/edit.html index 2e5da2f109..a5945bcadf 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/edit.html +++ b/src/Umbraco.Web.UI.Client/src/views/content/edit.html @@ -25,26 +25,26 @@ -
- - {{publishButtonLabel}} - - +
+ + + + {{defaultButton.labelKey}} + + + + + + -
diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index c95cd1efd1..9a1380b14a 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -480,7 +480,10 @@ namespace Umbraco.Web.Editors /// /// /// - [EnsureUserPermissionForContent("id", 'Z')] + //TODO: Unpublish is NOT an assignable permission therefore this won't work, I'd assume to unpublish you'd need to be able to publish??! + // still waiting on feedback from HQ. + //[EnsureUserPermissionForContent("id", 'Z')] + [EnsureUserPermissionForContent("id", 'U')] public ContentItemDisplay PostUnPublish(int id) { var foundContent = GetObjectFromRequest(() => Services.ContentService.GetById(id)); @@ -490,6 +493,9 @@ namespace Umbraco.Web.Editors Services.ContentService.UnPublish(foundContent); var content = Mapper.Map(foundContent); + + content.AddSuccessNotification(ui.Text("content", "unPublish"), ui.Text("speechBubbles", "contentUnpublished")); + return content; } @@ -599,7 +605,7 @@ namespace Umbraco.Web.Editors /// /// /// The content to lookup, if the contentItem is not specified - /// + /// /// Specifies the already resolved content item to check against /// internal static bool CheckPermissions( @@ -608,7 +614,7 @@ namespace Umbraco.Web.Editors IUserService userService, IContentService contentService, int nodeId, - char? permissionToCheck = null, + char[] permissionsToCheck = null, IContent contentItem = null) { @@ -642,19 +648,22 @@ namespace Umbraco.Web.Editors return false; } - if (permissionToCheck.HasValue == false) + if (permissionsToCheck == null || permissionsToCheck.Any() == false) { return true; } var permission = userService.GetPermissions(user, nodeId).FirstOrDefault(); - - if (permission != null && permission.AssignedPermissions.Contains(permissionToCheck.Value.ToString(CultureInfo.InvariantCulture))) - { - return true; - } - return false; + var allowed = true; + foreach (var p in permissionsToCheck) + { + if (permission == null || permission.AssignedPermissions.Contains(p.ToString(CultureInfo.InvariantCulture)) == false) + { + allowed = false; + } + } + return allowed; } } diff --git a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs index 79a6dd3836..24b684d9f7 100644 --- a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Net; using System.Net.Http; using System.Web.Http.Controllers; @@ -58,25 +59,24 @@ namespace Umbraco.Web.Editors //We now need to validate that the user is allowed to be doing what they are doing. //Based on the action we need to check different permissions. //Then if it is new, we need to lookup those permissions on the parent! - char permissionToCheck; + + var permissionToCheck = new List(); IContent contentToCheck = null; int contentIdToCheck; switch (contentItem.Action) { case ContentSaveAction.Save: - permissionToCheck = ActionUpdate.Instance.Letter; + permissionToCheck.Add(ActionUpdate.Instance.Letter); contentToCheck = contentItem.PersistedContent; contentIdToCheck = contentToCheck.Id; break; case ContentSaveAction.Publish: - permissionToCheck = ActionPublish.Instance.Letter; + permissionToCheck.Add(ActionPublish.Instance.Letter); contentToCheck = contentItem.PersistedContent; contentIdToCheck = contentToCheck.Id; break; - case ContentSaveAction.PublishNew: case ContentSaveAction.SaveNew: - default: - permissionToCheck = ActionNew.Instance.Letter; + permissionToCheck.Add(ActionNew.Instance.Letter); if (contentItem.ParentId != Constants.System.Root) { contentToCheck = ContentService.GetById(contentItem.ParentId); @@ -87,6 +87,24 @@ namespace Umbraco.Web.Editors contentIdToCheck = contentItem.ParentId; } break; + case ContentSaveAction.PublishNew: + //Publish new requires both ActionNew AND ActionPublish + + permissionToCheck.Add(ActionNew.Instance.Letter); + permissionToCheck.Add(ActionPublish.Instance.Letter); + + if (contentItem.ParentId != Constants.System.Root) + { + contentToCheck = ContentService.GetById(contentItem.ParentId); + contentIdToCheck = contentToCheck.Id; + } + else + { + contentIdToCheck = contentItem.ParentId; + } + break; + default: + throw new ArgumentOutOfRangeException(); } if (ContentController.CheckPermissions( @@ -95,7 +113,7 @@ namespace Umbraco.Web.Editors UserService, ContentService, contentIdToCheck, - permissionToCheck, + permissionToCheck.ToArray(), contentToCheck) == false) { actionContext.Response = actionContext.Request.CreateUserNoAccessResponse(); diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs index 974b5f325e..6994fe7312 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Runtime.Serialization; using System.Web.Http; @@ -28,6 +29,14 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "urls")] public string[] Urls { get; set; } - + + /// + /// The allowed 'actions' based on the user's permissions - Create, Update, Publish, Send to publish + /// + /// + /// Each char represents a button which we can then map on the front-end to the correct actions + /// + [DataMember(Name = "allowedActions")] + public IEnumerable AllowedActions { get; set; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs index ed0936dcdf..bd47ff635b 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Linq.Expressions; using System.Runtime.Serialization; @@ -13,6 +14,7 @@ using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using umbraco; using Umbraco.Web.Routing; +using umbraco.BusinessLogic.Actions; namespace Umbraco.Web.Models.Mapping { @@ -54,6 +56,8 @@ namespace Umbraco.Web.Models.Mapping : content.GetContentUrls())) .ForMember(display => display.Properties, expression => expression.Ignore()) .ForMember(display => display.Tabs, expression => expression.ResolveUsing()) + .ForMember(display => display.AllowedActions, expression => expression.ResolveUsing( + new ActionButtonsResolver(new Lazy(() => applicationContext.Services.UserService)))) .AfterMap(MapGenericCustomProperties); //FROM IContent TO ContentItemBasic @@ -157,5 +161,60 @@ namespace Umbraco.Web.Models.Mapping return null; } + /// + /// Creates the list of action buttons allowed for this user - Publish, Send to publish, save, unpublish returned as the button's 'letter' + /// + private class ActionButtonsResolver : ValueResolver> + { + private readonly Lazy _userService; + + public ActionButtonsResolver(Lazy userService) + { + _userService = userService; + } + + protected override IEnumerable ResolveCore(IContent source) + { + if (UmbracoContext.Current == null) + { + //cannot check permissions without a context + return Enumerable.Empty(); + } + var svc = _userService.Value; + + var permissions = svc.GetPermissions(UmbracoContext.Current.Security.CurrentUser, source.Id) + .FirstOrDefault(); + if (permissions == null) + { + return Enumerable.Empty(); + } + + var result = new List(); + + //can they publish ? + if (permissions.AssignedPermissions.Contains(ActionPublish.Instance.Letter.ToString(CultureInfo.InvariantCulture))) + { + result.Add(ActionPublish.Instance.Letter); + } + //can they send to publish ? + if (permissions.AssignedPermissions.Contains(ActionToPublish.Instance.Letter.ToString(CultureInfo.InvariantCulture))) + { + result.Add(ActionToPublish.Instance.Letter); + } + //can they save ? + if (permissions.AssignedPermissions.Contains(ActionUpdate.Instance.Letter.ToString(CultureInfo.InvariantCulture))) + { + result.Add(ActionUpdate.Instance.Letter); + } + //can they create ? + if (permissions.AssignedPermissions.Contains(ActionNew.Instance.Letter.ToString(CultureInfo.InvariantCulture))) + { + result.Add(ActionNew.Instance.Letter); + } + + return result; + } + } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs index 3fe408fa12..48900f28f9 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs @@ -100,7 +100,7 @@ namespace Umbraco.Web.WebApi.Filters actionContext.Request.Properties, UmbracoContext.Current.Security.CurrentUser, ApplicationContext.Current.Services.UserService, - ApplicationContext.Current.Services.ContentService, nodeId, _permissionToCheck)) + ApplicationContext.Current.Services.ContentService, nodeId, _permissionToCheck.HasValue ? new[]{_permissionToCheck.Value}: null)) { base.OnActionExecuting(actionContext); } diff --git a/src/umbraco.cms/Actions/ActionNewFolder.cs b/src/umbraco.cms/Actions/ActionNewFolder.cs index 341ef07488..a812842890 100644 --- a/src/umbraco.cms/Actions/ActionNewFolder.cs +++ b/src/umbraco.cms/Actions/ActionNewFolder.cs @@ -7,6 +7,7 @@ namespace umbraco.BusinessLogic.Actions /// /// This action is invoked upon creation of a document /// + [Obsolete("This class is no longer used and will be removed in future versions")] public class ActionNewFolder : IAction { //create singleton