ASP.net Identity admin reset user password

Question

I have an application which uses ASP.net Identity as for its user authentication.

I do not wish for users to reset their own password (for reasons which are not relevant here), so I have developed the following back end system to allow system administrators to reset the users password.

I have the following implementation:

Password Reset View:

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

    <div class="form-horizontal">
        <h4>ResetPasswordViewModel</h4>
        <hr />
        @Html.ValidationSummary(true, "", new { @class = "text-danger" })
        @Html.HiddenFor(model => model.Id)

        <div class="form-group">
            @Html.LabelFor(model => model.NewPassword, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.EditorFor(model => model.NewPassword, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.NewPassword, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            @Html.LabelFor(model => model.ConfirmPassword, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.EditorFor(model => model.ConfirmPassword, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.ConfirmPassword, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            <div class="col-md-offset-2 col-md-10">
                <input type="submit" value="Reset Password" class="btn btn-default" />
            </div>
        </div>
    </div>
}

AccountAdmin Controller

    // GET: /AccountAdmin/ResetPassword
    public ActionResult ResetPassword(string id)
    {
        if (id == null)
        {
            return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
        }
        ResetPasswordViewModel model = new ResetPasswordViewModel() { Id = id };
        return View(model);
    }

    //
    // POST: /AccountAdmin/ResetPassword
    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult ResetPassword(ResetPasswordViewModel model)
    {
        if (!ModelState.IsValid)
        {
            return View(model);
        }

        var removePassword = UserManager.RemovePassword(model.Id);
        if (removePassword.Succeeded)
        {
            //Removed Password Success
            var AddPassword = UserManager.AddPassword(model.Id, model.NewPassword);
            if (AddPassword.Succeeded)
            {
                return View("PasswordResetConfirm");
            }
        }

        return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
    }

ResetPassword View Model

public class ResetPasswordViewModel
{
    public string Id { get; set; }

    [Required]
    [StringLength(100, ErrorMessage = "The {0} must be at least {2} characters long.", MinimumLength = 6)]
    [DataType(DataType.Password)]
    [Display(Name = "New password")]
    public string NewPassword { get; set; }

    [DataType(DataType.Password)]
    [Display(Name = "Confirm new password")]
    [Compare("NewPassword", ErrorMessage = "The new password and confirmation password do not match.")]
    public string ConfirmPassword { get; set; }
}

Answer 1

One thing you should keep in mind is that when the Add fails, your user is left without a password as it's already been removed. When will this fail? Usually when your password does not pass the validation.

Of course, seeing how an admin is doing it, there's probably not so much of a problem as they can simply add a different one - as long as the message is clear enough to show there actually is a problem with adding the new password.

The way Microsoft wants you to tackle this is by using a PasswordResetToken.

var code = await _userManager.GeneratePasswordResetTokenAsync(user);
await _userManager.ResetPasswordAsync(user, code, newPassword);

For this you'll have to fiddle around a bit with the token generators that Identity provides.

Answer 2

My main objection was against AccountAdmin Controller. Your latest comment makes my point a bit moot, but here it is anyway.

This controller knows too much about the details of account management. It's not cool that you have to remove and re-add a password to reset it, and these operations pollute your code. A useful approach is to create a helper class to collect all the dirty elements in one place, until you find a better solution. For example, create a UserManagerHelper class with a ResetPassword method. This way your controller code can remain clean, and the strange remove password + re-add password stuff could be encapsulated and hidden in the helper.

Another thing that's not cool about this class is the two variants of the ResetPassword. The inspection from one to the other is confusing, and it seems to result in an unnecessary validity check when coming from a GET request. It would be better to factor out the common logic to a third method, and these two method should not call each other.