ASP.NET View Model / Identity Model

Question

I'm working in MVC5/WebAPI using ASP.NET Identity, with a custom storage provider for my UserStore, custom ApplicationUser inheriting from IdentityUser, and a pretty expansive configuration in the project (both MVC5 and WebAPI endpoints in the same project).

I've been working on the user management/settings controller (ManageController), following the same principles the template projects use; however, with the additional fields I have in my ApplicationUser, it is getting to the point that my view model is starting to be a duplicate of the ApplicationUser class. I wanted someone to take a look at some of my code and let me know if they think the way I'm setting things up currently is appropriate, or if I am cluttering a view model with fields.

ManageViewModels.cs

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using Microsoft.AspNet.Identity;
using Microsoft.Owin.Security;
using System.Security.Claims;

namespace Disco.Models
{
    public class IndexViewModel
    {        
        public string Email { get; set; }        
        public bool HasPassword { get; set; }
        public IList<UserLoginInfo> Logins { get; set; }
        public string PhoneNumber { get; set; }
        public bool TwoFactor { get; set; }
        public bool BrowserRemembered { get; set; }

        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string Picture { get; set; }
        public string Handle { get; set;}
    }

    public class ManageLoginsViewModel
    {
        public IList<UserLoginInfo> CurrentLogins { get; set; }
        public IList<AuthenticationDescription> OtherLogins { get; set; }
    }

    public class FactorViewModel
    {
        public string Purpose { get; set; }
    }

    public class SetPasswordViewModel
    {
        [Required]
        [StringLength(100, ErrorMessage = "The {0} must be at least {2} characters long.", MinimumLength = 6)]
        [DataType(DataType.Password)]
        [Display(Name = "New password")]
        public string NewPassword { get; set; }

        [DataType(DataType.Password)]
        [Display(Name = "Confirm new password")]
        [Compare("NewPassword", ErrorMessage = "The new password and confirmation password do not match.")]
        public string ConfirmPassword { get; set; }
    }

    public class ChangePasswordViewModel
    {
        [Required]
        [DataType(DataType.Password)]
        [Display(Name = "Current password")]
        public string OldPassword { get; set; }

        [Required]
        [StringLength(100, ErrorMessage = "The {0} must be at least {2} characters long.", MinimumLength = 6)]
        [DataType(DataType.Password)]
        [Display(Name = "New password")]
        public string NewPassword { get; set; }

        [DataType(DataType.Password)]
        [Display(Name = "Confirm new password")]
        [Compare("NewPassword", ErrorMessage = "The new password and confirmation password do not match.")]
        public string ConfirmPassword { get; set; }
    }

    public class AddPhoneNumberViewModel
    {
        [Required]
        [Phone]
        [Display(Name = "Phone Number")]
        public string Number { get; set; }
    }

    public class VerifyPhoneNumberViewModel
    {
        [Required]
        [Display(Name = "Code")]
        public string Code { get; set; }

        [Required]
        [Phone]
        [Display(Name = "Phone Number")]
        public string PhoneNumber { get; set; }
    }

    public class ConfigureTwoFactorViewModel
    {
        public string SelectedProvider { get; set; }
        public ICollection<System.Web.Mvc.SelectListItem> Providers { get; set; }
    }
}

ApplicationUser.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNet.Identity;
using System.Security.Claims;
using Newtonsoft.Json;

namespace Schloss.AspNet.Identity.Neo4j
{
    public class ApplicationUser : IdentityUser
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }

        public DateTimeOffset DateOfBirth { get; set; }

        // @ handle / username
        public string Handle { get; set; }

        public string Picture { get; set; }

        public char Gender { get; set; }

        public string DeviceId { get; set; } // Most recently used mobile phone ID (iPhone, iPad, Android, etc)

        // Address
        public string City { get; set; }
        public string State { get; set; }
        public string ZIPCode { get; set; }
        public string County { get; set; }
        public string Country { get; set; }
        public string Address1 { get; set; }
        public string Address2 { get; set; }
        public string Address3 { get; set; }

        // SignalR
        public HashSet<string> ConnectionIds { get; set; } // SignalR connections set (for handling multiple live sessions, propagation of push notifications)

        //public static string Labels { get { return "User"; } }

        // Helper Properties
        [JsonIgnore]
        public String FullName
        {
            get { return (FirstName + " " + LastName).Trim(); } // helper property to format user's full name
        }

        public async Task<ClaimsIdentity> GenerateUserIdentityAsync(UserManager<ApplicationUser> manager)
        {
            // Note the authenticationType must match the one defined in CookieAuthenticationOptions.AuthenticationType
            var userIdentity = await manager.CreateIdentityAsync(this, DefaultAuthenticationTypes.ApplicationCookie);

            // Add custom user claims here

            return userIdentity;
        }

        public async Task<ClaimsIdentity> GenerateUserIdentityAsync(UserManager<ApplicationUser> manager, string authenticationType)
        {
            // Note the authenticationType must match the one defined in CookieAuthenticationOptions.AuthenticationType
            var userIdentity = await manager.CreateIdentityAsync(this, authenticationType);

            // Add custom user claims here

            return userIdentity;
        }
    }
}

ManageController.cs (just the Index view)

//
        // GET: /Manage/Index
        public async Task<ActionResult> Index(ManageMessageId? message)
        {
            ViewBag.StatusMessage =
                message == ManageMessageId.ChangePasswordSuccess ? "Your password has been changed."
                : message == ManageMessageId.SetPasswordSuccess ? "Your password has been set."
                : message == ManageMessageId.SetTwoFactorSuccess ? "Your two-factor authentication provider has been set."
                : message == ManageMessageId.Error ? "An error has occurred."
                : message == ManageMessageId.AddPhoneSuccess ? "Your phone number was added."
                : message == ManageMessageId.RemovePhoneSuccess ? "Your phone number was removed."
                : "";

            var userId = User.Identity.GetUserId();
            var user = await UserManager.FindByIdAsync(userId);

            var model = new IndexViewModel
            {                
                Email = await UserManager.GetEmailAsync(userId),                
                HasPassword = await UserManager.HasPasswordAsync(userId),
                PhoneNumber = await UserManager.GetPhoneNumberAsync(userId),
                TwoFactor = await UserManager.GetTwoFactorEnabledAsync(userId),
                Logins = await UserManager.GetLoginsAsync(userId),
                BrowserRemembered = await AuthenticationManager.TwoFactorBrowserRememberedAsync(userId),

                FirstName = user.FirstName,
                LastName = user.LastName,
                Picture = user.Picture,
                Handle = user.Handle
            };
            return View(model);
        }

Answer 1

MergeController.cs

First off, I would use a switch on message. It's faster (though not by a lot) than ternary (?:) operators or if statements. Also, I would have each UserManager async method run at the same time. That is, save them to a var and then await each var in the IndexViewModel. (You may have to clean up syntax for that portion, I wrote it without the ASP.NET context.)

The idea behind moving the await keywords, is that it allows all the tasks to start before requesting data from them, so they can run in parallel. As it stands now, the programme will wait for user to run, then it will wait for email to run, etc. This way, it will get them all started, and then await what's left. (Though, again, I wrote it without the ASP.NET context, you may or may not be able to use that portion in that manner.)

public async Task<ActionResult> Index(ManageMessageId? message)
{
    switch (message)
    {
        case ManageMessageId.ChangePasswordSuccess:
            ViewBag.StatusMessage = "Your password has been changed.";
            break;
        case ManageMessageId.SetPasswordSuccess:
            ViewBag.StatusMessage = "Your password has been set.";
            break;
        case ManageMessageId.SetTwoFactorSuccess:
            ViewBag.StatusMessage = "Your two-factor authentication provider has been set.";
            break;
        case ManageMessageId.Error:
            ViewBag.StatusMessage = "An error has occurred.";
            break;
        case ManageMessageId.AddPhoneSuccess:
            ViewBag.StatusMessage = "Your phone number was added.";
            break;
        case ManageMessageId.RemovePhoneSuccess:
            ViewBag.StatusMessage = "Your phone number was removed.";
            break;
        default:
            ViewBag.StatusMessage = "";
            break;
    }

    var userId = User.Identity.GetUserId();
    var userTask = UserManager.FindByIdAsync(userId);

    var email = UserManager.GetEmailAsync(userId);
    var hasPassword = UserManager.HasPasswordAsync(userId);
    var phoneNumber = UserManager.GetPhoneNumberAsync(userId);
    var twoFactor = UserManager.GetTwoFactorEnabledAsync(userId);
    var logins = UserManager.GetLoginsAsync(userId);
    var browserRemembered = AuthenticationManager.TwoFactorBrowserRememberedAsync(userId);
    var user = await userTask;

    var model = new IndexViewModel
    {
        Email = await email,
        HasPassword = await hasPassword,
        PhoneNumber = await phoneNumber,
        TwoFactor = await twoFactor,
        Logins = await logins,
        BrowserRemembered = await browserRemembered,

        FirstName = user.FirstName,
        LastName = user.LastName,
        Picture = user.Picture,
        Handle = user.Handle
    };
    return View(model);
}

ApplicationUser.cs

I can't see anything here that really needs help. There's not much going in so it's inherently clean.

ManageViewModels.cs

Again, there's not much here so not a lot to optimize.

Overall, there's not a lot going on, so there's not a lot to optimize. Most of the optimizations that could be made are purely readability/maintenance, apart from the await keyword optimizations.

IndexViewModel

Your initial concern was the IndexViewModel, which as far as I can see, doesn't require much work. (If you need all those properties for display, then so be it. Don't try cutting out properties if you know you need them, just because you think there are too many.)

You could possibly replace properties there that are duplicates of the ApplicationUser, and then store an ApplicationUser in it, and access the properties by IndexViewModel.ApplicationUser.Property. Though you can really only do that for the common properties.