diff --git a/Jellyfin.Plugin.WatchedTogether.Tests/ProvisioningTests.cs b/Jellyfin.Plugin.WatchedTogether.Tests/ProvisioningTests.cs new file mode 100644 index 0000000..b6d9b22 --- /dev/null +++ b/Jellyfin.Plugin.WatchedTogether.Tests/ProvisioningTests.cs @@ -0,0 +1,151 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Plugin.WatchedTogether.Services; +using MediaBrowser.Controller.Library; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Plugin.WatchedTogether.Tests; + +/// +/// Covers shared-account provisioning against a user manager that dispatches password changes the +/// way Jellyfin's real one does. +/// +[Collection(nameof(PluginTestContext))] +public class ProvisioningTests +{ + private static readonly string AuthProviderId = + typeof(Auth.SharedAccountAuthenticationProvider).FullName!; + + private static User MakeUser(string name) => new(name, "Prov", "ResetProv"); + + /// + /// Builds a user manager that mimics the one behaviour that matters here: ChangePassword is + /// routed to the provider named by the user's AuthenticationProviderId, so an account already + /// claimed by us lands in our provider and is refused. + /// + private static Mock MakeUserManager(List users, List callLog) + { + var userManager = new Mock(); + + userManager.Setup(m => m.GetUserById(It.IsAny())) + .Returns((Guid id) => users.Find(u => u.Id == id)); + + userManager.Setup(m => m.CreateUserAsync(It.IsAny())) + .ReturnsAsync((string name) => + { + var created = MakeUser(name); + users.Add(created); + callLog.Add("CreateUser"); + return created; + }); + + userManager.Setup(m => m.ChangePassword(It.IsAny(), It.IsAny())) + .Returns((User user, string password) => + { + callLog.Add($"ChangePassword(provider={user.AuthenticationProviderId})"); + + // This is the dispatch that made provisioning fail in 0.0.3: once the account is + // claimed, the call reaches our provider, which refuses it by design. + if (string.Equals(user.AuthenticationProviderId, AuthProviderId, StringComparison.Ordinal)) + { + return new Auth.SharedAccountAuthenticationProvider( + Mock.Of(), + new Lazy(() => Mock.Of()), + new Lazy(() => Mock.Of()), + NullLogger.Instance) + .ChangePassword(user, password); + } + + user.Password = password; + return Task.CompletedTask; + }); + + userManager.Setup(m => m.UpdateUserAsync(It.IsAny())) + .Returns((User user) => + { + callLog.Add($"UpdateUser(provider={user.AuthenticationProviderId})"); + return Task.CompletedTask; + }); + + return userManager; + } + + private static ProvisioningService MakeService(Mock userManager) + => new( + userManager.Object, + Mock.Of(), + NullLogger.Instance); + + [Fact] + public async Task CreateGroupAsync_SetsPasswordBeforeClaimingTheAccount() + { + using var context = PluginTestContext.Create(); + + var alice = MakeUser("alice"); + var bob = MakeUser("bob"); + var users = new List { alice, bob }; + var callLog = new List(); + + var service = MakeService(MakeUserManager(users, callLog)); + + // Before the fix this threw NotSupportedException from our own ChangePassword. + var group = await service.CreateGroupAsync([alice.Id, bob.Id], null); + + Assert.NotEqual(Guid.Empty, group.SharedUserId); + + // The password must be set while the account is still on Jellyfin's default provider. + var changeIndex = callLog.FindIndex(c => c.StartsWith("ChangePassword", StringComparison.Ordinal)); + var claimIndex = callLog.FindIndex(c => c.Contains(AuthProviderId, StringComparison.Ordinal)); + + Assert.True(changeIndex >= 0, "provisioning should set a password on the shared account"); + Assert.True(claimIndex >= 0, "provisioning should claim the account for our provider"); + Assert.True( + changeIndex < claimIndex, + $"password must be set before the account is claimed, but call order was: {string.Join(" -> ", callLog)}"); + } + + [Fact] + public async Task CreateGroupAsync_LeavesTheAccountClaimedByOurProvider() + { + using var context = PluginTestContext.Create(); + + var alice = MakeUser("alice"); + var bob = MakeUser("bob"); + var users = new List { alice, bob }; + var callLog = new List(); + + var service = MakeService(MakeUserManager(users, callLog)); + + var group = await service.CreateGroupAsync([alice.Id, bob.Id], null); + + // Claiming the account is what routes its logins to us; provisioning is useless without it. + var sharedUser = users.Find(u => u.Id == group.SharedUserId); + Assert.NotNull(sharedUser); + Assert.Equal(AuthProviderId, sharedUser!.AuthenticationProviderId); + } + + [Fact] + public async Task CreateGroupAsync_GivesTheSharedAccountANonEmptyPassword() + { + using var context = PluginTestContext.Create(); + + var alice = MakeUser("alice"); + var bob = MakeUser("bob"); + var users = new List { alice, bob }; + var callLog = new List(); + + var service = MakeService(MakeUserManager(users, callLog)); + + var group = await service.CreateGroupAsync([alice.Id, bob.Id], null); + + // A passwordless shared account would be directly loginable if the provider were ever + // unassigned, which is the reason provisioning sets one at all. + var sharedUser = users.Find(u => u.Id == group.SharedUserId); + Assert.NotNull(sharedUser); + Assert.False(string.IsNullOrEmpty(sharedUser!.Password)); + } +} diff --git a/Jellyfin.Plugin.WatchedTogether.Tests/SharedAccountEndToEndTests.cs b/Jellyfin.Plugin.WatchedTogether.Tests/SharedAccountEndToEndTests.cs new file mode 100644 index 0000000..c771dc9 --- /dev/null +++ b/Jellyfin.Plugin.WatchedTogether.Tests/SharedAccountEndToEndTests.cs @@ -0,0 +1,160 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Plugin.WatchedTogether.Auth; +using Jellyfin.Plugin.WatchedTogether.Services; +using MediaBrowser.Controller.Authentication; +using MediaBrowser.Controller.Library; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Plugin.WatchedTogether.Tests; + +/// +/// Exercises the whole path a real login takes: a group created on demand by typing "alice+bob", +/// then logged into again afterwards by each member in turn. +/// +/// +/// The other suites mock , which is why an ordering bug inside the +/// real provisioning code reached a release. These tests wire the real services together and only +/// stub the host's user manager and crypto. +/// +[Collection(nameof(PluginTestContext))] +public class SharedAccountEndToEndTests +{ + private const string AliceHash = "$PBKDF2-SHA512$iterations=210000$A1A1A1A1$AAAAAAAABBBBBBBB"; + private const string BobHash = "$PBKDF2-SHA512$iterations=210000$B2B2B2B2$CCCCCCCCDDDDDDDD"; + + private static readonly string AuthProviderId = + typeof(SharedAccountAuthenticationProvider).FullName!; + + private sealed record Harness( + SharedAccountAuthenticationProvider Provider, + List Users); + + private static User MakeUser(string name, string? password = null) + => new(name, "Prov", "ResetProv") { Password = password! }; + + /// + /// Wires the real provisioning, group, dynamic-group and authentication services over a user + /// manager that behaves like Jellyfin's: password changes dispatch to the user's assigned + /// provider, and users resolve by both name and id. + /// + private static Harness MakeHarness(List users, params (string Hash, string Password)[] validPairs) + { + var crypto = new StubCryptoProvider(validPairs); + var userManager = new Mock(); + + userManager.Setup(m => m.GetUserById(It.IsAny())) + .Returns((Guid id) => users.Find(u => u.Id == id)!); + + userManager.Setup(m => m.GetUserByName(It.IsAny())) + .Returns((string n) => users.Find( + u => string.Equals(u.Username, n, StringComparison.OrdinalIgnoreCase))!); + + userManager.Setup(m => m.CreateUserAsync(It.IsAny())) + .ReturnsAsync((string name) => + { + var created = MakeUser(name); + users.Add(created); + return created; + }); + + userManager.Setup(m => m.UpdateUserAsync(It.IsAny())).Returns(Task.CompletedTask); + + userManager.Setup(m => m.ChangePassword(It.IsAny(), It.IsAny())) + .Returns((User user, string password) => + { + // Jellyfin routes this to the user's assigned provider; ours refuses by design. + if (string.Equals(user.AuthenticationProviderId, AuthProviderId, StringComparison.Ordinal)) + { + throw new NotSupportedException( + "A Watched Together shared account has no password of its own."); + } + + user.Password = password; + return Task.CompletedTask; + }); + + var groupService = new GroupService( + userManager.Object, + NullLogger.Instance); + + var provisioning = new ProvisioningService( + userManager.Object, + Mock.Of(), + NullLogger.Instance); + + var dynamicGroups = new DynamicGroupService( + userManager.Object, + provisioning, + crypto, + NullLogger.Instance); + + var provider = new SharedAccountAuthenticationProvider( + crypto, + new Lazy(() => groupService), + new Lazy(() => dynamicGroups), + NullLogger.Instance); + + return new Harness(provider, users); + } + + [Theory] + [InlineData("alice-pw")] + [InlineData("bob-pw")] + public async Task GroupCreatedOnDemand_ThenUnlockedByEitherMemberPassword(string creatingPassword) + { + using var ctx = PluginTestContext.Create(); + + var alice = MakeUser("alice", AliceHash); + var bob = MakeUser("bob", BobHash); + var users = new List { alice, bob }; + + var h = MakeHarness(users, (AliceHash, "alice-pw"), (BobHash, "bob-pw")); + + // Creating the group by typing both names. Whichever member types their own password, the + // resulting account must behave identically. + var created = await h.Provider.Authenticate("alice+bob", creatingPassword, null); + Assert.Equal("alice+bob", created.Username); + + var shared = h.Users.Find(u => u.Username == "alice+bob"); + Assert.NotNull(shared); + + // The account exists now, so Jellyfin resolves it and hands it to us as resolvedUser. Both + // members must be able to unlock it, regardless of who created it. + var asAlice = await h.Provider.Authenticate("alice+bob", "alice-pw", shared); + Assert.Equal("alice+bob", asAlice.Username); + + var asBob = await h.Provider.Authenticate("alice+bob", "bob-pw", shared); + Assert.Equal("alice+bob", asBob.Username); + + // And an outsider's password still must not. + await Assert.ThrowsAsync( + () => h.Provider.Authenticate("alice+bob", "not-a-member-pw", shared!)); + } + + [Fact] + public async Task GroupCreatedOnDemand_ClaimsTheAccountAndKeepsAPassword() + { + using var ctx = PluginTestContext.Create(); + + var alice = MakeUser("alice", AliceHash); + var bob = MakeUser("bob", BobHash); + var users = new List { alice, bob }; + + var h = MakeHarness(users, (AliceHash, "alice-pw"), (BobHash, "bob-pw")); + + await h.Provider.Authenticate("alice+bob", "alice-pw", null); + + var shared = h.Users.Find(u => u.Username == "alice+bob"); + Assert.NotNull(shared); + + // Claimed by us, so future logins route here, and holding a password of its own so it is + // not directly loginable if the provider is ever unassigned. + Assert.Equal(AuthProviderId, shared!.AuthenticationProviderId); + Assert.False(string.IsNullOrEmpty(shared.Password)); + } +} diff --git a/Jellyfin.Plugin.WatchedTogether/Services/ProvisioningService.cs b/Jellyfin.Plugin.WatchedTogether/Services/ProvisioningService.cs index 0badb9f..7093587 100644 --- a/Jellyfin.Plugin.WatchedTogether/Services/ProvisioningService.cs +++ b/Jellyfin.Plugin.WatchedTogether/Services/ProvisioningService.cs @@ -100,15 +100,20 @@ public class ProvisioningService : IProvisioningService var sharedUser = await _userManager.CreateUserAsync(accountName).ConfigureAwait(false); + // The shared account never authenticates against its own password - our provider checks + // member hashes instead. Setting a random one avoids leaving a passwordless account behind + // if the provider is ever unassigned. + // + // This must happen before the account is claimed below. IUserManager.ChangePassword + // dispatches to the provider the user is currently assigned to, and ours refuses the call + // by design, so claiming first would make provisioning throw NotSupportedException. A + // freshly created user is still on Jellyfin's default provider, which stores the hash. + await _userManager.ChangePassword(sharedUser, GenerateUnusedPassword()).ConfigureAwait(false); + // Route this account's logins through our provider. Jellyfin matches providers by // GetType().FullName, the same key the SSO plugin uses, and the assignment only sticks // once the user is updated. sharedUser.AuthenticationProviderId = AuthProviderId; - - // The shared account never authenticates against its own password - our provider checks - // member hashes instead. Setting a random one avoids leaving a passwordless account behind - // if the provider is ever unassigned. - await _userManager.ChangePassword(sharedUser, GenerateUnusedPassword()).ConfigureAwait(false); await _userManager.UpdateUserAsync(sharedUser).ConfigureAwait(false); await ApplyLibraryAccessAsync(sharedUser.Id, distinctIds).ConfigureAwait(false);