Including current user into every call to service layer from controller

Question

I perform model validation in my controllers, but a second business validation needs to take place at the service/business level. This is usually related to user permission: does the current user have access to the customer/order info he is trying to get or post?

My first (and working) approach is to pass either the entire User instance or its Id (by calling User.Identity.GetUserId()), which would be enough most -if not all- of the time. So I'll have something like this:

public IHttpActionResult Get(int id)
{
    try
    {
        var customer = customerService.GetById(id, userId);
        return Ok(customer);
    }
    catch (BusinessLogicException e)
    {
        return CreateErrorResponse(e);
    }
}

But I don't really like the fact that with this approach I'm going to have to include an extra parameter in pretty much every call to my service layer. If I am calling a GetById() method, I want to get something by providing an ID, not an ID and a user ID.

A simple workaround would be something along these lines, which also works:

public IHttpActionResult Get(int id)
{
    customerService.SetCurrentUser(User.Identity.GetUserId());

    try
    {
        var customer = customerService.GetById(id);
        return Ok(customer);
    }
    catch (BusinessLogicException e)
    {
        return CreateErrorResponse(e);
    }
}

But instead of having to make a separate call to set the current user, I'd like this to be done automatically with every call to the service. How can I do it?

Here's what my service looks like:

public class CustomerService : EntityService<Customer>, ICustomerService
{
    public string UserId;
    IContext context;
    public CustomerService(IContext context) : base(context)
    {
        this.context = context;
        this.dbSet = context.Set<Customer>();
    }

    public void SetCurrentUser(string userId)
    {
        UserId = userId;
    }

    public DTO.Customer GetById(int id)
    {
        if (!IsAccessibleByUser(id))
        {
            throw new BusinessLogicException(ErrorCode.UserError, "UserId: " + UserId);
        }

        return dbSet.FirstOrDefault(x => x.Id == id).ToDto<Customer, DTO.Customer>();
    }

    public bool IsAccessibleByUser(int id)
    {
        return context.UsersAPI.Any(a => a.AspNetUsersID == UserId);
    }
}

Answer 1

I would rather perform this authorization logic in a custom authorization filter. There's no need to even reach your controller action code if the user is not authenticated or authorized.

For example you could have something like this:

public class MyAuthorizeAttribute : AuthorizeAttribute
{
    protected override bool AuthorizeCore(HttpContextBase httpContext)
    {
        var authorized = base.AuthorizeCore(httpContext);
        if (!authorized)
        {
            return false;
        }

        var rd = httpContext.Request.RequestContext.RouteData;
        // Get the id of the requested resource from the route data
        string resourceId = rd.Values["id"] as string;
        if (string.IsNullOrEmpty(resourceId))
        {
            // No id of resource was specified => we do not allow access
            return false;
        }

        string userId = httpContext.User.Identity.GetUserId();
        return IsAccessibleByUser(resourceId, userId);
    }

    private bool IsAccessibleByUser(string resourceId, string userId)
    {
        // You know what to do here => fetch the requested resource 
        // from your data store and verify that the current user is
        // authorized to access this resource
    }
}

and then you could decorate your controllers or actions that require this kind of authorization with the custom attribute:

[MyAuthorize]
public IHttpActionResult Get(int id)
{
    try
    {
        // At this stage you know that the user is authorized to
        // access the requested resource
        var customer = customerService.GetById(id);
        return Ok(customer);
    }
    catch (BusinessLogicException e)
    {
        return CreateErrorResponse(e);
    }
}

Of course this custom attribute could be further improved by using a custom filter provider which would allow for injecting your data contexts into it so that you can perform the proper calls. Then you could only have a marker attribute which will be used by the filter provider in order to decide whether it should perform the authorization logic or not.