Login Button to read sql database and confirm values entered into textboxes with values in said sql database throws error

Question

I am a noob attempting to create a login page where the user enters their username and password that is already in the sqldatabase connected to the textboxes/button/form. The below code is my best attempt at doing so, but upon debugging it throws catch despite the textbox values entered being registered in the sql database. If any additional information is needed please ask.

 private bool compareStoD(string teststring1, string teststring2)
    {
        return String.Compare(teststring1, teststring2, true, System.Globalization.CultureInfo.InvariantCulture) == 0 ? true : false;
    }    

 private void button1_Click_1(object sender, EventArgs e)
    {
        try
        {
            SqlConnection connection = new SqlConnection(@"Data Source=DESKTOP-P3JSE1C;Initial Catalog=logins;Integrated Security=True");
            connection.Open();
            SqlCommand checker = new SqlCommand("SELECT COUNT (*) from users WHERE username='" + textBox1.Text + "'AND pssword='" + textBox3.Text + "'", connection);
            SqlDataReader reader = checker.ExecuteReader();
            string usernameText = textBox1.Text;
            string psswordText = textBox3.Text;
            while (reader.Read())
            {
                if (this.compareStoD(reader["username"].ToString(), textBox1.Text) && // replace textbox1.Text with text string usernameText
                    this.compareStoD(reader["pssword"].ToString(), textBox3.Text))   //replace textbox3.Text with text string psswordText
                {
                    main wen = new main();
                    wen.Show();
                }
            }
            reader.Close();
            connection.Close();
        }
        catch
        {
            MessageBox.Show("Incorrect password or username.");
        }

    }

you should not use this approach to get data from database, it is vulnerable to sql injection. you should use stored procedures (Recommended) or `sqlParameter` — Sony, Mar 01 '17 at 03:51
Do yourself a favour and _take note of the exception_. Not only that _put it in the question_. There's a lot of information in an exception. When you write 'throws an exception' it makes it clear that you're not making much effort to troubleshoot — Nick.Mc, Mar 01 '17 at 03:53
Here is also another link about using parameters in SQL Statements [Why do we always prefer using parameters in SQL statements?](http://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements) to assist you with people giving examples closest to your codes. — P. Pat, Mar 01 '17 at 04:32

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

It is most likely throwing an exception because your query is asking for the count but then you are reading columns username and password which do not exist in the reader. This is your query:

SELECT COUNT (*)

Change that to this:

SELECT username, password ...

Also, unless you want every savvy user to access your application, use SqlParameter to avoid SQL Injection

Another Suggestion

I am not sure what main is, my assumption it is some window, but I would not show it where you are showing right now. Try to close the reader as soon as possible and then show the window if the user is authenticated like this.

bool userIsAuthenticated = false;
if (reader.Read())
{
    // if a row was returned, it must be the row for the user you queried
    userIsAuthenticated = true;
}
reader.Close();
connection.Close();

// Now that the reader is closed, you can show the window so the reader does not stay
// open during the duration of the main window
if (userIsAuthenticated)
{
    main wen = new main();
    wen.Show();
}

score 0 · Answer 2 · answered Mar 01 '17 at 03:47

0

Select count returns the count not the row, if you want the row itself change to select username, password instead of select count(*) . See this link

answered Mar 01 '17 at 03:47

Sony

7,136
5
45
68

score 0 · Answer 3 · answered Mar 01 '17 at 04:41

There is over work being done by your code. You are querying the database by comparing the username and password values from UI to the values in the table. And once and if values are retrieved from the database you are again comparing value from UI to the values coming from the database. This is unnecessary.

The query will return the values only if values match in the database so you don't need to compare them again. So method compareStoD is not required at all.

The button1_Click can be changed as following to make it simpler.

private void button1_Click_1(object sender, EventArgs e)
{
    try
    {
        SqlConnection connection = new SqlConnection(@"Data Source=DESKTOP-P3JSE1C;Initial Catalog=logins;Integrated Security=True");
        connection.Open();
        SqlCommand checker = new SqlCommand("SELECT COUNT (*) from users WHERE username=@userName AND pssword = @password", connection);
        checker.Parameters.Add(new SqlParameter("@userName", textBox1.Text));
        checker.Parameters.Add(new SqlParameter("@password", textBox3.Text));
        var count = Convert.ToInt32(checker.ExecuteScalar());
        connection.Close();
        if(count > 0)
        {
           main wen = new main();
           wen.Show();
        }
        else
        {
            MessageBox.Show("Incorrect password or username.");
        }
    }
    catch
    {
        MessageBox.Show("Incorrect password or username.");
    }
}

score 0 · Answer 4 · answered Mar 01 '17 at 04:48

0

Also one good practice while supplying values from Textbox, you should use Textbox.Text.Trim() which helps in eliminating the spaces at the beginning and end. These spaces can create a problem in later stage.

answered Mar 01 '17 at 04:48

Ishwar .Patil

103
9

Login Button to read sql database and confirm values entered into textboxes with values in said sql database throws error

4 Answers4