MVC ModelState IsValid reports true when should be false

Question

I have the following 3 classes (all code now included)...

using System.ComponentModel.DataAnnotations;

namespace MyWebApp.Models
{
    public class ApplicationUser
    {
        [Display(Prompt = "Your Name", Name = "Contact Name"), Required(), StringLength(255, MinimumLength = 3, ErrorMessage = "Please enter a valid contact")]
        public string ContactName { get; set; }
        [Display(Name = "Username", Prompt = "Login As"), Required(), StringLength(255, MinimumLength = 3, ErrorMessage = "Your username must be at least 3 characters")]
        public string UserName { get; set; }
    }

    public class Account
    {
        [Display(Name = "Account Type", Prompt = "Account Type"), Required()]
        public int AccountType { get; set; }
        [Display(Name = "Organisation Name", Prompt = "Name of Organisation"), StringLength(255)]
        public string OrganisationName { get; set; }
    }

    public class RegisterViewModel
    {
        public ApplicationUser ApplicationUser { get; set; }
        public Account Account { get; set; }

        public RegisterViewModel()
        {
            ApplicationUser = new ApplicationUser();
            Account = new Account();
        }

        [Display(Name = "Password", Prompt = "Password"), Required(), DataType(DataType.Password), StringLength(255, MinimumLength = 5, ErrorMessage = "The password must be at least 7 characters")]
        public string Password { get; set; }

        [Display(Name = "Confirm Password", Prompt = "Confirm Password"), DataType(DataType.Password), Compare("Password", ErrorMessage = "Your confirmation doesn't match...")]
        public string PasswordConfirmation { get; set; }
    }
}

My Controller looks like this...

using System.Web.Mvc;
using MyWebApp.Models;

namespace MyWebApp.Controllers
{
    [Authorize]
    public class AccountController : Controller
    {
        // GET: /Account/
        [AllowAnonymous]
        public ActionResult Register()
        {
            RegisterViewModel mdl = new RegisterViewModel();
            return View(mdl);
        }

        [HttpPost]
        [AllowAnonymous]
        public ActionResult Register([Bind(Include = "Account.AccountType, ApplicationUser.ContactName, ApplicationUser.UserName, Password, Account.OrganisationName, Account.OrganisationRegistrationNumber, Account.AddressLine1, Account.AddressLine2, Account.AddressLine3, Account.City, Account.County, Account.Postcode, ApplicationUser.Email, ApplicationUser.PhoneNumber, PasswordConfirmation")]RegisterViewModel model)
        {
            if (ModelState.IsValid)
            {
                var user = model.ApplicationUser;
                var associatedAccount = model.Account;

                var u1 = Request.Form["ApplicationUser.UserName"];
                if (u1 == user.UserName)
                {
                    // we got it
                    ViewBag.Message = "We got it";
                }
                else
                {
                    // no we didn't
                    ViewBag.Message = "We failed!";
                }
                return View(model);
            }

            // If we got this far, something failed, redisplay form
            return View(model);
        }
    }
}

my Register.cshtml view looks like this...

@using MyWebApp.Models
@model MyWebApp.Models.RegisterViewModel

@{
    ViewBag.Title = "Register User";
}

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken()

    <div class="form-horizontal">
        <h4>New Account</h4>
        <hr />
        <p>@ViewBag.Message</p>
        @Html.ValidationSummary(true)
        <div class="col-md-6">
            <div class="form-group">
                @Html.LabelFor(model => model.ApplicationUser.ContactName, new { @class = "control-label col-md-5" })
                <div class="col-md-7">
                    @Html.EditorFor(model => model.ApplicationUser.ContactName, new { @class = "form-control" })
                    @Html.ValidationMessageFor(model => model.ApplicationUser.ContactName)
                </div>
            </div>
            <div class="form-group">
                @Html.LabelFor(model => model.ApplicationUser.UserName, new { @class = "control-label col-md-5" })
                <div class="col-md-7">
                    @Html.EditorFor(model => model.ApplicationUser.UserName, new { @class = "form-control" })
                    @Html.ValidationMessageFor(model => model.ApplicationUser.UserName)
                </div>
            </div>
            <div class="form-group">
                @Html.LabelFor(model => model.Password, new { @class = "control-label col-md-5" })
                <div class="col-md-7">
                    @Html.EditorFor(model => model.Password, new { @class = "form-control" })
                    @Html.ValidationMessageFor(model => model.Password)
                </div>
            </div>
            <div class="form-group">
                @Html.LabelFor(model => model.PasswordConfirmation, new { @class = "control-label col-md-5" })
                <div class="col-md-7">
                    @Html.EditorFor(model => model.PasswordConfirmation, new { @class = "form-control" })
                    @Html.ValidationMessageFor(model => model.PasswordConfirmation)
                </div>
            </div>
        </div>
        <div class="col-md-6">
            <div class="org-info">
                <div class="form-group">
                    @Html.LabelFor(model => model.Account.OrganisationName, new { @class = "control-label col-md-5" })
                    <div class="col-md-7">
                        @Html.EditorFor(model => model.Account.OrganisationName, new { @class = "form-control" })
                        @Html.ValidationMessageFor(model => model.Account.OrganisationName)
                    </div>
                </div>
            </div>
            <hr />
            <div class="form-group">
                <div class="col-md-8"></div>
                <div class="col-md-4">
                    <input type="submit" value="Sign Up" class="btn btn-default form-control" />
                </div>
            </div>
        </div>
    </div>
}

My issue is that when a user clicks Register without entering any details on the view, only the password and password confirmation fields show as having issues, none of the Account or ApplicationUser Properties show issue.

If I enter details for the password and confirmation fields and put a break point in the Register method, the ModelState.IsValid value is true even though I haven't added any Account or User details.

If I do enter say a username, evaluating...

Request.Form["ApplicationUser.UserName"]

gives the correct value and yet ApplicationUser.UserName which I would think should be populated by the Bind has nothing!?

I suspect it's my Bind declaration but I have tried UserName, ApplicationUser.UserName and ApplicationUser_UserName but all seem to have the same problem.

This question has come about from another question I raised (link below), so what am I missing?

Entity Framework 6 Reusing Data Annotations

Please note that I'm interested in what's wrong in this particular implementation rather than being offered alternate implementations for various reasons I don't really want to go into.

I've never used a view model with embedded classes like you're doing, but would adding a `[Required]` attribute to `ApplicationUser` and `Account` do anything? — Jeff Guillaume, May 07 '14 at 16:32
Please so your view. My proof of concept works with and without constructor and with and without Bind attribute. It must have something to do with your View, and how you're posting the values. — JotaBe, May 07 '14 at 19:57
Just added view field examples that I'm using and further information. — Hoots, May 08 '14 at 09:03

JotaBe · Accepted Answer · 2015-04-28T07:41:08.753

1

I've downloaded your code and included it in a new empty MVC4 application.

The bad news is that it didn't work.

The good news is that it can work perfectly with just an small change:

In your controller,

either remove the Bind attribute of the Register POST action parameter
or use this one: [Bind(Include = "Account, ApplicationUser, Password, PasswordConfirmation")]

And a pair of comments about your code:

unless you want to explicitly include or exclude part of your model (usually for security reasons) you don't need to use the Bind attribute. You'd rather create a ViewModel with the exact properties that you need (in that way you type and maintain less code!).
you don't need to supply a default constructor to initialize the nested objects. The MVC model binder will instantiate them automatically. In fact, I recommend you not to do it: if you forget to include the properties of the nested object in your view, that nested object should be null, and not an object with null properties. That can create a lot of confussion!!

edited Apr 28 '15 at 07:41

answered May 09 '14 at 09:38

JotaBe

38,030
8
98
117

Thanks for looking it's much appreciated. I'm glad my code showed up the issue for you. I think your answer has made it clear to me now. From what you're saying it's not possible to Bind to an internal object's property individually without specifying an additional property at the view level to Bind to? This does have implications for my Original Data Annotation Reuse question no? – Hoots May 09 '14 at 11:29
I'm sorry I don't understand the first part of your question (a simple example will do!!). As to the original question, it will work fine, you'll find no problems: if the type (class) of a nested property have a buddy class, its annotations are correctly applied. I made a proof of concept to test it. So you can have a combination of entities, buddy classes, view models... and it will work fine. You can modify your example project, and move the annotations to buddy classes, and you'll see it works perfectly. – JotaBe May 09 '14 at 13:16
First part of question illustration... Your answer states Removing Bind works, Binding to Account and ApplicationUser work (and I agree as I simply followed what you said and it's great), BUT binding to ApplicationUser.UserName doesn't work so this says to me you can't bind to individual internal object properties directly but would have to create another property in the ViewModel, e.g. string UserName { set { ApplicationUser.UserName=Value; } } and then Bind to "UserName". I was just asking whether you agree with that statement? I will try the MetadataType now. – Hoots May 09 '14 at 13:53
I'll keep comments relevant to the question and comment on the MetadataType implementation on the other one. – Hoots May 09 '14 at 14:13
If you're refering to the Bind attribute, yes, in the Bind attribute you cannot specify "subproperties": it only support properties in the root level. The question is: why do you want to use the Bind attribute instead of the default binding? If you're afraid of over-posting, using a model which only has the needed properties, or discarding the vulnerable properties in your controller should suffix. If you use buddy classes, you can decalre a model with less properties than his buddy, an it works perfectly. Why do you want to use bind? – JotaBe May 10 '14 at 01:19

malkam · Answer 2 · 2014-05-07T16:51:06.150

Try with including ApplicationUser and Account properties in RegisterViewModel itself.

    public class RegisterViewModel
{
    [Display(Prompt = "Your Name", Name = "Contact Name"), Required(), StringLength(255, MinimumLength = 3, ErrorMessage = "Please enter a valid contact")]
    public string ContactName { get; set; }
    [Display(Name = "Username", Prompt = "Login As"), Required(), StringLength(255, MinimumLength = 3, ErrorMessage = "Your username must be at least 3 characters")]
    public string UserName { get; set; }
[Display(Name = "Account Type", Prompt = "Account Type"), Required()]
    public AccountType AccountType { get; set; }
    [Display(Name = "Organisation Name", Prompt = "Name of Organisation"), StringLength(255)]
    public string OrganisationName { get; set; }
    public RegisterViewModel()
    {

    }

    [Display(Name = "Password", Prompt = "Password"), Required(), DataType(DataType.Password), StringLength(255, MinimumLength = 5, ErrorMessage = "The password must be at least 7 characters")]
    public string Password { get; set; }

    [Display(Name = "Confirm Password", Prompt = "Confirm Password"), Compare("Password", ErrorMessage = "Your confirmation doesn't match...")]
    public string PasswordConfirmation { get; set; }
}

Thanks, I know it's possible to just include everything in the view model, as you'll see from the link at the bottom of my post, it about doing this whilst keeping things in their own encapsulated objects, so the prerequisite is the Account and ApplicationUser objects have to stay as they are, reusing their Annotations. — Hoots, May 08 '14 at 09:07

score 0 · Answer 3 · answered May 07 '14 at 17:15

You might want to remove the constructor (since they are required its ok if they are null)

public RegisterViewModel()
{
    ApplicationUser = new ApplicationUser();
    Account = new Account();
}

and make ApplicationUser and Account properties required.

Validation happens during binding, and if there is no binding happening to both these properties then your model state will be valid by design.

MVC ModelState IsValid reports true when should be false

3 Answers3

Linked