From 9bfb43e46ac6975b836ab90b5542d83ae38d5a7a Mon Sep 17 00:00:00 2001 From: amazing-username Date: Sat, 11 May 2019 16:42:29 -0400 Subject: [PATCH] Addressed the unsecure login HTTP endpoint and user the Performers property instead of the deprecated Artists property from the TagLib library. #38 --- Controllers/LoginController.cs | 53 +++++-- Controllers/Managers/TokenManager.cs | 3 +- Controllers/RegisterController.cs | 26 ++- Controllers/Utilities/MetadataRetriever.cs | 4 +- Controllers/Utilities/PasswordEncryption.cs | 32 +++- Models/BaseResult.cs | 12 ++ Models/Context/AlbumStoreContext.cs | 4 +- Models/Context/ArtistStoreContext.cs | 4 +- Models/Context/BaseStoreContext.cs | 2 + Models/Context/MusicStoreContext.cs | 4 +- Models/Context/UserStoreContext.cs | 167 +++++++++++++++++--- Models/LoginResult.cs | 2 +- Models/RegisterResult.cs | 14 ++ 13 files changed, 267 insertions(+), 60 deletions(-) create mode 100644 Models/BaseResult.cs create mode 100644 Models/RegisterResult.cs diff --git a/Controllers/LoginController.cs b/Controllers/LoginController.cs index 611e5f4..f5e326b 100644 --- a/Controllers/LoginController.cs +++ b/Controllers/LoginController.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; using Icarus.Controllers.Managers; using Icarus.Controllers.Utilities; @@ -20,6 +21,7 @@ namespace Icarus.Controllers { #region Fields private IConfiguration _config; + private ILogger _logger; #endregion @@ -28,9 +30,10 @@ namespace Icarus.Controllers #region Contructors - public LoginController(IConfiguration config) + public LoginController(IConfiguration config, ILogger logger) { _config = config; + _logger = logger; } #endregion @@ -38,26 +41,48 @@ namespace Icarus.Controllers #region HTTP endpoints public IActionResult Post([FromBody] User user) { - // TODO: Secure this HTTP endpoint. #38 - // Currently there is no check done to determine whether or not the - // user's password sent with the request matches the password stored - // in the database. In fact there is not check if the user credentials - // sent with this request even exist in the database. I knowingly left - // this bug in here for the sole purpose of making it easier to test - // but it should now be addressed. - UserStoreContext context = HttpContext .RequestServices .GetService(typeof(UserStoreContext)) as UserStoreContext; - user = context.RetrieveUser(user); - Console.WriteLine($"Username: {user.Username}"); + _logger.LogInformation("Starting process of validating credentials"); + + var message = "Invalid credentials"; + var password = user.Password; - TokenManager tk = new TokenManager(_config); + var loginRes = new LoginResult + { + Username = user.Username + }; - LoginResult loginRes = tk.RetrieveLoginResult(user); + if (context.DoesUserExist(user)) + { + user = context.RetrieveUser(user); - return Ok(loginRes); + var validatePass = new PasswordEncryption(); + var validated = validatePass.VerifyPassword(user, password); + if (!validated) + { + loginRes.Message = message; + _logger.LogInformation(message); + + return Ok(loginRes); + } + + _logger.LogInformation("Successfully validated user credentials"); + + TokenManager tk = new TokenManager(_config); + + loginRes = tk.RetrieveLoginResult(user); + + return Ok(loginRes); + } + else + { + loginRes.Message = message; + + return NotFound(loginRes); + } } #endregion } diff --git a/Controllers/Managers/TokenManager.cs b/Controllers/Managers/TokenManager.cs index 9af7cbc..25ff888 100644 --- a/Controllers/Managers/TokenManager.cs +++ b/Controllers/Managers/TokenManager.cs @@ -59,7 +59,8 @@ namespace Icarus.Controllers.Managers Username = user.Username, Token = tokenResult.AccessToken, TokenType = tokenResult.TokenType, - Expiration = tokenResult.Expiration + Expiration = tokenResult.Expiration, + Message = "Successfully retrieved token" }; } diff --git a/Controllers/RegisterController.cs b/Controllers/RegisterController.cs index 8844c5c..0adaebe 100644 --- a/Controllers/RegisterController.cs +++ b/Controllers/RegisterController.cs @@ -35,21 +35,37 @@ namespace Icarus.Controllers #endregion [HttpPost] - public void Post([FromBody] User user) + public IActionResult Post([FromBody] User user) { - Console.WriteLine($"Username: {user.Username}"); - Console.WriteLine($"Password: {user.Password}"); - PasswordEncryption pe = new PasswordEncryption(); user.Password = pe.HashPassword(user); user.EmailVerified = false; - Console.WriteLine($"Hashed Password: {user.Password}"); UserStoreContext context = HttpContext .RequestServices .GetService(typeof(UserStoreContext)) as UserStoreContext; context.SaveUser(user); + + var registerResult = new RegisterResult + { + Username = user.Username + }; + + if (context.DoesUserExist(user)) + { + registerResult.Message = "Successful registration"; + registerResult.SuccessfullyRegistered = true; + + return Ok(registerResult); + } + else + { + registerResult.Message = "Registration failed"; + registerResult.SuccessfullyRegistered = false; + + return Ok(registerResult); + } } } } diff --git a/Controllers/Utilities/MetadataRetriever.cs b/Controllers/Utilities/MetadataRetriever.cs index 7f0681a..e539d33 100644 --- a/Controllers/Utilities/MetadataRetriever.cs +++ b/Controllers/Utilities/MetadataRetriever.cs @@ -51,7 +51,7 @@ namespace Icarus.Controllers.Utilities { TagLib.File fileTag = TagLib.File.Create(filePath); _title = fileTag.Tag.Title; - _artist = string.Join("", fileTag.Tag.Artists); + _artist = string.Join("", fileTag.Tag.Performers); _album = fileTag.Tag.Album; _genre = string.Join("", fileTag.Tag.Genres); _year = (int)fileTag.Tag.Year; @@ -141,7 +141,7 @@ namespace Icarus.Controllers.Utilities break; case "artists": _updatedSong.Artist = artist; - fileTag.Tag.Artists = new []{artist}; + fileTag.Tag.Performers = new []{artist}; break; case "album": _updatedSong.AlbumTitle = album; diff --git a/Controllers/Utilities/PasswordEncryption.cs b/Controllers/Utilities/PasswordEncryption.cs index e68ecd4..0551250 100644 --- a/Controllers/Utilities/PasswordEncryption.cs +++ b/Controllers/Utilities/PasswordEncryption.cs @@ -3,6 +3,7 @@ using System.Security.Cryptography; using BCrypt.Net; using Microsoft.AspNetCore.Cryptography.KeyDerivation; +using NLog; using Icarus.Models; @@ -11,6 +12,7 @@ namespace Icarus.Controllers.Utilities public class PasswordEncryption { #region Fields + private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); #endregion @@ -23,6 +25,23 @@ namespace Icarus.Controllers.Utilities #region Methods + public bool VerifyPassword(User user, string password) + { + try + { + var result = BCrypt.Net.BCrypt.Verify(password, user.Password); + + return result; + } + catch (Exception ex) + { + var msg = ex.Message; + _logger.Error(msg, "An error occurred"); + } + + return false; + } + public string HashPassword(User user) { try @@ -30,12 +49,15 @@ namespace Icarus.Controllers.Utilities string hashedPassword = string.Empty; hashedPassword = BCrypt.Net.BCrypt.HashPassword(user.Password); + _logger.Info("Successfully hashed password"); + return hashedPassword; } catch (Exception ex) { var exMsg = ex.Message; + _logger.Error(exMsg, "An error occurred"); } return null; @@ -45,11 +67,11 @@ namespace Icarus.Controllers.Utilities { string hashed = Convert.ToBase64String(KeyDerivation.Pbkdf2( - password: password, - salt: salt, - prf: KeyDerivationPrf.HMACSHA1, - iterationCount: 10000, - numBytesRequested: 256/8)); + password: password, + salt: salt, + prf: KeyDerivationPrf.HMACSHA1, + iterationCount: 10000, + numBytesRequested: 256/8)); return hashed; } diff --git a/Models/BaseResult.cs b/Models/BaseResult.cs new file mode 100644 index 0000000..f66f0f6 --- /dev/null +++ b/Models/BaseResult.cs @@ -0,0 +1,12 @@ +using System; + +using Newtonsoft.Json; + +namespace Icarus.Models +{ + public class BaseResult + { + [JsonProperty("message")] + public string Message { get; set; } + } +} diff --git a/Models/Context/AlbumStoreContext.cs b/Models/Context/AlbumStoreContext.cs index d37b2cc..44968f6 100644 --- a/Models/Context/AlbumStoreContext.cs +++ b/Models/Context/AlbumStoreContext.cs @@ -3,7 +3,7 @@ using System.Collections.Generic; using System.Globalization; using MySql.Data.MySqlClient; -using NLog; +//using NLog; using Icarus.Models; @@ -12,7 +12,7 @@ namespace Icarus.Models.Context public class AlbumStoreContext : BaseStoreContext { #region Fields - private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); + //private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); #endregion diff --git a/Models/Context/ArtistStoreContext.cs b/Models/Context/ArtistStoreContext.cs index 79d09da..daefe2d 100644 --- a/Models/Context/ArtistStoreContext.cs +++ b/Models/Context/ArtistStoreContext.cs @@ -4,7 +4,7 @@ using System.Globalization; using Microsoft.Extensions.Logging; using MySql.Data.MySqlClient; -using NLog; +//using NLog; using Icarus.Models; @@ -13,7 +13,7 @@ namespace Icarus.Models.Context public class ArtistStoreContext : BaseStoreContext { #region Fields - private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); + //private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); #endregion diff --git a/Models/Context/BaseStoreContext.cs b/Models/Context/BaseStoreContext.cs index 7e19957..91981ad 100644 --- a/Models/Context/BaseStoreContext.cs +++ b/Models/Context/BaseStoreContext.cs @@ -1,6 +1,7 @@ using System; using MySql.Data.MySqlClient; +using NLog; namespace Icarus.Models.Context { @@ -8,6 +9,7 @@ namespace Icarus.Models.Context { #region Fields protected string _connectionString; + protected static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); #endregion diff --git a/Models/Context/MusicStoreContext.cs b/Models/Context/MusicStoreContext.cs index 3096701..4a6c83e 100644 --- a/Models/Context/MusicStoreContext.cs +++ b/Models/Context/MusicStoreContext.cs @@ -2,14 +2,14 @@ using System; using System.Collections.Generic; using MySql.Data.MySqlClient; -using NLog; +//using NLog; namespace Icarus.Models.Context { public class MusicStoreContext : BaseStoreContext { #region Fields - private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); + //private static Logger _logger = NLog.LogManager.GetCurrentClassLogger(); #endregion diff --git a/Models/Context/UserStoreContext.cs b/Models/Context/UserStoreContext.cs index 69a67dc..e294362 100644 --- a/Models/Context/UserStoreContext.cs +++ b/Models/Context/UserStoreContext.cs @@ -31,15 +31,15 @@ namespace Icarus.Models.Context { try { + _logger.Info("Saving user"); + using (MySqlConnection conn = GetConnection()) { conn.Open(); - string query = "INSERT INTO Users(Username, Password, Nickname," + - "Email, PhoneNumber, Firstname, Lastname, " + - "EmailVerified, DateCreated, LastLogin) " + - "VALUES(@Username, @Password, @Nickname, " + - "@Email, @PhoneNumber, @Firstname, @Lastname," + - " @EmailVerified, @DateCreated, @LastLogin)"; + string query = "INSERT INTO User(Username, Password, Nickname, Email" + + ", PhoneNumber, Firstname, Lastname, EmailVerified) " + + "VALUES(@Username, @Password, @Nickname, @Email, @PhoneNumber," + + " @Firstname, @Lastname, @EmailVerified)"; using (MySqlCommand cmd = new MySqlCommand(query, conn)) { cmd.Parameters.AddWithValue("@Username", user.Username); @@ -50,18 +50,19 @@ namespace Icarus.Models.Context cmd.Parameters.AddWithValue("@Firstname", user.Firstname); cmd.Parameters.AddWithValue("@Lastname", user.Lastname); cmd.Parameters.AddWithValue("@EmailVerified", user.EmailVerified); - cmd.Parameters.AddWithValue("@DateCreated", DateTime.Now); - cmd.Parameters.AddWithValue("@LastLogin", DateTime.Now); cmd.ExecuteNonQuery(); } } + + _logger.Info("Successfully saved user"); } catch (Exception ex) { var exMsg = ex.Message; Console.WriteLine($"An error occurred:\n{exMsg}"); + _logger.Error(exMsg, "An error occurred"); } } @@ -69,10 +70,12 @@ namespace Icarus.Models.Context { try { + _logger.Info("Retrieving user"); + using (MySqlConnection conn = GetConnection()) { conn.Open(); - var query = "SELECT * FROM Users WHERE Username=@Username"; + var query = "SELECT * FROM User WHERE Username=@Username"; using (MySqlCommand cmd = new MySqlCommand(query, conn)) { @@ -80,36 +83,148 @@ namespace Icarus.Models.Context using (var reader = cmd.ExecuteReader()) { - while (reader.Read()) - { - var dateCreated = reader["DateCreated"].ToString(); - var lastLogin = reader["LastLogin"].ToString(); - var parsedC = DateTime.Parse(dateCreated); - var parsedL = DateTime.Parse(lastLogin); - - user.Id = Convert.ToInt32(reader["Id"]); - user.Nickname = reader["Nickname"].ToString(); - user.Email = reader["Email"].ToString(); - user.PhoneNumber = reader["PhoneNumber"].ToString(); - user.EmailVerified = (reader["EmailVerified"].ToString()) == "1"; - user.Firstname = reader["Firstname"].ToString(); - user.Lastname = reader["Lastname"].ToString(); - user.DateCreated = DateTime.Parse(parsedC.ToString("yyyy-MM-dd HH:mm:ss")); - user.LastLogin = DateTime.Parse(parsedL.ToString("yyyy-MM-dd HH:mm:ss")); - } + user = ParseSingleData(reader); } } } + + _logger.Info("Successfully retrieved user"); + return user; } catch (Exception ex) { var exMsg = ex.Message; Console.WriteLine($"An error occurred:\n{exMsg}"); + _logger.Error(exMsg, "An error occurred"); } return null; } + + public bool DoesUserExist(User user) + { + var username = user.Username; + try + { + _logger.Info($"Checking to see if {user.Username} exists"); + + using (var conn = GetConnection()) + { + conn.Open(); + var query = "SELECT * FROM User WHERE Username=@Username"; + + using (var cmd = new MySqlCommand(query, conn)) + { + cmd.Parameters.AddWithValue("@Username", user.Username); + + using (var reader = cmd.ExecuteReader()) + { + user = ParseSingleData(reader, true); + username = user.Username; + + if (!string.IsNullOrEmpty(username)) + { + _logger.Info($"The user {username} exists"); + + return true; + } + } + } + } + } + catch (Exception ex) + { + var msg = ex.Message; + _logger.Error(msg, "An error occurred"); + } + + _logger.Info($"The user {username} does not exists"); + + return false; + } + + private User ParseSingleData(MySqlDataReader reader) + { + var user = new User(); + + while (reader.Read()) + { + var id = Convert.ToInt32(reader["Id"].ToString()); + var username = reader["Username"].ToString(); + var password = reader["Password"].ToString(); + var nickname = reader["Nickname"].ToString(); + var email = reader["Email"].ToString(); + var phoneNumber = reader["PhoneNumber"].ToString(); + var emailVerified = reader["EmailVerified"].ToString() == "1"; + var firstname = reader["Firstname"].ToString(); + var lastname = reader["Lastname"].ToString(); + var rawLastLogin = reader["LastLogin"].ToString(); + + var parsedDateCreated = DateTime.Parse(reader["DateCreated"].ToString()); + + var dateCreated = DateTime.Parse(parsedDateCreated.ToString("yyyy-MM-dd HH:mm:ss")); + + if (!string.IsNullOrEmpty(rawLastLogin)) + { + var parsedLastLogin = DateTime.Parse(rawLastLogin); + var lastLogin = DateTime.Parse(parsedLastLogin.ToString("yyyy-MM-dd HH:mm:ss")); + user.LastLogin = lastLogin; + } + + user.Id = id; + user.Username = username; + user.Password = password; + user.Nickname = nickname; + user.Email = email; + user.PhoneNumber = phoneNumber; + user.EmailVerified = emailVerified; + user.Firstname = firstname; + user.Lastname = lastname; + user.DateCreated = dateCreated; + } + + return user; + } + private User ParseSingleData(MySqlDataReader reader, bool ignoreLastLogin) + { + var user = new User(); + + while (reader.Read()) + { + var id = Convert.ToInt32(reader["Id"].ToString()); + var username = reader["Username"].ToString(); + var nickname = reader["Nickname"].ToString(); + var email = reader["Email"].ToString(); + var phoneNumber = reader["PhoneNumber"].ToString(); + var emailVerified = reader["EmailVerified"].ToString() == "1"; + var firstname = reader["Firstname"].ToString(); + var lastname = reader["Lastname"].ToString(); + + var parsedDateCreated = DateTime.Parse(reader["DateCreated"].ToString()); + + var dateCreated = DateTime.Parse(parsedDateCreated.ToString("yyyy-MM-dd HH:mm:ss")); + + if (!ignoreLastLogin) + { + var parsedLastLogin = DateTime.Parse(reader["LastLogin"].ToString()); + var lastLogin = DateTime.Parse(parsedLastLogin.ToString("yyyy-MM-dd HH:mm:ss")); + user.LastLogin = lastLogin; + } + + user.Id = id; + user.Username = username; + user.Nickname = nickname; + user.Email = email; + user.PhoneNumber = phoneNumber; + user.EmailVerified = emailVerified; + user.Firstname = firstname; + user.Lastname = lastname; + user.DateCreated = dateCreated; + } + + return user; + } #endregion } } diff --git a/Models/LoginResult.cs b/Models/LoginResult.cs index bf8babb..3a48b5b 100644 --- a/Models/LoginResult.cs +++ b/Models/LoginResult.cs @@ -4,7 +4,7 @@ using Newtonsoft.Json; namespace Icarus.Models { - public class LoginResult + public class LoginResult : BaseResult { [JsonProperty("id")] public int UserId { get; set; } diff --git a/Models/RegisterResult.cs b/Models/RegisterResult.cs new file mode 100644 index 0000000..055f397 --- /dev/null +++ b/Models/RegisterResult.cs @@ -0,0 +1,14 @@ +using System; + +using Newtonsoft.Json; + +namespace Icarus.Models +{ + public class RegisterResult : BaseResult + { + [JsonProperty("username")] + public string Username { get; set; } + [JsonProperty("successfully_registered")] + public bool SuccessfullyRegistered { get; set; } + } +}