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.

Answer 2

Define a counter to store user's attempts:

public partial class Login : Form
{
    int counter = 0;
    ...
}

Then increment it each time user type invalid password or username:

else if (count > 1)
{
      MessageBox.Show("Duplicate Username or Password");
      counter++;
}
else
{
     MessageBox.Show("Username or Password do not match");
     counter++;
}

Then before you check whether or not the user exists, first check the counter and perform appropriate action:

private void btn_login_Click(object sender, EventArgs e)
{
       if(counter == 3) this.Close();
       ...
}

Answer 3

Your current problem is quite simple, you need to maintain the number of login attempts outside of the btn_login_click method. Create a private field outside of the method and initialize it to 0, increment it in the click handler and stop when it's X. But I'm not sure that is sufficient--typically, you want something to not only limit the number of attempts sequentially, but to limit the number of attempts in total and/or for a specific username.

Also your current code is subject to SQL injection and is poorly structured (the validation and query code should not be in the click event handler).

NOTE: Your code is storing the password directly in the database, this is a BIG "No-NO". You always store just a hash of the password, never the password itself. Even if you need the set the password automatically and then send it to the user -- there's really no reason to store the password directly instead of storing a hash of the password.