Disconnect existing client connections when another client instance for same user is detected

[bdach]

• Depends on (and is the server side of) Implement flow allowing disconnection from online services when another client instance for same user is detected osu#25522
• Depends on game package w/ above
• Closes The multiplayer server should not allow multiple connections from the same user on different devices #80

The detection of client instances relies on the JSON web tokens issued by web, namely by the uniqueness of the jti claim, which is optional but standardised to seemingly be suitable for this purpose.

Tokens are valid for 24 hours, which seems like a sane session length. If the use of jti causes problems, we can (will have to?) come up with our own unique ID generating scheme on the client side.

[bdach]

When working on something completely different I noticed that there is a place in GracefulShutdownManager where EntityStores are supposed to be registered. So in 54226e7 I registered the new entity store added in this pull there so that it also stops accepting new entities. I believe this should be fine since during a new spectator server version rollout only the newer instance should be accepting connections, but this is likely something that warrants to be scrutinised in review...?

[peppy]

Hmm, I find this just a tad confusing. Might be better to say something like "In SignalR, connection IDs are unique per connection. Because we use multiple hubs and a user is expected to be connected to each hub, we use a dictionary to track connections across all hubs for a specific user."

[peppy]

Here's a diff with this suggestion applied, alongside some other xmldoc and code structure improvements. Please apply as you see fit:

diff --git a/osu.Server.Spectator/ConcurrentConnectionLimiter.cs b/osu.Server.Spectator/ConcurrentConnectionLimiter.cs
index 9c25535..835143f 100644
--- a/osu.Server.Spectator/ConcurrentConnectionLimiter.cs
+++ b/osu.Server.Spectator/ConcurrentConnectionLimiter.cs
@@ -42,34 +42,41 @@ namespace osu.Server.Spectator
 
                 using (var userState = await connectionStates.GetForUse(userId, true))
                 {
-                    if (context.Context.GetTokenId() == userState.Item?.TokenId)
+                    if (userState.Item == null)
                     {
+                        log(context, "connection from first client instance");
+                        userState.Item = new ConnectionState(context);
+                        return;
+                    }
+
+                    if (context.Context.GetTokenId() == userState.Item.TokenId)
+                    {
+                        // The assumption is that the client has already dropped the old connection,
+                        // so we don't bother to ask for a disconnection.
+
                         log(context, "subsequent connection from same client instance, registering");
+
+                        // Importantly, this will replace the old connection, ensuring it cannot be
+                        // used to communicate on anymore.
                         userState.Item.RegisterConnectionId(context);
                         return;
                     }
 
-                    if (userState.Item != null)
+                    log(context, "connection from new client instance, dropping existing state");
+
+                    foreach (var hubType in stateful_user_hubs)
                     {
-                        log(context, "connection from new client instance, dropping existing state");
+                        var hubContextType = typeof(IHubContext<>).MakeGenericType(hubType);
+                        var hubContext = serviceProvider.GetRequiredService(hubContextType) as IHubContext;
 
-                        foreach (var hubType in stateful_user_hubs)
+                        if (userState.Item.ConnectionIds.TryGetValue(hubType, out var connectionId))
                         {
-                            var hubContextType = typeof(IHubContext<>).MakeGenericType(hubType);
-                            var hubContext = serviceProvider.GetRequiredService(hubContextType) as IHubContext;
-
-                            if (userState.Item.ConnectionIds.TryGetValue(hubType, out var connectionId))
-                            {
-                                hubContext?.Clients.Client(connectionId)
-                                          .SendCoreAsync(nameof(IStatefulUserHubClient.DisconnectRequested), Array.Empty<object>());
-                            }
+                            hubContext?.Clients.Client(connectionId)
+                                      .SendCoreAsync(nameof(IStatefulUserHubClient.DisconnectRequested), Array.Empty<object>());
                         }
-
-                        log(context, "existing state dropped");
                     }
-                    else
-                        log(context, "connection from first client instance");
 
+                    log(context, "existing state dropped");
                     userState.Item = new ConnectionState(context);
                 }
             }
diff --git a/osu.Server.Spectator/Entities/ConnectionState.cs b/osu.Server.Spectator/Entities/ConnectionState.cs
index dff8e00..80615f7 100644
--- a/osu.Server.Spectator/Entities/ConnectionState.cs
+++ b/osu.Server.Spectator/Entities/ConnectionState.cs
@@ -8,6 +8,9 @@ using osu.Server.Spectator.Extensions;
 
 namespace osu.Server.Spectator.Entities
 {
+    /// <summary>
+    /// Maintains the connection state of a single client (notably, client, not user) across multiple hubs.
+    /// </summary>
     public class ConnectionState
     {
         /// <summary>
@@ -17,11 +20,13 @@ namespace osu.Server.Spectator.Entities
         public readonly string TokenId;
 
         /// <summary>
-        /// The connection IDs of the user.
+        /// The connection IDs of the user for each hub type.
         /// </summary>
         /// <remarks>
-        /// In SignalR, connection IDs are unique per user, <em>and</em> per hub instance.
-        /// Therefore, to keep track of all of them, a dictionary is necessary.
+        /// In SignalR, connection IDs are unique per connection.
+        ///
+        /// Because we use multiple hubs and a user is expected to be connected to each hub,
+        /// we use a dictionary to track connections across all hubs for a specific user.
         /// </remarks>
         public readonly Dictionary<Type, string> ConnectionIds = new Dictionary<Type, string>();
 
@@ -32,6 +37,10 @@ namespace osu.Server.Spectator.Entities
             RegisterConnectionId(context);
         }
 
+        /// <summary>
+        /// Registers the provided hub/connection context, replacing any existing connection for the hub type.
+        /// </summary>
+        /// <param name="context">The hub context to retrieve information from.</param>
         public void RegisterConnectionId(HubLifetimeContext context)
             => ConnectionIds[context.Hub.GetType()] = context.Context.ConnectionId;
     }

[bdach]

Applied with minor alterations.