Resolve the group services lazily in the authentication provider

Jellyfin's UserManager constructor-injects every IAuthenticationProvider, so
building IUserManager forced SharedAccountAuthenticationProvider to be built
first. That provider eagerly required IGroupService and IDynamicGroupService,
both of which need IUserManager, and the container refused to start the server
with "a circular dependency was detected".

Take the two group services as Lazy<T> and dereference them at authentication
time instead. Nobody can log in before the host is up, so the deferred lookup
is always safe. Microsoft's container has no built-in Lazy<T> support, hence
the explicit factory registrations.

The accompanying test builds the service graph through a stand-in that mimics
UserManager's constructor shape and validates it on build, so a reintroduced
cycle fails in CI rather than at server startup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-09 10:46:09 +02:00
co-authored by Claude Opus 5
parent dfab79e92a
commit da779514fe
4 changed files with 132 additions and 11 deletions
@@ -63,8 +63,8 @@ public class AuthenticationTests
return new SharedAccountAuthenticationProvider(
crypto,
groups.Object,
dynamic.Object,
new Lazy<IGroupService>(() => groups.Object),
new Lazy<IDynamicGroupService>(() => dynamic.Object),
NullLogger<SharedAccountAuthenticationProvider>.Instance);
}
@@ -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;
/// <summary>
/// Guards the plugin's service graph against container-level cycles.
/// </summary>
/// <remarks>
/// Jellyfin's real <c>UserManager</c> constructor-injects <c>IEnumerable&lt;IAuthenticationProvider&gt;</c>.
/// That means any plugin service reachable eagerly from our authentication provider must not itself
/// require <see cref="IUserManager"/>, 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.
/// </remarks>
public class ServiceRegistrationTests
{
/// <summary>
/// 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.
/// </summary>
private sealed class UserManagerWithAuthProviders
{
public UserManagerWithAuthProviders(IEnumerable<IAuthenticationProvider> authenticationProviders)
{
AuthenticationProviders = authenticationProviders;
}
public IEnumerable<IAuthenticationProvider> 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<ILibraryManager>());
services.AddSingleton(Mock.Of<IUserDataManager>());
services.AddSingleton(Mock.Of<ICryptoProvider>());
// 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<UserManagerWithAuthProviders>();
services.AddSingleton(provider =>
{
provider.GetRequiredService<UserManagerWithAuthProviders>();
return Mock.Of<IUserManager>();
});
new ServiceRegistrator().RegisterServices(services, Mock.Of<MediaBrowser.Controller.IServerApplicationHost>());
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<IUserManager>();
Assert.NotNull(userManager);
}
[Fact]
public void AuthenticationProvider_IsConstructedWithoutResolvingUserManager()
{
using var provider = BuildHostLikeProvider();
var authProviders = provider.GetRequiredService<IEnumerable<IAuthenticationProvider>>();
Assert.Contains(authProviders, p => p is Auth.SharedAccountAuthenticationProvider);
}
[Fact]
public void GroupServices_AreStillResolvableOnceTheHostIsUp()
{
using var provider = BuildHostLikeProvider();
// The Lazy<T> 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<Lazy<Services.IGroupService>>();
var lazyDynamicGroupService = provider.GetRequiredService<Lazy<Services.IDynamicGroupService>>();
Assert.Same(provider.GetRequiredService<Services.IGroupService>(), lazyGroupService.Value);
Assert.Same(provider.GetRequiredService<Services.IDynamicGroupService>(), lazyDynamicGroupService.Value);
}
}
@@ -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<Services.IGroupService> _groupService;
private readonly Lazy<Services.IDynamicGroupService> _dynamicGroupService;
private readonly ILogger<SharedAccountAuthenticationProvider> _logger;
/// <summary>
/// Initializes a new instance of the <see cref="SharedAccountAuthenticationProvider"/> class.
/// </summary>
/// <param name="cryptoProvider">The crypto provider used to verify stored password hashes.</param>
/// <param name="groupService">The group service.</param>
/// <param name="dynamicGroupService">The on-demand group creation service.</param>
/// <param name="groupService">A deferred handle to the group service.</param>
/// <param name="dynamicGroupService">A deferred handle to the on-demand group creation service.</param>
/// <param name="logger">The logger.</param>
/// <remarks>
/// The group services are taken as <see cref="Lazy{T}"/> to break a container-level cycle.
/// Jellyfin's <c>UserManager</c> constructor-injects every <see cref="IAuthenticationProvider"/>,
/// so resolving those services eagerly here would require <c>IUserManager</c> 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.
/// </remarks>
public SharedAccountAuthenticationProvider(
ICryptoProvider cryptoProvider,
Services.IGroupService groupService,
Services.IDynamicGroupService dynamicGroupService,
Lazy<Services.IGroupService> groupService,
Lazy<Services.IDynamicGroupService> dynamicGroupService,
ILogger<SharedAccountAuthenticationProvider> 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(
@@ -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<IProvisioningService, ProvisioningService>();
serviceCollection.AddSingleton<IDynamicGroupService, DynamicGroupService>();
// 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<T> support, so the factories are explicit.
serviceCollection.AddSingleton(
provider => new Lazy<IGroupService>(provider.GetRequiredService<IGroupService>));
serviceCollection.AddSingleton(
provider => new Lazy<IDynamicGroupService>(provider.GetRequiredService<IDynamicGroupService>));
// Discovered by Jellyfin and matched to shared accounts via User.AuthenticationProviderId.
serviceCollection.AddSingleton<IAuthenticationProvider, SharedAccountAuthenticationProvider>();