Skip to content

Commit e5e71c7

Browse files
#8266: Security sensitive permissions (Lombiq Technologies: ORCH-214) (#8874)
* Adding IsSecurityCritical property to Permission class and setting up Security Critical permissions * Setting up default hint for security critical permissions * Adding GetSecurityCriticalPermissions to IRoleService * Indicating security critical permissions in the role editor * Indicating roles that have security sensitive permissions in the UserRolesPart editor * Indicating roles that have security sensitive permissions in the Role Admin list * Code styling * Styling * Removing empty line * Code styling in Orchard.Roles/Views/Admin/Index.cshtml * Optimizing and standardizing the processing of security critical permissions / roles + code styling * Simplifying the GetSecurityCriticalPermissions method Co-authored-by: Benedek Farkas <benedek.farkas@lombiq.com> * Removing setting up a Hint for security critical permissions * Adding a meaningful summary for the IsSecurityCritical property of Permission * Removing the Hint property from the Permission --------- Co-authored-by: Benedek Farkas <benedek.farkas@lombiq.com>
1 parent df2b7dd commit e5e71c7

File tree

21 files changed

+183
-102
lines changed

21 files changed

+183
-102
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace Orchard.Core.Settings
66
{
77
public class Permissions : IPermissionProvider
88
{
9-
public static readonly Permission ManageSettings = new Permission { Description = "Manage Settings", Name = "ManageSettings" };
9+
public static readonly Permission ManageSettings = new Permission { Description = "Manage Settings", Name = "ManageSettings", IsSecurityCritical = true };
1010

1111
public virtual Feature Feature { get; set; }
1212

@@ -28,4 +28,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
2828
}
2929

3030
}
31-
}
31+
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ namespace Orchard.AuditTrail
66
{
77
public class Permissions : IPermissionProvider
88
{
9-
public static readonly Permission ViewAuditTrail = new Permission { Description = "View audit trail", Name = "ViewAuditTrail" };
10-
public static readonly Permission ManageAuditTrailSettings = new Permission { Description = "Manage audit trail settings", Name = "ManageAuditTrailSettings" };
11-
public static readonly Permission ImportAuditTrail = new Permission { Description = "Import audit trail", Name = "ImportAuditTrail" };
12-
public static readonly Permission ManageClientIpAddressSettings = new Permission { Description = "Manage client IP address settings", Name = "ManageClientIpAddressSettings" };
9+
public static readonly Permission ViewAuditTrail = new Permission { Description = "View audit trail", Name = "ViewAuditTrail", IsSecurityCritical = true };
10+
public static readonly Permission ManageAuditTrailSettings = new Permission { Description = "Manage audit trail settings", Name = "ManageAuditTrailSettings", IsSecurityCritical = true };
11+
public static readonly Permission ImportAuditTrail = new Permission { Description = "Import audit trail", Name = "ImportAuditTrail", IsSecurityCritical = true };
12+
public static readonly Permission ManageClientIpAddressSettings = new Permission { Description = "Manage client IP address settings", Name = "ManageClientIpAddressSettings", IsSecurityCritical = true };
1313

1414
public virtual Feature Feature { get; set; }
1515

@@ -35,4 +35,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
3535
};
3636
}
3737
}
38-
}
38+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace Orchard.ContentTypes
77
public class Permissions : IPermissionProvider
88
{
99
public static readonly Permission ViewContentTypes = new Permission { Name = "ViewContentTypes", Description = "View content types" };
10-
public static readonly Permission EditContentTypes = new Permission { Name = "EditContentTypes", Description = "Edit content types" };
10+
public static readonly Permission EditContentTypes = new Permission { Name = "EditContentTypes", Description = "Edit content types", IsSecurityCritical = true };
1111

1212
public virtual Feature Feature { get; set; }
1313

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ namespace Orchard.ImportExport
66
{
77
public class Permissions : IPermissionProvider
88
{
9-
public static readonly Permission Import = new Permission { Description = "Import Data", Name = "Import" };
10-
public static readonly Permission Export = new Permission { Description = "Export Data", Name = "Export" };
9+
public static readonly Permission Import = new Permission { Description = "Import Data", Name = "Import", IsSecurityCritical = true };
10+
public static readonly Permission Export = new Permission { Description = "Export Data", Name = "Export", IsSecurityCritical = true };
1111

1212
public virtual Feature Feature { get; set; }
1313

@@ -26,4 +26,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
2626
};
2727
}
2828
}
29-
}
29+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace Orchard.Modules
66
{
77
public class Permissions : IPermissionProvider
88
{
9-
public static readonly Permission ManageFeatures = new Permission { Description = "Manage Features", Name = "ManageFeatures" };
9+
public static readonly Permission ManageFeatures = new Permission { Description = "Manage Features", Name = "ManageFeatures", IsSecurityCritical = true };
1010

1111
public virtual Feature Feature { get; set; }
1212

@@ -25,4 +25,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
2525
};
2626
}
2727
}
28-
}
28+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace Orchard.Packaging
66
{
77
public class Permissions : IPermissionProvider
88
{
9-
public static readonly Permission ManagePackages = new Permission { Description = "Manage packages", Name = "ManagePackages" };
9+
public static readonly Permission ManagePackages = new Permission { Description = "Manage packages", Name = "ManagePackages", IsSecurityCritical = true };
1010

1111
public virtual Feature Feature { get; set; }
1212

@@ -21,4 +21,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
2121
return new List<PermissionStereotype>();
2222
}
2323
}
24-
}
24+
}

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,16 @@ public ActionResult Index()
6565
if (!Services.Authorizer.Authorize(Permissions.ManageRoles, T("Not authorized to manage roles")))
6666
return new HttpUnauthorizedResult();
6767

68-
var model = new RolesIndexViewModel { Rows = _roleService.GetRoles().OrderBy(r => r.Name).ToList() };
68+
var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
69+
var roles = _roleService.GetRoles().OrderBy(r => r.Name).ToList();
70+
71+
var model = new RolesIndexViewModel
72+
{
73+
Rows = roles,
74+
RolesWithSecurityCriticalPermissions = roles.ToDictionary(
75+
role => role.Name,
76+
role => role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name)))
77+
};
6978

7079
return View(model);
7180
}
@@ -250,18 +259,21 @@ public ActionResult Assign(int id)
250259
{
251260
return new HttpUnauthorizedResult();
252261
}
262+
263+
var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
253264
// create the ViewModel used to manage a user's roles
254265
var model = new UserRolesViewModel
255266
{
256267
User = userRolesPart.As<IUser>(),
257268
UserRoles = userRolesPart,
258-
Roles = allRoles.Select(x => new UserRoleEntry
269+
Roles = allRoles.Select(role => new UserRoleEntry
259270
{
260-
RoleId = x.Id,
261-
Name = x.Name,
262-
Granted = userRolesPart.Roles.Contains(x.Name)
271+
RoleId = role.Id,
272+
Name = role.Name,
273+
Granted = userRolesPart.Roles.Contains(role.Name),
274+
HasSecurityCriticalPermissions = role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name))
263275
}).ToList(),
264-
AuthorizedRoleIds = authorizedRoleIds
276+
AuthorizedRoleIds = authorizedRoleIds,
265277
};
266278

267279
// this calls the same view used by the driver that lets users with higher

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,22 @@ protected override DriverResult Editor(UserRolesPart userRolesPart, dynamic shap
6767
{
6868
return null;
6969
}
70-
var allRoles = _allRoles.Value
71-
.Select(x => new UserRoleEntry
72-
{
73-
RoleId = x.Id,
74-
Name = x.Name,
75-
Granted = userRolesPart.Roles.Contains(x.Name)
76-
});
70+
71+
var securityCriticalPermissions = _roleService.GetSecurityCriticalPermissions().ToHashSet();
7772
var model = new UserRolesViewModel
7873
{
7974
User = userRolesPart.As<IUser>(),
8075
UserRoles = userRolesPart,
81-
Roles = allRoles.ToList(),
82-
AuthorizedRoleIds = authorizedRoleIds
76+
Roles = _allRoles.Value.Select(role => new UserRoleEntry
77+
{
78+
RoleId = role.Id,
79+
Name = role.Name,
80+
Granted = userRolesPart.Roles.Contains(role.Name),
81+
HasSecurityCriticalPermissions = role.RolesPermissions.Any(p => securityCriticalPermissions.Contains(p.Permission.Name))
82+
}).ToList(),
83+
AuthorizedRoleIds = authorizedRoleIds,
8384
};
85+
8486
return shapeHelper.EditorTemplate(TemplateName: TemplateName, Model: model, Prefix: Prefix);
8587
});
8688

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ public class Permissions : IPermissionProvider
1212
{
1313
private readonly IRepository<RoleRecord> _roleRepository;
1414

15-
public static readonly Permission ManageRoles = new Permission { Description = "Managing Roles", Name = "ManageRoles" };
16-
public static readonly Permission AssignRoles = new Permission { Description = "Assign Roles", Name = "AssignRoles", ImpliedBy = new[] { ManageRoles } };
15+
public static readonly Permission ManageRoles = new Permission { Description = "Managing Roles", Name = "ManageRoles", IsSecurityCritical = true, };
16+
public static readonly Permission AssignRoles = new Permission { Description = "Assign Roles", Name = "AssignRoles", IsSecurityCritical = true, ImpliedBy = new[] { ManageRoles } };
1717

1818
public virtual Feature Feature { get; set; }
1919

@@ -80,4 +80,4 @@ public IEnumerable<PermissionStereotype> GetDefaultStereotypes()
8080
}
8181

8282
}
83-
}
83+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public interface IRoleService : IDependency
1414
void UpdateRole(int id, string roleName, IEnumerable<string> rolePermissions);
1515
void DeleteRole(int id);
1616
IDictionary<string, IEnumerable<Permission>> GetInstalledPermissions();
17+
IEnumerable<string> GetSecurityCriticalPermissions();
1718
IEnumerable<string> GetPermissionsForRole(int id);
1819

1920
IEnumerable<string> GetPermissionsForRoleByName(string name);
@@ -26,4 +27,4 @@ public interface IRoleService : IDependency
2627
/// <returns>Returns false if a role with the given name already exits</returns>
2728
bool VerifyRoleUnicity(string name);
2829
}
29-
}
30+
}

0 commit comments

Comments
 (0)