Apply suggestions from code review

Adding minor stylistic suggestions from Bond-009

Co-Authored-By: LogicalPhallacy <44458166+LogicalPhallacy@users.noreply.github.com>
This commit is contained in:
LogicalPhallacy 2019-02-18 00:31:03 -08:00 committed by GitHub
parent d8e6808d77
commit 9f3aa2cead
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 264 additions and 255 deletions

View File

@ -72,7 +72,7 @@ namespace Emby.Server.Implementations.Cryptography
} }
private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations) private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations)
{ {
using (var r = new Rfc2898DeriveBytes(bytes, salt, iterations, new HashAlgorithmName(method))) using (var r = new Rfc2898DeriveBytes(bytes, salt, iterations, new HashAlgorithmName(method)))
{ {
return r.GetBytes(32); return r.GetBytes(32);
@ -107,9 +107,9 @@ namespace Emby.Server.Implementations.Cryptography
} }
else 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) public byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt)
{ {
@ -117,25 +117,32 @@ namespace Emby.Server.Implementations.Cryptography
} }
public byte[] ComputeHash(PasswordHash hash) public byte[] ComputeHash(PasswordHash hash)
{ {
int iterations = defaultiterations; int iterations = defaultiterations;
if (!hash.Parameters.ContainsKey("iterations")) if (!hash.Parameters.ContainsKey("iterations"))
{ {
hash.Parameters.Add("iterations", defaultiterations.ToString()); 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); 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);
}
public byte[] GenerateSalt() public byte[] GenerateSalt()
{ {
byte[] salt = new byte[64]; byte[] salt = new byte[64];
rng.GetBytes(salt); rng.GetBytes(salt);
return salt; return salt;
} }
} }
} }

View File

@ -1,85 +1,86 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.IO; using System.IO;
using MediaBrowser.Controller; using MediaBrowser.Controller;
using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities;
using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Persistence;
using MediaBrowser.Model.Serialization; using MediaBrowser.Model.Serialization;
using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging;
using SQLitePCL.pretty; using SQLitePCL.pretty;
namespace Emby.Server.Implementations.Data namespace Emby.Server.Implementations.Data
{ {
/// <summary> /// <summary>
/// Class SQLiteUserRepository /// Class SQLiteUserRepository
/// </summary> /// </summary>
public class SqliteUserRepository : BaseSqliteRepository, IUserRepository public class SqliteUserRepository : BaseSqliteRepository, IUserRepository
{ {
private readonly IJsonSerializer _jsonSerializer; private readonly IJsonSerializer _jsonSerializer;
public SqliteUserRepository( public SqliteUserRepository(
ILoggerFactory loggerFactory, ILoggerFactory loggerFactory,
IServerApplicationPaths appPaths, IServerApplicationPaths appPaths,
IJsonSerializer jsonSerializer) IJsonSerializer jsonSerializer)
: base(loggerFactory.CreateLogger(nameof(SqliteUserRepository))) : base(loggerFactory.CreateLogger(nameof(SqliteUserRepository)))
{ {
_jsonSerializer = jsonSerializer; _jsonSerializer = jsonSerializer;
DbFilePath = Path.Combine(appPaths.DataPath, "users.db"); DbFilePath = Path.Combine(appPaths.DataPath, "users.db");
} }
/// <summary> /// <summary>
/// Gets the name of the repository /// Gets the name of the repository
/// </summary> /// </summary>
/// <value>The name.</value> /// <value>The name.</value>
public string Name => "SQLite"; public string Name => "SQLite";
/// <summary> /// <summary>
/// Opens the connection to the database /// Opens the connection to the database
/// </summary> /// </summary>
/// <returns>Task.</returns> /// <returns>Task.</returns>
public void Initialize() public void Initialize()
{ {
using (var connection = CreateConnection()) using (var connection = CreateConnection())
{ {
RunDefaultInitialization(connection); RunDefaultInitialization(connection);
var localUsersTableExists = TableExists(connection, "LocalUsersv2"); var localUsersTableExists = TableExists(connection, "LocalUsersv2");
connection.RunQueries(new[] { connection.RunQueries(new[] {
"create table if not exists LocalUsersv2 (Id INTEGER PRIMARY KEY, guid GUID NOT NULL, data BLOB NOT NULL)", "create table if not exists LocalUsersv2 (Id INTEGER PRIMARY KEY, guid GUID NOT NULL, data BLOB NOT NULL)",
"drop index if exists idx_users" "drop index if exists idx_users"
}); });
if (!localUsersTableExists && TableExists(connection, "Users")) if (!localUsersTableExists && TableExists(connection, "Users"))
{ {
TryMigrateToLocalUsersTable(connection); TryMigrateToLocalUsersTable(connection);
} }
RemoveEmptyPasswordHashes(); RemoveEmptyPasswordHashes();
} }
} }
private void TryMigrateToLocalUsersTable(ManagedConnection connection) private void TryMigrateToLocalUsersTable(ManagedConnection connection)
{ {
try try
{ {
connection.RunQueries(new[] connection.RunQueries(new[]
{ {
"INSERT INTO LocalUsersv2 (guid, data) SELECT guid,data from users" "INSERT INTO LocalUsersv2 (guid, data) SELECT guid,data from users"
}); });
} }
catch (Exception ex) catch (Exception ex)
{ {
Logger.LogError(ex, "Error migrating users database"); Logger.LogError(ex, "Error migrating users database");
} }
} }
private void RemoveEmptyPasswordHashes() private void RemoveEmptyPasswordHashes()
{ {
foreach (var user in RetrieveAllUsers()) foreach (var user in RetrieveAllUsers())
{ {
// If the user password is the sha1 hash of the empty string, remove it // 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; continue;
} }
@ -103,160 +104,160 @@ namespace Emby.Server.Implementations.Data
} }
} }
} }
/// <summary> /// <summary>
/// Save a user in the repo /// Save a user in the repo
/// </summary> /// </summary>
public void CreateUser(User user) public void CreateUser(User user)
{ {
if (user == null) if (user == null)
{ {
throw new ArgumentNullException(nameof(user)); throw new ArgumentNullException(nameof(user));
} }
var serialized = _jsonSerializer.SerializeToBytes(user); var serialized = _jsonSerializer.SerializeToBytes(user);
using (WriteLock.Write()) using (WriteLock.Write())
{ {
using (var connection = CreateConnection()) using (var connection = CreateConnection())
{ {
connection.RunInTransaction(db => connection.RunInTransaction(db =>
{ {
using (var statement = db.PrepareStatement("insert into LocalUsersv2 (guid, data) values (@guid, @data)")) using (var statement = db.PrepareStatement("insert into LocalUsersv2 (guid, data) values (@guid, @data)"))
{ {
statement.TryBind("@guid", user.Id.ToGuidBlob()); statement.TryBind("@guid", user.Id.ToGuidBlob());
statement.TryBind("@data", serialized); statement.TryBind("@data", serialized);
statement.MoveNext(); statement.MoveNext();
} }
var createdUser = GetUser(user.Id, false); var createdUser = GetUser(user.Id, false);
if (createdUser == null) if (createdUser == null)
{ {
throw new ApplicationException("created user should never be null"); throw new ApplicationException("created user should never be null");
} }
user.InternalId = createdUser.InternalId; user.InternalId = createdUser.InternalId;
}, TransactionMode); }, TransactionMode);
} }
} }
} }
public void UpdateUser(User user) public void UpdateUser(User user)
{ {
if (user == null) if (user == null)
{ {
throw new ArgumentNullException(nameof(user)); throw new ArgumentNullException(nameof(user));
} }
var serialized = _jsonSerializer.SerializeToBytes(user); var serialized = _jsonSerializer.SerializeToBytes(user);
using (WriteLock.Write()) using (WriteLock.Write())
{ {
using (var connection = CreateConnection()) using (var connection = CreateConnection())
{ {
connection.RunInTransaction(db => connection.RunInTransaction(db =>
{ {
using (var statement = db.PrepareStatement("update LocalUsersv2 set data=@data where Id=@InternalId")) using (var statement = db.PrepareStatement("update LocalUsersv2 set data=@data where Id=@InternalId"))
{ {
statement.TryBind("@InternalId", user.InternalId); statement.TryBind("@InternalId", user.InternalId);
statement.TryBind("@data", serialized); statement.TryBind("@data", serialized);
statement.MoveNext(); statement.MoveNext();
} }
}, TransactionMode); }, TransactionMode);
} }
} }
} }
private User GetUser(Guid guid, bool openLock) private User GetUser(Guid guid, bool openLock)
{ {
using (openLock ? WriteLock.Read() : null) using (openLock ? WriteLock.Read() : null)
{ {
using (var connection = CreateConnection(true)) using (var connection = CreateConnection(true))
{ {
using (var statement = connection.PrepareStatement("select id,guid,data from LocalUsersv2 where guid=@guid")) using (var statement = connection.PrepareStatement("select id,guid,data from LocalUsersv2 where guid=@guid"))
{ {
statement.TryBind("@guid", guid); statement.TryBind("@guid", guid);
foreach (var row in statement.ExecuteQuery()) foreach (var row in statement.ExecuteQuery())
{ {
return GetUser(row); return GetUser(row);
} }
} }
} }
} }
return null; return null;
} }
private User GetUser(IReadOnlyList<IResultSetValue> row) private User GetUser(IReadOnlyList<IResultSetValue> row)
{ {
var id = row[0].ToInt64(); var id = row[0].ToInt64();
var guid = row[1].ReadGuidFromBlob(); var guid = row[1].ReadGuidFromBlob();
using (var stream = new MemoryStream(row[2].ToBlob())) using (var stream = new MemoryStream(row[2].ToBlob()))
{ {
stream.Position = 0; stream.Position = 0;
var user = _jsonSerializer.DeserializeFromStream<User>(stream); var user = _jsonSerializer.DeserializeFromStream<User>(stream);
user.InternalId = id; user.InternalId = id;
user.Id = guid; user.Id = guid;
return user; return user;
} }
} }
/// <summary> /// <summary>
/// Retrieve all users from the database /// Retrieve all users from the database
/// </summary> /// </summary>
/// <returns>IEnumerable{User}.</returns> /// <returns>IEnumerable{User}.</returns>
public List<User> RetrieveAllUsers() public List<User> RetrieveAllUsers()
{ {
var list = new List<User>(); var list = new List<User>();
using (WriteLock.Read()) using (WriteLock.Read())
{ {
using (var connection = CreateConnection(true)) using (var connection = CreateConnection(true))
{ {
foreach (var row in connection.Query("select id,guid,data from LocalUsersv2")) foreach (var row in connection.Query("select id,guid,data from LocalUsersv2"))
{ {
list.Add(GetUser(row)); list.Add(GetUser(row));
} }
} }
} }
return list; return list;
} }
/// <summary> /// <summary>
/// Deletes the user. /// Deletes the user.
/// </summary> /// </summary>
/// <param name="user">The user.</param> /// <param name="user">The user.</param>
/// <returns>Task.</returns> /// <returns>Task.</returns>
/// <exception cref="ArgumentNullException">user</exception> /// <exception cref="ArgumentNullException">user</exception>
public void DeleteUser(User user) public void DeleteUser(User user)
{ {
if (user == null) if (user == null)
{ {
throw new ArgumentNullException(nameof(user)); throw new ArgumentNullException(nameof(user));
} }
using (WriteLock.Write()) using (WriteLock.Write())
{ {
using (var connection = CreateConnection()) using (var connection = CreateConnection())
{ {
connection.RunInTransaction(db => connection.RunInTransaction(db =>
{ {
using (var statement = db.PrepareStatement("delete from LocalUsersv2 where Id=@id")) using (var statement = db.PrepareStatement("delete from LocalUsersv2 where Id=@id"))
{ {
statement.TryBind("@id", user.InternalId); statement.TryBind("@id", user.InternalId);
statement.MoveNext(); statement.MoveNext();
} }
}, TransactionMode); }, TransactionMode);
} }
} }
} }
} }
} }

View File

@ -44,7 +44,7 @@ namespace Emby.Server.Implementations.Library
PasswordHash readyHash = new PasswordHash(resolvedUser.Password); PasswordHash readyHash = new PasswordHash(resolvedUser.Password);
byte[] CalculatedHash; byte[] CalculatedHash;
string CalculatedHashString; string CalculatedHashString;
if (_cryptographyProvider.GetSupportedHashMethods().Any(i => i == readyHash.Id)) if (_cryptographyProvider.GetSupportedHashMethods().Contains(readyHash.Id))
{ {
if (String.IsNullOrEmpty(readyHash.Salt)) if (String.IsNullOrEmpty(readyHash.Salt))
{ {
@ -64,7 +64,7 @@ namespace Emby.Server.Implementations.Library
} }
else 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); //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("$")) if (user.EasyPassword != null && !user.EasyPassword.Contains("$"))
{ {
string hash = user.EasyPassword; 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.Salt = PasswordHash.ConvertToByteString(passwordHash.SaltBytes);
passwordHash.Id = _cryptographyProvider.DefaultHashMethod; passwordHash.Id = _cryptographyProvider.DefaultHashMethod;
passwordHash.Hash = GetHashedStringChangeAuth(newPassword, passwordHash); passwordHash.Hash = GetHashedStringChangeAuth(newPassword, passwordHash);
}else if (newPassword != null) }
else if (newPassword != null)
{ {
passwordHash.Hash = GetHashedString(user, newPassword); passwordHash.Hash = GetHashedString(user, newPassword);
} }