Looking for elegant and secure way for security-trimming "edit" views

68 Views Asked by At

Looking for a clean, secure way to only allow certain users (by role) to edit certain fields in a view. The key word here is edit, because it's one thing to just show/hide parts of a view (buttons, links, etc.), but another thing how to handle "protected" fields as they're being posted back to the controller.

For example, I could do this:

View Model

public class CustomerEditViewModel
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    // Other properties...

    public string Email { get; set; }
    public string Phone { get; set; }

    public bool CanEditContactInfo { get; set; } 
}

View

@Html.EditorFor(m => m.FirstName)
@Html.EditorFor(m => m.LastName)

@if (Model.CanEditContactInfo)
{
    @Html.EditorFor(m => m.Email)
    @Html.EditorFor(m => m.Phone)
}

Controller

[HttpGet]
public ActionResult Edit(int id)
{
    var customer = _customerService.GetCustomerById(id);
    var model = Mapper.Map<CustomerEditViewModel>(customer);
    model.CanEditContactInfo = User.IsInRole("Admin");
    return View(model);
}

[HttpPost]
public ActionResult Edit(CustomerEditViewModel model)
{
    var customer = _customerRepo.GetCustomerById(model.Id);
    Mapper.Map(model, customer);
    _customerService.UpdateCustomer(customer);
    // return to Index view
}

But the problem is that when a non-admin user is editing the customer, the fields Email and Phone are never rendered on the view, so they will be NULL when the form is posted back to the controller, and would further overwrite the actual values in the database with NULLs.

I could solve this like so:

@Html.EditorFor(m => m.FirstName)
@Html.EditorFor(m => m.LastName)

@if (Model.CanEditContactInfo)
{
    @Html.EditorFor(m => m.Email)
    @Html.EditorFor(m => m.Phone)
}
else
{
    @Html.HiddenFor(m => m.Email)
    @Html.HiddenFor(m => m.Phone)
}

That would preseve the original Email/Phone values even when a non-admin user is editing the record, but then the problem is that Email/Phone fields are available in the rendered HTML as hidden fields and could easily be manipulated by a browser tool before posting back.

I do have some ideas, but they are getting messy. So I would like to know what successful approaches might already be out there for something like this.

1

There are 1 best solutions below

2
On

The first rule of thumb in secure coding is that the client input cannot be trusted, which means you MUST apply your checks and validations in the server side. In your case, the HttpPost controller action should check the user role again. If user is not authorized to update all properties, then you could:

1- Load the original data again and overwrite the properties that can be updated with user input and then save this updated copy:

[HttpPost]
public ActionResult Edit(CustomerEditViewModel model)
{
    var customer = _customerRepo.GetCustomerById(model.Id);
    // Check the current user role.    
    If (!User.IsInRole("Admin"))
    {
       customer.FirstName = model.FirstName;
       customer.LastName = model.LastName;
    }
    else
    {
       Mapper.Map(model, customer);
    }
    _customerService.UpdateCustomer(customer);
    // return to Index view
}

2- Create another mapping method that ignores the properties that can not be updated from user input and call the right mapper based on the user permission

public static Customer ToEntity(this CustomerEditViewModel model, Customer destination)
{
    return Mapper.CreateMap<CustomerEditViewModel, Customer>()
                 .ForMember(dest => dest.Email, opt => opt.Ignore()
                 .ForMember(dest => dest.Phone, opt => opt.Ignore()
                );
}