Jeffry Vergara
Jeffry Vergara

Reputation: 11

SQL Server and SqlDataReader

I am trying to create a login page together with a SQL Server database but when I am trying to use the SqlDataReader, I get an error

System.Data.SqlClient.SqlException: 'Incorrect syntax near ','

I've attached my code. Thanks a lot in advance

namespace LoginAndRegistration
{
    public partial class frmLogIn : Form
    {
        public frmLogIn()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            SqlConnection con = new SqlConnection("Data Source=CONSOLE-03;Initial Catalog=db_users;Integrated Security=True");

            SqlDataAdapter adapter = new SqlDataAdapter();
            con.Open();

            SqlCommand cmd = new SqlCommand("SELECT * FROM tbl_users WHERE Username =('" + txtUsername.Text + "','" + txtPassword.Text + "')",con) ;

            SqlDataReader dr = cmd.ExecuteReader();

            if (dr.Read() == true)
            {
                new Dashboard().Show();
                this.Hide();
            }
            else
            {
                MessageBox.Show("Invalid Username or Password, Please try again", "Login Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
                txtUsername.Text = "";
                txtPassword.Text = "";
                txtUsername.Focus();
            }
            
            con.Close();
        }

        private void button2_Click(object sender, EventArgs e)
        {
            txtUsername.Text = "";
            txtPassword.Text = "";
            txtUsername.Focus();
        }

        private void checkbxShowPas_CheckedChanged(object sender, EventArgs e)
        {
            if (checkbxShowPas.Checked)
            {
                txtPassword.PasswordChar = '\0';
            }
            else
            {
                txtPassword.PasswordChar = '•';
            }
        }

        private void label6_Click(object sender, EventArgs e)
        {
            new frmRegistration().Show();
            this.Hide();
        }
    }
}

Upvotes: 1

Views: 111

Answers (1)

Charlieface
Charlieface

Reputation: 71144

There are a lot of issues with your code:

  • Concatting for no reason, it seems you just want to check the columns separately.
  • SQL injection. You should use parameters instead.
  • Returning the whole row instead of just returning a 1 to confirm existence.
  • You don't need a SqlDataAdapter, and if you only have one row you also don't need SqlDataReader as you can use cmd.ExecuteScalar().
  • Plain-text passwords: always salt-and-hash your passwords.
  • Store your connection string in a settings file, not hard-coded.
  • Missing using to dispose all the objects
private void button1_Click(object sender, EventArgs e)
{
    bool isCorrect;
    try
    {
        isCorrect = LoginUser(txtUsername.Text, txtPassword.Text);
    }
    catch(Exception ex)
    {
        MessageBox.Show(ex.Message, "Login Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

    if (isCorrect)
    {
        new Dashboard().Show();
        this.Hide();
    }
    else
    {
        MessageBox.Show("Invalid Username or Password, Please try again", "Login Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
        txtUsername.Text = "";
        txtPassword.Text = "";
        txtUsername.Focus();
    }
}

private bool LoginUser(string username, string password)
{
    const string query = @"
SELECT 1
FROM tbl_users
WHERE Username = @username
  AND PasswordHash = @passwordHash;
";
    using (SqlConnection con = new SqlConnection(YourConnString))
    using (SqlCommand cmd = new SqlCommand(query))
    {
        cmd.Parameters.Add("@username", SqlDbType.NVarChar, 100).Value = username;
        cmd.Parameters.Add("@passwordHash", SqlDbType.VarBinary, 128).Value = SaltAndHash(password, username);
        con.Open();
        return cmd.ExecuteScalar() == 1;
    }
}

You should also consider using async to avoid blocking the UI

private async void button1_Click(object sender, EventArgs e)
{
    bool isCorrect;
    try
    {
        isCorrect = await LoginUser(txtUsername.Text, txtPassword.Text);
    }
    catch(Exception ex)
    {
        MessageBox.Show(ex.Message, "Login Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

    if (isCorrect)
    {
        new Dashboard().Show();
        this.Hide();
    }
    else
    {
        MessageBox.Show("Invalid Username or Password, Please try again", "Login Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
        txtUsername.Text = "";
        txtPassword.Text = "";
        txtUsername.Focus();
    }
}

private async Task<bool> LoginUser(string username, string password)
{
    const string query = @"
SELECT 1
FROM tbl_users
WHERE Username = @username
  AND PasswordHash = @passwordHash;
";
    using (SqlConnection con = new SqlConnection(YourConnString))
    using (SqlCommand cmd = new SqlCommand(query))
    {
        cmd.Parameters.Add("@username", SqlDbType.NVarChar, 100).Value = username;
        cmd.Parameters.Add("@passwordHash", SqlDbType.VarBinary, 128).Value = SaltAndHash(password, username);
        await con.OpenAsync();
        return (await cmd.ExecuteScalar()) == 1;
    }
}

Upvotes: 3

Related Questions