fixes for auth connection reconnect issues

Author: dooly123

Basis Server/BasisNetworkServer/Auth/Interface.cs

@@ -17,6 +17,7 @@ public interface IAuthIdentity
         public void ProcessConnection(Configuration Configuration, ConnectionRequest ConnectionRequest, NetPeer NetPeer);
         public void DeInitialize();
         public void RemoveConnection(int NetPeer);
+        public bool RemoveConnection(int NetPeer, NetPeer Expected);
         public bool NetIDToUUID(NetPeer Peer, out string UUID);
         public bool UUIDToNetID(string UUID, out int Peer);
 

Basis Server/BasisNetworkServer/BasisServerHandleEvents.cs

@@ -347,16 +347,6 @@ public static void OnNetworkError(IPEndPoint endPoint, SocketError socketError)
         /// </summary>
         private static bool CleanupPeerSubsystems(NetPeer peer, int id)
         {
-            // A predecessor's disconnect can land after a reconnect has already taken the same id.
-            // Every teardown below is keyed by id alone, so running it for a peer that no longer
-            // owns the slot dismantles the live peer's state instead — the "direct connect works,
-            // then dies after a rejoin" symptom. An id held by nobody still cleans up, so a peer
-            // rejected before auth completed keeps releasing whatever partial state it made.
-            if (NetworkServer.AuthenticatedPeers.TryGetValue(id, out NetPeer holder) && !ReferenceEquals(holder, peer))
-            {
-                return false;
-            }
-
             // The auth-identity map is the primary UUID source, but it is empty when
             // UseAuthIdentity is off and can already be evicted on a reconnect collision. The
             // stored connect metadata carries the same server-computed UUID (OnNetworkAccepted
@@ -370,6 +360,19 @@ private static bool CleanupPeerSubsystems(NetPeer peer, int id)
                     uuid = meta.playerUUID;
                 }
             }
+
+            NetworkServer.AuthIdentity.RemoveConnection(id, peer);
+
+            // A predecessor's disconnect can land after a reconnect has already taken the same id.
+            // Every teardown below is keyed by id alone, so running it for a peer that no longer
+            // owns the slot dismantles the live peer's state instead — the "direct connect works,
+            // then dies after a rejoin" symptom. An id held by nobody still cleans up, so a peer
+            // rejected before auth completed keeps releasing whatever partial state it made.
+            if (NetworkServer.AuthenticatedPeers.TryGetValue(id, out NetPeer holder) && !ReferenceEquals(holder, peer))
+            {
+                return false;
+            }
+
             if (!string.IsNullOrEmpty(uuid))
             {
                 PermissionIntegration.RemovePlayerMeta(uuid);
@@ -378,7 +381,6 @@ private static bool CleanupPeerSubsystems(NetPeer peer, int id)
                 BasisNetworkResourceManagement.RemovePeerResources(uuid);
             }
 
-            NetworkServer.AuthIdentity.RemoveConnection(id);
             BasisNetworkOwnership.RemovePlayerOwnership(id);
             BasisSavedState.RemovePlayer(id);
             BasisServerReductionSystemEvents.RemovePlayer(id);

Basis Server/BasisNetworkServer/Security/BasisDIDAuthIdentity.cs

@@ -64,11 +64,16 @@ public static string UnpackString(byte[] compressedBytes)
             return Encoding.UTF8.GetString(compressedBytes, 0, compressedBytes.Length);
         }
 
-        public struct OnAuth
+        public struct OnAuth : IEquatable<OnAuth>
         {
             public ReadyMessage ReadyMessage;
             public Challenge Challenge;
             public Did Did;
+            public NetPeer Peer;
+
+            public bool Equals(OnAuth other) => ReferenceEquals(Peer, other.Peer);
+            public override bool Equals(object obj) => obj is OnAuth other && Equals(other);
+            public override int GetHashCode() => Peer?.GetHashCode() ?? 0;
         }
         public int CheckForDuplicates(Did Did)
         {
@@ -133,6 +138,12 @@ public void ProcessConnection(Configuration Configuration, ConnectionRequest Con
                         }
                         return;
                     }
+                    if (AuthIdentity.TryGetValue(newPeer.Id, out OnAuth Stale) && !ReferenceEquals(Stale.Peer, newPeer))
+                    {
+                        BNL.Log($"Auth slot {newPeer.Id} still held by a stale connection; releasing it for the incoming peer.");
+                        RemoveConnection(newPeer.Id, Stale.Peer);
+                    }
+
                     if (Configuration.HowManyDuplicateAuthCanExist <= CheckForDuplicates(playerDid))
                     {
                         BasisServerHandleEvents.RejectWithReason(newPeer, "To Many Auths From this DID!");
@@ -143,7 +154,8 @@ public void ProcessConnection(Configuration Configuration, ConnectionRequest Con
                     {
                         Did = playerDid,
                         Challenge = MakeChallenge(playerDid),
-                        ReadyMessage = readyMessage
+                        ReadyMessage = readyMessage,
+                        Peer = newPeer
                     };
 
                     if (AuthIdentity.TryAdd(newPeer.Id, OnAuth))
@@ -206,16 +218,12 @@ public async Task TimeOut(NetPeer newPeer, string UUID, CancellationTokenSource
             try
             {
                 await Task.Delay(GetAuthTimeoutMs(NetworkServer.Server?.ConnectedPeersCount ?? 0), cts.Token);
-                if (!_timeouts.ContainsKey(newPeer.Id)) return;
-                if (AuthIdentity.TryRemove(newPeer.Id, out OnAuth TimedOut))
+                if (!RemoveConnection(newPeer.Id, newPeer))
                 {
-                    ReleaseDid(TimedOut.Did);
+                    return;
                 }
-                _timeouts.TryRemove(newPeer.Id, out _);
-                cts.Dispose();
                 BNL.Log($"Authentication timeout for {UUID}.");
                 BasisServerHandleEvents.RejectWithReason(newPeer, "Authentication timeout");
-                newPeer.Disconnect();
             }
             catch (TaskCanceledException) { }
         }
@@ -295,15 +303,37 @@ public async Task<bool> RecvChallengeResponse(Response response, Challenge Chall
 
         public void RemoveConnection(int NetPeer)
         {
-            if (AuthIdentity.TryRemove(NetPeer, out var authIdentity))
+            RemoveConnection(NetPeer, null);
+        }
+
+        public bool RemoveConnection(int Id, NetPeer Expected)
+        {
+            bool Removed;
+            OnAuth Entry;
+            if (Expected == null)
             {
-                ReleaseDid(authIdentity.Did);
+                Removed = AuthIdentity.TryRemove(Id, out Entry);
             }
-            if (_timeouts.TryRemove(NetPeer, out var cts))
+            else
+            {
+                Removed = AuthIdentity.TryGetValue(Id, out Entry)
+                       && ReferenceEquals(Entry.Peer, Expected)
+                       && ((ICollection<KeyValuePair<int, OnAuth>>)AuthIdentity)
+                              .Remove(new KeyValuePair<int, OnAuth>(Id, Entry));
+            }
+
+            if (!Removed)
+            {
+                return false;
+            }
+
+            ReleaseDid(Entry.Did);
+            if (_timeouts.TryRemove(Id, out var cts))
             {
                 try { cts.Cancel(); } catch { }
                 cts.Dispose();
             }
+            return true;
         }
         public bool IsNetPeerAdmin(string UUID)
         {
@@ -391,7 +421,7 @@ static string[] LoadAdmins(string filePath)
 
         public bool NetIDToUUID(NetPeer Peer, out string UUID)
         {
-            if (AuthIdentity.TryGetValue(Peer.Id, out OnAuth OnAuth))
+            if (Peer != null && AuthIdentity.TryGetValue(Peer.Id, out OnAuth OnAuth) && ReferenceEquals(OnAuth.Peer, Peer))
             {
                 UUID = OnAuth.Did.V;
                 return true;

Basis Server/BasisServerTests/AuthIdentityRecycledIdTests.cs

@@ -0,0 +1,122 @@
+using Basis.Contrib.Auth.DecentralizedIds.Newtypes;
+using Basis.Network.Core;
+using BasisDidLink;
+using Xunit;
+using static BasisDidLink.BasisDIDAuthIdentity;
+
+namespace BasisServerTests;
+
+public class AuthIdentityRecycledIdTests
+{
+    private static OnAuth Entry(NetPeer owner, string uuid) => new()
+    {
+        Did = new Did(uuid),
+        Peer = owner,
+    };
+
+    private static BasisDIDAuthIdentity NewIdentity() => new();
+
+    [Fact]
+    public void RemoveConnection_ReleasesTheEntryItsOwnPeerCreated()
+    {
+        BasisDIDAuthIdentity identity = NewIdentity();
+        try
+        {
+            int id = LifecycleSupport.NextPeerId();
+            FakeNetPeer owner = LifecycleSupport.Peer(id);
+            identity.AuthIdentity[id] = Entry(owner, LifecycleSupport.NewUuid());
+
+            Assert.True(identity.RemoveConnection(id, owner));
+            Assert.False(identity.AuthIdentity.ContainsKey(id));
+            Assert.False(identity.RemoveConnection(id, owner));
+        }
+        finally
+        {
+            identity.DeInitialize();
+        }
+    }
+
+    [Fact]
+    public void RemoveConnection_LeavesAnEntryThatBelongsToAnotherConnection()
+    {
+        BasisDIDAuthIdentity identity = NewIdentity();
+        try
+        {
+            int id = LifecycleSupport.NextPeerId();
+            FakeNetPeer stale = LifecycleSupport.Peer(id);
+            FakeNetPeer live = LifecycleSupport.Peer(id);
+            identity.AuthIdentity[id] = Entry(live, LifecycleSupport.NewUuid());
+
+            Assert.False(identity.RemoveConnection(id, stale));
+            Assert.True(identity.AuthIdentity.ContainsKey(id));
+            Assert.Same(live, identity.AuthIdentity[id].Peer);
+        }
+        finally
+        {
+            identity.DeInitialize();
+        }
+    }
+
+    [Fact]
+    public void RemoveConnection_WithoutAPeer_RemovesWhicheverEntryHoldsTheId()
+    {
+        BasisDIDAuthIdentity identity = NewIdentity();
+        try
+        {
+            int id = LifecycleSupport.NextPeerId();
+            identity.AuthIdentity[id] = Entry(LifecycleSupport.Peer(id), LifecycleSupport.NewUuid());
+
+            identity.RemoveConnection(id);
+            Assert.False(identity.AuthIdentity.ContainsKey(id));
+        }
+        finally
+        {
+            identity.DeInitialize();
+        }
+    }
+
+    [Fact]
+    public void NetIDToUUID_AnswersOnlyForTheConnectionThatOwnsTheEntry()
+    {
+        BasisDIDAuthIdentity identity = NewIdentity();
+        try
+        {
+            int id = LifecycleSupport.NextPeerId();
+            FakeNetPeer owner = LifecycleSupport.Peer(id);
+            FakeNetPeer recycled = LifecycleSupport.Peer(id);
+            string uuid = LifecycleSupport.NewUuid();
+            identity.AuthIdentity[id] = Entry(owner, uuid);
+
+            Assert.True(identity.NetIDToUUID(owner, out string found));
+            Assert.Equal(uuid, found);
+
+            Assert.False(identity.NetIDToUUID(recycled, out string leaked));
+            Assert.Equal(string.Empty, leaked);
+        }
+        finally
+        {
+            identity.DeInitialize();
+        }
+    }
+
+    [Fact]
+    public void AStaleTimeout_CannotEvictTheConnectionThatInheritedTheId()
+    {
+        BasisDIDAuthIdentity identity = NewIdentity();
+        try
+        {
+            int id = LifecycleSupport.NextPeerId();
+            FakeNetPeer timedOut = LifecycleSupport.Peer(id);
+            FakeNetPeer inherited = LifecycleSupport.Peer(id);
+            identity.AuthIdentity[id] = Entry(inherited, LifecycleSupport.NewUuid());
+
+            Assert.False(identity.RemoveConnection(id, timedOut));
+            Assert.True(identity.AuthIdentity.ContainsKey(id));
+            Assert.Same(inherited, identity.AuthIdentity[id].Peer);
+        }
+        finally
+        {
+            identity.DeInitialize();
+        }
+    }
+}

Basis Server/BasisServerTests/BasisConnectionLifecycleTests.cs

@@ -701,4 +701,49 @@ public void StaleDisconnectAfterReconnectCollision_DoesNotEvictTheLivePeer()
             "the live peer was evicted by a stale peer's disconnect (key-only TryRemove)");
         Assert.Same(live, stored);
     }
+
+    [Fact]
+    public void StaleDisconnectAfterReconnectCollision_StillReleasesItsOwnAuthState()
+    {
+        using var scope = new ServerStaticsScope();
+        InstallServer();
+        MapAuthIdentity identity = (MapAuthIdentity)NetworkServer.AuthIdentity;
+
+        int id = LifecycleSupport.NextPeerId();
+        FakeNetPeer stale = LifecycleSupport.Peer(id);
+        FakeNetPeer live = LifecycleSupport.Peer(id);
+
+        identity.Register(LifecycleSupport.NewUuid(), id, stale);
+
+        NetworkServer.AuthenticatedPeers[id] = live;
+        NetworkServer.RebuildPeerSnapshot();
+
+        BasisServerHandleEvents.HandlePeerDisconnected(stale, Info());
+
+        Assert.Contains(id, identity.Released);
+        Assert.True(NetworkServer.AuthenticatedPeers.TryGetValue(id, out NetPeer stored));
+        Assert.Same(live, stored);
+    }
+
+    [Fact]
+    public void StaleDisconnect_DoesNotReleaseTheLivePeersAuthState()
+    {
+        using var scope = new ServerStaticsScope();
+        InstallServer();
+        MapAuthIdentity identity = (MapAuthIdentity)NetworkServer.AuthIdentity;
+
+        int id = LifecycleSupport.NextPeerId();
+        FakeNetPeer stale = LifecycleSupport.Peer(id);
+        FakeNetPeer live = LifecycleSupport.Peer(id);
+
+        identity.Register(LifecycleSupport.NewUuid(), id, live);
+        NetworkServer.AuthenticatedPeers[id] = live;
+        NetworkServer.RebuildPeerSnapshot();
+
+        BasisServerHandleEvents.HandlePeerDisconnected(stale, Info());
+
+        Assert.DoesNotContain(id, identity.Released);
+        Assert.True(identity.NetIDToUUID(live, out string uuid) && !string.IsNullOrEmpty(uuid),
+            "the live peer lost its identity to a stale peer's disconnect");
+    }
 }

Basis Server/BasisServerTests/BasisServerTests.csproj

@@ -16,6 +16,7 @@
   <ItemGroup>
     <ProjectReference Include="..\BasisNetworkCore\BasisNetworkCore.csproj" />
     <ProjectReference Include="..\BasisNetworkServer\BasisNetworkServer.csproj" />
+    <ProjectReference Include="..\Contrib\Auth\Did\Did.csproj" />
     <ProjectReference Include="..\Contrib\Crypto\Crypto.csproj" />
     <ProjectReference Include="..\LiteNetLib\LiteNetLib.csproj" />
   </ItemGroup>

Basis Server/BasisServerTests/ModerationAndPermissionManagerTests.cs

@@ -66,13 +66,22 @@ internal sealed class MapAuthIdentity : IAuthIdentity
 {
     private readonly ConcurrentDictionary<string, int> _uuidToId = new(StringComparer.OrdinalIgnoreCase);
     private readonly ConcurrentDictionary<int, string> _idToUuid = new();
+    private readonly ConcurrentDictionary<int, NetPeer> _owner = new();
+
+    public readonly List<int> Released = new();
 
     public void Register(string uuid, int netId)
     {
         _uuidToId[uuid] = netId;
         _idToUuid[netId] = uuid;
     }
 
+    public void Register(string uuid, int netId, NetPeer owner)
+    {
+        Register(uuid, netId);
+        _owner[netId] = owner;
+    }
+
     public void ProcessConnection(Configuration Configuration, ConnectionRequest ConnectionRequest, NetPeer NetPeer)
     {
     }
@@ -81,7 +90,22 @@ public void DeInitialize()
     {
     }
 
-    public void RemoveConnection(int NetPeer) => _idToUuid.TryRemove(NetPeer, out _);
+    public void RemoveConnection(int NetPeer) => RemoveConnection(NetPeer, null);
+
+    public bool RemoveConnection(int Id, NetPeer Expected)
+    {
+        if (Expected != null && _owner.TryGetValue(Id, out NetPeer? owner) && !ReferenceEquals(owner, Expected))
+        {
+            return false;
+        }
+        if (!_idToUuid.TryRemove(Id, out _))
+        {
+            return false;
+        }
+        _owner.TryRemove(Id, out _);
+        lock (Released) { Released.Add(Id); }
+        return true;
+    }
 
     public bool NetIDToUUID(NetPeer Peer, out string UUID)
     {

Basis Server/BasisServerTests/SecurityListAndLockManagerTests.cs

@@ -349,6 +349,7 @@ private sealed class FixedUuidAuthIdentity : IAuthIdentity
         public void ProcessConnection(Configuration configuration, ConnectionRequest connectionRequest, NetPeer netPeer) { }
         public void DeInitialize() { }
         public void RemoveConnection(int netPeer) { }
+        public bool RemoveConnection(int netPeer, NetPeer expected) => false;
         public bool NetIDToUUID(NetPeer peer, out string uuid)
         {
             uuid = _uuid;