Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've discovered that using hashed passwords with salts is much better idea than MD5/SHA256 so I'm not hashing them with PBKDF2. However I'm wondering if this is correct approach to authorize my user. I also have logic for logging authorizations and when same IP did wrong login/pass it is banned for 5 minutes.

  • Administrator a is passing username and password
  • Checking if username exists
  • Checking if password is correct for this user
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Authorize(Administrator a)
{
    // Check if there are no failed login attempts in last 5 minutes
    if (!this.CanAdminLogin)
    {
        TempData["loginTooManyAttempts"] = true;
        return RedirectToAction("index", "home");
    }

    // If model is not validated return login view to show error messages (javascript disabled)
    if (!TryValidateModel(a))
    {
        return View("Authorize", a);
    }

    // Check if username exists, if not then log and show that login failed
    var admin = _db.Administrators.Where(x => x.Username == a.Username).SingleOrDefault();
    if (admin == null || admin.Username != a.Username)
    {
        this.LogAuthorization(a, false);
        TempData["loginFailed"] = true;
        return RedirectToAction("index", "home");
    }

    // Username exists, check if passwords match
    ICryptoService cryptoService = new PBKDF2();
    string hash = cryptoService.Compute(a.Password, admin.PasswordSalt);
    if (hash == admin.Password)
    {
        this.LogAuthorization(a, true);
        Session["adminId"] = admin.ID;
    }
    else
    {
        this.LogAuthorization(a, false);
        TempData["loginFailed"] = true;
    }

    // Login successfull
    return RedirectToAction("index", "home");
}
share|improve this question
    
I think code like _db.Administrators.Where(...) should be put in a repository to aid Unit Testing. –  rhughes May 23 at 3:47

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.