CODEWITHSUNDEEP
c#asp.netasp.net-mvc-3authenticationforms-authentication
JonWillis
JonWillis

Reputation: 3147

Securing access to editing user details

I've hit a bit of a stumbling block on which direction to go on what I believe to be a very common task. Where do you verify that the logged in user is allowed to edit a piece of data. In this case, a User has an Id, EmailAddress, Name and a collection of Address objects. An Address contains several properties, one which is a UserId, the id of the user that it belongs to. A user should only be allowed to edit Address objects which belong to them. When editing an Address when/where do you check the user is allowed to do so.

I've just learned ASP.NET MVC3 this week and am putting it to practice. I have created a web application which logs the user in using the default MembershipProvider (I'll replace this with a custom one at a later date). The end result is that after logging in, the User.Identity.Name property in the controller returns the users email address.

If a user wants to view there own details, I call the Addresses action on the AccountController. As below.

[Authorize]
public ActionResult Addresses()
{
    IEnumerable<Address> addresses = _myService.GetUserAddresss(User.Identity.Name));
    return View(addresses);
}

Now If a user wants to edit address details they can call an action to get the address and display it, and an action to save the edits to the address.

[Authorize]
public ActionResult Address(int id)
{
    Address address= _myService.GetAddress(id));
    return View(address);
}

[Authorize]
[HttpPost]
public ActionResult Address(EditAddressModel model)
{
    Address address = _myService.SaveDetail(model.Address));
    return View(address );
}

The flaw in the above methods, is that if a user visits the url ../Account/Address/12. Then they can view and edit the Address with id 12 (if it exists), regardless of if they created it or not.

I'm following an N-tier approach. So I have the Controller talk to a service, which talks to business logic layer, which talks to a repository, which finally talks to a database using Entity Framework 4. Where should the authorisation checking take place?

I have considered the following solutions, but cannot decide on the suitable approach.

Idea 1

In the controller class, as the controller has access to the User.Identity.Name property. Using this property, it can check if the userid in the Address returned from the service, matches the currently logged in user. If not, then show an error page, otherwise let them view/edit as normal.

Advantage - Simple implementation, just add an extra if statement after the service returns the Address object. Controller has access to User.Identity.Name.

Disadvantage - Data is returned to the controller, all for the controller to decide the user cannot see it. Feels like business logic of "Only users can edit their own addresses" has creeped into the controller.

Idea 2

In the Business layer. The controller calls a service with userId and detailId (_myService.GetAddress(User.Identity.Name, detailId));), which in turn calls the business layer. The business layer has a method public Address GetAddress(int userId, int addressId) When the business layer is requested to get an Address from the database, it checks the returned address belongs to the user and return it. If not, it returns null. Thus the controller will get a null response from the service, and display a suitable error message.

Advantage - Business logic of users only editing their details, is in the business layer.

Disadvatage - As the business logic cannot access User.Identity.Name, each method in the service and business classes will need a userId parameter, which feels wrong and unneeded bloat.

Idea 3

As above, except that the Service and Business layer classes, have a property called UserId. This is used to check if the user can access a database resource. This can be set during creation or before calling the service. i.e.

[Authorize]
public ActionResult Address(int id)
{
    _myService.User = User.Identity.Name;
    Address address = _myService.GetAddress(id));
    return View(address);
}

Advantage - Business logic of users only editing their details, is in the business layer. No need to pass in the userId to each method call.

Disadvatage - I've never seen an example which uses this approach. And I do not feel it is right. Not that I'm using WCF as the service layer. But I'm sure if I was, it wouldn't work to have this extra property.

Idea 4

Access the User.Identity.Name in the business layer, without having to pass it though the service to the business layer from the controller. Not sure if it is possible.

Upvotes: 1

Views: 452

Answers (2)

Sean Glover
Sean Glover

Reputation: 1786

I would recommend doing the authorization in your business logic layer. You can access the current user's principal anywhere you like in your application (as long as it's the same app domain). All you need to do is access the static member HttpContext.Current and you can retrieve the User principal for current HttpContext.

Principal HttpContext.Current.User

Obviously, the assembly containing this code will need to reference System.Web. I don't think that should be a big concern in a web app.

To further decouple your business logic from calls to this static member I would recommend using the Adapter pattern to wrap calls and retrieve the user's principle. This way if you decide to ditch Membership in favor of some other authentication/authorization framework or you choose to mock it out you don't have a concrete coupling to HttpContext.Current.

Below is an example of an adapter to access the current context's user principle.

public interface IUserAdapter
{
    IPrincipal GetUserPrincipal();
    void SetAuthenticationCookie(IUser user);
    void SignOut();
}

// my business logic representation of a user
public interface IUser
{
    int Id { get; set; }
    string Name { get; set; }
}

public class WebUserAdapter : IUserAdapter
{
    public IPrincipal GetUserPrincipal()
    {
        return HttpContext.Current.User;
    }

    public void SetAuthenticationCookie(IUser user)
    {
        FormsAuthentication.SetAuthCookie(user.Id.ToString(), false);
    }

    public void SignOut()
    {
        FormsAuthentication.SignOut();
    }
}

Upvotes: 1

Adam Tuliper
Adam Tuliper

Reputation: 30152

Simply include the user's Id in every query. You've done this in idea two. As part of your security audit if doing this as part of a team ensure every data access method includes the parameter and also uses that user Id or name in the query to the DB in it's where clause. Integration tests can help here to create one record for a user and attempt to load it by id by calling the method passing in another users name parameter.

Upvotes: 1

Related Questions