I am trying to edit the data for one of my users. However, whenever I edit something, the passwords which are hidden, are also being changed and apparently set to null, which render the user unable to log in next time he wants to login. I know that I might have been able to solve the issue by using ViewModels, but im trying to do it without.
Model
public class User : IdentityUser
{
[Display(Name = "First name")]
public String FirstName { get; set; }
[Display(Name = "Last name")]
public string LastName { get; set; }
public string Email{ get; set; }
}
Please notice that the User-class extends from IdentityUser which holds password variables.
Edit in Controller
@using (Html.BeginForm())
{
@Html.AntiForgeryToken()
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Edit([Bind(Include = "FirstName,LastName,Email,PhoneNumber")] User user)
{
if (ModelState.IsValid)
{
db.Entry(user).State = System.Data.Entity.EntityState.Modified;
db.SaveChanges();
return RedirectToAction("Index");
}
return View(user);
}
View for Edit
<div class="form-horizontal">
<h4>User</h4>
<hr />
@Html.ValidationSummary(true, "", new { @class = "text-danger" })
@Html.HiddenFor(model => model.Id)
<div class="form-group">
@Html.LabelFor(model => model.FirstName, htmlAttributes: new { @class = "control-label col-md-2" })
<div class="col-md-10">
@Html.EditorFor(model => model.FirstName, new { htmlAttributes = new { @class = "form-control" } })
@Html.ValidationMessageFor(model => model.FirstName, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
@Html.LabelFor(model => model.LastName, htmlAttributes: new { @class = "control-label col-md-2" })
<div class="col-md-10">
@Html.EditorFor(model => model.LastName, new { htmlAttributes = new { @class = "form-control" } })
@Html.ValidationMessageFor(model => model.LastName, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
@Html.LabelFor(model => model.Email, htmlAttributes: new { @class = "control-label col-md-2" })
<div class="col-md-10">
@Html.EditorFor(model => model.Email, new { htmlAttributes = new { @class = "form-control" } })
@Html.ValidationMessageFor(model => model.Email, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
@Html.LabelFor(model => model.PhoneNumber, htmlAttributes: new { @class = "control-label col-md-2" })
<div class="col-md-10">
@Html.EditorFor(model => model.PhoneNumber, new { htmlAttributes = new { @class = "form-control" } })
@Html.ValidationMessageFor(model => model.PhoneNumber, "", new { @class = "text-danger" })
</div>
</div>
<div class="form-group">
<div class="col-md-offset-2 col-md-10">
<input type="submit" value="Save" class="btn btn-default" />
</div>
</div>
}
It is my understand that the Bind-parameter in the Edit-method either whitelist or blacklist the variables for editing. So for that reason i removed all of the values that shouldnt be edited by the user in Bind.
There are a couple of problems here. First, don't present these fields to the user in the first place. I can't imagine a reason why a user should be able to edit their "locked out" status, or their hashed password. Only include in the UI the fields which the user should actually be modifying. Hell, even this has horrible idea written all over it:
@Html.HiddenFor(model => model.Id)
You're not only allowing the user to edit everything about their user record, but you're also allowing them to specify another user record to edit. So any user in the system can completely edit any other user in the system.
Hopefully you see how this is a bad thing :)
Now, you can (and often must) include the identifier in a hidden field. The above problem is mainly bad because of what else you're doing:
db.Entry(user).State = System.Data.Entity.EntityState.Modified;
db.SaveChanges();
You are completely and implicitly trusting whatever the user sends you to be a whole, correct, and complete record for the database. And replacing whatever existing record is there with whatever the user sends you. That's... not good.
This approach can work for some models, but certainly not sensitive ones like user security data.
Instead, fetch the existing record and only edit the necessary fields. Something like this:
var existingUser = db.Users.Single(u => u.Id == currentUserId);
existingUser.FirstName = user.FirstName;
existingUser.LastName = user.LastName;
// etc.
db.SaveChanges();
Notice that I used an otherwise undefined variable called currentUserId. Do not use model.Id, because again that's allowing the user to specify which other user they want to edit. Determing the current user ID by their current logged in session, not by what they send in the form. However you currently identify your users. (User.Identity?)
In short...