Catching Button Tampering By a User C#

Question

Login Error

As you can see, I want to catch the exception if the user is tampering the Login Button if there are no values in the fields or if it doesn't match info in the database.

For example: The field has no values and I click Login button once, it says the error. After I clicked OK button, I click Login button again and now it says, "ExecuteReader requires an open and available Connection. The connection's current state is closed."

I use 3 tier Architecture Windows Application.

BEL:

    public SqlDataReader Login(BELLogin bellog)
    {
        SqlCommand cmd = new SqlCommand();
        cmd.Connection = Con.getcon();
        cmd.CommandType = CommandType.Text;
        cmd.CommandText = "SELECT username,password FROM tbl_login WHERE username = @Username AND password = @Password";
        cmd.Parameters.AddWithValue("@Username", bellog.Acctname);
        cmd.Parameters.AddWithValue("@Password", bellog.Password);
        SqlDataReader dr = cmd.ExecuteReader();
        return dr;
    }

BAL:

public class BELLogin
{
    public string Acctname { get; set; }
    public string Password { get; set; }
}

DBConnection:

public SqlConnection getcon()
    {
        if (con.State == System.Data.ConnectionState.Closed)
            con.Open();
        else if (con.State == System.Data.ConnectionState.Open)
            con.Close();
        return con;
    }

    public DataTable ExeReader(SqlCommand cmd)
    {
        getcon();
        cmd.Connection = getcon();
        SqlDataReader dr = cmd.ExecuteReader();
        DataTable dt = new DataTable();
        dt.Load(dr);
        return dt;
    }

GUI:

private void btn_login_Click(object sender, EventArgs e)
    {
        BELog.Acctname = txb_accName.Text;
        BELog.Password = txb_password.Text;

        SqlDataReader dr;
        dr = BALog.Login(BELog);

        if (txb_accName.Text == "" || txb_password.Text == "")
        {
            MessageBox.Show("Some fields are empty. Please fill up all fields before clicking LOGIN button.", "Login Status", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
        else
        {
            if (dr.HasRows == true)
            {
                dr.Read();
                Inventory Inv = new Inventory();
                Inv.Show();
                this.Hide();
            }
            else
            {
                MessageBox.Show("You have entered your password or account name incorrectly. Please check your password and account name and try again.", "Login Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }
        dr.Close();
}

Logging in is ok but what if the user tampering the button? Thank you for helping me :D

Answer 1

Just get rid of your DBConnection object, it's not really doing anything and just making your structure complex:

public BELLogin Login(BELLogin bellog)
{
    SqlConnection conn = new SqlConnection(connectionsString);
    try
    { 
        using (SqlCommand cmd = new SqlCommand())
        {
           conn.Open();
           cmd.Connection = conn;
           cmd.CommandType = CommandType.Text;
           cmd.CommandText = "SELECT username,password FROM tbl_login WHERE username = @Username AND password = @Password";
           cmd.Parameters.AddWithValue("@Username", bellog.Acctname);
           cmd.Parameters.AddWithValue("@Password", bellog.Password);
           //really this should be in a using as well. 
           //You be better off reading your data 
           //into a class and returnig the class not the reader.
           using (SqlDataReader dr = cmd.ExecuteReader())
           {
               BELLogin obj = new BELLogin();
               while(dr.Read())
               {
                    //populate obj
               }
               return obj;
           }
       }
   }
   finally
   {
       conn.Close();
       conn.Dispose();
   }
}

also the way you use it can cause memory leaks as your not explicitly disposing and closing your connections. Always dispose Sql objects in C#. Be wary of exceptions too. Any exceptions in your code will not close the connection, etc. This will result in memory leaks and connections locking