Checking for user permissions

Question

The idea here is we have Roles, Permissions, and a table called PermissionRoles that connect the two. So a Permissions can be in many Roles, and many Permissions can have the same Role. So what the following code does is:

1. Get all the permissions required for a user to authenticate, by ActionName
2. Loop through each of these permissions
3. Get the list PermissionsRole records that have one of those Permission objects
4. Get the list of Roles from the list of PermissionRoles
5. Loop through that list of Roles
6. Check if a user is in any of those roles

This is the code, I feel like there may be a more efficient way to write this. It is implied that I have instantiated a UserManager and Database Context.

string[] permissions = Permissions.Split(',').ToArray();

IEnumerable<string> perms = permissions.Intersect(db.Permissions.Select(p => p.ActionName));
List<IdentityRole> roles = new List<IdentityRole>();

if (perms.Count() > 0)
{
    foreach (var item in perms)
    {
        var currentUserId = httpContext.User.Identity.GetUserId();

        var permissionsRoles = db.PermissionRoles.Where(p => p.Permission.ActionName == item && p.CompanyId == companyId).ToList();
        var existingRoles = dbu.Roles.Select(x => x.Id).Intersect(permissionsRoles.Select(x => x.RoleId)).ToList();
        foreach (var role in existingRoles)
        {
            ApplicationRole thisRole = dbu.Roles.Find(role);
            if (userManager.IsInRole(currentUserId, thisRole.Name))
            {
                return true;
            }
        }
    }
}
return false;

Answer 1

I would say you do too many things at once. The important thing here is this check:

if (userManager.IsInRole(currentUserId, thisRole.Name))
{
    return true;
}

This needs a list of role-names to iterate over:

foreach (var roleName in roleNames)
{
    if (userManager.IsInRole(userId, roleName))
    {
        return true;
    }
}

return false;

The list of role-names seems to be dependent of some given permissions and a company:

var givenPermissions = GetGivenPermissions();
var roleNames = GetRoleNames(companyId, givenPermissions);

The first is easy enough to extract from the given code:

IEnumerable<string> GetGivenPermissions()
{
    return Permissions.Split(',').ToArray();
}

The other one is trickier, but I think I got it right:

IEnumerable<string> GetRoleNames(string companyId, IEnumerable<string> givenPermissions)
{
    var existingPermissions = db.Permissions.Select(p => p.ActionName);
    var relevantPermissions = givenPermissions.Intersect(existingPermissions);

    foreach (var permission in relevantPermissions)
    {
        var allRoles = db.Roles.Select(x => x.Id);
        var relevantPermissionRoles = db.PermissionRoles
                                        .Where(p => p.Permission.ActionName == permission && p.CompanyId == companyId)
                                        .Select(x => x.RoleId)
                                        .ToList();

        var relevantRoles = allRoles.Intersect(relevantPermissionRoles).ToList();

        foreach (var role in relevantRoles)
        {
            var thisRole = db.Roles.Find(role);
            var name = thisRole.Name;
            yield return name;
        }
    }
}

This is one nasty query:

• It's a good mix of things querying the database and things filtered in memory. - It's hard to figure out what it actually do, and what the requirements for the roles really are.
• It's also a mix of code operating on objects and primitive values, which is probably why db.Roles is queried twice.

What I would like to see in here is something that:

• First retrieve necessary data from the database, once.
• Filter it against in-memory data.
• Return the resulting role names.

Answer 2

This should do the same thing:

return db.PermissionRoles.Where(p => perms.Contains(p.Permission.ActionName)
                                  && p.CompanyId == companyId)
         .Select(pr => pr.Role.Name)
         .AsEnumerable()
         .Any(x => userManager.IsInRole(currentUserId, x.Name))

Note that I assume that there is a navigation property PermissionRole.Role. I expect it to be there if you generated the dbml following standard procedures.

I use .AsEnumerable() because userManager.IsInRole can't be translated into SQL.