Bunch of OIDC fixes and one extra (#4126)

Author: Fesaa

API/Controllers/OPDSController.cs

@@ -91,7 +91,7 @@ private async Task<Tuple<string, string>> GetPrefix()
         if (!Configuration.DefaultBaseUrl.Equals(baseUrl, StringComparison.InvariantCultureIgnoreCase))
         {
             // We need to update the Prefix to account for baseUrl
-            prefix = baseUrl + OpdsService.DefaultApiPrefix;
+            prefix = baseUrl.TrimEnd('/') + OpdsService.DefaultApiPrefix;
         }
 
         return new Tuple<string, string>(baseUrl, prefix);

API/Controllers/OidcController.cs

@@ -1,12 +1,10 @@
-using System;
-using System.Linq;
-using System.Threading.Tasks;
+using System.Threading.Tasks;
 using API.Extensions;
 using API.Services;
+using Kavita.Common;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 
 namespace API.Controllers;
@@ -19,6 +17,11 @@ public class OidcController: ControllerBase
     [HttpGet("login")]
     public IActionResult Login(string returnUrl = "/")
     {
+        if (returnUrl == "/")
+        {
+            returnUrl = Configuration.BaseUrl;
+        }
+
         var properties = new AuthenticationProperties { RedirectUri = returnUrl };
         return Challenge(properties, IdentityServiceExtensions.OpenIdConnect);
     }
@@ -29,18 +32,18 @@ public async Task<IActionResult> Logout()
 
         if (!Request.Cookies.ContainsKey(OidcService.CookieName))
         {
-            return Redirect("/");
+            return Redirect(Configuration.BaseUrl);
         }
 
         var res = await Request.HttpContext.AuthenticateAsync(CookieAuthenticationDefaults.AuthenticationScheme);
-        if (!res.Succeeded || res.Properties == null || string.IsNullOrEmpty(res.Properties.GetString(OidcService.IdToken)))
+        if (!res.Succeeded || res.Properties == null || string.IsNullOrEmpty(res.Properties.GetTokenValue(OidcService.IdToken)))
         {
             HttpContext.Response.Cookies.Delete(OidcService.CookieName);
-            return Redirect("/");
+            return Redirect(Configuration.BaseUrl);
         }
 
         return SignOut(
-            new AuthenticationProperties { RedirectUri = "/login" },
+            new AuthenticationProperties { RedirectUri = Configuration.BaseUrl+"login" },
             CookieAuthenticationDefaults.AuthenticationScheme,
             IdentityServiceExtensions.OpenIdConnect);
     }

API/Extensions/ApplicationServiceExtensions.cs

@@ -117,6 +117,7 @@ public static void AddApplicationServices(this IServiceCollection services, ICon
             options.SizeLimit = Configuration.CacheSize * 1024 * 1024; // 75 MB
             options.CompactionPercentage = 0.1; // LRU compaction (10%)
         });
+        // Needs to be registered after the memory cache, as it depends on it
         services.AddSingleton<ITicketStore, CustomTicketStore>();
 
         services.AddSwaggerGen(g =>

API/Extensions/IdentityServiceExtensions.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.IdentityModel.Tokens.Jwt;
 using System.Linq;
 using System.Security.Claims;
 using System.Text;
@@ -20,8 +21,10 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
+using Microsoft.IdentityModel.Protocols;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Microsoft.IdentityModel.Tokens;
+using Serilog;
 using MessageReceivedContext = Microsoft.AspNetCore.Authentication.JwtBearer.MessageReceivedContext;
 using TokenValidatedContext = Microsoft.AspNetCore.Authentication.OpenIdConnect.TokenValidatedContext;
 
@@ -139,8 +142,51 @@ private static void SetupOpenIdConnectAuthentication(this IServiceCollection ser
         var isDevelopment = environment.IsEnvironment(Environments.Development);
         var baseUrl = Configuration.BaseUrl;
 
-        var apiPrefix = baseUrl + "api";
-        var hubsPrefix = baseUrl + "hubs";
+        const string apiPrefix = "/api";
+        const string hubsPrefix = "/hubs";
+
+        var authority = Configuration.OidcSettings.Authority;
+        if (!isDevelopment && !authority.StartsWith("https"))
+        {
+            Log.Error("OpenIdConnect authority is not using https, you must configure tls for your idp.");
+            return;
+        }
+
+        var hasTrailingSlash = authority.EndsWith('/');
+        var url = authority + (hasTrailingSlash ? string.Empty : "/") + ".well-known/openid-configuration";
+
+        var configurationManager = new ConfigurationManager<OpenIdConnectConfiguration>(
+            url,
+            new OpenIdConnectConfigurationRetriever(),
+            new HttpDocumentRetriever { RequireHttps = !isDevelopment }
+        );
+
+        ICollection<string> supportedScopes;
+        try
+        {
+            supportedScopes = configurationManager.GetConfigurationAsync()
+                .ConfigureAwait(false)
+                .GetAwaiter()
+                .GetResult()
+                .ScopesSupported;
+        }
+        catch (Exception ex)
+        {
+            // Do not interrupt startup if OIDC fails (Network outage should still allow Kavita to run)
+            Log.Error(ex, "Failed to load OIDC configuration, OIDC will not be enabled. Restart to retry");
+            return;
+        }
+
+        List<string> scopes = ["openid", "profile", "offline_access", "roles", "email"];
+        scopes.AddRange(settings.CustomScopes);
+        var validScopes = scopes.Where(scope =>
+        {
+            if (supportedScopes.Contains(scope))
+                return true;
+
+            Log.Warning("Scope {Scope} is configured, but not supported by your OIDC provider. Skipping", scope);
+            return false;
+        }).ToList();
 
         services.AddOptions<CookieAuthenticationOptions>(CookieAuthenticationDefaults.AuthenticationScheme).Configure<ITicketStore>((options, store) =>
         {
@@ -150,6 +196,7 @@ private static void SetupOpenIdConnectAuthentication(this IServiceCollection ser
             options.Cookie.HttpOnly = true;
             options.Cookie.IsEssential = true;
             options.Cookie.MaxAge = TimeSpan.FromDays(7);
+            options.Cookie.SameSite = SameSiteMode.Strict;
             options.SessionStore = store;
 
             if (isDevelopment)
@@ -193,21 +240,25 @@ private static void SetupOpenIdConnectAuthentication(this IServiceCollection ser
 
             options.SaveTokens = true;
             options.GetClaimsFromUserInfoEndpoint = true;
-            options.Scope.Clear();
-            options.Scope.Add("openid");
-            options.Scope.Add("profile");
-            options.Scope.Add("offline_access");
-            options.Scope.Add("roles");
-            options.Scope.Add("email");
 
-            foreach (var customScope in settings.CustomScopes)
+            // Due to some (Authelia) OIDC providers, we need to map these claims explicitly. Such that no flow breaks in the
+            // OidcService
+            options.MapInboundClaims = true;
+            options.ClaimActions.MapJsonKey(ClaimTypes.Email, "email");
+            options.ClaimActions.MapJsonKey(ClaimTypes.Name, "name");
+            options.ClaimActions.MapJsonKey(JwtRegisteredClaimNames.PreferredUsername, "preferred_username");
+            options.ClaimActions.MapJsonKey(ClaimTypes.GivenName, "given_name");
+
+            options.Scope.Clear();
+            foreach (var scope in validScopes)
             {
-                options.Scope.Add(customScope);
+                options.Scope.Add(scope);
             }
 
+
             options.Events = new OpenIdConnectEvents
             {
-                OnTokenValidated = OidcClaimsPrincipalConverter,
+                OnTicketReceived = OidcClaimsPrincipalConverter,
                 OnAuthenticationFailed = ctx =>
                 {
                     ctx.Response.Redirect(baseUrl + "login?skipAutoLogin=true&error=" + Uri.EscapeDataString(ctx.Exception.Message));
@@ -252,7 +303,7 @@ private static void SetupOpenIdConnectAuthentication(this IServiceCollection ser
     /// Kavita roles the user has
     /// </summary>
     /// <param name="ctx"></param>
-    private static async Task OidcClaimsPrincipalConverter(TokenValidatedContext ctx)
+    private static async Task OidcClaimsPrincipalConverter(TicketReceivedContext ctx)
     {
         if (ctx.Principal == null) return;
 
@@ -264,58 +315,16 @@ private static async Task OidcClaimsPrincipalConverter(TokenValidatedContext ctx
         }
 
         var claims = await OidcService.ConstructNewClaimsList(ctx.HttpContext.RequestServices, ctx.Principal, user);
-        var tokens = CopyOidcTokens(ctx);
 
         var identity = new ClaimsIdentity(claims, ctx.Scheme.Name);
         var principal = new ClaimsPrincipal(identity);
 
-        ctx.Properties ??= new AuthenticationProperties();
-        ctx.Properties.StoreTokens(tokens);
-
         ctx.HttpContext.User = principal;
         ctx.Principal = principal;
 
         ctx.Success();
     }
 
-    /// <summary>
-    /// Copy tokens returned by the OIDC provider that we require later
-    /// </summary>
-    /// <param name="ctx"></param>
-    /// <returns></returns>
-    private static List<AuthenticationToken> CopyOidcTokens(TokenValidatedContext ctx)
-    {
-        if (ctx.TokenEndpointResponse == null)
-        {
-            return [];
-        }
-
-        var tokens = new List<AuthenticationToken>();
-
-        if (!string.IsNullOrEmpty(ctx.TokenEndpointResponse.RefreshToken))
-        {
-            tokens.Add(new AuthenticationToken { Name = OidcService.RefreshToken, Value = ctx.TokenEndpointResponse.RefreshToken });
-        }
-        else
-        {
-            var logger = ctx.HttpContext.RequestServices.GetRequiredService<ILogger<OidcService>>();
-            logger.LogWarning("OIDC login without refresh token, automatic sync will not work for this user");
-        }
-
-        if (!string.IsNullOrEmpty(ctx.TokenEndpointResponse.IdToken))
-        {
-            tokens.Add(new AuthenticationToken { Name = OidcService.IdToken, Value = ctx.TokenEndpointResponse.IdToken });
-        }
-
-        if (!string.IsNullOrEmpty(ctx.TokenEndpointResponse.ExpiresIn))
-        {
-            var expiresAt = DateTimeOffset.UtcNow.AddSeconds(double.Parse(ctx.TokenEndpointResponse.ExpiresIn));
-            tokens.Add(new AuthenticationToken { Name = OidcService.ExpiresAt, Value = expiresAt.ToString("o") });
-        }
-
-        return tokens;
-    }
-
     private static Task SetTokenFromQuery(MessageReceivedContext context)
     {
         var accessToken = context.Request.Query["access_token"];

API/Logging/LogLevelOptions.cs

@@ -13,6 +13,7 @@ namespace API.Logging;
 public static class LogLevelOptions
 {
     public const string LogFile = "config/logs/kavita.log";
+    public const string OutputTemplate = "[Kavita] [{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} {CorrelationId} {ThreadId}] [{Level}] {SourceContext} {Message:lj}{NewLine}{Exception}";
     public const bool LogRollingEnabled = true;
     /// <summary>
     /// Controls the Logging Level of the Application
@@ -37,7 +38,6 @@ public static class LogLevelOptions
 
     public static LoggerConfiguration CreateConfig(LoggerConfiguration configuration)
     {
-        const string outputTemplate = "[Kavita] [{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} {CorrelationId} {ThreadId}] [{Level}] {SourceContext} {Message:lj}{NewLine}{Exception}";
         return configuration
             .MinimumLevel
             .ControlledBy(LogLevelSwitch)
@@ -51,11 +51,11 @@ public static LoggerConfiguration CreateConfig(LoggerConfiguration configuration
             .Enrich.FromLogContext()
             .Enrich.WithThreadId()
             .Enrich.With(new ApiKeyEnricher())
-            .WriteTo.Console(new MessageTemplateTextFormatter(outputTemplate))
+            .WriteTo.Console(new MessageTemplateTextFormatter(OutputTemplate))
             .WriteTo.File(LogFile,
                 shared: true,
                 rollingInterval: RollingInterval.Day,
-                outputTemplate: outputTemplate)
+                outputTemplate: OutputTemplate)
             .Filter.ByIncludingOnly(ShouldIncludeLogStatement);
     }
 

API/Program.cs

@@ -26,6 +26,7 @@
 using Serilog.Events;
 using Serilog.Sinks.AspNetCore.SignalR.Extensions;
 using Log = Serilog.Log;
+using MessageTemplateTextFormatter = Serilog.Formatting.Display.MessageTemplateTextFormatter;
 
 namespace API;
 #nullable enable
@@ -42,7 +43,7 @@ public static async Task Main(string[] args)
     {
         Console.OutputEncoding = System.Text.Encoding.UTF8;
         Log.Logger = new LoggerConfiguration()
-            .WriteTo.Console()
+            .WriteTo.Console(new MessageTemplateTextFormatter(LogLevelOptions.OutputTemplate))
             .MinimumLevel
             .Information()
             .CreateBootstrapLogger();

API/Services/OidcService.cs

@@ -242,6 +242,7 @@ public async Task ClearOidcIds()
             .Where(s => PolicyConstants.ValidRoles.Contains(s)).ToList();
         if (settings.SyncUserSettings && accessRoles.Count == 0)
         {
+            logger.LogDebug("No valid roles where found under {Claim} with prefix {Prefix}", settings.RolesClaim, settings.RolesPrefix);
             throw new KavitaException("errors.oidc.role-not-assigned");
         }
 
@@ -265,6 +266,7 @@ public async Task ClearOidcIds()
         var roles = await userManager.GetRolesAsync(user);
         if (roles.Count == 0 || (!roles.Contains(PolicyConstants.LoginRole) && !roles.Contains(PolicyConstants.AdminRole)))
         {
+            logger.LogDebug("User does not have Login or AdminRole assigned. Has: {Roles}", string.Join(",", roles));
             throw new KavitaException("errors.oidc.disabled-account");
         }
 
@@ -360,9 +362,17 @@ private async Task SetDefaults(OidcConfigDto settings, AppUser user)
         // Assign libraries
         await accountService.UpdateLibrariesForUser(user, settings.DefaultLibraries, settings.DefaultRoles.Contains(PolicyConstants.AdminRole));
 
-        // Assign age rating
-        user.AgeRestriction = settings.DefaultAgeRestriction;
-        user.AgeRestrictionIncludeUnknowns = settings.DefaultIncludeUnknowns;
+        // Assign age rating, or bypass if admin
+        if (await userManager.IsInRoleAsync(user, PolicyConstants.AdminRole))
+        {
+            user.AgeRestriction = AgeRating.NotApplicable;
+            user.AgeRestrictionIncludeUnknowns = true;
+        }
+        else
+        {
+            user.AgeRestriction = settings.DefaultAgeRestriction;
+            user.AgeRestrictionIncludeUnknowns = settings.DefaultIncludeUnknowns;
+        }
 
         await unitOfWork.CommitAsync();
     }

API/Services/SettingsService.cs

@@ -20,6 +20,7 @@
 using Kavita.Common;
 using Kavita.Common.EnvironmentInfo;
 using Kavita.Common.Helpers;
+using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 
@@ -54,6 +55,7 @@ public class SettingsService : ISettingsService
     private readonly ITaskScheduler _taskScheduler;
     private readonly ILogger<SettingsService> _logger;
     private readonly IOidcService _oidcService;
+    private readonly bool _isDevelopment = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") == Environments.Development;
 
     public SettingsService(IUnitOfWork unitOfWork, IDirectoryService directoryService,
         ILibraryWatcher libraryWatcher, ITaskScheduler taskScheduler,
@@ -532,6 +534,11 @@ public async Task<bool> IsValidAuthority(string authority)
             return false;
         }
 
+        if (!_isDevelopment && !authority.StartsWith("https"))
+        {
+            return false;
+        }
+
         try
         {
             var hasTrailingSlash = authority.EndsWith('/');

API/Services/Store/CustomTicketStore.cs

@@ -7,6 +7,13 @@
 
 namespace API.Services.Store;
 
+/// <summary>
+/// The <see cref="ITicketStore"/> is used as <see cref="CookieAuthenticationOptions.SessionStore"/> for the OIDC implementation
+/// The full AuthenticationTicket cannot be included in the Cookie as popular reverse proxies (like nginx) will deny the request
+/// due the large header size. Instead, the key is used.
+/// </summary>
+/// <param name="cache"></param>
+/// <remarks>Note that this store is in memory, so OIDC authenticated users are logged out after restart</remarks>
 public class CustomTicketStore(IMemoryCache cache): ITicketStore
 {
 

UI/Web/src/app/_services/account.service.ts

@@ -300,7 +300,7 @@ export class AccountService {
     this.messageHub.stopHubConnection();
 
     if (!user.token) {
-      window.location.href = '/oidc/logout';
+      window.location.href = this.baseUrl.substring(0, environment.apiUrl.indexOf("api")) + 'oidc/logout';
       return;
     }