Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it safe to use reflection and enums for logic control of MVC application access?

Trying to manage access to a web site I created some necessary entities enter image description here

The goal is use a custom permission attribute for some controller's action method of my MVC application.

[Permissions(PermissionType.SomePermissionName, CrudType.CanDelete)]
public ActionResult SomeAction()
{
}

For this operation I have two enums

[Flags]
public enum CrudType
{
    CanCreate = 0x1,
    CanRead = 0x2,
    CanUpdate = 0x4,
    CanDelete = 0x8,
}

[Flags]
public enum PermissionType
{
   SomePermissionName = 0x1,
   //... 
}

Now I want the method below to check permissions

public static bool CanAccess(RolePermissions rp, CrudType crudType)
{
    var pInfo = rp.GetType().GetProperties();
    var res = pInfo.FirstOrDefault(x => x.Name == crudType.ToString());
    if(res != null)
    {
        return Convert.ToBoolean(res.GetValue(rp, null));
    }
    return false;
}

It works good but is it safe to use reflection here? Is it a good style?
One more question is about such piece of code

var permission = PermissionService.GetByName(permissionType.ToString());

Here I'm trying to get a permission object from a database using some named constant from the PermissionType enum.
In both cases the correct work depends on relationships between enums and some table fields or records. On other side I have a good mechanism of controlling logic (as it seems to me). Is that a good way?

like image 552
Alex Kovanev Avatar asked Dec 17 '11 11:12

Alex Kovanev


1 Answers

ANOTHER EDIT
In your case it would make sense to create a readonly property ExistingPermissions for the RolePermissions class, and do the merging of the four booleans into one CrudType within that property getter. Then you can just do rp.ExistingPermissions.HasFlag(permissionToCheck).

EDITED

Thanks to @DevDelivery for pointing out the issue - good catch. Unfortunately the fixed solution isn't as pretty as I was hoping for, so in this case it might make sense to go with @DevDelivery's approach.

Since you have your CrudType as "bitfields", you can use a cleaner approach (less code and better readability):

public static bool CanAccess(RolePermissions rp, CrudType permissionToCheck)
{
    CrudType existingPermissions = 
                                SetPermissionFlag(CrudType.CanCreate, rp.CanCreate) |
                                SetPermissionFlag(CrudType.CanRead, rp.CanRead) | 
                                SetPermissionFlag(CrudType.CanUpdate, rp.CanUpdate) |
                                SetPermissionFlag(CrudType.CanDelete, rp.CanDelete);

    return existingPermissions.HasFlag(permissionToCheck);
}

public static CrudType SetPermissionFlag(CrudType crudType, bool permission)
{
    return (CrudType)((int)crudType * Convert.ToInt32(permission));
}

The drawback compared to your solution is that you will have to modify this method in case you add more operations (to the existing CanRead, etc.).

like image 160
Yakimych Avatar answered Oct 03 '22 11:10

Yakimych