I have API where I need to validate my user model. I choose an approach where I create different classes for Create/Edit actions to avoid mass-assignment and divide validation and actual model apart.

I don't know why but ModelState.IsValid returns true even when it should not. Am I doing something wrong?

Controller

public HttpResponseMessage Post(UserCreate user)
{
    if (ModelState.IsValid) // It's valid even when user = null
    {
        var newUser = new User
        {
            Username = user.Username,
            Password = user.Password,
            Name = user.Name
        };
        _db.Users.Add(newUser);
        _db.SaveChanges();
        return Request.CreateResponse(HttpStatusCode.Created, new { newUser.Id, newUser.Username, newUser.Name });
    }
    return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
}

Model

public class UserCreate
{
    [Required]
    public string Username { get; set; }
    [Required]
    public string Password { get; set; }
    [Required]
    public string Name { get; set; }
}

Debug proof

Solution 1:

The ModelState.IsValid internally checks the Values.All(modelState => modelState.Errors.Count == 0) expression.

Because there was no input the Values collection will be empty so ModelState.IsValid will be true.

So you need to explicitly handle this case with:

if (user != null && ModelState.IsValid)
{

}

Whether this is a good or bad design decision that if you validate nothing it will true is a different question...

Solution 2:

Here is an action filter to check for null models or invalid models. (so you dont have to write the check on every action)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Web.Http.Controllers;
using System.Web.Http.Filters;

namespace Studio.Lms.TrackingServices.Filters
{
    public class ValidateViewModelAttribute : ActionFilterAttribute
    {
        public override void OnActionExecuting(HttpActionContext actionContext)
        {
            if (actionContext.ActionArguments.Any(kv => kv.Value == null)) {
                actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, "Arguments cannot be null");
            }

            if (actionContext.ModelState.IsValid == false) {
                actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, actionContext.ModelState);
            }
        }
    }
}

You can register it globally:

config.Filters.Add(new ValidateViewModelAttribute());

Or use it on demand on classes/actions

 [ValidateViewModel]
 public class UsersController : ApiController
 { ...

Solution 3:

I wrote a custom filter which not only ensures that all non optional object properties are passed, but also checks if model state is valid:

[AttributeUsage (AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false)]
public sealed class ValidateModelAttribute : ActionFilterAttribute
{
    private static readonly ConcurrentDictionary<HttpActionDescriptor, IList<string>> NotNullParameterNames =
        new ConcurrentDictionary<HttpActionDescriptor, IList<string>> ();


    /// <summary>
    /// Occurs before the action method is invoked.
    /// </summary>
    /// <param name="actionContext">The action context.</param>
    public override void OnActionExecuting (HttpActionContext actionContext)
    {
        var not_null_parameter_names = GetNotNullParameterNames (actionContext);
        foreach (var not_null_parameter_name in not_null_parameter_names)
        {
            object value;
            if (!actionContext.ActionArguments.TryGetValue (not_null_parameter_name, out value) || value == null)
                actionContext.ModelState.AddModelError (not_null_parameter_name, "Parameter \"" + not_null_parameter_name + "\" was not specified.");
        }


        if (actionContext.ModelState.IsValid == false)
            actionContext.Response = actionContext.Request.CreateErrorResponse (HttpStatusCode.BadRequest, actionContext.ModelState);
    }


    private static IList<string> GetNotNullParameterNames (HttpActionContext actionContext)
    {
        var result = NotNullParameterNames.GetOrAdd (actionContext.ActionDescriptor,
                                                     descriptor => descriptor.GetParameters ()
                                                                             .Where (p => !p.IsOptional && p.DefaultValue == null &&
                                                                                          !p.ParameterType.IsValueType &&
                                                                                          p.ParameterType != typeof (string))
                                                                             .Select (p => p.ParameterName)
                                                                             .ToList ());

        return result;
    }
}

And I put it in global filter for all Web API actions:

config.Filters.Add (new ValidateModelAttribute ());

Solution 4:

Updated slightly for asp.net core...

[AttributeUsage(AttributeTargets.Method)]
public sealed class CheckRequiredModelAttribute : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext context)
    {
        var requiredParameters = context.ActionDescriptor.Parameters.Where(
            p => ((ControllerParameterDescriptor)p).ParameterInfo.GetCustomAttribute<RequiredModelAttribute>() != null).Select(p => p.Name);

        foreach (var argument in context.ActionArguments.Where(a => requiredParameters.Contains(a.Key, StringComparer.Ordinal)))
        {
            if (argument.Value == null)
            {
                context.ModelState.AddModelError(argument.Key, $"The argument '{argument.Key}' cannot be null.");
            }
        }

        if (!context.ModelState.IsValid)
        {
            var errors = context.ModelState.Values.SelectMany(v => v.Errors).Select(e => e.ErrorMessage);
            context.Result = new BadRequestObjectResult(errors);
            return;
        }

        base.OnActionExecuting(context);
    }
}

[AttributeUsage(AttributeTargets.Parameter)]
public sealed class RequiredModelAttribute : Attribute
{
}

services.AddMvc(options =>
{
    options.Filters.Add(typeof(CheckRequiredModelAttribute));
});

public async Task<IActionResult> CreateAsync([FromBody][RequiredModel]RequestModel request, CancellationToken cancellationToken)
{
    //...
}