From fa451cc788cfaad966d45022592eb8e0f58fceae Mon Sep 17 00:00:00 2001 From: Dion Date: Wed, 16 Mar 2022 21:31:42 +0100 Subject: [PATCH 1/2] idea to avoid escaping chars --- .../Helpers/ExifToolCmdHelper.cs | 12 ++ .../Models/ExifToolModel.cs | 124 ------------------ .../Services/ExifToolService.cs | 2 +- .../starskytest/Models/ExifToolModelTest.cs | 119 ----------------- 4 files changed, 13 insertions(+), 244 deletions(-) delete mode 100644 starsky/starsky.foundation.writemeta/Models/ExifToolModel.cs delete mode 100644 starsky/starskytest/Models/ExifToolModelTest.cs diff --git a/starsky/starsky.foundation.writemeta/Helpers/ExifToolCmdHelper.cs b/starsky/starsky.foundation.writemeta/Helpers/ExifToolCmdHelper.cs index 1afb27e8f0..cedd0d85e6 100644 --- a/starsky/starsky.foundation.writemeta/Helpers/ExifToolCmdHelper.cs +++ b/starsky/starsky.foundation.writemeta/Helpers/ExifToolCmdHelper.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Text; using System.Threading.Tasks; using starsky.foundation.database.Helpers; using starsky.foundation.database.Models; @@ -270,6 +271,17 @@ private string UpdateGpsLongitudeCommand(string command, List comparedNa return command; } + internal static string ReplaceBadChars(string inputText) + { + var cleanText = new StringBuilder(); + const string badChars = "';`$\"\r\n\t*> !badChars.Contains(inputChar)) ) + { + cleanText.Append(inputChar); + } + return cleanText.ToString(); + } + private static string UpdateKeywordsCommand(string command, List comparedNames, FileIndexItem updateModel) { if (comparedNames.Contains( nameof(FileIndexItem.Tags).ToLowerInvariant() )) diff --git a/starsky/starsky.foundation.writemeta/Models/ExifToolModel.cs b/starsky/starsky.foundation.writemeta/Models/ExifToolModel.cs deleted file mode 100644 index 53d8ca35b3..0000000000 --- a/starsky/starsky.foundation.writemeta/Models/ExifToolModel.cs +++ /dev/null @@ -1,124 +0,0 @@ -using System.Text.Json.Serialization; -using System; -using System.Collections.Generic; -using System.Linq; -using starsky.foundation.database.Models; -using starsky.foundation.platform.Helpers; - -namespace starsky.foundation.writemeta.Models -{ - public class ExifToolModel - { - - public string SourceFile { get; set; } - - public ColorClassParser.Color ColorClass { get; set; } - - - // Replace user Quoted input with single quote to avoid SQL Injection. - private string _captionabstract; - - - [JsonPropertyName("Caption-Abstract")] - public string CaptionAbstract { - get { - return _captionabstract; - } - set - { - if(string.IsNullOrWhiteSpace(value)) return; - _captionabstract = value.Replace("\"", "\'"); - } - } - - // overwrite "-xmp:Description" over -CaptionAbstract - public string Description - { - set { - if (!string.IsNullOrWhiteSpace(value)) - { - CaptionAbstract = value; - } - } - get => null; - } - - // Don't ignore this one, when setting it will also ignored - public string Prefs - { - get { return null; } - set - { - // input: "Tagged:0, ColorClass:2, Rating:0, FrameNum:0" - var prefsList = value.Split(", ".ToCharArray()); - var firstColorClass = prefsList.FirstOrDefault(p => p.Contains("ColorClass")); - if (firstColorClass != null) - { - var stringColorClassItem = firstColorClass.Replace("ColorClass:", ""); - ColorClass = ColorClassParser.GetColorClass(stringColorClassItem); - } - } - } - - private HashSet _keywords = new HashSet(); - - public HashSet Keywords - { - get => _keywords; - // keep null? temp off - set { - if (value != null) - { - _keywords = value; - } - } - } - - public string Tags - { - get { return HashSetHelper.HashSetToString(_keywords); } - set - { - if (!string.IsNullOrWhiteSpace(value)) - { - _keywords = HashSetHelper.StringToHashSet(value); - } - } - } - - // overwrite "-xmp:subject" over -Keywords - public HashSet Subject - { - set { - if (_keywords.Count == 0) - { - _keywords = value; - } - } - get => new HashSet(); - } - - public string ObjectName { get; set; } = string.Empty; - - // overwrite "-xmp:title" over -ObjectName - public string Title - { - set { - if (!string.IsNullOrWhiteSpace(value)) - { - ObjectName = value; - } - } - get => null; - } - - - public DateTime AllDatesDateTime { get; set; } - - // Orientation : 6 - public FileIndexItem.Rotation Orientation { get; set; } - - public ushort ImageWidth { get; set; } - public ushort ImageHeight { get; set; } - } -} diff --git a/starsky/starsky.foundation.writemeta/Services/ExifToolService.cs b/starsky/starsky.foundation.writemeta/Services/ExifToolService.cs index 77c680ea91..45fd4b1507 100644 --- a/starsky/starsky.foundation.writemeta/Services/ExifToolService.cs +++ b/starsky/starsky.foundation.writemeta/Services/ExifToolService.cs @@ -20,8 +20,8 @@ public ExifToolService(ISelectorStorage selectorStorage, AppSettings appSettings var iStorage = selectorStorage.Get(SelectorStorage.StorageServices.SubPath); var thumbnailStorage = selectorStorage.Get(SelectorStorage.StorageServices.Thumbnail); _exifTool = new ExifTool(iStorage, thumbnailStorage, appSettings,logger); - } + public async Task WriteTagsAsync(string subPath, string command) { return await _exifTool.WriteTagsAsync(subPath,command); diff --git a/starsky/starskytest/Models/ExifToolModelTest.cs b/starsky/starskytest/Models/ExifToolModelTest.cs deleted file mode 100644 index d2a3bf2c0f..0000000000 --- a/starsky/starskytest/Models/ExifToolModelTest.cs +++ /dev/null @@ -1,119 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.VisualStudio.TestTools.UnitTesting; -using starsky.foundation.platform.Helpers; -using starsky.foundation.writemeta.Models; - -namespace starskytest.Models -{ - [TestClass] - public class ExifToolModelTest - { - [TestMethod] - public void ExifToolModelColorClassTest() - { - var exifToolModel = new ExifToolModel{ColorClass = ColorClassParser.Color.Winner}; - Assert.AreEqual(exifToolModel.ColorClass,ColorClassParser.Color.Winner); - } - - [TestMethod] - public void ExifToolCAllDatesDateTimeTest() - { - var datetime = new DateTime(); - var exifToolModel = new ExifToolModel{AllDatesDateTime = datetime}; - Assert.AreEqual(datetime,exifToolModel.AllDatesDateTime); - } - - [TestMethod] - public void ExifToolCaptionAbstractTest() - { - var exifToolModel = new ExifToolModel{CaptionAbstract = "test"}; - Assert.AreEqual("test",exifToolModel.CaptionAbstract); - } - - - [TestMethod] - public void ExifToolPrefsParseTest() - { - var exifToolModel = new ExifToolModel{Prefs = "Tagged:0, ColorClass:2, Rating:0, FrameNum:0"}; - Assert.AreEqual(ColorClassParser.Color.WinnerAlt, exifToolModel.ColorClass); - } - - [TestMethod] - public void ExifToolPrefsNullTest() - { - var exifToolModel = new ExifToolModel(); - Assert.AreEqual(null,exifToolModel.Prefs); - } - - [TestMethod] - public void ExifToolTagsKeywordsFirstTest() - { - var exifToolModel = new ExifToolModel{Tags = "Schiphol, airplane"}; - Assert.AreEqual("Schiphol",exifToolModel.Keywords.FirstOrDefault()); - } - - [TestMethod] - public void ExifToolSetHashSet1Test() - { - var list = new List {"Schiphol", "Schiphol"}; - var exifToolModel = new ExifToolModel{Keywords = list.ToHashSet()}; - Assert.AreEqual(1,exifToolModel.Keywords.Count); - } - - [TestMethod] - public void ExifTool_hashSetToStringTest() - { - // It is in memory stored as HashSet, not as string - var exifToolModel = new ExifToolModel{Tags = "Schiphol, airplane"}; - Assert.AreEqual("Schiphol, airplane",exifToolModel.Tags); - } - - [TestMethod] - public void ExifTool_hashSetToStringNullTest() - { - var exifToolModel = new ExifToolModel(); - Assert.AreEqual(string.Empty,exifToolModel.Tags); - } - - [TestMethod] - public void ExifToolKeywordsNullTest() - { - var exifToolModel = new ExifToolModel{Keywords = null}; - Assert.AreEqual(0,exifToolModel.Keywords.Count); - } - - [TestMethod] - public void ExifToolSubjectOverwrite() - { - var list = new List {"Schiphol", "Schiphol"}; - var exifToolModel = new ExifToolModel{Subject = list.ToHashSet()}; - Assert.AreEqual(1,exifToolModel.Keywords.Count); - Assert.AreEqual(0, exifToolModel.Subject.Count); - } - - [TestMethod] - public void ExifToolTitleOverwrite() - { - var exifToolModel = new ExifToolModel{Title = "testung"}; - Assert.AreEqual("testung",exifToolModel.ObjectName); - Assert.AreEqual(null, exifToolModel.Title); - } - - [TestMethod] - public void ExifToolDescriptionOverwrite() - { - var exifToolModel = new ExifToolModel{Description = "testung"}; - Assert.AreEqual("testung",exifToolModel.CaptionAbstract); - Assert.AreEqual(null, exifToolModel.Description); - } - - [TestMethod] - public void ExifToolModelSourceFile() - { - var exifToolModel = new ExifToolModel { SourceFile = "testung" }; - Assert.AreEqual("testung", exifToolModel.SourceFile); - } - } -} From 19fd3404e134988ba197ce832a50877d69d3d533 Mon Sep 17 00:00:00 2001 From: Dion Date: Wed, 16 Mar 2022 22:29:45 +0100 Subject: [PATCH 2/2] add null checks --- .../starsky/Controllers/AccountController.cs | 22 +++++++++++++------ .../BasicAuthenticationMiddlewareTest.cs | 2 +- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/starsky/starsky/Controllers/AccountController.cs b/starsky/starsky/Controllers/AccountController.cs index 05758f9d6a..70be3503d4 100644 --- a/starsky/starsky/Controllers/AccountController.cs +++ b/starsky/starsky/Controllers/AccountController.cs @@ -8,6 +8,7 @@ using starsky.foundation.accountmanagement.Interfaces; using starsky.foundation.accountmanagement.Models; using starsky.foundation.accountmanagement.Models.Account; +using starsky.foundation.platform.Interfaces; using starsky.foundation.platform.Models; using starsky.foundation.storage.Interfaces; using starsky.foundation.storage.Storage; @@ -21,13 +22,15 @@ public class AccountController : Controller private readonly AppSettings _appSettings; private readonly IAntiforgery _antiForgery; private readonly IStorage _storageHostFullPathFilesystem; + private readonly IWebLogger _logger; - public AccountController(IUserManager userManager, AppSettings appSettings, IAntiforgery antiForgery, ISelectorStorage selectorStorage) + public AccountController(IUserManager userManager, AppSettings appSettings, IAntiforgery antiForgery, ISelectorStorage selectorStorage, IWebLogger logger) { _userManager = userManager; _appSettings = appSettings; _antiForgery = antiForgery; _storageHostFullPathFilesystem = selectorStorage.Get(SelectorStorage.StorageServices.HostFilesystem); + _logger = logger; } /// @@ -51,7 +54,7 @@ public async Task Status() return Json("There are no accounts, you must create an account first"); } - if ( !User.Identity.IsAuthenticated ) return Unauthorized("false"); + if ( User.Identity?.IsAuthenticated == false) return Unauthorized("false"); // use model to avoid circular references var currentUser = _userManager.GetCurrentUser(HttpContext); @@ -112,6 +115,9 @@ public IActionResult LoginGet(string returnUrl = null, bool? fromLogout = null) [Produces("application/json")] public async Task LoginPost(LoginViewModel model) { + var remoteIp = Request.HttpContext.Connection.RemoteIpAddress?.ToString(); + _logger.LogInformation("[LoginPost]" +remoteIp); + ValidateResult validateResult = await _userManager.ValidateAsync("Email", model.Email, model.Password); if (!validateResult.Success) @@ -125,7 +131,7 @@ public async Task LoginPost(LoginViewModel model) } await _userManager.SignIn(HttpContext, validateResult.User, model.RememberMe); - if ( User.Identity.IsAuthenticated) + if ( User.Identity?.IsAuthenticated == true) { return Json("Login Success"); } @@ -178,7 +184,7 @@ public IActionResult Logout(string returnUrl = null) [Authorize] public async Task ChangeSecret(ChangePasswordViewModel model) { - if ( !User.Identity.IsAuthenticated ) return Unauthorized("please login first"); + if ( User.Identity?.IsAuthenticated != true ) return Unauthorized("please login first"); if ( !ModelState.IsValid || model.ChangedPassword != model.ChangedConfirmPassword ) return BadRequest("Model is not correct"); @@ -186,6 +192,8 @@ public async Task ChangeSecret(ChangePasswordViewModel model) var currentUserId = _userManager.GetCurrentUser(HttpContext).Id; + _logger.LogInformation($"[ChangeSecret] for {currentUserId}"); + var credential = _userManager.GetCredentialsByUserId(currentUserId); // Re-check password @@ -222,7 +230,7 @@ public async Task ChangeSecret(ChangePasswordViewModel model) [ValidateAntiForgeryToken] public async Task Register(RegisterViewModel model) { - if ( await IsAccountRegisterClosed(User.Identity.IsAuthenticated) ) + if ( await IsAccountRegisterClosed(User.Identity?.IsAuthenticated) ) { Response.StatusCode = 403; return Json("Account Register page is closed"); @@ -244,9 +252,9 @@ public async Task Register(RegisterViewModel model) /// /// /// - private async Task IsAccountRegisterClosed(bool userIdentityIsAuthenticated) + private async Task IsAccountRegisterClosed(bool? userIdentityIsAuthenticated) { - if ( userIdentityIsAuthenticated ) return false; + if ( userIdentityIsAuthenticated == true) return false; return _appSettings.IsAccountRegisterOpen != true && (await _userManager.AllUsersAsync()).Any(); } diff --git a/starsky/starskytest/Middleware/BasicAuthenticationMiddlewareTest.cs b/starsky/starskytest/Middleware/BasicAuthenticationMiddlewareTest.cs index dfb500e00c..d6de20845b 100644 --- a/starsky/starskytest/Middleware/BasicAuthenticationMiddlewareTest.cs +++ b/starsky/starskytest/Middleware/BasicAuthenticationMiddlewareTest.cs @@ -100,7 +100,7 @@ public async Task BasicAuthenticationMiddlewareLoginTest() var schemeProvider = _serviceProvider.GetRequiredService(); var controller = - new AccountController(_userManager, new AppSettings(), new FakeAntiforgery(), new FakeSelectorStorage()) + new AccountController(_userManager, new AppSettings(), new FakeAntiforgery(), new FakeSelectorStorage(), new FakeIWebLogger()) { ControllerContext = {HttpContext = httpContext} };