From 6e72c74fdefbf9cbd63937e2493eb6e52b6a00f9 Mon Sep 17 00:00:00 2001 From: Amelia <77553571+Fesaa@users.noreply.github.com> Date: Sun, 6 Jul 2025 16:42:21 +0200 Subject: [PATCH] Cleanup and some edge case fixes --- API/Controllers/AccountController.cs | 64 +++++------ API/Controllers/OidcControlller.cs | 9 ++ API/DTOs/Settings/OidcPublicConfigDto.cs | 2 +- API/Data/Seed.cs | 2 +- API/Extensions/EnumerableExtensions.cs | 6 + API/Extensions/IdentityServiceExtensions.cs | 14 +-- API/I18N/en.json | 1 + API/Services/AccountService.cs | 43 +++++-- API/Services/OidcService.cs | 106 +++++++++--------- API/Services/SettingsService.cs | 12 +- UI/Web/src/app/_models/user.ts | 2 +- UI/Web/src/app/_services/account.service.ts | 4 - UI/Web/src/app/_services/oidc.service.ts | 14 ++- UI/Web/src/app/admin/_models/oidc-config.ts | 26 ++--- .../admin/edit-user/edit-user.component.ts | 11 +- .../pdf-reader/pdf-reader.component.html | 2 +- UI/Web/src/assets/langs/en.json | 2 +- 17 files changed, 172 insertions(+), 148 deletions(-) diff --git a/API/Controllers/AccountController.cs b/API/Controllers/AccountController.cs index 76ccd95ea..640427114 100644 --- a/API/Controllers/AccountController.cs +++ b/API/Controllers/AccountController.cs @@ -77,14 +77,24 @@ public class AccountController : BaseApiController _oidcService = oidcService; } + /// + /// Returns the current user, as it would from login + /// + /// + /// + /// Also throws UnauthorizedAccessException if the users is missing the Login role + /// Syncs Oidc settings if enabled, and user is Oidc owned [HttpGet] public async Task> GetCurrentUserAsync() { var user = await _unitOfWork.UserRepository.GetUserByIdAsync(User.GetUserId(), AppUserIncludes.UserPreferences | AppUserIncludes.SideNavStreams); if (user == null) throw new UnauthorizedAccessException(); - var oidcSettings = (await _unitOfWork.SettingsRepository.GetSettingsDtoAsync()).OidcConfig; - await _oidcService.SyncUserSettings(oidcSettings, User, user); + if (user.Owner == AppUserOwner.OpenIdConnect) + { + var oidcSettings = (await _unitOfWork.SettingsRepository.GetSettingsDtoAsync()).OidcConfig; + await _oidcService.SyncUserSettings(oidcSettings, User, user); + } var roles = await _userManager.GetRolesAsync(user); if (!roles.Contains(PolicyConstants.LoginRole)) return Unauthorized(await _localizationService.Translate(user.Id, "disabled-account")); @@ -169,10 +179,10 @@ public class AccountController : BaseApiController if (!result.Succeeded) return BadRequest(result.Errors); // Assign default streams - AddDefaultStreamsToUser(user); + _accountService.AddDefaultStreamsToUser(user); // Assign default reading profile - await AddDefaultReadingProfileToUser(user); + await _accountService.AddDefaultReadingProfileToUser(user); var token = await _userManager.GenerateEmailConfirmationTokenAsync(user); if (string.IsNullOrEmpty(token)) return BadRequest(await _localizationService.Get("en", "confirm-token-gen")); @@ -243,7 +253,7 @@ public class AccountController : BaseApiController if (!roles.Contains(PolicyConstants.LoginRole)) return Unauthorized(await _localizationService.Translate(user.Id, "disabled-account")); var oidcConfig = (await _unitOfWork.SettingsRepository.GetSettingsDtoAsync()).OidcConfig; - // Setting only takes effect if OIDC is funcitonal, and if we're not logging in via ApiKey + // Setting only takes effect if OIDC is functional, and if we're not logging in via ApiKey var disablePasswordAuthentication = oidcConfig is {Enabled: true, DisablePasswordAuthentication: true} && string.IsNullOrEmpty(loginDto.ApiKey); if (disablePasswordAuthentication && !roles.Contains(PolicyConstants.AdminRole)) return Unauthorized(await _localizationService.Translate(user.Id, "password-authentication-disabled")); @@ -545,19 +555,24 @@ public class AccountController : BaseApiController var user = await _unitOfWork.UserRepository.GetUserByIdAsync(dto.UserId, AppUserIncludes.SideNavStreams); if (user == null) return BadRequest(await _localizationService.Translate(User.GetUserId(), "no-user")); - // Disallowed editing users synced via OIDC + // Disallowed editing users owned by OIDC var oidcSettings = (await _unitOfWork.SettingsRepository.GetSettingsDtoAsync()).OidcConfig; - if (user.Owner == AppUserOwner.OpenIdConnect && - dto.Owner != AppUserOwner.Native && - oidcSettings.SyncUserSettings) + if (user.Owner == AppUserOwner.OpenIdConnect && dto.Owner != AppUserOwner.Native && oidcSettings.SyncUserSettings) { return BadRequest(await _localizationService.Translate(User.GetUserId(), "oidc-managed")); } var defaultAdminUser = await _unitOfWork.UserRepository.GetDefaultAdminUser(); - if (user.Id != defaultAdminUser.Id) + if (user.Id == defaultAdminUser.Id && dto.Owner != AppUserOwner.Native) { - user.Owner = dto.Owner; + return BadRequest(await _localizationService.Translate(User.GetUserId(), "cannot-change-ownership-original-user")); + } + + if (user.Owner == AppUserOwner.OpenIdConnect) + { + // Do not change any other fields when the user is owned by OIDC + await _unitOfWork.CommitAsync(); + return Ok(); } // Check if username is changing @@ -713,10 +728,10 @@ public class AccountController : BaseApiController if (!result.Succeeded) return BadRequest(result.Errors); // Assign default streams - AddDefaultStreamsToUser(user); + _accountService.AddDefaultStreamsToUser(user); // Assign default reading profile - await AddDefaultReadingProfileToUser(user); + await _accountService.AddDefaultReadingProfileToUser(user); // Assign Roles var roles = dto.Roles; @@ -815,29 +830,6 @@ public class AccountController : BaseApiController return BadRequest(await _localizationService.Translate(User.GetUserId(), "generic-invite-user")); } - private void AddDefaultStreamsToUser(AppUser user) - { - foreach (var newStream in Seed.DefaultStreams.Select(stream => _mapper.Map(stream))) - { - user.DashboardStreams.Add(newStream); - } - - foreach (var stream in Seed.DefaultSideNavStreams.Select(stream => _mapper.Map(stream))) - { - user.SideNavStreams.Add(stream); - } - } - - private async Task AddDefaultReadingProfileToUser(AppUser user) - { - var profile = new AppUserReadingProfileBuilder(user.Id) - .WithName("Default Profile") - .WithKind(ReadingProfileKind.Default) - .Build(); - _unitOfWork.AppUserReadingProfileRepository.Add(profile); - await _unitOfWork.CommitAsync(); - } - /// /// Last step in authentication flow, confirms the email token for email /// diff --git a/API/Controllers/OidcControlller.cs b/API/Controllers/OidcControlller.cs index e0ba62bcc..4b74688d6 100644 --- a/API/Controllers/OidcControlller.cs +++ b/API/Controllers/OidcControlller.cs @@ -13,6 +13,10 @@ public class OidcController(ILogger logger, IUnitOfWork unitOfWo IMapper mapper, ISettingsService settingsService): BaseApiController { + /// + /// Retrieve publicly required configuration regarding Oidc + /// + /// [AllowAnonymous] [HttpGet("config")] public async Task> GetOidcConfig() @@ -21,6 +25,11 @@ public class OidcController(ILogger logger, IUnitOfWork unitOfWo return Ok(mapper.Map(settings.OidcConfig)); } + /// + /// Validate if the given authority is reachable from the server + /// + /// + /// [Authorize("RequireAdminRole")] [HttpPost("is-valid-authority")] public async Task> IsValidAuthority([FromBody] IsValidAuthorityBody authority) diff --git a/API/DTOs/Settings/OidcPublicConfigDto.cs b/API/DTOs/Settings/OidcPublicConfigDto.cs index 171e46d7e..7861161ea 100644 --- a/API/DTOs/Settings/OidcPublicConfigDto.cs +++ b/API/DTOs/Settings/OidcPublicConfigDto.cs @@ -10,7 +10,7 @@ public record OidcPublicConfigDto /// public string? ClientId { get; set; } /// - /// Optional OpenID Connect ClientSecret, required if authority is set + /// Automatically redirect to the Oidc login screen /// public bool AutoLogin { get; set; } /// diff --git a/API/Data/Seed.cs b/API/Data/Seed.cs index 12084e20a..914d79b9a 100644 --- a/API/Data/Seed.cs +++ b/API/Data/Seed.cs @@ -254,7 +254,7 @@ public static class Seed new() { Key = ServerSettingKey.CacheSize, Value = Configuration.DefaultCacheMemory + string.Empty }, // Not used from DB, but DB is sync with appSettings.json - new() { Key = ServerSettingKey.OidcAuthority, Value = Configuration.OidcAuthority }, + new() { Key = ServerSettingKey.OidcAuthority, Value = Configuration.OidcAuthority}, new() { Key = ServerSettingKey.OidcClientId, Value = Configuration.OidcClientId}, new() { Key = ServerSettingKey.OidcConfiguration, Value = JsonSerializer.Serialize(new OidcConfigDto())}, diff --git a/API/Extensions/EnumerableExtensions.cs b/API/Extensions/EnumerableExtensions.cs index 9bc06bab4..bcdd2808b 100644 --- a/API/Extensions/EnumerableExtensions.cs +++ b/API/Extensions/EnumerableExtensions.cs @@ -6,6 +6,7 @@ using API.Data.Misc; using API.Entities; using API.Entities.Enums; using API.Entities.Metadata; +using Microsoft.AspNetCore.Identity; namespace API.Extensions; #nullable enable @@ -68,4 +69,9 @@ public static class EnumerableExtensions return q; } + + public static string AsJoinedString(this IEnumerable errors) + { + return string.Join(",", errors.Select(e => e.Description)); + } } diff --git a/API/Extensions/IdentityServiceExtensions.cs b/API/Extensions/IdentityServiceExtensions.cs index e5f03430c..26958193a 100644 --- a/API/Extensions/IdentityServiceExtensions.cs +++ b/API/Extensions/IdentityServiceExtensions.cs @@ -146,14 +146,10 @@ public static class IdentityServiceExtensions }); - services.AddAuthorization(opt => - { - opt.AddPolicy("RequireAdminRole", policy => policy.RequireRole(PolicyConstants.AdminRole)); - opt.AddPolicy("RequireDownloadRole", - policy => policy.RequireRole(PolicyConstants.DownloadRole, PolicyConstants.AdminRole)); - opt.AddPolicy("RequireChangePasswordRole", - policy => policy.RequireRole(PolicyConstants.ChangePasswordRole, PolicyConstants.AdminRole)); - }); + services.AddAuthorizationBuilder() + .AddPolicy("RequireAdminRole", policy => policy.RequireRole(PolicyConstants.AdminRole)) + .AddPolicy("RequireDownloadRole", policy => policy.RequireRole(PolicyConstants.DownloadRole, PolicyConstants.AdminRole)) + .AddPolicy("RequireChangePasswordRole", policy => policy.RequireRole(PolicyConstants.ChangePasswordRole, PolicyConstants.AdminRole)); return services; } @@ -163,7 +159,6 @@ public static class IdentityServiceExtensions if (ctx.Principal == null) return; var oidcService = ctx.HttpContext.RequestServices.GetRequiredService(); - var unitOfWork = ctx.HttpContext.RequestServices.GetRequiredService(); var user = await oidcService.LoginOrCreate(ctx.Principal); if (user == null) { @@ -180,6 +175,7 @@ public static class IdentityServiceExtensions new(ClaimTypes.Name, user.UserName ?? string.Empty), }; + var unitOfWork = ctx.HttpContext.RequestServices.GetRequiredService(); var settings = await unitOfWork.SettingsRepository.GetSettingsDtoAsync(); if (user.Owner != AppUserOwner.OpenIdConnect || !settings.OidcConfig.SyncUserSettings) { diff --git a/API/I18N/en.json b/API/I18N/en.json index d1e8346ac..e598be54f 100644 --- a/API/I18N/en.json +++ b/API/I18N/en.json @@ -19,6 +19,7 @@ "age-restriction-update": "There was an error updating the age restriction", "no-user": "User does not exist", "oidc-managed": "This user is managed by OIDC, cannot edit", + "cannot-change-ownership-original-user": "Ownership of the original admin account cannot be changed", "username-taken": "Username already taken", "email-taken": "Email already in use", "user-already-confirmed": "User is already confirmed", diff --git a/API/Services/AccountService.cs b/API/Services/AccountService.cs index e331030ee..09f0df583 100644 --- a/API/Services/AccountService.cs +++ b/API/Services/AccountService.cs @@ -2,19 +2,20 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using System.Web; using API.Constants; using API.Data; using API.Data.Repositories; using API.DTOs.Account; using API.Entities; +using API.Entities.Enums; using API.Errors; using API.Extensions; +using API.Helpers.Builders; +using API.SignalR; +using AutoMapper; using Kavita.Common; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; namespace API.Services; @@ -41,6 +42,8 @@ public interface IAccountService /// Ensure that the users SideNavStreams are loaded Task UpdateLibrariesForUser(AppUser user, IList librariesIds, bool hasAdminRole); Task> UpdateRolesForUser(AppUser user, IList roles); + void AddDefaultStreamsToUser(AppUser user); + Task AddDefaultReadingProfileToUser(AppUser user); } public class AccountService : IAccountService @@ -48,13 +51,16 @@ public class AccountService : IAccountService private readonly UserManager _userManager; private readonly ILogger _logger; private readonly IUnitOfWork _unitOfWork; + private readonly IMapper _mapper; public const string DefaultPassword = "[k.2@RZ!mxCQkJzE"; - public AccountService(UserManager userManager, ILogger logger, IUnitOfWork unitOfWork) + public AccountService(UserManager userManager, ILogger logger, IUnitOfWork unitOfWork, + IMapper mapper) { _userManager = userManager; _logger = logger; _unitOfWork = unitOfWork; + _mapper = mapper; } public async Task> ChangeUserPassword(AppUser user, string newPassword) @@ -158,11 +164,11 @@ public class AccountService : IAccountService public async Task UpdateLibrariesForUser(AppUser user, IList librariesIds, bool hasAdminRole) { - var allLibraries = (await _unitOfWork.LibraryRepository.GetLibrariesAsync()).ToList(); + var allLibraries = (await _unitOfWork.LibraryRepository.GetLibrariesAsync(LibraryIncludes.AppUser)).ToList(); List libraries; if (hasAdminRole) { - _logger.LogInformation("{UserName} is being registered as admin. Granting access to all libraries", + _logger.LogInformation("{UserName} is admin. Granting access to all libraries", user.UserName); libraries = allLibraries; } @@ -176,7 +182,7 @@ public class AccountService : IAccountService user.RemoveSideNavFromLibrary(lib); } - libraries = (await _unitOfWork.LibraryRepository.GetLibraryForIdsAsync(librariesIds, LibraryIncludes.AppUser)).ToList(); + libraries = allLibraries.Where(lib => librariesIds.Contains(lib.Id)).ToList(); } foreach (var lib in libraries) @@ -207,4 +213,27 @@ public class AccountService : IAccountService return []; } + + public void AddDefaultStreamsToUser(AppUser user) + { + foreach (var newStream in Seed.DefaultStreams.Select(_mapper.Map)) + { + user.DashboardStreams.Add(newStream); + } + + foreach (var stream in Seed.DefaultSideNavStreams.Select(_mapper.Map)) + { + user.SideNavStreams.Add(stream); + } + } + + public async Task AddDefaultReadingProfileToUser(AppUser user) + { + var profile = new AppUserReadingProfileBuilder(user.Id) + .WithName("Default Profile") + .WithKind(ReadingProfileKind.Default) + .Build(); + _unitOfWork.AppUserReadingProfileRepository.Add(profile); + await _unitOfWork.CommitAsync(); + } } diff --git a/API/Services/OidcService.cs b/API/Services/OidcService.cs index af236cc33..a95361f58 100644 --- a/API/Services/OidcService.cs +++ b/API/Services/OidcService.cs @@ -12,7 +12,6 @@ using API.Entities; using API.Entities.Enums; using API.Extensions; using API.Helpers.Builders; -using AutoMapper; using Kavita.Common; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; @@ -43,7 +42,7 @@ public interface IOidcService } public class OidcService(ILogger logger, UserManager userManager, - IUnitOfWork unitOfWork, IMapper mapper, IAccountService accountService): IOidcService + IUnitOfWork unitOfWork, IAccountService accountService): IOidcService { private const string LibraryAccessPrefix = "library-"; private const string AgeRatingPrefix = "age-rating-"; @@ -71,7 +70,7 @@ public class OidcService(ILogger logger, UserManager userM user = await unitOfWork.UserRepository.GetUserByEmailAsync(email, AppUserIncludes.UserPreferences | AppUserIncludes.SideNavStreams); if (user != null) { - logger.LogInformation("User {Name} has matched on email to {ExternalId}", user.UserName, externalId); + logger.LogInformation("User {UserName} has matched on email to {ExternalId}", user.Id, externalId); user.ExternalId = externalId; await unitOfWork.CommitAsync(); return user; @@ -85,7 +84,7 @@ public class OidcService(ILogger logger, UserManager userM if (user == null) return null; var roles = await userManager.GetRolesAsync(user); - if (roles.Count > 0 && !roles.Contains(PolicyConstants.LoginRole)) + if (roles.Count == 0 || !roles.Contains(PolicyConstants.LoginRole)) throw new KavitaException("errors.oidc.disabled-account"); return user; @@ -102,6 +101,30 @@ public class OidcService(ILogger logger, UserManager userM await unitOfWork.CommitAsync(); } + private async Task FindBestAvailableName(ClaimsPrincipal claimsPrincipal) + { + var name = claimsPrincipal.FindFirstValue(JwtRegisteredClaimNames.PreferredUsername); + if (await IsNameAvailable(name)) return name; + + name = claimsPrincipal.FindFirstValue(ClaimTypes.Name); + if (await IsNameAvailable(name)) return name; + + name = claimsPrincipal.FindFirstValue(ClaimTypes.GivenName); + if (await IsNameAvailable(name)) return name; + + name = claimsPrincipal.FindFirstValue(ClaimTypes.Surname); + if (await IsNameAvailable(name)) return name; + + return null; + } + + private async Task IsNameAvailable(string? name) + { + if (string.IsNullOrEmpty(name)) return false; + + return await userManager.FindByNameAsync(name) == null; + } + private async Task NewUserFromOpenIdConnect(OidcConfigDto settings, ClaimsPrincipal claimsPrincipal, string externalId) { if (!settings.ProvisionAccounts) return null; @@ -109,20 +132,7 @@ public class OidcService(ILogger logger, UserManager userM var emailClaim = claimsPrincipal.FindFirst(ClaimTypes.Email); if (emailClaim == null || string.IsNullOrWhiteSpace(emailClaim.Value)) return null; - // TODO?: Try one by one, for more chance of a nicer username - var name = claimsPrincipal.FindFirstValue(JwtRegisteredClaimNames.PreferredUsername); - name ??= claimsPrincipal.FindFirstValue(ClaimTypes.Name); - name ??= claimsPrincipal.FindFirstValue(ClaimTypes.GivenName); - name ??= claimsPrincipal.FindFirstValue(ClaimTypes.Surname); - name ??= emailClaim.Value; - - var other = await userManager.FindByNameAsync(name); - if (other != null) - { - // We match by email, so this will always be unique - name = emailClaim.Value; - } - + var name = await FindBestAvailableName(claimsPrincipal) ?? emailClaim.Value; logger.LogInformation("Creating new user from OIDC: {Name} - {ExternalId}", name, externalId); // TODO: Move to account service, as we're sharing code with AccountController @@ -147,8 +157,8 @@ public class OidcService(ILogger logger, UserManager userM user.ExternalId = externalId; user.Owner = AppUserOwner.OpenIdConnect; - AddDefaultStreamsToUser(user, mapper); - await AddDefaultReadingProfileToUser(user); + accountService.AddDefaultStreamsToUser(user); + await accountService.AddDefaultReadingProfileToUser(user); await SyncUserSettings(settings, claimsPrincipal, user); await SetDefaults(settings, user); @@ -161,6 +171,9 @@ public class OidcService(ILogger logger, UserManager userM { if (settings.SyncUserSettings) return; + logger.LogDebug("Assigning defaults to newly created user; Roles: {Roles}, Libraries: {Libraries}, AgeRating: {AgeRating}, IncludeUnknowns: {IncludeUnknowns}", + settings.DefaultRoles, settings.DefaultLibraries, settings.DefaultAgeRating, settings.DefaultIncludeUnknowns); + // Assign roles var errors = await accountService.UpdateRolesForUser(user, settings.DefaultRoles); if (errors.Any()) throw new KavitaException("errors.oidc.syncing-user"); @@ -179,13 +192,14 @@ public class OidcService(ILogger logger, UserManager userM { if (!settings.SyncUserSettings) return; + // Never sync the default user var defaultAdminUser = await unitOfWork.UserRepository.GetDefaultAdminUser(); if (defaultAdminUser.Id == user.Id) return; - logger.LogDebug("Syncing user {UserId} from OIDC", user.Id); + logger.LogInformation("Syncing user {UserName} from OIDC", user.UserName); await SyncRoles(claimsPrincipal, user); await SyncLibraries(claimsPrincipal, user); - SyncAgeRestriction(claimsPrincipal, user); + await SyncAgeRestriction(claimsPrincipal, user); if (unitOfWork.HasChanges()) @@ -195,39 +209,45 @@ public class OidcService(ILogger logger, UserManager userM private async Task SyncRoles(ClaimsPrincipal claimsPrincipal, AppUser user) { var roles = claimsPrincipal.GetAccessRoles(); - logger.LogDebug("Syncing access roles for user {UserId}, found roles {Roles}", user.Id, roles); + logger.LogDebug("Syncing access roles for user {UserName}, found roles {Roles}", user.UserName, roles); + var errors = await accountService.UpdateRolesForUser(user, roles); if (errors.Any()) throw new KavitaException("errors.oidc.syncing-user"); } private async Task SyncLibraries(ClaimsPrincipal claimsPrincipal, AppUser user) { - var hasAdminRole = await userManager.IsInRoleAsync(user, PolicyConstants.AdminRole); - var libraryAccess = claimsPrincipal .FindAll(ClaimTypes.Role) .Where(r => r.Value.StartsWith(LibraryAccessPrefix)) .Select(r => r.Value.TrimPrefix(LibraryAccessPrefix)) .ToList(); - logger.LogDebug("Syncing libraries for user {UserId}, found library roles {Roles}", user.Id, libraryAccess); - if (libraryAccess.Count == 0 && !hasAdminRole) return; + + logger.LogDebug("Syncing libraries for user {UserName}, found library roles {Roles}", user.UserName, libraryAccess); var allLibraries = (await unitOfWork.LibraryRepository.GetLibrariesAsync()).ToList(); var librariesIds = allLibraries.Where(l => libraryAccess.Contains(l.Name)).Select(l => l.Id).ToList(); + var hasAdminRole = await userManager.IsInRoleAsync(user, PolicyConstants.AdminRole); await accountService.UpdateLibrariesForUser(user, librariesIds, hasAdminRole); } - private void SyncAgeRestriction(ClaimsPrincipal claimsPrincipal, AppUser user) + private async Task SyncAgeRestriction(ClaimsPrincipal claimsPrincipal, AppUser user) { + if (await userManager.IsInRoleAsync(user, PolicyConstants.AdminRole)) + { + logger.LogInformation("User {UserName} is admin, granting access to all age ratings", user.UserName); + user.AgeRestriction = AgeRating.NotApplicable; + user.AgeRestrictionIncludeUnknowns = true; + return; + } var ageRatings = claimsPrincipal .FindAll(ClaimTypes.Role) .Where(r => r.Value.StartsWith(AgeRatingPrefix)) .Select(r => r.Value.TrimPrefix(AgeRatingPrefix)) .ToList(); - logger.LogDebug("Syncing age restriction for user {UserId}, found restrictions {Restrictions}", user.Id, ageRatings); - if (ageRatings.Count == 0) return; + logger.LogDebug("Syncing age restriction for user {UserName}, found restrictions {Restrictions}", user.UserName, ageRatings); var highestAgeRating = AgeRating.Unknown; @@ -243,29 +263,9 @@ public class OidcService(ILogger logger, UserManager userM user.AgeRestriction = highestAgeRating; user.AgeRestrictionIncludeUnknowns = ageRatings.Contains(IncludeUnknowns); + + logger.LogDebug("Synced age restriction for user {UserName}, AgeRestriction {AgeRestriction}, IncludeUnknowns: {IncludeUnknowns}", + user.UserName, ageRatings, user.AgeRestrictionIncludeUnknowns); } - // DUPLICATED CODE - private static void AddDefaultStreamsToUser(AppUser user, IMapper mapper) - { - foreach (var newStream in Seed.DefaultStreams.Select(mapper.Map)) - { - user.DashboardStreams.Add(newStream); - } - - foreach (var stream in Seed.DefaultSideNavStreams.Select(mapper.Map)) - { - user.SideNavStreams.Add(stream); - } - } - - private async Task AddDefaultReadingProfileToUser(AppUser user) - { - var profile = new AppUserReadingProfileBuilder(user.Id) - .WithName("Default Profile") - .WithKind(ReadingProfileKind.Default) - .Build(); - unitOfWork.AppUserReadingProfileRepository.Add(profile); - await unitOfWork.CommitAsync(); - } } diff --git a/API/Services/SettingsService.cs b/API/Services/SettingsService.cs index 071968237..d41684b78 100644 --- a/API/Services/SettingsService.cs +++ b/API/Services/SettingsService.cs @@ -16,7 +16,6 @@ using Hangfire; using Kavita.Common; using Kavita.Common.EnvironmentInfo; using Kavita.Common.Helpers; -using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -367,10 +366,9 @@ public class SettingsService : ISettingsService var url = authority + "/.well-known/openid-configuration"; try { - //await url.GetJsonAsync(); - //return true; - var res = await url.GetAsync(); - return res.StatusCode == 200; + var json = await url.GetStringAsync(); + var config = OpenIdConnectConfiguration.Create(json); + return config.Issuer == Configuration.OidcAuthority; } catch (Exception e) { @@ -420,9 +418,11 @@ public class SettingsService : ISettingsService { throw new KavitaException("oidc-invalid-authority"); } + setting.Value = updateSettingsDto.OidcConfig.Authority; Configuration.OidcAuthority = updateSettingsDto.OidcConfig.Authority; _unitOfWork.SettingsRepository.Update(setting); + _logger.LogWarning("OIDC Authority is changing, clearing all external ids"); await _oidcService.ClearOidcIds(); return; @@ -441,7 +441,7 @@ public class SettingsService : ISettingsService var newValue = JsonSerializer.Serialize(updateSettingsDto.OidcConfig); if (setting.Value == newValue) return; - setting.Value = JsonSerializer.Serialize(updateSettingsDto.OidcConfig); + setting.Value = newValue; _unitOfWork.SettingsRepository.Update(setting); } diff --git a/UI/Web/src/app/_models/user.ts b/UI/Web/src/app/_models/user.ts index 5a8cb70b0..7ffbbdf9c 100644 --- a/UI/Web/src/app/_models/user.ts +++ b/UI/Web/src/app/_models/user.ts @@ -6,7 +6,7 @@ export interface User { username: string; // This is set by the oidc service, will always take precedence over the Kavita generated token // When set, the refresh logic for the Kavita token will not run - oidcToken: string; + oidcToken?: string; token: string; refreshToken: string; roles: string[]; diff --git a/UI/Web/src/app/_services/account.service.ts b/UI/Web/src/app/_services/account.service.ts index 52a618fc5..7395d106c 100644 --- a/UI/Web/src/app/_services/account.service.ts +++ b/UI/Web/src/app/_services/account.service.ts @@ -92,10 +92,6 @@ export class AccountService { }); } - oidcEnabled() { - return this.httpClient.get(this.baseUrl + "oidc/enabled"); - } - canInvokeAction(user: User, action: Action) { const isAdmin = this.hasAdminRole(user); const canDownload = this.hasDownloadRole(user); diff --git a/UI/Web/src/app/_services/oidc.service.ts b/UI/Web/src/app/_services/oidc.service.ts index 4710e5cbe..c8fc7ec05 100644 --- a/UI/Web/src/app/_services/oidc.service.ts +++ b/UI/Web/src/app/_services/oidc.service.ts @@ -10,6 +10,7 @@ import {take} from "rxjs/operators"; import {ToastrService} from "ngx-toastr"; import {translate} from "@jsverse/transloco"; import {APP_BASE_HREF} from "@angular/common"; +import {MessageHubService} from "./message-hub.service"; @Injectable({ providedIn: 'root' @@ -21,6 +22,7 @@ export class OidcService { private readonly accountService = inject(AccountService); private readonly destroyRef = inject(DestroyRef); private readonly toastR = inject(ToastrService); + private readonly messageHub = inject(MessageHubService); protected readonly baseUrl = inject(APP_BASE_HREF); apiBaseUrl = environment.apiUrl; @@ -33,7 +35,7 @@ export class OidcService { public readonly loaded$ = toObservable(this.loaded); /** - * OIDC discovery document has been loaded, and login tried and OIDC has been set up + * OIDC discovery document has been loaded, login tried and OIDC has been set up */ private readonly _ready = signal(false); public readonly ready = this._ready.asReadonly(); @@ -62,12 +64,15 @@ export class OidcService { this.accountService.currentUser$.pipe(take(1)).subscribe(user => { if (!user) return; // Don't update tokens when we're not logged in. But what's going on? - // TODO: Do we need to refresh the SignalR connection here? user.oidcToken = this.token; + this.messageHub.stopHubConnection(); + this.messageHub.createHubConnection(user); }); }); - this.config().subscribe(oidcSetting => { + this.getPublicOidcConfig().subscribe(oidcSetting => { + this._settings.set(oidcSetting); + if (!oidcSetting.authority) { this._loaded.set(true); return @@ -86,7 +91,6 @@ export class OidcService { // Not all OIDC providers follow this nicely strictDiscoveryDocumentValidation: false, }); - this._settings.set(oidcSetting); this.oauth2.setupAutomaticSilentRefresh(); from(this.oauth2.loadDiscoveryDocumentAndTryLogin()).subscribe({ @@ -113,7 +117,7 @@ export class OidcService { } } - config() { + getPublicOidcConfig() { return this.httpClient.get(this.apiBaseUrl + "oidc/config"); } diff --git a/UI/Web/src/app/admin/_models/oidc-config.ts b/UI/Web/src/app/admin/_models/oidc-config.ts index d48cdf088..f67d8c3e1 100644 --- a/UI/Web/src/app/admin/_models/oidc-config.ts +++ b/UI/Web/src/app/admin/_models/oidc-config.ts @@ -1,20 +1,5 @@ import {AgeRating} from "../../_models/metadata/age-rating"; -export interface OidcConfig { - authority: string; - clientId: string; - provisionAccounts: boolean; - requireVerifiedEmail: boolean; - syncUserSettings: boolean; - autoLogin: boolean; - disablePasswordAuthentication: boolean; - providerName: string; - defaultRoles: string[]; - defaultLibraries: number[]; - defaultAgeRating: AgeRating; - defaultIncludeUnknowns: boolean; -} - export interface OidcPublicConfig { authority: string; clientId: string; @@ -22,3 +7,14 @@ export interface OidcPublicConfig { disablePasswordAuthentication: boolean; providerName: string; } + +export interface OidcConfig extends OidcPublicConfig { + provisionAccounts: boolean; + requireVerifiedEmail: boolean; + syncUserSettings: boolean; + defaultRoles: string[]; + defaultLibraries: number[]; + defaultAgeRating: AgeRating; + defaultIncludeUnknowns: boolean; +} + diff --git a/UI/Web/src/app/admin/edit-user/edit-user.component.ts b/UI/Web/src/app/admin/edit-user/edit-user.component.ts index 124810678..28832d1aa 100644 --- a/UI/Web/src/app/admin/edit-user/edit-user.component.ts +++ b/UI/Web/src/app/admin/edit-user/edit-user.component.ts @@ -3,10 +3,9 @@ import { ChangeDetectorRef, Component, computed, - DestroyRef, effect, + DestroyRef, inject, - input, - Input, model, + model, OnInit } from '@angular/core'; import {FormControl, FormGroup, ReactiveFormsModule, Validators} from '@angular/forms'; @@ -27,8 +26,6 @@ import {takeUntilDestroyed} from "@angular/core/rxjs-interop"; import {ServerSettings} from "../_models/server-settings"; import {UserOwner, UserOwners} from "../../_models/user"; import {UserOwnerPipe} from "../../_pipes/user-owner.pipe"; -import {SettingItemComponent} from "../../settings/_components/setting-item/setting-item.component"; -import {OwnerIconComponent} from "../../shared/_components/owner-icon/owner-icon.component"; const AllowedUsernameCharacters = /^[\sa-zA-Z0-9\-._@+/\s]*$/; const EmailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; @@ -37,7 +34,7 @@ const EmailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; selector: 'app-edit-user', templateUrl: './edit-user.component.html', styleUrls: ['./edit-user.component.scss'], - imports: [ReactiveFormsModule, RoleSelectorComponent, LibrarySelectorComponent, RestrictionSelectorComponent, SentenceCasePipe, TranslocoDirective, AsyncPipe, UserOwnerPipe, SettingItemComponent, OwnerIconComponent], + imports: [ReactiveFormsModule, RoleSelectorComponent, LibrarySelectorComponent, RestrictionSelectorComponent, SentenceCasePipe, TranslocoDirective, AsyncPipe, UserOwnerPipe], changeDetection: ChangeDetectionStrategy.OnPush }) export class EditUserComponent implements OnInit { @@ -80,8 +77,6 @@ export class EditUserComponent implements OnInit { this.userForm.addControl('username', new FormControl(this.member().username, [Validators.required, Validators.pattern(AllowedUsernameCharacters)])); this.userForm.addControl('owner', new FormControl(this.member().owner, [Validators.required])); - // TODO: Rework, bad hack - // Work around isLocked so we're able to downgrade users this.userForm.get('owner')!.valueChanges.pipe( tap(value => { const newOwner = parseInt(value, 10) as UserOwner; diff --git a/UI/Web/src/app/pdf-reader/_components/pdf-reader/pdf-reader.component.html b/UI/Web/src/app/pdf-reader/_components/pdf-reader/pdf-reader.component.html index 7e75e541c..c3ce4bf50 100644 --- a/UI/Web/src/app/pdf-reader/_components/pdf-reader/pdf-reader.component.html +++ b/UI/Web/src/app/pdf-reader/_components/pdf-reader/pdf-reader.component.html @@ -17,7 +17,7 @@