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); } }
Improve this question

4 Answers 4

Reset to default
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.

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

3 Comments

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.
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: 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.
So don't just wait me write all the code for you, look around, experiment, come back with specific questions if you have such.
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.

Improve this answer

Comments

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

Improve this answer

Comments

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.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.