patterncsharpMinor
Generating and sending new password to user
Viewed 0 times
newusersendingpasswordgeneratingand
Problem
Generate new password and send to user, if user is registered.
generate new password and Email.
email then generate new password and Email.
Is there a better approach to avoid so many
- Two
TextBoxon page,txtUserName&txtEamil.
- User can provide either
UserNameorEmailto get new password.
- If
UserNameis provided then find User onusername, 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:
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
- 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.