From 2ce4ddcaa425c26fa20503a03ea9b0dacf040677 Mon Sep 17 00:00:00 2001 From: Joe Milazzo Date: Mon, 15 May 2023 12:53:43 -0500 Subject: [PATCH] More Fixes from Recent PRs (#1995) * Added extra debugging for logout issue * Fixed the null issue with ISBN * Allow web links to be cleared out * More logging on refresh token * More key fallback when building Table of Contents * Added better fallback implementation for building table of contents based on the many different ways epubs are packed and referenced. * Updated dependencies * Fixed up refresh token refresh which was invalidating sessions for no reason. Added it to update last active time as well. --- API.Tests/API.Tests.csproj | 4 +- API/API.csproj | 2 +- API/Controllers/AccountController.cs | 3 ++ API/Program.cs | 11 ++--- API/Services/BookService.cs | 63 ++++++++++++++++++++++++---- API/Services/SeriesService.cs | 13 +++--- API/Services/TokenService.cs | 34 ++++++++++++--- Kavita.Common/Configuration.cs | 3 +- openapi.json | 2 +- 9 files changed, 104 insertions(+), 31 deletions(-) diff --git a/API.Tests/API.Tests.csproj b/API.Tests/API.Tests.csproj index 16aaa018f..95828bfd7 100644 --- a/API.Tests/API.Tests.csproj +++ b/API.Tests/API.Tests.csproj @@ -10,8 +10,8 @@ - - + + runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/API/API.csproj b/API/API.csproj index 0decaef67..2eaa5f9ff 100644 --- a/API/API.csproj +++ b/API/API.csproj @@ -102,7 +102,7 @@ - + diff --git a/API/Controllers/AccountController.cs b/API/Controllers/AccountController.cs index ea9159d1c..ec2b1156d 100644 --- a/API/Controllers/AccountController.cs +++ b/API/Controllers/AccountController.cs @@ -443,6 +443,9 @@ public class AccountController : BaseApiController if (!roleResult.Succeeded) return BadRequest(roleResult.Errors); } + // We might want to check if they had admin and no longer, if so: + // await _userManager.UpdateSecurityStampAsync(user); to force them to re-authenticate + var allLibraries = (await _unitOfWork.LibraryRepository.GetLibrariesAsync()).ToList(); List libraries; diff --git a/API/Program.cs b/API/Program.cs index f0ebbf23e..028939891 100644 --- a/API/Program.cs +++ b/API/Program.cs @@ -165,16 +165,16 @@ public class Program var env = hostingContext.HostingEnvironment; - config.AddJsonFile("config/appsettings.json", optional: true, reloadOnChange: false) + config.AddJsonFile("config/appsettings.json", optional: false, reloadOnChange: false) .AddJsonFile($"config/appsettings.{env.EnvironmentName}.json", - optional: true, reloadOnChange: false); + optional: false, reloadOnChange: false); }) .ConfigureWebHostDefaults(webBuilder => { webBuilder.UseKestrel((opts) => { var ipAddresses = Configuration.IpAddresses; - if (new OsInfo(Array.Empty()).IsDocker || string.IsNullOrEmpty(ipAddresses) || ipAddresses.Equals(Configuration.DefaultIpAddresses)) + if (new OsInfo().IsDocker || string.IsNullOrEmpty(ipAddresses) || ipAddresses.Equals(Configuration.DefaultIpAddresses)) { opts.ListenAnyIP(HttpPort, options => { options.Protocols = HttpProtocols.Http1AndHttp2; }); } @@ -195,9 +195,6 @@ public class Program } }); - webBuilder.UseStartup(); + webBuilder.UseStartup(); }); - - - } diff --git a/API/Services/BookService.cs b/API/Services/BookService.cs index 2c13892d8..669e97282 100644 --- a/API/Services/BookService.cs +++ b/API/Services/BookService.cs @@ -435,7 +435,7 @@ public class BookService : IBookService }; ComicInfo.CleanComicInfo(info); - foreach (var identifier in epubBook.Schema.Package.Metadata.Identifiers.Where(id => id.Scheme.Equals("ISBN"))) + foreach (var identifier in epubBook.Schema.Package.Metadata.Identifiers.Where(id => !string.IsNullOrEmpty(id.Scheme) && id.Scheme.Equals("ISBN"))) { if (string.IsNullOrEmpty(identifier.Identifier)) continue; var isbn = identifier.Identifier.Replace("urn:isbn:", string.Empty).Replace("isbn:", string.Empty); @@ -494,7 +494,7 @@ public class BookService : IBookService break; case "title-type": break; - // This is currently not possible until VersOne update's to allow EPUB 3 Title to have attributes + // This is currently not possible until VersOne update's to allow EPUB 3 Title to have attributes (3.3 update) if (!metadataItem.Content.Equals("collection")) break; var titleId = metadataItem.Refines.Replace("#", string.Empty); var readingListElem = epubBook.Schema.Package.Metadata.MetaItems.FirstOrDefault(item => @@ -855,7 +855,7 @@ public class BookService : IBookService /// /// /// - private static string CoalesceKey(EpubBookRef book, IDictionary mappings, string key) + private static string CoalesceKey(EpubBookRef book, IReadOnlyDictionary mappings, string key) { if (mappings.ContainsKey(CleanContentKeys(key))) return key; @@ -866,6 +866,19 @@ public class BookService : IBookService key = correctedKey; } + var stepsBack = CountParentDirectory(book.Content.NavigationHtmlFile.FileName); + if (mappings.TryGetValue(key, out _)) + { + return key; + } + + var modifiedKey = RemovePathSegments(key, stepsBack); + if (mappings.TryGetValue(modifiedKey, out _)) + { + return modifiedKey; + } + + return key; } @@ -904,7 +917,7 @@ public class BookService : IBookService { if (navigationItem.NestedItems.Count == 0) { - CreateToCChapter(navigationItem, Array.Empty(), chaptersList, mappings); + CreateToCChapter(book, navigationItem, Array.Empty(), chaptersList, mappings); continue; } @@ -913,19 +926,19 @@ public class BookService : IBookService foreach (var nestedChapter in navigationItem.NestedItems.Where(n => n.Link != null)) { var key = CoalesceKey(book, mappings, nestedChapter.Link.ContentFileName); - if (mappings.ContainsKey(key)) + if (mappings.TryGetValue(key, out var mapping)) { nestedChapters.Add(new BookChapterItem { Title = nestedChapter.Title, - Page = mappings[key], + Page = mapping, Part = nestedChapter.Link.Anchor ?? string.Empty, Children = new List() }); } } - CreateToCChapter(navigationItem, nestedChapters, chaptersList, mappings); + CreateToCChapter(book, navigationItem, nestedChapters, chaptersList, mappings); } if (chaptersList.Count != 0) return chaptersList; @@ -964,6 +977,38 @@ public class BookService : IBookService return chaptersList; } + private static int CountParentDirectory(string path) + { + const string pattern = @"\.\./"; + var matches = Regex.Matches(path, pattern); + + return matches.Count; + } + + /// + /// Removes paths segments from the beginning of a path. Returns original path if any issues. + /// + /// + /// + /// + private static string RemovePathSegments(string path, int segmentsToRemove) + { + if (segmentsToRemove <= 0) + return path; + + var startIndex = 0; + for (var i = 0; i < segmentsToRemove; i++) + { + var slashIndex = path.IndexOf('/', startIndex); + if (slashIndex == -1) + return path; // Not enough segments to remove + + startIndex = slashIndex + 1; + } + + return path.Substring(startIndex); + } + /// /// This returns a single page within the epub book. All html will be rewritten to be scoped within our reader, /// all css is scoped, etc. @@ -1028,7 +1073,7 @@ public class BookService : IBookService throw new KavitaException("Could not find the appropriate html for that page"); } - private static void CreateToCChapter(EpubNavigationItemRef navigationItem, IList nestedChapters, + private static void CreateToCChapter(EpubBookRef book, EpubNavigationItemRef navigationItem, IList nestedChapters, ICollection chaptersList, IReadOnlyDictionary mappings) { if (navigationItem.Link == null) @@ -1047,7 +1092,7 @@ public class BookService : IBookService } else { - var groupKey = CleanContentKeys(navigationItem.Link.ContentFileName); + var groupKey = CoalesceKey(book, mappings, navigationItem.Link.ContentFileName); if (mappings.ContainsKey(groupKey)) { chaptersList.Add(new BookChapterItem diff --git a/API/Services/SeriesService.cs b/API/Services/SeriesService.cs index a3b30c6da..5be0ebfd4 100644 --- a/API/Services/SeriesService.cs +++ b/API/Services/SeriesService.cs @@ -98,10 +98,10 @@ public class SeriesService : ISeriesService } // This shouldn't be needed post v0.5.3 release - if (string.IsNullOrEmpty(series.Metadata.Summary)) - { - series.Metadata.Summary = string.Empty; - } + // if (string.IsNullOrEmpty(series.Metadata.Summary)) + // { + // series.Metadata.Summary = string.Empty; + // } if (string.IsNullOrEmpty(updateSeriesMetadataDto.SeriesMetadata.Summary)) { @@ -120,7 +120,10 @@ public class SeriesService : ISeriesService series.Metadata.LanguageLocked = true; } - if (!string.IsNullOrEmpty(updateSeriesMetadataDto.SeriesMetadata?.WebLinks)) + if (string.IsNullOrEmpty(updateSeriesMetadataDto.SeriesMetadata?.WebLinks)) + { + series.Metadata.WebLinks = string.Empty; + } else { series.Metadata.WebLinks = string.Join(",", updateSeriesMetadataDto.SeriesMetadata?.WebLinks .Split(",") diff --git a/API/Services/TokenService.cs b/API/Services/TokenService.cs index 0af710597..d3609de48 100644 --- a/API/Services/TokenService.cs +++ b/API/Services/TokenService.cs @@ -5,10 +5,12 @@ using System.Linq; using System.Security.Claims; using System.Text; using System.Threading.Tasks; +using API.Data; using API.DTOs.Account; using API.Entities; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.Tokens; using static System.Security.Claims.ClaimTypes; using JwtRegisteredClaimNames = Microsoft.IdentityModel.JsonWebTokens.JwtRegisteredClaimNames; @@ -27,13 +29,17 @@ public interface ITokenService public class TokenService : ITokenService { private readonly UserManager _userManager; + private readonly ILogger _logger; + private readonly IUnitOfWork _unitOfWork; private readonly SymmetricSecurityKey _key; private const string RefreshTokenName = "RefreshToken"; - public TokenService(IConfiguration config, UserManager userManager) + public TokenService(IConfiguration config, UserManager userManager, ILogger logger, IUnitOfWork unitOfWork) { _userManager = userManager; + _logger = logger; + _unitOfWork = unitOfWork; _key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(config["TokenKey"] ?? string.Empty)); } @@ -78,12 +84,28 @@ public class TokenService : ITokenService var tokenHandler = new JwtSecurityTokenHandler(); var tokenContent = tokenHandler.ReadJwtToken(request.Token); var username = tokenContent.Claims.FirstOrDefault(q => q.Type == JwtRegisteredClaimNames.Name)?.Value; - if (string.IsNullOrEmpty(username)) return null; + if (string.IsNullOrEmpty(username)) + { + _logger.LogDebug("[RefreshToken] failed to validate due to not finding user in RefreshToken"); + return null; + } var user = await _userManager.FindByNameAsync(username); - if (user == null) return null; // This forces a logout + if (user == null) + { + _logger.LogDebug("[RefreshToken] failed to validate due to not finding user in DB"); + return null; + } + var validated = await _userManager.VerifyUserTokenAsync(user, TokenOptions.DefaultProvider, RefreshTokenName, request.RefreshToken); - if (!validated) return null; - await _userManager.UpdateSecurityStampAsync(user); + if (!validated) + { + + _logger.LogDebug("[RefreshToken] failed to validate due to invalid refresh token"); + return null; + } + + user.UpdateLastActive(); + await _unitOfWork.CommitAsync(); return new TokenRequestDto() { @@ -93,11 +115,13 @@ public class TokenService : ITokenService } catch (SecurityTokenExpiredException ex) { // Handle expired token + _logger.LogError(ex, "Failed to validate refresh token"); return null; } catch (Exception ex) { // Handle other exceptions + _logger.LogError(ex, "Failed to validate refresh token"); return null; } } diff --git a/Kavita.Common/Configuration.cs b/Kavita.Common/Configuration.cs index cde819cb7..6ea7441d5 100644 --- a/Kavita.Common/Configuration.cs +++ b/Kavita.Common/Configuration.cs @@ -10,6 +10,7 @@ public static class Configuration { public const string DefaultIpAddresses = "0.0.0.0,::"; public const string DefaultBaseUrl = "/"; + public const int DefaultHttpPort = 5000; public const string DefaultXFrameOptions = "SAMEORIGIN"; private static readonly string AppSettingsFilename = Path.Join("config", GetAppSettingFilename()); @@ -307,7 +308,7 @@ public static class Configuration // ReSharper disable once MemberHidesStaticFromOuterClass public int Port { get; set; } // ReSharper disable once MemberHidesStaticFromOuterClass - public string IpAddresses { get; set; } + public string IpAddresses { get; set; } = string.Empty; // ReSharper disable once MemberHidesStaticFromOuterClass public string BaseUrl { get; set; } } diff --git a/openapi.json b/openapi.json index 593debb5b..3ed643b75 100644 --- a/openapi.json +++ b/openapi.json @@ -7,7 +7,7 @@ "name": "GPL-3.0", "url": "https://github.com/Kareadita/Kavita/blob/develop/LICENSE" }, - "version": "0.7.2.7" + "version": "0.7.2.8" }, "servers": [ {