ASP.NET MVC : How to prevent property injection

Use white-lists with UpdateModel() call. It is a list of property names that are legal to update. See the MSDN docs on UpdateModel

You mean TryUpdateModel?

UpdateModel and TryUpdateModel both support white lists.

So that worked until I had to do child objects.

Now the goal of this was to get this working (which IS working.. but because UpdateModel fails it doesnt work):

UpdateModel<IndexViewModel>(model, x => x.Shipment.Title);

However I don't think this works. For some reason they didn't make UpdateModel traverse down into children.

public class IndexViewModel
    {
        public Uship.Mobile2.Shipment Shipment { get; set; }
    }

    public class Shipment
    {
        public string Title { get; set; }
        public int TotalBids { get; set; }
    }

            IndexViewModel model = new IndexViewModel();
            UpdateModel<IndexViewModel >(model, new[] { "Shipment.Title" });

Select allOpen in new window

Best practices says do not use your entity/data/business model as a view model. Create separate view model.
Please refer below url for best practices:
http://codeclimber.net.nz/archive/2009/10/27/12-asp.net-mvc-best-practices.aspx

So now you have 2 classes/models for a view.
Let's say for Customer view;
we have 2 models;
First is entity model;
public class CustomerEntity
{
     public string FirstName{get;set;}
     public string LastName {get;set;}
     public string CompanyName {get;set;}
     
}
and last one is View model: CustomerModel
public class CustomerModel
{
     public string FirstName{get;set;}
     public string LastName {get;set;}
     public string CompanyName {get;set;}
     public void CleanUp(string AccessLevel)
    {
          //TODO: Write code as per access level
          //Access Level = 1 then only FirstName should be allowed to update and clear others fields
    }
    public void Update()
    {
          //TODO: Write code as per
         
    }
}

Now action body look like;
ActionResult UpdateCustomer(CustomerModel c)
{
    //TODO: Fetch accesslevel for current user from db/session
    c.CleanUp(accesslevel);
    c.Update();
}

Microsoft best practices say that update models (also known as input models) are completely optional and you should normally rely on domain objects for receiving input. Or taking input through your view model and either having BLL methods that accept view models. or translating the view model to business objects to then pass through the service layer.

Microsoft also highly suggests that whenever your UI matches your domain model, that you should probably simply use the domain model for the view model.

They say you can create update models, but they are purely optional and should not be forced to a 1:1 to the view because it becomes cumbersome on large applications.

They suggest using white lists, not a "CleanUp" method.

Infact, in a link to "best practices" it has domain objects, in the view model, which is what I am doing.
The example they give is when one domain object is being updated so they use the PREFIX bind attribute. Which is fine until you have to pass back all the domain objects for a complicated UI.

It is also common practice to get the object you are filling first, then call UpdateModel to fill in the rest, not the reverse (which would require custom code written).

Customer c = _service.GetCustomer(customerID)
UpdateModel(c);
_service.Update(c);

So what I tried out to see how I like it, is I made a method on a base class that will look through an object with a list of lambda expressions and invoke updatemodel multiple times on the same model, with different PREFIX values and white lists.

So for example with the object:
class Test {
public Test TestA;
public string B;
}

Invoking
UpdateModel2(testInstance, x => x.TestA.B, x => x.B);
or
UpdateModel2(testInstance, "TestA.B", "B");

Will run
UpdateModel(testInstance.TestA, "TestA", "B");
UpdateModel(testInstance, "B");

public abstract class BaseController : Controller
    {
        protected internal void UpdateModel2<T>(T model, params Expression<Func<T, object>>[] lambdas) where T : class
        {
            IEnumerable<string> whiteListProperties = from l in lambdas select GetFullExpression(l.Body);

            UpdateModel2<T>(model, whiteListProperties.ToArray());
        }

        protected internal void UpdateModel2<T>(T model, params string[] whitelist) where T : class
        {
            UpdateModel2<T>(model, string.Empty, whitelist);
        }

        protected internal void UpdateModel2<T>(T model, string prefix, params string[] whitelist) where T : class
        {
            Type modelType = model.GetType();
            foreach (PropertyInfo pi in modelType.GetProperties())
            {
                if (pi.PropertyType.IsClass && pi.PropertyType != typeof(string))
                {
                    dynamic child = pi.GetValue(model, null);
                    if (child != null)
                    {
                        if (!string.IsNullOrEmpty(prefix))
                            prefix += "." + pi.Name;
                        else
                            prefix = pi.Name;

                        string[] reducedWhitelist = (from s in whitelist
                                                    select s.Replace(prefix + ".", string.Empty)).ToArray();

                        UpdateModel(child, prefix, reducedWhitelist);
                        UpdateModel2(child, prefix, whitelist);
                    }
                }
            }
            UpdateModel(model, whitelist);
        }
}

Select allOpen in new window

Good solution!

I have one simple query.

Could you give me the sample for parsing model based on access right as per asked question?
Can I reuse above implementation in different places? (DRY principle- Do Not Repeat Your self)

Thanks in advance.

Reuse means;

Let's say in an action,
for x access right - UpdateModel(testInstance.TestA, "TestA", "B") will be executed.
for y access right - UpdateModel(testInstance, "B") will be executed.

Can I reuse above implementation in different controllers? If yes then how?
 

This is pulling the discussion in a security topic.

I was just using access rights as an example because it makes the most sense.

I am invoking access rights within the action, so if access rights don't match the user goes to the login page.

actionresult()
{
  if (!securitygroup.contains(securitylevel.basic)) return RedirectToAction()
 // code to update from view
}

The code you put as an example is not supposed to be shared because it is strictly bound to the view. It should never show up more than once because then you would re-use the same view.

I should also note, I have a view for each security level.
Because different items in the view are editable.

If you want to enforce security groups I'm pretty sure you can use a custom action invoker or something like that, with attributes on the action.

[SecurityRequired(Enum.SecurityLevel.Basic)]
public ActionResult Edit(int id)
{
}