I wrote a user authentication program(s) for an MVC application. Before you ask part of the project specs are I have to store user information in company databases on servers that aren't the web server so the default membership classes are out of the question (as far as I am aware membership creates its own database). If you would be so kind and let me know how I did.
The Entities
public enum ContactType
{
EmailPrimary,
Email,
PhonePrimary,
Phone,
Facebook,
Twitter,
Google,
Github,
BitBucket,
Microsoft
}
public class User
{
[Key]
public string UserName { get; set; }
public string NamePrefix { get; set; }
public string NameFirst { get; set; }
public string NameMiddle { get; set; }
public string NameLast { get; set; }
public string NameSuffix { get; set; }
public DateTime? DateOfBirth { get; set; }
}
public class UserAccount
{
[Key]
public int AccountId { get; set; }
public string UserName { get; set; }
public string PasswordHash { get; set; }
public bool Lock { get; set; }
public DateTime Created { get; set; }
public DateTime? Updated { get; set; }
public virtual User User { get; set; }
}
public class UserContact
{
[Key]
public int ContactId { get; set; }
public ContactType ContactTypeId { get; set; }
public string Contact { get; set; }
public string UserName { get; set; }
public bool Active { get; set; }
public DateTime Created { get; set; }
public DateTime? Updated { get; set; }
public virtual User User { get; set; }
}
The View Models
public class LogInViewModel
{
public string UserName { get; set; }
public string Password { get; set; }
public bool RememberMe { get; set; }
}
public class RegisterViewModel
{
public string UserName { get; set; }
public string Password { get; set; }
[Compare("Password", ErrorMessage="Passwords do not match.")]
public string ConfirmPassword { get; set; }
public string UserEmail { get; set; }
}
public class PasswordResetRequestViewModel
{
public string UserName { get; set; }
}
public class PasswordResetModel
{
public string Password { get; set; }
[Compare("Password", ErrorMessage="Passwords do not match.")]
public string ConfirmPassword { get; set; }
}
NOTE: Data annotations are on the TODO list for both the entities and the view models.
The Controller
private IAuthService authService = new AuthService();
public ActionResult Index()
{
return View();
}
[HttpPost()]
public ActionResult Index(LogInViewModel model)
{
var user = authService.GetAccount(model.UserName);
if (authService.IsLocked(user))
{
RedirectToAction("ResetPassword");
}
if (ModelState.IsValid)
{
switch (authService.AttemptLogin(user, model.Password).IsAuthenticated)
{
case true:
FormsAuthentication.SetAuthCookie(user.UserName, model.RememberMe);
// UNDONE: Where does the user go after login?
break;
case false:
ModelState.AddModelError("", "Username or Password incorrect.");
break;
}
}
return View(model);
}
public ActionResult Register()
{
return View();
}
[HttpPost()]
public ActionResult Register(RegisterViewModel model)
{
var newUser = authService.CreateAccount(model);
if (newUser != null)
{
// TODO: Send verification email
RedirectToAction("Index");
}
return View(model);
}
public ActionResult PasswordResetRequest()
{
return View();
}
[HttpPost()]
public ActionResult PasswordResetRequest(PasswordResetRequestViewModel model)
{
var passwordResetRequest = authService.CreatePasswordResetRequest(model.UserName);
if (passwordResetRequest != null)
{
// TODO: Send password reset email
}
return View(model);
}
public ActionResult PasswordReset(string id)
{
return View();
}
[HttpPost()]
public ActionResult PasswordReset(string id, PasswordResetModel model)
{
string cipher = id.Substring(0, 16);
string userName = StringCipher.Decrypt(id.Substring(16, id.Length - 16), cipher);
if (authService.ResetUserPassword(userName, model.Password) != null)
{
// TODO: do something to indicate password reset success.
// either log the user in directly from here or
// redirect them to log in with new password.
}
return View(model);
}
NOTE: The idea is if they need to reset their password an email will be sent with an URL and an id that is an encrypted string that equates to their UserName. I do have some concerns with this method.
The Interface/Service Class
public interface IAuthService
{
UserAccount GetAccount(string userName);
UserAccount CreateAccount(RegisterViewModel newAccount);
PasswordResetRequest CreatePasswordResetRequest(string userName);
UserAccount ResetUserPassword(string userName, string password);
bool IsLocked(UserAccount account);
AuthenticatedUser AttemptLogin(UserAccount account, string password);
}
public class AuthService : IAuthService
{
private WebContext db = new WebContext();
public UserAccount GetAccount(string userName)
{
return db.UserAccounts.SingleOrDefault(u=> u.UserName == userName);
}
public UserAccount CreateAccount(RegisterViewModel newAccount)
{
var user = GetAccount(newAccount.UserName);
if (user != null)
{
var accountBuilder = new UserAccountBuilder(newAccount);
Account account = accountBuilder.CreateAccount();
db.Entry(account.UserAccount).State = EntityState.Added;
db.Entry(account.User).State = EntityState.Added;
db.Entry(account.UserContact).State = EntityState.Added;
db.SaveChanges();
return account.UserAccount;
}
return null;
}
public PasswordResetRequest CreatePasswordResetRequest(string userName)
{
UserContact contact = db.UserContacts.FirstOrDefault(p => p.UserName == userName &
p.ContactTypeId == ContactType.EmailPrimary);
if (contact != null)
{
string cipher = StringCipher.GenerateCypher();
return new PasswordResetRequest()
{
UserEmail = contact.Contact,
Key = cipher + StringCipher.Encrypt(userName, cipher)
};
}
return null;
}
public UserAccount ResetUserPassword(string userName, string password)
{
UserAccount account = GetAccount(userName);
if (account != null)
{
account.PasswordHash = Crypto.HashPassword(password);
account.Updated = DateTime.UtcNow;
db.Entry(account).State = EntityState.Modified;
db.SaveChanges();
return account;
}
return null;
}
public bool IsLocked(UserAccount account)
{
return account.Lock;
}
public AuthenticatedUser AttemptLogin(UserAccount account, string password)
{
if (IsValidPassword(account, password))
{
return new AuthenticatedUser(account.User, true);
}
else
{
return new AuthenticatedUser(account.User);
}
}
private bool IsValidPassword(UserAccount account, string password)
{
return Crypto.VerifyHashedPassword(account.PasswordHash, password);
}
private bool AccountAlreadyExists(string UserName)
{
if (db.UserAccounts.Count(p => p.UserName == UserName) == 0)
{
return false;
}
return true;
}
}
public class AuthenticatedUser
{
public User User { get; set; }
public bool IsAuthenticated { get; private set; }
public AuthenticatedUser(User user) : this(user, false)
{
}
public AuthenticatedUser(User user, bool isAuthenticated)
{
User = user;
IsAuthenticated = isAuthenticated;
}
}
public class Account
{
public UserAccount UserAccount { get; set; }
public User User { get; set; }
public UserContact UserContact { get; set; }
}
public class PasswordResetRequest
{
public string UserEmail { get; set; }
public string Key { get; set; }
}
internal class UserAccountBuilder
{
private readonly RegisterViewModel newAccount;
public UserAccountBuilder(RegisterViewModel account)
{
newAccount = account;
}
public Account CreateAccount()
{
return new Account()
{
UserAccount = CreateUserAccount(),
User = CreateUser(),
UserContact = CreateUserContact()
};
}
private UserAccount CreateUserAccount()
{
return new UserAccount()
{
UserName = newAccount.UserName,
PasswordHash = CreatePasswordHash(newAccount.Password),
Lock = true,
Created = DateTime.UtcNow
};
}
private User CreateUser()
{
return new User()
{
UserName = newAccount.UserName
};
}
private UserContact CreateUserContact()
{
string cipher = StringCipher.GenerateCypher();
return new UserContact()
{
ContactTypeId = ContactType.EmailPrimary,
Contact = cipher + StringCipher.Encrypt(newAccount.UserEmail, cipher),
UserName = newAccount.UserName,
Active = true,
Created = DateTime.UtcNow
};
}
private string CreatePasswordHash(string password)
{
return Crypto.HashPassword(password);
}
}
NOTE: I didn't feel it necessary to include the context models. If you disagree with this decision I would be more than happy to post them. Rest assured
WebContextis an instance of a context class. I also didn't feel the need to include the Helper classes I've create for encryption as they are readily available online. Anytime I callStringCipherI am referring to that helper class.
Now that you've taken the time to read all that (and thank you for that), please "let me have it" so to speak. As the tags of this post indicate I am in no way amazingly experienced with C# and have only a year of .NET experience under my belt. What "best practices" am I killing here that make you cringe? What can I do differently more efficiently? How would you make this code better, or perhaps more importantly how would you write this differently?
