Roles Updating and History

src/OrchardCore.Modules/OrchardCore.Recipes/Manifest.cs

@@ -21,8 +21,8 @@
 
 [assembly: Feature(
     Id = "OrchardCore.Recipes.Core",
-    Name = "Recipes",
-    Description = "Provides recipe services.",
+    Name = "Recipes Core Services",
+    Description = "Provides recipe core services.",
     Category = "Infrastructure",
     EnabledByDependencyOnly = true
 )]

src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs

@@ -10,10 +10,8 @@
 using Microsoft.Extensions.Localization;
 using OrchardCore.Data.Documents;
 using OrchardCore.DisplayManagement.Notify;
-using OrchardCore.Documents;
 using OrchardCore.Environment.Extensions;
 using OrchardCore.Environment.Extensions.Features;
-using OrchardCore.Roles.Models;
 using OrchardCore.Roles.ViewModels;
 using OrchardCore.Security;
 using OrchardCore.Security.Permissions;
@@ -24,39 +22,35 @@ namespace OrchardCore.Roles.Controllers
     public class AdminController : Controller
     {
         private readonly IDocumentStore _documentStore;
-        private readonly IAuthorizationService _authorizationService;
-        private readonly IStringLocalizer S;
         private readonly RoleManager<IRole> _roleManager;
-        private readonly IDocumentManager<RolesDocument> _rolesDocumentManager;
+        private readonly IAuthorizationService _authorizationService;
         private readonly IEnumerable<IPermissionProvider> _permissionProviders;
         private readonly ITypeFeatureProvider _typeFeatureProvider;
         private readonly IRoleService _roleService;
         private readonly INotifier _notifier;
+        private readonly IStringLocalizer S;
         private readonly IHtmlLocalizer H;
 
         public AdminController(
-            IAuthorizationService authorizationService,
-            ITypeFeatureProvider typeFeatureProvider,
             IDocumentStore documentStore,
-            IStringLocalizer<AdminController> stringLocalizer,
-            IHtmlLocalizer<AdminController> htmlLocalizer,
             RoleManager<IRole> roleManager,
-            IDocumentManager<RolesDocument> rolesDocumentManager,
+            IAuthorizationService authorizationService,
+            IEnumerable<IPermissionProvider> permissionProviders,
+            ITypeFeatureProvider typeFeatureProvider,
             IRoleService roleService,
             INotifier notifier,
-            IEnumerable<IPermissionProvider> permissionProviders
-            )
+            IStringLocalizer<AdminController> stringLocalizer,
+            IHtmlLocalizer<AdminController> htmlLocalizer)
         {
-            H = htmlLocalizer;
-            _notifier = notifier;
-            _roleService = roleService;
-            _typeFeatureProvider = typeFeatureProvider;
-            _permissionProviders = permissionProviders;
+            _documentStore = documentStore;
             _roleManager = roleManager;
-            _rolesDocumentManager = rolesDocumentManager;
-            S = stringLocalizer;
             _authorizationService = authorizationService;
-            _documentStore = documentStore;
+            _permissionProviders = permissionProviders;
+            _typeFeatureProvider = typeFeatureProvider;
+            _roleService = roleService;
+            _notifier = notifier;
+            S = stringLocalizer;
+            H = htmlLocalizer;
         }
 
         public async Task<ActionResult> Index()
@@ -211,42 +205,13 @@ public async Task<IActionResult> EditPost(string id, string roleDescription)
 
             role.RoleDescription = roleDescription;
 
-            var rolesDocument = await _rolesDocumentManager.GetOrCreateMutableAsync();
-            var updateRolesDocument = false;
-
-            rolesDocument.PermissionGroups.TryAdd(role.RoleName, new List<string>());
-
-            var permissionNames = _permissionProviders.SelectMany(x => x.GetDefaultStereotypes())
-                .SelectMany(y => y.Permissions ?? Enumerable.Empty<Permission>())
-                .Select(x => x.Name)
-                .ToList();
-
-            // Save
+            // Save.
             var rolePermissions = new List<RoleClaim>();
             foreach (var key in Request.Form.Keys)
             {
-                var permissionName = key;
-
-                if (key.StartsWith("Checkbox.", StringComparison.Ordinal))
-                {
-                    permissionName = key.Substring("Checkbox.".Length);
-                }
-
-                if (!permissionNames.Contains(permissionName, StringComparer.OrdinalIgnoreCase))
-                {
-                    // The request contains an invalid permission, let's ignore it
-                    continue;
-                }
-
-                if (!rolesDocument.PermissionGroups[role.RoleName].Contains(permissionName, StringComparer.OrdinalIgnoreCase))
-                {
-                    // Save the permission in the permissions history so it never gets auto assigned to this role.
-                    rolesDocument.PermissionGroups[role.RoleName].Add(permissionName);
-                    updateRolesDocument = true;
-                }
-
-                if (String.Equals(Request.Form[key], "true", StringComparison.OrdinalIgnoreCase))
+                if (key.StartsWith("Checkbox.", StringComparison.Ordinal) && Request.Form[key] == "true")
                 {
+                    var permissionName = key["Checkbox.".Length..];
                     rolePermissions.Add(new RoleClaim { ClaimType = Permission.ClaimType, ClaimValue = permissionName });
                 }
             }
@@ -256,11 +221,6 @@ public async Task<IActionResult> EditPost(string id, string roleDescription)
 
             await _roleManager.UpdateAsync(role);
 
-            if (updateRolesDocument)
-            {
-                await _rolesDocumentManager.UpdateAsync(rolesDocument);
-            }
-
             await _notifier.SuccessAsync(H["Role updated successfully."]);
 
             return RedirectToAction(nameof(Index));

src/OrchardCore.Modules/OrchardCore.Roles/Manifest.cs

@@ -5,6 +5,21 @@
     Author = ManifestConstants.OrchardCoreTeam,
     Website = ManifestConstants.OrchardCoreWebsite,
     Version = ManifestConstants.OrchardCoreVersion,
-    Description = "The roles module adds the permissions to assign roles to users. It's also provides a set of default roles for which other modules can define default permissions.",
+    Category = "Security"
+)]
+
+[assembly: Feature(
+    Id = "OrchardCore.Roles",
+    Name = "Roles",
+    Description = "The roles module adds the permissions to assign roles to users. It also updates existing default roles with the default permissions provided by features.",
+    Dependencies = new[] { "OrchardCore.Roles.Core" },
+    Category = "Security"
+)]
+
+[assembly: Feature(
+    Id = "OrchardCore.Roles.Core",
+    Name = "Roles Core Services",
+    Description = "Provides role core services.",
+    EnabledByDependencyOnly = true,
     Category = "Security"
 )]

src/OrchardCore.Modules/OrchardCore.Roles/Models/RolesDocument.cs

@@ -1,19 +1,11 @@
-using System;
 using System.Collections.Generic;
 using OrchardCore.Data.Documents;
 using OrchardCore.Security;
-using OrchardCore.Security.Permissions;
 
 namespace OrchardCore.Roles.Models
 {
     public class RolesDocument : Document
     {
         public List<Role> Roles { get; set; } = new List<Role>();
-
-
-        /// <summary>
-        /// Keeps track of all permission that were automaticly assigned to a role using <see cref="IPermissionProvider"></see>/>
-        /// </summary>
-        public Dictionary<string, List<string>> PermissionGroups { get; set; } = new(StringComparer.OrdinalIgnoreCase);
     }
 }

src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs

@@ -67,10 +67,17 @@ public async Task<IdentityResult> CreateAsync(IRole role, CancellationToken canc
                 throw new ArgumentNullException(nameof(role));
             }
 
+            var roleToCreate = (Role)role;
+
             var roles = await LoadRolesAsync();
-            roles.Roles.Add((Role)role);
+            roles.Roles.Add(roleToCreate);
             await UpdateRolesAsync(roles);
 
+            var roleCreatedEventHandlers = _serviceProvider.GetRequiredService<IEnumerable<IRoleCreatedEventHandler>>();
+
+            await roleCreatedEventHandlers.InvokeAsync((handler, roleToCreate) =>
+                handler.RoleCreatedAsync(roleToCreate.RoleName), roleToCreate, _logger);
+
             return IdentityResult.Success;
         }
 
@@ -90,7 +97,9 @@ public async Task<IdentityResult> DeleteAsync(IRole role, CancellationToken canc
             }
 
             var roleRemovedEventHandlers = _serviceProvider.GetRequiredService<IEnumerable<IRoleRemovedEventHandler>>();
-            await roleRemovedEventHandlers.InvokeAsync((handler, roleToRemove) => handler.RoleRemovedAsync(roleToRemove.RoleName), roleToRemove, _logger);
+
+            await roleRemovedEventHandlers.InvokeAsync((handler, roleToRemove) =>
+                handler.RoleRemovedAsync(roleToRemove.RoleName), roleToRemove, _logger);
 
             var roles = await LoadRolesAsync();
             roleToRemove = roles.Roles.FirstOrDefault(r => r.RoleName == roleToRemove.RoleName);
@@ -252,8 +261,10 @@ public async Task<IdentityResult> UpdateAsync(IRole role, CancellationToken canc
 
         #endregion IRoleClaimStore<IRole>
 
+#pragma warning disable CA1816
         public void Dispose()
         {
         }
+#pragma warning restore CA1816
     }
 }