share quick connect state between instances

Move the pending requests and authorized secrets out of process so the
initiate, authorize and exchange legs can land on different replicas.

tests/Jellyfin.Server.Implementations.Tests/QuickConnect/QuickConnectManagerTests.cs

@@ -8,6 +8,7 @@ using MediaBrowser.Common.Extensions;
 using MediaBrowser.Controller.Authentication;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Net;
+using MediaBrowser.Controller.QuickConnect;
 using MediaBrowser.Model.Configuration;
 using Moq;
 using Xunit;
@@ -40,6 +41,8 @@ namespace Jellyfin.Server.Implementations.Tests.QuickConnect
                 ConfigureMembers = true
             }).Inject(configManager.Object);
 
+            _fixture.Inject<IQuickConnectStore>(new InMemoryQuickConnectStore());
+
             // User object contains circular references.
             _fixture.Behaviors.OfType<ThrowingRecursionBehavior>().ToList()
                 .ForEach(b => _fixture.Behaviors.Remove(b));
@@ -60,8 +63,8 @@ namespace Jellyfin.Server.Implementations.Tests.QuickConnect
         [InlineData("Device", "", "Client", "1.0.0")]
         [InlineData("Device", "DeviceId", "", "1.0.0")]
         [InlineData("Device", "DeviceId", "Client", "")]
-        public void TryConnect_InvalidAuthorizationInfo_ThrowsArgumentException(string device, string deviceId, string client, string version)
-            => Assert.Throws<ArgumentException>(() => _quickConnectManager.TryConnect(
+        public async Task TryConnect_InvalidAuthorizationInfo_ThrowsArgumentException(string device, string deviceId, string client, string version)
+            => await Assert.ThrowsAsync<ArgumentException>(() => _quickConnectManager.TryConnect(
                 new AuthorizationInfo
                 {
                     Device = device,
@@ -71,17 +74,17 @@ namespace Jellyfin.Server.Implementations.Tests.QuickConnect
                 }));
 
         [Fact]
-        public void TryConnect_QuickConnectUnavailable_ThrowsAuthenticationException()
+        public async Task TryConnect_QuickConnectUnavailable_ThrowsAuthenticationException()
         {
             _config.QuickConnectAvailable = false;
-            Assert.Throws<AuthenticationException>(() => _quickConnectManager.TryConnect(_quickConnectAuthInfo));
+            await Assert.ThrowsAsync<AuthenticationException>(() => _quickConnectManager.TryConnect(_quickConnectAuthInfo));
         }
 
         [Fact]
-        public void CheckRequestStatus_QuickConnectUnavailable_ThrowsAuthenticationException()
+        public async Task CheckRequestStatus_QuickConnectUnavailable_ThrowsAuthenticationException()
         {
             _config.QuickConnectAvailable = false;
-            Assert.Throws<AuthenticationException>(() => _quickConnectManager.CheckRequestStatus(string.Empty));
+            await Assert.ThrowsAsync<AuthenticationException>(() => _quickConnectManager.CheckRequestStatus(string.Empty));
         }
 
         [Fact]
@@ -92,10 +95,10 @@ namespace Jellyfin.Server.Implementations.Tests.QuickConnect
         }
 
         [Fact]
-        public void GetAuthorizedRequest_QuickConnectUnavailable_ThrowsAuthenticationException()
+        public async Task GetAuthorizedRequest_QuickConnectUnavailable_ThrowsAuthenticationException()
         {
             _config.QuickConnectAvailable = false;
-            Assert.Throws<AuthenticationException>(() => _quickConnectManager.GetAuthorizedRequest(string.Empty));
+            await Assert.ThrowsAsync<AuthenticationException>(() => _quickConnectManager.GetAuthorizedRequest(string.Empty));
         }
 
         [Fact]
@@ -106,34 +109,46 @@ namespace Jellyfin.Server.Implementations.Tests.QuickConnect
         }
 
         [Fact]
-        public void CheckRequestStatus_QuickConnectAvailable_Success()
+        public async Task CheckRequestStatus_QuickConnectAvailable_Success()
         {
             _config.QuickConnectAvailable = true;
-            var res1 = _quickConnectManager.TryConnect(_quickConnectAuthInfo);
-            var res2 = _quickConnectManager.CheckRequestStatus(res1.Secret);
-            Assert.Equal(res1, res2);
+            var res1 = await _quickConnectManager.TryConnect(_quickConnectAuthInfo);
+            var res2 = await _quickConnectManager.CheckRequestStatus(res1.Secret);
+            Assert.Equal(res1.Secret, res2.Secret);
+            Assert.Equal(res1.Code, res2.Code);
         }
 
         [Fact]
-        public void CheckRequestStatus_UnknownSecret_ThrowsResourceNotFoundException()
+        public async Task CheckRequestStatus_UnknownSecret_ThrowsResourceNotFoundException()
         {
             _config.QuickConnectAvailable = true;
-            Assert.Throws<ResourceNotFoundException>(() => _quickConnectManager.CheckRequestStatus("Unknown secret"));
+            await Assert.ThrowsAsync<ResourceNotFoundException>(() => _quickConnectManager.CheckRequestStatus("Unknown secret"));
         }
 
         [Fact]
-        public void GetAuthorizedRequest_UnknownSecret_ThrowsResourceNotFoundException()
+        public async Task GetAuthorizedRequest_UnknownSecret_ThrowsResourceNotFoundException()
         {
             _config.QuickConnectAvailable = true;
-            Assert.Throws<ResourceNotFoundException>(() => _quickConnectManager.GetAuthorizedRequest("Unknown secret"));
+            await Assert.ThrowsAsync<ResourceNotFoundException>(() => _quickConnectManager.GetAuthorizedRequest("Unknown secret"));
         }
 
         [Fact]
         public async Task AuthorizeRequest_QuickConnectAvailable_Success()
         {
             _config.QuickConnectAvailable = true;
-            var res = _quickConnectManager.TryConnect(_quickConnectAuthInfo);
+            var res = await _quickConnectManager.TryConnect(_quickConnectAuthInfo);
             Assert.True(await _quickConnectManager.AuthorizeRequest(Guid.Empty, res.Code));
         }
+
+        [Fact]
+        public async Task GetAuthorizedRequest_SecondExchange_ThrowsResourceNotFoundException()
+        {
+            _config.QuickConnectAvailable = true;
+            var res = await _quickConnectManager.TryConnect(_quickConnectAuthInfo);
+            await _quickConnectManager.AuthorizeRequest(Guid.Empty, res.Code);
+
+            Assert.NotNull(await _quickConnectManager.GetAuthorizedRequest(res.Secret));
+            await Assert.ThrowsAsync<ResourceNotFoundException>(() => _quickConnectManager.GetAuthorizedRequest(res.Secret));
+        }
     }
 }

tests/Jellyfin.Server.Tests/HighAvailability/RedisTestServer.cs

@@ -0,0 +1,91 @@
+using System;
+using System.Threading.Tasks;
+using StackExchange.Redis;
+using Testcontainers.Redis;
+
+namespace Jellyfin.Server.Tests.HighAvailability;
+
+/// <summary>
+/// Hands out a Redis server for the tests that need one. A server named by <c>JELLYFIN_TEST_REDIS</c> is
+/// used as is, so CI can run one beside the step instead of a docker daemon of its own; without it a
+/// container is started through testcontainers.
+/// </summary>
+public sealed class RedisTestServer : IAsyncDisposable
+{
+    /// <summary>
+    /// The connection string of an already running server.
+    /// </summary>
+    public const string ConnectionStringVariable = "JELLYFIN_TEST_REDIS";
+
+    private readonly RedisContainer? _container;
+
+    private RedisTestServer(RedisContainer? container, string connectionString)
+    {
+        _container = container;
+        ConnectionString = connectionString;
+    }
+
+    /// <summary>
+    /// Gets the connection string of the running server.
+    /// </summary>
+    public string ConnectionString { get; }
+
+    /// <summary>
+    /// Starts or attaches to a Redis server and waits until it accepts connections.
+    /// </summary>
+    /// <returns>The running server.</returns>
+    public static async Task<RedisTestServer> StartAsync()
+    {
+        var provided = Environment.GetEnvironmentVariable(ConnectionStringVariable);
+        if (!string.IsNullOrWhiteSpace(provided))
+        {
+            var attached = new RedisTestServer(null, provided);
+            await attached.WaitUntilReadyAsync().ConfigureAwait(false);
+            return attached;
+        }
+
+        var container = new RedisBuilder("redis:7-alpine").Build();
+        await container.StartAsync().ConfigureAwait(false);
+
+        var started = new RedisTestServer(container, container.GetConnectionString());
+        await started.WaitUntilReadyAsync().ConfigureAwait(false);
+        return started;
+    }
+
+    /// <summary>
+    /// Opens a connection of its own, so each in-process stand-in for a replica talks to the server the
+    /// way a separate pod would.
+    /// </summary>
+    /// <returns>A new multiplexer.</returns>
+    public async Task<IConnectionMultiplexer> ConnectAsync()
+        => await ConnectionMultiplexer.ConnectAsync(ConnectionString).ConfigureAwait(false);
+
+    /// <inheritdoc/>
+    public async ValueTask DisposeAsync()
+    {
+        if (_container is not null)
+        {
+            await _container.DisposeAsync().ConfigureAwait(false);
+        }
+    }
+
+    private async Task WaitUntilReadyAsync()
+    {
+        for (var attempt = 1; ; attempt++)
+        {
+            try
+            {
+                var connection = await ConnectionMultiplexer.ConnectAsync(ConnectionString).ConfigureAwait(false);
+                await using (connection.ConfigureAwait(false))
+                {
+                    await connection.GetDatabase().PingAsync().ConfigureAwait(false);
+                    return;
+                }
+            }
+            catch (RedisException) when (attempt < 60)
+            {
+                await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false);
+            }
+        }
+    }
+}

tests/Jellyfin.Server.Tests/Jellyfin.Server.Tests.csproj

@@ -12,6 +12,7 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="Npgsql" />
     <PackageReference Include="Testcontainers.PostgreSql" />
+    <PackageReference Include="Testcontainers.Redis" />
     <PackageReference Include="xunit.v3" />
     <PackageReference Include="xunit.runner.visualstudio">
       <PrivateAssets>all</PrivateAssets>

tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectReplicaTests.cs

@@ -0,0 +1,308 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Threading;
+using System.Threading.Tasks;
+using Emby.Server.Implementations.QuickConnect;
+using Jellyfin.Data.Queries;
+using Jellyfin.Database.Implementations;
+using Jellyfin.Database.Implementations.DbConfiguration;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Entities.Security;
+using Jellyfin.Database.Implementations.Locking;
+using Jellyfin.Database.Providers.PostgreSQL;
+using Jellyfin.Server.Implementations.Devices;
+using Jellyfin.Server.Tests.HighAvailability;
+using Jellyfin.Server.Tests.Migrations;
+using MediaBrowser.Common.Extensions;
+using MediaBrowser.Controller.Authentication;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Devices;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Net;
+using MediaBrowser.Controller.QuickConnect;
+using MediaBrowser.Controller.Session;
+using MediaBrowser.Model.Configuration;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.QuickConnect;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Npgsql;
+using StackExchange.Redis;
+using Xunit;
+
+namespace Jellyfin.Server.Tests.QuickConnect;
+
+/// <summary>
+/// Three independently constructed <see cref="QuickConnectManager"/> instances over one PostgreSQL
+/// database and one Redis are the in-process stand-in for three replicas without sticky sessions: the
+/// initiate, authorize and exchange legs of one flow each land on a different one.
+/// </summary>
+[Trait("Category", "RequiresDocker")]
+public sealed class QuickConnectReplicaTests : IAsyncLifetime
+{
+    private static readonly AuthorizationInfo _authorizationInfo = new AuthorizationInfo
+    {
+        Device = "Living Room TV",
+        DeviceId = "device-1",
+        Client = "Jellyfin Web",
+        Version = "1.0.0"
+    };
+
+    private readonly List<IConnectionMultiplexer> _connections = new();
+
+    private PostgreSqlTestServer _postgres = null!;
+    private RedisTestServer _redis = null!;
+
+    /// <inheritdoc/>
+    public async ValueTask InitializeAsync()
+    {
+        _postgres = await PostgreSqlTestServer.StartAsync().ConfigureAwait(false);
+        _redis = await RedisTestServer.StartAsync().ConfigureAwait(false);
+    }
+
+    /// <inheritdoc/>
+    public async ValueTask DisposeAsync()
+    {
+        foreach (var connection in _connections)
+        {
+            await connection.DisposeAsync().ConfigureAwait(false);
+        }
+
+        await _redis.DisposeAsync().ConfigureAwait(false);
+        await _postgres.DisposeAsync().ConfigureAwait(false);
+    }
+
+    /// <summary>
+    /// The three legs of a quick connect flow land on three different replicas, and the token the third
+    /// one hands out is the one the second one minted into the shared database.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+    [Fact]
+    public async Task InitiateAuthorizeExchange_AcrossThreeReplicas_Succeeds()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var connectionString = await _postgres.CreateDatabaseAsync("quickconnect_replica_flow", cancellationToken);
+
+        await using var dataSource = new NpgsqlDataSourceBuilder(connectionString).Build();
+        var user = await CreateSchemaWithUserAsync(dataSource, cancellationToken);
+
+        var replicaA = await CreateReplicaAsync(dataSource, user);
+        var replicaB = await CreateReplicaAsync(dataSource, user);
+        var replicaC = await CreateReplicaAsync(dataSource, user);
+
+        var initiated = await replicaA.Manager.TryConnect(_authorizationInfo);
+
+        // The code is shown to the user on whichever replica serves the dashboard.
+        Assert.True(await replicaB.Manager.AuthorizeRequest(user.Id, initiated.Code));
+
+        var polled = await replicaC.Manager.CheckRequestStatus(initiated.Secret);
+        Assert.True(polled.Authenticated);
+        Assert.Equal(initiated.Code, polled.Code);
+        Assert.Equal(_authorizationInfo.DeviceId, polled.DeviceId);
+
+        var exchanged = await replicaC.Manager.GetAuthorizedRequest(initiated.Secret);
+
+        Assert.False(string.IsNullOrEmpty(exchanged.AccessToken));
+        Assert.Equal(user.Id, exchanged.User.Id);
+
+        var devices = await replicaA.Devices.GetDevices(new DeviceQuery { AccessToken = exchanged.AccessToken });
+        Assert.Equal(user.Id, Assert.Single(devices.Items).UserId);
+    }
+
+    /// <summary>
+    /// A secret is single use across the whole deployment: two replicas racing to exchange it must not
+    /// both hand out an access token.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+    [Fact]
+    public async Task Exchange_RacedOnTwoReplicas_SucceedsOnce()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var connectionString = await _postgres.CreateDatabaseAsync("quickconnect_replica_race", cancellationToken);
+
+        await using var dataSource = new NpgsqlDataSourceBuilder(connectionString).Build();
+        var user = await CreateSchemaWithUserAsync(dataSource, cancellationToken);
+
+        var replicaA = await CreateReplicaAsync(dataSource, user);
+        var replicaB = await CreateReplicaAsync(dataSource, user);
+        var replicaC = await CreateReplicaAsync(dataSource, user);
+
+        var initiated = await replicaA.Manager.TryConnect(_authorizationInfo);
+        await replicaB.Manager.AuthorizeRequest(user.Id, initiated.Code);
+
+        var onA = ExchangeAsync(replicaA.Manager, initiated.Secret);
+        var onC = ExchangeAsync(replicaC.Manager, initiated.Secret);
+        var outcomes = await Task.WhenAll(onA, onC);
+
+        Assert.Single(outcomes, outcome => outcome is not null);
+
+        // And it stays consumed for every later attempt, on any replica.
+        await Assert.ThrowsAsync<ResourceNotFoundException>(() => replicaB.Manager.GetAuthorizedRequest(initiated.Secret));
+    }
+
+    /// <summary>
+    /// An expired request is rejected on a replica that never saw it created, rather than resolving to a
+    /// stale authorization.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+    [Fact]
+    public async Task ExpiredRequest_IsRejectedOnEveryReplica()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var connectionString = await _postgres.CreateDatabaseAsync("quickconnect_replica_expiry", cancellationToken);
+
+        await using var dataSource = new NpgsqlDataSourceBuilder(connectionString).Build();
+        var user = await CreateSchemaWithUserAsync(dataSource, cancellationToken);
+
+        var replicaA = await CreateReplicaAsync(dataSource, user);
+        var replicaB = await CreateReplicaAsync(dataSource, user);
+
+        var initiated = await replicaA.Manager.TryConnect(_authorizationInfo);
+        Assert.NotNull(await replicaB.Manager.CheckRequestStatus(initiated.Secret));
+
+        // Shorten the stored expiry instead of waiting out the ten minute timeout.
+        await replicaA.Store.SetRequestAsync(initiated, DateTime.UtcNow.AddSeconds(1), cancellationToken);
+        await Task.Delay(TimeSpan.FromSeconds(2), cancellationToken);
+
+        await Assert.ThrowsAsync<ResourceNotFoundException>(() => replicaB.Manager.CheckRequestStatus(initiated.Secret));
+        await Assert.ThrowsAsync<ResourceNotFoundException>(() => replicaB.Manager.AuthorizeRequest(user.Id, initiated.Code));
+    }
+
+    /// <summary>
+    /// An authorization that was never exchanged expires too, so a code authorized and then abandoned
+    /// cannot be redeemed later from another replica.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+    [Fact]
+    public async Task ExpiredAuthorization_IsRejectedOnEveryReplica()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var connectionString = await _postgres.CreateDatabaseAsync("quickconnect_replica_auth_expiry", cancellationToken);
+
+        await using var dataSource = new NpgsqlDataSourceBuilder(connectionString).Build();
+        var user = await CreateSchemaWithUserAsync(dataSource, cancellationToken);
+
+        var replicaA = await CreateReplicaAsync(dataSource, user);
+        var replicaB = await CreateReplicaAsync(dataSource, user);
+
+        var initiated = await replicaA.Manager.TryConnect(_authorizationInfo);
+        await replicaA.Manager.AuthorizeRequest(user.Id, initiated.Code);
+
+        var stored = await replicaA.Store.GetRequestBySecretAsync(initiated.Secret, cancellationToken);
+        Assert.True(stored?.Authenticated);
+
+        await replicaA.Store.SetAuthorizationAsync(
+            initiated.Secret,
+            new AuthenticationResult { AccessToken = "stale" },
+            DateTime.UtcNow.AddSeconds(1),
+            cancellationToken);
+        await Task.Delay(TimeSpan.FromSeconds(2), cancellationToken);
+
+        await Assert.ThrowsAsync<ResourceNotFoundException>(() => replicaB.Manager.GetAuthorizedRequest(initiated.Secret));
+    }
+
+    private static async Task<AuthenticationResult?> ExchangeAsync(IQuickConnect manager, string secret)
+    {
+        try
+        {
+            return await manager.GetAuthorizedRequest(secret).ConfigureAwait(false);
+        }
+        catch (ResourceNotFoundException)
+        {
+            return null;
+        }
+    }
+
+    private static async Task<User> CreateSchemaWithUserAsync(NpgsqlDataSource dataSource, CancellationToken cancellationToken)
+    {
+        var context = CreateContext(dataSource);
+        await using (context.ConfigureAwait(false))
+        {
+            await context.Database.EnsureCreatedAsync(cancellationToken).ConfigureAwait(false);
+
+            var user = new User("quickconnect-user", "provider", "provider");
+            context.Users.Add(user);
+            await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
+
+            return user;
+        }
+    }
+
+    private static JellyfinDbContext CreateContext(NpgsqlDataSource dataSource)
+    {
+        var optionsBuilder = new DbContextOptionsBuilder<JellyfinDbContext>();
+        var provider = new PostgreSqlDatabaseProvider(dataSource);
+        provider.Initialise(optionsBuilder, new DatabaseConfigurationOptions { DatabaseType = "PostgreSQL" });
+        return new JellyfinDbContext(
+            optionsBuilder.Options,
+            NullLogger<JellyfinDbContext>.Instance,
+            provider,
+            new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
+    }
+
+    private async Task<Replica> CreateReplicaAsync(NpgsqlDataSource dataSource, User user)
+    {
+        var connection = await _redis.ConnectAsync().ConfigureAwait(false);
+        _connections.Add(connection);
+
+        var userManager = new Mock<IUserManager>();
+        userManager.Setup(manager => manager.GetUserById(user.Id)).Returns(user);
+        var deviceManager = new DeviceManager(new DataSourceContextFactory(dataSource), userManager.Object);
+
+        var configManager = new Mock<IServerConfigurationManager>();
+        configManager.Setup(manager => manager.Configuration).Returns(new ServerConfiguration { QuickConnectAvailable = true });
+
+        // Stands in for SessionManager.AuthenticateDirect: the token has to be minted into the shared
+        // database, because the replica that exchanges the secret is not the one that authorized it.
+        var sessionManager = new Mock<ISessionManager>();
+        sessionManager
+            .Setup(manager => manager.AuthenticateDirect(It.IsAny<AuthenticationRequest>()))
+            .Returns<AuthenticationRequest>(async request =>
+            {
+                var device = await deviceManager.CreateDevice(
+                    new Device(request.UserId, request.App, request.AppVersion, request.DeviceName, request.DeviceId)).ConfigureAwait(false);
+
+                return new AuthenticationResult
+                {
+                    AccessToken = device.AccessToken,
+                    ServerId = "server-1",
+                    User = new UserDto { Id = user.Id, Name = user.Username, ServerId = "server-1" },
+                    SessionInfo = new SessionInfoDto
+                    {
+                        Id = device.Id.ToString(CultureInfo.InvariantCulture),
+                        UserId = user.Id,
+                        UserName = user.Username,
+                        Client = request.App,
+                        DeviceId = request.DeviceId,
+                        DeviceName = request.DeviceName,
+                        ApplicationVersion = request.AppVersion
+                    }
+                };
+            });
+
+        var store = new RedisQuickConnectStore(connection, NullLogger<RedisQuickConnectStore>.Instance);
+        var manager = new QuickConnectManager(
+            configManager.Object,
+            NullLogger<QuickConnectManager>.Instance,
+            sessionManager.Object,
+            store);
+
+        return new Replica(manager, store, deviceManager);
+    }
+
+    private sealed record Replica(IQuickConnect Manager, IQuickConnectStore Store, IDeviceManager Devices);
+
+    private sealed class DataSourceContextFactory : IDbContextFactory<JellyfinDbContext>
+    {
+        private readonly NpgsqlDataSource _dataSource;
+
+        public DataSourceContextFactory(NpgsqlDataSource dataSource)
+        {
+            _dataSource = dataSource;
+        }
+
+        public JellyfinDbContext CreateDbContext() => CreateContext(_dataSource);
+    }
+}