User Login Logic

I want to see if this is as streamlined as possible. Most of the logic is accomplished through methods in the model. The controller does one of four things:

• redirects new users to registration

• sends locked users to reset password

• logs the user in and redirects them to where they should go

• fails the login and allows them to try again

The Controller

using System.Web.Mvc;
using System.Web.Security;
using Authenticator.Models;

namespace Authenticator.Controllers
{
    public class HomeController : Controller
    {
        // Get index
        public ActionResult Index()
        {
            return View();
        }

        [HttpPost()]
        public ActionResult Index(LoginViewModel model)
        {
            // Redirect User to Register if they do not exist;
            if (model.VerifyAccountExists() == false)
            {
                return RedirectToAction("Register");
            }

            // Determine if account is locked and redirect user to reset password.
            if (model.VerifyLock())
            {
                return RedirectToAction("PasswordReset");
            }

            if (model.VerifyPassword())
            {
                model.PassLogin();
                FormsAuthentication.SetAuthCookie(model.UserId, model.RememberMe);
                return Redirect(model.GetAppUrl());
            }
            else 
            {
                model.FailLogin();
                ModelState.AddModelError("", "Incorrect Network ID or Password.");
            }

            return View(model);
        }

        // shared view data
        protected override void OnActionExecuted(ActionExecutedContext filterContext)
        {
            base.OnActionExecuted(filterContext);
            ViewBag.BodyClass = "authenticator";  
        }
    }
}

The View Model

```
using System;
using System.Linq;
using System.Data;
using System.Web.Helpers;

namespace Authenticator.Models
{
public class LoginViewModel
{

Viewmodels from my understanding should be as lean as free of domain logic as possible. This is because they are a link from your domain model to your view and so can provide a layer/separation between the two if your view incorporates elements from various places/models.

Hence, all the logic/methods you have in your view model I would pull out into a Service class that is responsible for performing these as required.

public class LoginService
{
    public bool VerifyAccountExists()
    {
        using (var db = new WebContext())
        {
            if (db.UserAccounts.Count(p => p.UserId == UserId) > 0)
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }
}

Also, containing data access layer code in your view model is considered bad practice from my understanding See this answer on SO. So further to this I would consider a number of changes (based on whether you consider the complexity worth it),

• Abstracting your services behind an interface for better separation of concerns.

• Injecting your services into your controllers using tools such as Inject, Unit, AutoFac etc

• Injecting your database into your LoginService

An example of a starting point for a refactored solution might be along these lines:

```
public interface ILoginService
{
User GetUser(int userId);
bool IsLocked(User user);
AuthenticatedUser AttemptLogin(user, string password);

// NOTE: I would prefer this to be seperate from the service and part of a UnitOfWork class
void SaveChanges();
}

public class LoginService
{
private readonly WebContext _context;

public LoginService(WebContext context)
{
_context = context;
}

public void SaveChanges()
{
_context.SaveChanges();
}

public User GetUser(int userId)
{
return _context.UserAccounts.SingleOrDefault(userId);
}

public bool IsLocked(User user)
{
return user.Lock;
}

public AuthenticatedUser AttemptLogin(User user, string password)
{
if(IsValidPassword(user, password))
{
RegisterSuccessLoginAttempt(user);

return new AuthenticatedUser(
user,
GetUserWelcomeUrl(user),
true);
);
}
else
{
RegisterFailedLoginAttempt();
return new AuthenticatedUser(user);
}
}

private string GetUserWelcomeUrl(User user)
{
var query = (from a in _context.Permissions
join b in _context.UserPermissions on a.Id equals b.PermissionId
join c in _context.Applications on a.Name equals c.AppName
where a.Type == PermissionType.AppAccess & b.UserId == UserId
select c).ToList();

// NOTE: I would pull these urls from a config file, or a IConfiguration class passed into this service
switch (query.Count)
{
case 0:
return "http://azshisp11/Tickets/Create?subject==Request%20For%20Application%20Access";
// TODO: code ticket logic to except this argument
case 1:
return query[0].AppUrl;
default:
return "http://azshisp11/Dashboard/" + UserId;
}
}

private void RegisterSuccessLoginAttempt(User user)
{
user.FailedAttempts = 0;
user.Lock = false;
user.DateLastLogin = DateTime.Now;
user.DateUpdated = DateTime.Now;
}

private void RegisterFailedLoginAttempt(User user)
{
switch (user.FailedAttempts)
{
case 1 - 3:
user.FailedAttempts = account.FailedAttempts + 1;
user.DateUpdated = DateTime.Now;
break;
default:
user.FailedAttempts = 4;
user.Lock = true;
user.DateUpdated = DateTime.Now;
break;
}
}

private bool IsValidPassword(User user, string password)
{
return Crypto.VerifyHashedPassword(user.PasswordHash, password);
}
}

public class AuthenticatedUser
{
public string HomeUrl { get; private set; }
public User User { get; private set; }
public bool IsAuthenticated { get; private set; }

public AuthenticatedUser(
User user) : this(user, string.empty, false)
{
}

public AuthenticatedUser(
User user,
string welcomeUrl,
bool isAuthenticated)
{
User = user;
WelcomeUrl = welcomeUrl;
IsAuthenticated = isAuthenticated;
}
}

public class HomeController : Controller
{
private readonly ILoginService _loginService;

public HomeController(ILoginService loginService)
{
_loginService = loginService;
}

// Get index
public ActionResult Index()
{
return

StackExchange Code Review Q#54570, answer score: 3