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

why the ... are you trimming the password!? Some people add one or more spaces at the end (or at the beginning) to make it harder for someone who sees the password in clear text to steal it.. — Andrei Rînea
– Andrei Rînea, Commented Mar 2, 2016 at 20:20

Josh · Accepted Answer · 2011-04-28 04:05:27Z

20

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!

answered Apr 28, 2011 at 4:05

Josh

45k7 gold badges106 silver badges125 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Loupi Over a year ago

Thanks for the advice. I'm aware of the threat of sql injection is just that I've made this on the fly and will not implement it until I've finished making my custom membership provider. I'm also looking for a way to make it less vulnerable. I think I've heard you can use parameters in queries like in daniweb.com/software-development/csharp/threads/301561

James Harris · Accepted Answer · 2011-04-28 04:12:05Z

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.

Erik Funkenbusch · Accepted Answer · 2011-04-28 04:15:41Z

3

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.

answered Apr 28, 2011 at 4:15

Erik Funkenbusch

93.7k29 gold badges201 silver badges292 bronze badges

3 Comments

Josh Over a year ago

I'll even mention that it is pretty darned easy to implement a custom provider as well. I have done this numerous times and it really isn't terribly difficult.

Andrei Rînea Over a year ago

The membership used to have a lot of drawbacks, no wonder it got rewritten by Microsoft (long) after your answer...

Erik Funkenbusch Over a year ago

@AndreiRînea - Even with its drawbacks, it was better than rolling things yourself, the newer Identity is a better solution but that doesn't mean that Membership (back in the day) wasn't the best solution.

Luis Aguilar · Accepted Answer · 2011-04-28 04:02:02Z

2

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.

answered Apr 28, 2011 at 4:02

Luis Aguilar

4,3816 gold badges38 silver badges55 bronze badges

Collectives™ on Stack Overflow

Custom Login ASP.NET C#

4 Answers 4

1 Comment

Comments

3 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related