Ado.net C# login check Ms Sql using a windows form application

Question

Hi i am tring to develope a login screen. In a windows form aplication. This is what i got:

   private void button1_Click(object sender, EventArgs e)
   {
       if ((textBox1.Text == "") || (textBox2.Text == ""))
       {
           MessageBox.Show("Bu alanları boş bırakamazsınız.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Error);
           return;
       }

       SqlConnection conn = new SqlConnection();
       conn.ConnectionString = "Server=CAGDAS-LAPTOP;Database=chat;Trusted_Connection=true;";
       conn.Open();


       DataSet ds = new DataSet();
       SqlDataAdapter sda = new SqlDataAdapter("select * from Kullanici WHERE kul_adi='" + textBox1.Text + "' AND sifre='" + textBox2.Text + "'", conn);
       sda.Fill(ds);

       if (ds.Tables.Count == 0)
       {
           MessageBox.Show("Geçersiz Kullanıcı.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Error);

       }

       else if(ds.Tables.Count == 1)
       {
               MessageBox.Show("Hoşgeldiniz.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Information);
       }

by the way i am using Ms sql and logged as windows authentication. I am not sure did i write my connection string right. But when i run the program doesnt matter what i write, unless if i fill the both textbox i get "hosgeldiniz"(welcome) message. What is the problem? What am i doing wrong? For your information: kullanici is user geçersiz kullanici means wrong login info hosgeldiniz means welcome in my language.

[Might want to browse this](http://en.wikipedia.org/wiki/SQL_injection) — TheNorthWes, May 17 '14 at 17:20

score 2 · Accepted Answer · edited May 23 '17 at 12:32

Whatever your user types the correct password and account or not, your code returns always a datatable inside the dataset; with no rows, if the login is incorrect, with one row. if the login is correct. So, testing if there is a table or not will always return that there is at least one table.

You should check if there are DataRows in the table returned

   DataSet ds = new DataSet();
   SqlDataAdapter sda = new SqlDataAdapter("select * from Kullanici WHERE ....");
   sda.Fill(ds);

   if (ds.Tables[0].Rows.Count == 0)
   {
       MessageBox.Show("Geçersiz Kullanıcı.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Error);

   }
   else if(ds.Tables[0].Rows.Count == 1)
   {
       MessageBox.Show("Hoşgeldiniz.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Information);
   }

Said that, there a better ways to check the correctness of the input and also to avoid a very big problem called Sql Injection

private void button1_Click(object sender, EventArgs e)
{
   if ((textBox1.Text == "") || (textBox2.Text == ""))
   {
       MessageBox.Show("Bu alanları boş bırakamazsınız.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Error);
       return;
   }

   string cmdText = @"select COUNT(*) from Kullanici 
                      WHERE kul_adi=@user AND sifre=@pwd";
   using(SqlConnection conn = new SqlConnection("Server=CAGDAS-LAPTOP;Database=chat;Trusted_Connection=true;"))
   using(SqlCommand cmd = new SqlCommand(cmdText, conn))
   {
       conn.Open();
       cmd.Parameters.AddWithValue("@user", textBox1.Text);
       cmd.Parameters.AddWithValue("@pwd", textBox2.Text);
       int userCount = Convert.ToInt32(cmd.ExecuteScalar());
       if (userCount == 0)
       {
           MessageBox.Show("Geçersiz Kullanıcı.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Error);

       }
       else if(userCount == 1)
       {
           MessageBox.Show("Hoşgeldiniz.", "Chat Giriş", MessageBoxButtons.OK, MessageBoxIcon.Information);
       }
   }
}

In this way, there is no need to have a dataset, just count the records that match the passed login informations.

Notice that I have also placed the opening of the connection and the building of the command inside a using statement block that will ensure the proper closing and disposing of the connection and the command also in case of exceptions.

A final note. It is a very bad idea to store a password in clear text in your database. For better security consider using an hashing algorithm that transform the password in something unreadable and not decryptable. Store the hashed text instead of the password in clear text and, when you need to check the password. reapply the same hashing algorithm to your user input and check the resulting text with the stored text.

Ado.net C# login check Ms Sql using a windows form application

1 Answers1