-1

I have code here where it will check if the entered username and password are correct, my problem is that in my database I have a Username: "ADMIN" and Password: "ADMIN" but whenever I try to input "admin" for both username and password it still allows me to go to the main window which means my bool was true.

Here is my code:

public bool IsAccountValid(string userLogin, string userPassword)
{
     bool flag = false;
     try
     {
         accessToDatabase.OpeningDatabase();
         String query = "SELECT * FROM Users where Username=@Username AND Password=@Password";
         SqlCommand sqlCmd = accessToDatabase.Command(query);

         sqlCmd.CommandType = CommandType.Text;
         sqlCmd.Parameters.AddWithValue("@Username", userLogin);
         sqlCmd.Parameters.AddWithValue("@Password", userPassword);

         if (sqlCmd.ExecuteScalar() != null)
             flag = true;
         else
             flag = false;
     }
     catch (Exception ex)
     {
         MessageBox.Show(ex.Message);
     }
     finally
     {
         accessToDatabase.ClosingDataBase();
     }

     return flag; //returns false if query does not exists...
 }
user11401
  • 89
  • 9
  • `I have a Username: "ADMIN" and Password: "ADMIN" but whenever I try to input "admin" for both username and password it still allows me to go to the main window`. Ok so why do you thing it should not? – CodingYoshi Feb 17 '18 at 07:52
  • @CodingYoshi I agree it does not make sense for username, but password match should be case sensitive, right ? – rahulaga-msft Feb 17 '18 at 07:54
  • @RahulAgarwal no, not as far as SQL server is concerned. Refere [this](https://stackoverflow.com/questions/1411161/sql-server-check-case-sensitivity) please. – CodingYoshi Feb 17 '18 at 07:58
  • Why use `select *` if you then use `ExecuteScalar`? Use `select count(1)` but this is beyond the point. Create a table with a varchar column and put `ADMIN` in one row and `admin` in another. Then do a select `select * from whatever where colum1 = 'admin'` and you will get 2 rows. – CodingYoshi Feb 17 '18 at 08:01

2 Answers2

3

You can use ExecuteReader on SqlCommand to read values- which you can compare against user provided values. User Name shouldn't be case sensitive though.

My response assumes that username is unique in your user table - which i believe is pretty much valid assumption.

Since you only need username and pwd, you might want to modify your select query to :

SELECT UserName, Password 
FROM Users 
WHERE Username = @Username AND Password = @Password

Probably your revised snippet in that case would be something like :

public bool IsAccountValid(string userLogin, string userPassword)
    {
        bool flag = false;

        try
        {
            accessToDatabase.OpeningDatabase();
            String query = "SELECT * FROM Users where Username=@Username AND Password=@Password";
            SqlCommand sqlCmd = accessToDatabase.Command(query);

            sqlCmd.CommandType = CommandType.Text;
            sqlCmd.Parameters.AddWithValue("@Username", userLogin);
            sqlCmd.Parameters.AddWithValue("@Password", userPassword);

             var reader = sqlCmd.ExecuteReader();
                    if (reader.HasRows)
                    {
                        while (reader.Read())
                        {
                            if(userPassword == reader[0].ToString())
                            {
                                flag = true;
                            }
                        }
                    }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
        finally
        {
            accessToDatabase.ClosingDataBase();
        }

    return flag; //returns false if query does not exists...
}

Other alternative would be modify your sql query itself to ensure it takes care of case sensitive match in where clause like here

In that case, you need not to make any changes to your ADO.NET code.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
rahulaga-msft
  • 3,964
  • 6
  • 26
  • 44
  • There's no point in returning and comparing the password if it's already compared in the statement. Usually it's even frowned upon since then it might linger in memory and possibly be grabbed. Of course it shouldn't be clear text but still. – Sami Kuhmonen Feb 17 '18 at 07:54
  • @SamiKuhmonen _already compared in if statement_ do you mean in sql where clause ? – rahulaga-msft Feb 17 '18 at 07:56
  • I didn't say "in if statement" and yes I meant in the SQL statement. – Sami Kuhmonen Feb 17 '18 at 07:57
  • As i mentioned, alternatively sql query can be modified to ensure where clause takes care of case sensitive search (which is not a case by default) – rahulaga-msft Feb 17 '18 at 07:57
  • thank you! this is easy to understand! the username in my table should be unique but I did not set it as a primary key because I want the username to be editable but no duplicate username, shouldn't username be case sensitive for layer of protection? – user11401 Feb 17 '18 at 09:04
  • @AubreyIvanApungan : If this addresses your question, mark it answered to close it. thanks!! – rahulaga-msft Feb 17 '18 at 09:10
1

Better way would be to change query for case sensitive search:

String query = "SELECT * FROM Users where 
Username=@Username COLLATE SQL_Latin1_General_CP1_CS_AS 
AND Password=@Password COLLATE SQL_Latin1_General_CP1_CS_AS";   

Another method is do binary casting:

String query = "SELECT * FROM Users 
where CAST(Username as varbinary(100))=CAST(@Username as varbinary(100))
 AND CAST(Password as varbinary(100))=CAST(@Password as varbinary(100))";       
FaizanHussainRabbani
  • 3,256
  • 3
  • 26
  • 46