Log in page says user does not exist when they do

Question

I have a log in page that checks with a database for current registered users. At the moment my code will tell me me the user does not exist, when in fact they are on the database, or the page will remain the same if they aren't, when the text corresponding to the problem should come up.

So the name 'Tom' is already on the database, when i type 'Tom' i get the message "user does not exist".

protected void Page_Load(object sender, EventArgs e)
{

}

protected void SqlDataSource1_Selecting(object sender, SqlDataSourceSelectingEventArgs e)
{

}

protected void Button_LogIn_Click(object sender, EventArgs e)
{
    SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings[@"\\MAC\HOME\DESKTOP\NIMV1.MDFConnectionString"].ConnectionString);
    conn.Open();
    string checkuser = "select count(*) from [Table] where UserName= '" + TextBoxLogIn.Text + "'";
    SqlCommand com = new SqlCommand(checkuser, conn);
    int temp = Convert.ToInt32(com.ExecuteScalar().ToString());
    conn.Close();
    if (temp == 1)
    {
       conn.Open();
        if (checkuser == TextBoxLogIn.Text)
        {
            Session["New"] = TextBoxLogIn.Text;
            Response.Write("User name is correct");
        }
        else
        { 

        Response.Write("user does not exist");
    }
        conn.Close();
    }
}

`checkuser == TextBoxLogIn.Text` – `checkuser` is your SQL query. It’s unlikely that the user entered that SQL query as its username… — poke, Nov 21 '15 at 13:46
You should really look into how you write safe SQL queries with parameters. Doing string concatenation will only make you get SQL injections. — poke, Nov 21 '15 at 13:50

score 1 · Answer 1 · answered Nov 21 '15 at 13:48

1

You're comparing your SQL string against the user name that has been passed in.

   string checkuser = "select count(*) from [Table] where UserName= '" + TextBoxLogIn.Text + "'";

    //...

    if (checkuser == TextBoxLogIn.Text)
    {
        Session["New"] = TextBoxLogIn.Text;
        Response.Write("User name is correct");
    } else { 
        Response.Write("user does not exist");
    }

I assume this will always evaluate to false unless the user has an SQL query for a name :D

[EDIT] Actually even if their name was an SQL query the code would never get to that point because you wouldn't find their name in the database in the first place.

answered Nov 21 '15 at 13:48

Zac Braddy

611
5
12

Since the SQL query is “some text + entered user name + some text” it’s actually impossible to enter a user name that matches the whole query. – poke Nov 21 '15 at 13:50
Yeah, I must have made the edit somewhere between making the original post and you making the comment. You're right. – Zac Braddy Nov 21 '15 at 13:51
Thanks, how can i compare it to the usernames in the database rather than the sql string? – user3050340 Nov 21 '15 at 14:01
As soon as your query comes back with a result from your count select statement then you know the Username matches an entry in the database I would have thought that would be enough validation of the username to make the problem test superfluous to requirements. If you really need to get the name from the database then you'd have to make another select statement that returns that result. I would have thought at that point though you would not use the count select statement and just count the rows returned in memory to save having to make two separate SQL calls for essentially the same data. – Zac Braddy Nov 21 '15 at 14:11

score 0 · Answer 2 · answered Nov 22 '15 at 00:11

You are comparing records using user name in database with entered text.

string checkuser = "select count(1) from [Table] where UserName= '" + 
                           TextBoxLogIn.Text + "'";

After executing query instead of:

int temp = Convert.ToInt32(com.ExecuteScalar().ToString());
conn.Close();

if (temp == 1)
{
    conn.Open();
    if (checkuser == TextBoxLogIn.Text)
    ///
    ///

Use following code:

if (temp >= 1)//if there is any user we will get temp >= 1
{
    //conn.Open(); //No need to open connection again

    //checkuser is SQL query so will never be equal to 
    //what user enters (like joe@example.com)
    //if (checkuser == TextBoxLogIn.Text)
    //{
    Session["New"] = TextBoxLogIn.Text;
    Response.Write("User name is correct");
    //}
else //if no user with given name, temp will be 0
{
    Response.Write("user does not exist");
    //}
    //conn.Close();
}

score 0 · Answer 3 · edited May 23 '17 at 12:15

Peope already told you about the error you've made about comparing the variable that holds the query agains the value you typed (doh!!) but i want to DISCOURAGE you against writing the queries the way you do: it isn't a good practice to write query the way you've done because you are vulnerable to SQL Injection or at least nasty bugs if you don't escape correctly your variables.

I invite you to read about how to use what the .Net framework gives to you: use Parameters!

here you can find another answer with code that you can use or at least understand: https://stackoverflow.com/a/11905249/1716620

Log in page says user does not exist when they do

3 Answers3