Ability for users to use previous usernames

Refresh.Database/GameDatabaseContext.Registration.cs

@@ -96,20 +96,18 @@ public bool IsEmailQueued(string emailAddress)
         return this.QueuedRegistrations.Any(r => r.EmailAddress == emailAddress);
     }
 
-    public bool IsUsernameTaken(string username, GameUser? userToName = null)
+    public bool IsUsernameTaken(string username)
     {
         if (this.GameUsers.Any(u => u.Username == username)) return true;
         if (this.QueuedRegistrations.Any(r => r.Username == username)) return true;
-        if (this.IsUserDisallowed(username)) return true;
-
-        PreviousUsername? previous = this.PreviousUsernames.FirstOrDefault(p => p.Username == username);
-        // no one has ever had this name before
-        if (previous == null) return false;
-        // this is not the initial owner of the name (only previous owners may be renamed back)
-        if (userToName == null || userToName.UserId != previous.UserId) return true;
 
         return false;
     }
+
+    public bool WasUsernamePreviouslyTaken(string username)
+    {
+        return this.PreviousUsernames.Any(u => u.Username == username);
+    }
 
     public bool IsEmailTaken(string emailAddress)
     {

Refresh.Database/GameDatabaseContext.Users.cs

@@ -370,7 +370,7 @@ public void RenameUser(GameUser user, string newUsername, bool force = false)
                 throw new ArgumentException("Username is invalid!", nameof(newUsername));
             }
 
-            if (this.IsUsernameTaken(newUsername, user))
+            if (this.IsUsernameTaken(newUsername))
             {
                 throw new ArgumentException("Username is already taken!", nameof(newUsername));
             }

Refresh.Interfaces.APIv3/Endpoints/Admin/AdminUserApiEndpoints.cs

@@ -173,7 +173,7 @@ public ApiResponse<ApiExtendedGameUserResponse> UpdateUser(RequestContext contex
                 return new ApiValidationError(ApiValidationError.InvalidUsernameErrorWhen
                     + " Are you sure you used a PSN/RPCN username, or prepended it with ! if it's a fake user?");
 
-            if (database.IsUsernameTaken(body.Username, targetUser))
+            if (database.IsUsernameTaken(body.Username))
                 return ApiValidationError.UsernameTakenError;
 
             database.RenameUser(targetUser, body.Username);

RefreshTests.GameServer/Tests/ApiV3/AdminUserEditApiTests.cs

@@ -269,13 +269,17 @@ public void CannotRenameToTakenUsername()
     }
 
     [Test]
-    public void CannotRenameToOtherUsersPreviousName()
+    public void CanRenameToOtherUsersPreviousName()
     {
         using TestContext context = this.GetServer();
 
         GameUser mod = context.CreateUser(null, GameUserRole.Moderator);
         GameUser owner = context.CreateUser("original", GameUserRole.User);
         GameUser target = context.CreateUser("stinker", GameUserRole.User);
+
+        // Ensure we're tracking neither usernames
+        Assert.That(!context.Database.WasUsernamePreviouslyTaken("original"));
+        Assert.That(!context.Database.WasUsernamePreviouslyTaken("stinker"));
 
         context.Database.RenameUser(owner, "original_2");
         GameUser? modifiedOwner = context.Database.GetUserByObjectId(owner.UserId);
@@ -288,15 +292,20 @@ public void CannotRenameToOtherUsersPreviousName()
             Username = "original"
         };
 
-        ApiResponse<ApiExtendedGameUserResponse>? response = client.PatchData<ApiExtendedGameUserResponse>($"/api/v3/admin/users/uuid/{target.UserId}", request, false, true);
-        Assert.That(response?.Error, Is.Not.Null);
-        Assert.That(response!.Error!.StatusCode, Is.EqualTo(BadRequest));
+        ApiResponse<ApiExtendedGameUserResponse>? response = client.PatchData<ApiExtendedGameUserResponse>($"/api/v3/admin/users/uuid/{target.UserId}", request, true, false);
+        Assert.That(response?.Data, Is.Not.Null);
+        Assert.That(response!.Data!.Username, Is.EqualTo("original"));
+        Assert.That(response!.Data!.UserId, Is.EqualTo(target.UserId.ToString()));
 
         context.Database.Refresh();
 
         GameUser? modifiedTarget = context.Database.GetUserByObjectId(target.UserId);
         Assert.That(modifiedTarget, Is.Not.Null);
-        Assert.That(modifiedTarget!.Username, Is.EqualTo("stinker"));
+        Assert.That(modifiedTarget!.Username, Is.EqualTo("original"));
+
+        // Ensure we're tracking both usernames
+        Assert.That(context.Database.WasUsernamePreviouslyTaken("original"));
+        Assert.That(context.Database.WasUsernamePreviouslyTaken("stinker"));
     }
 
     [Test]

RefreshTests.GameServer/Tests/ApiV3/UserApiTests.cs

@@ -47,7 +47,7 @@ public void RegisterAccount()
     }
 
     [Test]
-    public void CannotRegisterAccountWithPreviouslyTakenUsername()
+    public void CanRegisterAccountWithPreviouslyTakenUsername()
     {
         using TestContext context = this.GetServer();
         GameUser owner = context.CreateUser("original", GameUserRole.User);
@@ -62,13 +62,46 @@ public void CannotRegisterAccountWithPreviouslyTakenUsername()
             Username = "original",
             EmailAddress = "guy@lil.com",
             PasswordSha512 = "ee26b0dd4af7e749aa1a8ee3c10ae9923f618980772e473f8819a5d4940e0db27ac185f8a0e1d5f84f88bc887fd67b143732c304cc5fa9ad8e6f57f50028a8ff",
-        }, false, true);
+        }, true, false);
+        Assert.That(response, Is.Not.Null);
+        Assert.That(response!.Data, Is.Not.Null);
+
+        context.Database.Refresh();
+        GameUser? newUser = context.Database.GetUserByUuid(response!.Data!.UserId);
+        Assert.That(newUser, Is.Not.Null);
+        Assert.That(newUser!.Username, Is.EqualTo("original"));
+
+        // Ensure the original "original" usage is tracked
+        Assert.That(context.Database.WasUsernamePreviouslyTaken("original"));
+    }
+
+    [Test]
+    public void CannotRegisterAccountWithCurrentlyTakenUsername()
+    {
+        using TestContext context = this.GetServer();
+        GameUser owner = context.CreateUser("original", GameUserRole.User);
+
+        ApiRegisterRequest request = new ApiRegisterRequest
+        {
+            Username = "original",
+            EmailAddress = "guy@lil.com",
+            PasswordSha512 = "ee26b0dd4af7e749aa1a8ee3c10ae9923f618980772e473f8819a5d4940e0db27ac185f8a0e1d5f84f88bc887fd67b143732c304cc5fa9ad8e6f57f50028a8ff",
+        };
+        ApiResponse<ApiAuthenticationResponse>? response = context.Http.PostData<ApiAuthenticationResponse>("/api/v3/register", request, false, true);
         Assert.That(response, Is.Not.Null);
-        Assert.That(response!.Error, Is.Not.EqualTo(null));
-        Assert.That(response.Error!.Name, Is.EqualTo("ApiAuthenticationError"));
+        Assert.That(response!.Error, Is.Not.Null);
 
         context.Database.Refresh();
-        Assert.That(context.Database.GetTotalUserCount(), Is.EqualTo(1));
+
+        // Our initial user still exists and hasn't been touched
+        GameUser? newUser = context.Database.GetUserByObjectId(owner.UserId);
+        Assert.That(newUser, Is.Not.Null);
+        Assert.That(newUser!.Username, Is.EqualTo("original"));
+        Assert.That(newUser!.EmailAddress, Is.Not.EqualTo(request.EmailAddress));
+        Assert.That(newUser!.PasswordBcrypt, Is.Not.EqualTo(request.PasswordSha512));
+
+        // Ensure there is nothing tracked as no rename has happened
+        Assert.That(!context.Database.WasUsernamePreviouslyTaken("original"));
     }
 
     [TestCase("4")]