Where should I put code for access permisions in my ASP.NET MVC Site?

I have an ASP.NET MVC site which makes use of the repository pattern for accessing and modifying data. My repository interface is passed to each controller through their constructors. I'm also using Ninject to inject my concrete repository type through DependencyResolver.SetResolver().

Users on my site should only be able to access data that is assigned to them. What I'm trying to figure out is where should I 开发者_运维知识库check that the current user has permission to perform the current request?

For example the user may submit an delete item confirmation form to the URL /Item/Delete/123, which will delete the Item with ID 123. However, I don't want a user to be able to manipulate this URL and end up deleting another user's Item.

Should I add the user validation code to the controllers so the first thing each action method does is check if the current user even owns the data they are trying to modify? This seems like it would be adding too much intelligence to the controller which should be rather thin.

I think it would make more sense to add this logic to my repository? For example I may have a Repository.GetItem(int id, string user) rather then Repository.GetItem(int id), which would throw an exception unless "user" owns the requested item.

Alternatively I was thinking that each instance of my repository created could be assigned to a specific user when it is instantiated. These user specific repository would then throw exceptions if there is ever an attempt to access or modify data that is not owned by the current user. The controller would then simply need to catch theses exceptions and redirect the user to an error page if one is caught.

---

I recently came across the exact same issue. I ended up going with a custom ActionFilter that inherits from AuthorizeAttribute.

It basically has the same functionality as Authorize (checks if a user belongs to at least one of the roles listed) but also adds the capability to check if a user "owns" the particular data.

Here's the code for you to use as an example. If anything is not clear, please comment, and I'll try to explain.

[Edit - Based on Ryan's suggestion, I made params UserRole[] a constructor parameter instead of a public property and added AllowAnyRolesIfNoneSpecified.]

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, Inherited = true, AllowMultiple = false)]
public class AccountAuthorizeAttribute : AuthorizeAttribute
{
    private readonly UserRole[] _userRoles;

    public bool MustBeInRoleOrPageOwner { get; set; }
    public bool MustBeInRoleAndPageOwner { get; set; }
    public bool MustBeInRoleAndNotPageOwner { get; set; }
    public bool AllowAnyRolesIfNoneSpecified { get; set; }
    public string AccessDeniedViewName { get; set; }

    public AccountAuthorizeAttribute(params UserRole[] userRoles)
    {
        _userRoles = userRoles;
        MustBeInRoleOrPageOwner = false;
        MustBeInRoleAndPageOwner = false;
        MustBeInRoleAndNotPageOwner = false;
        AllowAnyRolesIfNoneSpecified = true;
        AccessDeniedViewName = "AccessDenied";
    }

    protected void CacheValidateHandler(HttpContext context, object data, ref HttpValidationStatus validationStatus)
    {
        validationStatus = OnCacheAuthorization(new HttpContextWrapper(context));
    }

    public override void OnAuthorization(AuthorizationContext filterContext)
    {
        if (!filterContext.HttpContext.User.Identity.IsAuthenticated)
        {
            ShowLogOnPage(filterContext);
            return;
        }
        using (var dbContext = new MainDbContext())
        {
            var accountService = new AccountService(dbContext);
            var emailAddress = filterContext.HttpContext.User.Identity.Name;
            if (IsUserInRole(accountService, emailAddress))
            {
                var isPageOwner = IsUserPageOwner(filterContext, dbContext, accountService, emailAddress);
                if (MustBeInRoleAndPageOwner && !isPageOwner || MustBeInRoleAndNotPageOwner && isPageOwner)
                    ShowAccessDeniedPage(filterContext);
            }
            else
            {
                if (!MustBeInRoleOrPageOwner)
                    ShowAccessDeniedPage(filterContext);
                else if (!IsUserPageOwner(filterContext, dbContext, accountService, emailAddress))
                    ShowAccessDeniedPage(filterContext);
            }
        }
    }

    private bool IsUserInRole(AccountService accountService, string emailAddress)
    {
        if (_userRoles.Length == 0 && AllowAnyRolesIfNoneSpecified) return true;
        return accountService.IsUserInRole(emailAddress, _userRoles);
    }

    protected virtual bool IsUserPageOwner(
        AuthorizationContext filterContext, MainDbContext dbContext, AccountService accountService, string emailAddress)
    {
        var id = GetRouteId(filterContext);
        return IsUserPageOwner(dbContext, accountService, emailAddress, id);
    }

    protected int GetRouteId(AuthorizationContext filterContext)
    {
        return Convert.ToInt32(filterContext.RouteData.Values["id"]);
    }

    private bool IsUserPageOwner(MainDbContext dbContext, AccountService accountService, string emailAddress, int id)
    {
        return accountService.IsUserPageOwner(emailAddress, id);
    }

    private void ShowLogOnPage(AuthorizationContext filterContext)
    {
        filterContext.Result = new HttpUnauthorizedResult();
    }

    private void ShowAccessDeniedPage(AuthorizationContext filterContext)
    {
        filterContext.Result = new ViewResult { ViewName = "ErrorPages/" + AccessDeniedViewName };
    }

    private void PreventPageFromBeingCached(AuthorizationContext filterContext)
    {
        var cachePolicy = filterContext.HttpContext.Response.Cache;
        cachePolicy.SetProxyMaxAge(new TimeSpan(0));
        cachePolicy.AddValidationCallback(CacheValidateHandler, null);
    }
}

A couple notes:

To avoid "magic strings", I used an array of UserRole enum values instead of a single string. Also, I built it to handle several scenarios I came across:

• User must be in a role or the owner of the page/data (e.g., an admin can edit anyone's data and anyone can edit their own data)
• User must be in a role and the owner of the page/data (e.g., a user can only edit his/her own page/data -- usually used without any role restrictions)
• User must be in a role and not the owner of the page/data (e.g., an admin can edit anyone's page/data except his own -- say, to prevent an admin from deleting his own account)
• No user is allowed to view this page, AllowAnyRolesIfNoneSpecified = false (e.g., you have a controller method for a page that doesn't exist but you need to include the method because your controller inherits from a base class that has this method)

Here's an example attribute declaration:

[AccountAuthorize(UserRole.Admin, MustBeInRoleAndNotPageOwner = true)]
public override ActionResult DeleteConfirmed(int id)
{
    ...
}

(This means an admin can delete any account but his own.)