Trouble Retrieving Data

Hi! It looks like hf_users_id is never initialized. What is this variable for?

Hi ivan_vagunin,

hf_users_id will contain int values.

So should not this be cmd.Parameters.Add("@users_id", SqlDbType.Int).Value = UsersID;? What problem do you have? Some exception?

Yeah, that is what i tried but no data is retrieved :(

attached is my updated code along with stored procedure.

protected void Page_Load(object sender, EventArgs e)
    {
        RetrieveUsersIDFullName();
    }

    protected void RetrieveUsersIDFullName()
    {
        int UsersID = Convert.ToInt32(Request.QueryString["users_id"]);

        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_RetrieveUsersIDFullName";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.Add("@users_id", SqlDbType.Int).Value = UsersID;

            try
            {
                conn.Open();

                SqlDataReader rdr = cmd.ExecuteReader();

                if (rdr.Read())
                {
                    lblUsersFullName.Text = rdr["users_flname"].ToString();
                }
            }

            catch (Exception ex)
            {
                ex.Message.ToString();
            }
        }
    }

Select allOpen in new window

ALTER PROCEDURE [dbo].[HealthCourses_RetrieveUsersIDFullName]

(
@users_id int
)

AS 

SELECT users_id, users_flname
FROM dbo.HealthCourses_Users
WHERE users_id = @users_id

Select allOpen in new window

Now code looks right. So you need to check if UsersID is parsed correctly and if db contains users with such UsersID.

Hi ivan vaqunin,

Ok, so all code attached below works fine. Do you mind looking over it and telling me if it's good to use, or if there are a few things that you would change? Do I need to close the SqlDataReader, or is it fine since it's wrapped into a using statement?

protected void RetrieveUsersIDFullName()
    {
        int User = Convert.ToInt32(Request.QueryString["users_id"]);

        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_RetrieveUsersIDFullName";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.Add("@users_id", SqlDbType.Int).Value = User;

            try
            {
                conn.Open();

                SqlDataReader rdr = cmd.ExecuteReader();

                if (rdr.Read())
                {
                    hf_users_id.Value = rdr["users_id"].ToString();
                    lblUsersFullName.Text = rdr["users_flname"].ToString();
                }
            }

            catch (Exception ex)
            {
                ex.Message.ToString();
            }
        }
    }

    protected void btn_ResetPassword_Click(object sender, EventArgs e)
    {
        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_UpdateUsersPassword";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.Add("@users_id", SqlDbType.Int).Value = hf_users_id.Value;
            cmd.Parameters.Add("@users_password", SqlDbType.Binary, 96).Value = PasswordHash.HashPassword(txtPassword.Text);

            try
            {
                conn.Open();
                cmd.ExecuteNonQuery();

                Response.Redirect("resetpassword_success.aspx");
            }

            catch (Exception ex)
            {
                lblResetPasswordError.Text = ex.Message.ToString();
                //ex.Message.ToString();
            }
        }
    }

Select allOpen in new window

Ok, how would you perform the following below then?

>> The only thing I would worry about is secutiry. It is not secure to pass user id in query string, it looks like any user can reset passwords of other user by calling btn_ResetPassword_Click()

I would propose to generate more complex temp link for that puporse to make it more secure. Having static link for password reset is bad, because somebody could know this link sooner or later. And simply passing integer id as user key is not an option at all, because it's not hard to guess how to hack another account.
So I would two more columns to users table - PasswordResetKeyHash, and PasswordResetKeyValid. Once user requested a password reset you should generate a secret key (some random password), write key hash (e.g. SHA1) to PasswordResetKey column and write time or key generation to PasswordResetKeyValid column. Then you send secret key to user via email (maybe as url param):
http://programinfo/ghap/resertpassword.aspx?h=8293yh23hkh98y2309y230h023yh32ee2
Once user goes to this page you calculate password hash with the same algorithm and seach users table by secret key's hash. If found, clear the PasswordResetKeyHash (you don't need it any more) read PasswordResetKeyValid  and check that hash is not expired and you're ready to set new password. Don't forget to clear PasswordResetKeyValid column.
Using this sequence does not allow user to generate password reset link on his own. Hope that helps!

Hi ivan_vagunin,

That method sounds good. I never thought about the flaw in how I was handling it now. Can you assist with the code?

I will add those two columns to my table now.

Hi ivan_vagunin,

Also, would that need to be an Insert or Update into the DB when a user needs to generate a random password to send in to their email? Because, I'm not sure how that would be handled, since a user may forget more than once.

So why do think it would be a problem with several password resets? The secret PasswordResetKeyHash should be generated each time password reset is requested by user - it will require Update.

Ok, so, it would be an update in my stored procedure even if the user requested a password change for the first time? I guess I was confused on how to handle the insert/update logic if it was the first time for a user to click on the reset button to have those two values added to the table.

Also, why do I need two "PasswordResetKeyHash" and "PasswordResetKeyValid"?

Yes, user row should be updated every time user has requested password reset. "PasswordResetKeyValid" is to check password reset request expiration. Usually, password reset hyperlink is valid only for short period of time - few hours (the shorter - more secure). So need to know when secret key was created (or when it gets expired) and this date should be stored in PasswordResetKeyValid. However that's not neccessary.

What are the fields data types in the Database? Is "PasswordResetKeyValid" a DateTime?

Also, would you be able to help me with the code when you have time?

"PasswordResetKeyValid" is DateTime and "PasswordResetKeyHash" is text (length depends on Hash algorith you want to use).
Yes, I will help with code, unfortunately I am afraid I won't have much time until tomorrow evening.

Hi ivan_vagunin,

Thank you! That is fine, I just appreciate you helping me with the code. Take your time and I look forward seeing what you come up with later :)

I would like to use SHA-2 SHA-256 or SHA-512 for the HASH if possible when you do the code for me later.

Hi!
So what you need is some like following. First, you need function to generate password reset link. It should be like following:
//generate random string
protected string GetRandomKey()
{
      byte[] Nonce = new byte[50];
      RandomNumberGenerator rng = RandomNumberGenerator.Create();
      rng.GetBytes(Nonce);
      return Convert.ToBase64String(Nonce);
}
string GetResetPasswordLink(string userName)
{
    string res = string.Empty;
    int userID = GetUserIDByLoginName(userName);//this function should search users table and return ID if found
    if(userID != -1)
    {
             string secretKey = GetRandomKey();
             DateTime secretKeyGenerated = DateTime.Now;

             SHA512 sha512 = SHA512.Create();
            string hash = Convert.ToBase64String(sha256.ComputeHash(System.Text.Encoding.Unicode.GetBytes(secretKey)));        
           
             SaveHashToDB(userID, hash, secretKeyGenerated);//this function should update user row with new hash
//and hash generation time
             
             //create url
              res = string.Format("http://programinfo/ghap/resertpassword.aspx?p={0}", hash);
    }
    return res;
}

Once url is generated you should send it to client and wait while user clicks it. When user clicks it (navigate to resetpassword.aspx) you should check that Url contains valid key hash and reset password.
proctected IsValidHash(stirng hash)
{
      DateTime generatedTime = ReadHashGeneratedTime(hash);//This function should search row from users table //that contains specified hash and return value from PasswordResetKeyValid column and DateTime.Min if row not //found

     TimeSpan time = DateTime.Now - generatedTime;
     return time.TotalHours < MaxValidHours;//max valid hours is a const that specify how long secret key is valid.
}

BTW, I'm not sure that computing hash of secret key is making it more secure, perhaps it's enough just to send random generated password to client without hashing.

Hi ivan vaqunin,

Thank you, please bare with me though, as this stuff is of a different language to me. I copied so far the following code below to my forgotpassword.aspx pages codebehind and I have a few red lines in the code.

The following red lines are on the lines below:

Line of Code with Red Line:
int userID = GetUserIDByLoginName(userName);

Message:
The name 'GetUserIDByLoginName' does not exist in the current context.

Line of Code with Red Line:
SaveHashToDB(userID, hash, secretKeyGenerated);

Message:
The name 'SaveHashToDB' does not exist in the current context.

protected string GetRandomKey()
    {
        byte[] Nonce = new byte[50];
        RandomNumberGenerator rng = RandomNumberGenerator.Create();
        rng.GetBytes(Nonce);
        return Convert.ToBase64String(Nonce);
    }

    string GetResetPasswordLink(string userName)
    {
        string res = string.Empty;
        int userID = GetUserIDByLoginName(userName);//this function should search users table and return ID if found
        if (userID != -1)
        {
            string secretKey = GetRandomKey();
            DateTime secretKeyGenerated = DateTime.Now;

            SHA512 sha512 = SHA512.Create();
            string hash = Convert.ToBase64String(sha512.ComputeHash(System.Text.Encoding.Unicode.GetBytes(secretKey)));

            SaveHashToDB(userID, hash, secretKeyGenerated);//this function should update user row with new hash
            //and hash generation time

            //create url
            res = string.Format("http://programinfo/ghap/resertpassword.aspx?p={0}", hash);
        }
        return res;
    }

Select allOpen in new window

Hi!

I meant that these functions (GetUserIDByLoginName and SaveHashToDB) you have to declare and implement.
The GetUserIDByLogin name should be like following:
protected string GetUserIDByLoginName(string login)
{
        int userId = -1;
         using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "SELECT users_id, users_login FROM dbo.HealthCourses_Users WHERE users_login  = @users_login ";
            cmd.Connection = conn;

            cmd.Parameters.Add("@ users_login").Value = login;
            conn.Open();

            SqlDataReader rdr = cmd.ExecuteReader();

            if (rdr.Read())
            {
                 userId  = int.Parse(rdr["users_id"].ToString());
            }
        }
     return userId;
}

The SaveHashToDB should update row with userId (you can write a stored procedure for that or just use SQL statement as in previous function
protected void SaveHashToDB(int userId, string secretKey, DateTime keyDate)
{
   //TODO add code here
}

Hi ivan_vagunin,

Ok, I will try to attempt to create the logic for those two functions above. However, how will those two functions be executed? The user first has to perform the "btn_ForgotPassword_Click" but how do I tie the other two functions in to that? The email is generated from the "btn_ForgotPassword_Click" function.

Also, would it be easier if I passed the salted+hashed password in the URL rather than doing all this? Then compare the salted + hashed password to the resetpassword.aspx page? Then if users_id and salted + hashed password matches then allow password to be changed?

Hi!
Sorry for very late response. What I proposed is indeed similar to what you've described. The difference is that your algorythm generates links that are valid forever, while mine links are valid for short period of time (so it is more secure).

Hi ivan_vagunin,

No problem, you have a life to :)

Before I talk about the GetUserIDByLogin and SaveHashToDB. I have a question about the current code that generates the resetpassword link now. Please see my attached code that generates the link to the user.

I don't understand why the link below has a short encryption/hash string instead of it being longer like you would normally see. I would think that the BbvhIxaZ/Y0= would be alot longer than that.

resetpassword.aspx?users_id=BbvhIxaZ/Y0=

CODEBEHIND:

    protected void btn_ForgotPassword_Click(object sender, EventArgs e)
    {
        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_VerifyUsernameEmail";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.Add("@users_username", SqlDbType.VarChar, 50).Value = txtUsername.Text;
            cmd.Parameters.Add("@users_email", SqlDbType.VarChar, 100).Value = txtEmail.Text;

            try
            {
                conn.Open();

                SqlDataReader rdr = cmd.ExecuteReader();

                if (rdr.Read())
                {
                    hf_users_id.Value = NewEncryption.Encrypt(rdr["users_id"].ToString());

                    SendForgotPasswordEmail();
                    Response.Redirect("forgotpassword_success.aspx");
                }
                else
                {

                    lblForgotPasswordError.Text = "We are unable to locate your account";
                }
            }

            catch (Exception ex)
            {
                lblForgotPasswordError.Text = ex.Message.ToString();
            }
        }
    }

Select allOpen in new window

NEWENCRYPTION CLASS CODE:

using System;
using System.Data;
using System.Configuration;
using System.Linq;
using System.Web;
using System.Web.Security;
using System.Web.UI;
using System.Web.UI.HtmlControls;
using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
using System.Xml.Linq;
using System.Text;
using System.Security.Cryptography;
using System.IO;

/// <summary>
/// Summary description for NewEncryption
/// </summary>
public class NewEncryption
{
    #region Fields
    private static byte[] key = { };
    private static byte[] IV = { 38, 55, 206, 48, 28, 64, 20, 16 };
    private static string stringKey = "!5663a#KN";
    #endregion

    #region Public Methods

    public static string Encrypt(string text)
    {
        try
        {
            key = Encoding.UTF8.GetBytes(stringKey.Substring(0, 8));
            DESCryptoServiceProvider des = new DESCryptoServiceProvider();
            Byte[] byteArray = Encoding.UTF8.GetBytes(text);
            MemoryStream memoryStream = new MemoryStream();
            CryptoStream cryptoStream = new CryptoStream(memoryStream,

            des.CreateEncryptor(key, IV), CryptoStreamMode.Write);
            cryptoStream.Write(byteArray, 0, byteArray.Length);
            cryptoStream.FlushFinalBlock();

            return Convert.ToBase64String(memoryStream.ToArray());
        }

        catch (Exception ex)
        {
            // Handle Exception Here
        }

        return string.Empty;
    }

    public static string Decrypt(string text)
    {
        try
        {
            key = Encoding.UTF8.GetBytes(stringKey.Substring(0, 8));
            DESCryptoServiceProvider des = new DESCryptoServiceProvider();
            Byte[] byteArray = Convert.FromBase64String(text);
            MemoryStream memoryStream = new MemoryStream();
            CryptoStream cryptoStream = new CryptoStream(memoryStream,

            des.CreateDecryptor(key, IV), CryptoStreamMode.Write);
            cryptoStream.Write(byteArray, 0, byteArray.Length);
            cryptoStream.FlushFinalBlock();

            return Encoding.UTF8.GetString(memoryStream.ToArray());
        }

        catch (Exception ex)
        {
            // Handle Exception Here
        }

        return string.Empty;
    }

    #endregion
}

Select allOpen in new window

In your code you use DES encryptions algoryth (http://en.wikipedia.org/wiki/Data_Encryption_Standard). DES is a block algoryth - it takes 64-bit blocks of plain text and provide 64-bit encrypted data block for each plain text block. It means the encrypted message is of the same size as plain message that's why shoul encrypted message is short (since plain message is short too).
So you should either enlarge plaing message (e.g. UserID + "SOMEVERYVERYVERYVERYVERYVERYVERYLONGSECRETDATA") but it won't make it more secure, because encrypted blocks in the end will be the same with different UserIDs. The better way is to use another encryption or hash algorythm (like SHA256) which produces encrypted message of longer size then plain text.

Ok, now I use a different class for Encrypting and Decrypting data in another one of my applications. I was able to get some help writing this so I don't fully understand it but enough to get me by. Is there a way I could implement this class to Encrypt the users_id and then Decrypt it?

using System;
using System.IO;
using System.Security.Cryptography;
using System.Configuration;
using System.Text;

/// <summary>
/// Summary description for MyEncryptionUtil
/// </summary>
public class MyEncryptionUtil
{
    // I'm storing the password into your web.config file:
    //<configuration>
    //  <appSettings>
    //    <add key="MyEncryptionUtil.Password" value="MyPassword"/>
    //  </appSettings>
    //</configuration>
    static readonly string Password = ConfigurationManager.AppSettings["MyEncryptionUtil.Password"];

    public static byte[] EncryptObject(object objectToEncrypt)
    {
        // We are using Salt to make it harder to guess our key
        // using a dictionary attack.
        byte[] salt = Encoding.ASCII.GetBytes(Password.Length.ToString());

        // The (Secret Key) will be generated from the specified
        // password and Salt.

        //PasswordDeriveBytes -- It Derives a key from a password
        PasswordDeriveBytes secretKey = new PasswordDeriveBytes(Password, salt);

        // We are now going to create an instance of the
        // Rihndael class. 
        using (RijndaelManaged rijndaelCipher = new RijndaelManaged())
        {
            // Create a encryptor from the existing SecretKey bytes.
            // We use 32 bytes for the secret key
            // (the default Rijndael key length is 256 bit = 32 bytes) and
            // then 16 bytes for the IV (initialization vector),
            // (the default Rijndael IV length is 128 bit = 16 bytes)
            ICryptoTransform encryptor = rijndaelCipher.CreateEncryptor(secretKey.GetBytes(16), secretKey.GetBytes(16));

            // Create a MemoryStream that is going to hold the encrypted bytes
            using (MemoryStream encryptedObject = new MemoryStream())
            {
                // Create a CryptoStream through which we are going to be processing our data.
                // CryptoStreamMode.Write means that we are going to be writing data
                // to the stream and the output will be written in the MemoryStream
                // we have provided. (always use write mode for encryption)
                using (CryptoStream cryptoStream = new CryptoStream(encryptedObject, encryptor, CryptoStreamMode.Write))
                {
                    // Serialize object to the encryption stream.
                    System.Runtime.Serialization.Formatters.Binary.BinaryFormatter bf = new System.Runtime.Serialization.Formatters.Binary.BinaryFormatter();
                    bf.Serialize(cryptoStream, objectToEncrypt);

                    // Finish encrypting.
                    cryptoStream.FlushFinalBlock();
                }

                return encryptedObject.ToArray();
            }
        }
    }

    public static object DecryptObject(byte[] encryptedObjectBytes)
    {
        byte[] salt = Encoding.ASCII.GetBytes(Password.Length.ToString());
        PasswordDeriveBytes secretKey = new PasswordDeriveBytes(Password, salt);

        using (MemoryStream memoryStream = new MemoryStream(encryptedObjectBytes))
        {
            using (RijndaelManaged rijndaelCipher = new RijndaelManaged())
            {
                // Create a decryptor from the existing SecretKey bytes.
                ICryptoTransform decryptor = rijndaelCipher.CreateDecryptor(secretKey.GetBytes(16), secretKey.GetBytes(16));

                // Create a CryptoStream. (always use Read mode for decryption).
                using (CryptoStream cryptoStream = new CryptoStream(memoryStream, decryptor, CryptoStreamMode.Read))
                {
                    System.Runtime.Serialization.Formatters.Binary.BinaryFormatter bf = new System.Runtime.Serialization.Formatters.Binary.BinaryFormatter();
                    return bf.Deserialize(cryptoStream);
                }
            }
        }
    }

    public static string DecryptObjectToString(byte[] encryptedObjectBytes)
    {
        return (string)DecryptObject(encryptedObjectBytes);
    }

    public static int DecryptObjectToInt(byte[] encryptedObjectBytes)
    {
        return (int)DecryptObject(encryptedObjectBytes);
    }

    public static DateTime DecryptObjectToDateTime(byte[] encryptedObjectBytes)
    {
        return (DateTime)DecryptObject(encryptedObjectBytes);
    }

    public static bool DecryptObjectToBoolean(byte[] encryptedObjectBytes)
    {
        return (bool)DecryptObject(encryptedObjectBytes);
    }
}

Select allOpen in new window

Hi ivan_vagunin,

Did you have a chance to look at post 37222638 to see if I could use the "MyEncryptionUtil" Class to Encrypt/Decrypt the users_id value?

Hi!

In MyEncryptionUtil you use Rijndael encryption algorythm wich is in fact enhanced version of DES (it is also called AES), so it is block algorythm too. Though it is more secure then DES, but block algorythms are not what is usually used to encrypt small data blocks (like passwords or id).
I would advice to use Hash alogryth (but remember that hash can't be decrypted).

Maybe this post will give some ideas on implementing hash:
http://www.spsamples.com/2011/06/aspnet-username-token-authentication.html

It describes on of the simplest authentication algorythm in WS-Security.

Hi ivanvagunin,

Ok, but what method should I use for securing the users_id when passing the users_id via email to have them reset their password?

The MyEncryptionUtil class works GREAT, I have some people help me with that to make it more secure and I do understand that Rijndael can be cracked given time and also access to my secret password in my web.config file and I also understand that HASHING can't be decrypted but can easily be cracked with dictionary attacks and rainbow tables. So i'm not sure which method you suggest but either one I'm game with if you could possible help assist me with that.