Improvements to UserManager

11 files changed, 237 insertions, 274 deletions

diff --git a/Emby.Notifications/NotificationManager.cs b/Emby.Notifications/NotificationManager.cs
index a767e541e..eecbbea07 100644
--- a/Emby.Notifications/NotificationManager.cs
+++ b/Emby.Notifications/NotificationManager.cs
@@ -89,7 +89,7 @@ namespace Emby.Notifications
                         return _userManager.Users.Where(i => i.Policy.IsAdministrator)
                                 .Select(i => i.Id);
                     case SendToUserType.All:
-                        return _userManager.Users.Select(i => i.Id);
+                        return _userManager.UsersIds;
                     case SendToUserType.Custom:
                         return request.UserIds;
                     default:
diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs
index 0b3b81f94..c390ba635 100644
--- a/Emby.Server.Implementations/ApplicationHost.cs
+++ b/Emby.Server.Implementations/ApplicationHost.cs
@@ -770,7 +770,8 @@ namespace Emby.Server.Implementations
 
             _userRepository = GetUserRepository();
 
-            UserManager = new UserManager(LoggerFactory, ServerConfigurationManager, _userRepository, XmlSerializer, NetworkManager, () => ImageProcessor, () => DtoService, this, JsonSerializer, FileSystemManager);
+            UserManager = new UserManager(LoggerFactory.CreateLogger<UserManager>(), _userRepository, XmlSerializer, NetworkManager, () => ImageProcessor, () => DtoService, this, JsonSerializer, FileSystemManager);
+
             serviceCollection.AddSingleton(UserManager);
 
             LibraryManager = new LibraryManager(this, LoggerFactory, TaskManager, UserManager, ServerConfigurationManager, UserDataManager, () => LibraryMonitor, FileSystemManager, () => ProviderManager, () => UserViewManager);
diff --git a/Emby.Server.Implementations/Data/SqliteUserDataRepository.cs b/Emby.Server.Implementations/Data/SqliteUserDataRepository.cs
index 4035bb99d..9d4855bcf 100644
--- a/Emby.Server.Implementations/Data/SqliteUserDataRepository.cs
+++ b/Emby.Server.Implementations/Data/SqliteUserDataRepository.cs
@@ -44,7 +44,7 @@ namespace Emby.Server.Implementations.Data
                 var userDatasTableExists = TableExists(connection, "UserDatas");
                 var userDataTableExists = TableExists(connection, "userdata");
 
-                var users = userDatasTableExists ? null : userManager.Users.ToArray();
+                var users = userDatasTableExists ? null : userManager.Users;
 
                 connection.RunInTransaction(db =>
                 {
@@ -84,7 +84,7 @@ namespace Emby.Server.Implementations.Data
             }
         }
 
-        private void ImportUserIds(IDatabaseConnection db, User[] users)
+        private void ImportUserIds(IDatabaseConnection db, IEnumerable<User> users)
         {
             var userIdsWithUserData = GetAllUserIdsWithUserData(db);
 
diff --git a/Emby.Server.Implementations/EntryPoints/RefreshUsersMetadata.cs b/Emby.Server.Implementations/EntryPoints/RefreshUsersMetadata.cs
index b7565adec..b2328121e 100644
--- a/Emby.Server.Implementations/EntryPoints/RefreshUsersMetadata.cs
+++ b/Emby.Server.Implementations/EntryPoints/RefreshUsersMetadata.cs
@@ -50,9 +50,7 @@ namespace Emby.Server.Implementations.EntryPoints
 
         public async Task Execute(CancellationToken cancellationToken, IProgress<double> progress)
         {
-            var users = _userManager.Users.ToList();
-
-            foreach (var user in users)
+            foreach (var user in _userManager.Users)
             {
                 cancellationToken.ThrowIfCancellationRequested();
 
diff --git a/Emby.Server.Implementations/Library/DefaultPasswordResetProvider.cs b/Emby.Server.Implementations/Library/DefaultPasswordResetProvider.cs
index c7044820c..fa6bbcf91 100644
--- a/Emby.Server.Implementations/Library/DefaultPasswordResetProvider.cs
+++ b/Emby.Server.Implementations/Library/DefaultPasswordResetProvider.cs
@@ -1,15 +1,12 @@
 using System;
 using System.Collections.Generic;
-using System.Globalization;
 using System.IO;
-using System.Linq;
-using System.Text;
+using System.Security.Cryptography;
 using System.Threading.Tasks;
 using MediaBrowser.Common.Extensions;
 using MediaBrowser.Controller.Authentication;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Library;
-using MediaBrowser.Model.Cryptography;
 using MediaBrowser.Model.Serialization;
 using MediaBrowser.Model.Users;
 
@@ -17,32 +14,37 @@ namespace Emby.Server.Implementations.Library
 {
     public class DefaultPasswordResetProvider : IPasswordResetProvider
     {
-        public string Name => "Default Password Reset Provider";
+        private const string BaseResetFileName = "passwordreset";
 
-        public bool IsEnabled => true;
+        private readonly IJsonSerializer _jsonSerializer;
+        private readonly IUserManager _userManager;
 
         private readonly string _passwordResetFileBase;
         private readonly string _passwordResetFileBaseDir;
-        private readonly string _passwordResetFileBaseName = "passwordreset";
 
-        private readonly IJsonSerializer _jsonSerializer;
-        private readonly IUserManager _userManager;
-        private readonly ICryptoProvider _crypto;
-
-        public DefaultPasswordResetProvider(IServerConfigurationManager configurationManager, IJsonSerializer jsonSerializer, IUserManager userManager, ICryptoProvider cryptoProvider)
+        public DefaultPasswordResetProvider(
+            IServerConfigurationManager configurationManager,
+            IJsonSerializer jsonSerializer,
+            IUserManager userManager)
         {
             _passwordResetFileBaseDir = configurationManager.ApplicationPaths.ProgramDataPath;
-            _passwordResetFileBase = Path.Combine(_passwordResetFileBaseDir, _passwordResetFileBaseName);
+            _passwordResetFileBase = Path.Combine(_passwordResetFileBaseDir, BaseResetFileName);
             _jsonSerializer = jsonSerializer;
             _userManager = userManager;
-            _crypto = cryptoProvider;
         }
 
+        /// <inheritdoc />
+        public string Name => "Default Password Reset Provider";
+
+        /// <inheritdoc />
+        public bool IsEnabled => true;
+
+        /// <inheritdoc />
         public async Task<PinRedeemResult> RedeemPasswordResetPin(string pin)
         {
             SerializablePasswordReset spr;
-            HashSet<string> usersreset = new HashSet<string>();
-            foreach (var resetfile in Directory.EnumerateFiles(_passwordResetFileBaseDir, $"{_passwordResetFileBaseName}*"))
+            List<string> usersreset = new List<string>();
+            foreach (var resetfile in Directory.EnumerateFiles(_passwordResetFileBaseDir, $"{BaseResetFileName}*"))
             {
                 using (var str = File.OpenRead(resetfile))
                 {
@@ -53,12 +55,15 @@ namespace Emby.Server.Implementations.Library
                 {
                     File.Delete(resetfile);
                 }
-                else if (spr.Pin.Replace("-", "").Equals(pin.Replace("-", ""), StringComparison.InvariantCultureIgnoreCase))
+                else if (string.Equals(
+                    spr.Pin.Replace("-", string.Empty),
+                    pin.Replace("-", string.Empty),
+                    StringComparison.InvariantCultureIgnoreCase))
                 {
                     var resetUser = _userManager.GetUserByName(spr.UserName);
                     if (resetUser == null)
                     {
-                        throw new Exception($"User with a username of {spr.UserName} not found");
+                        throw new ResourceNotFoundException($"User with a username of {spr.UserName} not found");
                     }
 
                     await _userManager.ChangePassword(resetUser, pin).ConfigureAwait(false);
@@ -81,10 +86,11 @@ namespace Emby.Server.Implementations.Library
             }
         }
 
+        /// <inheritdoc />
         public async Task<ForgotPasswordResult> StartForgotPasswordProcess(MediaBrowser.Controller.Entities.User user, bool isInNetwork)
         {
             string pin = string.Empty;
-            using (var cryptoRandom = System.Security.Cryptography.RandomNumberGenerator.Create())
+            using (var cryptoRandom = RandomNumberGenerator.Create())
             {
                 byte[] bytes = new byte[4];
                 cryptoRandom.GetBytes(bytes);
diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs
index c8c8a108d..086527883 100644
--- a/Emby.Server.Implementations/Library/UserManager.cs
+++ b/Emby.Server.Implementations/Library/UserManager.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
@@ -11,13 +12,11 @@ using MediaBrowser.Common.Events;
 using MediaBrowser.Common.Net;
 using MediaBrowser.Controller;
 using MediaBrowser.Controller.Authentication;
-using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Devices;
 using MediaBrowser.Controller.Drawing;
 using MediaBrowser.Controller.Dto;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
-using MediaBrowser.Controller.Net;
 using MediaBrowser.Controller.Persistence;
 using MediaBrowser.Controller.Plugins;
 using MediaBrowser.Controller.Providers;
@@ -41,34 +40,19 @@ namespace Emby.Server.Implementations.Library
     public class UserManager : IUserManager
     {
         /// <summary>
-        /// Gets the users.
-        /// </summary>
-        /// <value>The users.</value>
-        public IEnumerable<User> Users => _users;
-
-        private User[] _users;
-
-        /// <summary>
         /// The _logger
         /// </summary>
         private readonly ILogger _logger;
 
-        /// <summary>
-        /// Gets or sets the configuration manager.
-        /// </summary>
-        /// <value>The configuration manager.</value>
-        private IServerConfigurationManager ConfigurationManager { get; set; }
+        private readonly object _policySyncLock = new object();
 
         /// <summary>
         /// Gets the active user repository
         /// </summary>
         /// <value>The user repository.</value>
-        private IUserRepository UserRepository { get; set; }
-        public event EventHandler<GenericEventArgs<User>> UserPasswordChanged;
-
+        private readonly IUserRepository _userRepository;
         private readonly IXmlSerializer _xmlSerializer;
         private readonly IJsonSerializer _jsonSerializer;
-
         private readonly INetworkManager _networkManager;
 
         private readonly Func<IImageProcessor> _imageProcessorFactory;
@@ -76,6 +60,8 @@ namespace Emby.Server.Implementations.Library
         private readonly IServerApplicationHost _appHost;
         private readonly IFileSystem _fileSystem;
 
+        private ConcurrentDictionary<Guid, User> _users;
+
         private IAuthenticationProvider[] _authenticationProviders;
         private DefaultAuthenticationProvider _defaultAuthenticationProvider;
 
@@ -85,8 +71,7 @@ namespace Emby.Server.Implementations.Library
         private DefaultPasswordResetProvider _defaultPasswordResetProvider;
 
         public UserManager(
-            ILoggerFactory loggerFactory,
-            IServerConfigurationManager configurationManager,
+            ILogger<UserManager> logger,
             IUserRepository userRepository,
             IXmlSerializer xmlSerializer,
             INetworkManager networkManager,
@@ -96,8 +81,8 @@ namespace Emby.Server.Implementations.Library
             IJsonSerializer jsonSerializer,
             IFileSystem fileSystem)
         {
-            _logger = loggerFactory.CreateLogger(nameof(UserManager));
-            UserRepository = userRepository;
+            _logger = logger;
+            _userRepository = userRepository;
             _xmlSerializer = xmlSerializer;
             _networkManager = networkManager;
             _imageProcessorFactory = imageProcessorFactory;
@@ -105,8 +90,51 @@ namespace Emby.Server.Implementations.Library
             _appHost = appHost;
             _jsonSerializer = jsonSerializer;
             _fileSystem = fileSystem;
-            ConfigurationManager = configurationManager;
-            _users = Array.Empty<User>();
+            _users = null;
+        }
+
+        public event EventHandler<GenericEventArgs<User>> UserPasswordChanged;
+
+        /// <summary>
+        /// Occurs when [user updated].
+        /// </summary>
+        public event EventHandler<GenericEventArgs<User>> UserUpdated;
+
+        public event EventHandler<GenericEventArgs<User>> UserPolicyUpdated;
+
+        public event EventHandler<GenericEventArgs<User>> UserConfigurationUpdated;
+
+        public event EventHandler<GenericEventArgs<User>> UserLockedOut;
+
+        public event EventHandler<GenericEventArgs<User>> UserCreated;
+
+        /// <summary>
+        /// Occurs when [user deleted].
+        /// </summary>
+        public event EventHandler<GenericEventArgs<User>> UserDeleted;
+
+        /// <inheritdoc />
+        public IEnumerable<User> Users => _users.Values;
+
+        /// <inheritdoc />
+        public IEnumerable<Guid> UsersIds => _users.Keys;
+
+        /// <summary>
+        /// Called when [user updated].
+        /// </summary>
+        /// <param name="user">The user.</param>
+        private void OnUserUpdated(User user)
+        {
+            UserUpdated?.Invoke(this, new GenericEventArgs<User> { Argument = user });
+        }
+
+        /// <summary>
+        /// Called when [user deleted].
+        /// </summary>
+        /// <param name="user">The user.</param>
+        private void OnUserDeleted(User user)
+        {
+            UserDeleted?.Invoke(this, new GenericEventArgs<User> { Argument = user });
         }
 
         public NameIdPair[] GetAuthenticationProviders()
@@ -137,7 +165,7 @@ namespace Emby.Server.Implementations.Library
                 .ToArray();
         }
 
-        public void AddParts(IEnumerable<IAuthenticationProvider> authenticationProviders,IEnumerable<IPasswordResetProvider> passwordResetProviders)
+        public void AddParts(IEnumerable<IAuthenticationProvider> authenticationProviders, IEnumerable<IPasswordResetProvider> passwordResetProviders)
         {
             _authenticationProviders = authenticationProviders.ToArray();
 
@@ -150,54 +178,21 @@ namespace Emby.Server.Implementations.Library
             _defaultPasswordResetProvider = passwordResetProviders.OfType<DefaultPasswordResetProvider>().First();
         }
 
-        #region UserUpdated Event
         /// <summary>
-        /// Occurs when [user updated].
-        /// </summary>
-        public event EventHandler<GenericEventArgs<User>> UserUpdated;
-        public event EventHandler<GenericEventArgs<User>> UserPolicyUpdated;
-        public event EventHandler<GenericEventArgs<User>> UserConfigurationUpdated;
-        public event EventHandler<GenericEventArgs<User>> UserLockedOut;
-
-        /// <summary>
-        /// Called when [user updated].
-        /// </summary>
-        /// <param name="user">The user.</param>
-        private void OnUserUpdated(User user)
-        {
-            UserUpdated?.Invoke(this, new GenericEventArgs<User> { Argument = user });
-        }
-        #endregion
-
-        #region UserDeleted Event
-        /// <summary>
-        /// Occurs when [user deleted].
-        /// </summary>
-        public event EventHandler<GenericEventArgs<User>> UserDeleted;
-        /// <summary>
-        /// Called when [user deleted].
-        /// </summary>
-        /// <param name="user">The user.</param>
-        private void OnUserDeleted(User user)
-        {
-            UserDeleted?.Invoke(this, new GenericEventArgs<User> { Argument = user });
-        }
-        #endregion
-
-        /// <summary>
-        /// Gets a User by Id
+        /// Gets a User by Id.
         /// </summary>
         /// <param name="id">The id.</param>
         /// <returns>User.</returns>
-        /// <exception cref="ArgumentNullException"></exception>
+        /// <exception cref="ArgumentException"></exception>
         public User GetUserById(Guid id)
         {
             if (id == Guid.Empty)
             {
-                throw new ArgumentException(nameof(id), "Guid can't be empty");
+                throw new ArgumentException("Guid can't be empty", nameof(id));
             }
 
-            return Users.FirstOrDefault(u => u.Id == id);
+            _users.TryGetValue(id, out User user);
+            return user;
         }
 
         /// <summary>
@@ -206,15 +201,13 @@ namespace Emby.Server.Implementations.Library
         /// <param name="id">The identifier.</param>
         /// <returns>User.</returns>
         public User GetUserById(string id)
-        {
-            return GetUserById(new Guid(id));
-        }
+            => GetUserById(new Guid(id));
 
         public User GetUserByName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
             {
-                throw new ArgumentNullException(nameof(name));
+                throw new ArgumentException("Invalid username", nameof(name));
             }
 
             return Users.FirstOrDefault(u => string.Equals(u.Name, name, StringComparison.OrdinalIgnoreCase));
@@ -222,8 +215,9 @@ namespace Emby.Server.Implementations.Library
 
         public void Initialize()
         {
-            var users = LoadUsers();
-            _users = users.ToArray();
+            LoadUsers();
+
+            var users = Users;
 
             // If there are no local users with admin rights, make them all admins
             if (!users.Any(i => i.Policy.IsAdministrator))
@@ -240,14 +234,12 @@ namespace Emby.Server.Implementations.Library
         {
             // This is some regex that matches only on unicode "word" characters, as well as -, _ and @
             // In theory this will cut out most if not all 'control' characters which should help minimize any weirdness
-             // Usernames can contain letters (a-z + whatever else unicode is cool with), numbers (0-9), at-signs (@), dashes (-), underscores (_), apostrophes ('), and periods (.)
+            // Usernames can contain letters (a-z + whatever else unicode is cool with), numbers (0-9), at-signs (@), dashes (-), underscores (_), apostrophes ('), and periods (.)
             return Regex.IsMatch(username, @"^[\w\-'._@]*$");
         }
 
         private static bool IsValidUsernameCharacter(char i)
-        {
-            return IsValidUsername(i.ToString());
-        }
+            => IsValidUsername(i.ToString(CultureInfo.InvariantCulture));
 
         public string MakeValidUsername(string username)
         {
@@ -277,8 +269,7 @@ namespace Emby.Server.Implementations.Library
                 throw new ArgumentNullException(nameof(username));
             }
 
-            var user = Users
-                .FirstOrDefault(i => string.Equals(username, i.Name, StringComparison.OrdinalIgnoreCase));
+            var user = Users.FirstOrDefault(i => string.Equals(username, i.Name, StringComparison.OrdinalIgnoreCase));
 
             var success = false;
             string updatedUsername = null;
@@ -299,13 +290,12 @@ namespace Emby.Server.Implementations.Library
                 updatedUsername = authResult.username;
                 success = authResult.success;
 
-                if (success && authenticationProvider != null && !(authenticationProvider is DefaultAuthenticationProvider))
+                if (success
+                    && authenticationProvider != null
+                    && !(authenticationProvider is DefaultAuthenticationProvider))
                 {
                     // We should trust the user that the authprovider says, not what was typed
-                    if (updatedUsername != username)
-                    {
-                        username = updatedUsername;
-                    }
+                    username = updatedUsername;
 
                     // Search the database for the user again; the authprovider might have created it
                     user = Users
@@ -337,10 +327,11 @@ namespace Emby.Server.Implementations.Library
 
             if (user.Policy.IsDisabled)
             {
-                throw new AuthenticationException(string.Format(
-                    CultureInfo.InvariantCulture,
-                    "The {0} account is currently disabled. Please consult with your administrator.",
-                    user.Name));
+                throw new AuthenticationException(
+                    string.Format(
+                        CultureInfo.InvariantCulture,
+                        "The {0} account is currently disabled. Please consult with your administrator.",
+                        user.Name));
             }
 
             if (!user.Policy.EnableRemoteAccess && !_networkManager.IsInLocalNetwork(remoteEndPoint))
@@ -386,7 +377,7 @@ namespace Emby.Server.Implementations.Library
 
         private IAuthenticationProvider GetAuthenticationProvider(User user)
         {
-            return GetAuthenticationProviders(user).First();
+            return GetAuthenticationProviders(user)[0];
         }
 
         private IPasswordResetProvider GetPasswordResetProvider(User user)
@@ -396,7 +387,7 @@ namespace Emby.Server.Implementations.Library
 
         private IAuthenticationProvider[] GetAuthenticationProviders(User user)
         {
-            var authenticationProviderId = user == null ? null : user.Policy.AuthenticationProviderId;
+            var authenticationProviderId = user?.Policy.AuthenticationProviderId;
 
             var providers = _authenticationProviders.Where(i => i.IsEnabled).ToArray();
 
@@ -438,16 +429,10 @@ namespace Emby.Server.Implementations.Library
         {
             try
             {
-                var requiresResolvedUser = provider as IRequiresResolvedUser;
-                ProviderAuthenticationResult authenticationResult = null;
-                if (requiresResolvedUser != null)
-                {
-                    authenticationResult = await requiresResolvedUser.Authenticate(username, password, resolvedUser).ConfigureAwait(false);
-                }
-                else
-                {
-                    authenticationResult = await provider.Authenticate(username, password).ConfigureAwait(false);
-                }
+
+                var authenticationResult = provider is IRequiresResolvedUser requiresResolvedUser
+                    ? await requiresResolvedUser.Authenticate(username, password, resolvedUser).ConfigureAwait(false)
+                    : await provider.Authenticate(username, password).ConfigureAwait(false);
 
                 if (authenticationResult.Username != username)
                 {
@@ -467,7 +452,6 @@ namespace Emby.Server.Implementations.Library
 
         private async Task<(IAuthenticationProvider authenticationProvider, string username, bool success)> AuthenticateLocalUser(string username, string password, string hashedPassword, User user, string remoteEndPoint)
         {
-            string updatedUsername = null;
             bool success = false;
             IAuthenticationProvider authenticationProvider = null;
 
@@ -487,7 +471,7 @@ namespace Emby.Server.Implementations.Library
                 foreach (var provider in GetAuthenticationProviders(user))
                 {
                     var providerAuthResult = await AuthenticateWithProvider(provider, username, password, user).ConfigureAwait(false);
-                    updatedUsername = providerAuthResult.username;
+                    var updatedUsername = providerAuthResult.username;
                     success = providerAuthResult.success;
 
                     if (success)
@@ -499,25 +483,32 @@ namespace Emby.Server.Implementations.Library
                 }
             }
 
-            if (user != null)
+            if (user != null
+                && !success
+                && _networkManager.IsInLocalNetwork(remoteEndPoint)
+                && user.Configuration.EnableLocalPassword)
             {
-                if (!success && _networkManager.IsInLocalNetwork(remoteEndPoint) && user.Configuration.EnableLocalPassword)
+                if (password == null)
                 {
-                    if (password == null)
-                    {
-                        // legacy
-                        success = string.Equals(GetAuthenticationProvider(user).GetEasyPasswordHash(user), hashedPassword.Replace("-", string.Empty), StringComparison.OrdinalIgnoreCase);
-                    }
-                    else
-                    {
-                        success = string.Equals(GetAuthenticationProvider(user).GetEasyPasswordHash(user), _defaultAuthenticationProvider.GetHashedString(user, password), StringComparison.OrdinalIgnoreCase);
-                    }
+                    // legacy
+                    success = string.Equals(GetLocalPasswordHash(user), hashedPassword.Replace("-", string.Empty), StringComparison.OrdinalIgnoreCase);
+                }
+                else
+                {
+                    success = string.Equals(GetLocalPasswordHash(user), _defaultAuthenticationProvider.GetHashedString(user, password), StringComparison.OrdinalIgnoreCase);
                 }
             }
 
             return (authenticationProvider, username, success);
         }
 
+        private string GetLocalPasswordHash(User user)
+        {
+            return string.IsNullOrEmpty(user.EasyPassword)
+                ? null
+                : PasswordHash.ConvertToByteString(new PasswordHash(user.EasyPassword).Hash);
+        }
+
         private void UpdateInvalidLoginAttemptCount(User user, int newValue)
         {
             if (user.Policy.InvalidLoginAttemptCount == newValue || newValue <= 0)
@@ -556,17 +547,17 @@ namespace Emby.Server.Implementations.Library
         }
 
         /// <summary>
-        /// Loads the users from the repository
+        /// Loads the users from the repository.
         /// </summary>
-        /// <returns>IEnumerable{User}.</returns>
-        private List<User> LoadUsers()
+        private void LoadUsers()
         {
-            var users = UserRepository.RetrieveAllUsers();
+            var users = _userRepository.RetrieveAllUsers();
 
             // There always has to be at least one user.
             if (users.Count != 0)
             {
-                return users;
+                _users = new ConcurrentDictionary<Guid, User>(
+                    users.Select(x => new KeyValuePair<Guid, User>(x.Id, x)));
             }
 
             var defaultName = Environment.UserName;
@@ -581,14 +572,15 @@ namespace Emby.Server.Implementations.Library
 
             user.DateLastSaved = DateTime.UtcNow;
 
-            UserRepository.CreateUser(user);
+            _userRepository.CreateUser(user);
 
             user.Policy.IsAdministrator = true;
             user.Policy.EnableContentDeletion = true;
             user.Policy.EnableRemoteControlOfOtherUsers = true;
             UpdateUserPolicy(user, user.Policy, false);
 
-            return new List<User> { user };
+            _users = new ConcurrentDictionary<Guid, User>();
+            _users[user.Id] = user;
         }
 
         public UserDto GetUserDto(User user, string remoteEndPoint = null)
@@ -619,7 +611,7 @@ namespace Emby.Server.Implementations.Library
                 Policy = user.Policy
             };
 
-            if (!hasPassword && Users.Count() == 1)
+            if (!hasPassword && _users.Count == 1)
             {
                 dto.EnableAutoLogin = true;
             }
@@ -694,22 +686,26 @@ namespace Emby.Server.Implementations.Library
                 throw new ArgumentNullException(nameof(user));
             }
 
-            if (string.IsNullOrEmpty(newName))
+            if (string.IsNullOrWhiteSpace(newName))
             {
-                throw new ArgumentNullException(nameof(newName));
+                throw new ArgumentException("Invalid username", nameof(newName));
             }
 
-            if (Users.Any(u => u.Id != user.Id && u.Name.Equals(newName, StringComparison.OrdinalIgnoreCase)))
+            if (user.Name.Equals(newName, StringComparison.OrdinalIgnoreCase))
             {
-                throw new ArgumentException(string.Format("A user with the name '{0}' already exists.", newName));
+                throw new ArgumentException("The new and old names must be different.");
             }
 
-            if (user.Name.Equals(newName, StringComparison.Ordinal))
+            if (Users.Any(
+                u => u.Id != user.Id && u.Name.Equals(newName, StringComparison.OrdinalIgnoreCase)))
             {
-                throw new ArgumentException("The new and old names must be different.");
+                throw new ArgumentException(string.Format(
+                    CultureInfo.InvariantCulture,
+                    "A user with the name '{0}' already exists.",
+                    newName));
             }
 
-            await user.Rename(newName);
+            await user.Rename(newName).ConfigureAwait(false);
 
             OnUserUpdated(user);
         }
@@ -727,23 +723,30 @@ namespace Emby.Server.Implementations.Library
                 throw new ArgumentNullException(nameof(user));
             }
 
-            if (user.Id.Equals(Guid.Empty) || !Users.Any(u => u.Id.Equals(user.Id)))
+            if (user.Id == Guid.Empty)
             {
-                throw new ArgumentException(string.Format("User with name '{0}' and Id {1} does not exist.", user.Name, user.Id));
+                throw new ArgumentException("Id can't be empty.", nameof(user));
+            }
+
+            if (!_users.ContainsKey(user.Id))
+            {
+                throw new ArgumentException(
+                    string.Format(
+                        CultureInfo.InvariantCulture,
+                        "A user '{0}' with Id {1} does not exist.",
+                        user.Name,
+                        user.Id),
+                    nameof(user));
             }
 
             user.DateModified = DateTime.UtcNow;
             user.DateLastSaved = DateTime.UtcNow;
 
-            UserRepository.UpdateUser(user);
+            _userRepository.UpdateUser(user);
 
             OnUserUpdated(user);
         }
 
-        public event EventHandler<GenericEventArgs<User>> UserCreated;
-
-        private readonly SemaphoreSlim _userListLock = new SemaphoreSlim(1, 1);
-
         /// <summary>
         /// Creates the user.
         /// </summary>
@@ -751,7 +754,7 @@ namespace Emby.Server.Implementations.Library
         /// <returns>User.</returns>
         /// <exception cref="ArgumentNullException">name</exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task<User> CreateUser(string name)
+        public User CreateUser(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
             {
@@ -768,28 +771,17 @@ namespace Emby.Server.Implementations.Library
                 throw new ArgumentException(string.Format("A user with the name '{0}' already exists.", name));
             }
 
-            await _userListLock.WaitAsync(CancellationToken.None).ConfigureAwait(false);
-
-            try
-            {
-                var user = InstantiateNewUser(name);
+            var user = InstantiateNewUser(name);
 
-                var list = Users.ToList();
-                list.Add(user);
-                _users = list.ToArray();
+            _users[user.Id] = user;
 
-                user.DateLastSaved = DateTime.UtcNow;
+            user.DateLastSaved = DateTime.UtcNow;
 
-                UserRepository.CreateUser(user);
+            _userRepository.CreateUser(user);
 
-                EventHelper.QueueEventIfNotNull(UserCreated, this, new GenericEventArgs<User> { Argument = user }, _logger);
+            EventHelper.QueueEventIfNotNull(UserCreated, this, new GenericEventArgs<User> { Argument = user }, _logger);
 
-                return user;
-            }
-            finally
-            {
-                _userListLock.Release();
-            }
+            return user;
         }
 
         /// <summary>
@@ -799,57 +791,59 @@ namespace Emby.Server.Implementations.Library
         /// <returns>Task.</returns>
         /// <exception cref="ArgumentNullException">user</exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task DeleteUser(User user)
+        public void DeleteUser(User user)
         {
             if (user == null)
             {
                 throw new ArgumentNullException(nameof(user));
             }
 
-            var allUsers = Users.ToList();
-
-            if (allUsers.FirstOrDefault(u => u.Id == user.Id) == null)
+            if (!_users.ContainsKey(user.Id))
             {
-                throw new ArgumentException(string.Format("The user cannot be deleted because there is no user with the Name {0} and Id {1}.", user.Name, user.Id));
+                throw new ArgumentException(string.Format(
+                    CultureInfo.InvariantCulture,
+                    "The user cannot be deleted because there is no user with the Name {0} and Id {1}.",
+                    user.Name,
+                    user.Id));
             }
 
-            if (allUsers.Count == 1)
+            if (_users.Count == 1)
             {
-                throw new ArgumentException(string.Format("The user '{0}' cannot be deleted because there must be at least one user in the system.", user.Name));
+                throw new ArgumentException(string.Format(
+                    CultureInfo.InvariantCulture,
+                    "The user '{0}' cannot be deleted because there must be at least one user in the system.",
+                    user.Name));
             }
 
-            if (user.Policy.IsAdministrator && allUsers.Count(i => i.Policy.IsAdministrator) == 1)
+            if (user.Policy.IsAdministrator
+                && Users.Count(i => i.Policy.IsAdministrator) == 1)
             {
-                throw new ArgumentException(string.Format("The user '{0}' cannot be deleted because there must be at least one admin user in the system.", user.Name));
+                throw new ArgumentException(
+                    string.Format(
+                        CultureInfo.InvariantCulture,
+                        "The user '{0}' cannot be deleted because there must be at least one admin user in the system.",
+                        user.Name),
+                    nameof(user));
             }
 
-            await _userListLock.WaitAsync(CancellationToken.None).ConfigureAwait(false);
+            var configPath = GetConfigurationFilePath(user);
+
+            _userRepository.DeleteUser(user);
 
             try
             {
-                var configPath = GetConfigurationFilePath(user);
-
-                UserRepository.DeleteUser(user);
-
-                try
-                {
-                    _fileSystem.DeleteFile(configPath);
-                }
-                catch (IOException ex)
-                {
-                    _logger.LogError(ex, "Error deleting file {path}", configPath);
-                }
-
-                DeleteUserPolicy(user);
-
-                _users = allUsers.Where(i => i.Id != user.Id).ToArray();
-
-                OnUserDeleted(user);
+                _fileSystem.DeleteFile(configPath);
             }
-            finally
+            catch (IOException ex)
             {
-                _userListLock.Release();
+                _logger.LogError(ex, "Error deleting file {path}", configPath);
             }
+
+            DeleteUserPolicy(user);
+
+            _users.TryRemove(user.Id, out _);
+
+            OnUserDeleted(user);
         }
 
         /// <summary>
@@ -906,8 +900,7 @@ namespace Emby.Server.Implementations.Library
                 Name = name,
                 Id = Guid.NewGuid(),
                 DateCreated = DateTime.UtcNow,
-                DateModified = DateTime.UtcNow,
-                UsesIdForConfigurationPath = true
+                DateModified = DateTime.UtcNow
             };
         }
 
@@ -989,7 +982,6 @@ namespace Emby.Server.Implementations.Library
             };
         }
 
-        private readonly object _policySyncLock = new object();
         public void UpdateUserPolicy(Guid userId, UserPolicy userPolicy)
         {
             var user = GetUserById(userId);
diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs
index 0347100a4..61329160a 100644
--- a/Emby.Server.Implementations/Session/SessionManager.cs
+++ b/Emby.Server.Implementations/Session/SessionManager.cs
@@ -1375,16 +1375,14 @@ namespace Emby.Server.Implementations.Session
             CheckDisposed();
 
             User user = null;
-            if (!request.UserId.Equals(Guid.Empty))
+            if (request.UserId != Guid.Empty)
             {
-                user = _userManager.Users
-                    .FirstOrDefault(i => i.Id == request.UserId);
+                user = _userManager.GetUserById(request.UserId);
             }
 
             if (user == null)
             {
-                user = _userManager.Users
-                    .FirstOrDefault(i => string.Equals(request.Username, i.Name, StringComparison.OrdinalIgnoreCase));
+                user = _userManager.GetUserByName(request.Username);
             }
 
             if (user != null)
diff --git a/MediaBrowser.Api/UserService.cs b/MediaBrowser.Api/UserService.cs
index fa70a52aa..21a94a4e0 100644
--- a/MediaBrowser.Api/UserService.cs
+++ b/MediaBrowser.Api/UserService.cs
@@ -365,8 +365,8 @@ namespace MediaBrowser.Api
             }
 
             _sessionMananger.RevokeUserTokens(user.Id, null);
-
-            return _userManager.DeleteUser(user);
+            _userManager.DeleteUser(user);
+            return Task.CompletedTask;
         }
 
         /// <summary>
@@ -503,9 +503,14 @@ namespace MediaBrowser.Api
             }
         }
 
+        /// <summary>
+        /// Posts the specified request.
+        /// </summary>
+        /// <param name="request">The request.</param>
+        /// <returns>System.Object.</returns>
         public async Task<object> Post(CreateUserByName request)
         {
-            var newUser = await _userManager.CreateUser(request.Name).ConfigureAwait(false);
+            var newUser = _userManager.CreateUser(request.Name);
 
             // no need to authenticate password for new user
             if (request.Password != null)
diff --git a/MediaBrowser.Controller/Authentication/AuthenticationException.cs b/MediaBrowser.Controller/Authentication/AuthenticationException.cs
index 045cbcdae..62eca3ea9 100644
--- a/MediaBrowser.Controller/Authentication/AuthenticationException.cs
+++ b/MediaBrowser.Controller/Authentication/AuthenticationException.cs
@@ -1,4 +1,5 @@
 using System;
+
 namespace MediaBrowser.Controller.Authentication
 {
     /// <summary>
diff --git a/MediaBrowser.Controller/Entities/User.cs b/MediaBrowser.Controller/Entities/User.cs
index 968d72579..7d245d4aa 100644
--- a/MediaBrowser.Controller/Entities/User.cs
+++ b/MediaBrowser.Controller/Entities/User.cs
@@ -17,13 +17,6 @@ namespace MediaBrowser.Controller.Entities
     public class User : BaseItem
     {
         public static IUserManager UserManager { get; set; }
-        public static IXmlSerializer XmlSerializer { get; set; }
-
-        /// <summary>
-        /// From now on all user paths will be Id-based.
-        /// This is for backwards compatibility.
-        /// </summary>
-        public bool UsesIdForConfigurationPath { get; set; }
 
         /// <summary>
         /// Gets or sets the password.
@@ -31,7 +24,6 @@ namespace MediaBrowser.Controller.Entities
         /// <value>The password.</value>
         public string Password { get; set; }
         public string EasyPassword { get; set; }
-        public string Salt { get; set; }
 
         // Strictly to remove IgnoreDataMember
         public override ItemImageInfo[] ImageInfos
@@ -148,46 +140,23 @@ namespace MediaBrowser.Controller.Entities
         /// <exception cref="ArgumentNullException"></exception>
         public Task Rename(string newName)
         {
-            if (string.IsNullOrEmpty(newName))
-            {
-                throw new ArgumentNullException(nameof(newName));
-            }
-
-            // If only the casing is changing, leave the file system alone
-            if (!UsesIdForConfigurationPath && !string.Equals(newName, Name, StringComparison.OrdinalIgnoreCase))
+            if (string.IsNullOrWhiteSpace(newName))
             {
-                UsesIdForConfigurationPath = true;
-
-                // Move configuration
-                var newConfigDirectory = GetConfigurationDirectoryPath(newName);
-                var oldConfigurationDirectory = ConfigurationDirectoryPath;
-
-                // Exceptions will be thrown if these paths already exist
-                if (Directory.Exists(newConfigDirectory))
-                {
-                    Directory.Delete(newConfigDirectory, true);
-                }
-
-                if (Directory.Exists(oldConfigurationDirectory))
-                {
-                    Directory.Move(oldConfigurationDirectory, newConfigDirectory);
-                }
-                else
-                {
-                    Directory.CreateDirectory(newConfigDirectory);
-                }
+                throw new ArgumentException("Username can't be empty", nameof(newName));
             }
 
             Name = newName;
 
-            return RefreshMetadata(new MetadataRefreshOptions(new DirectoryService(Logger, FileSystem))
-            {
-                ReplaceAllMetadata = true,
-                ImageRefreshMode = MetadataRefreshMode.FullRefresh,
-                MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
-                ForceSave = true
+            return RefreshMetadata(
+                new MetadataRefreshOptions(new DirectoryService(Logger, FileSystem))
+                {
+                    ReplaceAllMetadata = true,
+                    ImageRefreshMode = MetadataRefreshMode.FullRefresh,
+                    MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+                    ForceSave = true
 
-            }, CancellationToken.None);
+                },
+                CancellationToken.None);
         }
 
         public override void UpdateToRepository(ItemUpdateType updateReason, CancellationToken cancellationToken)
@@ -216,19 +185,6 @@ namespace MediaBrowser.Controller.Entities
         {
             var parentPath = ConfigurationManager.ApplicationPaths.UserConfigurationDirectoryPath;
 
-            // Legacy
-            if (!UsesIdForConfigurationPath)
-            {
-                if (string.IsNullOrEmpty(username))
-                {
-                    throw new ArgumentNullException(nameof(username));
-                }
-
-                var safeFolderName = FileSystem.GetValidFilename(username);
-
-                return System.IO.Path.Combine(ConfigurationManager.ApplicationPaths.UserConfigurationDirectoryPath, safeFolderName);
-            }
-
             // TODO: Remove idPath and just use usernamePath for future releases
             var usernamePath = System.IO.Path.Combine(parentPath, username);
             var idPath = System.IO.Path.Combine(parentPath, Id.ToString("N", CultureInfo.InvariantCulture));
diff --git a/MediaBrowser.Controller/Library/IUserManager.cs b/MediaBrowser.Controller/Library/IUserManager.cs
index 7f7370893..bbedc0442 100644
--- a/MediaBrowser.Controller/Library/IUserManager.cs
+++ b/MediaBrowser.Controller/Library/IUserManager.cs
@@ -23,6 +23,12 @@ namespace MediaBrowser.Controller.Library
         IEnumerable<User> Users { get; }
 
         /// <summary>
+        /// Gets the user ids.
+        /// </summary>
+        /// <value>The users ids.</value>
+        IEnumerable<Guid> UsersIds { get; }
+
+        /// <summary>
         /// Occurs when [user updated].
         /// </summary>
         event EventHandler<GenericEventArgs<User>> UserUpdated;
@@ -92,7 +98,7 @@ namespace MediaBrowser.Controller.Library
         /// <returns>User.</returns>
         /// <exception cref="ArgumentNullException">name</exception>
         /// <exception cref="ArgumentException"></exception>
-        Task<User> CreateUser(string name);
+        User CreateUser(string name);
 
         /// <summary>
         /// Deletes the user.
@@ -101,7 +107,7 @@ namespace MediaBrowser.Controller.Library
         /// <returns>Task.</returns>
         /// <exception cref="ArgumentNullException">user</exception>
         /// <exception cref="ArgumentException"></exception>
-        Task DeleteUser(User user);
+        void DeleteUser(User user);
 
         /// <summary>
         /// Resets the password.