From 209edd38a4163a8cf4abd5e47bfe0ea1a100f351 Mon Sep 17 00:00:00 2001 From: cvium Date: Wed, 8 Feb 2023 23:55:26 +0100 Subject: refactor: simplify authz --- .../DefaultAuthorizationHandler.cs | 47 +++++++++++++++++++--- .../DefaultAuthorizationRequirement.cs | 13 ++++++ 2 files changed, 55 insertions(+), 5 deletions(-) (limited to 'Jellyfin.Api/Auth/DefaultAuthorizationPolicy') diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs index be77b7a4e4..7489e2a35c 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs @@ -1,4 +1,8 @@ using System.Threading.Tasks; +using Jellyfin.Api.Constants; +using Jellyfin.Api.Extensions; +using Jellyfin.Data.Enums; +using MediaBrowser.Common.Extensions; using MediaBrowser.Common.Net; using MediaBrowser.Controller.Library; using Microsoft.AspNetCore.Authorization; @@ -9,8 +13,12 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy /// /// Default authorization handler. /// - public class DefaultAuthorizationHandler : BaseAuthorizationHandler + public class DefaultAuthorizationHandler : AuthorizationHandler { + private readonly IUserManager _userManager; + private readonly INetworkManager _networkManager; + private readonly IHttpContextAccessor _httpContextAccessor; + /// /// Initializes a new instance of the class. /// @@ -21,21 +29,50 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy IUserManager userManager, INetworkManager networkManager, IHttpContextAccessor httpContextAccessor) - : base(userManager, networkManager, httpContextAccessor) { + _userManager = userManager; + _networkManager = networkManager; + _httpContextAccessor = httpContextAccessor; } /// protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DefaultAuthorizationRequirement requirement) { - var validated = ValidateClaims(context.User); - if (validated) + // Admins can do everything + if (context.User.GetIsApiKey() || context.User.IsInRole(UserRoles.Administrator)) { context.Succeed(requirement); + return Task.CompletedTask; } - else + + var userId = context.User.GetUserId(); + // This likely only happens during the wizard, so skip the default checks and let any other handlers do it + if (userId.Equals(default)) + { + return Task.CompletedTask; + } + + var isInLocalNetwork = _httpContextAccessor.HttpContext is not null + && _networkManager.IsInLocalNetwork(_httpContextAccessor.HttpContext.GetNormalizedRemoteIp()); + var user = _userManager.GetUserById(userId); + // User cannot access remotely and user is remote + if (!isInLocalNetwork && !user.HasPermission(PermissionKind.EnableRemoteAccess)) { context.Fail(); + return Task.CompletedTask; + } + + // It's not great to have this check, but parental schedule must usually be honored except in a few rare cases + if (requirement.ValidateParentalSchedule && !user.IsParentalScheduleAllowed()) + { + context.Fail(); + return Task.CompletedTask; + } + + // Only succeed if the requirement isn't a subclass as any subclassed requirement will handle success in its own handler + if (requirement.GetType() == typeof(DefaultAuthorizationRequirement)) + { + context.Succeed(requirement); } return Task.CompletedTask; diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs index 7cea00b694..0846e7515a 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs @@ -7,5 +7,18 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy /// public class DefaultAuthorizationRequirement : IAuthorizationRequirement { + /// + /// Initializes a new instance of the class. + /// + /// A value indicating whether to validate parental schedule. + public DefaultAuthorizationRequirement(bool validateParentalSchedule = true) + { + ValidateParentalSchedule = validateParentalSchedule; + } + + /// + /// Gets a value indicating whether to ignore parental schedule. + /// + public bool ValidateParentalSchedule { get; init; } } } -- cgit v1.2.3 From f984f31896d9f5b34b488efb845d73f901fc9a80 Mon Sep 17 00:00:00 2001 From: cvium Date: Thu, 9 Feb 2023 08:53:59 +0100 Subject: admins shouldn't be able to circumvent remote access policies --- .../DefaultAuthorizationHandler.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'Jellyfin.Api/Auth/DefaultAuthorizationPolicy') diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs index 7489e2a35c..0f3c69abc8 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs @@ -38,13 +38,6 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy /// protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DefaultAuthorizationRequirement requirement) { - // Admins can do everything - if (context.User.GetIsApiKey() || context.User.IsInRole(UserRoles.Administrator)) - { - context.Succeed(requirement); - return Task.CompletedTask; - } - var userId = context.User.GetUserId(); // This likely only happens during the wizard, so skip the default checks and let any other handlers do it if (userId.Equals(default)) @@ -62,6 +55,13 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy return Task.CompletedTask; } + // Admins can do everything + if (context.User.GetIsApiKey() || context.User.IsInRole(UserRoles.Administrator)) + { + context.Succeed(requirement); + return Task.CompletedTask; + } + // It's not great to have this check, but parental schedule must usually be honored except in a few rare cases if (requirement.ValidateParentalSchedule && !user.IsParentalScheduleAllowed()) { -- cgit v1.2.3 From f4a7583c46e25c5146953bef144f91f0c4c4519e Mon Sep 17 00:00:00 2001 From: cvium Date: Thu, 9 Feb 2023 12:51:20 +0100 Subject: fix empty user id check for api keys --- .../Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'Jellyfin.Api/Auth/DefaultAuthorizationPolicy') diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs index 0f3c69abc8..2d9ce0631f 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs @@ -38,9 +38,10 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy /// protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DefaultAuthorizationRequirement requirement) { + var isApiKey = context.User.GetIsApiKey(); var userId = context.User.GetUserId(); // This likely only happens during the wizard, so skip the default checks and let any other handlers do it - if (userId.Equals(default)) + if (!isApiKey && userId.Equals(default)) { return Task.CompletedTask; } @@ -56,7 +57,7 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy } // Admins can do everything - if (context.User.GetIsApiKey() || context.User.IsInRole(UserRoles.Administrator)) + if (isApiKey || context.User.IsInRole(UserRoles.Administrator)) { context.Succeed(requirement); return Task.CompletedTask; -- cgit v1.2.3 From ac118e10f04de5489e6d0ac17c8ca16bdf6d0dc3 Mon Sep 17 00:00:00 2001 From: cvium Date: Thu, 9 Feb 2023 15:01:04 +0100 Subject: remove unnecessary init --- .../Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs | 2 +- Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupRequirement.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'Jellyfin.Api/Auth/DefaultAuthorizationPolicy') diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs index 0846e7515a..5ba1bc330d 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationRequirement.cs @@ -19,6 +19,6 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy /// /// Gets a value indicating whether to ignore parental schedule. /// - public bool ValidateParentalSchedule { get; init; } + public bool ValidateParentalSchedule { get; } } } diff --git a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupRequirement.cs b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupRequirement.cs index 8b7a94954e..6252a2feb8 100644 --- a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupRequirement.cs +++ b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupRequirement.cs @@ -12,9 +12,8 @@ namespace Jellyfin.Api.Auth.FirstTimeSetupPolicy /// /// A value indicating whether to ignore parental schedule. /// A value indicating whether administrator role is required. - public FirstTimeSetupRequirement(bool validateParentalSchedule = false, bool requireAdmin = true) + public FirstTimeSetupRequirement(bool validateParentalSchedule = false, bool requireAdmin = true) : base(validateParentalSchedule) { - ValidateParentalSchedule = validateParentalSchedule; RequireAdmin = requireAdmin; } -- cgit v1.2.3 From a5e2ae4979ece439ade037ba2c88a4003a7e8f68 Mon Sep 17 00:00:00 2001 From: cvium Date: Sun, 12 Feb 2023 23:01:30 +0100 Subject: fix merge conflict --- .../Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs | 5 +++++ Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs | 6 ++++++ Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs | 5 +++++ Jellyfin.Api/Auth/UserPermissionPolicy/UserPermissionHandler.cs | 6 ++++++ 4 files changed, 22 insertions(+) (limited to 'Jellyfin.Api/Auth/DefaultAuthorizationPolicy') diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs index 2d9ce0631f..b1d97e4a1d 100644 --- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs +++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs @@ -49,6 +49,11 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy var isInLocalNetwork = _httpContextAccessor.HttpContext is not null && _networkManager.IsInLocalNetwork(_httpContextAccessor.HttpContext.GetNormalizedRemoteIp()); var user = _userManager.GetUserById(userId); + if (user is null) + { + throw new ResourceNotFoundException(); + } + // User cannot access remotely and user is remote if (!isInLocalNetwork && !user.HasPermission(PermissionKind.EnableRemoteAccess)) { diff --git a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs index 302e052a7c..28ba258503 100644 --- a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs +++ b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs @@ -2,6 +2,7 @@ using System.Threading.Tasks; using Jellyfin.Api.Constants; using Jellyfin.Api.Extensions; using MediaBrowser.Common.Configuration; +using MediaBrowser.Common.Extensions; using MediaBrowser.Controller.Library; using Microsoft.AspNetCore.Authorization; @@ -50,6 +51,11 @@ namespace Jellyfin.Api.Auth.FirstTimeSetupPolicy } var user = _userManager.GetUserById(context.User.GetUserId()); + if (user is null) + { + throw new ResourceNotFoundException(); + } + if (user.IsParentalScheduleAllowed()) { context.Succeed(requirement); diff --git a/Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs b/Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs index 5c1029b383..75ec9fcec6 100644 --- a/Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs +++ b/Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs @@ -1,6 +1,7 @@ using System.Threading.Tasks; using Jellyfin.Api.Extensions; using Jellyfin.Data.Enums; +using MediaBrowser.Common.Extensions; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.SyncPlay; using Microsoft.AspNetCore.Authorization; @@ -33,6 +34,10 @@ namespace Jellyfin.Api.Auth.SyncPlayAccessPolicy { var userId = context.User.GetUserId(); var user = _userManager.GetUserById(userId); + if (user is null) + { + throw new ResourceNotFoundException(); + } if (requirement.RequiredAccess == SyncPlayAccessRequirementType.HasAccess) { diff --git a/Jellyfin.Api/Auth/UserPermissionPolicy/UserPermissionHandler.cs b/Jellyfin.Api/Auth/UserPermissionPolicy/UserPermissionHandler.cs index c3de7be328..ba2b1b657e 100644 --- a/Jellyfin.Api/Auth/UserPermissionPolicy/UserPermissionHandler.cs +++ b/Jellyfin.Api/Auth/UserPermissionPolicy/UserPermissionHandler.cs @@ -1,6 +1,7 @@ using System.Threading.Tasks; using Jellyfin.Api.Auth.DownloadPolicy; using Jellyfin.Api.Extensions; +using MediaBrowser.Common.Extensions; using MediaBrowser.Controller.Library; using Microsoft.AspNetCore.Authorization; @@ -26,6 +27,11 @@ namespace Jellyfin.Api.Auth.UserPermissionPolicy protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, UserPermissionRequirement requirement) { var user = _userManager.GetUserById(context.User.GetUserId()); + if (user is null) + { + throw new ResourceNotFoundException(); + } + if (user.HasPermission(requirement.RequiredPermission)) { context.Succeed(requirement); -- cgit v1.2.3