10

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);
    }
}
erictrigo
  • 989
  • 2
  • 13
  • 41

5 Answers5

3

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.

Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • 1
    I am already using a `CustomAuthorizeAttribute` to return a specific error message in case the token is not valid or the user doesn't have the required role. This approach seems very interesting since it would get rid of a *ton* of validation logic in my business logic layer, but it does feel a little weird to have lots of validation logic somewhere else in my application. Aside from that, could you expand on your last paragraph? Injecting my data context (my service, in this case) into the authorization filter seems like another problem I'd need to solve in order to use this approach. – erictrigo Mar 04 '16 at 10:37
  • As I already stated in my answer if you need dependency injection, all you need to use is a custom filter provider. So after some searching you may find: http://haacked.com/archive/2011/04/25/conditional-filters.aspx/ A marker attribute on your controller actions could allow you to detect in your custom filter provider if you need to apply the authorization logic. And you should be asking yourself: Am I using this *business layer* anywhere outside my MVC application that I should care about? If so probably this business layer is already wrapped behind a RESTful facade. – Darin Dimitrov Mar 04 '16 at 13:50
  • So don't just wait me write all the code for you, look around, experiment, come back with specific questions if you have such. – Darin Dimitrov Mar 04 '16 at 13:56
0

Does your service need to take the user id into account when performing its logic? If so then it's reasonable for it to be part of the input. Or is it just more authorization, where the service should reject the request without processing depending on who the user is (but once authorized it doesn't matter who the user is)? You could insert additional data into the header of the WCF request and then inspect it on the incoming request. But a) it's a pain, and b) it's not at all apparent from the service interface that the client should be providing that data.

For that reason I'd put a user id in the input. I wouldn't make it a property of the service. It's really a property of the input, not the class handling that input.

An interceptor is still a good idea if you don't want your service to have the extra responsibility of authorizing requests. That way you put an attribute on your service or service method and keep the authorization in the interceptor. If the user can't call the service or method then it rejects the call before it ever reaches the service class.

Scott Hannen
  • 27,588
  • 3
  • 45
  • 62
0

Late answer but I understand your concern, best practice you want to achieve in your code and make it as clean as possible, I had exactly the same issue as yours long time ago and same as you I felt that this is not right, there should be a better way to do it, and in my case I used on all my layers:
System.Threading.Thread.CurrentPrincipal.Identity
and this answer on SO describe it: https://stackoverflow.com/a/27636802/20126

and while this is the way I always use, it worth looking at what others did as well:
Accessing HttpContext and User Identity from data layer
How to get User ID inside a separate assembly
Retrieving the current logged in user's Id in a class library outside of an ASP.NET Webforms App using forms authentication
Get the ID of the current user ASP.NET Membership

Amr Elgarhy
  • 66,568
  • 69
  • 184
  • 301
0

You can directly get the User Principal from

HttpContext.Current.User

in your services. You need to set your User Principal in the authorization layer after you check for the token validation or somewhere prior to calling the service.

But, I feel this makes your service more constrained to use some specific authentication procedure and you will not be able to use this service function in some other application unless the User Principal is not set.

-1

In startup.cs add below to ConfigureServices:

services.AddHttpContextAccessor();

and then in constructor of your class inject IHttpContextAccessor and after that you can use it like this:

context.HttpContext.User.Identity.GetUserId() 
PurTahan
  • 789
  • 8
  • 24