views:

3252

answers:

4

I'm using the Data Annotation validation extensively in ASP.NET MVC 2. This new feature has been a huge time saver, as I'm now able to define both client-side validation and server-side validation in one place. However, while I was doing some detailed testing, I realized that it's quite easy for someone to bypass the server-side validation if I relied on Data Annotation validation alone. For example, if I defined a required field by annotating the property with the [Required] attribute and placed a textbox for that required field in a form, a user could simply remove the textbox from the DOM (which can easily be done through Firebug) and now the Data Annotation validation will not be triggered on that property during ModelBinding inside of a Controller. To ensure that the "required" validation is triggered, I can repeat the validation after ModelBinding happens, but then I'd be repeating my validation logic.

What is everyone's recommendation on validation? Is Data Annotation validation enough? Or does the validation need to be repeated to ensure that validations get triggered in all situations?

Follow-up comment: Based on the answers below, it seems that I can't rely on the Model Binder and Data Annotation validation alone. Since we're concluding that additional server-side validation is required, is there an easy way for my Service layer to trigger validation based on what's been defined in the Data Annotations? It seems that this will get us the best of both words...we won't need to repeat the validation code, but we'll still ensure that the validation gets executed even if Model Binder doesn't trigger it.

I'm going to post this follow-up comment as a separate question, as it poses a different question than the original one.

+2  A: 

The DataAnnotation is certainly not enough. I use it extensively also to pre-validate my calls to the domain model to get better error reporting and fail as early as possible.

You can however tweak the DataAnnotation Model yourself to ensure properties with [Required] MUST be posted. (will follow up with code later today).

UPDATE Get the source for DataAnnotations Model Binder and find this line in DataAnnotationsModelBinder.cs

// Only bind properties that are part of the request
if (bindingContext.ValueProvider.DoesAnyKeyHavePrefix(fullPropertyKey)) {

Change it to

// Only bind properties that are part of the request
bool contextHasKey = bindingContext.ValueProvider.DoesAnyKeyHavePrefix(fullPropertyKey);
bool isRequired = GetValidationAttributes(propertyDescriptor).OfType<RequiredAttribute>().Count() > 0;
if (contextHasKey || (!contextHasKey && isRequired)) {
Martijn Laarman
Thanks Martijn. I look forward to seeing your code to.
JohnnyO
Sure, I posted this prior to leaving to work (at work still now) so no time to code :(. I modified the binder before slightly since it didn't checked nested objects and reset invalid properties to null which I disagreed with see http://stackoverflow.com/questions/820468/how-does-dataannotationsmodelbinder-work-with-custom-viewmodels/864541#864541 I have added the Required checks since then too but would like to test them out when I get home before posting them.
Martijn Laarman
Updated with code, but I am not in the position to test it out properly will do so again tomorrow but posted it up so perhaps you can evaluate it quicker. It passes the unit tests of the project but to be fair there is no test testing this case :D.
Martijn Laarman
Thanks Martijn. I'll have to leave this for another day, as this will require making changes to source. For now, I'm going to take your (and other people's) suggestion and ensure that validation is also duplicated in the Domain Model.
JohnnyO
Sure no problem. It would be great if you could offload data integrity checks to DataAnnotations though. That way your domain model only has to validate business rules.
Martijn Laarman
Domain model still has to call DataAnnotations too though, you never know when you want to export the domain as a seperate DLL :).
Martijn Laarman
+5  A: 

I think to be vigilant concerning security you should choose to you make server validation the priority and ensure that this is always your fallback. Your server validation should work without the client validation. Client validation is more for UX and tho that is paramount to your design, it is secondary to security. With this in mind you will find yourself repeating your validation. A goal is often trying to design your app so that the server and client validation can be integrated as much as possible to reduce the work required to validate on the server and the client. But be assured you must do both.

If bypassing the client validation (by means of DOM manipulation) is avoiding the server validation (which it seems you are indicating) then your server validation for this instance may not be employed appropriately. You should be invoking your server validation again in your controller action or in a service layer. The scenario you describe should not be defeating your server validation.

With the scenario you describe, the DataAnnotation attributes method should be sufficient. It seems that you simply need to make a few code changes to ensure that your server validation is invoked also when submitting the form.

cottsak
+6  A: 

I paired xVal with DataAnnotations and have written my own Action filter that checks any Entity type parameters for validation purposes. So if some field is missing in the postback, this validator will fill ModelState dictionary hence having model invalid.

Prerequisites:

  • my entity/model objects all implement IObjectValidator interface which declares Validate() method.
  • my attribute class is called ValidateBusinessObjectAttribute
  • xVal validation library

Action filter code:

public void OnActionExecuting(ActionExecutingContext filterContext)
{
 IEnumerable<KeyValuePair<string, object>> parameters = filterContext.ActionParameters.Where<KeyValuePair<string, object>>(p => p.Value.GetType().Equals(this.ObjectType ?? p.Value.GetType()) && p.Value is IObjectValidator);
 foreach (KeyValuePair<string, object> param in parameters)
 {
  object value;
  if ((value = param.Value) != null)
  {
   IEnumerable<ErrorInfo> errors = ((IObjectValidator)value).Validate();
   if (errors.Any())
   {
    new RulesException(errors).AddModelStateErrors(filterContext.Controller.ViewData.ModelState, param.Key);
   }
  }
 }
}

My controller action is defined like this then:

[ValidateBusinessObject]
public ActionResult Register(User user, Company company, RegistrationData registrationData)
{
 if (!this.ModelState.IsValid)
 {
  return View();
 }
 ...
}
Robert Koritnik
+2  A: 

I wrote my own ValidationService for MVC 1.0 by copying patterns from both xVal's DataAnnotationsRuleProvider and Microsoft's DataAnnotationsModelBinder (and Martijn's comments). The service interface is below:

public interface IValidationService
{
    void Validate(object instance);

    IEnumerable<ErrorInfo> GetErrors(object instance);
}

public abstract class BaseValidationService : IValidationService
{
    public void Validate(object instance)
    {
        var errors = GetErrors(instance);

        if (errors.Any())
            throw new RulesException(errors);
    }

    public abstract IEnumerable<ErrorInfo> GetErrors(object instance);
}

The service is a validation runner that walks the property tree of the object instance it receives and actually executes the validation attributes that it finds on each property, building a list of ErrorInfo objects when attributes are not valid. (I'd post the whole source but it was written for a client and I don't know yet if I'm authorized to do so.)

You can then have your controllers, business logic services explicitly invoke validation when you are ready, rather than relying exclusively on the model binder for validation.

There are a couple of other pitfalls that you should be aware of:

  • The default DataTypeAttribute in data annotations doesn't actually do any data type validation, so you'll need to write a new attribute that actually uses xVal regular expressions (or something else) to perform server-side data type validation.
  • xVal doesn't walk properties to create client-side validation, so you may want to make some changes there to get more robust client-side validation.

If I am allowed and have time, I will try to make more source available...

John Bledsoe