2

I am writing some server-side validation in asp.net for login page.

Now, I am coming from a "write it from scratch" PHP perspective and i am learning and struggling with some of these asp.net concepts which i'm unaware of.

I am trying to set a username and password variable to "valid" if the input is valid, and i am trying to use these variables to proceed with login.

I'm also not sure if this is the correct way to do things.

protected void loginbutton_Click(object sender, EventArgs e)
    {
        string UsernameRegex = "[a-zA-Z]+";
        string PasswordRegex = "[a-zA-Z0-9]+";

        if (!Regex.IsMatch(usernametextbox.Text, UsernameRegex))
        {
            string UsernameCheck = "valid";
        }
        else
        {
            string UsernameCheck = "invalid";
        }

        if (!Regex.IsMatch(passwordtextbox.Text, PasswordRegex))
        {
            string PasswordCheck = "valid";
        }
        else
        {
            string PasswordCheck = "invalid";
        }


        if(UsernameCheck = "valid") //i will include password here after i solved the problem
        {
            //do something
        }
            SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnection"].ConnectionString);
            conn.Open();
            string checkuser = "select count(*) from Users where Username = @username and Password = @password";

            SqlCommand com = new SqlCommand(checkuser, conn);
            com.Parameters.Add("@username", SqlDbType.NVarChar).Value = usernametextbox.Text;
            com.Parameters.Add("@password", SqlDbType.NVarChar).Value = passwordtextbox.Text;

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

            if (temp > 0)
            {
                Response.Redirect("Cars.aspx");
            }
            else
            {
                loginfaillabel.Text = "Your Username or Password doesn't match our records";
            }
        }

Help and feedback is appreciated.

Brandon
  • 68,708
  • 30
  • 194
  • 223
  • Here's how you'd do it in [Web Forms](https://msdn.microsoft.com/en-us/library/a0z2h4sw.aspx). Hth. – EdSF Mar 03 '17 at 17:11
  • 2
    Possible duplicate of [Variable scope confusion in C#](http://stackoverflow.com/questions/1196941/variable-scope-confusion-in-c-sharp) – Brandon Mar 03 '17 at 17:12
  • 1
    Also `==` in `if(UsernameCheck = "valid")`...etc. – EdSF Mar 03 '17 at 17:14
  • I have written client-side validation and i want to include server-side validation as well – Reece Costello Mar 03 '17 at 17:16
  • in which line this error occurred? – Saif Mar 03 '17 at 17:21
  • While you can do things using the answers provided here, imho, you should leverage the built-in validation afforded by WebForms (see the link above) that provide both client and server side validation. If none of them meet your needs, then that's the time you think about going some other way. Hth... – EdSF Mar 03 '17 at 17:51
  • I can't tell if the asp.net validation controls are client, server or both validation, i implemented the Validators in asp.net but im not sure if this is server/client or both. – Reece Costello Mar 03 '17 at 17:55
  • Both - trivial example below. Hth... – EdSF Mar 03 '17 at 18:53

4 Answers4

3

Ok, lots of feedback here.

  1. Use booleans, not strings! I replaced them for you. The main issue here was what had scope. You could declare the variable outside the if and solve the issue but better to use a boolean and just get rid of the if block all together as it becomes more readable.
  2. Always wrap your Ado.Net types that implements IDisposable in using blocks. This way if the code encounters an Exception your connection is still closed (a good thing)
  3. No need to do count in your sql statement, just return 1. If there is a user you get a result otherwise not.
  4. Never store passwords in clear text! I did not touch this, that is up to you. There are many proper password hashing algorithms to choose from like pbkdf2, bcrypt, and scrypt to name a few of the more generally accepted secure algorithms.
  5. Are you sure user name is Unicode? If not change the parameter type to VarChar in your SqlParameter's type.

Modified code

protected void loginbutton_Click(object sender, EventArgs e)
{
    string UsernameRegex = "[a-zA-Z]+";
    string PasswordRegex = "[a-zA-Z0-9]+";

    boolean isUsernameValid = Regex.IsMatch(usernametextbox.Text, UsernameRegex)
    boolean isPasswordValid = Regex.IsMatch(passwordtextbox.Text, PasswordRegex);


    if(!isUsernameValid || !isPasswordValid) //i will include password here after i solved the problem
    {
        //do something
    }
    else
    {
        const string checkuser = "SELECT 1 FROM Users WHERE Username = @username and Password = @password";

        using(SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnection"].ConnectionString))
        using(SqlCommand com = new SqlCommand(checkuser, conn))
        {
            conn.Open();

            com.Parameters.Add("@username", SqlDbType.NVarChar).Value = usernametextbox.Text;
            com.Parameters.Add("@password", SqlDbType.NVarChar).Value = passwordtextbox.Text;

            object temp = com.ExecuteScalar();

            // I do not remember if it is null or System.DbNull.Value that is returned if nothing is returned
            // you will have to test it
            var didUserMatch = temp == null || temp == System.DbNull.Value ? false : true;

            if (didUserMatch)
            {
                Response.Redirect("Cars.aspx");
            }
            else
            {
                loginfaillabel.Text = "Your Username or Password doesn't match our records";
            }
        }
    }
}
Graham
  • 7,431
  • 18
  • 59
  • 84
Igor
  • 60,821
  • 10
  • 100
  • 175
2

I see 3 problems with your code:

  1. You are using Strings for you variable *Check, you should use booleans.

  2. In the line if(UsernameCheck = "valid") you are actually assigning the value "valid" to UsernameCheck, if you want to test for equality, use if(UsernameCheck == "valid")

  3. The problem that you actually have is due to the variables scope. You are declaring the variable UsernameCheck and PasswordCheck inside the if/else statements, this means they only exist inside the if/else, when the code execution exists the if/else, the variable ceases to exist, try this code (and please read a bit more about C#):

protected void loginbutton_Click(object sender, EventArgs e)
{
    string UsernameRegex = "[a-zA-Z]+";
    string PasswordRegex = "[a-zA-Z0-9]+";

    bool UsernameCheck = false; // better name for this is isUsernameValie

    if (!Regex.IsMatch(usernametextbox.Text, UsernameRegex)) {
        UsernameCheck = true;
    } else {
        UsernameCheck = false;
    }

    bool PasswordCheck = false;// better name for this is isPasswordValid
    if (!Regex.IsMatch(passwordtextbox.Text, PasswordRegex)) {
                 PasswordCheck = true;
    } else {
        PasswordCheck = false;
    }


    if (UsernameCheck == true) //i will include password here after i solved the problem
    {
        //do something
    }
    SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnection"].ConnectionString);
    conn.Open();
    string checkuser = "select count(*) from Users where Username = @username and Password = @password";

    SqlCommand com = new SqlCommand(checkuser, conn);
    com.Parameters.Add("@username", SqlDbType.NVarChar).Value = usernametextbox.Text;
    com.Parameters.Add("@password", SqlDbType.NVarChar).Value = passwordtextbox.Text;

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

    if (temp > 0) {
        Response.Redirect("Cars.aspx");
    } else {
        loginfaillabel.Text = "Your Username or Password doesn't match our records";
    }
}
berserck
  • 499
  • 4
  • 19
0

you need check variable and method scops.

Code needs a little editing

    protected void loginbutton_Click(object sender, EventArgs e)
    {
        string UsernameRegex = "[a-zA-Z]+";
        string PasswordRegex = "[a-zA-Z0-9]+";

        var userName = usernametextbox.Text;
        var password = passwordtextbox.Text;

        if (!Regex.IsMatch(userName, UsernameRegex))
        {
            // do something
            return; // There is no need to go on
        }

        if(!Regex.IsMatch(password, PasswordRegex))
        {
            // do something
            return; // There is no need to go on
        }

        //If we can come here, we can go DB

        // To be dispose when the job is done
        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["DefaultConnection"].ConnectionString))
        {

            try
            {
                // To be dispose when the job is done
                using (SqlCommand com = new SqlCommand(checkuser, conn))
                {
                    conn.Open();
                    string checkuser = "select count(*) from Users where Username = @username and Password = @password";
                    com.Parameters.Add("@username", SqlDbType.NVarChar).Value = userName;
                    com.Parameters.Add("@password", SqlDbType.NVarChar).Value = password;
                    int temp = Convert.ToInt32(com.ExecuteScalar().ToString());
                    if (temp > 0)
                    {
                        Response.Redirect("Cars.aspx");
                    }
                    else
                    {
                        loginfaillabel.Text = "Your Username or Password doesn't match our records";
                    }
                }
            }
            catch (Exception ex)
            {

                // you can handle error. maybe logs
            }
        }
    }
levent
  • 3,464
  • 1
  • 12
  • 22
0

While you can do things based on the other answers, IMHO, leverage the built in Web Forms Validation first. If it falls short, then do something else.

Trivial Example:

  • foo.aspx

    <p>Username (Alphabetic only, no spaces):<br />
        <asp:TextBox ID="TextBox1" runat="server"></asp:TextBox>
        <asp:RequiredFieldValidator ID="RequiredFieldValidator1" runat="server" ControlToValidate="TextBox1" Display="Dynamic" ErrorMessage="Username is required"></asp:RequiredFieldValidator>
        <asp:RegularExpressionValidator ID="NameValidator" runat="server" ControlToValidate="TextBox1" Display="Dynamic" ErrorMessage="Invalid - Alaphabetic only" ValidationExpression="[a-zA-Z]+" EnableClientScript="True"></asp:RegularExpressionValidator>
    </p>
    <p>Password (Alphanumeric only, no spaces):<br />
        <asp:TextBox ID="TextBox2" runat="server"></asp:TextBox>
        <asp:RequiredFieldValidator ID="RequiredFieldValidator2" runat="server" ControlToValidate="TextBox2" Display="Dynamic" ErrorMessage="Password is required"></asp:RequiredFieldValidator>
        <asp:RegularExpressionValidator ID="PwdValidator" runat="server" ControlToValidate="TextBox2" Display="Dynamic" ErrorMessage="Invalid -Alphanumeric Only" ValidationExpression="[\w]+" EnableClientScript="True"></asp:RegularExpressionValidator>
    </p>
    <p>
        <asp:Button ID="Button1" runat="server" OnClick="BtnSubmit" Text="Login" />
    </p>
    

    EnableClientScript is True by default. You can set it to False to test things or see what happens without client side validation (see server side validation in action).

  • foo.aspx.cs (aka "code behind")

    public partial class foo: Page
    {
        protected void Page_Load(object sender, EventArgs e)
        {
    
        }
    
        protected void BtnSubmit(object sender, EventArgs e)
        {
            if (Page.IsValid)
            {
                //Do what you need to do only if IsValid which is the server-side validation check
            }
        }
    }
    
EdSF
  • 11,753
  • 6
  • 42
  • 83