Set the shared account's password before claiming it
Provisioning assigned AuthenticationProviderId and only then called IUserManager.ChangePassword. Jellyfin dispatches that call to the provider the user is currently assigned to, so it reached this plugin's own ChangePassword, which refuses by design. Creating a group by typing "alice+bob" at the login screen therefore died with NotSupportedException. Set the placeholder password first, while the freshly created account is still on Jellyfin's default provider, then claim it. The new end-to-end tests wire the real provisioning, group and authentication services together rather than mocking IProvisioningService, and cover a group created on demand being unlocked afterwards by either member's password. With the old ordering restored, six of them fail with the original exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Covers shared-account provisioning against a user manager that dispatches password changes the
|
||||
/// way Jellyfin's real one does.
|
||||
/// </summary>
|
||||
[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");
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private static Mock<IUserManager> MakeUserManager(List<User> users, List<string> callLog)
|
||||
{
|
||||
var userManager = new Mock<IUserManager>();
|
||||
|
||||
userManager.Setup(m => m.GetUserById(It.IsAny<Guid>()))
|
||||
.Returns((Guid id) => users.Find(u => u.Id == id));
|
||||
|
||||
userManager.Setup(m => m.CreateUserAsync(It.IsAny<string>()))
|
||||
.ReturnsAsync((string name) =>
|
||||
{
|
||||
var created = MakeUser(name);
|
||||
users.Add(created);
|
||||
callLog.Add("CreateUser");
|
||||
return created;
|
||||
});
|
||||
|
||||
userManager.Setup(m => m.ChangePassword(It.IsAny<User>(), It.IsAny<string>()))
|
||||
.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<MediaBrowser.Model.Cryptography.ICryptoProvider>(),
|
||||
new Lazy<IGroupService>(() => Mock.Of<IGroupService>()),
|
||||
new Lazy<IDynamicGroupService>(() => Mock.Of<IDynamicGroupService>()),
|
||||
NullLogger<Auth.SharedAccountAuthenticationProvider>.Instance)
|
||||
.ChangePassword(user, password);
|
||||
}
|
||||
|
||||
user.Password = password;
|
||||
return Task.CompletedTask;
|
||||
});
|
||||
|
||||
userManager.Setup(m => m.UpdateUserAsync(It.IsAny<User>()))
|
||||
.Returns((User user) =>
|
||||
{
|
||||
callLog.Add($"UpdateUser(provider={user.AuthenticationProviderId})");
|
||||
return Task.CompletedTask;
|
||||
});
|
||||
|
||||
return userManager;
|
||||
}
|
||||
|
||||
private static ProvisioningService MakeService(Mock<IUserManager> userManager)
|
||||
=> new(
|
||||
userManager.Object,
|
||||
Mock.Of<ILibraryAccessService>(),
|
||||
NullLogger<ProvisioningService>.Instance);
|
||||
|
||||
[Fact]
|
||||
public async Task CreateGroupAsync_SetsPasswordBeforeClaimingTheAccount()
|
||||
{
|
||||
using var context = PluginTestContext.Create();
|
||||
|
||||
var alice = MakeUser("alice");
|
||||
var bob = MakeUser("bob");
|
||||
var users = new List<User> { alice, bob };
|
||||
var callLog = new List<string>();
|
||||
|
||||
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<User> { alice, bob };
|
||||
var callLog = new List<string>();
|
||||
|
||||
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<User> { alice, bob };
|
||||
var callLog = new List<string>();
|
||||
|
||||
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));
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The other suites mock <see cref="IProvisioningService"/>, 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.
|
||||
/// </remarks>
|
||||
[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<User> Users);
|
||||
|
||||
private static User MakeUser(string name, string? password = null)
|
||||
=> new(name, "Prov", "ResetProv") { Password = password! };
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private static Harness MakeHarness(List<User> users, params (string Hash, string Password)[] validPairs)
|
||||
{
|
||||
var crypto = new StubCryptoProvider(validPairs);
|
||||
var userManager = new Mock<IUserManager>();
|
||||
|
||||
userManager.Setup(m => m.GetUserById(It.IsAny<Guid>()))
|
||||
.Returns((Guid id) => users.Find(u => u.Id == id)!);
|
||||
|
||||
userManager.Setup(m => m.GetUserByName(It.IsAny<string>()))
|
||||
.Returns((string n) => users.Find(
|
||||
u => string.Equals(u.Username, n, StringComparison.OrdinalIgnoreCase))!);
|
||||
|
||||
userManager.Setup(m => m.CreateUserAsync(It.IsAny<string>()))
|
||||
.ReturnsAsync((string name) =>
|
||||
{
|
||||
var created = MakeUser(name);
|
||||
users.Add(created);
|
||||
return created;
|
||||
});
|
||||
|
||||
userManager.Setup(m => m.UpdateUserAsync(It.IsAny<User>())).Returns(Task.CompletedTask);
|
||||
|
||||
userManager.Setup(m => m.ChangePassword(It.IsAny<User>(), It.IsAny<string>()))
|
||||
.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<GroupService>.Instance);
|
||||
|
||||
var provisioning = new ProvisioningService(
|
||||
userManager.Object,
|
||||
Mock.Of<ILibraryAccessService>(),
|
||||
NullLogger<ProvisioningService>.Instance);
|
||||
|
||||
var dynamicGroups = new DynamicGroupService(
|
||||
userManager.Object,
|
||||
provisioning,
|
||||
crypto,
|
||||
NullLogger<DynamicGroupService>.Instance);
|
||||
|
||||
var provider = new SharedAccountAuthenticationProvider(
|
||||
crypto,
|
||||
new Lazy<IGroupService>(() => groupService),
|
||||
new Lazy<IDynamicGroupService>(() => dynamicGroups),
|
||||
NullLogger<SharedAccountAuthenticationProvider>.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<User> { 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<AuthenticationException>(
|
||||
() => 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<User> { 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));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user