From dc2db22c3db983a3c787877e2e359d7bab4970d2 Mon Sep 17 00:00:00 2001 From: thornbill Date: Sat, 1 Jun 2024 18:41:08 -0400 Subject: [PATCH] Backport pull request #11873 from jellyfin/release-10.9.z Fix FirstTimeSetupHandler allowing public access Original-merge: 869dab2ba2900c18f9de817607e1b0d3681f8ac9 Merged-by: joshuaboniface Backported-by: Joshua M. Boniface --- .../FirstTimeSetupHandler.cs | 28 +++++---- .../FirstTimeSetupHandlerTests.cs | 60 +++++++++++++------ 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs index 9b4e2182c5..e425000cd6 100644 --- a/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs +++ b/Jellyfin.Api/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandler.cs @@ -1,5 +1,6 @@ using System.Threading.Tasks; using Jellyfin.Api.Constants; +using Jellyfin.Api.Extensions; using MediaBrowser.Common.Configuration; using Microsoft.AspNetCore.Authorization; @@ -24,24 +25,31 @@ namespace Jellyfin.Api.Auth.FirstTimeSetupPolicy /// protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, FirstTimeSetupRequirement requirement) { + // Succeed if the startup wizard / first time setup is not complete if (!_configurationManager.CommonConfiguration.IsStartupWizardCompleted) { context.Succeed(requirement); } - else if (requirement.RequireAdmin && !context.User.IsInRole(UserRoles.Administrator)) + + // Succeed if user is admin + else if (context.User.IsInRole(UserRoles.Administrator)) { - context.Fail(); - } - else if (!requirement.RequireAdmin && context.User.IsInRole(UserRoles.Guest)) - { - context.Fail(); - } - else - { - // Any user-specific checks are handled in the DefaultAuthorizationHandler. context.Succeed(requirement); } + // Fail if admin is required and user is not admin + else if (requirement.RequireAdmin) + { + context.Fail(); + } + + // Succeed if admin is not required and user is not guest + else if (context.User.IsInRole(UserRoles.User)) + { + context.Succeed(requirement); + } + + // Any user-specific checks are handled in the DefaultAuthorizationHandler. return Task.CompletedTask; } } diff --git a/tests/Jellyfin.Api.Tests/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandlerTests.cs b/tests/Jellyfin.Api.Tests/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandlerTests.cs index 2e6ffb5f6a..31d2b486b3 100644 --- a/tests/Jellyfin.Api.Tests/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandlerTests.cs +++ b/tests/Jellyfin.Api.Tests/Auth/FirstTimeSetupPolicy/FirstTimeSetupHandlerTests.cs @@ -1,14 +1,19 @@ +using System; using System.Collections.Generic; using System.Security.Claims; using System.Threading.Tasks; using AutoFixture; using AutoFixture.AutoMoq; +using Jellyfin.Api.Auth.DefaultAuthorizationPolicy; using Jellyfin.Api.Auth.FirstTimeSetupPolicy; using Jellyfin.Api.Constants; +using Jellyfin.Data.Entities; +using Jellyfin.Data.Enums; using MediaBrowser.Common.Configuration; using MediaBrowser.Controller.Library; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Moq; using Xunit; @@ -18,7 +23,9 @@ namespace Jellyfin.Api.Tests.Auth.FirstTimeSetupPolicy { private readonly Mock _configurationManagerMock; private readonly List _requirements; + private readonly DefaultAuthorizationHandler _defaultAuthorizationHandler; private readonly FirstTimeSetupHandler _firstTimeSetupHandler; + private readonly IAuthorizationService _authorizationService; private readonly Mock _userManagerMock; private readonly Mock _httpContextAccessor; @@ -31,6 +38,21 @@ namespace Jellyfin.Api.Tests.Auth.FirstTimeSetupPolicy _httpContextAccessor = fixture.Freeze>(); _firstTimeSetupHandler = fixture.Create(); + _defaultAuthorizationHandler = fixture.Create(); + + var services = new ServiceCollection(); + services.AddAuthorizationCore(); + services.AddLogging(); + services.AddOptions(); + services.AddSingleton(_defaultAuthorizationHandler); + services.AddSingleton(_firstTimeSetupHandler); + services.AddAuthorization(options => + { + options.AddPolicy("FirstTime", policy => policy.Requirements.Add(new FirstTimeSetupRequirement())); + options.AddPolicy("FirstTimeNoAdmin", policy => policy.Requirements.Add(new FirstTimeSetupRequirement(false, false))); + options.AddPolicy("FirstTimeSchedule", policy => policy.Requirements.Add(new FirstTimeSetupRequirement(true, false))); + }); + _authorizationService = services.BuildServiceProvider().GetRequiredService(); } [Theory] @@ -45,10 +67,9 @@ namespace Jellyfin.Api.Tests.Auth.FirstTimeSetupPolicy _httpContextAccessor, userRole); - var context = new AuthorizationHandlerContext(_requirements, claims, null); + var allowed = await _authorizationService.AuthorizeAsync(claims, "FirstTime"); - await _firstTimeSetupHandler.HandleAsync(context); - Assert.True(context.HasSucceeded); + Assert.True(allowed.Succeeded); } [Theory] @@ -63,17 +84,16 @@ namespace Jellyfin.Api.Tests.Auth.FirstTimeSetupPolicy _httpContextAccessor, userRole); - var context = new AuthorizationHandlerContext(_requirements, claims, null); + var allowed = await _authorizationService.AuthorizeAsync(claims, "FirstTime"); - await _firstTimeSetupHandler.HandleAsync(context); - Assert.Equal(shouldSucceed, context.HasSucceeded); + Assert.Equal(shouldSucceed, allowed.Succeeded); } [Theory] [InlineData(UserRoles.Administrator, true)] [InlineData(UserRoles.Guest, false)] [InlineData(UserRoles.User, true)] - public async Task ShouldRequireUserIfNotRequiresAdmin(string userRole, bool shouldSucceed) + public async Task ShouldRequireUserIfNotAdministrator(string userRole, bool shouldSucceed) { TestHelpers.SetupConfigurationManager(_configurationManagerMock, true); var claims = TestHelpers.SetupUser( @@ -81,24 +101,26 @@ namespace Jellyfin.Api.Tests.Auth.FirstTimeSetupPolicy _httpContextAccessor, userRole); - var context = new AuthorizationHandlerContext( - new List { new FirstTimeSetupRequirement(false, false) }, - claims, - null); + var allowed = await _authorizationService.AuthorizeAsync(claims, "FirstTimeNoAdmin"); - await _firstTimeSetupHandler.HandleAsync(context); - Assert.Equal(shouldSucceed, context.HasSucceeded); + Assert.Equal(shouldSucceed, allowed.Succeeded); } [Fact] - public async Task ShouldAllowAdminApiKeyIfStartupWizardComplete() + public async Task ShouldDisallowUserIfOutsideSchedule() { - TestHelpers.SetupConfigurationManager(_configurationManagerMock, true); - var claims = new ClaimsPrincipal(new ClaimsIdentity([new Claim(ClaimTypes.Role, UserRoles.Administrator)])); - var context = new AuthorizationHandlerContext(_requirements, claims, null); + AccessSchedule[] accessSchedules = { new AccessSchedule(DynamicDayOfWeek.Everyday, 0, 0, Guid.Empty) }; - await _firstTimeSetupHandler.HandleAsync(context); - Assert.True(context.HasSucceeded); + TestHelpers.SetupConfigurationManager(_configurationManagerMock, true); + var claims = TestHelpers.SetupUser( + _userManagerMock, + _httpContextAccessor, + UserRoles.User, + accessSchedules); + + var allowed = await _authorizationService.AuthorizeAsync(claims, "FirstTimeSchedule"); + + Assert.False(allowed.Succeeded); } } }