0

I'm creating a validation for form data coming from database and then comparing it with data entered in textboxes. It always executes else part whether I enter correct or incorrect data in textboxes, please help with this.

c.Uname = Text1.Value.ToString();
c.Cnic  = Text2.Value.ToString();
c.pass = Text3.Value.ToString();

SqlConnection sqlConn = new SqlConnection(@"Data Source=DESKTOP-Q4AAHCG;Initial Catalog=practise;User ID=;Password=;Trusted_Connection=True");

SqlCommand sqlComm = new SqlCommand("select Uname , Cnic, password from carregister", sqlConn);
sqlConn.Open();

SqlDataReader dr = sqlComm.ExecuteReader();

while (dr.Read())
{
    name = dr["Uname"].ToString();
    cnic = dr["Cnic"].ToString();
    passs = dr["password"].ToString();

    if (name.Equals(c.Uname) && cnic.Equals(c.Cnic) && passs.Equals(c.pass))
    {
        Session["Uname"] = Text1.Value.ToString();
        Session["cnic"] = Text2.Value.ToString();

        Response.Redirect("Carloby.aspx");
    }
    else 
    {
        Response.Redirect("wrongidpass.aspx");
    }
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
yaseen
  • 43
  • 4
  • 2
    use the debugger to inspect the runtime values of Uname, Cnic and pass. if you have more than one user in the database, this logic won't work. you want to fetch either only a single record from database which already matches the user name, or move the contents of the else block past the loop (when nothing matched, wrongidpass). – Cee McSharpface Jun 07 '21 at 09:03
  • 2
    ***never*** store passwords as plain text! hash them! also: i recommend imagining what will happen if you have _two rows_ in your database, and you're trying to log in with the credentials of the _second_ row. run through the code in your head, and/or the debugger. – Franz Gleichmann Jun 07 '21 at 09:05
  • you should have `WHERE` clause in query to get the user's details based on the username entered by the user. If the user is found then only you should compare the password. If user is not found or the password does not match you should redirect to workingpass.aspx. – Chetan Jun 07 '21 at 09:43

1 Answers1

1

You are reading ALL rows of your usertable and start comparing with the first received row. If this doesn't match, you are already redirecting ...

You could count only the matching rows from your database, and if that returns anything other than 1, there is an error with username or password (or your database).

c.Uname = Text1.Value.ToString();
c.Cnic  = Text2.Value.ToString();
//you don't store plaintext passwords in your db, do you?
c.pass = hash_the_password(Text3.Value.ToString());  

SqlConnection sqlConn = new SqlConnection(@"Data Source=DESKTOP-Q4AAHCG;Initial Catalog=practise;User ID=;Password=;Trusted_Connection=True");

SqlCommand sqlComm = new SqlCommand("SELECT COUNT(*) FROM carregister WHERE uname = @uname and cnic = @cnic and password = @hashedpassword", sqlConn);
sqlComm.Parameters.Add("@uname", SqlDbType.NVarchar).Value = c.Uname;
sqlComm.Parameters.Add("@cnic", SqlDbType.NVarchar).Value = c.Cnic;
sqlComm.Parameters.Add("@hashedpassword", SqlDbType.NVarchar).Value = c.pass;
sqlConn.Open();

if (Convert.ToInt32(sqlComm.ExecuteScalar()) == 1) {
  //you have exactly one row where uname, cnic and password match the entered values
    Session["Uname"] = Text1.Value.ToString();
    Session["cnic"] = Text2.Value.ToString();

    Response.Redirect("Carloby.aspx");
}
else 
{
    //no row matched 
    //(or more than one which is an error in the database, because uname should probably be unique)
    Response.Redirect("wrongidpass.aspx");
}
derpirscher
  • 14,418
  • 3
  • 18
  • 35
  • yes brother I stored the password in plain text . your answer helped me a lot now it's easier for me to understand what I am doing wrong. thanks a lot, brother. – yaseen Jun 07 '21 at 10:04
  • 1
    Please do not store passwords in plaintext! Not even for your own demo projects ... – derpirscher Jun 07 '21 at 10:05
  • 1
    https://stackoverflow.com/questions/1054022/best-way-to-store-password-in-database – derpirscher Jun 07 '21 at 10:11