3

I'm trying to make a login facility for Windows Forms Application project. I'm using Visual Studio 2010 and MS Sql Server 2008.

I referenced this article: http://www.codeproject.com/Articles/4416/Beginners-guide-to-accessing-SQL-Server-through-C

Here is my database table named user: enter image description here

I have TextBox1 for user name , TextBox2 for user password and Button1 for starting login process. Here is my code for Button1_Click method:

private void button1_Click(object sender, EventArgs e)
{
    string kullaniciAdi; // user name
    string sifre; // password

    SqlConnection myConn = new SqlConnection();
    myConn.ConnectionString = "Data Source=localhost; database=EKS; uid=sa; pwd=123; connection lifetime=20; connection timeout=25; packet size=1024;";
    myConn.Open();
    try 
    {
        SqlDataReader myReader;
        string myQuery = ("select u_password from user where u_name='" + textBox1.Text + "';");
        SqlCommand myCommand = new SqlCommand(myQuery,myConn);
        myReader = myCommand.ExecuteReader();
        while (myReader.Read())
        {
            sifre = myReader["u_password"].ToString();
        }
    }
    catch (Exception x) 
    {
        MessageBox.Show(x.ToString());
    }
    myConn.Close();
}

I don't have much experience with C# but i think i'm missing something small to do it right. Below i share exception message that i catched. Can you show me what i'm missing? (line 33 is myReader = myCommand.ExecuteReader();)

enter image description here

Considerin given answers, i updated my try block as in below but it still does not work.

try
{
    SqlDataReader myReader;
    string myQuery = ("select u_password from [user] where u_name=@user");
    SqlCommand myCommand = new SqlCommand(myQuery, myConn);
    myCommand.Parameters.AddWithValue("@user", textBox1.Text);
    myReader = myCommand.ExecuteReader();
    while (myReader.Read())
    {
        sifre = myReader["u_password"].ToString();
    }

    if (textBox2.Text.Equals(sifre))
    {
        Form2 admnPnl = new Form2();
        admnPnl.Show();
    }
}

After changing whole code as below by sine's suggestion, screenshot is also below: And i think, somehow i cannot assign password in database to the string sifre.

code:

string sifre = "";
var builder = new SqlConnectionStringBuilder();
builder.DataSource = "localhost";
builder.InitialCatalog = "EKS";
builder.UserID = "sa";
builder.Password = "123";

using (var conn = new SqlConnection(builder.ToString()))
{
    using (var cmd = new SqlCommand())
    {
        cmd.Connection = conn;
        cmd.CommandText = "select u_password from [user] where u_name = @u_name";
        cmd.Parameters.AddWithValue("@u_name", textBox1.Text);
        conn.Open();

        using (var reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                var tmp = reader["u_password"];
                if (tmp != DBNull.Value)
                {
                    sifre = reader["u_password"].ToString();
                }
            }

            if (textBox2.Text.Equals(sifre))
            {
                try
                {
                    AdminPanel admnPnl = new AdminPanel();
                    admnPnl.Show();
                }
                catch (Exception y)
                {
                    MessageBox.Show(y.ToString());
                }
            }
            else
            {
                MessageBox.Show("incorrect password!");
            }
        }
    }
}

enter image description here

Marc
  • 3,905
  • 4
  • 21
  • 37
t1w
  • 1,408
  • 5
  • 25
  • 55
  • Hi, I attached it as image. It's in the end of my question – t1w Sep 16 '13 at 13:54
  • try changing `FROM user` to `FROM EKS.dbo.user` – makciook Sep 16 '13 at 13:55
  • 3
    you should use Parameters for Queries!!! – makim Sep 16 '13 at 13:56
  • 1
    Not related to your problem at all but you need to be careful of somthing called [SQL Injection](http://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work). Try typing this in your user login box: `'; drop database EKS; --` – Scott Chamberlain Sep 16 '13 at 13:57
  • @makciook it's still giving same error message – t1w Sep 16 '13 at 13:59
  • @sine hi, i'm afraid i didn't understand what you meant. – t1w Sep 16 '13 at 14:00
  • 2
    @TimurAykutYıldırım Look at Soner Gonul's answer, he changed your query to use parameters. If you don't do that people can run code on your SQL server (like deleting your database like my example did in my last comment). By using parameters like `@user` they can no longer do that. – Scott Chamberlain Sep 16 '13 at 14:01
  • @SonerGönül Sadly it's not solved. I changed my try block, it's at the end of my post. – t1w Sep 16 '13 at 16:10

5 Answers5

4

User is a reserved keyword in T-SQL. You should use it with square brackets like [User].

And you should use parameterized sql instead. This kind of string concatenations are open for SQL Injection attacks.

string myQuery = "select u_password from [user] where u_name=@user";
SqlCommand myCommand = new SqlCommand(myQuery,myConn);
myCommand.Parameters.AddWithValue("@user", textBox1.Text);

As a general recomendation, don't use reserved keywords for your identifiers and object names in your database.

Soner Gönül
  • 97,193
  • 102
  • 206
  • 364
2

Try to put user into [ ] because it is a reseved Keyword in T-SQL and use Parameters, your code is open to SQL-Injection!

private void button1_Click(object sender, EventArgs e)
{
    var builder = new SqlConnectionStringBuilder();
    builder.DataSource = "servername";
    builder.InitialCatalog = "databasename";
    builder.UserID = "username";
    builder.Password = "yourpassword";

    using(var conn = new SqlConnection(builder.ToString()))
    {
        using(var cmd = new SqlCommand())
        {
            cmd.Connection = conn;
            cmd.CommandText = "select u_password from [user] where u_name = @u_name";
            cmd.Parameters.AddWithValue("@u_name", textBox1.Text);
            conn.Open();

            using(var reader = cmd.ExecuteReader())
            {
                 while (reader.Read())
                 {
                     var tmp = reader["u_password"];
                     if(tmp != DBNull.Value)
                     {
                         sifre = reader["u_password"].ToString();
                     }
                 }
            }
        }
    }
}
makim
  • 3,154
  • 4
  • 30
  • 49
  • i revised my code based on your recommendation but it's not working. can you check latest state of my try block? it's in the end of my post. I cannot find what i'm missing – t1w Sep 16 '13 at 16:17
  • Are you still getting the same Exception or a differnt one? – makim Sep 16 '13 at 16:25
  • 1
    though I do not know whats causing the Problem, I´ve updated my answer try this code and if it doesn´t work, update your question with the Exception(if it´s not the same as before) – makim Sep 16 '13 at 16:35
  • var tmp = myReader["u_password"]; in the while loop gives this error: Use of unassigned local variable 'myReader' – t1w Sep 16 '13 at 18:39
  • sorry for that, my bad forgot to change the name! It should be reader[..] updated my answer! – makim Sep 16 '13 at 18:49
  • i think last change didn't make any difference. still gives the same error as previous. i updated question, you can check error message from there. btw, appreciated for help :) – t1w Sep 16 '13 at 18:59
  • blame on me...didn´t run the code and forgot to assign the connection to the SqlCommand updated my answer! Try it again ;-) – makim Sep 16 '13 at 19:24
  • and if you´ve got it running maybe you should summarize you´re Question ;-) – makim Sep 16 '13 at 19:27
  • updated my question with screenshot. i think, somehow i cannot assign password in database to the string sifre. i'm starting to get confused :s – t1w Sep 16 '13 at 19:52
  • check the chat ;) http://chat.stackoverflow.com/rooms/37476/discussion-between-sine-and-timur-aykut-yildirim – t1w Sep 18 '13 at 14:33
  • check the chat too ;) – makim Sep 30 '13 at 12:01
  • i checked chat, check chat :D – t1w Sep 30 '13 at 19:57
1

User is a reserved keyword in SQL, you need to do this:

select u_password from [user]  where u_name=@user

And as ever, with basic SQL questions, you should always use parameterised queries to prevent people from running any old commands on your DB via a textbox.

SqlCommand myCommand = new SqlCommand(myQuery,myConn);
myCommand.Parameters.AddWithValue("@user", textBox1.Text);
Colm Prunty
  • 1,595
  • 1
  • 11
  • 29
1

USER is a reserved word in T-SQL

Try putting [] around reserved words.

string myQuery = ("select u_password from [user] where u_name='" + textBox1.Text + "';");
Xpanse
  • 136
  • 8
1

user is a keyword.

Change it to something like

string myQuery = ("select u_password from [user] where u_name='" + textBox1.Text + "';");

Futher to that I recomend you have a look at Using Parameterized queries to prevent SQL Injection Attacks in SQL Server

Adriaan Stander
  • 162,879
  • 31
  • 289
  • 284