HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Username, Password and UserType Validation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usertypevalidationpasswordusernameand

Problem

I have two tables in the database:

-
Credentials (userid, password, usertype)

-
Customer (customername, userid(foreign key))

I need to validate username,password and usertype and create a session variable for customername.

I have organized the code and it works fine, but how can I minimize the code and make it efficient?

```
// Function that enables LOGON for a registered User into site
public string LogOn(UserInformation objLogOnUserInformation)
{
ConnectionManager conCm = new ConnectionManager();
try
{
SqlConnection con = conCm.OpenConnection();
SqlCommand cmd,cmdOne;
SqlDataReader dr;
string _str = "U";
cmd = new SqlCommand("select UserID from Credentials", con);
dr = cmd.ExecuteReader();

while (dr.Read())
{
//checking user name is present in database
if (dr.GetValue(0).ToString() == objLogOnUserInformation.UserId)
{
dr.Close();
//retriving Password for the existing user
cmd = new SqlCommand("select Password from Credentials where UserId='" + objLogOnUserInformation.UserId + "'", con);
dr = cmd.ExecuteReader();

while (dr.Read())
{
//checking the password is matching with the database
if (dr.GetValue(0).ToString() == objLogOnUserInformation.Password)
{
dr.Close();
cmd = new SqlCommand("select UserType from Credentials where userid='" + objLogOnUserInformation.UserId + "'", con);
dr = cmd.ExecuteReader();

while (dr.Read())
{
//checking user type
if (dr.GetValue(0).ToString() == _str)
{
dr.Close();
cmdOne = new SqlCo

Solution

Here are some observations:

  • you first iterate all rows in Credentials and then select the Password yet again - why not just select the pasword?



  • seems like your password is plain-text - BAD idea - hash your password together with a salt please



  • don't concat strings to get SELECT queries - use parameters instead



  • you are using C#/ASP.NET so why don't you use the provided login mechanisms?



  • you return a string "A" if the login succeeds and the error message if not - this is IMO bad design - return a simple struct/class with more information instead

Context

StackExchange Code Review Q#56087, answer score: 13

Revisions (0)

No revisions yet.