What is done wrong?

Question

C++ / C#

Hello!
Here is the following C# code that is executed when a button is pressed on a form:

private void Vhod_Click(object sender, EventArgs e)
        {
            try
            {
                SHA1 hash = new SHA1();
                LoginTextbox = LoginField.Text;
                HashTextbox = hash.Hash(PasswordField.Text);
                RoleTextbox = RoleField.Text;
                SQLiteConnection conn = new SQLiteConnection("Data source = accounts.db");
                conn.Open();
                SQLiteCommand cmd = new SQLiteCommand($"SELECT login,password,role FROM users WHERE login = '{LoginTextbox}' AND password = '{HashTextbox}' AND role = '{RoleTextbox}'", conn);
                SQLiteDataReader reader = cmd.ExecuteReader();
                while (reader.Read())
                {
                    if (LoginTextbox.Trim() == reader["login"].ToString() && HashTextbox.Trim() == reader["password"].ToString() && RoleTextbox.Trim() == reader["role"].ToString())
                    {
                        if (RoleTextbox == "Администратор")
                        {
                            this.Hide();
                            var MainForm = new MainForm();
                            MainForm.Closed += (s, args) => this.Close();
                            MainForm.Show();
                        }
                        else if (RoleTextbox == "Менеджер")
                        {
                            this.Hide();
                            var MainForm = new MainForm1();
                            MainForm.Closed += (s, args) => this.Close();
                            MainForm.Show();
                        }
                        else if (RoleTextbox == "Бухгалтер")
                        {
                            this.Hide();
                            var MainForm = new MainForm2();
                            MainForm.Closed += (s, args) => this.Close();
                            MainForm.Show();
                        }
                    }
                    else
                    {
                        MessageBox.Show("Неверный логин или пароль! Пожалуйста,повторите попытку. Также возможно, что вы забыли выбрать роль пользователя.", "Ошибка!");
                        LoginField.Text = "";
                        LoginField.Clear();
                        PasswordField.Text = "";
                        PasswordField.Clear();
                        RoleField.Text = "";
                        RoleField.Clear();
                    }
                }
                reader.Close();
                conn.Close();
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

With the help of this code, authorization is carried out
. And here is the problem: for cases when the user entered the wrong login, password or role, this piece was written:

else
                    {
                        MessageBox.Show("Неверный логин или пароль! Пожалуйста,повторите попытку. Также возможно, что вы забыли выбрать роль пользователя.", "Ошибка!");
                        LoginField.Text = "";
                        LoginField.Clear();
                        PasswordField.Text = "";
                        PasswordField.Clear();
                        RoleField.Text = "";
                        RoleField.Clear();
                    }

This piece displays a message and clears the data entry fields, and if you enter the correct data, then authorization will be successful and another form will open, and if the data is entered incorrectly, then the message about incorrectly entered data does not appear, and I cannot understand what is the reason for this .
Thanks in advance

Answer 1

The point is that you placed a check that a user with such parameters does not exist inside the while loop. The query itself will simply not return any data in case of input errors, and execution will not even reach this check.
To avoid such problems, I advise you to move the reading from the database into a separate method.
What can this method return? Just what we want to check is whether such a user exists in our system and what access rights he has.
For convenience, we will add a separate class for storing and using this information in the future. It is better to store the role not in the form of strings, but in the form of enumerations - it is more compact and easier to work with.

public enum Role 
    { 
        Admin, 
        Manager,
        Accountant
    }

    public class User
    {
        public User(string login, Role role)
        {
            Login = login;
            Role = role;
        }

        public string Login { get; }

        public Role Role { get; }
    }

So, the new method will return an instance of the User class if the parameters are entered correctly and null if there is no such user.
In your code, the user chooses the role himself, which seems a little strange, in my opinion, it is better to omit this memory check.

public static User GetUserOrNull(string login, string password)
{
    using (var connection = new SqliteConnection(connectionString))
    {
        connection.Open();
        using (var command = connection.CreateCommand())
        {
            var query =
                @"SELECT role FROM users 
                    WHERE login = @login AND password = @password";

            command.CommandText = query;
            command.Parameters.AddWithValue("login", login);
            command.Parameters.AddWithValue("password", password);

            using (var reader = command.ExecuteReader())
            {
                while (reader.Read())
                {
                    // GetOrdinal на тот случай если будем возвращать другие значения
                    var role = reader.GetInt32(reader.GetOrdinal("role"));

                    return new User(login, (Role)role);
                }
            }

            return null;
        }
    }
}

I deliberately omit error and exception handling, it's up to you.
Also, pay attention when compiling a SQL query, values ​​are not inserted through string interpolation, but parameters are used.
Now the processing will be much simpler and cleaner - after all, the request itself will be located in a different place.

private void Vhod_Click(object sender, EventArgs e)
{
    try
    {
        SHA1 hash = new SHA1();
        var login = LoginField.Text;
        var pHash = hash.Hash(PasswordField.Text);
        var user = DataBase.GetUserOrNull(login, pHash);
        if (user != null)
        {
            this.Hide();
            var MainForm = new MainForm();
            MainForm.Closed += (s, args) => this.Close();
            MainForm.Show();
        }
        else
        {
            MessageBox.Show("Неверный логин или пароль! Пожалуйста,повторите попытку.", "Ошибка!");
            LoginField.Text = "";
            LoginField.Clear();
            PasswordField.Text = "";
            PasswordField.Clear();
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}