Simple authentication in ASP.NET MVC 5

Question

I am building an ASP.NET MVC 5 application and, for reasons which are irrelevant at this point, I am attempting to build my own means of authenticating users. I'm still very new to programming, especially to this sort of thing, and I realize that what looks fine to me may be full of problems. Thus, I present my code, simplified for demonstration purposes, for review.

The Contact model:

public class Contact
{
    [Key]
    public int ContactID { get; set; }

    [Required]
    public string Email { get; set; }

    [Required]
    public string Password { get; set; }
}

The Login action:

[HttpPost]
public ActionResult Login(Contact contact)
{
    bool validEmail = db.Contacts.Any(x => x.Email == contact.Email);

    if (!validEmail)
    {
        return RedirectToAction("Login");
    }

    string password = db.Contacts.Where(x => x.Email == loginForm.Email)
                                 .Select(x => x.Password)
                                 .Single();

    bool passwordMatches = Crypto.VerifyHashedPassword(password, loginForm.Password);

    if (!passwordMatches)
    {
        return RedirectToAction("Login");
    }

    string authId = Guid.NewGuid().ToString();

    Session["AuthID"] = authId;

    var cookie = new HttpCookie("AuthID");
    cookie.Value = authId;
    Response.Cookies.Add(cookie);

    return RedirectToAction("Private");
}

Actions that require the user to be logged in will look like this:

public ActionResult Private()
{
    try
    {
        if (Request.Cookies["AuthID"].Value == Session["AuthID"].ToString())
        {
            return View();
        }
        else
        {
            return RedirectToAction("Index");
        }
    }
    catch
    {
        return RedirectToAction("Index");
    }
}

I feel like this is probably really bad. However, every "solution" I find seems overly complex and/or lacking any documentation that would allow me to make sense of it.

All I want is to make sure the user has a valid username and password before they place an order or update their account. Will my code do that securely?

Answer 1

I fear that the credentials could be sniff by some attacker if they are listening to during the connection process. If https is not enabled, the password (if there is no protection) could be seen by any attacker during the HTTP requests.

This point is not directly related as to how you implemented your code, but it will go a long way as to how your code is safe.

---

string authId = Guid.NewGuid().ToString();  

I'm not a security expert, but using GUID for security purpose is something that I would recommend. It seems to be frown upon :

GUIDs were not designed to be a part of any security system; use of them in a security system is an "off label" use and is therefore a very bad idea. I can't see much at the moment that would break the mechanism.

You should take this advice as-is, and do some more research on it. I would not take a comment on a question as a good reference, but it would make me wonder if it's in fact a good idea.

---

There is a lot of documentation to help you develop a secure authentication system,but this great question on SO would cover it for most of the important things.

---

The authentication mechanism looks fine to me. Storing a cookie and check the value in the Session is a good way to assure that user is authenticated.

The only drawback is if someone hijack the session and the cookie, he don't need to login with credentials, since the token will assure him that the system see him as an authenticated user. There is not much in your code that you could do to prevent that.

What you will miss most by rolling your own mechanism is some "perk" from using the .Net implementation. From what I've previously done at school and search, you'll miss the [Authorize][3] properties where your code could look :

[Authorize]
public ActionResult Private()
{
    return View();

}

Which would hide all the cookie related code. What that could help you is, if for a specific reason you can't use cookies you won't need to change all the place where you were verifying the identity of your user. This is just for code readability and maintenance, this is not related to security.

Answer 2

You can create a custom authorization class inheriting ActionFilterAttribute

public class AuthorizationFilter : ActionFilterAttribute
{
  public override void OnActionExecuting(ActionExecutingContex   filterContext)
  {
  //Write you authentication logic here . you can use Request headers,cookies ,..etc 
  }

}

Then decorate your action method with [AuthorizationFilter]

[AuthorizationFilter]
public ActionResult GetAllEmployee()
{
  //Action method logic
}

Answer 3

One addition to what has been said:

It's not enough to make sure that the login is done using https, as you don't want the authentication cookie to be submitted during any non-secure requests, as it could be intercepted by anyone listening to network traffic.

So, you also need both of the following conditions to be true:

1. The authentication cookie needs to be marked as secure, so that it is only submitted on HTTPS requests.
2. All actions that require authentication need to be available only using HTTPS.

In practice, if you have HTTPS enforced for everything across the entire site (and any other sub-domains that someone might set up), then point 1 could be avoided, but there's no reason to do that.