Username, Password and UserType Validation

if (dr.GetValue(0).ToString() == _str)
{
   dr.Close();
   cmdOne = new SqlCommand("select customername from customer where userid='" + objLogOnUserInformation.UserId + "'", con);
   dr = cmdOne.ExecuteReader();
   dr.Read();
   objLogOnUserInformation.SessionUserName = dr.GetValue(0).ToString();
   dr.Close();
   //if usertype is customer, U is returned
   return _str;
}
else
{
   dr.Close();
   cmdOne = new SqlCommand("select customername from customer where userid='" + objLogOnUserInformation.UserId + "'", con);
   dr = cmdOne.ExecuteReader();
   dr.Read();
   objLogOnUserInformation.SessionUserName = dr.GetValue(0).ToString();
   dr.Close();
   //If usertype is Admin, A is returned
   return "A";
}

I'll just start with all the things I see in this code.

Naming:

dr, con, objLogOnUserInformation, cmdOne

All these names are bad for various reasons.

dr (and con) is waay too short. You can allow yourself to name your variables after what they do and not some cryptic unpronounciable names. --> use dataReader (and connection) instead

objLogOnUserInformation is bad because ad 1 it's not clear, and ad 2 it uses hungarian notation. Never use hungarian notation, instead:

--> use: loggingInUserInfo

cmdOne doesn't tell you what it is and it is numbered! If you have no better idea than numbering equal variables, you are probably doing something wrong. I'd use sqlCommand

User-type:

Your user type is a String. THIS IS DANGEROUS and also quite prone to bugs. C# has a nice "feature" called enums. It looks like your UserType is one:

public enum UserType 
{
    CUSTOMER, ADMIN, //maybe others
}

And now the coolest thing happens.. Your if-else is useless. you do exactly the same either way you return the UserType. why not remove the if-statement completely (as well as the outer data reader and merge it with your "select usertype" statement:

dataReader.close(); 
// if you use dataReader with a using statement early on, you can leave that out
sqlCommand = new SqlCommand("select customername, usertype from customer where userId='" + loggingInUserInfo.UserId + "'", connection);
using (dataReader = sqlCommand.ExecuteReader())
{
    dataReader.read();

    loggingInUserInformation.SessionUserName = dataReader.Item["customername"];
    return dataReader.Item["usertype"];
}

To make the code efficient you should select only that data that you need.

Your code:

cmd = new SqlCommand("select UserID  from Credentials", con);

This selects

1. ids of all users but you need only the one that will match to objLogOnUserInformation!
2. userid but userid is already known!

Must be

cmd = new SqlCommand("select UserID  from Credentials WHERE UserID=@UserID", con);
cmd.Parameters.AddWithValue("@UserID", objLogOnUserInformation.UserId);

This will select only the one which has required id and will not require to perform a loop.

Your code

cmd = new SqlCommand("select Password  from Credentials where UserId='" + objLogOnUserInformation.UserId + "'", con);

Same thing as above. You should select only required user and not all users.

Combine the query which we already have with a new condition

cmd = new SqlCommand(@"select UserID  from Credentials WHERE UserID=@UserID
              AND Password=@Password", con);
cmd.Parameters.AddWithValue("@UserID", objLogOnUserInformation.UserId);
cmd.Parameters.AddWithValue("@Password", objLogOnUserInformation.Password);

Do the same change for all further conditions. At the end you should have only one SELECT-statement that should select data in one shot.

Well, you can start by removing obvious copy-paste:

dr.Close();
cmd = new SqlCommand("select Password  from Credentials where UserId='" + objLogOnUserInformation.UserId + "'", con);
dr = cmd.ExecuteReader();

and

dr.Close();
cmdOne = new SqlCommand("select customername from customer where userid='" + objLogOnUserInformation.UserId + "'", con);
dr = cmdOne.ExecuteReader();
dr.Read();
objLogOnUserInformation.SessionUserName = dr.GetValue(0).ToString();
dr.Close();

In addition to the current answers, I would suggest you to not reuse the dr object like that. Use one DataReader for each query that you need and scope it correctly with a using statement. You may not be able to nest the while statements that deeply if your SqlConnection does not allow MultipleActiveResultSets, but that may be a good thing. By reusing the dr object the way you did, it is hard to see which da data is available and how long the resource is being used.

colleagues have pointed out all the major mistakes. i want to stress the deadly mistake of storing the plain-text password in the DB. check this out as a starter. This as an advanced implementation.

in any case the way you build your code is prone to side channel attacks (especially timing attack). In the web, you may find solutions to password hash comparison timing attacks but none towards username timing attacks. In many contexts such as records/document management users are horrible in password decisions. They have words from their usernames in their passwords. 2FA authentication is not an option. I have seen examples like username = [email protected] password = sarah1 when pwd is limited to 6 digit alpha-numeric. You would get Sarah123 in case of 8 digit uppercase+lowercase+numeric constraint. Therefore, timing attack against username and then dictionary/birthday attack on password works just fine on such contexts. Hacking valid usernames (which are mostly real mail addresses) is often overlooked. In your case + you have password hashing if the userId matches, your code exits much earlier if userId is invalid. Remember password hashing algorithms should be slow enough to fight brute attacks. This opens the back door to the timing attack. The attacker checking the response time difference can easily pick the valid usernames from invalid ones. In fact, the attacker would get most if not all of your usernames in one attack episode.