0
\$\begingroup\$

Would you have any suggestions improvements for the below functions classes ?

Ok here how do i make a registered member login

    HttpCookie LoginInfo = new HttpCookie("LoginInfo");
    LoginInfo.Values["UserName"] = srUserName;
    LoginInfo.Values["Password"] = srPassword;
    LoginInfo.Values["selectedLanguage"] = srSelectedLanguage;
    Response.Cookies.Add(LoginInfo);

Here how do i check visitor is logged in or not

public static void controlOfLoginStatus()
{
    string srQuery = "";
    string srUserName = "";
    string srPassword = "";
    string srLang = "";

    if (HttpContext.Current.Session["UserId"] == null)
    {
        if (HttpContext.Current.Request.Cookies["LoginInfo"] != null)
        {
            try
            {
                srUserName = HttpContext.Current.Request.Cookies["LoginInfo"]["UserName"].ToString();
                srPassword = HttpContext.Current.Request.Cookies["LoginInfo"]["Password"].ToString();
                srLang = HttpContext.Current.Request.Cookies["LoginInfo"]["selectedLanguage"].ToString();
            }
            catch
            {

            }
        }
        string srUserIdTemp = csPublicFunctions.ReturnUserIdUsernamePassword(srUserName, srPassword);
        if (srUserIdTemp == "0")
        {
            HttpContext.Current.Session.Clear();
            HttpContext.Current.Session.Abandon();
            HttpContext.Current.Response.Redirect("Login");
        }
        else
        {
            csPublicFunctions.insertIntoOnlineUsers(srUserIdTemp, HttpContext.Current.Session.SessionID);
            HttpContext.Current.Session["UserId"] = srUserIdTemp;
            if (HttpContext.Current.Session["lang"] == null)
                HttpContext.Current.Session["lang"] = srLang;
        }
    }

    srQuery = "SELECT UserId " +
     " FROM BannedUsers" +
     " WHERE UserId = " + HttpContext.Current.Session["UserId"].ToString();
    using (DataTable dtTemp = DbConnection.db_Select_DataTable(srQuery))
    {
        if (dtTemp.Rows.Count > 0)
        {
            HttpContext.Current.Response.Redirect("exit.aspx");
        }
    }
}

Here how do i log-out

public static void exitLogout()
{
    string srQuery = "delete from OnlineUsers where UserId=" + HttpContext.Current.Session["UserId"].ToString();
    DbConnection.db_Update_Delete_Query(srQuery);

    try
    {
        HttpContext.Current.Session["UserId"] = "0";
        HttpContext.Current.Session.Clear();
        HttpContext.Current.Session.Abandon();
    }
    catch
    {

    }

    try
    {
        HttpCookie LoginInfo = new HttpCookie("LoginInfo");
        LoginInfo.Values["UserName"] = "21412zxcvzxc343245243vvc";
        LoginInfo.Values["Password"] = "21412zxcvzxc343245243vvc";
        LoginInfo.Values["selectedLanguage"] = "en";
        HttpContext.Current.Response.Cookies.Add(LoginInfo);
    }
    catch
    {            
    }
}

csPublicFunctions.ReturnUserIdUsernamePassword uses parametrized queries so no possible risk of SQL injection

\$\endgroup\$
2

2 Answers 2

4
\$\begingroup\$

In addition to losing the hungarian notation there are a few things worth mentioning:

  1. Don't use try-catch construction with empty 'catch' clause. It is almost always a bad idea to hide an exception if it was thrown.
  2. It would be better to check explicitly if the key is in the collection using appropriate methods. BTW exception handling is quite expensive speedwise and explicit checking will work faster.
  3. Try to refactor your controlOfLoginStatus() method into several distinct functions with more meaningful purpose.
  4. Refactor your database code into separate class.
  5. Use parameterized queries while working with database. It will save you from sql injections and it will make your queries run faster.
\$\endgroup\$
0
\$\begingroup\$

Don't store the users password in a cookie. That's an unnecessary security risk.

Create a new random session id to put there instead. That session id needs to be invalidated on the server on logout. Or if you want auto-login that crosses sessions, you still should create a token that doesn't depend on the user password.

Additionally you should set the Secure and HttpOnly flags.

  • Secure prevents sending of the cookie over unencrypted connections i.e. it enforces https over http. This flag is essential to prevent an attacker on your network from sniffing the cookie.
  • HttpOnly prevents javascript from reading the cookie value. This reduces the impact of XSS vulnerabilities a bit.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.