From bd65eed4b34e712270c660976f5d0df5b566addb Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sun, 12 Jan 2025 07:39:14 +0300 Subject: [PATCH 01/28] Create system roles by default --- .../OrchardCore.Roles/Startup.cs | 5 +++ .../ApplicationBuilderExtensions.cs | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs index 6adff2c006c..236fd1d77e6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs @@ -1,5 +1,7 @@ using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -79,4 +81,7 @@ public override void ConfigureServices(IServiceCollection services) services.AddScoped(sp => sp.GetRequiredService()); services.AddScoped(sp => sp.GetRequiredService()); } + + public override void Configure(IApplicationBuilder app, IEndpointRouteBuilder routes, IServiceProvider serviceProvider) + => app.UseSystemRoles(); } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs new file mode 100644 index 00000000000..59c9be9bfa4 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs @@ -0,0 +1,40 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using OrchardCore.Security; + +namespace Microsoft.AspNetCore.Builder; + +public static class ApplicationBuilderExtensions +{ + private static readonly IRole[] _systemRoles = + [ + new Role + { + RoleName = "Administrator", + RoleDescription = "A system role that grants all permissions to the assigned users." + }, + new Role + { + RoleName = "Authenticated", + RoleDescription = "A system role representing all authenticated users." + }, + new Role + { + RoleName = "Anonymous", + RoleDescription = "A system role representing all non-authenticated users." + } + ]; + + public static IApplicationBuilder UseSystemRoles(this IApplicationBuilder app) + { + using var scope = app.ApplicationServices.CreateAsyncScope(); + var roleManager = scope.ServiceProvider.GetService>(); + + foreach (var role in _systemRoles) + { + roleManager.CreateAsync(role); + } + + return app; + } +} From 151ad09b543b2d54eeddd92c72e4d243878530e0 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sun, 12 Jan 2025 07:50:45 +0300 Subject: [PATCH 02/28] Add missing roles --- .../Extensions/ApplicationBuilderExtensions.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs index 59c9be9bfa4..9e9fad8772a 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs @@ -22,6 +22,21 @@ public static class ApplicationBuilderExtensions { RoleName = "Anonymous", RoleDescription = "A system role representing all non-authenticated users." + }, + new Role + { + RoleName = "Moderator", + RoleDescription = "Grants users the ability to moderate content." + }, + new Role + { + RoleName = "Author", + RoleDescription = "Grants users the ability to create content." + }, + new Role + { + RoleName = "Contributor", + RoleDescription = "Grants users the ability to contribute content." } ]; From 6117d1988669c99e5a8c085a46075b8c084b923d Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sun, 12 Jan 2025 07:51:07 +0300 Subject: [PATCH 03/28] Remove roles from *.recipe.json --- .../TheAdmin/Recipes/blank.recipe.json | 40 ----------------- .../TheAdmin/Recipes/headless.recipe.json | 44 ------------------- .../TheAgencyTheme/Recipes/agency.recipe.json | 40 ----------------- .../TheBlogTheme/Recipes/blog.recipe.json | 40 ----------------- .../Recipes/comingsoon.recipe.json | 40 ----------------- 5 files changed, 204 deletions(-) diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json index 5b170e73669..a9dd2357acb 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json @@ -65,46 +65,6 @@ "name": "themes", "admin": "TheAdmin", "site": "" - }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Moderator", - "Description": "Grants users the ability to moderate content.", - "Permissions": [] - }, - { - "Name": "Editor", - "Description": "Grants users the ability to edit existing content.", - "Permissions": [] - }, - { - "Name": "Author", - "Description": "Grants users the ability to create content.", - "Permissions": [] - }, - { - "Name": "Contributor", - "Description": "Grants users the ability to contribute content.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "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 65a4b0a24d3..f5c3ab88d2d 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json @@ -61,50 +61,6 @@ "TheAdmin" ] }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Moderator", - "Description": "Grants users the ability to moderate content.", - "Permissions": [] - }, - { - "Name": "Editor", - "Description": "Grants users the ability to edit existing content.", - "Permissions": [] - }, - { - "Name": "Author", - "Description": "Grants users the ability to create content.", - "Permissions": [] - }, - { - "Name": "Contributor", - "Description": "Grants users the ability to contribute content.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [ - "ViewContent", - "ExecuteGraphQL", - "ExecuteApiAll" - ] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json index 9c99ed5b29d..e2c9fccd2b7 100644 --- a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json +++ b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json @@ -70,46 +70,6 @@ "admin": "TheAdmin", "site": "TheAgencyTheme" }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Moderator", - "Description": "Grants users the ability to moderate content.", - "Permissions": [] - }, - { - "Name": "Editor", - "Description": "Grants users the ability to edit existing content.", - "Permissions": [] - }, - { - "Name": "Author", - "Description": "Grants users the ability to create content.", - "Permissions": [] - }, - { - "Name": "Contributor", - "Description": "Grants users the ability to contribute content.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json index ceabdaee507..1b42aa67a3f 100644 --- a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json +++ b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json @@ -84,46 +84,6 @@ "admin": "TheAdmin", "site": "TheBlogTheme" }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Moderator", - "Description": "Grants users the ability to moderate content.", - "Permissions": [] - }, - { - "Name": "Editor", - "Description": "Grants users the ability to edit existing content.", - "Permissions": [] - }, - { - "Name": "Author", - "Description": "Grants users the ability to create content.", - "Permissions": [] - }, - { - "Name": "Contributor", - "Description": "Grants users the ability to contribute content.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json index bc8b03a9ebe..29f7348c055 100644 --- a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json +++ b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json @@ -67,46 +67,6 @@ "admin": "TheAdmin", "site": "TheComingSoonTheme" }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Moderator", - "Description": "Grants users the ability to moderate content.", - "Permissions": [] - }, - { - "Name": "Editor", - "Description": "Grants users the ability to edit existing content.", - "Permissions": [] - }, - { - "Name": "Author", - "Description": "Grants users the ability to create content.", - "Permissions": [] - }, - { - "Name": "Contributor", - "Description": "Grants users the ability to contribute content.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { From da197040f1053c2fb3441fa3db634d018b24ba4b Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Mon, 13 Jan 2025 03:06:36 +0300 Subject: [PATCH 04/28] Address feedback --- .../Migrations/RolesMigrations.cs | 18 ++++++ .../OrchardCore.Roles/Startup.cs | 5 -- .../TheAdmin/Recipes/blank.recipe.json | 25 +++++++++ .../TheAdmin/Recipes/headless.recipe.json | 25 +++++++++ .../TheAgencyTheme/Recipes/agency.recipe.json | 25 +++++++++ .../TheBlogTheme/Recipes/blog.recipe.json | 25 +++++++++ .../Recipes/comingsoon.recipe.json | 25 +++++++++ .../ApplicationBuilderExtensions.cs | 55 ------------------- 8 files changed, 143 insertions(+), 60 deletions(-) delete mode 100644 src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs index eae78a2ce3a..ab2098f10b5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs @@ -16,13 +16,19 @@ public sealed class RolesMigrations : DataMigration private static readonly string _alternativeAdminRoleName = "SiteOwner"; private readonly SystemRoleOptions _systemRoleOptions; + private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly RoleManager _roleManager; private readonly ILogger _logger; public RolesMigrations( IOptions systemRoleOptions, + ISystemRoleNameProvider systemRoleNameProvider, + RoleManager roleManager, ILogger logger) { _systemRoleOptions = systemRoleOptions.Value; + _systemRoleNameProvider = systemRoleNameProvider; + _roleManager = roleManager; _logger = logger; } @@ -126,6 +132,18 @@ await HttpBackgroundJob.ExecuteAfterEndOfRequestAsync("MigrateAdminUsersToNewAdm return 1; } + public async Task UpdateFrom1Async() + { + var systemRoles = await _systemRoleNameProvider.GetSystemRolesAsync(); + + foreach (var role in systemRoles) + { + await _roleManager.CreateAsync(new Role { RoleName = role }); + } + + return 2; + } + private static string GenerateNewAdminRoleName(List roles) { var counter = 1; diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs index 236fd1d77e6..6adff2c006c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs @@ -1,7 +1,5 @@ using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Identity; -using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -81,7 +79,4 @@ public override void ConfigureServices(IServiceCollection services) services.AddScoped(sp => sp.GetRequiredService()); services.AddScoped(sp => sp.GetRequiredService()); } - - public override void Configure(IApplicationBuilder app, IEndpointRouteBuilder routes, IServiceProvider serviceProvider) - => app.UseSystemRoles(); } diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json index a9dd2357acb..0e9cdc2a774 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json @@ -65,6 +65,31 @@ "name": "themes", "admin": "TheAdmin", "site": "" + }, + { + "name": "Roles", + "Roles": [ + { + "Name": "Moderator", + "Description": "Grants users the ability to moderate content.", + "Permissions": [] + }, + { + "Name": "Editor", + "Description": "Grants users the ability to edit existing content.", + "Permissions": [] + }, + { + "Name": "Author", + "Description": "Grants users the ability to create content.", + "Permissions": [] + }, + { + "Name": "Contributor", + "Description": "Grants users the ability to contribute content.", + "Permissions": [] + } + ] } ] } diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json index f5c3ab88d2d..385c6f45259 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json @@ -61,6 +61,31 @@ "TheAdmin" ] }, + { + "name": "Roles", + "Roles": [ + { + "Name": "Moderator", + "Description": "Grants users the ability to moderate content.", + "Permissions": [] + }, + { + "Name": "Editor", + "Description": "Grants users the ability to edit existing content.", + "Permissions": [] + }, + { + "Name": "Author", + "Description": "Grants users the ability to create content.", + "Permissions": [] + }, + { + "Name": "Contributor", + "Description": "Grants users the ability to contribute content.", + "Permissions": [] + } + ] + }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json index e2c9fccd2b7..c8dd925d27e 100644 --- a/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json +++ b/src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json @@ -70,6 +70,31 @@ "admin": "TheAdmin", "site": "TheAgencyTheme" }, + { + "name": "Roles", + "Roles": [ + { + "Name": "Moderator", + "Description": "Grants users the ability to moderate content.", + "Permissions": [] + }, + { + "Name": "Editor", + "Description": "Grants users the ability to edit existing content.", + "Permissions": [] + }, + { + "Name": "Author", + "Description": "Grants users the ability to create content.", + "Permissions": [] + }, + { + "Name": "Contributor", + "Description": "Grants users the ability to contribute content.", + "Permissions": [] + } + ] + }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json index 1b42aa67a3f..662f6ecd704 100644 --- a/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json +++ b/src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json @@ -84,6 +84,31 @@ "admin": "TheAdmin", "site": "TheBlogTheme" }, + { + "name": "Roles", + "Roles": [ + { + "Name": "Moderator", + "Description": "Grants users the ability to moderate content.", + "Permissions": [] + }, + { + "Name": "Editor", + "Description": "Grants users the ability to edit existing content.", + "Permissions": [] + }, + { + "Name": "Author", + "Description": "Grants users the ability to create content.", + "Permissions": [] + }, + { + "Name": "Contributor", + "Description": "Grants users the ability to contribute content.", + "Permissions": [] + } + ] + }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json index 29f7348c055..03a0016101a 100644 --- a/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json +++ b/src/OrchardCore.Themes/TheComingSoonTheme/Recipes/comingsoon.recipe.json @@ -67,6 +67,31 @@ "admin": "TheAdmin", "site": "TheComingSoonTheme" }, + { + "name": "Roles", + "Roles": [ + { + "Name": "Moderator", + "Description": "Grants users the ability to moderate content.", + "Permissions": [] + }, + { + "Name": "Editor", + "Description": "Grants users the ability to edit existing content.", + "Permissions": [] + }, + { + "Name": "Author", + "Description": "Grants users the ability to create content.", + "Permissions": [] + }, + { + "Name": "Contributor", + "Description": "Grants users the ability to contribute content.", + "Permissions": [] + } + ] + }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs deleted file mode 100644 index 9e9fad8772a..00000000000 --- a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/ApplicationBuilderExtensions.cs +++ /dev/null @@ -1,55 +0,0 @@ -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.DependencyInjection; -using OrchardCore.Security; - -namespace Microsoft.AspNetCore.Builder; - -public static class ApplicationBuilderExtensions -{ - private static readonly IRole[] _systemRoles = - [ - new Role - { - RoleName = "Administrator", - RoleDescription = "A system role that grants all permissions to the assigned users." - }, - new Role - { - RoleName = "Authenticated", - RoleDescription = "A system role representing all authenticated users." - }, - new Role - { - RoleName = "Anonymous", - RoleDescription = "A system role representing all non-authenticated users." - }, - new Role - { - RoleName = "Moderator", - RoleDescription = "Grants users the ability to moderate content." - }, - new Role - { - RoleName = "Author", - RoleDescription = "Grants users the ability to create content." - }, - new Role - { - RoleName = "Contributor", - RoleDescription = "Grants users the ability to contribute content." - } - ]; - - public static IApplicationBuilder UseSystemRoles(this IApplicationBuilder app) - { - using var scope = app.ApplicationServices.CreateAsyncScope(); - var roleManager = scope.ServiceProvider.GetService>(); - - foreach (var role in _systemRoles) - { - roleManager.CreateAsync(role); - } - - return app; - } -} From a79ade298654e6481890a5b75b5ab1d44c68c587 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Mon, 13 Jan 2025 03:13:05 +0300 Subject: [PATCH 05/28] Create a separate data migration --- .../Migrations/RolesMigrations.cs | 18 --------- .../Migrations/SystemRolesMigrations.cs | 37 +++++++++++++++++++ .../OrchardCore.Roles/Startup.cs | 5 ++- 3 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs index ab2098f10b5..eae78a2ce3a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/RolesMigrations.cs @@ -16,19 +16,13 @@ public sealed class RolesMigrations : DataMigration private static readonly string _alternativeAdminRoleName = "SiteOwner"; private readonly SystemRoleOptions _systemRoleOptions; - private readonly ISystemRoleNameProvider _systemRoleNameProvider; - private readonly RoleManager _roleManager; private readonly ILogger _logger; public RolesMigrations( IOptions systemRoleOptions, - ISystemRoleNameProvider systemRoleNameProvider, - RoleManager roleManager, ILogger logger) { _systemRoleOptions = systemRoleOptions.Value; - _systemRoleNameProvider = systemRoleNameProvider; - _roleManager = roleManager; _logger = logger; } @@ -132,18 +126,6 @@ await HttpBackgroundJob.ExecuteAfterEndOfRequestAsync("MigrateAdminUsersToNewAdm return 1; } - public async Task UpdateFrom1Async() - { - var systemRoles = await _systemRoleNameProvider.GetSystemRolesAsync(); - - foreach (var role in systemRoles) - { - await _roleManager.CreateAsync(new Role { RoleName = role }); - } - - return 2; - } - private static string GenerateNewAdminRoleName(List roles) { var counter = 1; diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs new file mode 100644 index 00000000000..5bcb5511be5 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs @@ -0,0 +1,37 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using OrchardCore.Data.Migration; +using OrchardCore.Security; + +namespace OrchardCore.Roles.Migrations; + +public sealed class SystemRolesMigrations : DataMigration +{ + private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly RoleManager _roleManager; + private readonly ILogger _logger; + + public SystemRolesMigrations( + ISystemRoleNameProvider systemRoleNameProvider, + RoleManager roleManager, + ILogger logger) + { + _systemRoleNameProvider = systemRoleNameProvider; + _roleManager = roleManager; + _logger = logger; + } + + public async Task CreateAsync() + { + var systemRoles = await _systemRoleNameProvider.GetSystemRolesAsync(); + + foreach (var role in systemRoles) + { + await _roleManager.CreateAsync(new Role { RoleName = role }); + } + + _logger.LogInformation("The system roles have been created successfully."); + + return 1; + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs index 6adff2c006c..b35f7f3d3ea 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs @@ -33,7 +33,10 @@ public Startup(IShellConfiguration shellConfiguration) public override void ConfigureServices(IServiceCollection services) { services.AddScoped(); - services.AddDataMigration(); + + services.AddDataMigration() + .AddDataMigration(); + services.AddScoped(); services.Replace(ServiceDescriptor.Scoped>(sp => sp.GetRequiredService())); services.Replace(ServiceDescriptor.Scoped>(sp => sp.GetRequiredService())); From 5e21655ff10d7d3047b07d8a1f9d838527605e53 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Mon, 13 Jan 2025 08:20:46 +0300 Subject: [PATCH 06/28] Remove system roles from migrations.recipe.json --- .../cms-tests/Recipes/migrations.recipe.json | 20 ------------------- 1 file changed, 20 deletions(-) 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 dcb325626a3..09b72937861 100644 --- a/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json +++ b/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json @@ -84,26 +84,6 @@ "admin": "TheAdmin", "site": "TheBlogTheme" }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { From e5e0c0d6155621b457eddf64dd7dc59c59204a81 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Mon, 13 Jan 2025 18:38:37 +0300 Subject: [PATCH 07/28] Address feedback --- .../Migrations/SystemRolesMigrations.cs | 5 ++++- .../TheAdmin/Recipes/headless.recipe.json | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs index 5bcb5511be5..6bccab0f1b5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs @@ -27,7 +27,10 @@ public async Task CreateAsync() foreach (var role in systemRoles) { - await _roleManager.CreateAsync(new Role { RoleName = role }); + if (await _roleManager.FindByNameAsync(role) is null) + { + await _roleManager.CreateAsync(new Role { RoleName = role }); + } } _logger.LogInformation("The system roles have been created successfully."); diff --git a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json index 385c6f45259..a050cdf8920 100644 --- a/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json +++ b/src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json @@ -83,6 +83,15 @@ "Name": "Contributor", "Description": "Grants users the ability to contribute content.", "Permissions": [] + }, + { + "Name": "Authenticated", + "Description": "A system role representing all authenticated users.", + "Permissions": [ + "ViewContent", + "ExecuteGraphQL", + "ExecuteApiAll" + ] } ] }, From 5b70ef24395de18779c9313f044279d9e74e5ec9 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Tue, 14 Jan 2025 06:14:29 +0300 Subject: [PATCH 08/28] Remove roles from pages.recipe.json --- .../Recipes/pages.recipe.json | 15 --------------- 1 file changed, 15 deletions(-) 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 5b2faf258ed..3ca773de57c 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 @@ -74,11 +74,6 @@ { "name": "Roles", "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, { "Name": "Moderator", "Description": "Grants users the ability to moderate content.", @@ -98,16 +93,6 @@ "Name": "Contributor", "Description": "Grants users the ability to contribute content.", "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] } ] } From 784e91209f9cd0d63900eef945971259bf3267cb Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Wed, 15 Jan 2025 14:19:05 +0300 Subject: [PATCH 09/28] Introduce ISystemRoleProvider --- .../Migrations/SystemRolesMigrations.cs | 16 +++++---- .../OrchardCore.Roles/Recipes/RolesStep.cs | 8 ++--- .../Services/RoleClaimsProvider.cs | 7 ++-- .../OrchardCore.Roles/Services/RoleStore.cs | 4 +-- .../OrchardCore.Roles/Services/RoleUpdater.cs | 9 ++--- .../Services/SuperUserHandler.cs | 11 +++--- .../ISystemRoleNameProvider.cs | 1 + .../ISystemRoleProvider.cs | 8 +++++ .../DefaultSystemRoleProvider.cs | 35 +++++++++++++++++++ .../SystemRoleProviderExtensions.cs | 27 ++++++++++++++ .../ServiceCollectionExtensions.cs | 1 + .../Services/RoleService.cs | 6 ++-- 12 files changed, 107 insertions(+), 26 deletions(-) create mode 100644 src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs create mode 100644 src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs index 6bccab0f1b5..80a085e31e2 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs @@ -7,29 +7,31 @@ namespace OrchardCore.Roles.Migrations; public sealed class SystemRolesMigrations : DataMigration { - private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly ISystemRoleProvider _systemRoleProvider; private readonly RoleManager _roleManager; private readonly ILogger _logger; public SystemRolesMigrations( - ISystemRoleNameProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleProvider, RoleManager roleManager, ILogger logger) { - _systemRoleNameProvider = systemRoleNameProvider; + _systemRoleProvider = systemRoleProvider; _roleManager = roleManager; _logger = logger; } public async Task CreateAsync() { - var systemRoles = await _systemRoleNameProvider.GetSystemRolesAsync(); + var systemRoles = await _systemRoleProvider.GetSystemRolesAsync(); - foreach (var role in systemRoles) + foreach (var systemRole in systemRoles) { - if (await _roleManager.FindByNameAsync(role) is null) + var role = await _roleManager.FindByNameAsync(systemRole.RoleName); + + if (role is null) { - await _roleManager.CreateAsync(new Role { RoleName = role }); + await _roleManager.CreateAsync(role); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs index f6031d9b0ed..a5d56117934 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs @@ -13,15 +13,15 @@ namespace OrchardCore.Roles.Recipes; public sealed class RolesStep : NamedRecipeStepHandler { private readonly RoleManager _roleManager; - private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly ISystemRoleProvider _systemRoleProvider; public RolesStep( RoleManager roleManager, - ISystemRoleNameProvider systemRoleNameProvider) + ISystemRoleProvider systemRoleProvider) : base("Roles") { _roleManager = roleManager; - _systemRoleNameProvider = systemRoleNameProvider; + _systemRoleProvider = systemRoleProvider; } protected override async Task HandleAsync(RecipeExecutionContext context) @@ -59,7 +59,7 @@ protected override async Task HandleAsync(RecipeExecutionContext context) r.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); } - if (!await _systemRoleNameProvider.IsAdminRoleAsync(roleName)) + if (!await _systemRoleProvider.IsAdminRoleAsync(roleName)) { if (roleEntry.PermissionBehavior == PermissionBehavior.Remove) { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs index 5e423fef13b..263a151c83c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs @@ -11,18 +11,19 @@ public class RoleClaimsProvider : IUserClaimsProvider { private readonly UserManager _userManager; private readonly RoleManager _roleManager; - private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly ISystemRoleProvider _systemRoleProvider; private readonly IdentityOptions _identityOptions; public RoleClaimsProvider( UserManager userManager, RoleManager roleManager, ISystemRoleNameProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleProvider, IOptions identityOptions) { _userManager = userManager; _roleManager = roleManager; - _systemRoleNameProvider = systemRoleNameProvider; + _systemRoleProvider = systemRoleProvider; _identityOptions = identityOptions.Value; } @@ -39,7 +40,7 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims) foreach (var roleName in roleNames) { - if (await _systemRoleNameProvider.IsAdminRoleAsync(roleName)) + if (await _systemRoleProvider.IsAdminRoleAsync(roleName)) { isAdministrator = true; break; diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs index 9d427afcb79..cbc0a919c01 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs @@ -13,7 +13,7 @@ namespace OrchardCore.Roles.Services; public class RoleStore : IRoleClaimStore, IQueryableRoleStore { private readonly IServiceProvider _serviceProvider; - private readonly ISystemRoleNameProvider _systemRoleProvider; + private readonly ISystemRoleProvider _systemRoleProvider; private readonly IDocumentManager _documentManager; protected readonly IStringLocalizer S; private readonly ILogger _logger; @@ -22,7 +22,7 @@ public class RoleStore : IRoleClaimStore, IQueryableRoleStore public RoleStore( IServiceProvider serviceProvider, - ISystemRoleNameProvider systemRoleProvider, + ISystemRoleProvider systemRoleProvider, IDocumentManager documentManager, IStringLocalizer stringLocalizer, ILogger logger) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs index 140c5f193c6..557f9ad4111 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs @@ -15,7 +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 ISystemRoleProvider _systemRoleProvider; private readonly IEnumerable _permissionProviders; private readonly ITypeFeatureProvider _typeFeatureProvider; private readonly ILogger _logger; @@ -26,7 +26,8 @@ public RoleUpdater( ShellDescriptor shellDescriptor, IExtensionManager extensionManager, IDocumentManager documentManager, - ISystemRoleNameProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleProvider, IEnumerable permissionProviders, ITypeFeatureProvider typeFeatureProvider, ILogger logger) @@ -34,7 +35,7 @@ public RoleUpdater( _shellDescriptor = shellDescriptor; _extensionManager = extensionManager; _documentManager = documentManager; - _systemRoleNameProvider = systemRoleNameProvider; + _systemRoleProvider = systemRoleProvider; _permissionProviders = permissionProviders; _typeFeatureProvider = typeFeatureProvider; _logger = logger; @@ -204,7 +205,7 @@ private Task UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable UpdatePermissionsAsync(Role role, IEnumerable permissions) { - if (await _systemRoleNameProvider.IsAdminRoleAsync(role.RoleName)) + if (await _systemRoleProvider.IsAdminRoleAsync(role.RoleName)) { // Don't update claims for admin role. return true; diff --git a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs index f6c9c37c67b..209877db7d7 100644 --- a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs @@ -11,14 +11,15 @@ namespace OrchardCore.Settings.Services; public class SuperUserHandler : IAuthorizationHandler { private readonly ISiteService _siteService; - private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly ISystemRoleProvider _systemRoleProvider; public SuperUserHandler( ISiteService siteService, - ISystemRoleNameProvider systemRoleNameProvider) + ISystemRoleNameProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleProvider) { _siteService = siteService; - _systemRoleNameProvider = systemRoleNameProvider; + _systemRoleProvider = systemRoleProvider; } public async Task HandleAsync(AuthorizationHandlerContext context) @@ -30,7 +31,9 @@ public async Task HandleAsync(AuthorizationHandlerContext context) return; } - if (user.IsInRole(await _systemRoleNameProvider.GetAdminRoleAsync())) + var adminRole = await _systemRoleProvider.GetAdminRoleAsync(); + + if (user.IsInRole(adminRole.RoleName)) { SucceedAllRequirements(context); diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs index 7b7190acd8c..30893c4a9ad 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs @@ -2,6 +2,7 @@ namespace OrchardCore.Roles; +[Obsolete("This interface has been deprecated, use ISystemRoleNameProvider instead.")] public interface ISystemRoleNameProvider { ValueTask GetAdminRoleAsync(); diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs new file mode 100644 index 00000000000..86c7aec38ab --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs @@ -0,0 +1,8 @@ +using OrchardCore.Security; + +namespace OrchardCore.Roles; + +public interface ISystemRoleProvider +{ + ValueTask> GetSystemRolesAsync(); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs new file mode 100644 index 00000000000..a6ab913f4c1 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -0,0 +1,35 @@ +using OrchardCore.Environment.Shell; +using OrchardCore.Security; + +namespace OrchardCore.Roles; + +public class DefaultSystemRoleProvider : ISystemRoleProvider +{ + private readonly IEnumerable _systemRoles; + + public DefaultSystemRoleProvider(ShellSettings shellSettings) + { + _systemRoles = [ + new Role + { + RoleName = string.IsNullOrEmpty(shellSettings["AdminRoleName"]) + ? OrchardCoreConstants.Roles.Administrator + : shellSettings["AdminRoleName"], + RoleDescription = "A system role that grants all permissions to the assigned users." + }, + new Role + { + RoleName = "Authenticated", + RoleDescription = "A system role representing all authenticated users." + }, + new Role + { + RoleName = "Anonymous", + RoleDescription = "A system role representing all non-authenticated users." + } + ]; + } + + public ValueTask> GetSystemRolesAsync() + => ValueTask.FromResult(_systemRoles); +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs new file mode 100644 index 00000000000..328c72c7107 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs @@ -0,0 +1,27 @@ +using OrchardCore.Security; + +namespace OrchardCore.Roles; + +public static class SystemRoleProviderExtensions +{ + public static async ValueTask GetAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider) + => (await systemRoleNameProvider.GetSystemRolesAsync()).First(); + + public static async ValueTask IsSystemRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) + { + ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); + + var systemRoles = await systemRoleNameProvider.GetSystemRolesAsync(); + + return systemRoles.Any(role => role.RoleName == name); + } + + public static async ValueTask IsAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) + { + ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); + + var adminRole = await systemRoleNameProvider.GetAdminRoleAsync(); + + return adminRole.RoleName == name; + } +} diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs index f27ce780e99..b269c5fb3e0 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs @@ -8,6 +8,7 @@ public static class ServiceCollectionExtensions public static IServiceCollection AddRolesCoreServices(this IServiceCollection services) { services.TryAddSingleton(); + services.TryAddSingleton(); return services; } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs index 7f8f0dbc7fc..7c4c447a1d7 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs @@ -8,11 +8,13 @@ namespace OrchardCore.Roles.Services; public class RoleService : IRoleService { private readonly RoleManager _roleManager; - private readonly ISystemRoleNameProvider _systemRoleProvider; + private readonly ISystemRoleNameProvider _systemRoleNameProvider; + private readonly ISystemRoleProvider _systemRoleProvider; public RoleService( RoleManager roleManager, - ISystemRoleNameProvider systemRoleProvider) + ISystemRoleNameProvider systemRoleNameProvider, + ISystemRoleProvider systemRoleProvider) { _roleManager = roleManager; _systemRoleProvider = systemRoleProvider; From 7ee0c376c68116ca9a1159fd7c90e3500431b0e2 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Wed, 15 Jan 2025 15:11:42 +0300 Subject: [PATCH 10/28] Remove warnings --- .../Services/RoleClaimsProvider.cs | 4 ++ .../OrchardCore.Roles/Services/RoleUpdater.cs | 2 + .../Services/SuperUserHandler.cs | 4 ++ .../SystemRoleProviderExtensions.cs | 4 +- .../DefaultSystemRoleProvider.cs | 18 ++++-- .../ServiceCollectionExtensions.cs | 4 ++ .../Services/DefaultSystemRoleNameProvider.cs | 1 + .../Services/RoleService.cs | 5 +- .../SystemRoleNameProviderExtensions.cs | 4 ++ .../DefaultSystemRoleNameProviderTests.cs | 61 ++++++++----------- 10 files changed, 64 insertions(+), 43 deletions(-) rename src/OrchardCore/{OrchardCore.Roles.Core => OrchardCore.Roles.Abstractions}/Extensions/SystemRoleProviderExtensions.cs (81%) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs index 263a151c83c..229857838f6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs @@ -17,7 +17,11 @@ public class RoleClaimsProvider : IUserClaimsProvider public RoleClaimsProvider( UserManager userManager, RoleManager roleManager, +#pragma warning disable CS0618 // Type or member is obsolete +#pragma warning disable IDE0060 // Remove unused parameter ISystemRoleNameProvider systemRoleNameProvider, +#pragma warning restore IDE0060 // Remove unused parameter +#pragma warning restore CS0618 // Type or member is obsolete ISystemRoleProvider systemRoleProvider, IOptions identityOptions) { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs index 557f9ad4111..97bef939cc9 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs @@ -26,7 +26,9 @@ public RoleUpdater( ShellDescriptor shellDescriptor, IExtensionManager extensionManager, IDocumentManager documentManager, +#pragma warning disable IDE0060 // Remove unused parameter ISystemRoleProvider systemRoleNameProvider, +#pragma warning restore IDE0060 // Remove unused parameter ISystemRoleProvider systemRoleProvider, IEnumerable permissionProviders, ITypeFeatureProvider typeFeatureProvider, diff --git a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs index 209877db7d7..2190cd0e352 100644 --- a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs @@ -15,7 +15,11 @@ public class SuperUserHandler : IAuthorizationHandler public SuperUserHandler( ISiteService siteService, +#pragma warning disable CS0618 // Type or member is obsolete +#pragma warning disable IDE0060 // Remove unused parameter ISystemRoleNameProvider systemRoleNameProvider, +#pragma warning restore IDE0060 // Remove unused parameter +#pragma warning restore CS0618 // Type or member is obsolete ISystemRoleProvider systemRoleProvider) { _siteService = siteService; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs similarity index 81% rename from src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs rename to src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index 328c72c7107..633b5a983a7 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -13,7 +13,7 @@ public static async ValueTask IsSystemRoleAsync(this ISystemRoleProvider s var systemRoles = await systemRoleNameProvider.GetSystemRolesAsync(); - return systemRoles.Any(role => role.RoleName == name); + return systemRoles.Any(role => role.RoleName.Equals(name, StringComparison.OrdinalIgnoreCase)); } public static async ValueTask IsAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) @@ -22,6 +22,6 @@ public static async ValueTask IsAdminRoleAsync(this ISystemRoleProvider sy var adminRole = await systemRoleNameProvider.GetAdminRoleAsync(); - return adminRole.RoleName == name; + return adminRole.RoleName.Equals(name, StringComparison.OrdinalIgnoreCase); } } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index a6ab913f4c1..20f4d51e9af 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Options; using OrchardCore.Environment.Shell; using OrchardCore.Security; @@ -7,14 +8,23 @@ public class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly IEnumerable _systemRoles; - public DefaultSystemRoleProvider(ShellSettings shellSettings) + public DefaultSystemRoleProvider(ShellSettings shellSettings, IOptions options) { + var adminRoleName = shellSettings["AdminRoleName"]; + if (string.IsNullOrWhiteSpace(adminRoleName)) + { + adminRoleName = options.Value.SystemAdminRoleName; + } + + if (string.IsNullOrWhiteSpace(adminRoleName)) + { + adminRoleName = OrchardCoreConstants.Roles.Administrator; + } + _systemRoles = [ new Role { - RoleName = string.IsNullOrEmpty(shellSettings["AdminRoleName"]) - ? OrchardCoreConstants.Roles.Administrator - : shellSettings["AdminRoleName"], + RoleName = adminRoleName, RoleDescription = "A system role that grants all permissions to the assigned users." }, new Role diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs index b269c5fb3e0..24e89a4cc64 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs @@ -7,7 +7,11 @@ public static class ServiceCollectionExtensions { public static IServiceCollection AddRolesCoreServices(this IServiceCollection services) { +#pragma warning disable CS0618 // Type or member is obsolete +#pragma warning disable CS0612 // Type or member is obsolete services.TryAddSingleton(); +#pragma warning restore CS0612 // Type or member is obsolete +#pragma warning restore CS0618 // Type or member is obsolete services.TryAddSingleton(); return services; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs index 9b981320cfe..bec2264116c 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs @@ -4,6 +4,7 @@ namespace OrchardCore.Roles; +[Obsolete] internal sealed class DefaultSystemRoleNameProvider : ISystemRoleNameProvider { private readonly string _adminRoleName; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs index 7c4c447a1d7..a358952df06 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs @@ -8,12 +8,15 @@ namespace OrchardCore.Roles.Services; public class RoleService : IRoleService { private readonly RoleManager _roleManager; - private readonly ISystemRoleNameProvider _systemRoleNameProvider; private readonly ISystemRoleProvider _systemRoleProvider; public RoleService( RoleManager roleManager, +#pragma warning disable IDE0060 // Remove unused parameter +#pragma warning disable CS0618 // Type or member is obsolete ISystemRoleNameProvider systemRoleNameProvider, +#pragma warning restore CS0618 // Type or member is obsolete +#pragma warning restore IDE0060 // Remove unused parameter ISystemRoleProvider systemRoleProvider) { _roleManager = roleManager; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs index 6ae30673bd0..34140e540c8 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs @@ -2,7 +2,9 @@ namespace OrchardCore.Roles; public static class SystemRoleNameProviderExtensions { +#pragma warning disable CS0618 // Type or member is obsolete public static async ValueTask IsAdminRoleAsync(this ISystemRoleNameProvider provider, string roleName) +#pragma warning restore CS0618 // Type or member is obsolete { ArgumentNullException.ThrowIfNull(roleName); @@ -11,7 +13,9 @@ public static async ValueTask IsAdminRoleAsync(this ISystemRoleNameProvide return roleName.Equals(adminRole, StringComparison.OrdinalIgnoreCase); } +#pragma warning disable CS0618 // Type or member is obsolete public static async ValueTask IsSystemRoleAsync(this ISystemRoleNameProvider provider, string roleName) +#pragma warning restore CS0618 // Type or member is obsolete { ArgumentNullException.ThrowIfNull(roleName); diff --git a/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs b/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs index 4d28f1360b9..89af43ea759 100644 --- a/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs +++ b/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs @@ -10,16 +10,17 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsDefaultAdminRo { // Arrange var shellSettings = new ShellSettings(); - var options = new Mock>(); - options.Setup(x => x.Value).Returns(new SystemRoleOptions()); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); - // Assert + // Act var roles = await provider.GetSystemRolesAsync(); - Assert.Contains(OrchardCoreConstants.Roles.Administrator, roles as IEnumerable); + // Assert + Assert.Contains(OrchardCoreConstants.Roles.Administrator, roles.Select(r => r.RoleName)); } [Fact] @@ -28,55 +29,43 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdmi // Arrange var shellSettings = new ShellSettings(); var configureSystemAdminRoleName = "SystemAdmin"; - var options = new Mock>(); - options.Setup(x => x.Value).Returns(new SystemRoleOptions - { - SystemAdminRoleName = configureSystemAdminRoleName, - }); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions + { + SystemAdminRoleName = configureSystemAdminRoleName, + }); - var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); - // Assert + // Act 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); + var roleNames = roles.Select(r => r.RoleName); + Assert.Contains(configureSystemAdminRoleName, roleNames); + Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roleNames); } [Fact] - public async Task SystemRoleNamesContains_WhenCalled_ReturnsCaseInsensitive() + 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()); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleNameProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); - // Assert + // Act var roles = await provider.GetSystemRolesAsync(); - Assert.Contains("fOo", roles as IEnumerable); + // Assert + var roleNames = roles.Select(r => r.RoleName); + Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roleNames); + Assert.Contains("Foo", roleNames); } } From 56ffeb3ac5725d6ccdcde5441a27b40319f89554 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Wed, 15 Jan 2025 16:11:04 +0300 Subject: [PATCH 11/28] Apply suggestions from code review Co-authored-by: Mike Alhayek --- .../ISystemRoleNameProvider.cs | 2 +- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs index 30893c4a9ad..619449f3bb0 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleNameProvider.cs @@ -2,7 +2,7 @@ namespace OrchardCore.Roles; -[Obsolete("This interface has been deprecated, use ISystemRoleNameProvider instead.")] +[Obsolete("This interface has been deprecated, use ISystemRoleProvider instead.")] public interface ISystemRoleNameProvider { ValueTask GetAdminRoleAsync(); diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 20f4d51e9af..5830b900648 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -4,7 +4,7 @@ namespace OrchardCore.Roles; -public class DefaultSystemRoleProvider : ISystemRoleProvider +internal sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly IEnumerable _systemRoles; @@ -29,12 +29,12 @@ public DefaultSystemRoleProvider(ShellSettings shellSettings, IOptions Date: Sat, 18 Jan 2025 04:38:07 +0300 Subject: [PATCH 12/28] localize the description --- .../DefaultSystemRoleProvider.cs | 17 ++++++++++++----- .../DefaultSystemRoleNameProviderTests.cs | 10 +++++++--- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 5830b900648..40d020e55f4 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -1,15 +1,22 @@ +using Microsoft.Extensions.Localization; using Microsoft.Extensions.Options; using OrchardCore.Environment.Shell; using OrchardCore.Security; namespace OrchardCore.Roles; -internal sealed class DefaultSystemRoleProvider : ISystemRoleProvider +public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly IEnumerable _systemRoles; + private readonly IStringLocalizer S; - public DefaultSystemRoleProvider(ShellSettings shellSettings, IOptions options) + public DefaultSystemRoleProvider( + ShellSettings shellSettings, + IStringLocalizer localizer, + IOptions options) { + S = localizer; + var adminRoleName = shellSettings["AdminRoleName"]; if (string.IsNullOrWhiteSpace(adminRoleName)) { @@ -25,17 +32,17 @@ public DefaultSystemRoleProvider(ShellSettings shellSettings, IOptions>(); var shellSettings = new ShellSettings(); var options = new Mock>(); options.Setup(o => o.Value) .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act var roles = await provider.GetSystemRolesAsync(); @@ -27,6 +29,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsDefaultAdminRo public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdminRole() { // Arrange + var localizer = Mock.Of>(); var shellSettings = new ShellSettings(); var configureSystemAdminRoleName = "SystemAdmin"; var options = new Mock>(); @@ -36,7 +39,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdmi SystemAdminRoleName = configureSystemAdminRoleName, }); - var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act var roles = await provider.GetSystemRolesAsync(); @@ -51,6 +54,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdmi public async Task SystemRoleNamesContains_WhenConstructed_ContainsAppSettingsRole() { // Arrange + var localizer = Mock.Of>(); var shellSettings = new ShellSettings(); shellSettings["AdminRoleName"] = "Foo"; @@ -58,7 +62,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsAppSettingsRol options.Setup(o => o.Value) .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleProvider(shellSettings, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act var roles = await provider.GetSystemRolesAsync(); From 7a2da40b245d805f4621d0b15ad1dcb1c8d69cae Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 04:39:31 +0300 Subject: [PATCH 13/28] Remove obsolete from DefaultSystemRoleNameProvider --- .../Services/DefaultSystemRoleNameProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs index bec2264116c..44a4d0f3b1a 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs @@ -4,8 +4,9 @@ namespace OrchardCore.Roles; -[Obsolete] +#pragma warning disable CS0618 // Type or member is obsolete internal sealed class DefaultSystemRoleNameProvider : ISystemRoleNameProvider +#pragma warning restore CS0618 // Type or member is obsolete { private readonly string _adminRoleName; From b97848f164a60b173fa4c56af4e9b76b1415e265 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 04:43:12 +0300 Subject: [PATCH 14/28] Don't depend on role position --- .../Extensions/SystemRoleProviderExtensions.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index 633b5a983a7..ba492aaa2ee 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -5,7 +5,11 @@ namespace OrchardCore.Roles; public static class SystemRoleProviderExtensions { public static async ValueTask GetAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider) - => (await systemRoleNameProvider.GetSystemRolesAsync()).First(); + { + var systemRoles = await systemRoleNameProvider.GetSystemRolesAsync(); + + return systemRoles.SingleOrDefault(role => role.RoleName.Equals(OrchardCoreConstants.Roles.Administrator, StringComparison.OrdinalIgnoreCase)); + } public static async ValueTask IsSystemRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) { From 9026f4651ff5147426ac7b2957a14fc15ba6f28c Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 07:06:46 +0300 Subject: [PATCH 15/28] Add GetAdminRoleAsync() to ISystemRoleProvider --- .../Extensions/SystemRoleProviderExtensions.cs | 7 ------- .../ISystemRoleProvider.cs | 2 ++ .../DefaultSystemRoleProvider.cs | 16 +++++++++++----- ...ests.cs => DefaultSystemRoleProviderTests.cs} | 8 ++++---- 4 files changed, 17 insertions(+), 16 deletions(-) rename test/OrchardCore.Tests/Security/{DefaultSystemRoleNameProviderTests.cs => DefaultSystemRoleProviderTests.cs} (88%) diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index ba492aaa2ee..5995fa2b75f 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -4,13 +4,6 @@ namespace OrchardCore.Roles; public static class SystemRoleProviderExtensions { - public static async ValueTask GetAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider) - { - var systemRoles = await systemRoleNameProvider.GetSystemRolesAsync(); - - return systemRoles.SingleOrDefault(role => role.RoleName.Equals(OrchardCoreConstants.Roles.Administrator, StringComparison.OrdinalIgnoreCase)); - } - public static async ValueTask IsSystemRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) { ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs index 86c7aec38ab..3b892e62a4e 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs @@ -5,4 +5,6 @@ namespace OrchardCore.Roles; public interface ISystemRoleProvider { ValueTask> GetSystemRolesAsync(); + + ValueTask GetAdminRoleAsync(); } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 40d020e55f4..b76fd7961f6 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -8,6 +8,7 @@ namespace OrchardCore.Roles; public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly IEnumerable _systemRoles; + private readonly IRole _adminRole; private readonly IStringLocalizer S; public DefaultSystemRoleProvider( @@ -28,12 +29,14 @@ public DefaultSystemRoleProvider( adminRoleName = OrchardCoreConstants.Roles.Administrator; } + _adminRole = new Role + { + RoleName = adminRoleName, + RoleDescription = S["A system role that grants all permissions to the assigned users."] + }; + _systemRoles = [ - new Role - { - RoleName = adminRoleName, - RoleDescription = S["A system role that grants all permissions to the assigned users."] - }, + _adminRole, new Role { RoleName = OrchardCoreConstants.Roles.Authenticated, @@ -49,4 +52,7 @@ public DefaultSystemRoleProvider( public ValueTask> GetSystemRolesAsync() => ValueTask.FromResult(_systemRoles); + + public ValueTask GetAdminRoleAsync() + => ValueTask.FromResult(_adminRole); } diff --git a/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs b/test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs similarity index 88% rename from test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs rename to test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs index 25fef752c16..413bfcba430 100644 --- a/test/OrchardCore.Tests/Security/DefaultSystemRoleNameProviderTests.cs +++ b/test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs @@ -4,10 +4,10 @@ namespace OrchardCore.Tests.Security; -public class DefaultSystemRoleNameProviderTests +public class DefaultSystemRoleProviderTests { [Fact] - public async Task SystemRoleNamesContains_WhenConstructed_ContainsDefaultAdminRole() + public async Task SystemRolesContains_WhenConstructed_ContainsDefaultAdminRole() { // Arrange var localizer = Mock.Of>(); @@ -26,7 +26,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsDefaultAdminRo } [Fact] - public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdminRole() + public async Task SystemRolesContains_WhenConstructed_ContainsConfiguredAdminRole() { // Arrange var localizer = Mock.Of>(); @@ -51,7 +51,7 @@ public async Task SystemRoleNamesContains_WhenConstructed_ContainsConfiguredAdmi } [Fact] - public async Task SystemRoleNamesContains_WhenConstructed_ContainsAppSettingsRole() + public async Task SystemRolesContains_WhenConstructed_ContainsAppSettingsRole() { // Arrange var localizer = Mock.Of>(); From 48a37191107d363dd1c86d8c513be56c7769c4c0 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 15:31:50 +0300 Subject: [PATCH 16/28] Remove ISystemRoleNameProvider injection --- .../OrchardCore.Roles/Services/RoleClaimsProvider.cs | 5 ----- .../OrchardCore.Roles/Services/RoleUpdater.cs | 3 --- .../OrchardCore.Settings/Services/SuperUserHandler.cs | 5 ----- .../OrchardCore.Roles.Core/ServiceCollectionExtensions.cs | 2 -- .../OrchardCore.Roles.Core/Services/RoleService.cs | 5 ----- 5 files changed, 20 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs index 229857838f6..76873387143 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs @@ -17,11 +17,6 @@ public class RoleClaimsProvider : IUserClaimsProvider public RoleClaimsProvider( UserManager userManager, RoleManager roleManager, -#pragma warning disable CS0618 // Type or member is obsolete -#pragma warning disable IDE0060 // Remove unused parameter - ISystemRoleNameProvider systemRoleNameProvider, -#pragma warning restore IDE0060 // Remove unused parameter -#pragma warning restore CS0618 // Type or member is obsolete ISystemRoleProvider systemRoleProvider, IOptions identityOptions) { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs index 97bef939cc9..7d6882b8db9 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs @@ -26,9 +26,6 @@ public RoleUpdater( ShellDescriptor shellDescriptor, IExtensionManager extensionManager, IDocumentManager documentManager, -#pragma warning disable IDE0060 // Remove unused parameter - ISystemRoleProvider systemRoleNameProvider, -#pragma warning restore IDE0060 // Remove unused parameter ISystemRoleProvider systemRoleProvider, IEnumerable permissionProviders, ITypeFeatureProvider typeFeatureProvider, diff --git a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs index 2190cd0e352..f8f0a47f05b 100644 --- a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs @@ -15,11 +15,6 @@ public class SuperUserHandler : IAuthorizationHandler public SuperUserHandler( ISiteService siteService, -#pragma warning disable CS0618 // Type or member is obsolete -#pragma warning disable IDE0060 // Remove unused parameter - ISystemRoleNameProvider systemRoleNameProvider, -#pragma warning restore IDE0060 // Remove unused parameter -#pragma warning restore CS0618 // Type or member is obsolete ISystemRoleProvider systemRoleProvider) { _siteService = siteService; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs index 24e89a4cc64..81da37754e9 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs @@ -8,9 +8,7 @@ public static class ServiceCollectionExtensions public static IServiceCollection AddRolesCoreServices(this IServiceCollection services) { #pragma warning disable CS0618 // Type or member is obsolete -#pragma warning disable CS0612 // Type or member is obsolete services.TryAddSingleton(); -#pragma warning restore CS0612 // Type or member is obsolete #pragma warning restore CS0618 // Type or member is obsolete services.TryAddSingleton(); diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs index a358952df06..3ef98ad44aa 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs @@ -12,11 +12,6 @@ public class RoleService : IRoleService public RoleService( RoleManager roleManager, -#pragma warning disable IDE0060 // Remove unused parameter -#pragma warning disable CS0618 // Type or member is obsolete - ISystemRoleNameProvider systemRoleNameProvider, -#pragma warning restore CS0618 // Type or member is obsolete -#pragma warning restore IDE0060 // Remove unused parameter ISystemRoleProvider systemRoleProvider) { _roleManager = roleManager; From 3f104ab57539774793738b918a1481c518576b40 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 16:34:27 +0300 Subject: [PATCH 17/28] Add more unit tests --- .../DefaultSystemRoleProviderTests.cs | 32 +++++----- .../SystemRoleProviderExtensionsTests.cs | 60 +++++++++++++++++++ 2 files changed, 76 insertions(+), 16 deletions(-) rename test/OrchardCore.Tests/{Security => Roles}/DefaultSystemRoleProviderTests.cs (66%) create mode 100644 test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs diff --git a/test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs similarity index 66% rename from test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs rename to test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs index 413bfcba430..0269f6227fa 100644 --- a/test/OrchardCore.Tests/Security/DefaultSystemRoleProviderTests.cs +++ b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs @@ -1,13 +1,11 @@ using OrchardCore.Environment.Shell; -using OrchardCore.Localization; -using OrchardCore.Roles; -namespace OrchardCore.Tests.Security; +namespace OrchardCore.Roles.Tests; public class DefaultSystemRoleProviderTests { [Fact] - public async Task SystemRolesContains_WhenConstructed_ContainsDefaultAdminRole() + public async Task GetSystemRoles_Have_Administrator_Authenticated_Anonymous() { // Arrange var localizer = Mock.Of>(); @@ -22,11 +20,14 @@ public async Task SystemRolesContains_WhenConstructed_ContainsDefaultAdminRole() var roles = await provider.GetSystemRolesAsync(); // Assert - Assert.Contains(OrchardCoreConstants.Roles.Administrator, roles.Select(r => r.RoleName)); + var roleNames = roles.Select(r => r.RoleName); + Assert.Contains(OrchardCoreConstants.Roles.Administrator, roleNames); + Assert.Contains(OrchardCoreConstants.Roles.Authenticated, roleNames); + Assert.Contains(OrchardCoreConstants.Roles.Anonymous, roleNames); } [Fact] - public async Task SystemRolesContains_WhenConstructed_ContainsConfiguredAdminRole() + public async Task GetAdminRole_FromConfiguredAdminRole() { // Arrange var localizer = Mock.Of>(); @@ -42,21 +43,21 @@ public async Task SystemRolesContains_WhenConstructed_ContainsConfiguredAdminRol var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act - var roles = await provider.GetSystemRolesAsync(); + var role = await provider.GetAdminRoleAsync(); // Assert - var roleNames = roles.Select(r => r.RoleName); - Assert.Contains(configureSystemAdminRoleName, roleNames); - Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roleNames); + Assert.Equal(configureSystemAdminRoleName, role.RoleName); + Assert.NotEqual(OrchardCoreConstants.Roles.Administrator, role.RoleName); } [Fact] - public async Task SystemRolesContains_WhenConstructed_ContainsAppSettingsRole() + public async Task GetAdminRole_FromAppSettings() { // Arrange + var adminRoleName = "Foo"; var localizer = Mock.Of>(); var shellSettings = new ShellSettings(); - shellSettings["AdminRoleName"] = "Foo"; + shellSettings["AdminRoleName"] = adminRoleName; var options = new Mock>(); options.Setup(o => o.Value) @@ -65,11 +66,10 @@ public async Task SystemRolesContains_WhenConstructed_ContainsAppSettingsRole() var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act - var roles = await provider.GetSystemRolesAsync(); + var role = await provider.GetAdminRoleAsync(); // Assert - var roleNames = roles.Select(r => r.RoleName); - Assert.DoesNotContain(OrchardCoreConstants.Roles.Administrator, roleNames); - Assert.Contains("Foo", roleNames); + Assert.Equal(adminRoleName, role.RoleName); + Assert.NotEqual(OrchardCoreConstants.Roles.Administrator, role.RoleName); } } diff --git a/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs new file mode 100644 index 00000000000..5c7872d6890 --- /dev/null +++ b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs @@ -0,0 +1,60 @@ +using OrchardCore.Environment.Shell; + +namespace OrchardCore.Roles.Tests; + +public class SystemRoleProviderExtensionsTests +{ + [Theory] + [InlineData("Administrator", true)] + [InlineData("ADMINISTRATOR", true)] + [InlineData("Authenticated", true)] + [InlineData("authenticated", true)] + [InlineData("Anonymous", true)] + [InlineData("AnonYmouS", true)] + [InlineData("Test", false)] + [InlineData("TEST", false)] + [InlineData("TesT", false)] + [InlineData("test", false)] + public async Task IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) + { + // Arrange + var localizer = Mock.Of>(); + var shellSettings = new ShellSettings(); + + var options = new Mock>(); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions()); + + // Act + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + + // Assert + Assert.Equal(expectedResult, await provider.IsSystemRoleAsync(roleName)); + } + + [Theory] + [InlineData("Administrator", true)] + [InlineData("ADMINISTRATOR", true)] + [InlineData("administrator", true)] + [InlineData("AdminiSTratoR", true)] + [InlineData("Test", false)] + [InlineData("TEST", false)] + [InlineData("TesT", false)] + [InlineData("test", false)] + public async Task IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) + { + // Arrange + var localizer = Mock.Of>(); + var shellSettings = new ShellSettings(); + + var options = new Mock>(); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions()); + + // Act + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + + // Assert + Assert.Equal(expectedResult, await provider.IsAdminRoleAsync(roleName)); + } +} From 674c862e6566377d81161b709edd38884859e0b9 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 20:12:56 +0300 Subject: [PATCH 18/28] Fix the build --- .../OrchardCore.Roles/Migrations/SystemRolesMigrations.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs index 80a085e31e2..77822e2fca0 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs @@ -27,11 +27,9 @@ public async Task CreateAsync() foreach (var systemRole in systemRoles) { - var role = await _roleManager.FindByNameAsync(systemRole.RoleName); - - if (role is null) + if (!await _roleManager.RoleExistsAsync(systemRole.RoleName)) { - await _roleManager.CreateAsync(role); + await _roleManager.CreateAsync(systemRole); } } From 748e6d8b4d23646842e45301c5172058ce90dd42 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 22:08:20 +0300 Subject: [PATCH 19/28] localizer -> stringLocalizer --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index b76fd7961f6..dc593d17f93 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -13,10 +13,10 @@ public sealed class DefaultSystemRoleProvider : ISystemRoleProvider public DefaultSystemRoleProvider( ShellSettings shellSettings, - IStringLocalizer localizer, + IStringLocalizer stringLocalizer, IOptions options) { - S = localizer; + S = stringLocalizer; var adminRoleName = shellSettings["AdminRoleName"]; if (string.IsNullOrWhiteSpace(adminRoleName)) From 313651e91541076645ebec68b1a73b9490b504d9 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sat, 18 Jan 2025 22:09:58 +0300 Subject: [PATCH 20/28] Obsolete SystemRoleNameProviderExtensions --- .../Extensions/SystemRoleProviderExtensions.cs | 2 -- .../SystemRoleNameProviderExtensions.cs | 5 +---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index 5995fa2b75f..b5043570158 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -1,5 +1,3 @@ -using OrchardCore.Security; - namespace OrchardCore.Roles; public static class SystemRoleProviderExtensions diff --git a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs index 34140e540c8..1f19d7f86dd 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/SystemRoleNameProviderExtensions.cs @@ -1,10 +1,9 @@ namespace OrchardCore.Roles; +[Obsolete("This class has been deprecated, please use SystemRoleProviderExtensions instead.")] public static class SystemRoleNameProviderExtensions { -#pragma warning disable CS0618 // Type or member is obsolete public static async ValueTask IsAdminRoleAsync(this ISystemRoleNameProvider provider, string roleName) -#pragma warning restore CS0618 // Type or member is obsolete { ArgumentNullException.ThrowIfNull(roleName); @@ -13,9 +12,7 @@ public static async ValueTask IsAdminRoleAsync(this ISystemRoleNameProvide return roleName.Equals(adminRole, StringComparison.OrdinalIgnoreCase); } -#pragma warning disable CS0618 // Type or member is obsolete public static async ValueTask IsSystemRoleAsync(this ISystemRoleNameProvider provider, string roleName) -#pragma warning restore CS0618 // Type or member is obsolete { ArgumentNullException.ThrowIfNull(roleName); From d2245e3460660298699444ac18e7b6530be72ae0 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sun, 19 Jan 2025 21:45:17 +0300 Subject: [PATCH 21/28] Address feedback --- .../Migrations/SystemRolesMigrations.cs | 2 +- .../OrchardCore.Roles/Recipes/RolesStep.cs | 2 +- .../Services/RoleClaimsProvider.cs | 2 +- .../OrchardCore.Roles/Services/RoleStore.cs | 2 +- .../OrchardCore.Roles/Services/RoleUpdater.cs | 18 ++++----- .../Services/SuperUserHandler.cs | 2 +- .../SystemRoleProviderExtensions.cs | 13 +----- .../ISystemRoleProvider.cs | 6 ++- .../DefaultSystemRoleProvider.cs | 40 ++++++++++++------- .../Services/RoleService.cs | 4 +- .../Roles/DefaultSystemRoleProviderTests.cs | 40 ++++++++++++++++--- .../SystemRoleProviderExtensionsTests.cs | 32 +-------------- 12 files changed, 84 insertions(+), 79 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs index 77822e2fca0..4ca7c7a884d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Migrations/SystemRolesMigrations.cs @@ -23,7 +23,7 @@ public SystemRolesMigrations( public async Task CreateAsync() { - var systemRoles = await _systemRoleProvider.GetSystemRolesAsync(); + var systemRoles = _systemRoleProvider.GetSystemRoles(); foreach (var systemRole in systemRoles) { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs index a5d56117934..6b7d9e332cc 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs @@ -59,7 +59,7 @@ protected override async Task HandleAsync(RecipeExecutionContext context) r.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType); } - if (!await _systemRoleProvider.IsAdminRoleAsync(roleName)) + if (!_systemRoleProvider.IsAdminRole(roleName)) { if (roleEntry.PermissionBehavior == PermissionBehavior.Remove) { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs index 76873387143..3bef2d20232 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleClaimsProvider.cs @@ -39,7 +39,7 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims) foreach (var roleName in roleNames) { - if (await _systemRoleProvider.IsAdminRoleAsync(roleName)) + if (_systemRoleProvider.IsAdminRole(roleName)) { isAdministrator = true; break; diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs index cbc0a919c01..ade8d145746 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs @@ -91,7 +91,7 @@ public async Task DeleteAsync(IRole role, CancellationToken canc }); } - if (await _systemRoleProvider.IsSystemRoleAsync(roleToRemove.RoleName)) + if (_systemRoleProvider.IsSystemRole(roleToRemove.RoleName)) { return IdentityResult.Failed(new IdentityError { diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs index 7d6882b8db9..1845cc342f3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs @@ -80,7 +80,7 @@ private async Task UpdateRolesForInstalledFeatureAsync(IFeatureInfo feature) var permissions = (stereotype.Permissions ?? []) .Select(stereotype => stereotype.Name); - if (await UpdatePermissionsAsync(role, permissions)) + if (UpdatePermissions(role, permissions)) { updated = true; } @@ -121,7 +121,7 @@ private async Task UpdateRolesForEnabledFeatureAsync(IFeatureInfo feature) updated = true; missingFeatures.Remove(feature.Id); - await UpdateRolesForEnabledFeatureAsync(role, providers); + UpdateRolesForEnabledFeature(role, providers); } if (updated) @@ -166,7 +166,7 @@ private async Task UpdateRoleForInstalledFeaturesAsync(string roleName) .SelectMany(stereotype => stereotype.Permissions ?? []) .Select(stereotype => stereotype.Name); - await UpdatePermissionsAsync(role, permissions); + UpdatePermissions(role, permissions); } private async Task RemoveRoleForMissingFeaturesAsync(string roleName) @@ -179,7 +179,7 @@ private async Task RemoveRoleForMissingFeaturesAsync(string roleName) } } - private Task UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable providers) + private bool UpdateRolesForEnabledFeature(Role role, IEnumerable providers) { var stereotypes = providers .SelectMany(provider => provider.GetDefaultStereotypes()) @@ -187,7 +187,7 @@ private Task UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable UpdatePermissionsAsync(Role role, IEnumerable permissions) + private bool UpdatePermissions(Role role, IEnumerable permissions) { - if (await _systemRoleProvider.IsAdminRoleAsync(role.RoleName)) + if (_systemRoleProvider.IsAdminRole(role.RoleName)) { // Don't update claims for admin role. return true; diff --git a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs index f8f0a47f05b..cae844eb27f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs @@ -30,7 +30,7 @@ public async Task HandleAsync(AuthorizationHandlerContext context) return; } - var adminRole = await _systemRoleProvider.GetAdminRoleAsync(); + var adminRole = _systemRoleProvider.GetAdminRole(); if (user.IsInRole(adminRole.RoleName)) { diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index b5043570158..187bbb10dd4 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -2,20 +2,11 @@ namespace OrchardCore.Roles; public static class SystemRoleProviderExtensions { - public static async ValueTask IsSystemRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) + public static bool IsAdminRole(this ISystemRoleProvider systemRoleNameProvider, string name) { ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); - var systemRoles = await systemRoleNameProvider.GetSystemRolesAsync(); - - return systemRoles.Any(role => role.RoleName.Equals(name, StringComparison.OrdinalIgnoreCase)); - } - - public static async ValueTask IsAdminRoleAsync(this ISystemRoleProvider systemRoleNameProvider, string name) - { - ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); - - var adminRole = await systemRoleNameProvider.GetAdminRoleAsync(); + var adminRole = systemRoleNameProvider.GetAdminRole(); return adminRole.RoleName.Equals(name, StringComparison.OrdinalIgnoreCase); } diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs index 3b892e62a4e..5386b232901 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs @@ -4,7 +4,9 @@ namespace OrchardCore.Roles; public interface ISystemRoleProvider { - ValueTask> GetSystemRolesAsync(); + IEnumerable GetSystemRoles(); - ValueTask GetAdminRoleAsync(); + IRole GetAdminRole(); + + bool IsSystemRole(string role); } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index dc593d17f93..d8f866119ec 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -1,3 +1,4 @@ +using System.Collections.Frozen; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Options; using OrchardCore.Environment.Shell; @@ -7,7 +8,7 @@ namespace OrchardCore.Roles; public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { - private readonly IEnumerable _systemRoles; + private readonly FrozenDictionary _systemRoles; private readonly IRole _adminRole; private readonly IStringLocalizer S; @@ -35,24 +36,35 @@ public DefaultSystemRoleProvider( RoleDescription = S["A system role that grants all permissions to the assigned users."] }; - _systemRoles = [ - _adminRole, - new Role + _systemRoles = new Dictionary() + { + { _adminRole.RoleName, _adminRole }, { - RoleName = OrchardCoreConstants.Roles.Authenticated, - RoleDescription = S["A system role representing all authenticated users."] + OrchardCoreConstants.Roles.Authenticated, new Role + { + RoleName = OrchardCoreConstants.Roles.Authenticated, + RoleDescription = S["A system role representing all authenticated users."] + } }, - new Role { - RoleName = OrchardCoreConstants.Roles.Anonymous, - RoleDescription = S["A system role representing all non-authenticated users."] + OrchardCoreConstants.Roles.Anonymous, new Role + { + RoleName = OrchardCoreConstants.Roles.Anonymous, + RoleDescription = S["A system role representing all non-authenticated users."] + } } - ]; + }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); } - public ValueTask> GetSystemRolesAsync() - => ValueTask.FromResult(_systemRoles); + public IEnumerable GetSystemRoles() + => _systemRoles.Values.AsEnumerable(); + + public IRole GetAdminRole() => _adminRole; - public ValueTask GetAdminRoleAsync() - => ValueTask.FromResult(_adminRole); + public bool IsSystemRole(string roleName) + { + ArgumentException.ThrowIfNullOrEmpty(roleName, nameof(roleName)); + + return _systemRoles.ContainsKey(roleName); + } } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs index 3ef98ad44aa..7d35155800b 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/RoleService.cs @@ -50,8 +50,8 @@ public Task> GetNormalizedRoleNamesAsync() } public ValueTask IsAdminRoleAsync(string role) - => _systemRoleProvider.IsAdminRoleAsync(role); + => ValueTask.FromResult(_systemRoleProvider.IsAdminRole(role)); public ValueTask IsSystemRoleAsync(string role) - => _systemRoleProvider.IsSystemRoleAsync(role); + => ValueTask.FromResult(_systemRoleProvider.IsSystemRole(role)); } diff --git a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs index 0269f6227fa..d1ae037d84a 100644 --- a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs +++ b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs @@ -5,7 +5,7 @@ namespace OrchardCore.Roles.Tests; public class DefaultSystemRoleProviderTests { [Fact] - public async Task GetSystemRoles_Have_Administrator_Authenticated_Anonymous() + public void GetSystemRoles_Have_Administrator_Authenticated_Anonymous() { // Arrange var localizer = Mock.Of>(); @@ -17,7 +17,7 @@ public async Task GetSystemRoles_Have_Administrator_Authenticated_Anonymous() var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act - var roles = await provider.GetSystemRolesAsync(); + var roles = provider.GetSystemRoles(); // Assert var roleNames = roles.Select(r => r.RoleName); @@ -27,7 +27,7 @@ public async Task GetSystemRoles_Have_Administrator_Authenticated_Anonymous() } [Fact] - public async Task GetAdminRole_FromConfiguredAdminRole() + public void GetAdminRole_FromConfiguredAdminRole() { // Arrange var localizer = Mock.Of>(); @@ -43,7 +43,7 @@ public async Task GetAdminRole_FromConfiguredAdminRole() var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act - var role = await provider.GetAdminRoleAsync(); + var role = provider.GetAdminRole(); // Assert Assert.Equal(configureSystemAdminRoleName, role.RoleName); @@ -51,7 +51,7 @@ public async Task GetAdminRole_FromConfiguredAdminRole() } [Fact] - public async Task GetAdminRole_FromAppSettings() + public void GetAdminRole_FromAppSettings() { // Arrange var adminRoleName = "Foo"; @@ -66,10 +66,38 @@ public async Task GetAdminRole_FromAppSettings() var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Act - var role = await provider.GetAdminRoleAsync(); + var role = provider.GetAdminRole(); // Assert Assert.Equal(adminRoleName, role.RoleName); Assert.NotEqual(OrchardCoreConstants.Roles.Administrator, role.RoleName); } + + [Theory] + [InlineData("Administrator", true)] + [InlineData("ADMINISTRATOR", true)] + [InlineData("Authenticated", true)] + [InlineData("authenticated", true)] + [InlineData("Anonymous", true)] + [InlineData("AnonYmouS", true)] + [InlineData("Test", false)] + [InlineData("TEST", false)] + [InlineData("TesT", false)] + [InlineData("test", false)] + public void IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) + { + // Arrange + var localizer = Mock.Of>(); + var shellSettings = new ShellSettings(); + + var options = new Mock>(); + options.Setup(o => o.Value) + .Returns(new SystemRoleOptions()); + + // Act + var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + + // Assert + Assert.Equal(expectedResult, provider.IsSystemRole(roleName)); + } } diff --git a/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs index 5c7872d6890..ebdf7615512 100644 --- a/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs +++ b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs @@ -4,34 +4,6 @@ namespace OrchardCore.Roles.Tests; public class SystemRoleProviderExtensionsTests { - [Theory] - [InlineData("Administrator", true)] - [InlineData("ADMINISTRATOR", true)] - [InlineData("Authenticated", true)] - [InlineData("authenticated", true)] - [InlineData("Anonymous", true)] - [InlineData("AnonYmouS", true)] - [InlineData("Test", false)] - [InlineData("TEST", false)] - [InlineData("TesT", false)] - [InlineData("test", false)] - public async Task IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) - { - // Arrange - var localizer = Mock.Of>(); - var shellSettings = new ShellSettings(); - - var options = new Mock>(); - options.Setup(o => o.Value) - .Returns(new SystemRoleOptions()); - - // Act - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); - - // Assert - Assert.Equal(expectedResult, await provider.IsSystemRoleAsync(roleName)); - } - [Theory] [InlineData("Administrator", true)] [InlineData("ADMINISTRATOR", true)] @@ -41,7 +13,7 @@ public async Task IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool [InlineData("TEST", false)] [InlineData("TesT", false)] [InlineData("test", false)] - public async Task IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) + public void IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) { // Arrange var localizer = Mock.Of>(); @@ -55,6 +27,6 @@ public async Task IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); // Assert - Assert.Equal(expectedResult, await provider.IsAdminRoleAsync(roleName)); + Assert.Equal(expectedResult, provider.IsAdminRole(roleName)); } } From 2470fb70e2f43b5ff04c684678bd1acfc9e33d55 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Sun, 19 Jan 2025 21:58:06 +0300 Subject: [PATCH 22/28] Fix warnings --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index d8f866119ec..79b3878a4dc 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -8,8 +8,8 @@ namespace OrchardCore.Roles; public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { - private readonly FrozenDictionary _systemRoles; - private readonly IRole _adminRole; + private readonly FrozenDictionary _systemRoles; + private readonly Role _adminRole; private readonly IStringLocalizer S; public DefaultSystemRoleProvider( @@ -36,7 +36,7 @@ public DefaultSystemRoleProvider( RoleDescription = S["A system role that grants all permissions to the assigned users."] }; - _systemRoles = new Dictionary() + _systemRoles = new Dictionary() { { _adminRole.RoleName, _adminRole }, { From 7b7ba19d394646cdefe8fe44b272c712157402c8 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Mon, 20 Jan 2025 03:50:43 +0300 Subject: [PATCH 23/28] Apply suggestions from code review Co-authored-by: Mike Alhayek --- .../OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs | 2 +- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs index 5386b232901..f370e317baf 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/ISystemRoleProvider.cs @@ -8,5 +8,5 @@ public interface ISystemRoleProvider IRole GetAdminRole(); - bool IsSystemRole(string role); + bool IsSystemRole(string name); } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 79b3878a4dc..339ca22ede3 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -61,7 +61,7 @@ public IEnumerable GetSystemRoles() public IRole GetAdminRole() => _adminRole; - public bool IsSystemRole(string roleName) + public bool IsSystemRole(string name) { ArgumentException.ThrowIfNullOrEmpty(roleName, nameof(roleName)); From b3c15a6528bcfc3470edc0a7c9ea0674cbc1f62e Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Tue, 21 Jan 2025 02:02:33 +0300 Subject: [PATCH 24/28] Fix the build --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 339ca22ede3..23abd06c628 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -63,8 +63,8 @@ public IEnumerable GetSystemRoles() public bool IsSystemRole(string name) { - ArgumentException.ThrowIfNullOrEmpty(roleName, nameof(roleName)); + ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); - return _systemRoles.ContainsKey(roleName); + return _systemRoles.ContainsKey(name); } } From 26177f3e554b9ccf3e7ab132b609e5047fe3a032 Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Tue, 21 Jan 2025 02:03:02 +0300 Subject: [PATCH 25/28] Remove unnecessary local field --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index 23abd06c628..eb903d8906e 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -10,15 +10,12 @@ public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly FrozenDictionary _systemRoles; private readonly Role _adminRole; - private readonly IStringLocalizer S; public DefaultSystemRoleProvider( ShellSettings shellSettings, - IStringLocalizer stringLocalizer, + IStringLocalizer S, IOptions options) { - S = stringLocalizer; - var adminRoleName = shellSettings["AdminRoleName"]; if (string.IsNullOrWhiteSpace(adminRoleName)) { From fd7e657064b302ea27b3d558654acaa4d1762cee Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Tue, 21 Jan 2025 02:06:49 +0300 Subject: [PATCH 26/28] Remove extra cast --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index eb903d8906e..ca459dfac11 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -53,8 +53,7 @@ public DefaultSystemRoleProvider( }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); } - public IEnumerable GetSystemRoles() - => _systemRoles.Values.AsEnumerable(); + public IEnumerable GetSystemRoles() => _systemRoles.Values; public IRole GetAdminRole() => _adminRole; From ea580d06eebd27fc628b984e634cf0e7b5bb5b35 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 20 Jan 2025 18:01:10 -0800 Subject: [PATCH 27/28] cleanup for approval --- .../OrchardCore.Roles/Services/RoleStore.cs | 3 +- .../OrchardCore.Roles/Startup.cs | 4 +-- .../TheTheme/Recipes/saas.recipe.json | 20 ------------- .../SystemRoleProviderExtensions.cs | 2 +- .../DefaultSystemRoleProvider.cs | 11 +++---- .../ServiceCollectionExtensions.cs | 1 + .../Services/DefaultSystemRoleNameProvider.cs | 30 ++++--------------- .../cms-tests/Recipes/migrations.recipe.json | 2 +- .../Roles/DefaultSystemRoleProviderTests.cs | 19 ++++++------ .../SystemRoleProviderExtensionsTests.cs | 4 +-- 10 files changed, 30 insertions(+), 66 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs index ade8d145746..f8783a8f609 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs @@ -15,9 +15,10 @@ public class RoleStore : IRoleClaimStore, IQueryableRoleStore private readonly IServiceProvider _serviceProvider; private readonly ISystemRoleProvider _systemRoleProvider; private readonly IDocumentManager _documentManager; - protected readonly IStringLocalizer S; private readonly ILogger _logger; + protected readonly IStringLocalizer S; + private bool _updating; public RoleStore( diff --git a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs index b35f7f3d3ea..226ea3e9e8a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs @@ -34,8 +34,8 @@ public override void ConfigureServices(IServiceCollection services) { services.AddScoped(); - services.AddDataMigration() - .AddDataMigration(); + services.AddDataMigration(); + services.AddDataMigration(); services.AddScoped(); services.Replace(ServiceDescriptor.Scoped>(sp => sp.GetRequiredService())); diff --git a/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json b/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json index 51bb484fe18..6c3177230ba 100644 --- a/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json +++ b/src/OrchardCore.Themes/TheTheme/Recipes/saas.recipe.json @@ -40,26 +40,6 @@ "admin": "TheAdmin", "site": "TheTheme" }, - { - "name": "Roles", - "Roles": [ - { - "Name": "Administrator", - "Description": "A system role that grants all permissions to the assigned users.", - "Permissions": [] - }, - { - "Name": "Authenticated", - "Description": "A system role representing all authenticated users.", - "Permissions": [] - }, - { - "Name": "Anonymous", - "Description": "A system role representing all non-authenticated users.", - "Permissions": [] - } - ] - }, { "name": "settings", "HomeRoute": { diff --git a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs index 187bbb10dd4..8c73e9b8d84 100644 --- a/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Abstractions/Extensions/SystemRoleProviderExtensions.cs @@ -4,7 +4,7 @@ public static class SystemRoleProviderExtensions { public static bool IsAdminRole(this ISystemRoleProvider systemRoleNameProvider, string name) { - ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); + ArgumentException.ThrowIfNullOrEmpty(name); var adminRole = systemRoleNameProvider.GetAdminRole(); diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index ca459dfac11..d1c1b10dc10 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -6,15 +6,15 @@ namespace OrchardCore.Roles; -public sealed class DefaultSystemRoleProvider : ISystemRoleProvider +internal sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly FrozenDictionary _systemRoles; private readonly Role _adminRole; public DefaultSystemRoleProvider( ShellSettings shellSettings, - IStringLocalizer S, - IOptions options) + IOptions options, + IStringLocalizer S) { var adminRoleName = shellSettings["AdminRoleName"]; if (string.IsNullOrWhiteSpace(adminRoleName)) @@ -53,13 +53,14 @@ public DefaultSystemRoleProvider( }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); } - public IEnumerable GetSystemRoles() => _systemRoles.Values; + public IEnumerable GetSystemRoles() + => _systemRoles.Values; public IRole GetAdminRole() => _adminRole; public bool IsSystemRole(string name) { - ArgumentException.ThrowIfNullOrEmpty(name, nameof(name)); + ArgumentException.ThrowIfNullOrEmpty(name); return _systemRoles.ContainsKey(name); } diff --git a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs index 81da37754e9..11e99c812f1 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/ServiceCollectionExtensions.cs @@ -10,6 +10,7 @@ public static IServiceCollection AddRolesCoreServices(this IServiceCollection se #pragma warning disable CS0618 // Type or member is obsolete services.TryAddSingleton(); #pragma warning restore CS0618 // Type or member is obsolete + services.TryAddSingleton(); return services; diff --git a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs index 44a4d0f3b1a..7fe4bd6e5e4 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/Services/DefaultSystemRoleNameProvider.cs @@ -8,38 +8,18 @@ namespace OrchardCore.Roles; internal sealed class DefaultSystemRoleNameProvider : ISystemRoleNameProvider #pragma warning restore CS0618 // Type or member is obsolete { - private readonly string _adminRoleName; + private readonly ISystemRoleProvider _provider; private readonly FrozenSet _systemRoleNames; - public DefaultSystemRoleNameProvider( - ShellSettings shellSettings, - IOptions options) + public DefaultSystemRoleNameProvider(ISystemRoleProvider provider) { - _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); + _provider = provider; + _systemRoleNames = provider.GetSystemRoles().Select(x => x.RoleName).ToFrozenSet(StringComparer.OrdinalIgnoreCase); } public ValueTask GetAdminRoleAsync() - => ValueTask.FromResult(_adminRoleName); + => ValueTask.FromResult(_provider.GetAdminRole().RoleName); public ValueTask> GetSystemRolesAsync() => ValueTask.FromResult(_systemRoleNames); 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 09b72937861..5e9a605034f 100644 --- a/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json +++ b/test/OrchardCore.Tests.Functional/cms-tests/Recipes/migrations.recipe.json @@ -11,7 +11,7 @@ // The variables are evaluated the first time they are accessed, and reused across steps "variables": { - "homeContentItemId": "[js:uuid()]", + "homeContentItemId": "[js:uuid()]" }, "steps": [ diff --git a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs index d1ae037d84a..b3351e5214d 100644 --- a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs +++ b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Localization; using OrchardCore.Environment.Shell; namespace OrchardCore.Roles.Tests; @@ -8,13 +9,13 @@ public class DefaultSystemRoleProviderTests public void GetSystemRoles_Have_Administrator_Authenticated_Anonymous() { // Arrange - var localizer = Mock.Of>(); + var stringLocalizer = Mock.Of>(); var shellSettings = new ShellSettings(); var options = new Mock>(); options.Setup(o => o.Value) .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object, stringLocalizer); // Act var roles = provider.GetSystemRoles(); @@ -30,7 +31,7 @@ public void GetSystemRoles_Have_Administrator_Authenticated_Anonymous() public void GetAdminRole_FromConfiguredAdminRole() { // Arrange - var localizer = Mock.Of>(); + var stringLocalizer = Mock.Of>(); var shellSettings = new ShellSettings(); var configureSystemAdminRoleName = "SystemAdmin"; var options = new Mock>(); @@ -40,7 +41,7 @@ public void GetAdminRole_FromConfiguredAdminRole() SystemAdminRoleName = configureSystemAdminRoleName, }); - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object, stringLocalizer); // Act var role = provider.GetAdminRole(); @@ -55,15 +56,15 @@ public void GetAdminRole_FromAppSettings() { // Arrange var adminRoleName = "Foo"; - var localizer = Mock.Of>(); + var stringLocalizer = Mock.Of>(); var shellSettings = new ShellSettings(); shellSettings["AdminRoleName"] = adminRoleName; var options = new Mock>(); options.Setup(o => o.Value) - .Returns(new SystemRoleOptions()); + .Returns(new SystemRoleOptions()); - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object, stringLocalizer); // Act var role = provider.GetAdminRole(); @@ -87,7 +88,7 @@ public void GetAdminRole_FromAppSettings() public void IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) { // Arrange - var localizer = Mock.Of>(); + var stringLocalizer = Mock.Of>(); var shellSettings = new ShellSettings(); var options = new Mock>(); @@ -95,7 +96,7 @@ public void IsSystemRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expec .Returns(new SystemRoleOptions()); // Act - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object, stringLocalizer); // Assert Assert.Equal(expectedResult, provider.IsSystemRole(roleName)); diff --git a/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs index ebdf7615512..2cf779c66b3 100644 --- a/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs +++ b/test/OrchardCore.Tests/Roles/Extensions/SystemRoleProviderExtensionsTests.cs @@ -16,7 +16,7 @@ public class SystemRoleProviderExtensionsTests public void IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expectedResult) { // Arrange - var localizer = Mock.Of>(); + var stringLocalizer = Mock.Of>(); var shellSettings = new ShellSettings(); var options = new Mock>(); @@ -24,7 +24,7 @@ public void IsAdminRole_ReturnsTrue_IfTheRoleExists(string roleName, bool expect .Returns(new SystemRoleOptions()); // Act - var provider = new DefaultSystemRoleProvider(shellSettings, localizer, options.Object); + var provider = new DefaultSystemRoleProvider(shellSettings, options.Object, stringLocalizer); // Assert Assert.Equal(expectedResult, provider.IsAdminRole(roleName)); From 179130007a6fe84f90cbec3cbd99060c94bb8bf6 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 21 Jan 2025 06:24:59 -0800 Subject: [PATCH 28/28] Fix tests --- .../OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs | 2 +- test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs index d1c1b10dc10..f409da74b83 100644 --- a/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs +++ b/src/OrchardCore/OrchardCore.Roles.Core/DefaultSystemRoleProvider.cs @@ -6,7 +6,7 @@ namespace OrchardCore.Roles; -internal sealed class DefaultSystemRoleProvider : ISystemRoleProvider +public sealed class DefaultSystemRoleProvider : ISystemRoleProvider { private readonly FrozenDictionary _systemRoles; private readonly Role _adminRole; diff --git a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs index b3351e5214d..813da08b72a 100644 --- a/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs +++ b/test/OrchardCore.Tests/Roles/DefaultSystemRoleProviderTests.cs @@ -1,4 +1,3 @@ -using Microsoft.Extensions.Localization; using OrchardCore.Environment.Shell; namespace OrchardCore.Roles.Tests;