Login page code enhancement

I am starting to learn ASP.NET, and I am designing a page for an admin to login and do some stuff. I have the username and password for the admin stored in Web.config (Is that a good practice?).

Second thing, I am checking if the user filled the username and password fields, then I am checking if the values for the admin and password exist in the web.config, then I am checking if these values equal each other, so I am having 3 nested if. Is that a good practice as well?

Here’s my code:

if (!string.IsNullOrEmpty(txtLoginId.Text) && !string.IsNullOrEmpty(txtPassword.Text))
            {
                string configLoginID = WebConfigurationManager.AppSettings["AdminLoginID"];
                string configPassword = WebConfigurationManager.AppSettings["AdminPassword"];
                if (!string.IsNullOrEmpty(configLoginID) && !string.IsNullOrEmpty(configPassword))
                {
                    if (txtLoginId.Text == configLoginID && txtPassword.Text == configPassword)
                    {
                        Session["ShoppingAdmin"] = "ShoppingAdmin";
                        Response.Redirect("~/Admin/AddNewProducts.aspx");
                    }
                    else
                    {
                        lblAlert.Text = "Incorrect Username/Password";
                    }

                }
            }
            else
            {
                lblAlert.Text = "Please enter a usernamd and password";
            }

Last thing, checking empty fields using !string.IsNullOrEmpty for each field, is that the right thing to do?

I would like to write a clean code, thanks for helping me with these 3 cases.

I have the username and password for the admin stored in Web.config (Is that a good practice?).

No, it’s not good practice to store the Admin login (or any login, for that matter) in the Web.config.

Depending on what ASP.NET model you’re using, you have a few options. MVC has a built-in Roles provider which makes it simple to make certain pages/areas only accessible by a specific role.

Web Forms is also pretty simple, though you have to roll your own access provisions. You can use the built-in Simple Membership if you like, which usually suits quite well once extended to fit your needs. (You can add properties to users pretty easily and such.)

You also aren’t encrypting or hashing the password, which is a huge no-no. At the very least you should MD5 hash it, though for security I would recommend SHA1-salt hashing at the very weakest. Though, if you use the built-in Simple Membership for Web Forms or MVC, you’ll get encryption and salting by default.

I didn’t review the actual code as Heslacher did that just fine, but I do think you need to completely revisit the login system. ASP.NET has built-in providers for this, and they’re super easy and, quite honestly, pretty fun to set up. (I don’t know why no one has pointed this out in the several months this question has been here, or how it got on my radar, but hopefully you still get this advice.)

Using a guard condtion by switching the conditions of the outer if condition you can return early and its more obvious what is hapening like so

if (string.IsNullOrEmpty(txtLoginId.Text) || string.IsNullOrEmpty(txtPassword.Text))
{ 
    lblAlert.Text = "Please enter a username and password";
    return;
}  

The same can be done for the next if condition like so

string configLoginID = WebConfigurationManager.AppSettings["AdminLoginID"];
string configPassword = WebConfigurationManager.AppSettings["AdminPassword"];
if (string.IsNullOrEmpty(configLoginID) || string.IsNullOrEmpty(configPassword))
{
    return;
}  

leaving the remaining if..else like so

if (txtLoginId.Text == configLoginID && txtPassword.Text == configPassword)
{
    Session["ShoppingAdmin"] = "ShoppingAdmin";
    Response.Redirect("~/Admin/AddNewProducts.aspx");
}
else
{
    lblAlert.Text = "Incorrect Username/Password";
}  

Doing is this way will reduce the nesting hence reduces the horizontal spacing which makes the code more readable.