-->

ASP.NET Membership change password not working

2020-07-03 05:32发布

问题:

I have this code for changing a user's password when they click the password reset button (with extra code to log to ELMAH so I can try to figure out what is going wrong).

This is in ASP.NET MVC 2, using the standard aspnet membership provider, with a simple View like this:

New Password:     ______
Confirm Password: ______
[Reset] [Cancel]

The route to this view is /Account/Reset/guid, where guid is the user's id in the aspnet membership database.

The key portion of the code is where it calls user.ChangePassword(). You can see that it logs a message when successful. The problem is that for some users, the success message is logged, but they can not log in with the new password. For other users it logs the success message and they can log in.

if (user.ChangePassword(pwd, confirmPassword))
{
    ErrorSignal.FromCurrentContext().Raise(
        new Exception("ResetPassword - changed successfully!"));
    return Json(new { 
        Msg = "You have reset your password successfully." }, 
        JsonRequestBehavior.AllowGet);
 }

The full code listing is:

[HttpPost]
public JsonResult ResetPassword(string id, string newPassword, string confirmPassword)
{
    ErrorSignal.FromCurrentContext().Raise(new Exception("ResetPassword started for " + id));

    ViewData["PasswordLength"] = Membership.MinRequiredPasswordLength;

    if (string.IsNullOrWhiteSpace(newPassword))
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - new password was blank."));
        ModelState.AddModelError("_FORM", "Please enter a new password.");
        return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
    }

    if (newPassword.Length < Membership.MinRequiredPasswordLength)
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - new password was less than minimum length."));
        ModelState.AddModelError("_FORM", 
            string.Format("The password must be at least {0} characters long.", 
            Membership.MinRequiredPasswordLength));
        return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
    }

    if (string.IsNullOrWhiteSpace(confirmPassword))
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - confirm password was blank."));
        ModelState.AddModelError("_FORM", 
            "Please enter the same new password in the confirm password textbox.");
        return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
    }

    if (confirmPassword.Length < Membership.MinRequiredPasswordLength)
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - confirm password was less than minimum length."));
        ModelState.AddModelError("_FORM", 
            string.Format("The password must be at least {0} characters long.", 
            Membership.MinRequiredPasswordLength));
        return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
    }

    if (confirmPassword != newPassword)
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - new password did not match the confirm password."));
        ModelState.AddModelError("_FORM", "Please enter the same password again.");
        return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
    }

    bool isMatch = ValidationHelper.IsGUID(id);
    if (string.IsNullOrWhiteSpace(id) || !isMatch)
    {
        ErrorSignal.FromCurrentContext().Raise(
            new Exception("ResetPassword - id was not a guid."));
        ModelState.AddModelError("_FORM", "An invalid ID value was passed in through the URL");
    }
    else
    {
        //ID exists and is kosher, see if this user is already approved
        //Get the ID sent in the querystring
        Guid userId = new Guid(id);

        try
        {
            //Get information about the user
            MembershipUser user = Membership.GetUser(userId);
            if (user == null)
            {
                //could not find the user
                ErrorSignal.FromCurrentContext().Raise(
                    new Exception("ResetPassword - could not find user by id " + id));
                ModelState.AddModelError("_FORM", 
                    "The user account can not be found in the system.");
            }
            else
            {
                ErrorSignal.FromCurrentContext().Raise(
                    new Exception("ResetPassword - user is " + user.UserName));
                string pwd = user.ResetPassword();

                if (user.ChangePassword(pwd, confirmPassword))
                {
                    ErrorSignal.FromCurrentContext().Raise(
                        new Exception("ResetPassword - changed successfully!"));
                    return Json(new { 
                        Msg = "You have reset your password successfully." }, 
                        JsonRequestBehavior.AllowGet);
                }
                ErrorSignal.FromCurrentContext().Raise(
                    new Exception("ResetPassword 
                    - failed to change the password, for an unknown reason"));
            }
        }
        catch (Exception ex)
        {
            ErrorSignal.FromCurrentContext().Raise(
                new Exception("ResetPassword: " + ex));
            return Json(new { Error = ex.Message + " -> " 
                + ex.InnerException.Message }, JsonRequestBehavior.AllowGet);
        }
    }

    return Json(new { Errors = ModelState.Errors() }, JsonRequestBehavior.AllowGet);
}

Edit: Adding a bounty to try to get this solved. This is one of the most annoying problems on my issue list, and I have no idea how to proceed.

回答1:

If the user needs to reset his password, there is a chance their account has been locked out from too many invalid attempts. If this is the case, then the password is being reset successfully, but the user cannot log in until the lockout condition is cleared.

Try checking MembershipUser.IsLockedOut:

Users are most commonly locked out and cannot be validated by the ValidateUser method when the MaxInvalidPasswordAttempts is reached within the PasswordAttemptWindow.

To set this property to false and let the user try to log in again, you can use the UnlockUser method.

Edit

Did you also check IsApproved? Authentication will fail is this is false for the user.

Also, assuming by default membership provider, you mean the SqlMembershipProvider, can you run the following query against your database and make sure everything looks correct?

select IsApproved, IsLockedOut, FailedPasswordAttemptCount
from aspnet_Membership
where ApplicationId = @yourApplicationId and UserId = @userId

Try executing the query before attempting to sign in to verify IsApproved and IsLockedOut are ok. Also note the value for FailedPasswordAttemptCount.

Try signing in, and then run the query again. If signin fails, has the value for FailedPasswordAttemptCount been incremented?

You could also look at PasswordFormat in the aspnet_Membership table and make sure it is the correct value depending on the format you are using (0 for Clear, 1 for Hashed, and 2 for Encrypted).



回答2:

Hmm, I've always used

bool MembershipUser.ChangePassword(string oldPassword, string newPassword)

I've never had an issue with it returning true and the password not being changed correctly. As fas as I can tell your code looks ok. It is difficult to follow, with all the Elmah noise in there. (you might want to remove it or replace with a simple log call so it's easier to follow).

Verify that the string id that you pass as an argument corresponds to the UserId of the intended user. You might be sending the userId from some other user and changing that users password instead.



回答3:

Edited - following answer is false see comments

So wait are you trying to locate someone by a Guid? By doing

Guid userId = new Guid(id);

You are practically creating a guaranteed to be unique ID. So my guess is you are never finding a user and you are resetting a password successfully for nobody. Can you not just find them by the id parameter you pass in?



回答4:

This works for me:

<%@ Page Title="Change Password" Language="C#" MasterPageFile="~/Site.master" AutoEventWireup="true"
    CodeBehind="ChangePassword.aspx.cs" Inherits="WebPages.Account.ChangePassword" %>

<asp:Content ID="HeaderContent" runat="server" ContentPlaceHolderID="HeadContent">
</asp:Content>
<asp:Content ID="BodyContent" runat="server" ContentPlaceHolderID="MainContent">
    <h2>
        Change Password
    </h2>
    <p>
        Use the form below to change your password.
    </p>
    <p>
        New passwords are required to be a minimum of <%= Membership.MinRequiredPasswordLength %> characters in length.
    </p>
    <asp:ChangePassword ID="ChangeUserPassword" runat="server" CancelDestinationPageUrl="~/" EnableViewState="false" RenderOuterTable="false" 
        OnChangedPassword="ChangeUserPassword_ChangedPassword">
        <ChangePasswordTemplate>
            <span class="failureNotification">
                <asp:Literal ID="FailureText" runat="server"></asp:Literal>
            </span>
            <asp:ValidationSummary ID="ChangeUserPasswordValidationSummary" runat="server" CssClass="failureNotification" 
                 ValidationGroup="ChangeUserPasswordValidationGroup"/>
            <div class="accountInfo">
                <fieldset class="changePassword">
                    <legend>Account Information</legend>
                    <p>
                        <asp:Label ID="CurrentPasswordLabel" runat="server" AssociatedControlID="CurrentPassword">Old Password:</asp:Label>
                        <asp:TextBox ID="CurrentPassword" runat="server" CssClass="passwordEntry" TextMode="Password"></asp:TextBox>
                        <asp:RequiredFieldValidator ID="CurrentPasswordRequired" runat="server" ControlToValidate="CurrentPassword" 
                             CssClass="failureNotification" ErrorMessage="Password is required." ToolTip="Old Password is required." 
                             ValidationGroup="ChangeUserPasswordValidationGroup">*</asp:RequiredFieldValidator>
                    </p>
                    <p>
                        <asp:Label ID="NewPasswordLabel" runat="server" AssociatedControlID="NewPassword">New Password:</asp:Label>
                        <asp:TextBox ID="NewPassword" runat="server" CssClass="passwordEntry" TextMode="Password"></asp:TextBox>
                        <asp:RequiredFieldValidator ID="NewPasswordRequired" runat="server" ControlToValidate="NewPassword" 
                             CssClass="failureNotification" ErrorMessage="New Password is required." ToolTip="New Password is required." 
                             ValidationGroup="ChangeUserPasswordValidationGroup">*</asp:RequiredFieldValidator>
                    </p>
                    <p>
                        <asp:Label ID="ConfirmNewPasswordLabel" runat="server" AssociatedControlID="ConfirmNewPassword">Confirm New Password:</asp:Label>
                        <asp:TextBox ID="ConfirmNewPassword" runat="server" CssClass="passwordEntry" TextMode="Password"></asp:TextBox>
                        <asp:RequiredFieldValidator ID="ConfirmNewPasswordRequired" runat="server" ControlToValidate="ConfirmNewPassword" 
                             CssClass="failureNotification" Display="Dynamic" ErrorMessage="Confirm New Password is required."
                             ToolTip="Confirm New Password is required." ValidationGroup="ChangeUserPasswordValidationGroup">*</asp:RequiredFieldValidator>
                        <asp:CompareValidator ID="NewPasswordCompare" runat="server" ControlToCompare="NewPassword" ControlToValidate="ConfirmNewPassword" 
                             CssClass="failureNotification" Display="Dynamic" ErrorMessage="The Confirm New Password must match the New Password entry."
                             ValidationGroup="ChangeUserPasswordValidationGroup">*</asp:CompareValidator>
                    </p>
                </fieldset>
                <p class="submitButton">
                    <asp:Button ID="CancelPushButton" runat="server" CausesValidation="False" CommandName="Cancel" Text="Cancel"/>
                    <asp:Button ID="ChangePasswordPushButton" runat="server" CommandName="ChangePassword" Text="Change Password" 
                         ValidationGroup="ChangeUserPasswordValidationGroup"/>
                </p>
            </div>
        </ChangePasswordTemplate>
        <SuccessTemplate>
            <div class="accountInfo">
                <fieldset class="changePassword">
                    <legend>Password changed</legend>
                        <p>
                            Your password has been changed. A confirmation e-mail has been sent to you.
                        </p>
                </fieldset>
            </div>
        </SuccessTemplate>
    </asp:ChangePassword>
</asp:Content>


回答5:

I wonder if the problem is that you are resetting the password right before you change it. Without getting into all of the internals of the Membership class, could you try putting in some kind of delay between those two commands?



回答6:

Which MemberShipProvider are you using? Is it the same for every user? For instance, if you use SqlMembershipProvider and set enablePasswordReset to false, it will quietly fail to update the password. ChangePassword, in this case, returns true as if everything went fine.



回答7:

If you are using the built-in SQLServer based providers take a look at your SQL Stored procs. This is what my default proc looks like:

ALTER PROCEDURE dbo.aspnet_Membership_SetPassword
    @ApplicationName  nvarchar(256),
    @UserName         nvarchar(256),
    @NewPassword      nvarchar(128),
    @PasswordSalt     nvarchar(128),
    @CurrentTimeUtc   datetime,
    @PasswordFormat   int = 0
AS
BEGIN
    DECLARE @UserId uniqueidentifier
    SELECT  @UserId = NULL
    SELECT  @UserId = u.UserId
    FROM    dbo.aspnet_Users u, dbo.aspnet_Applications a, dbo.aspnet_Membership m
    WHERE   LoweredUserName = LOWER(@UserName) AND
            u.ApplicationId = a.ApplicationId  AND
            LOWER(@ApplicationName) = a.LoweredApplicationName AND
            u.UserId = m.UserId

    IF (@UserId IS NULL)
        RETURN(1)

    UPDATE dbo.aspnet_Membership
    SET Password = @NewPassword, PasswordFormat = @PasswordFormat, PasswordSalt = @PasswordSalt,
        LastPasswordChangedDate = @CurrentTimeUtc
    WHERE @UserId = UserId
    RETURN(0)
END

As you can see the update statement could totally fail and the stored proc could return true. I think this is where your errors are probably coming from. Could be locking issues...



回答8:

Well this is certainly an interesting one. The "it works for some, not for others" part is really bizarre.

Is this an intermittent problem, or does it always occur for certain users, and always not occur for other users?

One of the other people here suggested running ValidateUser(username, newPassword) to confirm that the user could properly authenticate before assuming success.

Have you tried this? You could continuously loop, resetting + changing the password until ValidateUser is successful, perhaps exiting after N failures.

bool success = false;
int numAttempts = 0;
do
{
    string pwd = user.ResetPassword();
    if (user.ChangePassword(pwd, confirmPassword))
    {
        success = Membership.ValidateUser(user.UserName, pwd);
    }
    numAttempts++;
} while(numAttempts < 5 && !success);

Note: This is not for use in production, just for testing to see if this resolves the problem.



回答9:

Are you using 1 webserver or multiple webservers? With multiple servers it could be that the machinekey used for encrypting the password is not the same on al servers.



回答10:

Could your main catch block be throwing an exception itself that you haven't noticed?

catch (Exception ex)
{
    ErrorSignal.FromCurrentContext().Raise(new Exception("ResetPassword: " + ex));
    return Json(new { Error = ex.Message + " -> " 
            + ex.InnerException.Message }, JsonRequestBehavior.AllowGet);
}

The ex.InnerException.Message statement is not safe because it could throw a NullReferenceException.