Custom Login ASP.NET C#

Question

I'm currently making a custom login in ASP.NET. I've modified the code of the Login Control to use my database instead of the Aspnet table. Here's a sample of my code;

using System;
using System.Data;
using System.Configuration;
using System.Web;
using System.Web.Security;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
using System.Web.UI.HtmlControls;
using System.Data.SqlClient;


public partial class Login : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {

    }

    // Custom login control
    protected void Login1_Authenticate(object sender, AuthenticateEventArgs e)
    {
        try
        {
            string uname = Login1.UserName.Trim();
            string password = Login1.Password.Trim();

            bool flag = AuthenticateUser(uname, password);
            if (flag == true)
            {
                e.Authenticated = true;
                Login1.DestinationPageUrl = "Default.aspx";
            }
            else
                e.Authenticated = false;
        }
        catch (Exception)
        {
            e.Authenticated = false;
        }
    }

    private bool AuthenticateUser(string uname, string password)
    {
        bool bflag = false;
        string connString = "Server=DEVSERVER;User ID=sa;Password=whatpassword;Database=CommonUser";
string connstring2 = "Server=DEVSERVER;User ID=sa;Password=whatpassword;Database=Admins";
        string strSQL = "Select * from dbo.Users where Username ='" + uname + "' and Password ='" + password + "'";
        DataSet userDS = new DataSet();
        SqlConnection m_conn;
        SqlDataAdapter m_dataAdapter;
        SqlCommand m_Command;
        try
        {
            m_conn = new SqlConnection(connString);
            m_conn.Open();
            m_dataAdapter = new SqlDataAdapter(strSQL, m_conn);
            m_dataAdapter.Fill(userDS);
            m_conn.Close();
        }
        catch (Exception)
        {
            userDS = null;
        }

        if (userDS != null)
        {
            if (userDS.Tables[0].Rows.Count > 0)
                bflag = true;
        }
        return bflag;

    }
}

I have another database for the Admin users. So my question is how can I make it check the database for the admin users. Also how can I restrict common users from certain pages like ~Admin/AdminPages.aspx? I'm currently trying to figure This.

Any help would be much appreciated ;)

Thanks in advance

Answer 1

Ok, so I am going to say this, but know that I mean it in the nicest possible way...

You are doing it wrong!

I'm not arguing against the use of a custom database although Asp.Net already has this built in. I'm not even arguing against hand coding this in a method when you could be using the very nice pluggable provider model that Asp.Net has built in. What I am arguing against is how wide open this code is to a Sql Injection attack.

Consider for a second what would happen if I typed in x'; DROP TABLE Users; -- as the username? BAD THINGS MAN!!!!

Ok, so seriously follow the links I put in there and please please please at the very least use parameterized queries!

Answer 2

There are quite a few things wrong with the code you posted.

string strSQL = "Select * from dbo.Users where Username ='" + uname + "' and Password ='" + password + "'";

Never EVER EVER do string concatenation to build a query. This leaves your application open to SQL Injection. Use a parametrized query instead.

Why do you have a separate database for Admins and Common Users? Wouldn't you store all the logins in a single table in a single database. Then either use a single field "IsAdmin" or use a separate Roles table and UsersInRoles table to determine which users are Admin or not.

What is your reason for not using the builtin Membership Provider? You configure the builtin provider to use any database, not just the AppData\aspnet.mdf.

You typically restrict different pages to different users using Roles, this can be set in the web.config file inside the authorization element.

If you REALLY want to create a custom simple authentication system, use something like http://csharpdotnetfreak.blogspot.com/2009/02/formsauthentication-ticket-roles-aspnet.html to manually assign the user roles to the user identity.

Answer 3

I really think you shouldn't hardcode database connection parameters at all. Is a bad practice because if the DB changes, you'll have to recompile. So, what you must do is implement a custom membership and role provider. See this article. Basically you need to create a custom class that inherits from System.Web.Security.RoleProvider and System.Web.Security.MembershipProvider. Membership manages users and roles, well.. user permissions.

After it is all set, you can check user permissions via Page.User property on your aspx page code-behind file.

Answer 4

To add my voice to the mix. Those who do not learn from history are doomed to repeat it. The ASP.NET membership system is the result of years of people trying to implement their own user authentication systems. Microsoft learned from that. So should you.

Use the membership provider model. If you don't want to use the default provider, then implement your own custom one. But honestly, it's very easy to use the built-in provider and adapt it to whatever you want.