C# Login Form, Login Button , security

Question

So, I am new to VS and C#, I am self-teaching to get a better understanding of the back-end of the product I work with. I have created a small database with some information and a Login form. Everything appears to compile correctly but is that the security way to do that or there is another way, any help is appreciated. Thanks.

 private void button2_Click(object sender, EventArgs e)
{

    SqlCommand cmd = new SqlCommand("select * from tbladmin where username=@username and password=@password", sqlcon);
    cmd.Parameters.AddWithValue("@username", txtusername.Text);
    cmd.Parameters.AddWithValue("@password", txtpassword.Text);
    SqlDataAdapter sda = new SqlDataAdapter(cmd);
    DataTable dtbl = new DataTable();
    sda.Fill(dtbl);


    try
    {

        if (dtbl.Rows.Count > 0)
        {
            if (dtbl.Rows[0]["role"].ToString() == "Admin")
            {
                SqlCommand cmd2 = new SqlCommand("select date from tbladmin where username=@username and password=@password", sqlcon);
                cmd2.Parameters.AddWithValue("@username", txtusername.Text);
                cmd2.Parameters.AddWithValue("@password", txtpassword.Text);
                SqlDataAdapter sda2 = new SqlDataAdapter(cmd2);
                DataTable dss = new DataTable();
                sda2.Fill(dss);
                String value2 = dss.Rows[0][0].ToString();
                DateTime date = DateTime.Parse(dss.Rows[0][0].ToString());
                Class1.Txtusername = txtusername.Text;
                Debug.WriteLine("value is :   " + value2);

                if (date.AddDays(90) < DateTime.Now)
                {
                    Changpassad obj2 = new Changpassad();
                    this.Hide();
                    obj2.Show();
                }
                else
                {

                    calladmin obj = new calladmin(dss.Rows[0][0].ToString());
                    this.Hide();
                    obj.Show();
                }
            }
        }
        else if (dtbl.Rows.Count == 0)
        {
            SqlCommand cmd3 = new SqlCommand("select date from tblcallcenter where username=@username and password=@password", sqlcon);
            cmd3.Parameters.AddWithValue("@username", txtusername.Text);
            cmd3.Parameters.AddWithValue("@password", txtpassword.Text);
            SqlDataAdapter sda2 = new SqlDataAdapter(cmd3);
            DataTable dss = new DataTable();
            sda2.Fill(dss);
            String value2 = dss.Rows[0][0].ToString();
            DateTime date = DateTime.Parse(dss.Rows[0][0].ToString());

            Debug.WriteLine("value is :   " + value2);

            if (date.AddDays(90) < DateTime.Now)
            {
                Changpass obj2 = new Changpass()/;
                this.Hide();
                obj2.Show();
            }
            else
            {
                SqlCommand cmd4 = new SqlCommand("select user_id , username from tblcallcenter where username=@username and password=@password", sqlcon);
                cmd4.Parameters.AddWithValue("@username", txtusername.Text);
                cmd4.Parameters.AddWithValue("@password", txtpassword.Text);
                SqlDataAdapter From_sda = new SqlDataAdapter(cmd4);
                DataTable From_ds = new DataTable();
                From_sda.Fill(From_ds);
                String value1 = From_ds.Rows[0][1].ToString();
                int id = int.Parse(From_ds.Rows[0][0].ToString());

                Debug.WriteLine("value is :   " + value1);
                Class1.Txtusername = txtusername.Text;
                this.Hide();
                SqlCommand cmd5 = new SqlCommand("select  [from], Take from tblcallcenter where username=@username and password=@password", sqlcon);
                cmd5.Parameters.AddWithValue("@username", txtusername.Text);
                cmd5.Parameters.AddWithValue("@password", txtpassword.Text);
                SqlDataAdapter sda1 = new SqlDataAdapter(cmd5);
                DataTable ds = new DataTable();
                sda1.Fill(ds);
                Callcenter1 obj = new Callcenter1(ds.Rows[0][0].ToString(), ds.Rows[0][1].ToString());
                this.Hide();
                obj.Show();
            }
        }

        else
        {
            MessageBox.Show("Invalid Login try checking Useraname Or Password !", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
    }
    catch (Exception)
    {
        MessageBox.Show("Invalid Login try checking Useraname Or Password !", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

Answer 1

The code looks like it is coming from Windows Forms, I can't be certain, it could be WPF.

The code does not seem to be in a MVVM or MVC design pattern. Either of these design patterns would allow data abstraction from the user interface.

The code might be less complex and easier to maintain if there was some separation between the UI (view) and the data (Model). The code might also be less complex and easier to maintain if it was broken up into smaller functions that did only one thing (Single Responsibility Principle).

All SQL Queries can throw errors

The first SQL query in the file is not in a try/catch block, if the SQL query fails the program will fail.

The error message in the catch block "Invalid Login try checking Username Or Password !" doesn't represent what actually happened. If this catch block executes it means that the database that is being queried threw the exception and that there is an error either in the SQL code or the table doesn't exist in the database.

Use Using Blocks

A using statement provides additional error checking as well as cleaning up after the SQL call. There are some examples of using statements in this stackoverflow question.

You can wrap connections and SQL commands in using statements. For an SQL connection wrapped in a using statement the database the connection will be closed automatically. An SQL command will be properly disposed of it is wrapped in a using statement.

    private DataRow GetRawSeriesData(uint seriesId)
    {
        DataRow rawData = null;

        if (seriesId > 0)
        {
            try
            {
                using (MySqlConnection conn = new MySqlConnection(_dbConnectionString))
                {
                    string queryString = "SELECT * FROM series WHERE idSeries = '" + seriesId.ToString() + "';";
                    int ResultCount = 0;
                    DataTable Dt = new DataTable();
                    conn.Open();
                    using (MySqlCommand cmd = new MySqlCommand())
                    {
                        cmd.Connection = conn;
                        cmd.CommandType = CommandType.Text;
                        cmd.CommandText = queryString;

                        MySqlDataAdapter sda = new MySqlDataAdapter(cmd);
                        ResultCount = sda.Fill(Dt);
                        if (ResultCount > 0)
                        {
                            rawData = Dt.Rows[0];
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                string errorMsg = "Database Error: " + ex.Message;
                MessageBox.Show(errorMsg);
            }
        }

        return rawData;
    }

User Input Error Checking

There doesn't seem to be any error checking on the username or password, in some cases this can allow SQL Injection attacks.

On more secure websites the password might be checked against a set of rules to make it more secure, such as a requirement for 2 lowercase characters, 2 uppercase characters, 2 integers, and special characters, as well as a minimum length.

There might also be a count of the number of times the button was pushed to prevent a bot attack.