Custom security scenario in ASP.NET MVC

Question

I don't have a lot of experience with this and I am really hoping to get a good suggestion from you guys. I need to implement the following security scenario and I would like to know the best way to do it.

Imagine we have Employees, Supervisors and Department managers. Both Employees and Supervisors have ManagerId assigned based off and pointing to the department manager they belong to.

When a supervisor user logs in I want him to only see records for employees that belong to the same ManagerId as his. If another supervisor with another ManagerId user logs in and manually punches other employee's information in url (ex: wwww.domain.com/employee/details/{id} ), because his ManagerId != employee's ManagerId I would like the access to be restricted.

Does it make sense ?

I started typing out checks on all ActionMethods such as:

public ActionResult Details(int id)
{
    var employee = employeeRepository.Get(id)
    var user = (CustomIdentity)ControllerContext.HttpContext.User.Identity;

    if(employee.managerId == user.managerId)
    {
        Do whatever...
    }   
    else    
    {
        Not allowed
    }
}

But typing that out in all ActionMethods seems redundant and just..ehh... I know there must be a better way.

agarcian · Accepted Answer · 2012-04-26 18:36:21Z

2

Here is a stab at a solution. It needs a bit of cleanup but should give you everything you need.

Create a custom ActionFilter, and then decorate your methods with it.

[ManagerIdAuthentication]
public ActionResult Details(int id)
{
     // Gets executed if the filter allows it to go through.
}

The next class can be created in a separate library so you can include it in all your actions that require this validation.

public class ManagerIdAuthentication : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        // the next line needs improvement, only works on an httpGet since retrieves
        // the id from the url.  Improve this line to obtain the id regardless of 
        // the  method (GET, POST, etc.)
        var id = filterContext.HttpContext.Request.QueryString["id"];

        var employee = employeeRepository.Get(id);
        var user = filterContext.HttpContext.User.Identity;
        if (employee.managerId  == user.managerId)
        {
            var res = filterContext.HttpContext.Response;
            res.StatusCode = 402;
            res.End();
            filterContext.Result = new EmptyResult();  //may use content result if want to provide additional info in the error message.
        }
        else
        {
            // OK, let it through.
        }
    }
}

answered Apr 26, 2012 at 18:36

agarcian

3,9623 gold badges35 silver badges55 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

InspiredBy Over a year ago

Thank you for your response. Very interesting. Do I really need "Else" in here ? Can't I say If(employee.managerId != user.managerId) throw new MyUnauthorizedException(); ...........Otherwise a method that implements this will trigger. No ? What am I missing ? What's the point of the stuff you have in your if (employee.managerId == user.managerId) statement ?

agarcian Over a year ago

No need for the else... Just to make the point that it will continue executing.

antijon · Accepted Answer · 2012-04-26 18:31:46Z

2

I had a similar issue in the past, what I would consider per-object permissions. What I did was add a member to the object similar to:

public bool CanUserAccess(User user) {
    return managerId == user.managerId;
}

Then, at the top of each action providing access to a controlled resource:

public ActionResult Details(int id)
{
    var employee = employeeRepository.Get(id)
    var user = (CustomIdentity)ControllerContext.HttpContext.User.Identity;
    if(!employee.CanUserAccess(user))
        return new HttpUnauthorizedResult();

    // Normal logic here
}

It's certainly not perfect, but it does centralize the permission handling and allows you to easily increase the complexity in the future (allow access up the chain, special rules for HR, etc.). You could also write another overload/extension to access the User.Identity property for a bit more automation (or at least handle the type conversions).

Since I was dealing with ACL's, I would have additional methods/parameters to specify the basic nature of the action (e.g. Read, Write, Delete, Create, etc.).

answered Apr 26, 2012 at 18:31

antijon

9926 silver badges7 bronze badges

2 Comments

InspiredBy Over a year ago

So pretty much the same thing except taking my IF logic to a separate statement ?

antijon Over a year ago

Basically. Just centralizing the permission checks so it's a bit more DRY. The ActionFilterAttribute approach is probably the cleanest approach, assuming it will work in your application. You need to be able to get access to the repository (which isn't that hard), but you also need to be able to decode which object is being used for the access checks (which may be more challenging).

Collectives™ on Stack Overflow

Custom security scenario in ASP.NET MVC

2 Answers 2

2 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related