HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generating and sending new password to user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
newusersendingpasswordgeneratingand

Problem

Generate new password and send to user, if user is registered.

  • Two TextBox on page, txtUserName & txtEamil.



  • User can provide either UserName or Email to get new password.



  • If UserName is provided then find User on username, if exists then


generate new password and Email.

  • If user provided email then check if any user exists with provided


email then generate new password and Email.

protected void btnSubmit_Click(object sender, EventArgs e)
{
    string userName = txtUserName.Text.Trim();
    string email = txtEmail.Text.Trim();

    //
    if (string.IsNullOrEmpty(userName) && string.IsNullOrEmpty(email))
    {
        lblMessage.Text = "Please provide at least either of the one, User Name or Email.";
    }
    else
    {
        SendNewPasswordToUser(userName, email);
    }
}

private void SendNewPasswordToUser(string userName, string email)
{
    if (!string.IsNullOrEmpty(userName))
    {
        MembershipUser mu = Membership.GetUser(userName);
        if (mu != null)
        {
            string password = mu.ResetPassword();
            EmailPassword(password, mu.Email);
        }
    }
    else if (!string.IsNullOrEmpty(email))
    {
        userName = Membership.GetUserNameByEmail(email);
        if (!string.IsNullOrEmpty(userName))
        {
            MembershipUser mu = Membership.GetUser(userName);
            string password = mu.ResetPassword();
            EmailPassword(password, mu.Email);
        }
    }
}

private void EmailPassword(string password, string toEmail)
{
    string mailBody = string.Format("Your new password is {0}", password);
    Mail.SendMail("admin@abc.com", toEmail, "New Password", mailBody);
}


Is there a better approach to avoid so many ifs?

Solution

Before reviewing the code, the concept itself has some flaws:

  • It is possible to reset the password of another user by entering their email address or username.



  • The password is provided directly in the email which is bad practise (not only is this in clear text but it will be retained in the user's inbox which is not a good place to store passwords).



Entering the email or username should display the same message to the website user regardless of whether the user exists or not. This would stop an attacker from enumerating usernames (i.e. finding whether a particular username or email address is registered on your system). In the email itself include either the details of a password reset link or an account not found message as appropriate for that username/email - this way only the email address owner can see whether they have an account on your system.

The password reset link should be include a time limited reset token that will allow the user to change their password to one of their choosing and it should not then email this out anywhere. Make sure the token is securely generated and does not use a predictable sequence (i.e. a cryptographically secure sequence and not one generated using a Random).

Context

StackExchange Code Review Q#33341, answer score: 4

Revisions (0)

No revisions yet.