#8266: Security sensitive permissions (Lombiq Technologies: ORCH-214)

src/Orchard.Web/Core/Settings/Permissions.cs

@@ -6,7 +6,7 @@ namespace Orchard.Core.Settings
 {
     public class Permissions : IPermissionProvider
     {
-        public static readonly Permission ManageSettings = new Permission { Description = "Manage Settings", Name = "ManageSettings" };
+        public static readonly Permission ManageSettings = new Permission { Description = "Manage Settings", Name = "ManageSettings", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 
@@ -28,4 +28,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
         }
 
     }
-}
+}

src/Orchard.Web/Modules/Orchard.AuditTrail/Permissions.cs

@@ -6,10 +6,10 @@ namespace Orchard.AuditTrail
 {
     public class Permissions : IPermissionProvider
     {
-        public static readonly Permission ViewAuditTrail = new Permission { Description = "View audit trail", Name = "ViewAuditTrail" };
-        public static readonly Permission ManageAuditTrailSettings = new Permission { Description = "Manage audit trail settings", Name = "ManageAuditTrailSettings" };
-        public static readonly Permission ImportAuditTrail = new Permission { Description = "Import audit trail", Name = "ImportAuditTrail" };
-        public static readonly Permission ManageClientIpAddressSettings = new Permission { Description = "Manage client IP address settings", Name = "ManageClientIpAddressSettings" };
+        public static readonly Permission ViewAuditTrail = new Permission { Description = "View audit trail", Name = "ViewAuditTrail", IsSecurityCritical = true };
+        public static readonly Permission ManageAuditTrailSettings = new Permission { Description = "Manage audit trail settings", Name = "ManageAuditTrailSettings", IsSecurityCritical = true };
+        public static readonly Permission ImportAuditTrail = new Permission { Description = "Import audit trail", Name = "ImportAuditTrail", IsSecurityCritical = true };
+        public static readonly Permission ManageClientIpAddressSettings = new Permission { Description = "Manage client IP address settings", Name = "ManageClientIpAddressSettings", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 
@@ -35,4 +35,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
             };
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.ContentTypes/Permissions.cs

@@ -7,7 +7,7 @@ namespace Orchard.ContentTypes
     public class Permissions : IPermissionProvider
     {
         public static readonly Permission ViewContentTypes = new Permission { Name = "ViewContentTypes", Description = "View content types" };
-        public static readonly Permission EditContentTypes = new Permission { Name = "EditContentTypes", Description = "Edit content types" };
+        public static readonly Permission EditContentTypes = new Permission { Name = "EditContentTypes", Description = "Edit content types", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 

src/Orchard.Web/Modules/Orchard.ImportExport/Permissions.cs

@@ -6,8 +6,8 @@ namespace Orchard.ImportExport
 {
     public class Permissions : IPermissionProvider
     {
-        public static readonly Permission Import = new Permission { Description = "Import Data", Name = "Import" };
-        public static readonly Permission Export = new Permission { Description = "Export Data", Name = "Export" };
+        public static readonly Permission Import = new Permission { Description = "Import Data", Name = "Import", IsSecurityCritical = true };
+        public static readonly Permission Export = new Permission { Description = "Export Data", Name = "Export", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 
@@ -26,4 +26,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
                          };
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Modules/Permissions.cs

@@ -6,7 +6,7 @@ namespace Orchard.Modules
 {
     public class Permissions : IPermissionProvider
     {
-        public static readonly Permission ManageFeatures = new Permission { Description = "Manage Features", Name = "ManageFeatures" };
+        public static readonly Permission ManageFeatures = new Permission { Description = "Manage Features", Name = "ManageFeatures", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 
@@ -25,4 +25,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
                          };
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Packaging/Permissions.cs

@@ -6,7 +6,7 @@ namespace Orchard.Packaging
 {
     public class Permissions : IPermissionProvider
     {
-        public static readonly Permission ManagePackages = new Permission { Description = "Manage packages", Name = "ManagePackages" };
+        public static readonly Permission ManagePackages = new Permission { Description = "Manage packages", Name = "ManagePackages", IsSecurityCritical = true };
 
         public virtual Feature Feature { get; set; }
 
@@ -21,4 +21,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
             return new List<PermissionStereotype>();
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Roles/Controllers/AdminController.cs

@@ -65,7 +65,16 @@ public ActionResult Index()
             if (!Services.Authorizer.Authorize(Permissions.ManageRoles, T("Not authorized to manage roles")))
                 return new HttpUnauthorizedResult();
 
-            var model = new RolesIndexViewModel { Rows = _roleService.GetRoles().OrderBy(r => r.Name).ToList() };
+            var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
+            var roles = _roleService.GetRoles().OrderBy(r => r.Name).ToList();
+
+            var model = new RolesIndexViewModel
+            {
+                Rows = roles,
+                RolesWithSecurityCriticalPermissions = roles.ToDictionary(
+                    role => role.Name,
+                    role => role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name)))
+            };
 
             return View(model);
         }
@@ -250,18 +259,21 @@ public ActionResult Assign(int id)
             {
                 return new HttpUnauthorizedResult();
             }
+
+            var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
             // create the ViewModel used to manage a user's roles
             var model = new UserRolesViewModel
             {
                 User = userRolesPart.As<IUser>(),
                 UserRoles = userRolesPart,
-                Roles = allRoles.Select(x => new UserRoleEntry
+                Roles = allRoles.Select(role => new UserRoleEntry
                 {
-                    RoleId = x.Id,
-                    Name = x.Name,
-                    Granted = userRolesPart.Roles.Contains(x.Name)
+                    RoleId = role.Id,
+                    Name = role.Name,
+                    Granted = userRolesPart.Roles.Contains(role.Name),
+                    HasSecurityCriticalPermissions = role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name))
                 }).ToList(),
-                AuthorizedRoleIds = authorizedRoleIds
+                AuthorizedRoleIds = authorizedRoleIds,
             };
 
             // this calls the same view used by the driver that lets users with higher

src/Orchard.Web/Modules/Orchard.Roles/Drivers/UserRolesPartDriver.cs

@@ -67,20 +67,22 @@ protected override DriverResult Editor(UserRolesPart userRolesPart, dynamic shap
                 {
                     return null;
                 }
-                var allRoles = _allRoles.Value
-                    .Select(x => new UserRoleEntry
-                    {
-                        RoleId = x.Id,
-                        Name = x.Name,
-                        Granted = userRolesPart.Roles.Contains(x.Name)
-                    });
+
+                var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
                 var model = new UserRolesViewModel
                 {
                     User = userRolesPart.As<IUser>(),
                     UserRoles = userRolesPart,
-                    Roles = allRoles.ToList(),
-                    AuthorizedRoleIds = authorizedRoleIds
+                    Roles = _allRoles.Value.Select(role => new UserRoleEntry
+                    {
+                        RoleId = role.Id,
+                        Name = role.Name,
+                        Granted = userRolesPart.Roles.Contains(role.Name),
+                        HasSecurityCriticalPermissions = role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name))
+                    }).ToList(),
+                    AuthorizedRoleIds = authorizedRoleIds,
                 };
+
                 return shapeHelper.EditorTemplate(TemplateName: TemplateName, Model: model, Prefix: Prefix);
             });
 

src/Orchard.Web/Modules/Orchard.Roles/Permissions.cs

@@ -12,8 +12,8 @@ public class Permissions : IPermissionProvider
     {
         private readonly IRepository<RoleRecord> _roleRepository;
 
-        public static readonly Permission ManageRoles = new Permission { Description = "Managing Roles", Name = "ManageRoles" };
-        public static readonly Permission AssignRoles = new Permission { Description = "Assign Roles", Name = "AssignRoles", ImpliedBy = new[] { ManageRoles } };
+        public static readonly Permission ManageRoles = new Permission { Description = "Managing Roles", Name = "ManageRoles", IsSecurityCritical = true, };
+        public static readonly Permission AssignRoles = new Permission { Description = "Assign Roles", Name = "AssignRoles", IsSecurityCritical = true, ImpliedBy = new[] { ManageRoles } };
 
         public virtual Feature Feature { get; set; }
 
@@ -80,4 +80,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
         }
 
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Roles/Services/IRoleService.cs

@@ -14,6 +14,7 @@ public interface IRoleService : IDependency
         void UpdateRole(int id, string roleName, IEnumerable<string> rolePermissions);
         void DeleteRole(int id);
         IDictionary<string, IEnumerable<Permission>> GetInstalledPermissions();
+        IEnumerable<string> GetSecurityCriticalPermissions();
         IEnumerable<string> GetPermissionsForRole(int id);
 
         IEnumerable<string> GetPermissionsForRoleByName(string name);
@@ -26,4 +27,4 @@ public interface IRoleService : IDependency
         /// <returns>Returns false if a role with the given name already exits</returns>
         bool VerifyRoleUnicity(string name);
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Roles/Services/RoleService.cs

@@ -203,6 +203,9 @@ public IDictionary<string, IEnumerable<Permission>> GetInstalledPermissions()
             return installedPermissions;
         }
 
+        public IEnumerable<string> GetSecurityCriticalPermissions() =>
+            _permissionProviders.SelectMany(pp => pp.GetPermissions().Where(p => p.IsSecurityCritical)).Select(p => p.Name);
+
         public IEnumerable<string> GetPermissionsForRole(int id)
         {
             var permissions = new List<string>();
@@ -251,4 +254,4 @@ private void TriggerSignal()
             _signals.Trigger(SignalName);
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Roles/ViewModels/RolesIndexViewModel.cs

@@ -6,5 +6,7 @@ namespace Orchard.Roles.ViewModels
     public class RolesIndexViewModel
     {
         public IList<RoleRecord> Rows { get; set; }
+
+        public Dictionary<string, bool> RolesWithSecurityCriticalPermissions { get; set; }
     }
 }

src/Orchard.Web/Modules/Orchard.Roles/ViewModels/UserRolesViewModel.cs

@@ -23,5 +23,6 @@ public class UserRoleEntry
         public int RoleId { get; set; }
         public string Name { get; set; }
         public bool Granted { get; set; }
+        public bool HasSecurityCriticalPermissions { get; set; }
     }
 }

src/Orchard.Web/Modules/Orchard.Roles/Views/Admin/Edit.cshtml

@@ -1,67 +1,93 @@
+@using Orchard.Roles.ViewModels
+
 @model RoleEditViewModel
-@using Orchard.Roles.ViewModels;
 
-@{ Layout.Title = T("Edit Role").ToString(); }
+@{
+    Style.Require("FontAwesome");
+    Layout.Title = T("Edit Role").ToString();
+}
 
-@using(Html.BeginFormAntiForgeryPost()) {
+@using (Html.BeginFormAntiForgeryPost())
+{
     @Html.ValidationSummary();
     <fieldset>
         <legend>@T("Information")</legend>
         <label for="pageTitle">@T("Role Name:")</label>
-        @if (Model.Name == "Administrator") { // the one special case
+        @if (Model.Name == "Administrator")
+        { // the one special case
             <input id="Name" class="text" type="text" value="@Model.Name" readonly="readonly" />
             <input class="text" name="Name" type="hidden" value="@Model.Name" />
         }
-        else {
+        else
+        {
             <input id="Name" class="text" name="Name" type="text" value="@Model.Name" />
         }
         <input type="hidden" value="@Model.Id" name="Id" />
     </fieldset>
     <fieldset>
         <legend>@T("Permissions")</legend>
-        @foreach (var category in Model.RoleCategoryPermissions.Keys) {
-        <fieldset>
-            <legend>@category</legend>
-            <table class="items">
-                <colgroup>
-                    <col id="Col1" />
-                    <col id="Col2" />
-                </colgroup>
-                <thead>
-                    <tr>
-                        <th scope="col">@T("Permission")</th>
-                        <th scope="col">@T("Allow")</th>
-                        <th scope="col">@T("Effective")</th>
-                    </tr>
-                </thead>
-                @foreach (var permission in Model.RoleCategoryPermissions[category]) {
-                <tr>
-                    <td>@T(permission.Description)</td>
-                    <td style="width:60px;">
-                        @if (Model.CurrentPermissions.Contains(permission.Name)) {
-                            <input type="checkbox" value="true" name="Checkbox.@permission.Name" checked="checked"/>
-                         } 
-                         else {
-                            <input type="checkbox" value="true" name="Checkbox.@permission.Name"/>
-                        }
-                    </td>    
-                    <td style="width:60px;">
-                    @if (Model.EffectivePermissions.Contains(permission.Name)) {
-                            <input type="checkbox" disabled="disabled" name="Effective.@permission.Name" checked="checked"/>
-                     } else {
-                            <input type="checkbox" disabled="disabled" name="Effective.@permission.Name"/>
-                        }
-                    </td>                
-                </tr>
-                }
-            </table>
+
+        <p>@T("Allow: Permission is granted explicitly.")</p>
+        <p>@T("Effective: Permission is implied by a higher level permission, or explicitly granted.")</p>
+
+        <br />
+
+        @foreach (var category in Model.RoleCategoryPermissions.Keys)
+        {
+            <fieldset>
+                <legend>@category</legend>
+                <table class="items">
+                    <colgroup>
+                        <col id="Col1" />
+                        <col id="Col2" />
+                    </colgroup>
+                    <thead>
+                        <tr>
+                            <th scope="col">@T("Permission")</th>
+                            <th scope="col">@T("Allow")</th>
+                            <th scope="col">@T("Effective")</th>
+                        </tr>
+                    </thead>
+                    @foreach (var permission in Model.RoleCategoryPermissions[category])
+                    {
+                        <tr>
+                            <td>@T(permission.Description)</td>
+                            <td style="width:60px;">
+                                @if (Model.CurrentPermissions.Contains(permission.Name))
+                                {
+                                    <input type="checkbox" value="true" name="Checkbox.@permission.Name" checked="checked" />
+                                }
+                                else
+                                {
+                                    <input type="checkbox" value="true" name="Checkbox.@permission.Name" />
+                                }
+                                @if (permission.IsSecurityCritical)
+                                {
+                                    <i class="fa-solid fa-triangle-exclamation" style="color: red"
+                                       title="@T("This permission could allow a user to elevate their other permissions. Grant it with extreme consideration.")"></i>
+                                }
+                            </td>
+                            <td style="width:60px;">
+                                @if (Model.EffectivePermissions.Contains(permission.Name))
+                                {
+                                    <input type="checkbox" disabled="disabled" name="Effective.@permission.Name" checked="checked" />
+                                }
+                                else
+                                {
+                                    <input type="checkbox" disabled="disabled" name="Effective.@permission.Name" />
+                                }
+                            </td>
+                        </tr>
+                    }
+                </table>
             </fieldset>
-            }
+        }
     </fieldset>
     <fieldset>
         <button class="primaryAction" type="submit" name="submit.Save" value="@T("Save")">@T("Save")</button>
-        @if (Model.Name != "Administrator") {
-        <button type="submit" name="submit.Delete" value="@T("Delete")" itemprop="RemoveUrl">@T("Delete")</button>
+        @if (Model.Name != "Administrator")
+        {
+            <button type="submit" name="submit.Delete" value="@T("Delete")" itemprop="RemoveUrl">@T("Delete")</button>
         }
     </fieldset>
-}
+}