How to stop value only returning true when selecting boolean column from database

Question

I am building a login system that has two different user types. Within the user field their is a boolean column called user_isAdmin. If true, the user is an admin, if false the user is a default user. Currently I have written a code that logs in but currently returns true for every user. Here is the first attempt:

    private void confirmBtn_Click(object sender, EventArgs e)
    {
        Connect database = new Connect();
        String username = usernameField.Text;
        String password = passwordField.Text;

        DataTable table = new DataTable();
        MySqlDataAdapter adapter = new MySqlDataAdapter();
        MySqlCommand command = new MySqlCommand("SELECT * From tbl_user WHERE `username` = @uname and `user_password` = @pwd", database.getConnection());
        command.Parameters.Add("uname", MySqlDbType.VarChar).Value = username;
        command.Parameters.Add("pwd", MySqlDbType.VarChar).Value = password;

        adapter.SelectCommand = command;
        adapter.Fill(table);


        if (!checkInputFields())
        {
            if (username.ToLower().Trim().Equals("") || password.Trim().Equals(""))
            {
                /*Please Enter Username or Password*/
                MessageBox.Show("Please Enter Username or Password!", "", MessageBoxButtons.OKCancel, MessageBoxIcon.Error);
            }
            else if (table.Rows.Count > 0)
            {
                /*Login Success for Admin*/
                if (!checkIfAdmin()) {
                    MessageBox.Show("Admin Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomeAdmin admin = new HomeAdmin();
                    admin.ShowDialog();
                }
                else
                {
                    /*Login Success for user*/
                    MessageBox.Show("User Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomePlayer user = new HomePlayer();
                    user.ShowDialog();
                }
            }
            else
            {
                /*Login Error No Username*/
                if (username.Trim().Equals(""))
                {
                    MessageBox.Show("Please Enter Your Username", "", MessageBoxButtons.OKCancel, MessageBoxIcon.Error);
                }
                /*Login Error No password*/
                else if (password.Trim().Equals(""))
                {
                    MessageBox.Show("Please Enter Your Password", "", MessageBoxButtons.OKCancel, MessageBoxIcon.Error);
                }
                else
                {
                    /*Login Error Wrong Username or password*/
                        MessageBox.Show("Username or Password is incorrect", "", MessageBoxButtons.OKCancel, MessageBoxIcon.Error);
                }
            }
        }
        else
        {
            MessageBox.Show("Please Enter all your details", "", MessageBoxButtons.OKCancel, MessageBoxIcon.Error);
        }
    }
    /*Check All Fields Have Inputs*/
    public Boolean checkInputFields()
    {
        String username = usernameField.Text;
        String password = passwordField.Text;
        if (username.ToLower().Trim().Equals("username") || password.ToLower().Trim().Equals("password"))
        {
            return true;
        }
        else
        {
            return false;
        }
    }
    public Boolean checkIfAdmin()
    {
        Connect database = new Connect();
        MySqlCommand command = new MySqlCommand("SELECT user_isAdmin From tbl_user WHERE userID = userID", database.getConnection());
        command.Connection.Open();
        bool isAdmin = (bool)command.ExecuteScalar();

        if (isAdmin)
        {
            return true;

        }
        else
        {
            return false;
        }
       
    }

It reaches this section where it calls checkIfAdmin:

if (!checkIfAdmin()) {
                    MessageBox.Show("Admin Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomeAdmin admin = new HomeAdmin();
                    admin.ShowDialog();
                }
                else
                {
                    /*Login Success for user*/
                    MessageBox.Show("User Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomePlayer user = new HomePlayer();
                    user.ShowDialog();
                }

This is checkIfAdmin:

    public Boolean checkIfAdmin()
{
    Connect database = new Connect();
    MySqlCommand command = new MySqlCommand("SELECT user_isAdmin From tbl_user WHERE userID = userID", database.getConnection());
    command.Connection.Open();
    bool isAdmin = (bool)command.ExecuteScalar();

    if (isAdmin)
    {
        return true;
    }
    else
    {
        return false;
    }

}

Currently it only returns true which says that all users are admins, even thought they arent. What I want to know is how I can select the user_isAdmin value based on whether or not they are an admin. If they are an admin, calling the checkIsAdming would run:

                    MessageBox.Show("Admin Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                this.Hide();
                HomeAdmin admin = new HomeAdmin();
                admin.ShowDialog();

Whereas a user would run:

                    MessageBox.Show("User Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                this.Hide();
                HomePlayer user = new HomePlayer();
                user.ShowDialog();

Here is my user table if it helps:

Plain text, not hashed, passwords in your `tbl_user`? Really? Don't do that. Please. Because cybercreeps. Read about password hashing. — O. Jones, Mar 31 '21 at 23:21
The intention isnt for an actual public website, its just a local game for an assessment — de19, Mar 31 '21 at 23:23
Im not trying to make something to complicated, just a simple sign up form for my own user so I can create multiple users — de19, Mar 31 '21 at 23:24
Why do you have `if (!checkIfAdmin()) { //Admin Login Successful! }` when the method returns `true` if the *Admin status* checks out (and you left out a Parameter)? But, why don't you also load that Column while you load the user data? Similar for `checkInputFields()`; kind of difficult to understand what that's for, also based on what comes after. — Jimi, Mar 31 '21 at 23:24
I wouldve assumed for it to work, I would then need to first check if they are an admin or not, then call "if (!checkIfAdmin()) { //Admin Login Successful! }" to say if is admin, do this, else do something else — de19, Mar 31 '21 at 23:29
`"SELECT user_isAdmin From tbl_user WHERE userID = userID"` is not using a variable for `userID`. Shouldn't you first look up the id from the `username` first and then pass that id as a parameter to this select statement? — Rufus L, Mar 31 '21 at 23:33
so like (select userID from tbl_user where username = something) — de19, Mar 31 '21 at 23:43
then "something".Parameters.Add("isAdmin", MySqlDbType.Boolean)... — de19, Mar 31 '21 at 23:45
You are doing`adapter.Fill(table)` but do not use the table. You should use it for all the further logic. If `table. Rows.Count>0` that means user does not exist. Else you check value of `user_isAdmin` column of `table.Rows[0]` to decide if user is admin or not. — Chetan, Apr 01 '21 at 00:25
https://stackoverflow.com/questions/9022118/access-cell-value-of-datatable — Chetan, Apr 01 '21 at 00:27
a users `userID` would be a value in the `userID` column. Would I need to declare it as a variable? — de19, Apr 01 '21 at 02:03
But you checked `WHERE userID = userID` in your sql. How did you get the second `"userID"`. If the field `username` is also unique, why not get the corresponding `user_isAdmin` via `username`? — 大陸北方網友, Apr 01 '21 at 02:15

score 1 · Accepted Answer · answered Apr 01 '21 at 00:50

When you use SELECT user_isAdmin From tbl_user WHERE userID = userID, as others pointed out, you are not selecting a row for a particular user ID, because you're not passing the userID as a parameter.

The command you used is equivalent to SELECT user_isAdmin From tbl_user WHERE 1 = 1. When you execute this command, it will return a list of the user_isAdmin values for all the users in your table.

In order to achieve what you want, you need to get the user_isAdmin value for the unique user. This could be done by passing userID as a parameter to your SQL query, just like you did here: SELECT * From tbl_user WHERE username = @uname and user_password = @pwd.

The CheckIfAdmin method should look like this:

public Boolean CheckIfAdmin(Guid userId) // or whatever is the type of userId
{
    Connect database = new Connect();
    MySqlCommand command = new MySqlCommand("SELECT user_isAdmin From tbl_user WHERE userID = @userID", database.getConnection());
    command.Parameters.Add("userID", MySqlDbType.Guid).Value = userId;        
    command.Connection.Open();
    bool isAdmin = (bool)command.ExecuteScalar();

    if (isAdmin)
    {
        return true;
    }
    else
    {
        return false;
    }

}

This will get the userID for the user in question.

Of course, before calling CheckIfAdmin, you have to fetch the userID from the database. If the username is unique (which I recommend), you can use it instead of the userID.

All of the usernames are unique when you create them in my sign up form. When `table.Rows.Count > 0` is true, it then calls the checkIfAdmin. By default all of the users are set to false. I have manually set an admin to test with. The user Id is `userID`, the user_isAdmin is a boolean value, and the username is a varchar. I could do it based off of the usernames, but I believe that selecting the `userID` would be better. @uodami — de19, Apr 01 '21 at 01:18
do I need to pass the parameters into here `if (!CheckIfAdmin("parameters")) {MessageBox.Show("Admin Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information); this.Hide();HomeAdmin admin = new HomeAdmin();admin.ShowDialog();}` — de19, Apr 01 '21 at 01:21
Yes, you should call `CheckIfAdmin(userId)` or `CheckIfAdmin(username)` depending on whether you will get the user by its ID or username. — uodami, Apr 01 '21 at 01:31
When i do `CheckIfAdmin(userId)`, it throws `the name userID does not exist in the current context` — de19, Apr 01 '21 at 01:46
Thanks for the help. Although this was not the exact fix, it was very close to what I was looking for — de19, Apr 01 '21 at 02:24

score 1 · Answer 2 · answered Apr 01 '21 at 02:23

I found a method that does the job: I passed a parameter called username into CheckIfAdmin

 if (!CheckIfAdmin(username)) {
                    MessageBox.Show("Admin Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomeAdmin admin = new HomeAdmin();
                    admin.ShowDialog();
                }
                else
                {
                    /*Login Success for user*/
                    MessageBox.Show("User Login Successful!", "", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    this.Hide();
                    HomePlayer user = new HomePlayer();
                    user.ShowDialog();
                }

I changed the CheckIfAdmin to this:

        public Boolean CheckIfAdmin(String username) // or whatever is the type of userId
    {
        Connect database = new Connect();
        MySqlCommand command = new MySqlCommand("SELECT user_isAdmin From tbl_user WHERE username = @username", database.getConnection());
        command.Parameters.Add("username", MySqlDbType.VarChar).Value = username;
        command.Connection.Open();
        bool isAdmin = (bool)command.ExecuteScalar();

        if (isAdmin)
        {
            return false;
        }
        else
        {
            return true;
        }

    }

The result of this seems to do what I wanted, so I think it is correct. I swapped the true and false around. If a user logs in, it pops up with the user box and if an admin logs in, it pops up with the admin box. Im not sure if its a good approach, but it works

How to stop value only returning true when selecting boolean column from database

2 Answers2