From c14840a06624434af68e33c3c15209d6a2cce57b Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 3 Oct 2024 12:17:14 -0700 Subject: [PATCH] Deprecate `SiteOwner` permission and retain Administrator as system role (#16781) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Zoltán Lehóczky Co-authored-by: Sébastien Ros Co-authored-by: Georg von Kries --- src/OrchardCore.Cms.Web/appsettings.json | 6 +- .../Settings/UserPickerFieldSettingsDriver.cs | 25 +-- .../OrchardCore.Recipes/AdminMenu.cs | 3 +- .../Controllers/AdminController.cs | 5 +- .../RecipesPermissionProvider.cs | 23 +++ .../OrchardCore.Recipes/Startup.cs | 3 +- .../Controllers/AdminController.cs | 148 +++++++++-------- .../Deployment/AllRolesDeploymentSource.cs | 10 +- .../Migrations/RolesMigrations.cs | 127 +++++++++++++++ .../OrchardCore.Roles.csproj | 2 +- .../OrchardCore.Roles/Permissions.cs | 1 - .../OrchardCore.Roles/Recipes/RolesStep.cs | 34 ++-- .../Services/RoleClaimsProvider.cs | 75 +++++++++ .../OrchardCore.Roles/Services/RoleStore.cs | 71 +++++++-- .../OrchardCore.Roles/Services/RoleUpdater.cs | 50 +++--- .../OrchardCore.Roles/Startup.cs | 32 +++- .../ViewModels/EditRoleViewModel.cs | 4 + .../ViewModels/RolesViewModel.cs | 7 + .../Views/Admin/Create.cshtml | 8 +- .../OrchardCore.Roles/Views/Admin/Edit.cshtml | 149 +++++++++--------- .../Views/Admin/Index.cshtml | 37 +++-- .../Controllers/AdminController.cs | 3 +- .../Drivers/RoleLoginSettingsDisplayDriver.cs | 18 +-- .../Drivers/UserMenuDisplayDriver.cs | 14 +- .../Drivers/UserRoleDisplayDriver.cs | 12 +- .../Services/RoleAuthorizationHandler.cs | 5 +- .../Services/RolesAdminListFilterProvider.cs | 2 +- .../OrchardCore.Users/Startup.cs | 1 + .../OrchardCore.Users/UserRolePermissions.cs | 17 +- .../TheAdmin/Recipes/blank.recipe.json | 14 +- .../TheAdmin/Recipes/headless.recipe.json | 14 +- .../TheAgencyTheme/Recipes/agency.recipe.json | 14 +- .../TheBlogTheme/Recipes/blog.recipe.json | 14 +- .../Recipes/comingsoon.recipe.json | 14 +- .../TheTheme/Recipes/saas.recipe.json | 6 +- .../Security/Role.cs | 5 +- .../Security/RoleClaim.cs | 21 +++ .../Security/Services/IRoleService.cs | 8 + .../Security/Services/RoleHelper.cs | 1 + .../Services/RoleServiceExtensions.cs | 21 ++- .../Security/StandardPermissions.cs | 1 + .../DefaultPermissionGrantingService.cs | 3 - .../RecipePermissions.cs | 8 + .../ISystemRoleNameProvider.cs | 10 ++ .../OrchardCore.Roles.Core.csproj | 12 +- .../RolesPermissionHandler.cs | 44 ++++++ .../ServiceCollectionExtensions.cs | 11 ++ .../Services/DefaultSystemRoleNameProvider.cs | 44 ++++++ .../SystemRoleNameProviderExtensions.cs | 22 +++ .../SystemRoleOptions.cs | 5 + .../OrchardCore.Users.Core.csproj | 1 + ...faultUserClaimsPrincipalProviderFactory.cs | 7 +- .../Services/RoleService.cs | 12 +- src/docs/reference/modules/Roles/README.md | 27 ++-- src/docs/releases/2.1.0.md | 35 ++++ .../cms-tests/Recipes/migrations.recipe.json | 6 +- .../Recipes/pages.recipe.json | 14 +- .../Apis/Context/AuthenticationContext.cs | 3 - ...sts.cs => PermissionHandlerHelperTests.cs} | 16 +- ...UserClaimsPrincipalProviderFactoryTests.cs | 5 +- .../DefaultSystemRoleNameProviderTests.cs | 82 ++++++++++ .../Security/PermissionHandlerHelper.cs | 11 +- .../Security/PermissionHandlerTests.cs | 4 +- .../Security/RolesPermissionHandlerTests.cs | 60 +++++++ 64 files changed, 1116 insertions(+), 351 deletions(-) create mode 100644 src/OrchardCore.Modules/OrchardCore.Recipes/RecipesPermissionProvider.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs create mode 100644 src/OrchardCore/OrchardCore.Recipes.Core/RecipePermissions.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/ISystemRoleNameProvider.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/RolesPermissionHandler.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/SystemRoleOptions.cs rename test/OrchardCore.Tests/Modules/OrchardCore.Roles/{RolesPermissionsHandlerTests.cs => PermissionHandlerHelperTests.cs} (85%) create mode 100644 test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs create mode 100644 test/OrchardCore.Tests/Security/RolesPermissionHandlerTests.cs diff --git a/src/OrchardCore.Cms.Web/appsettings.json b/src/OrchardCore.Cms.Web/appsettings.json index 00b3945e8f0..4dbff971208 100644 --- a/src/OrchardCore.Cms.Web/appsettings.json +++ b/src/OrchardCore.Cms.Web/appsettings.json @@ -173,7 +173,11 @@ // "Key": "" // } //}, - // "Url": "http://localhost", + // Roles + //"OrchardCore_Roles": { + // "AdminRoleName":"Administrator" + //}, + //"Url": "http://localhost", // "Ports": [ 9200 ], // "Username": "admin", // "Password": "admin", diff --git a/src/OrchardCore.Modules/OrchardCore.ContentFields/Settings/UserPickerFieldSettingsDriver.cs b/src/OrchardCore.Modules/OrchardCore.ContentFields/Settings/UserPickerFieldSettingsDriver.cs index c6fbb21ea96..71b4f6f00b1 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentFields/Settings/UserPickerFieldSettingsDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentFields/Settings/UserPickerFieldSettingsDriver.cs @@ -24,17 +24,15 @@ public override IDisplayResult Edit(ContentPartFieldDefinition partFieldDefiniti model.Hint = settings.Hint; model.Required = settings.Required; model.Multiple = settings.Multiple; - var roles = (await _roleService.GetRoleNamesAsync()) - .Except(RoleHelper.SystemRoleNames, StringComparer.OrdinalIgnoreCase) - .Select(roleName => new RoleEntry - { - Role = roleName, - IsSelected = settings.DisplayedRoles.Contains(roleName, StringComparer.OrdinalIgnoreCase) - }) - .ToArray(); + var roles = await _roleService.GetAssignableRolesAsync(); + var roleEntries = roles.Select(role => new RoleEntry + { + Role = role.RoleName, + IsSelected = settings.DisplayedRoles.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase), + }).ToArray(); - model.Roles = roles; - model.DisplayAllUsers = settings.DisplayAllUsers || !roles.Where(x => x.IsSelected).Any(); + model.Roles = roleEntries; + model.DisplayAllUsers = settings.DisplayAllUsers || !roleEntries.Where(x => x.IsSelected).Any(); }).Location("Content"); } @@ -51,7 +49,12 @@ public override async Task UpdateAsync(ContentPartFieldDefinitio Multiple = model.Multiple }; - var selectedRoles = model.Roles.Where(x => x.IsSelected).Select(x => x.Role).ToArray(); + var roles = await _roleService.GetAssignableRolesAsync(); + + var selectedRoles = model.Roles + .Where(x => x.IsSelected && roles.Any(y => y.RoleName == x.Role)) + .Select(x => x.Role) + .ToArray(); if (model.DisplayAllUsers || selectedRoles.Length == 0) { diff --git a/src/OrchardCore.Modules/OrchardCore.Recipes/AdminMenu.cs b/src/OrchardCore.Modules/OrchardCore.Recipes/AdminMenu.cs index a60ff2635e5..447e726db00 100644 --- a/src/OrchardCore.Modules/OrchardCore.Recipes/AdminMenu.cs +++ b/src/OrchardCore.Modules/OrchardCore.Recipes/AdminMenu.cs @@ -1,6 +1,5 @@ using Microsoft.Extensions.Localization; using OrchardCore.Navigation; -using OrchardCore.Security; namespace OrchardCore.Recipes; @@ -20,7 +19,7 @@ protected override ValueTask BuildAsync(NavigationBuilder builder) .Add(S["Recipes"], S["Recipes"].PrefixPosition(), recipes => recipes .AddClass("recipes") .Id("recipes") - .Permission(StandardPermissions.SiteOwner) + .Permission(RecipePermissions.ManageRecipes) .Action("Index", "Admin", "OrchardCore.Recipes") .LocalNav() ) diff --git a/src/OrchardCore.Modules/OrchardCore.Recipes/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Recipes/Controllers/AdminController.cs index 9dcab0a2e12..4c3645d99fa 100644 --- a/src/OrchardCore.Modules/OrchardCore.Recipes/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Recipes/Controllers/AdminController.cs @@ -11,7 +11,6 @@ using OrchardCore.Recipes.Models; using OrchardCore.Recipes.Services; using OrchardCore.Recipes.ViewModels; -using OrchardCore.Security; namespace OrchardCore.Recipes.Controllers; @@ -60,7 +59,7 @@ public AdminController( [Admin("Recipes", "Recipes")] public async Task Index() { - if (!await _authorizationService.AuthorizeAsync(User, StandardPermissions.SiteOwner)) + if (!await _authorizationService.AuthorizeAsync(User, RecipePermissions.ManageRecipes)) { return Forbid(); } @@ -86,7 +85,7 @@ public async Task Index() [HttpPost] public async Task Execute(string basePath, string fileName) { - if (!await _authorizationService.AuthorizeAsync(User, StandardPermissions.SiteOwner)) + if (!await _authorizationService.AuthorizeAsync(User, RecipePermissions.ManageRecipes)) { return Forbid(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Recipes/RecipesPermissionProvider.cs b/src/OrchardCore.Modules/OrchardCore.Recipes/RecipesPermissionProvider.cs new file mode 100644 index 00000000000..d0745bf1af4 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Recipes/RecipesPermissionProvider.cs @@ -0,0 +1,23 @@ +using OrchardCore.Security.Permissions; + +namespace OrchardCore.Recipes; + +public sealed class RecipesPermissionProvider : IPermissionProvider +{ + private readonly IEnumerable _allPermissions = + [ + RecipePermissions.ManageRecipes, + ]; + + public Task> GetPermissionsAsync() + => Task.FromResult(_allPermissions); + + public IEnumerable GetDefaultStereotypes() => + [ + new PermissionStereotype + { + Name = OrchardCoreConstants.Roles.Administrator, + Permissions = _allPermissions, + }, + ]; +} diff --git a/src/OrchardCore.Modules/OrchardCore.Recipes/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Recipes/Startup.cs index b44b250b4dd..176372484d4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Recipes/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Recipes/Startup.cs @@ -4,6 +4,7 @@ using OrchardCore.Navigation; using OrchardCore.Recipes.RecipeSteps; using OrchardCore.Recipes.Services; +using OrchardCore.Security.Permissions; namespace OrchardCore.Recipes; @@ -15,7 +16,7 @@ public sealed class Startup : StartupBase public override void ConfigureServices(IServiceCollection services) { services.AddNavigationProvider(); - + services.AddPermissionProvider(); services.AddRecipeExecutionStep(); services.AddRecipeExecutionStep(); diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs index 5e6a0495a43..e61517c8c73 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs @@ -65,11 +65,21 @@ public async Task Index() var roles = await _roleService.GetRolesAsync(); - var model = new RolesViewModel + var model = new RolesViewModel() { - RoleEntries = roles.OrderBy(r => r.RoleName).Select(BuildRoleEntry).ToList() + RoleEntries = [], }; + foreach (var role in roles.OrderBy(r => r.RoleName)) + { + model.RoleEntries.Add(new RoleEntry + { + Name = role.RoleName, + Description = role.RoleDescription, + IsSystemRole = await _roleService.IsSystemRoleAsync(role.RoleName), + }); + } + return View(model); } @@ -102,7 +112,7 @@ public async Task Create(CreateRoleViewModel model) ModelState.AddModelError(string.Empty, S["Invalid role name."]); } - if (await _roleManager.FindByNameAsync(_roleManager.NormalizeKey(model.RoleName)) != null) + if (await _roleManager.FindByNameAsync(model.RoleName) != null) { ModelState.AddModelError(string.Empty, S["The role is already used."]); } @@ -110,11 +120,18 @@ public async Task Create(CreateRoleViewModel model) if (ModelState.IsValid) { - var role = new Role { RoleName = model.RoleName, RoleDescription = model.RoleDescription }; + var role = new Role + { + RoleName = model.RoleName, + RoleDescription = model.RoleDescription, + }; + var result = await _roleManager.CreateAsync(role); + if (result.Succeeded) { await _notifier.SuccessAsync(H["Role created successfully."]); + return RedirectToAction(nameof(Index)); } @@ -126,46 +143,10 @@ public async Task Create(CreateRoleViewModel model) } } - // If we got this far, something failed, redisplay form + // If we got this far, something failed, redisplay form. return View(model); } - [HttpPost] - public async Task Delete(string id) - { - if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ManageRoles)) - { - return Forbid(); - } - - var currentRole = await _roleManager.FindByIdAsync(id); - - if (currentRole == null) - { - return NotFound(); - } - - var result = await _roleManager.DeleteAsync(currentRole); - - if (result.Succeeded) - { - await _notifier.SuccessAsync(H["Role deleted successfully."]); - } - else - { - await _documentStore.CancelAsync(); - - await _notifier.ErrorAsync(H["Could not delete this role."]); - - foreach (var error in result.Errors) - { - await _notifier.ErrorAsync(H[error.Description]); - } - } - - return RedirectToAction(nameof(Index)); - } - public async Task Edit(string id) { if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ManageRoles)) @@ -173,23 +154,28 @@ public async Task Edit(string id) return Forbid(); } - if (await _roleManager.FindByNameAsync(_roleManager.NormalizeKey(id)) is not Role role) + if (await _roleManager.FindByIdAsync(id) is not Role role) { return NotFound(); } - var installedPermissions = await GetInstalledPermissionsAsync(); - var allPermissions = installedPermissions.SelectMany(x => x.Value); - var model = new EditRoleViewModel { Role = role, Name = role.RoleName, RoleDescription = role.RoleDescription, - EffectivePermissions = await GetEffectivePermissions(role, allPermissions), - RoleCategoryPermissions = installedPermissions + IsAdminRole = await _roleService.IsAdminRoleAsync(role.RoleName), }; + if (!await _roleService.IsAdminRoleAsync(role.RoleName)) + { + var installedPermissions = await GetInstalledPermissionsAsync(); + var allPermissions = installedPermissions.SelectMany(x => x.Value); + + model.EffectivePermissions = await GetEffectivePermissions(role, allPermissions); + model.RoleCategoryPermissions = installedPermissions; + } + return View(model); } @@ -201,26 +187,29 @@ public async Task EditPost(string id, string roleDescription) return Forbid(); } - if (await _roleManager.FindByNameAsync(_roleManager.NormalizeKey(id)) is not Role role) + if (await _roleManager.FindByIdAsync(id) is not Role role) { return NotFound(); } role.RoleDescription = roleDescription; - // Save. - var rolePermissions = new List(); - foreach (var key in Request.Form.Keys) + if (!await _roleService.IsAdminRoleAsync(role.RoleName)) { - if (key.StartsWith("Checkbox.", StringComparison.Ordinal) && Request.Form[key] == "true") + var rolePermissions = new List(); + + foreach (var key in Request.Form.Keys) { - var permissionName = key["Checkbox.".Length..]; - rolePermissions.Add(new RoleClaim { ClaimType = Permission.ClaimType, ClaimValue = permissionName }); + if (key.StartsWith("Checkbox.", StringComparison.Ordinal) && Request.Form[key] == "true") + { + var permissionName = key["Checkbox.".Length..]; + rolePermissions.Add(RoleClaim.Create(permissionName)); + } } - } - role.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); - role.RoleClaims.AddRange(rolePermissions); + role.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); + role.RoleClaims.AddRange(rolePermissions); + } await _roleManager.UpdateAsync(role); @@ -229,14 +218,47 @@ public async Task EditPost(string id, string roleDescription) return RedirectToAction(nameof(Index)); } - private RoleEntry BuildRoleEntry(IRole role) + [HttpPost] + public async Task Delete(string id) { - return new RoleEntry + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ManageRoles)) { - Name = role.RoleName, - Description = role.RoleDescription, - Selected = false - }; + return Forbid(); + } + + var role = await _roleManager.FindByIdAsync(id); + + if (role == null) + { + return NotFound(); + } + + if (await _roleService.IsSystemRoleAsync(role.RoleName)) + { + await _notifier.ErrorAsync(H["System roles cannot be deleted."]); + + return RedirectToAction(nameof(Index)); + } + + var result = await _roleManager.DeleteAsync(role); + + if (result.Succeeded) + { + await _notifier.SuccessAsync(H["Role deleted successfully."]); + } + else + { + await _documentStore.CancelAsync(); + + await _notifier.ErrorAsync(H["Could not delete this role."]); + + foreach (var error in result.Errors) + { + await _notifier.ErrorAsync(new LocalizedHtmlString(error.Description, error.Description)); + } + } + + return RedirectToAction(nameof(Index)); } private async Task>> GetInstalledPermissionsAsync() diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Deployment/AllRolesDeploymentSource.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Deployment/AllRolesDeploymentSource.cs index 36c2e0982c4..7fba39dc296 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Deployment/AllRolesDeploymentSource.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Deployment/AllRolesDeploymentSource.cs @@ -37,16 +37,16 @@ public async Task ProcessDeploymentStepAsync(DeploymentStep step, DeploymentPlan foreach (var role in allRoles) { - var currentRole = (Role)await _roleManager.FindByNameAsync(_roleManager.NormalizeKey(role.RoleName)); + var currentRole = await _roleManager.FindByNameAsync(role.RoleName); - if (currentRole != null) + if (currentRole is Role r) { permissions.Add(JObject.FromObject( new RolesStepRoleModel { - Name = currentRole.RoleName, - Description = currentRole.RoleDescription, - Permissions = currentRole.RoleClaims.Where(x => x.ClaimType == Permission.ClaimType).Select(x => x.ClaimValue).ToArray() + Name = r.RoleName, + Description = r.RoleDescription, + Permissions = r.RoleClaims.Where(x => x.ClaimType == Permission.ClaimType).Select(x => x.ClaimValue).ToArray() })); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs new file mode 100644 index 00000000000..b9bd4bf00b9 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs @@ -0,0 +1,127 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using OrchardCore.BackgroundJobs; +using OrchardCore.Data.Migration; +using OrchardCore.Environment.Shell; +using OrchardCore.Environment.Shell.Scope; +using OrchardCore.Security; +using OrchardCore.Users; + +namespace OrchardCore.Roles.Migrations; + +public sealed class RolesMigrations : DataMigration +{ + private static readonly string _alternativeAdminRoleName = "SiteOwner"; + + private SystemRoleOptions _systemRoleOptions; + + public RolesMigrations(IOptions systemRoleOptions) + { + _systemRoleOptions = systemRoleOptions.Value; + } + +#pragma warning disable CA1822 // Mark members as static + public int Create() +#pragma warning restore CA1822 // Mark members as static + { + ShellScope.AddDeferredTask(async scope => + { + var roleManager = scope.ServiceProvider.GetRequiredService>(); + + var roles = roleManager.Roles.ToList(); + + var adminRoles = new List(); + var adminSystemRoleName = _systemRoleOptions.SystemAdminRoleName; + + foreach (var role in roles) + { + if (role is not Role r) + { + continue; + } + + // When a new tenant is created, the RoleClaims will be empty for Admin roles. + var hasSiteOwner = r.RoleClaims is null || + r.RoleClaims.Count == 0 || + r.RoleClaims.Any(x => x.ClaimValue == "SiteOwner"); + + if (r.RoleName.Equals(OrchardCoreConstants.Roles.Administrator, StringComparison.OrdinalIgnoreCase)) + { + if (!hasSiteOwner) + { + // At this point, the tenant is using the Administrator role without 'SiteOwner' permission. + // We'll need to create a new role name that does not exists and assign it as the system 'Administrator' role. + adminSystemRoleName = GenerateNewAdminRoleName(roles); + + await roleManager.CreateAsync(new Role + { + RoleName = adminSystemRoleName, + }); + } + else + { + r.RoleClaims.Clear(); + + await roleManager.UpdateAsync(r); + } + } + + if (hasSiteOwner) + { + adminRoles.Add(r); + } + } + + if (adminRoles.Count > 0) + { + // Run the migration in the background to ensure that the newly added role is committed to the database first, preventing potential exceptions. + await HttpBackgroundJob.ExecuteAfterEndOfRequestAsync("MigrateAdminUsersToNewAdminRole", async subScope => + { + var userManager = subScope.ServiceProvider.GetRequiredService>(); + + foreach (var adminRole in adminRoles) + { + var users = await userManager.GetUsersInRoleAsync(adminRole.RoleName); + + foreach (var user in users) + { + await userManager.AddToRoleAsync(user, adminSystemRoleName); + } + } + }); + } + + if (adminSystemRoleName != OrchardCoreConstants.Roles.Administrator) + { + // At this point, we'll update the shell settings and release the shell. + var shellSettings = scope.ServiceProvider.GetRequiredService(); + var shellHost = scope.ServiceProvider.GetRequiredService(); + + shellSettings["AdminRoleName"] = adminSystemRoleName; + + await shellHost.UpdateShellSettingsAsync(shellSettings); + await shellHost.ReleaseShellContextAsync(shellSettings); + } + }); + + return 1; + } + + private static string GenerateNewAdminRoleName(List roles) + { + var counter = 1; + var roleName = _alternativeAdminRoleName; + + while (RoleExists(roles, roleName)) + { + // Generate names like this SiteOwner1, SiteOwner2...SiteOwner{N} + roleName = _alternativeAdminRoleName + counter++; + } + + return roleName; + } + + private static bool RoleExists(List roles, string roleName) + => roles.Any(role => role.RoleName.Equals(roleName, StringComparison.OrdinalIgnoreCase)); +} diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/OrchardCore.Roles.csproj b/src/OrchardCore.Modules/OrchardCore.Roles/OrchardCore.Roles.csproj index 86a1c6a0046..286d9b5bb78 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/OrchardCore.Roles.csproj +++ b/src/OrchardCore.Modules/OrchardCore.Roles/OrchardCore.Roles.csproj @@ -5,7 +5,7 @@ OrchardCore Roles $(OCCMSDescription) - + The roles module adds the ability to manage roles and assign permissions to roles. $(PackageTags) OrchardCoreCMS diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Permissions.cs index 1de94cd3411..33e7f2a0f72 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Permissions.cs @@ -17,7 +17,6 @@ public sealed class Permissions : IPermissionProvider private readonly IEnumerable _allPermissions = [ CommonPermissions.ManageRoles, - StandardPermissions.SiteOwner, ]; public Task> GetPermissionsAsync() diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs index 332e6fecf18..f15adcd4ae9 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs @@ -4,6 +4,7 @@ using OrchardCore.Recipes.Services; using OrchardCore.Security; using OrchardCore.Security.Permissions; +using OrchardCore.Security.Services; namespace OrchardCore.Roles.Recipes; @@ -13,10 +14,14 @@ namespace OrchardCore.Roles.Recipes; public sealed class RolesStep : IRecipeStepHandler { private readonly RoleManager _roleManager; + private readonly IRoleService _roleService; - public RolesStep(RoleManager roleManager) + public RolesStep( + RoleManager roleManager, + IRoleService roleService) { _roleManager = roleManager; + _roleService = roleService; } public async Task ExecuteAsync(RecipeExecutionContext context) @@ -28,31 +33,36 @@ public async Task ExecuteAsync(RecipeExecutionContext context) var model = context.Step.ToObject(); - foreach (var importedRole in model.Roles) + foreach (var roleEntry in model.Roles) { - if (string.IsNullOrWhiteSpace(importedRole.Name)) + var roleName = roleEntry.Name?.Trim(); + + if (string.IsNullOrWhiteSpace(roleName)) { continue; } - var role = (Role)await _roleManager.FindByNameAsync(importedRole.Name); + var role = await _roleManager.FindByNameAsync(roleName); var isNewRole = role == null; if (isNewRole) { role = new Role { - RoleName = importedRole.Name + RoleName = roleName, }; } - role.RoleDescription = importedRole.Description; - role.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); - role.RoleClaims.AddRange(importedRole.Permissions.Select(p => new RoleClaim + if (role is Role r) { - ClaimType = Permission.ClaimType, - ClaimValue = p, - })); + r.RoleDescription = roleEntry.Description; + r.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); + + if (!await _roleService.IsAdminRoleAsync(roleName)) + { + r.RoleClaims.AddRange(roleEntry.Permissions.Select(RoleClaim.Create)); + } + } if (isNewRole) { @@ -74,6 +84,8 @@ public sealed class RolesStepModel public sealed class RolesStepRoleModel { public string Name { get; set; } + public string Description { get; set; } + public string[] Permissions { get; set; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs new file mode 100644 index 00000000000..af2599a527a --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs @@ -0,0 +1,75 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using OrchardCore.Security; +using OrchardCore.Security.Services; +using OrchardCore.Users; +using OrchardCore.Users.Services; + +namespace OrchardCore.Roles; + +public class RoleClaimsProvider : IUserClaimsProvider +{ + private readonly UserManager _userManager; + private readonly RoleManager _roleManager; + private readonly IRoleService _roleService; + private readonly IdentityOptions _identityOptions; + + public RoleClaimsProvider( + UserManager userManager, + RoleManager roleManager, + IRoleService roleService, + IOptions identityOptions) + { + _userManager = userManager; + _roleManager = roleManager; + _roleService = roleService; + _identityOptions = identityOptions.Value; + } + + public async Task GenerateAsync(IUser user, ClaimsIdentity claims) + { + if (!_userManager.SupportsUserRole) + { + return; + } + + var roleNames = await _userManager.GetRolesAsync(user); + var roles = new List(); + var addClaims = true; + + foreach (var roleName in roleNames) + { + claims.AddClaim(new Claim(_identityOptions.ClaimsIdentity.RoleClaimType, roleName)); + + if (!_roleManager.SupportsRoleClaims) + { + continue; + } + + var role = await _roleManager.FindByNameAsync(roleName); + + if (role == null) + { + continue; + } + + if (addClaims && await _roleService.IsAdminRoleAsync(role.RoleName)) + { + addClaims = false; + } + + roles.Add(role); + } + + if (roles.Count == 0 || !addClaims) + { + return; + } + + foreach (var role in roles) + { + claims.AddClaims(await _roleManager.GetClaimsAsync(role)); + } + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs index e81acb6d7a8..9d427afcb79 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs @@ -13,6 +13,7 @@ namespace OrchardCore.Roles.Services; public class RoleStore : IRoleClaimStore, IQueryableRoleStore { private readonly IServiceProvider _serviceProvider; + private readonly ISystemRoleNameProvider _systemRoleProvider; private readonly IDocumentManager _documentManager; protected readonly IStringLocalizer S; private readonly ILogger _logger; @@ -21,27 +22,32 @@ public class RoleStore : IRoleClaimStore, IQueryableRoleStore public RoleStore( IServiceProvider serviceProvider, + ISystemRoleNameProvider systemRoleProvider, IDocumentManager documentManager, IStringLocalizer stringLocalizer, ILogger logger) { _serviceProvider = serviceProvider; + _systemRoleProvider = systemRoleProvider; _documentManager = documentManager; S = stringLocalizer; _logger = logger; } - public IQueryable Roles => GetRolesAsync().GetAwaiter().GetResult().Roles.AsQueryable(); + public IQueryable Roles + => GetRolesAsync().GetAwaiter().GetResult().Roles.AsQueryable(); /// /// Loads the roles document from the store for updating and that should not be cached. /// - private Task LoadRolesAsync() => _documentManager.GetOrCreateMutableAsync(); + private Task LoadRolesAsync() + => _documentManager.GetOrCreateMutableAsync(); /// /// Gets the roles document from the cache for sharing and that should not be updated. /// - private Task GetRolesAsync() => _documentManager.GetOrCreateImmutableAsync(); + private Task GetRolesAsync() + => _documentManager.GetOrCreateImmutableAsync(); /// /// Updates the store with the provided roles document and then updates the cache. @@ -77,12 +83,20 @@ public async Task DeleteAsync(IRole role, CancellationToken canc { ArgumentNullException.ThrowIfNull(role); - var roleToRemove = (Role)role; + if (role is not Role roleToRemove) + { + return IdentityResult.Failed(new IdentityError + { + Description = S["Role is not of a '{0}' type.", nameof(Role)], + }); + } - if (string.Equals(roleToRemove.NormalizedRoleName, "ANONYMOUS", StringComparison.Ordinal) || - string.Equals(roleToRemove.NormalizedRoleName, "AUTHENTICATED", StringComparison.Ordinal)) + if (await _systemRoleProvider.IsSystemRoleAsync(roleToRemove.RoleName)) { - return IdentityResult.Failed(new IdentityError { Description = S["Can't delete system roles."] }); + return IdentityResult.Failed(new IdentityError + { + Description = S["Can't delete system roles."], + }); } var roleRemovedEventHandlers = _serviceProvider.GetRequiredService>(); @@ -133,7 +147,12 @@ public Task GetNormalizedRoleNameAsync(IRole role, CancellationToken can { ArgumentNullException.ThrowIfNull(role); - return Task.FromResult(((Role)role).NormalizedRoleName); + if (role is Role r) + { + return Task.FromResult(r.NormalizedRoleName); + } + + return Task.FromResult(role.RoleName); } public Task GetRoleIdAsync(IRole role, CancellationToken cancellationToken) @@ -154,7 +173,10 @@ public Task SetNormalizedRoleNameAsync(IRole role, string normalizedName, Cancel { ArgumentNullException.ThrowIfNull(role); - ((Role)role).NormalizedRoleName = normalizedName; + if (role is Role r) + { + r.NormalizedRoleName = normalizedName; + } return Task.CompletedTask; } @@ -163,7 +185,10 @@ public Task SetRoleNameAsync(IRole role, string roleName, CancellationToken canc { ArgumentNullException.ThrowIfNull(role); - ((Role)role).RoleName = roleName; + if (role is Role r) + { + r.RoleName = roleName; + } return Task.CompletedTask; } @@ -175,7 +200,11 @@ public async Task UpdateAsync(IRole role, CancellationToken canc var roles = await LoadRolesAsync(); var existingRole = roles.Roles.FirstOrDefault(x => string.Equals(x.RoleName, role.RoleName, StringComparison.OrdinalIgnoreCase)); roles.Roles.Remove(existingRole); - roles.Roles.Add((Role)role); + + if (role is Role r) + { + roles.Roles.Add(r); + } await UpdateRolesAsync(roles); @@ -189,10 +218,12 @@ public async Task UpdateAsync(IRole role, CancellationToken canc public Task AddClaimAsync(IRole role, Claim claim, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(role); - ArgumentNullException.ThrowIfNull(claim); - ((Role)role).RoleClaims.Add(new RoleClaim { ClaimType = claim.Type, ClaimValue = claim.Value }); + if (role is Role r) + { + r.RoleClaims.Add(new RoleClaim(type: claim.Type, value: claim.Value)); + } return Task.CompletedTask; } @@ -201,17 +232,23 @@ public Task> GetClaimsAsync(IRole role, CancellationToken cancellat { ArgumentNullException.ThrowIfNull(role); - return Task.FromResult>(((Role)role).RoleClaims.Select(x => x.ToClaim()).ToList()); + if (role is Role r) + { + return Task.FromResult>(r.RoleClaims.Select(x => x.ToClaim()).ToArray()); + } + + return Task.FromResult>([]); } public Task RemoveClaimAsync(IRole role, Claim claim, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(role); - ArgumentNullException.ThrowIfNull(claim); - ((Role)role).RoleClaims.RemoveAll(x => x.ClaimType == claim.Type && x.ClaimValue == claim.Value); - + if (role is Role r) + { + r.RoleClaims.RemoveAll(x => x.ClaimType == claim.Type && x.ClaimValue == claim.Value); + } return Task.CompletedTask; } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs index e60df800fbe..140c5f193c6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs @@ -15,6 +15,7 @@ public class RoleUpdater : FeatureEventHandler, IRoleCreatedEventHandler, IRoleR private readonly ShellDescriptor _shellDescriptor; private readonly IExtensionManager _extensionManager; private readonly IDocumentManager _documentManager; + private readonly ISystemRoleNameProvider _systemRoleNameProvider; private readonly IEnumerable _permissionProviders; private readonly ITypeFeatureProvider _typeFeatureProvider; private readonly ILogger _logger; @@ -25,6 +26,7 @@ public RoleUpdater( ShellDescriptor shellDescriptor, IExtensionManager extensionManager, IDocumentManager documentManager, + ISystemRoleNameProvider systemRoleNameProvider, IEnumerable permissionProviders, ITypeFeatureProvider typeFeatureProvider, ILogger logger) @@ -32,18 +34,23 @@ public RoleUpdater( _shellDescriptor = shellDescriptor; _extensionManager = extensionManager; _documentManager = documentManager; + _systemRoleNameProvider = systemRoleNameProvider; _permissionProviders = permissionProviders; _typeFeatureProvider = typeFeatureProvider; _logger = logger; } - public override Task InstalledAsync(IFeatureInfo feature) => UpdateRolesForInstalledFeatureAsync(feature); + public override Task InstalledAsync(IFeatureInfo feature) + => UpdateRolesForInstalledFeatureAsync(feature); - public override Task EnabledAsync(IFeatureInfo feature) => UpdateRolesForEnabledFeatureAsync(feature); + public override Task EnabledAsync(IFeatureInfo feature) + => UpdateRolesForEnabledFeatureAsync(feature); - public Task RoleCreatedAsync(string roleName) => UpdateRoleForInstalledFeaturesAsync(roleName); + public Task RoleCreatedAsync(string roleName) + => UpdateRoleForInstalledFeaturesAsync(roleName); - public Task RoleRemovedAsync(string roleName) => RemoveRoleForMissingFeaturesAsync(roleName); + public Task RoleRemovedAsync(string roleName) + => RemoveRoleForMissingFeaturesAsync(roleName); private async Task UpdateRolesForInstalledFeatureAsync(IFeatureInfo feature) { @@ -73,7 +80,7 @@ private async Task UpdateRolesForInstalledFeatureAsync(IFeatureInfo feature) var permissions = (stereotype.Permissions ?? []) .Select(stereotype => stereotype.Name); - if (UpdateRole(role, permissions, _logger)) + if (await UpdatePermissionsAsync(role, permissions)) { updated = true; } @@ -114,7 +121,7 @@ private async Task UpdateRolesForEnabledFeatureAsync(IFeatureInfo feature) updated = true; missingFeatures.Remove(feature.Id); - UpdateRolesForEnabledFeature(role, providers, _logger); + await UpdateRolesForEnabledFeatureAsync(role, providers); } if (updated) @@ -159,7 +166,7 @@ private async Task UpdateRoleForInstalledFeaturesAsync(string roleName) .SelectMany(stereotype => stereotype.Permissions ?? []) .Select(stereotype => stereotype.Name); - UpdateRole(role, permissions, _logger); + await UpdatePermissionsAsync(role, permissions); } private async Task RemoveRoleForMissingFeaturesAsync(string roleName) @@ -172,7 +179,7 @@ private async Task RemoveRoleForMissingFeaturesAsync(string roleName) } } - private static bool UpdateRolesForEnabledFeature(Role role, IEnumerable providers, ILogger logger) + private Task UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable providers) { var stereotypes = providers .SelectMany(provider => provider.GetDefaultStereotypes()) @@ -180,7 +187,7 @@ private static bool UpdateRolesForEnabledFeature(Role role, IEnumerable permissions, ILogger logger) + private async Task UpdatePermissionsAsync(Role role, IEnumerable permissions) { + if (await _systemRoleNameProvider.IsAdminRoleAsync(role.RoleName)) + { + // Don't update claims for admin role. + return true; + } + var currentPermissions = role.RoleClaims .Where(roleClaim => roleClaim.ClaimType == Permission.ClaimType) .Select(roleClaim => roleClaim.ClaimValue); - var distinctPermissions = currentPermissions + var additionalPermissions = currentPermissions .Union(permissions) - .Distinct(); + .Distinct() + .Except(currentPermissions); - var additionalPermissions = distinctPermissions.Except(currentPermissions); if (!additionalPermissions.Any()) { return false; @@ -213,12 +226,9 @@ private static bool UpdateRole(Role role, IEnumerable permissions, ILogg foreach (var permission in additionalPermissions) { - if (logger.IsEnabled(LogLevel.Debug)) - { - logger.LogDebug("Default role '{RoleName}' granted permission '{PermissionName}'.", role.RoleName, permission); - } + _logger.LogDebug("Default role '{RoleName}' granted permission '{PermissionName}'.", role.RoleName, permission); - role.RoleClaims.Add(new RoleClaim { ClaimType = Permission.ClaimType, ClaimValue = permission }); + role.RoleClaims.Add(RoleClaim.Create(permission)); } return true; diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs index e471648fdd2..4886ee4412a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs @@ -1,33 +1,61 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using OrchardCore.Data.Migration; using OrchardCore.Deployment; using OrchardCore.Environment.Shell; +using OrchardCore.Environment.Shell.Configuration; using OrchardCore.Modules; using OrchardCore.Navigation; using OrchardCore.Recipes; +using OrchardCore.Roles.Core; using OrchardCore.Roles.Deployment; +using OrchardCore.Roles.Migrations; using OrchardCore.Roles.Recipes; using OrchardCore.Roles.Services; using OrchardCore.Security; using OrchardCore.Security.Permissions; using OrchardCore.Security.Services; +using OrchardCore.Users.Services; namespace OrchardCore.Roles; public sealed class Startup : StartupBase { + private readonly IShellConfiguration _shellConfiguration; + + public Startup(IShellConfiguration shellConfiguration) + { + _shellConfiguration = shellConfiguration; + } + public override void ConfigureServices(IServiceCollection services) { + services.AddScoped(); + services.AddDataMigration(); + services.AddScoped(); services.AddScoped(); services.Replace(ServiceDescriptor.Scoped>(sp => sp.GetRequiredService())); services.Replace(ServiceDescriptor.Scoped>(sp => sp.GetRequiredService())); - services.AddRecipeExecutionStep(); services.AddScoped(); services.AddPermissionProvider(); services.AddNavigationProvider(); + services.Configure(options => + { + var adminRoleName = _shellConfiguration.GetSection("OrchardCore_Roles").GetValue("AdminRoleName"); + + if (!string.IsNullOrWhiteSpace(adminRoleName)) + { + options.SystemAdminRoleName = adminRoleName; + } + else + { + options.SystemAdminRoleName = OrchardCoreConstants.Roles.Administrator; + } + }); } } @@ -45,9 +73,9 @@ public sealed class RoleUpdaterStartup : StartupBase { public override void ConfigureServices(IServiceCollection services) { + services.AddRolesCoreServices(); services.AddScoped>(); services.AddScoped(); - services.AddScoped(); services.AddScoped(sp => sp.GetRequiredService()); services.AddScoped(sp => sp.GetRequiredService()); diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/EditRoleViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/EditRoleViewModel.cs index 1d5223c6915..0c4df604b78 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/EditRoleViewModel.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/EditRoleViewModel.cs @@ -7,8 +7,12 @@ namespace OrchardCore.Roles.ViewModels; public class EditRoleViewModel { public string Name { get; set; } + public string RoleDescription { get; set; } + [BindNever] + public bool IsAdminRole { get; set; } + [BindNever] public IDictionary> RoleCategoryPermissions { get; set; } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/RolesViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/RolesViewModel.cs index 36f919979fb..9bebbae1c9d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/RolesViewModel.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/ViewModels/RolesViewModel.cs @@ -1,3 +1,5 @@ +using Microsoft.AspNetCore.Mvc.ModelBinding; + namespace OrchardCore.Roles.ViewModels; public class RolesViewModel @@ -8,6 +10,11 @@ public class RolesViewModel public class RoleEntry { public string Name { get; set; } + public string Description { get; set; } + public bool Selected { get; set; } + + [BindNever] + public bool IsSystemRole { get; set; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Create.cshtml b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Create.cshtml index 5b5775d106c..c87207af1c4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Create.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Create.cshtml @@ -4,16 +4,18 @@

@RenderTitleSegments(T["Create Role"])
+
- +
- - + + +
diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Edit.cshtml index 7260db3389e..93aa0c4c5cb 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Edit.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Edit.cshtml @@ -1,88 +1,91 @@ @model EditRoleViewModel @using OrchardCore.Roles.ViewModels -

@RenderTitleSegments(T["Edit Role {0}", Model.Name])

 - - - -
-
-
- -
-
-
- +

@RenderTitleSegments(T["Edit '{0}' Role", Model.Name])
-

@T["Edit Role Detail"]

+
- - + + +
-
-

@T["Role permissions"]

+ @if (Model.IsAdminRole) + { + + } + else + { +
+ +
+
+ @T["Allow"] @T["Permission is granted explicitly."] +
+
+ @T["Effective"] @T["Permission is implied by a higher level permission, or explicitly granted."] +
+
-
- @T["Allow"] @T["Permission is granted explicitly."] -
-
- @T["Effective"] @T["Permission is implied by a higher level permission, or explicitly granted."] -
-
+
+
+ +
+
- @foreach (var group in Model.RoleCategoryPermissions.OrderBy(x => x.Key.Title).ThenBy(x => x.Key.Source)) - { -
- - @group.Key.Title - @if (!string.IsNullOrEmpty(group.Key.Source)) - { - (@group.Key.Source) - } - + @foreach (var group in Model.RoleCategoryPermissions.OrderBy(x => x.Key.Title).ThenBy(x => x.Key.Source)) + { +
+ + @group.Key.Title + @if (!string.IsNullOrEmpty(group.Key.Source)) + { + (@group.Key.Source) + } + - - - - - - - - - - @foreach (var permission in group.Value.OrderBy(x => x.Description)) - { - - - - - - } - -
@T["Permission"]@T["Allow"]@T["Effective"]
- @permission.Description - @if (permission.IsSecurityCritical) - { -
- @T["Security Critical"] -
- } - 		-
- - -
- 		-
- - -
-
-
+ + + + + + + + + + @foreach (var permission in group.Value.OrderBy(x => x.Description)) + { + + + + + + } + +
@T["Permission"]@T["Allow"]@T["Effective"]
+ @permission.Description + @if (permission.IsSecurityCritical) + { +
+ @T["Security Critical"] +
+ } + 		+
+ + +
+ 		+
+ + +
+
+
+ } +
} -
@T["Cancel"] diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Index.cshtml b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Index.cshtml index 7e016a0b479..a2cbf495713 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Index.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Views/Admin/Index.cshtml @@ -21,21 +21,40 @@
+ @{ + var message = T["Are you sure you want to delete this role?"]; + }
    @for (var i = 0; i < Model.RoleEntries.Count; i++) { - var message = T["Are you sure you want to delete this role?"]; + var entry = Model.RoleEntries[i]; -
  • -
    -
    - @T["Edit"] - @T["Delete"] +
  • +
    +
    + @entry.Name + + @if (entry.IsSystemRole) + { + + @T["System"] + + } +
    + +
    + @T["Edit"] + + @if (!entry.IsSystemRole) + { + @T["Delete"] + }
    - @**@ - @Model.RoleEntries[i].Name -

    @Model.RoleEntries[i].Description

    + @if (!string.IsNullOrWhiteSpace(entry.Description)) + { +

    @entry.Description

    + }
    • }
diff --git a/src/OrchardCore.Modules/OrchardCore.Themes/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Themes/Controllers/AdminController.cs index 9bd159d8182..e0234c6ed20 100644 --- a/src/OrchardCore.Modules/OrchardCore.Themes/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Themes/Controllers/AdminController.cs @@ -7,7 +7,6 @@ using OrchardCore.Environment.Extensions; using OrchardCore.Environment.Shell; using OrchardCore.Modules.Manifest; -using OrchardCore.Security; using OrchardCore.Themes.Models; using OrchardCore.Themes.Services; @@ -44,7 +43,7 @@ public AdminController( [Admin("Themes", "Themes.Index")] public async Task Index() { - var installThemes = await _authorizationService.AuthorizeAsync(User, StandardPermissions.SiteOwner); // only site owners + var installThemes = await _authorizationService.AuthorizeAsync(User, Permissions.ApplyTheme); if (!installThemes) { diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RoleLoginSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RoleLoginSettingsDisplayDriver.cs index b85f9234962..75f59f73880 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RoleLoginSettingsDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RoleLoginSettingsDisplayDriver.cs @@ -41,16 +41,15 @@ public override IDisplayResult Edit(ISite site, RoleLoginSettings settings, Buil return Initialize("LoginSettingsRoles_Edit", async model => { model.RequireTwoFactorAuthenticationForSpecificRoles = settings.RequireTwoFactorAuthenticationForSpecificRoles; - var roles = await _roleService.GetRolesAsync(); + var roles = await _roleService.GetAssignableRolesAsync(); model.Roles = roles - .Where(role => !RoleHelper.SystemRoleNames.Contains(role.RoleName)) - .Select(role => new RoleEntry() - { - Role = role.RoleName, - IsSelected = settings.Roles != null && settings.Roles.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase), - }).OrderBy(entry => entry.Role) - .ToArray(); + .Select(role => new RoleEntry() + { + Role = role.RoleName, + IsSelected = settings.Roles != null && settings.Roles.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase), + }).OrderBy(entry => entry.Role) + .ToArray(); }).Location("Content:6#Two-Factor Authentication") .RenderWhen(() => _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext?.User, CommonPermissions.ManageUsers)) .OnGroup(SettingsGroupId); @@ -69,8 +68,7 @@ public override async Task UpdateAsync(ISite site, RoleLoginSett if (model.RequireTwoFactorAuthenticationForSpecificRoles) { - var roles = (await _roleService.GetRolesAsync()) - .Where(role => !RoleHelper.SystemRoleNames.Contains(role.RoleName)); + var roles = await _roleService.GetAssignableRolesAsync(); var selectedRoles = model.Roles.Where(x => x.IsSelected) .Join(roles, e => e.Role, r => r.RoleName, (e, r) => r.RoleName) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserMenuDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserMenuDisplayDriver.cs index 68fe0a1d66b..32b010f1ea6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserMenuDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserMenuDisplayDriver.cs @@ -1,4 +1,6 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; +using OrchardCore.Admin; using OrchardCore.DisplayManagement.Handlers; using OrchardCore.DisplayManagement.Views; using OrchardCore.Users.Models; @@ -8,13 +10,17 @@ namespace OrchardCore.Users.Drivers; public sealed class UserMenuDisplayDriver : DisplayDriver { private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IAuthorizationService _authorizationService; - public UserMenuDisplayDriver(IHttpContextAccessor httpContextAccessor) + public UserMenuDisplayDriver( + IHttpContextAccessor httpContextAccessor, + IAuthorizationService authorizationService) { _httpContextAccessor = httpContextAccessor; + _authorizationService = authorizationService; } - public override Task DisplayAsync(UserMenu model, BuildDisplayContext context) + public override async Task DisplayAsync(UserMenu model, BuildDisplayContext context) { var results = new List { @@ -38,13 +44,13 @@ public override Task DisplayAsync(UserMenu model, BuildDisplayCo .Differentiator("SignOut"), }; - if (_httpContextAccessor.HttpContext.User.HasClaim("Permission", "AccessAdminPanel")) + if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, AdminPermissions.AccessAdminPanel)) { results.Add(View("UserMenuItems__Dashboard", model) .Location("Detail", "Content:1.1") .Differentiator("Dashboard")); } - return CombineAsync(results); + return Combine(results); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index a6c8e22a24e..23a5c3a5b1a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -57,7 +57,7 @@ public override IDisplayResult Edit(User user, BuildEditorContext context) // This view is always rendered, however there will be no editable roles if the user does not have permission to edit them. return Initialize("UserRoleFields_Edit", async model => { - var roles = await GetRoleAsync(); + var roles = await _roleService.GetAssignableRolesAsync(); // When a user is in a role that the current user cannot manage the role is not shown. var authorizedRoleNames = await GetAccessibleRoleNamesAsync(roles); @@ -87,7 +87,8 @@ public override async Task UpdateAsync(User user, UpdateEditorCo await context.Updater.TryUpdateModelAsync(model, Prefix); - var roles = await GetRoleAsync(); + var roles = await _roleService.GetAssignableRolesAsync(); + // Authorize each role in the model to prevent html injection. var accessibleRoleNames = await GetAccessibleRoleNamesAsync(roles); var currentUserRoleNames = await _userRoleStore.GetRolesAsync(user, default); @@ -153,13 +154,6 @@ public override async Task UpdateAsync(User user, UpdateEditorCo return Edit(user, context); } - private async Task> GetRoleAsync() - { - var roles = await _roleService.GetRolesAsync(); - - return roles.Where(role => !RoleHelper.SystemRoleNames.Contains(role.RoleName)); - } - private async Task> GetAccessibleRoleNamesAsync(IEnumerable roles) { var authorizedRoleNames = new List(); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs index 840c7a26adb..f5c1cc3d974 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs @@ -69,12 +69,13 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext return; } - var roleNames = user.RoleNames ?? []; + IEnumerable roleNames = user.RoleNames ?? []; if (!roleNames.Any()) { // When the user is in no roles, we check to see if the current user can manage any roles. - roleNames = (await _roleService.GetRoleNamesAsync()).Where(roleName => !RoleHelper.SystemRoleNames.Contains(roleName)).ToList(); + roleNames = (await _roleService.GetAssignableRolesAsync()) + .Select(x => x.RoleName); } // Check every role to see if the current user has permission to at least one role. diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs index 39548fe9ab3..57a6939112e 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs @@ -27,7 +27,7 @@ public void Build(QueryEngineBuilder builder) if (user != null && !await authorizationService.AuthorizeAsync(user, CommonPermissions.ListUsers)) { // At this point the user cannot see all users, so lets see what role does he have access too and filter by them. - var accessibleRoles = await roleService.GetAccessibleRoleNamesAsync(authorizationService, user, CommonPermissions.ListUsers); + var accessibleRoles = (await roleService.GetAssignableRolesAsync()).Select(x => x.RoleName); query.With(index => index.RoleName.IsIn(accessibleRoles)); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs index 5c81c3275e5..528e2b737a8 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs @@ -31,6 +31,7 @@ using OrchardCore.Recipes; using OrchardCore.Recipes.Services; using OrchardCore.ResourceManagement; +using OrchardCore.Roles; using OrchardCore.Security; using OrchardCore.Security.Permissions; using OrchardCore.Settings.Deployment; diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index 1f2adca045b..2d0aba7b839 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -22,16 +22,23 @@ public async Task> GetPermissionsAsync() CommonPermissions.AssignRoleToUsers, }; - var roleNames = (await _roleService.GetRoleNamesAsync()) - .Where(roleName => !RoleHelper.SystemRoleNames.Contains(roleName)) - .OrderBy(roleName => roleName); + var roleNames = (await _roleService.GetAssignableRolesAsync()) + .Select(role => role.RoleName) + .OrderBy(roleName => roleName); foreach (var roleName in roleNames) { permissions.Add(CommonPermissions.CreateListUsersInRolePermission(roleName)); permissions.Add(CommonPermissions.CreateEditUsersInRolePermission(roleName)); - permissions.Add(CommonPermissions.CreateDeleteUsersInRolePermission(roleName)); - permissions.Add(CommonPermissions.CreateAssignRoleToUsersPermission(roleName)); + + if (!await _roleService.IsAdminRoleAsync(roleName)) + { + // Do not create permissions for deleting or creating admins. + // These operations are restricted to admin users only. + permissions.Add(CommonPermissions.CreateDeleteUsersInRolePermission(roleName)); + permissions.Add(CommonPermissions.CreateAssignRoleToUsersPermission(roleName)); + } + permissions.Add(CommonPermissions.CreatePermissionForManageUsersInRole(roleName)); } diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json index b80fef537fb..5b170e73669 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json @@ -71,37 +71,37 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json index 5ad0ff0b514..65a4b0a24d3 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json @@ -66,32 +66,32 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [ "ViewContent", "ExecuteGraphQL", @@ -100,7 +100,7 @@ }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json index e3f9daeef9b..9c99ed5b29d 100644 --- a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json +++ b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json @@ -75,37 +75,37 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json index ab02cf5402e..bce48abc1cf 100644 --- a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json +++ b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json @@ -89,37 +89,37 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json index 6491a21a8d0..bc8b03a9ebe 100644 --- a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json +++ b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json @@ -72,37 +72,37 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json b/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json index 5d0de0b9d3c..51bb484fe18 100644 --- a/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json +++ b/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json @@ -45,17 +45,17 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Role.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Role.cs index 3f55fc05ab3..1caa46dc4d6 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Role.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Role.cs @@ -3,8 +3,11 @@ namespace OrchardCore.Security; public class Role : IRole { public string RoleName { get; set; } + public string RoleDescription { get; set; } + public string NormalizedRoleName { get; set; } + public List RoleClaims { get; set; } = []; public Role Clone() @@ -18,7 +21,7 @@ public Role Clone() foreach (var claim in RoleClaims) { - role.RoleClaims.Add(new RoleClaim() { ClaimType = claim.ClaimType, ClaimValue = claim.ClaimValue }); + role.RoleClaims.Add(claim.Clone()); } return role; diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/RoleClaim.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/RoleClaim.cs index 1c112559aef..66722388a5a 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/RoleClaim.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/RoleClaim.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using OrchardCore.Security.Permissions; namespace OrchardCore.Security; @@ -17,11 +18,31 @@ public class RoleClaim ///
public string ClaimValue { get; set; } + public RoleClaim() + { + } + + public RoleClaim(string type, string value) + { + ClaimType = type; + ClaimValue = value; + } + + public static RoleClaim Create(string value) + { + return new RoleClaim(type: Permission.ClaimType, value: value); + } + public Claim ToClaim() { return new Claim(ClaimType, ClaimValue); } + public RoleClaim Clone() + { + return new RoleClaim(type: ClaimType, value: ClaimValue); + } + public static implicit operator Claim(RoleClaim claim) { return claim.ToClaim(); diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/IRoleService.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/IRoleService.cs index a050aa4a874..00a405b94f5 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/IRoleService.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/IRoleService.cs @@ -5,6 +5,14 @@ namespace OrchardCore.Security.Services; public interface IRoleService { Task> GetRolesAsync(); + Task> GetRoleClaimsAsync(string role, CancellationToken cancellationToken = default); + Task> GetNormalizedRoleNamesAsync(); + + ValueTask IsAdminRoleAsync(string role) + => ValueTask.FromResult(false); + + ValueTask IsSystemRoleAsync(string role) + => ValueTask.FromResult(false); } diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs index 9363dfd8488..d9313983258 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs @@ -2,6 +2,7 @@ namespace OrchardCore.Security.Services; public static class RoleHelper { + [Obsolete("This method has been marked as obsolete and will be removed in future releases. Instead of using this helper, please use the IRoleService.IsSystemRoleAsync() method.")] public static readonly HashSet SystemRoleNames = new(StringComparer.OrdinalIgnoreCase) { OrchardCoreConstants.Roles.Anonymous, diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs index 7b881f1ebec..5234a95143f 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs @@ -13,18 +13,32 @@ public static async Task> GetRoleNamesAsync(this IRoleServic return roles.Select(r => r.RoleName); } - public static async Task> GetAccessibleRolesAsync(this IRoleService roleService, IAuthorizationService authorizationService, ClaimsPrincipal user, Permission permission) + public static async Task> GetAssignableRolesAsync(this IRoleService roleService) { var roles = await roleService.GetRolesAsync(); - var accessibleRoles = new List(); + var assignableRoles = new List(); foreach (var role in roles) { - if (RoleHelper.SystemRoleNames.Contains(role.RoleName)) + if (!await roleService.IsAdminRoleAsync(role.RoleName) && await roleService.IsSystemRoleAsync(role.RoleName)) { continue; } + assignableRoles.Add(role); + } + + return assignableRoles; + } + + public static async Task> GetAccessibleRolesAsync(this IRoleService roleService, IAuthorizationService authorizationService, ClaimsPrincipal user, Permission permission) + { + var roles = await roleService.GetAssignableRolesAsync(); + + var accessibleRoles = new List(); + + foreach (var role in roles) + { if (!await authorizationService.AuthorizeAsync(user, permission, role)) { continue; @@ -36,6 +50,7 @@ public static async Task> GetAccessibleRolesAsync(this IRoleS return accessibleRoles; } + [Obsolete("This method is obsolete and will be removed in future releases.")] public static async Task> GetAccessibleRoleNamesAsync(this IRoleService roleService, IAuthorizationService authorizationService, ClaimsPrincipal user, Permission permission) { var roles = await roleService.GetAccessibleRolesAsync(authorizationService, user, permission); diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/StandardPermissions.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/StandardPermissions.cs index f012a857685..86cb1ec8b9c 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/StandardPermissions.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/StandardPermissions.cs @@ -4,5 +4,6 @@ namespace OrchardCore.Security; public static class StandardPermissions { + [Obsolete("This permission is deprecated and will be removed in future releases. Instead, consider adding users to the system administrator role.")] public static readonly Permission SiteOwner = new("SiteOwner", "Site Owners Permission", isSecurityCritical: true); } diff --git a/src/OrchardCore/OrchardCore.Infrastructure/Security/DefaultPermissionGrantingService.cs b/src/OrchardCore/OrchardCore.Infrastructure/Security/DefaultPermissionGrantingService.cs index 6acce307320..4c4ee2d02c5 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure/Security/DefaultPermissionGrantingService.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure/Security/DefaultPermissionGrantingService.cs @@ -20,9 +20,6 @@ public bool IsGranted(PermissionRequirement requirement, IEnumerable clai GetGrantingNamesInternal(requirement.Permission, grantingNames); - // SiteOwner permission grants them all - grantingNames.Add(StandardPermissions.SiteOwner.Name); - return claims.Any(claim => string.Equals(claim.Type, Permission.ClaimType, StringComparison.OrdinalIgnoreCase) && grantingNames.Contains(claim.Value)); } diff --git a/src/OrchardCore/OrchardCore.Recipes.Core/RecipePermissions.cs b/src/OrchardCore/OrchardCore.Recipes.Core/RecipePermissions.cs new file mode 100644 index 00000000000..17ea1022278 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Recipes.Core/RecipePermissions.cs @@ -0,0 +1,8 @@ +using OrchardCore.Security.Permissions; + +namespace OrchardCore.Recipes; + +public static class RecipePermissions +{ + public static readonly Permission ManageRecipes = new Permission("ManageRecipes", "Manage Recipes", isSecurityCritical: true); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ISystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/ISystemRoleNameProvider.cs new file mode 100644 index 00000000000..7b7190acd8c --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/ISystemRoleNameProvider.cs @@ -0,0 +1,10 @@ +using System.Collections.Frozen; + +namespace OrchardCore.Roles; + +public interface ISystemRoleNameProvider +{ + ValueTask GetAdminRoleAsync(); + + ValueTask> GetSystemRolesAsync(); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/OrchardCore.Roles.Core.csproj b/src/OrchardCore/OrchardCore.Roles.Core/OrchardCore.Roles.Core.csproj index 566caa49dde..798696b6db3 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/OrchardCore.Roles.Core.csproj +++ b/src/OrchardCore/OrchardCore.Roles.Core/OrchardCore.Roles.Core.csproj @@ -4,9 +4,11 @@ OrchardCore.Roles OrchardCore Roles Core - $(OCCMSDescription) - - Core implementation for Roles module. + + $(OCCMSDescription) + + Core implementation for Roles module. + $(PackageTags) OrchardCoreCMS @@ -14,6 +16,10 @@ + + + + diff --git a/src/OrchardCore/OrchardCore.Roles.Core/RolesPermissionHandler.cs b/src/OrchardCore/OrchardCore.Roles.Core/RolesPermissionHandler.cs new file mode 100644 index 00000000000..3157153b85b --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/RolesPermissionHandler.cs @@ -0,0 +1,44 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using OrchardCore.Security; +using OrchardCore.Security.Services; + +namespace OrchardCore.Roles.Core; + +public class RolesPermissionHandler : AuthorizationHandler +{ + private readonly IRoleService _roleService; + private readonly IdentityOptions _identityOptions; + + public RolesPermissionHandler( + IRoleService roleService, + IOptions identityOptions) + { + _roleService = roleService; + _identityOptions = identityOptions.Value; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, PermissionRequirement requirement) + { + if (context.HasSucceeded || context?.User?.Identity?.IsAuthenticated == false) + { + return; + } + + foreach (var claim in context.User.FindAll(IsRoleClaim)) + { + if (await _roleService.IsAdminRoleAsync(claim.Value)) + { + context.Succeed(requirement); + + return; + } + } + } + + private bool IsRoleClaim(Claim claim) + => claim.Type == _identityOptions.ClaimsIdentity.RoleClaimType || + (claim.Type is "role" or ClaimTypes.Role); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs new file mode 100644 index 00000000000..2cc52cc7edd --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs @@ -0,0 +1,11 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace OrchardCore.Roles; + +public static class ServiceCollectionExtensions +{ + public static IServiceCollection AddRolesCoreServices(this IServiceCollection services) + { + return services.AddSingleton(); + } +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs new file mode 100644 index 00000000000..9b981320cfe --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs @@ -0,0 +1,44 @@ +using System.Collections.Frozen; +using Microsoft.Extensions.Options; +using OrchardCore.Environment.Shell; + +namespace OrchardCore.Roles; + +internal sealed class DefaultSystemRoleNameProvider : ISystemRoleNameProvider +{ + private readonly string _adminRoleName; + + private readonly FrozenSet _systemRoleNames; + + public DefaultSystemRoleNameProvider( + ShellSettings shellSettings, + IOptions options) + { + _adminRoleName = shellSettings["AdminRoleName"]; + + if (string.IsNullOrWhiteSpace(_adminRoleName)) + { + _adminRoleName = options.Value.SystemAdminRoleName; + } + + if (string.IsNullOrWhiteSpace(_adminRoleName)) + { + _adminRoleName = OrchardCoreConstants.Roles.Administrator; + } + + var roles = new HashSet(StringComparer.OrdinalIgnoreCase) + { + OrchardCoreConstants.Roles.Anonymous, + OrchardCoreConstants.Roles.Authenticated, + _adminRoleName, + }; + + _systemRoleNames = roles.ToFrozenSet(StringComparer.OrdinalIgnoreCase); + } + + public ValueTask GetAdminRoleAsync() + => ValueTask.FromResult(_adminRoleName); + + public ValueTask> GetSystemRolesAsync() + => ValueTask.FromResult(_systemRoleNames); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs new file mode 100644 index 00000000000..6ae30673bd0 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs @@ -0,0 +1,22 @@ +namespace OrchardCore.Roles; + +public static class SystemRoleNameProviderExtensions +{ + public static async ValueTask IsAdminRoleAsync(this ISystemRoleNameProvider provider, string roleName) + { + ArgumentNullException.ThrowIfNull(roleName); + + var adminRole = await provider.GetAdminRoleAsync(); + + return roleName.Equals(adminRole, StringComparison.OrdinalIgnoreCase); + } + + public static async ValueTask IsSystemRoleAsync(this ISystemRoleNameProvider provider, string roleName) + { + ArgumentNullException.ThrowIfNull(roleName); + + var roleNames = await provider.GetSystemRolesAsync(); + + return roleNames.Contains(roleName); + } +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleOptions.cs b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleOptions.cs new file mode 100644 index 00000000000..5d97c79ae0b --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleOptions.cs @@ -0,0 +1,5 @@ +namespace OrchardCore.Roles; +public class SystemRoleOptions +{ + public string SystemAdminRoleName { get; set; } +} diff --git a/src/OrchardCore/OrchardCore.Users.Core/OrchardCore.Users.Core.csproj b/src/OrchardCore/OrchardCore.Users.Core/OrchardCore.Users.Core.csproj index dfe002f36c0..a4b769e6d3a 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/OrchardCore.Users.Core.csproj +++ b/src/OrchardCore/OrchardCore.Users.Core/OrchardCore.Users.Core.csproj @@ -21,6 +21,7 @@ + diff --git a/src/OrchardCore/OrchardCore.Users.Core/Services/DefaultUserClaimsPrincipalProviderFactory.cs b/src/OrchardCore/OrchardCore.Users.Core/Services/DefaultUserClaimsPrincipalProviderFactory.cs index 3e9619bf14f..4c360c127b7 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/Services/DefaultUserClaimsPrincipalProviderFactory.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/Services/DefaultUserClaimsPrincipalProviderFactory.cs @@ -3,24 +3,23 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using OrchardCore.Modules; -using OrchardCore.Security; namespace OrchardCore.Users.Services; /// /// Custom implementation of allowing adding claims by implementing the . /// -public class DefaultUserClaimsPrincipalProviderFactory : UserClaimsPrincipalFactory +public class DefaultUserClaimsPrincipalProviderFactory : UserClaimsPrincipalFactory { private readonly IEnumerable _claimsProviders; private readonly ILogger _logger; public DefaultUserClaimsPrincipalProviderFactory( UserManager userManager, - RoleManager roleManager, IOptions identityOptions, IEnumerable claimsProviders, - ILogger logger) : base(userManager, roleManager, identityOptions) + ILogger logger) + : base(userManager, identityOptions) { _claimsProviders = claimsProviders; _logger = logger; diff --git a/src/OrchardCore/OrchardCore.Users.Core/Services/RoleService.cs b/src/OrchardCore/OrchardCore.Users.Core/Services/RoleService.cs index 48340cbfd57..7f8f0dbc7fc 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/Services/RoleService.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/Services/RoleService.cs @@ -8,10 +8,14 @@ namespace OrchardCore.Roles.Services; public class RoleService : IRoleService { private readonly RoleManager _roleManager; + private readonly ISystemRoleNameProvider _systemRoleProvider; - public RoleService(RoleManager roleManager) + public RoleService( + RoleManager roleManager, + ISystemRoleNameProvider systemRoleProvider) { _roleManager = roleManager; + _systemRoleProvider = systemRoleProvider; } public async Task> GetRoleClaimsAsync(string role, CancellationToken cancellationToken = default) @@ -44,4 +48,10 @@ public Task> GetNormalizedRoleNamesAsync() { return Task.FromResult>(_roleManager.Roles.Select(a => _roleManager.NormalizeKey(a.RoleName))); } + + public ValueTask IsAdminRoleAsync(string role) + => _systemRoleProvider.IsAdminRoleAsync(role); + + public ValueTask IsSystemRoleAsync(string role) + => _systemRoleProvider.IsSystemRoleAsync(role); } diff --git a/src/docs/reference/modules/Roles/README.md b/src/docs/reference/modules/Roles/README.md index fec041dd6a0..df512d77dc5 100644 --- a/src/docs/reference/modules/Roles/README.md +++ b/src/docs/reference/modules/Roles/README.md @@ -4,17 +4,20 @@ Enabling the `OrchardCore.Roles` module will allow you to manage the user roles. ## Predefined Roles -Orchard Core come up with the following predefined permission stereotypes: +Orchard Core defines the following predefined permission stereotypes: | Name | Description | | --- | --- | -| `Administrator` | Contains all the administrator users. | -| `Anonymous` | Contains all the non authenticated users. | -| `Authenticated` | Contains all the authenticated users. | -| `Author` | Contains all the users who have the ability to author contents. | -| `Contributor` | Contains all the users who have the ability to contribute to the contents. | -| `Editor` | Contains all the users who have the ability to edit the contents. | -| `Moderator` | Contains all the users who have the ability to moderate the contents. | +| `Administrator` | A **system role** that grants all permissions to the assigned users. | +| `Anonymous` | A **system role** representing all non-authenticated users. | +| `Authenticated` | A **system role** representing all authenticated users. | +| `Author` | Grants users the ability to create content. | +| `Contributor` | Grants users the ability to contribute content. | +| `Editor` | Grants users the ability to edit existing content. | +| `Moderator` | Grants users the ability to moderate content. | + +!!! note + System roles cannot be deleted, and the `Administrator` role cannot be edited. ## Roles Configuration @@ -28,12 +31,14 @@ A sample of a roles configuration step: "Roles": [ { "Name": "Journalist", - "Permissions": [ "PublishContent", "EditContent" ] + "Description" "Journalist Role", + "Permissions": ["PublishContent", "EditContent"] }, { "Name": "Subscriber", - "Permissions": [ ] - }, + "Description" "Subscriber Role", + "Permissions": [] + } ] } ``` diff --git a/src/docs/releases/2.1.0.md b/src/docs/releases/2.1.0.md index 79e188cec71..d9bd134801d 100644 --- a/src/docs/releases/2.1.0.md +++ b/src/docs/releases/2.1.0.md @@ -63,6 +63,41 @@ In the Roles feature, there were previously `AssignRoles` and `AssignRole_{RoleN !!! warning Please review all your recipes and replace occurrences of `AssignRoles` with `AssignRoleToUsers`, and `AssignRole_{RoleName}` with `AssignRoleToUsers_{RoleName}`. +#### Site Owner Permission Deprecated, Administrator Role Retained as System Role + +The `SiteOwner` permission has been deprecated and will be removed in future releases. Instead, the `Administrator` role has been preserved as a system role, similar to `Authenticated` and `Anonymous`. This change was made to improve performance and grant all permissions at the role level, rather than relying on a single super-permission. A fast-track migration automatically assigns the `Administrator` role to users who previously held the `SiteOwner` permission. + +The Recipes feature now checks permissions against the new **Manage Recipes** permission, replacing the deprecated `SiteOwner` permission. + +Lastly, if you'd like to change the admin's role name `Administrator` globally, you can do so through any settings provider. For example, to rename it to `Admin` using `appsettings.json`, add the following configuration: + +```json +"OrchardCore_Roles": { + "AdminRoleName": "Admin" +} +``` + +!!! warning + If the existing `Administrator` role did not previously include the `SiteOwner` permission, a new system admin role will be generated. This role may be named `SiteAdmin` or `SiteAdmin{N}`, where `{N}` ensures uniqueness. Users who were assigned the `SiteOwner` permission will automatically be granted this newly created role. + +#### New Methods Added to `IRoleService` + +Two new methods have been introduced to the `IRoleService` interface: `ValueTask IsAdminRoleAsync(string role)` and `ValueTask IsSystemRoleAsync(string role)`. To prevent breaking changes in the current release, these methods have been provided with default implementations. + +However, if you have a custom implementation of this interface, it is crucial to implement these new methods. In the next major release, the default implementations will be removed, and failing to implement them will result in a breaking change. + +### Recipes Feature + +#### New 'Manage Recipes' Permission Added + +Previously, only users with the `SiteOwner` permission could run recipes. Now, a new `ManageRecipes` permission allows you to grant recipe management capabilities to any role, providing greater flexibility in permission assignment. + +### Themes Feature + +#### Users with 'Apply Theme' Permission Can List Themes + +Previously, only users with the `SiteOwner` permission could list themes. Now, users with the existing `ApplyTheme` permission can also list and apply themes, enhancing theme management capabilities. + ### Azure Communication Services Email Feature #### Azure Communication Services Email Feature Name Update diff --git a/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json b/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json index 580e7d08bb8..dcb325626a3 100644 --- a/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json +++ b/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json @@ -89,17 +89,17 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/test/OrchardCore.Tests.Pages/OrchardCore.Application.Pages/Recipes/pages.recipe.json b/test/OrchardCore.Tests.Pages/OrchardCore.Application.Pages/Recipes/pages.recipe.json index ea329e8fb81..5b2faf258ed 100644 --- a/test/OrchardCore.Tests.Pages/OrchardCore.Application.Pages/Recipes/pages.recipe.json +++ b/test/OrchardCore.Tests.Pages/OrchardCore.Application.Pages/Recipes/pages.recipe.json @@ -76,37 +76,37 @@ "Roles": [ { "Name": "Administrator", - "Description": "Administrator", + "Description": "A system role that grants all permissions to the assigned users.", "Permissions": [] }, { "Name": "Moderator", - "Description": "Moderator", + "Description": "Grants users the ability to moderate content.", "Permissions": [] }, { "Name": "Editor", - "Description": "Editor", + "Description": "Grants users the ability to edit existing content.", "Permissions": [] }, { "Name": "Author", - "Description": "Author", + "Description": "Grants users the ability to create content.", "Permissions": [] }, { "Name": "Contributor", - "Description": "Contributor", + "Description": "Grants users the ability to contribute content.", "Permissions": [] }, { "Name": "Authenticated", - "Description": "Authenticated", + "Description": "A system role representing all authenticated users.", "Permissions": [] }, { "Name": "Anonymous", - "Description": "Anonymous", + "Description": "A system role representing all non-authenticated users.", "Permissions": [] } ] diff --git a/test/OrchardCore.Tests/Apis/Context/AuthenticationContext.cs b/test/OrchardCore.Tests/Apis/Context/AuthenticationContext.cs index 1bbeda67f25..f22bdfad0b4 100644 --- a/test/OrchardCore.Tests/Apis/Context/AuthenticationContext.cs +++ b/test/OrchardCore.Tests/Apis/Context/AuthenticationContext.cs @@ -50,9 +50,6 @@ protected override Task HandleRequirementAsync(AuthorizationHandlerContext conte GetGrantingNamesInternal(requirement.Permission, grantingNames); - // SiteOwner permission grants them all - grantingNames.Add(StandardPermissions.SiteOwner.Name); - if (permissions.Any(p => grantingNames.Contains(p.Name))) { context.Succeed(requirement); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Roles/RolesPermissionsHandlerTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Roles/PermissionHandlerHelperTests.cs similarity index 85% rename from test/OrchardCore.Tests/Modules/OrchardCore.Roles/RolesPermissionsHandlerTests.cs rename to test/OrchardCore.Tests/Modules/OrchardCore.Roles/PermissionHandlerHelperTests.cs index 9ff5929e327..39dfa9ff305 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Roles/RolesPermissionsHandlerTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Roles/PermissionHandlerHelperTests.cs @@ -5,7 +5,7 @@ namespace OrchardCore.Tests.Modules.OrchardCore.Roles; -public class RolesPermissionsHandlerTests +public class PermissionHandlerHelperTests { [Theory] [InlineData("AllowAnonymous", true, true)] @@ -23,7 +23,7 @@ public async Task GrantsRolesPermissions(string required, bool authenticated, bo RoleName = OrchardCoreConstants.Roles.Anonymous, RoleClaims = [ - new() { ClaimType = Permission.ClaimType, ClaimValue = "AllowAnonymous" } + RoleClaim.Create("AllowAnonymous"), ] }, new Role @@ -31,7 +31,7 @@ public async Task GrantsRolesPermissions(string required, bool authenticated, bo RoleName = OrchardCoreConstants.Roles.Authenticated, RoleClaims = [ - new() { ClaimType = Permission.ClaimType, ClaimValue = "AllowAuthenticated" } + RoleClaim.Create("AllowAuthenticated"), ] } ); @@ -44,7 +44,7 @@ public async Task GrantsRolesPermissions(string required, bool authenticated, bo } [Fact] - public async Task DontRevokeExistingGrants() + public async Task DoNotRevokeExistingGrants() { // Arrange var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(new Permission("Required"), ["Other"], true); @@ -76,7 +76,7 @@ public async Task GrantsInheritedPermissions() RoleName = OrchardCoreConstants.Roles.Anonymous, RoleClaims = [ - new() { ClaimType = Permission.ClaimType, ClaimValue = "Implicit2" } + RoleClaim.Create("Implicit2"), ] } ); @@ -91,7 +91,7 @@ public async Task GrantsInheritedPermissions() [Theory] [InlineData("AllowAnonymous", true)] [InlineData("AllowAuthenticated", true)] - public async Task IsCaseIsensitive(string required, bool authenticated) + public async Task IsCaseInsensitive(string required, bool authenticated) { // Arrange var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(new Permission(required), authenticated: authenticated); @@ -102,7 +102,7 @@ public async Task IsCaseIsensitive(string required, bool authenticated) RoleName = OrchardCoreConstants.Roles.Anonymous, RoleClaims = [ - new() { ClaimType = Permission.ClaimType, ClaimValue = "aLlOwAnOnYmOuS" } + RoleClaim.Create("aLlOwAnOnYmOuS"), ] }, new Role @@ -110,7 +110,7 @@ public async Task IsCaseIsensitive(string required, bool authenticated) RoleName = OrchardCoreConstants.Roles.Authenticated, RoleClaims = [ - new() { ClaimType = Permission.ClaimType, ClaimValue = "aLlOwAuThEnTiCaTeD" } + RoleClaim.Create("aLlOwAuThEnTiCaTeD"), ] } ); diff --git a/test/OrchardCore.Tests/OrchardCore.Users/OrchardCore.Users.Core/DefaultUserClaimsPrincipalProviderFactoryTests.cs b/test/OrchardCore.Tests/OrchardCore.Users/OrchardCore.Users.Core/DefaultUserClaimsPrincipalProviderFactoryTests.cs index 055be82e3fb..08e917dda6d 100644 --- a/test/OrchardCore.Tests/OrchardCore.Users/OrchardCore.Users.Core/DefaultUserClaimsPrincipalProviderFactoryTests.cs +++ b/test/OrchardCore.Tests/OrchardCore.Users/OrchardCore.Users.Core/DefaultUserClaimsPrincipalProviderFactoryTests.cs @@ -1,4 +1,3 @@ -using OrchardCore.Security; using OrchardCore.Users; using OrchardCore.Users.Models; using OrchardCore.Users.Services; @@ -24,8 +23,6 @@ public async Task EnsurePrincipalHasExpectedClaims(bool emailVerified) userManager.Setup(m => m.IsEmailConfirmedAsync(user)).ReturnsAsync(emailVerified); } - var roleManager = UsersMockHelper.MockRoleManager().Object; - var options = new Mock>(); options.Setup(a => a.Value).Returns(new IdentityOptions()); @@ -34,7 +31,7 @@ public async Task EnsurePrincipalHasExpectedClaims(bool emailVerified) new EmailClaimsProvider(userManager.Object) }; - var factory = new DefaultUserClaimsPrincipalProviderFactory(userManager.Object, roleManager, options.Object, claimsProviders, null); + var factory = new DefaultUserClaimsPrincipalProviderFactory(userManager.Object, options.Object, claimsProviders, null); //Act var principal = await factory.CreateAsync(user); diff --git a/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs b/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs new file mode 100644 index 00000000000..4d28f1360b9 --- /dev/null +++ b/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs @@ -0,0 +1,82 @@ +using OrchardCore.Environment.Shell; +using OrchardCore.Roles; + +namespace OrchardCore.Tests.Security; + +public class DefaultSystemRoleNameProviderTests +{ + [Fact] + public async Task SystemRoleNamesContains_WhenConstructed_ContainsDefaultAdminRole() + { + // Arrange + var shellSettings = new ShellSettings(); + + var options = new Mock>(); + options.Setup(x => x.Value).Returns(new SystemRoleOptions()); + + var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + + // Assert + var roles = await provider.GetSystemRolesAsync(); + + Assert.Contains(OrchardCoreConstants.Roles.Administrator, roles as IEnumerable); + } + + [Fact] + public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdminRole() + { + // Arrange + var shellSettings = new ShellSettings(); + var configureSystemAdminRoleName = "SystemAdmin"; + + var options = new Mock>(); + options.Setup(x => x.Value).Returns(new SystemRoleOptions + { + SystemAdminRoleName = configureSystemAdminRoleName, + }); + + var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + + // Assert + var roles = await provider.GetSystemRolesAsync(); + + Assert.Contains(configureSystemAdminRoleName, roles as IEnumerable); + Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roles as IEnumerable); + } + + [Fact] + public async Task SystemRoleNamesContains_WhenConstructed_ContainsAppSettingsRole() + { + // Arrange + var shellSettings = new ShellSettings(); + shellSettings["AdminRoleName"] = "Foo"; + + var options = new Mock>(); + options.Setup(x => x.Value).Returns(new SystemRoleOptions()); + + var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + // Assert + var roles = await provider.GetSystemRolesAsync(); + + Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roles as IEnumerable); + Assert.Contains("Foo", roles as IEnumerable); + } + + [Fact] + public async Task SystemRoleNamesContains_WhenCalled_ReturnsCaseInsensitive() + { + // Arrange + var shellSettings = new ShellSettings(); + shellSettings["AdminRoleName"] = "Foo"; + + var options = new Mock>(); + options.Setup(x => x.Value).Returns(new SystemRoleOptions()); + + var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + + // Assert + var roles = await provider.GetSystemRolesAsync(); + + Assert.Contains("fOo", roles as IEnumerable); + } +} diff --git a/test/OrchardCore.Tests/Security/PermissionHandlerHelper.cs b/test/OrchardCore.Tests/Security/PermissionHandlerHelper.cs index c0e25b4bc81..06a44d0dc90 100644 --- a/test/OrchardCore.Tests/Security/PermissionHandlerHelper.cs +++ b/test/OrchardCore.Tests/Security/PermissionHandlerHelper.cs @@ -6,17 +6,20 @@ namespace OrchardCore.Tests.Security; public static class PermissionHandlerHelper { public static AuthorizationHandlerContext CreateTestAuthorizationHandlerContext(Permission required, string[] allowed = null, bool authenticated = false, object resource = null) + => CreateTestAuthorizationHandlerContext(required, allowed?.Select(x => new Claim(Permission.ClaimType, x)) ?? [], authenticated, resource); + + public static AuthorizationHandlerContext CreateTestAuthorizationHandlerContext(Permission required, IEnumerable allowed, bool authenticated, object resource = null) { - var identity = authenticated ? new ClaimsIdentity("Test") : new ClaimsIdentity(); + var identity = authenticated + ? new ClaimsIdentity("Test") + : new ClaimsIdentity(); if (allowed != null) { - foreach (var permissionName in allowed) + foreach (var permission in allowed) { - var permission = new Permission(permissionName); identity.AddClaim(permission); } - } var principal = new ClaimsPrincipal(identity); diff --git a/test/OrchardCore.Tests/Security/PermissionHandlerTests.cs b/test/OrchardCore.Tests/Security/PermissionHandlerTests.cs index 1b0212d55f6..27b0e82749e 100644 --- a/test/OrchardCore.Tests/Security/PermissionHandlerTests.cs +++ b/test/OrchardCore.Tests/Security/PermissionHandlerTests.cs @@ -23,7 +23,7 @@ public async Task GrantsClaimsPermissions(string required, bool success) } [Fact] - public async Task DontRevokeExistingGrants() + public async Task DoNotRevokeExistingGrants() { // Arrange var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(new Permission("Required"), ["Other"], true); @@ -39,7 +39,7 @@ public async Task DontRevokeExistingGrants() } [Fact] - public async Task DontHandleNonAuthenticated() + public async Task DoNotHandleNonAuthenticated() { // Arrange var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(new Permission("Allowed"), ["Allowed"]); diff --git a/test/OrchardCore.Tests/Security/RolesPermissionHandlerTests.cs b/test/OrchardCore.Tests/Security/RolesPermissionHandlerTests.cs new file mode 100644 index 00000000000..0b5c38990d5 --- /dev/null +++ b/test/OrchardCore.Tests/Security/RolesPermissionHandlerTests.cs @@ -0,0 +1,60 @@ +using OrchardCore.Roles.Core; +using OrchardCore.Security.Permissions; +using OrchardCore.Security.Services; + +namespace OrchardCore.Tests.Security; + +public class RolesPermissionHandlerTests +{ + [Fact] + public async Task HandleAsync_WhenCalled_AdminsShouldBeGrantedPermissions() + { + // Arrange + var adminRolePermission = new Claim("role", "Administrator"); + var required = new Permission("Required", "Foo"); + + var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(required, [adminRolePermission], true); + + var permissionHandler = GetRolesPermissionHandler(true); + + // Act + await permissionHandler.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Fact] + public async Task HandleAsync_WhenCalled_NonAdminsShouldNotBeGrantedPermissions() + { + // Arrange + var adminRolePermission = new Claim("role", "Editor"); + var required = new Permission("Required", "Foo"); + + var context = PermissionHandlerHelper.CreateTestAuthorizationHandlerContext(required, [adminRolePermission], true); + + var permissionHandler = GetRolesPermissionHandler(false); + + // Act + await permissionHandler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } + + private static RolesPermissionHandler GetRolesPermissionHandler(bool userIsAdmin) + { + var options = new Mock>(); + + options.Setup(x => x.Value).Returns(new IdentityOptions()); + + var roleService = new Mock(); + + roleService.Setup(x => x.IsAdminRoleAsync(It.IsAny())) + .ReturnsAsync(userIsAdmin); + + var permissionHandler = new RolesPermissionHandler(roleService.Object, options.Object); + + return permissionHandler; + } +}