Visual C# Programming a Login form connected to a Access Db that gives only tries to log into

Question

I am developing a login form in C#. This form connects to the database to match the username and password and also to find any duplicate. What I was trying to do is to implement a loop to accept only three tries then it will close. The code is:

namespace RoyalCollegeApp
{
    public partial class Login : Form
    {
        public Login()
        {
            InitializeComponent();
        }

        private void Login_Load(object sender, EventArgs e)
        {

        }

        private void btn_login_Click(object sender, EventArgs e)
        {
            if (string.IsNullOrEmpty(txt_user.Text))
            {
                MessageBox.Show("Please type your Username");
                txt_user.Focus();
                return;
            }

            if (string.IsNullOrEmpty(txt_pass.Text))
            {
                MessageBox.Show("Please type your Password");
                txt_pass.Focus();
                return;
            }

            try
            {
                string constring = @"Provider=Microsoft.ACE.OLEDB.12.0;
                Data Source=C:\...\Auth_Credentials.accdb;";
                OleDbConnection conDataBase = new OleDbConnection(constring);
                OleDbCommand cmdDataBase = new OleDbCommand("Select * from Auth_Credentials where
                    Username='"
                    + this.txt_user.Text
                    + "' and Password='"
                    + this.txt_pass.Text
                    + "';", conDataBase);
                OleDbDataReader myReader;

                conDataBase.Open();
                myReader = cmdDataBase.ExecuteReader();

                int count = 0;

                while (myReader.Read())
                {
                    count = count + 1;
                }
                if (count == 1)
                {
                    MessageBox.Show("Login Successful");
                    this.Hide();
                    RCM RCM = new RCM();
                    RCM.Show();
                    this.Hide();
                    RCM.FormClosing += RCM_Closing;
                }
                else if (count > 1)
                {
                    MessageBox.Show("Duplicate Username or Password");
                }
                else
                {
                    MessageBox.Show("Username or Password do not match");
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

        private void RCM_Closing(object sender, FormClosingEventArgs e)
        {
            Application.Exit();
        }
    }
}

I have tried many solutions but I am still groping in the dark. Any suggestions will be appreciated, cheers.

Answer 1

There are a few helpfull answers, but I prefered to have one correct example. Your code has a few problems

• It's easy to hack your database using SQL Injection
• There are some issues with IDisposable, which can create memory leaks.
• The code is very tight coupled to the frontend, which is almost always the case in winforms. Nevertheless I prefer to limit the calls to the UI Components.

The code I've is the following: (it has inline comments)

private int _failedLoginCounter = 0;

private void btnLogin_Click(object sender, EventArgs e)
{
    var username = txtUsername.Text;
    var password = txtPassword.Text;

    if (string.IsNullOrEmpty(username))
    {
        MessageBox.Show("Please type your Username");
        txtUsername.Focus();
        return;
    }

    if (string.IsNullOrEmpty(password))
    {
        MessageBox.Show("Please type your Password");
        txtPassword.Focus();
        return;
    }

    // Seperate the login check and make it lously coupled from the UI (= do not refer to the UI elements, instead pass the values to a method)
    CheckLogin(username, password);
}

private void CheckLogin(string username, string password)
{
    try
    {
        string constring = @"Provider=Microsoft.ACE.OLEDB.12.0; Data Source=C:\...\Auth_Credentials.accdb;";

        // You need to use a using statement since OleDbConnection implements IDisposable
        // more inf: http://msdn.microsoft.com/en-us/library/system.data.oledb.oledbconnection(v=vs.110).aspx
        using (OleDbConnection conDataBase = new OleDbConnection(constring))
        {
            // You need to use a using statement since OleDbCommand implements IDisposable
            // more info: http://msdn.microsoft.com/en-us/library/system.data.oledb.oledbcommand(v=vs.110).aspx
            using (OleDbCommand cmdDataBase = conDataBase.CreateCommand())
            {
                cmdDataBase.CommandText =
                    "SELECT * FROM Auth_Credentials WHERE Username=@username AND Password = @password";

                cmdDataBase.Parameters.AddRange(new OleDbParameter[]
                {
                    new OleDbParameter("@username", username),
                    new OleDbParameter("@password", password)
                });

                // Open database if not open
                if (conDataBase.State != ConnectionState.Open)
                    conDataBase.Open();

                var numberOrResults = 0;

                // You need to use a using statement since OleDbDataReader inherits DbDataReader which implements IDisposable
                // more info: http://msdn.microsoft.com/en-us/library/system.data.common.dbdatareader(v=vs.110).aspx
                using (OleDbDataReader myReader = cmdDataBase.ExecuteReader())
                {
                    while (myReader != null && myReader.Read())
                    {
                        numberOrResults++;
                    }
                }

                // If only one result was returned by the database => Succesful login
                if (numberOrResults == 1)
                {
                    MessageBox.Show("Login Successful");
                    this.Hide();
                }

                // If more than 1 result was returned by the database => Failed login
                // This is not a good idea, this situation should never occor.
                // Always make sure username + pass (or whatever you use for authentication) is unique.
                else if (numberOrResults > 1)
                {
                    MessageBox.Show("Duplicate Username or Password");
                    // increment the failed login counter
                    _failedLoginCounter++;
                }
                // No match was found in te database => Failed login
                else if (numberOrResults == 0)
                {
                    MessageBox.Show("Username or Password do not match");
                    // increment the failed login counter
                    _failedLoginCounter++;
                }
            }

        }

        // If the user has 3 failed login attempts on a row => close.
        if (_failedLoginCounter >= 3)
            this.Close();
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

For your initial question I've finished the answer of Selman22, basicly I've used a private field which keeps track of the number of failed tries. Each time the user tries to login, we check if its the 3th time. If so we close the form.

This still isn't the best approach imho, but I didnt want to change your context ;-)

I've also removed the SQL Injection possibilities by adding Parameters to the query.

To work with IDisposable implementations, you have to Dispose the object correctly. This is done in the finally block of the try/catch statement (or by a USING statement, like I did).

I hope this is helpfull, if not freel free to comment.