Is it safe to use reflection and enumerations to logically control access to MVC applications?

Question

Trying to control access to the website, I created some necessary objects

The goal is to use a special permission attribute for some action method of the controller of my MVC application.

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

For this operation, I have two enumerations

 [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 well, but is it safe to use reflection here? Is this a good style?

Another question concerns this part of the code.

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

Here I am trying to get a permission object from a database using some named constant from the PermissionType enumeration.

In both cases, proper operation depends on the relationship between the enumerations and some table fields or records. On the other hand, I have a good mechanism for managing logic (as it seems to me). Is this a good way?

Answer 1

OTHER IMAGE
In your case, it would be advisable to create the readonly ExistingPermissions property for the RolePermissions class and merge the four logic elements into one CrudType inside this getter property. Then you can just do rp.ExistingPermissions.HasFlag(permissionToCheck) .

EDITED

Thanks to @DevDelivery for pointing out the problem - good catch. Unfortunately, a fixed solution is not as beautiful as I hoped, so in this case it makes sense to go with the @DevDelivery approach.

Since you have CrudType as “bit fields,” 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 disadvantage compared to your solution is that you will have to modify this method if you add more operations (to existing CanRead , etc.).

Answer 2

Using reflection affects performance, and late binding means that changing the enumeration name or property does not get into the compiler.

In addition, this code is very difficult to understand, so it is difficult to maintain.

There are only 4 verification options. The simple switch statement is simpler, faster, and cleaner.

Using reflection makes sense if you are trying to allow changes to the database or third-party components to introduce new permissions.