Merge pull request #238 from bdach/better-client-identifier

Read & use client-provided session GUID for concurrency control

SampleMultiplayerClient/SampleMultiplayerClient.csproj

@@ -11,7 +11,7 @@
  <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.MessagePack" Version="8.0.2" />
  <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="8.0.2" />
  <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
- <PackageReference Include="ppy.osu.Game" Version="2024.628.0" />
+ <PackageReference Include="ppy.osu.Game" Version="2024.718.0" />
  </ItemGroup>
 
 </Project>

SampleSpectatorClient/SampleSpectatorClient.csproj

@@ -11,7 +11,7 @@
  <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.MessagePack" Version="8.0.2" />
  <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="8.0.2" />
  <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
- <PackageReference Include="ppy.osu.Game" Version="2024.628.0" />
+ <PackageReference Include="ppy.osu.Game" Version="2024.718.0" />
  </ItemGroup>
 
 </Project>

osu.Server.Spectator/ConcurrentConnectionLimiter.cs

@@ -55,7 +55,7 @@ private async Task registerConnection(HubLifetimeContext context)
  return;
  }
 
- if (context.Context.GetTokenId() == userState.Item.TokenId)
+ if (userState.Item.IsConnectionFromSameClient(context))
  {
  // The assumption is that the client has already dropped the old connection,
  // so we don't bother to ask for a disconnection.
@@ -99,15 +99,7 @@ private void log(HubLifetimeContext context, string message)
 
  using (var userState = await connectionStates.GetForUse(userId))
  {
- string? registeredConnectionId = null;
-
- bool tokenIdMatches = invocationContext.Context.GetTokenId() == userState.Item?.TokenId;
- bool hubRegistered = userState.Item?.ConnectionIds.TryGetValue(invocationContext.Hub.GetType(), out registeredConnectionId) == true;
- bool connectionIdMatches = registeredConnectionId == invocationContext.Context.ConnectionId;
-
- bool connectionIsValid = tokenIdMatches && hubRegistered && connectionIdMatches;
-
- if (!connectionIsValid)
+ if (userState.Item?.ExistingConnectionMatches(invocationContext) != true)
  throw new InvalidOperationException($"State is not valid for this connection, context: {LoggingHubFilter.GetMethodCallDisplayString(invocationContext)})");
  }
 
@@ -129,15 +121,7 @@ private async Task unregisterConnection(HubLifetimeContext context, Exception? e
 
  using (var userState = await connectionStates.GetForUse(userId, true))
  {
- string? registeredConnectionId = null;
-
- bool tokenIdMatches = context.Context.GetTokenId() == userState.Item?.TokenId;
- bool hubRegistered = userState.Item?.ConnectionIds.TryGetValue(context.Hub.GetType(), out registeredConnectionId) == true;
- bool connectionIdMatches = registeredConnectionId == context.Context.ConnectionId;
-
- bool connectionCanBeCleanedUp = tokenIdMatches && hubRegistered && connectionIdMatches;
-
- if (connectionCanBeCleanedUp)
+ if (userState.Item?.ExistingConnectionMatches(context) == true)
  {
  log(context, "disconnected from hub");
  userState.Item!.ConnectionIds.Remove(context.Hub.GetType());

osu.Server.Spectator/Entities/ConnectionState.cs

@@ -4,8 +4,11 @@
 using System;
 using System.Collections.Generic;
 using Microsoft.AspNetCore.SignalR;
+using osu.Game.Online;
 using osu.Server.Spectator.Extensions;
 
+#pragma warning disable CS0618 // Type or member is obsolete
+
 namespace osu.Server.Spectator.Entities
 {
  /// <summary>
@@ -14,9 +17,19 @@ namespace osu.Server.Spectator.Entities
  public class ConnectionState
  {
  /// <summary>
- /// The unique ID of the JWT the user is using to authenticate.
+ /// A client-side generated GUID identifying the client instance connecting to this server.
  /// This is used to control user uniqueness.
  /// </summary>
+ public readonly Guid? ClientSessionId;
+
+ /// <summary>
+ /// The unique ID of the JWT the user is using to authenticate.
+ /// </summary>
+ /// <remarks>
+ /// This was previously used as a method of controlling user uniqueness / limiting concurrency,
+ /// but it turned out to be a bad fit for the purpose (see https://github.com/ppy/osu/issues/26338#issuecomment-2222935517).
+ /// </remarks>
+ [Obsolete("Use ClientSessionId instead.")] // Can be removed 2024-08-18
  public readonly string TokenId;
 
  /// <summary>
@@ -33,6 +46,9 @@ public ConnectionState(HubLifetimeContext context)
  {
  TokenId = context.Context.GetTokenId();
 
+ if (tryGetClientSessionID(context, out var clientSessionId))
+ ClientSessionId = clientSessionId;
+
  RegisterConnectionId(context);
  }
 
@@ -42,5 +58,37 @@ public ConnectionState(HubLifetimeContext context)
  /// <param name="context">The hub context to retrieve information from.</param>
  public void RegisterConnectionId(HubLifetimeContext context)
  => ConnectionIds[context.Hub.GetType()] = context.Context.ConnectionId;
+
+ public bool IsConnectionFromSameClient(HubLifetimeContext context)
+ {
+ if (tryGetClientSessionID(context, out var clientSessionId))
+ return ClientSessionId == clientSessionId;
+
+ // Legacy pathway using JTI claim left for compatibility with older clients – can be removed 2024-08-18
+ return TokenId == context.Context.GetTokenId();
+ }
+
+ public bool ExistingConnectionMatches(HubInvocationContext context)
+ {
+ bool hubRegistered = ConnectionIds.TryGetValue(context.Hub.GetType(), out string? registeredConnectionId);
+ bool connectionIdMatches = registeredConnectionId == context.Context.ConnectionId;
+
+ return hubRegistered && connectionIdMatches;
+ }
+
+ public bool ExistingConnectionMatches(HubLifetimeContext context)
+ {
+ bool hubRegistered = ConnectionIds.TryGetValue(context.Hub.GetType(), out string? registeredConnectionId);
+ bool connectionIdMatches = registeredConnectionId == context.Context.ConnectionId;
+
+ return hubRegistered && connectionIdMatches;
+ }
+
+ private static bool tryGetClientSessionID(HubLifetimeContext context, out Guid clientSessionId)
+ {
+ clientSessionId = Guid.Empty;
+ return context.Context.GetHttpContext()?.Request.Headers.TryGetValue(HubClientConnector.CLIENT_SESSION_ID_HEADER, out var value) == true
+ && Guid.TryParse(value, out clientSessionId);
+ }
  }
 }

osu.Server.Spectator/Hubs/Metadata/MetadataHub.cs

@@ -7,6 +7,7 @@
 using Microsoft.Extensions.Caching.Distributed;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Extensions.Logging;
+using osu.Game.Online;
 using osu.Game.Online.Metadata;
 using osu.Game.Users;
 using osu.Server.Spectator.Database;
@@ -48,7 +49,7 @@ public override async Task OnConnectedAsync()
  {
  string? versionHash = null;
 
- if (Context.GetHttpContext()?.Request.Headers.TryGetValue("OsuVersionHash", out StringValues headerValue) == true)
+ if (Context.GetHttpContext()?.Request.Headers.TryGetValue(HubClientConnector.VERSION_HASH_HEADER, out StringValues headerValue) == true)
  {
  versionHash = headerValue;
 

osu.Server.Spectator/ServerShuttingDownException.cs

@@ -2,13 +2,14 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using Microsoft.AspNetCore.SignalR;
+using osu.Game.Online;
 
 namespace osu.Server.Spectator
 {
  public class ServerShuttingDownException : HubException
  {
  public ServerShuttingDownException()
- : base("Server is shutting down.")
+ : base(HubClientConnector.SERVER_SHUTDOWN_MESSAGE)
  {
  }
  }

osu.Server.Spectator/osu.Server.Spectator.csproj

@@ -15,11 +15,11 @@
  <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="8.0.2" />
  <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.2" />
  <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
- <PackageReference Include="ppy.osu.Game" Version="2024.628.0" />
- <PackageReference Include="ppy.osu.Game.Rulesets.Catch" Version="2024.628.0" />
- <PackageReference Include="ppy.osu.Game.Rulesets.Mania" Version="2024.628.0" />
- <PackageReference Include="ppy.osu.Game.Rulesets.Osu" Version="2024.628.0" />
- <PackageReference Include="ppy.osu.Game.Rulesets.Taiko" Version="2024.628.0" />
+ <PackageReference Include="ppy.osu.Game" Version="2024.718.0" />
+ <PackageReference Include="ppy.osu.Game.Rulesets.Catch" Version="2024.718.0" />
+ <PackageReference Include="ppy.osu.Game.Rulesets.Mania" Version="2024.718.0" />
+ <PackageReference Include="ppy.osu.Game.Rulesets.Osu" Version="2024.718.0" />
+ <PackageReference Include="ppy.osu.Game.Rulesets.Taiko" Version="2024.718.0" />
  <PackageReference Include="ppy.osu.Server.OsuQueueProcessor" Version="2024.507.0" />
  <PackageReference Include="Sentry.AspNetCore" Version="4.3.0" />
  </ItemGroup>