Simplify how to check if the user is an admin (#16866)

src/OrchardCore.Modules/OrchardCore.Roles/Recipes/RolesStep.cs

@@ -4,7 +4,6 @@
 using OrchardCore.Recipes.Services;
 using OrchardCore.Security;
 using OrchardCore.Security.Permissions;
-using OrchardCore.Security.Services;
 
 namespace OrchardCore.Roles.Recipes;
 
@@ -14,15 +13,15 @@ namespace OrchardCore.Roles.Recipes;
 public sealed class RolesStep : NamedRecipeStepHandler
 {
     private readonly RoleManager<IRole> _roleManager;
-    private readonly IRoleService _roleService;
+    private readonly ISystemRoleNameProvider _systemRoleNameProvider;
 
     public RolesStep(
         RoleManager<IRole> roleManager,
-        IRoleService roleService)
+        ISystemRoleNameProvider systemRoleNameProvider)
         : base("Roles")
     {
         _roleManager = roleManager;
-        _roleService = roleService;
+        _systemRoleNameProvider = systemRoleNameProvider;
     }
 
     protected override async Task HandleAsync(RecipeExecutionContext context)
@@ -54,7 +53,7 @@ protected override async Task HandleAsync(RecipeExecutionContext context)
                 r.RoleDescription = roleEntry.Description;
                 r.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType);
 
-                if (!await _roleService.IsAdminRoleAsync(roleName))
+                if (!await _systemRoleNameProvider.IsAdminRoleAsync(roleName))
                 {
                     r.RoleClaims.AddRange(roleEntry.Permissions.Select(RoleClaim.Create));
                 }

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

@@ -2,7 +2,6 @@
 using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.Options;
 using OrchardCore.Security;
-using OrchardCore.Security.Services;
 using OrchardCore.Users;
 using OrchardCore.Users.Services;
 
@@ -12,18 +11,18 @@ public class RoleClaimsProvider : IUserClaimsProvider
 {
     private readonly UserManager<IUser> _userManager;
     private readonly RoleManager<IRole> _roleManager;
-    private readonly IRoleService _roleService;
+    private readonly ISystemRoleNameProvider _systemRoleNameProvider;
     private readonly IdentityOptions _identityOptions;
 
     public RoleClaimsProvider(
         UserManager<IUser> userManager,
         RoleManager<IRole> roleManager,
-        IRoleService roleService,
+        ISystemRoleNameProvider systemRoleNameProvider,
         IOptions<IdentityOptions> identityOptions)
     {
         _userManager = userManager;
         _roleManager = roleManager;
-        _roleService = roleService;
+        _systemRoleNameProvider = systemRoleNameProvider;
         _identityOptions = identityOptions.Value;
     }
 
@@ -34,15 +33,22 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims)
             return;
         }
 
+        var isAdministrator = false;
+
+        if (await _userManager.IsInRoleAsync(user, await _systemRoleNameProvider.GetAdminRoleAsync()))
+        {
+            claims.AddClaim(StandardClaims.SiteOwner);
+
+            isAdministrator = true;
+        }
+
         var roleNames = await _userManager.GetRolesAsync(user);
-        var roles = new List<IRole>();
-        var addClaims = true;
 
         foreach (var roleName in roleNames)
         {
             claims.AddClaim(new Claim(_identityOptions.ClaimsIdentity.RoleClaimType, roleName));
 
-            if (!_roleManager.SupportsRoleClaims)
+            if (isAdministrator || !_roleManager.SupportsRoleClaims)
             {
                 continue;
             }
@@ -54,21 +60,6 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims)
                 continue;
             }
 
-            if (addClaims && await _roleService.IsAdminRoleAsync(role.RoleName))
-            {
-                addClaims = false;
-            }
-
-            roles.Add(role);
-        }
-
-        if (roles.Count == 0 || !addClaims)
-        {
-            return;
-        }
-
-        foreach (var role in roles)
-        {
             claims.AddClaims(await _roleManager.GetClaimsAsync(role));
         }
     }

src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs

@@ -10,7 +10,6 @@
 using OrchardCore.Modules;
 using OrchardCore.Navigation;
 using OrchardCore.Recipes;
-using OrchardCore.Roles.Core;
 using OrchardCore.Roles.Deployment;
 using OrchardCore.Roles.Migrations;
 using OrchardCore.Roles.Recipes;
@@ -35,7 +34,6 @@ public override void ConfigureServices(IServiceCollection services)
     {
         services.AddScoped<IUserClaimsProvider, RoleClaimsProvider>();
         services.AddDataMigration<RolesMigrations>();
-        services.AddScoped<IAuthorizationHandler, RolesPermissionHandler>();
         services.AddScoped<RoleStore>();
         services.Replace(ServiceDescriptor.Scoped<IRoleClaimStore<IRole>>(sp => sp.GetRequiredService<RoleStore>()));
         services.Replace(ServiceDescriptor.Scoped<IRoleStore<IRole>>(sp => sp.GetRequiredService<RoleStore>()));

src/OrchardCore.Modules/OrchardCore.Settings/Services/SuperUserHandler.cs

@@ -18,7 +18,21 @@ public SuperUserHandler(ISiteService siteService)
 
     public async Task HandleAsync(AuthorizationHandlerContext context)
     {
-        var userId = context?.User?.FindFirstValue(ClaimTypes.NameIdentifier);
+        var user = context?.User;
+
+        if (user == null)
+        {
+            return;
+        }
+
+        if (user.HasClaim(StandardClaims.SiteOwner.Type, StandardClaims.SiteOwner.Value))
+        {
+            SucceedAllRequirements(context);
+
+            return;
+        }
+
+        var userId = user.FindFirstValue(ClaimTypes.NameIdentifier);
         if (userId == null)
         {
             return;

src/OrchardCore.Modules/OrchardCore.Users/Liquid/HasPermissionFilter.cs

@@ -23,13 +23,32 @@ public async ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArgument
         if (input.ToObjectValue() is LiquidUserAccessor)
         {
             var user = _httpContextAccessor.HttpContext?.User;
-            if (user != null)
+            if (user != null && arguments.Count > 0)
             {
                 var permissionName = arguments["permission"].Or(arguments.At(0)).ToStringValue();
-                var resource = arguments["resource"].Or(arguments.At(1)).ToObjectValue();
 
-                if (!string.IsNullOrEmpty(permissionName) &&
-                    await _authorizationService.AuthorizeAsync(user, new Permission(permissionName), resource))
+                if (string.IsNullOrWhiteSpace(permissionName))
+                {
+                    return BooleanValue.False;
+                }
+
+                var permission = new Permission(permissionName);
+
+                if (arguments.Count > 1)
+                {
+                    var resource = arguments["resource"].Or(arguments.At(1)).ToObjectValue();
+
+                    if (resource != null)
+                    {
+                        if (!string.IsNullOrEmpty(permissionName) &&
+                            await _authorizationService.AuthorizeAsync(user, permission, resource))
+                        {
+                            return BooleanValue.True;
+                        }
+                    }
+                }
+
+                if (await _authorizationService.AuthorizeAsync(user, permission))
                 {
                     return BooleanValue.True;
                 }

src/OrchardCore.Modules/OrchardCore.Users/Liquid/UserFilters.cs

@@ -2,18 +2,16 @@
 using Fluid;
 using Fluid.Values;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Options;
 using OrchardCore.Liquid;
-using OrchardCore.Roles;
+using OrchardCore.Security;
 using OrchardCore.Security.Permissions;
 
 namespace OrchardCore.Users.Liquid;
 
 public static class UserFilters
 {
-    public static async ValueTask<FluidValue> HasClaim(FluidValue input, FilterArguments arguments, TemplateContext ctx)
+    public static ValueTask<FluidValue> HasClaim(FluidValue input, FilterArguments arguments, TemplateContext ctx)
     {
         if (input.ToObjectValue() is LiquidUserAccessor)
         {
@@ -31,21 +29,24 @@ public static async ValueTask<FluidValue> HasClaim(FluidValue input, FilterArgum
                     return BooleanValue.True;
                 }
 
-                if (string.Equals(claimType, Permission.ClaimType, StringComparison.OrdinalIgnoreCase))
+                // The following if condition was added in 2.1 for backward compatibility. It should be removed in v3 and documented as a breaking change.
+                // The change log should state the following:
+                // The `Administrator` role no longer registers permission-based claims by default during login. This means that directly checking for specific claims in Liquid, such as:
+                //
+                // ```liquid
+                // {% assign isAuthorized = User | has_claim: "Permission", "AccessAdminPanel" %}
+                // ```
+                //
+                // will return `false` for administrators, even though they still have full access. Non-admin users, however, may return `true` if they have the claim. 
+                // it's important to use the `has_permission` filter for permission checks going forward:
+                //
+                // ```liquid
+                // {% assign isAuthorized = User | has_permission: "AccessAdminPanel" %}
+                // ```
+                if (string.Equals(claimType, Permission.ClaimType, StringComparison.OrdinalIgnoreCase) &&
+                    user.HasClaim(StandardClaims.SiteOwner.Type, StandardClaims.SiteOwner.Value))
                 {
-                    var systemRoleNameProvider = context.Services.GetService<ISystemRoleNameProvider>();
-
-                    if (systemRoleNameProvider != null)
-                    {
-                        // Administrator users do not register individual permissions during login.
-                        // However, they are designed to automatically have all application permissions granted.
-                        var identityOptions = context.Services.GetRequiredService<IOptions<IdentityOptions>>().Value;
-
-                        if (user.HasClaim(identityOptions.ClaimsIdentity.RoleClaimType, await systemRoleNameProvider.GetAdminRoleAsync()))
-                        {
-                            return BooleanValue.True;
-                        }
-                    }
+                    return BooleanValue.True;
                 }
             }
         }

src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/StandardClaims.cs

@@ -0,0 +1,11 @@
+using System.Security.Claims;
+
+namespace OrchardCore.Security;
+
+public static class StandardClaims
+{
+    /// <summary>
+    /// This claim is assigned by the system during the login process if the user belongs to the Administrator role.
+    /// </summary>
+    public static readonly Claim SiteOwner = new("SiteOwner", "true");
+}

src/docs/reference/modules/Liquid/README.md

@@ -410,7 +410,6 @@ You can use this filter to load the user information of the current authenticate
 {% assign user = User | user_id | users_by_id %}
 
 {{ user.UserName }} - {{ user.Email }}
-
 ```
 
 You can use this filter with the UserPicker field to load the picked user's information.
@@ -421,34 +420,35 @@ You can use this filter with the UserPicker field to load the picked user's info
 {% for user in users %}
   {{ user.UserName }} - {{ user.Email }}
 {% endfor %}
-
 ```
 
 #### User has_permission filter
 
 Checks if the User has permission clearance, optionally on a resource
 
 ```liquid
-{{ User | has_permission:"EditContent",Model.ContentItem }}
+{{ User | has_permission: "EditContent", Model.ContentItem }}
 ```
 
 #### User is_in_role filter
 
 Checks if the user is in role
 
 ```liquid
-{{ User | is_in_role:"Administrator" }}
+{{ User | is_in_role: "Administrator" }}
 ```
 
 #### User has_claim filter
 
 Checks if the user has a claim of the specified type
 
 ```liquid
-{{ User | has_claim:"email_verified","true" }}
-{{ User | has_claim:"Permission","ManageSettings" }}
+{{ User | has_claim: "email_verified", "true" }}
 ```
 
+!!! warning
+    To avoid false negatives for Administrator users, ensure you use the `has_permission` filter instead of `has_claim` when checking if a user has a permission. This ensures accurate permission evaluation, especially for administrators who may not have explicit claims but still possess full access rights.
+
 ### Site
 
 Gives access to the current site settings, e.g `Site.SiteName`.