Getting trouble in Login Page 3-Tire architecture

Question

Bussiness Access Layer :

    public static int login(string userlogin, string pwdlogin)
    {
        SqlConnection con = new SqlConnection();
        con.ConnectionString = GetConnectionString();
        con.Open();
        int id = 0;
        string selectstr = "SELECT UserName, Password FROM Registration WHERE UserName = '" + userlogin.Trim() + "' AND Password = '" + pwdlogin.Trim() + "'";
        SqlCommand cmd = new SqlCommand();
        cmd.CommandText = selectstr;
        cmd.CommandType = System.Data.CommandType.Text;
        cmd.Connection = con;
        id = cmd.ExecuteNonQuery();
        cmd = null;
        con.Close();
        return id;
    }

Login.cs

 protected void Button1_Click(object sender, EventArgs e)
    {
        int id = BusinessAccessLayer.login(userlogin.Text.Trim(), pwdlogin.Text.Trim());
        if (id > 0)
        {
            message.Text = " valid";
        }
        else
        {
            message.Text = "in valid";
        }   
    }

Answer 1

The ExecuteNonQuery is used for For UPDATE, INSERT, and DELETE statements. For SELECT statements, use ExecuteReader

public static int login(string userlogin, string pwdlogin)
{
        SqlConnection con = new SqlConnection();
        con.ConnectionString = GetConnectionString();
        con.Open();
        int id = 0;
        string selectstr = "SELECT UserName, Password FROM Registration WHERE UserName = '" + userlogin.Trim() + "' AND Password = '" + pwdlogin.Trim() + "'";
        SqlCommand cmd = new SqlCommand();
        cmd.CommandText = selectstr;
        cmd.CommandType = System.Data.CommandType.Text;
        cmd.Connection = con;
        SqlDataReader reader = cmd.ExecuteReader();
        while (reader.Read())
        {
               id++; 
        }
        cmd = null;
        reader.Close();
        con.Close();
        return id;
}

Answer 2

Okay, there are multiple things wrong here:

1) You should use using statements to make sure you close your connection and command even if exceptions are thrown

2) You should use parameterized SQL instead of putting the values directly into your SQL statement, to avoid SQL Injection Attacks

3) You appear to be storing passwords in plain text. Don't do that. Use a salted hash or something similar (ideally something slow to compute).

4) You're ignoring .NET naming conventions; methods should be in PascalCase

5) Your SQL never looks at any field which appears to be related to the user ID. It's not clear what you expect ExecuteNonQuery to return, but if you want the actual ID, you'll need to refer to it in the SQL. (Even if initially you just want to know whether or not the user's password is valid, I strongly suspect that at some point you'll want to user the real user ID, so you should make your code return it. If you really only want to know whether or not the password is valid, you should change the method's return type to bool.)

6) You're using ExecuteNonQuery when your command clearly is a query. Either use ExecuteReader or ExecuteScalar instead. (ExecuteNonQuery is meant for insert, delete and update statements, and it returns you the number of rows affected by the command.)

So something like:

public static int Login(string user, string password)
{
    using (var conn = new SqlConnection(GetConnectionString()))
    {
        conn.Open();
        string sql = "select Id, PasswordHash from logins where Username=@Username";
        using (var command = new SqlCommand(sql))
        {
            command.Parameters.Add("@Username", SqlDbType.NVarChar).Value = user;

            using (var reader = command.ExecuteRead())
            {
                if (reader.Read())
                {
                    int id = reader.GetInt32(0);
                    string hash = reader.GetString(1);
                    // TODO: Hash provided password with the same salt and compare
                    // results
                    if (CheckPassword(password, hash))
                    {
                        return id;
                    }
                }
                return 0; // Or use an int? return type and return null
            }
        }
    }
}

Answer 3

You can't use .ExecuteNonQuery if you want a result. Use .ExecuteReader.

public static int login(string userlogin, string pwdlogin)
{
    SqlConnection con = new SqlConnection();
    con.ConnectionString = GetConnectionString();
    con.Open();
    int id = 0;
    string selectstr = "SELECT UserId FROM Registration WHERE UserName = '" +    userlogin.Trim() + "' AND Password = '" + pwdlogin.Trim() + "'";
    SqlCommand cmd = new SqlCommand();
    cmd.CommandText = selectstr;
    cmd.CommandType = System.Data.CommandType.Text;
    cmd.Connection = con;

    SqlDataReader reader = cmd.ExecuteReader();
    reader.Read();
    id = reader.GetInt32("UserId");

    reader.Close();
    con.Close();

    return id;
}