further optimization of permissions - removed reference to Role to reduce API payload and minimize information disclosure

This commit is contained in:
Shaun Walker 2023-03-10 08:28:37 -05:00
parent 78adb24a75
commit ef4e99b3a7
7 changed files with 58 additions and 51 deletions

View File

@ -167,7 +167,7 @@
_permissions.Add(new Permission(ModuleState.SiteId, segments[0], segments[1], role, null, true));
}
// ensure admin access
if (!_permissions.Any(item => item.EntityName == segments[0] && item.PermissionName == segments[1] && item.Role.Name == RoleNames.Admin))
if (!_permissions.Any(item => item.EntityName == segments[0] && item.PermissionName == segments[1] && item.RoleName == RoleNames.Admin))
{
_permissions.Add(new Permission(ModuleState.SiteId, segments[0], segments[1], RoleNames.Admin, null, true));
}
@ -203,7 +203,7 @@
bool? isauthorized = null;
if (roleName != "")
{
var permission = _permissions.FirstOrDefault(item => item.EntityName == GetEntityName(permissionName) && item.PermissionName == GetPermissionName(permissionName) && item.Role.Name == roleName);
var permission = _permissions.FirstOrDefault(item => item.EntityName == GetEntityName(permissionName) && item.PermissionName == GetPermissionName(permissionName) && item.RoleName == roleName);
if (permission != null)
{
isauthorized = permission.IsAuthorized;
@ -243,7 +243,7 @@
{
if (roleName != "")
{
var permission = _permissions.FirstOrDefault(item => item.EntityName == GetEntityName(permissionName) && item.PermissionName == GetPermissionName(permissionName) && item.Role.Name == roleName);
var permission = _permissions.FirstOrDefault(item => item.EntityName == GetEntityName(permissionName) && item.PermissionName == GetPermissionName(permissionName) && item.RoleName == roleName);
if (permission != null)
{
_permissions.Remove(permission);
@ -307,7 +307,7 @@
{
// remove deny all users, unauthenticated, and registered users
var permissions = _permissions.Where(item => !item.IsAuthorized &&
(item.Role.Name == RoleNames.Everyone || item.Role.Name == RoleNames.Unauthenticated || item.Role.Name == RoleNames.Registered)).ToList();
(item.RoleName == RoleNames.Everyone || item.RoleName == RoleNames.Unauthenticated || item.RoleName == RoleNames.Registered)).ToList();
foreach (var permission in permissions)
{
_permissions.Remove(permission);
@ -316,7 +316,7 @@
{
// remove deny administrators and host users
permissions = _permissions.Where(item => !item.IsAuthorized &&
(item.Role.Name == RoleNames.Admin || item.Role.Name == RoleNames.Host)).ToList();
(item.RoleName == RoleNames.Admin || item.RoleName == RoleNames.Host)).ToList();
foreach (var permission in permissions)
{
_permissions.Remove(permission);
@ -325,7 +325,7 @@
{
// add administrators role if neither host or administrator is assigned
if (!_permissions.Any(item => item.EntityName == GetEntityName(permissionname) && item.PermissionName == GetPermissionName(permissionname) &&
(item.Role.Name == RoleNames.Admin || item.Role.Name == RoleNames.Host)))
(item.RoleName == RoleNames.Admin || item.RoleName == RoleNames.Host)))
{
_permissions.Add(new Permission(ModuleState.SiteId, GetEntityName(permissionname), GetPermissionName(permissionname), RoleNames.Admin, null, true));
}

View File

@ -137,11 +137,11 @@ namespace Oqtane.Themes.Controls
private async Task<string> Publish(string url, PageModule pagemodule)
{
var permissions = pagemodule.Module.PermissionList;
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Everyone))
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Everyone))
{
permissions.Add(new Permission(ModuleState.SiteId, EntityNames.Page, pagemodule.PageId, PermissionNames.View, RoleNames.Everyone, null, true));
}
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Registered))
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Registered))
{
permissions.Add(new Permission(ModuleState.SiteId, EntityNames.Page, pagemodule.PageId, PermissionNames.View, RoleNames.Registered, null, true));
}
@ -153,13 +153,13 @@ namespace Oqtane.Themes.Controls
private async Task<string> Unpublish(string url, PageModule pagemodule)
{
var permissions = pagemodule.Module.PermissionList;
if (permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Everyone))
if (permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Everyone))
{
permissions.Remove(permissions.First(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Everyone));
permissions.Remove(permissions.First(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Everyone));
}
if (permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Registered))
if (permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Registered))
{
permissions.Remove(permissions.First(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Registered));
permissions.Remove(permissions.First(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Registered));
}
pagemodule.Module.PermissionList = permissions;
await ModuleService.UpdateModuleAsync(pagemodule.Module);

View File

@ -537,11 +537,11 @@
if (UserSecurity.IsAuthorized(PageState.User, PermissionNames.Edit, PageState.Page.PermissionList))
{
var permissions = PageState.Page.PermissionList;
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Everyone))
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Everyone))
{
permissions.Add(new Permission(PageState.Site.SiteId, EntityNames.Page, PageState.Page.PageId, PermissionNames.View, RoleNames.Everyone, null, true));
}
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.Role.Name == RoleNames.Registered))
if (!permissions.Any(item => item.PermissionName == PermissionNames.View && item.RoleName == RoleNames.Registered))
{
permissions.Add(new Permission(PageState.Site.SiteId, EntityNames.Page, PageState.Page.PageId, PermissionNames.View, RoleNames.Registered, null, true));
}

View File

@ -300,7 +300,7 @@ namespace Oqtane.Repository
permission.EntityId = p.EntityId;
permission.PermissionName = p.PermissionName;
permission.RoleId = p.RoleId;
permission.Role = new Role { Name = p.Role.Name };
permission.RoleName = p.RoleName;
permission.UserId = p.UserId;
permission.IsAuthorized = p.IsAuthorized;
permissions.Add(permission);

View File

@ -30,10 +30,17 @@ namespace Oqtane.Repository
{
return _cache.GetOrCreate($"permissions:{alias.TenantId}:{siteId}:{entityName}", entry =>
{
var roles = _roles.GetRoles(siteId, true).ToList();
var permissions = _db.Permission.Where(item => item.SiteId == siteId).Where(item => item.EntityName == entityName).ToList();
foreach (var permission in permissions)
{
if (permission.RoleId != null)
{
permission.RoleName = roles.Find(item => item.RoleId == permission.RoleId).Name;
}
}
entry.SlidingExpiration = TimeSpan.FromMinutes(30);
return _db.Permission.Where(item => item.SiteId == siteId)
.Where(item => item.EntityName == entityName)
.Include(item => item.Role).ToList(); // eager load roles
return permissions;
});
}
return null;
@ -84,15 +91,14 @@ namespace Oqtane.Repository
permission.SiteId = siteId;
permission.EntityName = (string.IsNullOrEmpty(permission.EntityName)) ? entityName : permission.EntityName;
permission.EntityId = (permission.EntityName == entityName) ? entityId : -1;
if (permission.RoleId == null && permission.Role != null && !string.IsNullOrEmpty(permission.Role.Name))
if (permission.UserId == null && permission.RoleId == null && !string.IsNullOrEmpty(permission.RoleName))
{
var role = roles.FirstOrDefault(item => item.Name == permission.Role.Name);
var role = roles.FirstOrDefault(item => item.Name == permission.RoleName);
if (role != null)
{
permission.RoleId = role.RoleId;
}
}
permission.Role = null;
}
// add or update permissions
bool modified = false;
@ -112,7 +118,6 @@ namespace Oqtane.Repository
if (current.IsAuthorized != permission.IsAuthorized)
{
current.IsAuthorized = permission.IsAuthorized;
current.Role = null; // remove linked reference to Role which can cause errors in EF Core change tracking
_db.Entry(current).State = EntityState.Modified;
modified = true;
}
@ -129,7 +134,6 @@ namespace Oqtane.Repository
if (!permissions.Any(item => item.EntityName == permission.EntityName && item.PermissionName == permission.PermissionName
&& item.EntityId == permission.EntityId && item.RoleId == permission.RoleId && item.UserId == permission.UserId))
{
permission.Role = null; // remove linked reference to Role which can cause errors in EF Core change tracking
_db.Permission.Remove(permission);
modified = true;
}

View File

@ -1,3 +1,7 @@
using System.ComponentModel.DataAnnotations.Schema;
using System.Text.Json.Serialization;
using System;
namespace Oqtane.Models
{
/// <summary>
@ -17,47 +21,41 @@ namespace Oqtane.Models
public int SiteId { get; set; }
/// <summary>
/// Name of the Entity these permissions apply to.
/// Name of the Entity these permissions apply to (ie. Module )
/// </summary>
public string EntityName { get; set; }
/// <summary>
/// ID of the Entity these permissions apply to.
/// ID of the Entity these permissions apply to (ie. a ModuleId). A value of -1 indicates the permission applies to all EntityNames regardless of ID (ie. API permissions)
/// </summary>
public int EntityId { get; set; }
/// <summary>
/// What this permission is called.
/// TODO: todoc - must clarify what exactly this means, I assume any module can give it's own names for Permissions
/// Name of the permission (ie. View)
/// </summary>
public string PermissionName { get; set; }
/// <summary>
/// <see cref="Role"/> this permission applies to. So if all users in the Role _Customers_ have this permission, then it would reference that Role.
/// If null, then the permission doesn't target a role but probably a <see cref="User"/> (see <see cref="UserId"/>).
/// <see cref="Role"/> this permission applies to. If null then this is a <see cref="User"/> permission.
/// </summary>
public int? RoleId { get; set; }
/// <summary>
/// The role name associated to the RoleId.
/// </summary>
[NotMapped]
public string RoleName { get; set; }
/// <summary>
/// <see cref="User"/> this permission applies to.
/// If null, then the permission doesn't target a User but probably a <see cref="Role"/> (see <see cref="RoleId"/>).
/// <see cref="User"/> this permission applies to. If null then this is a <see cref="Role"/> permission.
/// </summary>
public int? UserId { get; set; }
/// <summary>
/// Determines if Authorization is sufficient to receive this permission.
/// The type of permission (ie. grant = true, deny = false)
/// </summary>
public bool IsAuthorized { get; set; }
/// <summary>
/// Reference to the <see cref="Role"/> based on the <see cref="RoleId"/> - can be nullable.
/// </summary>
/// <remarks>
/// It's not certain if this will always be populated. TODO: todoc/verify
/// </remarks>
public Role Role { get; set; }
public Permission()
{
}
@ -90,17 +88,22 @@ namespace Oqtane.Models
PermissionName = permissionName;
if (!string.IsNullOrEmpty(roleName))
{
Role = new Role { Name = roleName };
RoleId = null;
RoleName = roleName;
UserId = null;
}
else
{
Role = null;
RoleId = null;
RoleName = null;
UserId = userId;
}
IsAuthorized = isAuthorized;
}
[Obsolete("The Role property is deprecated", false)]
[NotMapped]
[JsonIgnore] // exclude from API payload
public Role Role { get; set; }
}
}

View File

@ -51,20 +51,20 @@ namespace Oqtane.Security
{
// check if denied first
isAuthorized = !permissionList.Where(item => !item.IsAuthorized && (
(item.Role != null && (
(item.Role.Name == RoleNames.Everyone) ||
(item.Role.Name == RoleNames.Unauthenticated && userId == -1) ||
roles.Split(';', StringSplitOptions.RemoveEmptyEntries).Contains(item.Role.Name))) ||
(item.UserId == null && (
(item.RoleName == RoleNames.Everyone) ||
(item.RoleName == RoleNames.Unauthenticated && userId == -1) ||
roles.Split(';', StringSplitOptions.RemoveEmptyEntries).Contains(item.RoleName))) ||
(item.UserId != null && item.UserId.Value == userId))).Any();
if (isAuthorized)
{
// then check if authorized
isAuthorized = permissionList.Where(item => item.IsAuthorized && (
(item.Role != null && (
(item.Role.Name == RoleNames.Everyone) ||
(item.Role.Name == RoleNames.Unauthenticated && userId == -1) ||
roles.Split(';', StringSplitOptions.RemoveEmptyEntries).Contains(item.Role.Name))) ||
(item.UserId == null && (
(item.RoleName == RoleNames.Everyone) ||
(item.RoleName == RoleNames.Unauthenticated && userId == -1) ||
roles.Split(';', StringSplitOptions.RemoveEmptyEntries).Contains(item.RoleName))) ||
(item.UserId != null && item.UserId.Value == userId))).Any();
}
}
@ -74,7 +74,7 @@ namespace Oqtane.Security
public static bool ContainsRole(List<Permission> permissions, string permissionName, string roleName)
{
return permissions.Any(item => item.PermissionName == permissionName && item.Role.Name == roleName);
return permissions.Any(item => item.PermissionName == permissionName && item.RoleName == roleName);
}
public static bool ContainsUser(List<Permission> permissions, string permissionName, int userId)