diff --git a/Jellyfin.Plugin.WatchedTogether.Tests/AuthenticationTests.cs b/Jellyfin.Plugin.WatchedTogether.Tests/AuthenticationTests.cs index a48d626..6c3ccea 100644 --- a/Jellyfin.Plugin.WatchedTogether.Tests/AuthenticationTests.cs +++ b/Jellyfin.Plugin.WatchedTogether.Tests/AuthenticationTests.cs @@ -63,8 +63,8 @@ public class AuthenticationTests return new SharedAccountAuthenticationProvider( crypto, - groups.Object, - dynamic.Object, + new Lazy(() => groups.Object), + new Lazy(() => dynamic.Object), NullLogger.Instance); } diff --git a/Jellyfin.Plugin.WatchedTogether.Tests/ServiceRegistrationTests.cs b/Jellyfin.Plugin.WatchedTogether.Tests/ServiceRegistrationTests.cs new file mode 100644 index 0000000..2499f2e --- /dev/null +++ b/Jellyfin.Plugin.WatchedTogether.Tests/ServiceRegistrationTests.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using Jellyfin.Plugin.WatchedTogether; +using MediaBrowser.Controller.Authentication; +using MediaBrowser.Controller.Library; +using MediaBrowser.Model.Cryptography; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Plugin.WatchedTogether.Tests; + +/// +/// Guards the plugin's service graph against container-level cycles. +/// +/// +/// Jellyfin's real UserManager constructor-injects IEnumerable<IAuthenticationProvider>. +/// That means any plugin service reachable eagerly from our authentication provider must not itself +/// require , or the host dies at startup with "a circular dependency was +/// detected". A cycle like that is invisible to unit tests that construct services by hand, so these +/// tests build the graph the way the host does. +/// +public class ServiceRegistrationTests +{ + /// + /// Stands in for Jellyfin's UserManager, whose constructor takes every registered authentication + /// provider. Only the constructor shape matters here - it is what closes the cycle. + /// + private sealed class UserManagerWithAuthProviders + { + public UserManagerWithAuthProviders(IEnumerable authenticationProviders) + { + AuthenticationProviders = authenticationProviders; + } + + public IEnumerable AuthenticationProviders { get; } + } + + private static ServiceProvider BuildHostLikeProvider() + { + var services = new ServiceCollection(); + + services.AddLogging(builder => builder.AddProvider(NullLoggerProvider.Instance)); + + // Host services the plugin consumes, other than IUserManager. + services.AddSingleton(Mock.Of()); + services.AddSingleton(Mock.Of()); + services.AddSingleton(Mock.Of()); + + // IUserManager resolves through the fake UserManager so that building it forces every + // IAuthenticationProvider to be built first, exactly as the real host does. + services.AddSingleton(); + services.AddSingleton(provider => + { + provider.GetRequiredService(); + return Mock.Of(); + }); + + new ServiceRegistrator().RegisterServices(services, Mock.Of()); + + return services.BuildServiceProvider(new ServiceProviderOptions + { + ValidateOnBuild = true, + ValidateScopes = true + }); + } + + [Fact] + public void PluginServices_ResolveWithoutCircularDependency() + { + using var provider = BuildHostLikeProvider(); + + // Resolving IUserManager is what the host does during startup, and is the exact path that + // previously threw InvalidOperationException for a circular dependency. + var userManager = provider.GetRequiredService(); + + Assert.NotNull(userManager); + } + + [Fact] + public void AuthenticationProvider_IsConstructedWithoutResolvingUserManager() + { + using var provider = BuildHostLikeProvider(); + + var authProviders = provider.GetRequiredService>(); + + Assert.Contains(authProviders, p => p is Auth.SharedAccountAuthenticationProvider); + } + + [Fact] + public void GroupServices_AreStillResolvableOnceTheHostIsUp() + { + using var provider = BuildHostLikeProvider(); + + // The Lazy indirection must not change what the services resolve to at authentication + // time, and must hand back the same singletons the rest of the plugin uses. + var lazyGroupService = provider.GetRequiredService>(); + var lazyDynamicGroupService = provider.GetRequiredService>(); + + Assert.Same(provider.GetRequiredService(), lazyGroupService.Value); + Assert.Same(provider.GetRequiredService(), lazyDynamicGroupService.Value); + } +} diff --git a/Jellyfin.Plugin.WatchedTogether/Auth/SharedAccountAuthenticationProvider.cs b/Jellyfin.Plugin.WatchedTogether/Auth/SharedAccountAuthenticationProvider.cs index 37d535d..8a97f1b 100644 --- a/Jellyfin.Plugin.WatchedTogether/Auth/SharedAccountAuthenticationProvider.cs +++ b/Jellyfin.Plugin.WatchedTogether/Auth/SharedAccountAuthenticationProvider.cs @@ -28,21 +28,28 @@ namespace Jellyfin.Plugin.WatchedTogether.Auth; public class SharedAccountAuthenticationProvider : IAuthenticationProvider, IRequiresResolvedUser { private readonly ICryptoProvider _cryptoProvider; - private readonly Services.IGroupService _groupService; - private readonly Services.IDynamicGroupService _dynamicGroupService; + private readonly Lazy _groupService; + private readonly Lazy _dynamicGroupService; private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// /// The crypto provider used to verify stored password hashes. - /// The group service. - /// The on-demand group creation service. + /// A deferred handle to the group service. + /// A deferred handle to the on-demand group creation service. /// The logger. + /// + /// The group services are taken as to break a container-level cycle. + /// Jellyfin's UserManager constructor-injects every , + /// so resolving those services eagerly here would require IUserManager while it is still + /// being built and the host would refuse to start. Deferring the lookup to the first + /// authentication is safe: nobody can log in until the host is fully up. + /// public SharedAccountAuthenticationProvider( ICryptoProvider cryptoProvider, - Services.IGroupService groupService, - Services.IDynamicGroupService dynamicGroupService, + Lazy groupService, + Lazy dynamicGroupService, ILogger logger) { _cryptoProvider = cryptoProvider; @@ -73,7 +80,7 @@ public class SharedAccountAuthenticationProvider : IAuthenticationProvider, IReq // a real user with that exact name always resolves first and never reaches this branch. if (resolvedUser is null) { - var created = await _dynamicGroupService + var created = await _dynamicGroupService.Value .TryCreateFromLoginAsync(username, password) .ConfigureAwait(false); @@ -85,7 +92,7 @@ public class SharedAccountAuthenticationProvider : IAuthenticationProvider, IReq return new ProviderAuthenticationResult { Username = created.SharedUsername }; } - var group = _groupService.GetGroupForSharedUser(resolvedUser.Id); + var group = _groupService.Value.GetGroupForSharedUser(resolvedUser.Id); if (group is null) { // Either not one of ours, or the group is disabled. Either way this account has no @@ -96,7 +103,7 @@ public class SharedAccountAuthenticationProvider : IAuthenticationProvider, IReq throw new AuthenticationException("Invalid username or password."); } - var members = _groupService.GetEligibleMembers(group); + var members = _groupService.Value.GetEligibleMembers(group); if (members.Count == 0) { _logger.LogWarning( diff --git a/Jellyfin.Plugin.WatchedTogether/ServiceRegistrator.cs b/Jellyfin.Plugin.WatchedTogether/ServiceRegistrator.cs index 156790b..4818f16 100644 --- a/Jellyfin.Plugin.WatchedTogether/ServiceRegistrator.cs +++ b/Jellyfin.Plugin.WatchedTogether/ServiceRegistrator.cs @@ -1,3 +1,4 @@ +using System; using Jellyfin.Plugin.WatchedTogether.Auth; using Jellyfin.Plugin.WatchedTogether.Services; using MediaBrowser.Controller; @@ -20,6 +21,14 @@ public class ServiceRegistrator : IPluginServiceRegistrator serviceCollection.AddSingleton(); serviceCollection.AddSingleton(); + // The auth provider takes these lazily so the container can build it while IUserManager is + // still under construction; see SharedAccountAuthenticationProvider's constructor remarks. + // Microsoft's container has no built-in Lazy support, so the factories are explicit. + serviceCollection.AddSingleton( + provider => new Lazy(provider.GetRequiredService)); + serviceCollection.AddSingleton( + provider => new Lazy(provider.GetRequiredService)); + // Discovered by Jellyfin and matched to shared accounts via User.AuthenticationProviderId. serviceCollection.AddSingleton();