Apply suggestions from code review

Adding minor stylistic suggestions from Bond-009

Co-Authored-By: LogicalPhallacy <44458166+LogicalPhallacy@users.noreply.github.com>

3 files changed, 264 insertions, 255 deletions

diff --git a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
index 7817989e7..436443f06 100644
--- a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
+++ b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
@@ -72,7 +72,7 @@ namespace Emby.Server.Implementations.Cryptography
         }

 

         private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations)

-        {
+        {

             using (var r = new Rfc2898DeriveBytes(bytes, salt, iterations, new HashAlgorithmName(method)))

             {

                 return r.GetBytes(32);

@@ -107,9 +107,9 @@ namespace Emby.Server.Implementations.Cryptography
             }

             else

             {

-                throw new CryptographicException(String.Format("Requested hash method is not supported: {0}", HashMethod));

+                throw new CryptographicException($"Requested hash method is not supported: {HashMethod}"));

             }

-        }
+        }

 

         public byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt)

         {

@@ -117,25 +117,32 @@ namespace Emby.Server.Implementations.Cryptography
         }

         

         public byte[] ComputeHash(PasswordHash hash)

-        {
-            int iterations = defaultiterations;
-            if (!hash.Parameters.ContainsKey("iterations"))
-            {
-                hash.Parameters.Add("iterations", defaultiterations.ToString());
-            }
-            else
-            {
-                try { iterations = int.Parse(hash.Parameters["iterations"]); }
-                catch (Exception e) { iterations = defaultiterations; throw new Exception($"Couldn't successfully parse iterations value from string:{hash.Parameters["iterations"]}", e); }
+        {

+            int iterations = defaultiterations;

+            if (!hash.Parameters.ContainsKey("iterations"))

+            {

+                hash.Parameters.Add("iterations", defaultiterations.ToString(CultureInfo.InvariantCulture));

+            }

+            else

+            {

+                try

+                {

+                    iterations = int.Parse(hash.Parameters["iterations"]);

+                }

+                catch (Exception e)

+                {

+                    iterations = defaultiterations;

+                    throw new Exception($"Couldn't successfully parse iterations value from string:{hash.Parameters["iterations"]}", e);

+                }

             }

-            return PBKDF2(hash.Id, hash.HashBytes, hash.SaltBytes,iterations);

-        }
-
+            return PBKDF2(hash.Id, hash.HashBytes, hash.SaltBytes, iterations);

+        }

+

         public byte[] GenerateSalt()

         {

             byte[] salt = new byte[64];

             rng.GetBytes(salt);

             return salt;

-        }
+        }

     }

 }

diff --git a/Emby.Server.Implementations/Data/SqliteUserRepository.cs b/Emby.Server.Implementations/Data/SqliteUserRepository.cs
index b3d457342..1b6deae7d 100644
--- a/Emby.Server.Implementations/Data/SqliteUserRepository.cs
+++ b/Emby.Server.Implementations/Data/SqliteUserRepository.cs
@@ -1,85 +1,86 @@
-using System;
-using System.Collections.Generic;
-using System.IO;
-using MediaBrowser.Controller;
-using MediaBrowser.Controller.Entities;
-using MediaBrowser.Controller.Persistence;
-using MediaBrowser.Model.Serialization;
-using Microsoft.Extensions.Logging;
-using SQLitePCL.pretty;
-
-namespace Emby.Server.Implementations.Data
-{
-    /// <summary>
-    /// Class SQLiteUserRepository
-    /// </summary>
-    public class SqliteUserRepository : BaseSqliteRepository, IUserRepository
-    {
-        private readonly IJsonSerializer _jsonSerializer;
-
-        public SqliteUserRepository(
-            ILoggerFactory loggerFactory,
-            IServerApplicationPaths appPaths,
-            IJsonSerializer jsonSerializer)
-            : base(loggerFactory.CreateLogger(nameof(SqliteUserRepository)))
-        {
-            _jsonSerializer = jsonSerializer;
-
-            DbFilePath = Path.Combine(appPaths.DataPath, "users.db");
-        }
-
-        /// <summary>
-        /// Gets the name of the repository
-        /// </summary>
-        /// <value>The name.</value>
-        public string Name => "SQLite";
-
-        /// <summary>
-        /// Opens the connection to the database
-        /// </summary>
-        /// <returns>Task.</returns>
-        public void Initialize()
-        {
-            using (var connection = CreateConnection())
-            {
-                RunDefaultInitialization(connection);
-
-                var localUsersTableExists = TableExists(connection, "LocalUsersv2");
-
-                connection.RunQueries(new[] {
-                    "create table if not exists LocalUsersv2 (Id INTEGER PRIMARY KEY, guid GUID NOT NULL, data BLOB NOT NULL)",
-                    "drop index if exists idx_users"
-                });
-
-                if (!localUsersTableExists && TableExists(connection, "Users"))
-                {
-                    TryMigrateToLocalUsersTable(connection);
-                }
-                RemoveEmptyPasswordHashes();
-            }
-        }
-
-        private void TryMigrateToLocalUsersTable(ManagedConnection connection)
-        {
-            try
-            {
-                connection.RunQueries(new[]
-                {
-                    "INSERT INTO LocalUsersv2 (guid, data) SELECT guid,data from users"
-                });
-            }
-            catch (Exception ex)
-            {
-                Logger.LogError(ex, "Error migrating users database");
-            }
-        }
-
+using System;

+using System.Collections.Generic;

+using System.IO;

+using MediaBrowser.Controller;

+using MediaBrowser.Controller.Entities;

+using MediaBrowser.Controller.Persistence;

+using MediaBrowser.Model.Serialization;

+using Microsoft.Extensions.Logging;

+using SQLitePCL.pretty;

+

+namespace Emby.Server.Implementations.Data

+{

+    /// <summary>

+    /// Class SQLiteUserRepository

+    /// </summary>

+    public class SqliteUserRepository : BaseSqliteRepository, IUserRepository

+    {

+        private readonly IJsonSerializer _jsonSerializer;

+

+        public SqliteUserRepository(

+            ILoggerFactory loggerFactory,

+            IServerApplicationPaths appPaths,

+            IJsonSerializer jsonSerializer)

+            : base(loggerFactory.CreateLogger(nameof(SqliteUserRepository)))

+        {

+            _jsonSerializer = jsonSerializer;

+

+            DbFilePath = Path.Combine(appPaths.DataPath, "users.db");

+        }

+

+        /// <summary>

+        /// Gets the name of the repository

+        /// </summary>

+        /// <value>The name.</value>

+        public string Name => "SQLite";

+

+        /// <summary>

+        /// Opens the connection to the database

+        /// </summary>

+        /// <returns>Task.</returns>

+        public void Initialize()

+        {

+            using (var connection = CreateConnection())

+            {

+                RunDefaultInitialization(connection);

+

+                var localUsersTableExists = TableExists(connection, "LocalUsersv2");

+

+                connection.RunQueries(new[] {

+                    "create table if not exists LocalUsersv2 (Id INTEGER PRIMARY KEY, guid GUID NOT NULL, data BLOB NOT NULL)",

+                    "drop index if exists idx_users"

+                });

+

+                if (!localUsersTableExists && TableExists(connection, "Users"))

+                {

+                    TryMigrateToLocalUsersTable(connection);

+                }

+                RemoveEmptyPasswordHashes();

+            }

+        }

+

+        private void TryMigrateToLocalUsersTable(ManagedConnection connection)

+        {

+            try

+            {

+                connection.RunQueries(new[]

+                {

+                    "INSERT INTO LocalUsersv2 (guid, data) SELECT guid,data from users"

+                });

+            }

+            catch (Exception ex)

+            {

+                Logger.LogError(ex, "Error migrating users database");

+            }

+        }

+

         private void RemoveEmptyPasswordHashes()

         {

             foreach (var user in RetrieveAllUsers())

             {

                 // If the user password is the sha1 hash of the empty string, remove it

-                if (!string.Equals(user.Password, "DA39A3EE5E6B4B0D3255BFEF95601890AFD80709") || !string.Equals(user.Password, "$SHA1$DA39A3EE5E6B4B0D3255BFEF95601890AFD80709"))

+                if (!string.Equals(user.Password, "DA39A3EE5E6B4B0D3255BFEF95601890AFD80709", StringComparison.Ordinal)

+                    || !string.Equals(user.Password, "$SHA1$DA39A3EE5E6B4B0D3255BFEF95601890AFD80709", StringComparison.Ordinal))

                 {

                     continue;

                 }

@@ -103,160 +104,160 @@ namespace Emby.Server.Implementations.Data
                 }

             }

 

-        }
-
-        /// <summary>
-        /// Save a user in the repo
-        /// </summary>
-        public void CreateUser(User user)
-        {
-            if (user == null)
-            {
-                throw new ArgumentNullException(nameof(user));
-            }
-
-            var serialized = _jsonSerializer.SerializeToBytes(user);
-
-            using (WriteLock.Write())
-            {
-                using (var connection = CreateConnection())
-                {
-                    connection.RunInTransaction(db =>
-                    {
-                        using (var statement = db.PrepareStatement("insert into LocalUsersv2 (guid, data) values (@guid, @data)"))
-                        {
-                            statement.TryBind("@guid", user.Id.ToGuidBlob());
-                            statement.TryBind("@data", serialized);
-
-                            statement.MoveNext();
-                        }
-
-                        var createdUser = GetUser(user.Id, false);
-
-                        if (createdUser == null)
-                        {
-                            throw new ApplicationException("created user should never be null");
-                        }
-
-                        user.InternalId = createdUser.InternalId;
-
-                    }, TransactionMode);
-                }
-            }
-        }
-
-        public void UpdateUser(User user)
-        {
-            if (user == null)
-            {
-                throw new ArgumentNullException(nameof(user));
-            }
-
-            var serialized = _jsonSerializer.SerializeToBytes(user);
-
-            using (WriteLock.Write())
-            {
-                using (var connection = CreateConnection())
-                {
-                    connection.RunInTransaction(db =>
-                    {
-                        using (var statement = db.PrepareStatement("update LocalUsersv2 set data=@data where Id=@InternalId"))
-                        {
-                            statement.TryBind("@InternalId", user.InternalId);
-                            statement.TryBind("@data", serialized);
-                            statement.MoveNext();
-                        }
-
-                    }, TransactionMode);
-                }
-            }
-        }
-
-        private User GetUser(Guid guid, bool openLock)
-        {
-            using (openLock ? WriteLock.Read() : null)
-            {
-                using (var connection = CreateConnection(true))
-                {
-                    using (var statement = connection.PrepareStatement("select id,guid,data from LocalUsersv2 where guid=@guid"))
-                    {
-                        statement.TryBind("@guid", guid);
-
-                        foreach (var row in statement.ExecuteQuery())
-                        {
-                            return GetUser(row);
-                        }
-                    }
-                }
-            }
-
-            return null;
-        }
-
-        private User GetUser(IReadOnlyList<IResultSetValue> row)
-        {
-            var id = row[0].ToInt64();
-            var guid = row[1].ReadGuidFromBlob();
-
-            using (var stream = new MemoryStream(row[2].ToBlob()))
-            {
-                stream.Position = 0;
-                var user = _jsonSerializer.DeserializeFromStream<User>(stream);
-                user.InternalId = id;
-                user.Id = guid;
-                return user;
-            }
-        }
-
-        /// <summary>
-        /// Retrieve all users from the database
-        /// </summary>
-        /// <returns>IEnumerable{User}.</returns>
-        public List<User> RetrieveAllUsers()
-        {
-            var list = new List<User>();
-
-            using (WriteLock.Read())
-            {
-                using (var connection = CreateConnection(true))
-                {
-                    foreach (var row in connection.Query("select id,guid,data from LocalUsersv2"))
-                    {
-                        list.Add(GetUser(row));
-                    }
-                }
-            }
-
-            return list;
-        }
-
-        /// <summary>
-        /// Deletes the user.
-        /// </summary>
-        /// <param name="user">The user.</param>
-        /// <returns>Task.</returns>
-        /// <exception cref="ArgumentNullException">user</exception>
-        public void DeleteUser(User user)
-        {
-            if (user == null)
-            {
-                throw new ArgumentNullException(nameof(user));
-            }
-
-            using (WriteLock.Write())
-            {
-                using (var connection = CreateConnection())
-                {
-                    connection.RunInTransaction(db =>
-                    {
-                        using (var statement = db.PrepareStatement("delete from LocalUsersv2 where Id=@id"))
-                        {
-                            statement.TryBind("@id", user.InternalId);
-                            statement.MoveNext();
-                        }
-                    }, TransactionMode);
-                }
-            }
-        }
-    }
-}
+        }

+

+        /// <summary>

+        /// Save a user in the repo

+        /// </summary>

+        public void CreateUser(User user)

+        {

+            if (user == null)

+            {

+                throw new ArgumentNullException(nameof(user));

+            }

+

+            var serialized = _jsonSerializer.SerializeToBytes(user);

+

+            using (WriteLock.Write())

+            {

+                using (var connection = CreateConnection())

+                {

+                    connection.RunInTransaction(db =>

+                    {

+                        using (var statement = db.PrepareStatement("insert into LocalUsersv2 (guid, data) values (@guid, @data)"))

+                        {

+                            statement.TryBind("@guid", user.Id.ToGuidBlob());

+                            statement.TryBind("@data", serialized);

+

+                            statement.MoveNext();

+                        }

+

+                        var createdUser = GetUser(user.Id, false);

+

+                        if (createdUser == null)

+                        {

+                            throw new ApplicationException("created user should never be null");

+                        }

+

+                        user.InternalId = createdUser.InternalId;

+

+                    }, TransactionMode);

+                }

+            }

+        }

+

+        public void UpdateUser(User user)

+        {

+            if (user == null)

+            {

+                throw new ArgumentNullException(nameof(user));

+            }

+

+            var serialized = _jsonSerializer.SerializeToBytes(user);

+

+            using (WriteLock.Write())

+            {

+                using (var connection = CreateConnection())

+                {

+                    connection.RunInTransaction(db =>

+                    {

+                        using (var statement = db.PrepareStatement("update LocalUsersv2 set data=@data where Id=@InternalId"))

+                        {

+                            statement.TryBind("@InternalId", user.InternalId);

+                            statement.TryBind("@data", serialized);

+                            statement.MoveNext();

+                        }

+

+                    }, TransactionMode);

+                }

+            }

+        }

+

+        private User GetUser(Guid guid, bool openLock)

+        {

+            using (openLock ? WriteLock.Read() : null)

+            {

+                using (var connection = CreateConnection(true))

+                {

+                    using (var statement = connection.PrepareStatement("select id,guid,data from LocalUsersv2 where guid=@guid"))

+                    {

+                        statement.TryBind("@guid", guid);

+

+                        foreach (var row in statement.ExecuteQuery())

+                        {

+                            return GetUser(row);

+                        }

+                    }

+                }

+            }

+

+            return null;

+        }

+

+        private User GetUser(IReadOnlyList<IResultSetValue> row)

+        {

+            var id = row[0].ToInt64();

+            var guid = row[1].ReadGuidFromBlob();

+

+            using (var stream = new MemoryStream(row[2].ToBlob()))

+            {

+                stream.Position = 0;

+                var user = _jsonSerializer.DeserializeFromStream<User>(stream);

+                user.InternalId = id;

+                user.Id = guid;

+                return user;

+            }

+        }

+

+        /// <summary>

+        /// Retrieve all users from the database

+        /// </summary>

+        /// <returns>IEnumerable{User}.</returns>

+        public List<User> RetrieveAllUsers()

+        {

+            var list = new List<User>();

+

+            using (WriteLock.Read())

+            {

+                using (var connection = CreateConnection(true))

+                {

+                    foreach (var row in connection.Query("select id,guid,data from LocalUsersv2"))

+                    {

+                        list.Add(GetUser(row));

+                    }

+                }

+            }

+

+            return list;

+        }

+

+        /// <summary>

+        /// Deletes the user.

+        /// </summary>

+        /// <param name="user">The user.</param>

+        /// <returns>Task.</returns>

+        /// <exception cref="ArgumentNullException">user</exception>

+        public void DeleteUser(User user)

+        {

+            if (user == null)

+            {

+                throw new ArgumentNullException(nameof(user));

+            }

+

+            using (WriteLock.Write())

+            {

+                using (var connection = CreateConnection())

+                {

+                    connection.RunInTransaction(db =>

+                    {

+                        using (var statement = db.PrepareStatement("delete from LocalUsersv2 where Id=@id"))

+                        {

+                            statement.TryBind("@id", user.InternalId);

+                            statement.MoveNext();

+                        }

+                    }, TransactionMode);

+                }

+            }

+        }

+    }

+}

diff --git a/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs b/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs
index 33428c05e..016de6db7 100644
--- a/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs
+++ b/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs
@@ -44,7 +44,7 @@ namespace Emby.Server.Implementations.Library
             PasswordHash readyHash = new PasswordHash(resolvedUser.Password);

             byte[] CalculatedHash;

             string CalculatedHashString;

-            if (_cryptographyProvider.GetSupportedHashMethods().Any(i => i == readyHash.Id))

+            if (_cryptographyProvider.GetSupportedHashMethods().Contains(readyHash.Id))

             {

                 if (String.IsNullOrEmpty(readyHash.Salt))

                 {

@@ -64,7 +64,7 @@ namespace Emby.Server.Implementations.Library
             }

             else

             {

-                throw new Exception(String.Format("Requested crypto method not available in provider: {0}", readyHash.Id));

+                throw new Exception(String.Format($"Requested crypto method not available in provider: {readyHash.Id}"));

             }

 

             //var success = string.Equals(GetPasswordHash(resolvedUser), GetHashedString(resolvedUser, password), StringComparison.OrdinalIgnoreCase);

@@ -95,7 +95,7 @@ namespace Emby.Server.Implementations.Library
                 if (user.EasyPassword != null && !user.EasyPassword.Contains("$"))

                 {

                     string hash = user.EasyPassword;

-                    user.EasyPassword = String.Format("$SHA1${0}", hash);

+                    user.EasyPassword = string.Format("$SHA1${0}", hash);

                 }

             }

         }

@@ -122,7 +122,8 @@ namespace Emby.Server.Implementations.Library
                 passwordHash.Salt = PasswordHash.ConvertToByteString(passwordHash.SaltBytes);

                 passwordHash.Id = _cryptographyProvider.DefaultHashMethod;

                 passwordHash.Hash = GetHashedStringChangeAuth(newPassword, passwordHash);

-            }else if (newPassword != null)

+            }

+            else if (newPassword != null)

             {

                 passwordHash.Hash = GetHashedString(user, newPassword);

             }