diff --git a/Refresh.Database/GameDatabaseContext.Registration.cs b/Refresh.Database/GameDatabaseContext.Registration.cs index 58e2a5318..0df6fd181 100644 --- a/Refresh.Database/GameDatabaseContext.Registration.cs +++ b/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) { diff --git a/Refresh.Database/GameDatabaseContext.Users.cs b/Refresh.Database/GameDatabaseContext.Users.cs index 40832aa5a..ed9be74b7 100644 --- a/Refresh.Database/GameDatabaseContext.Users.cs +++ b/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)); } diff --git a/Refresh.Interfaces.APIv3/Endpoints/Admin/AdminUserApiEndpoints.cs b/Refresh.Interfaces.APIv3/Endpoints/Admin/AdminUserApiEndpoints.cs index 444702150..e5f92e636 100644 --- a/Refresh.Interfaces.APIv3/Endpoints/Admin/AdminUserApiEndpoints.cs +++ b/Refresh.Interfaces.APIv3/Endpoints/Admin/AdminUserApiEndpoints.cs @@ -173,7 +173,7 @@ public ApiResponse 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); diff --git a/RefreshTests.GameServer/Tests/ApiV3/AdminUserEditApiTests.cs b/RefreshTests.GameServer/Tests/ApiV3/AdminUserEditApiTests.cs index da984a89e..cf9dbbe40 100644 --- a/RefreshTests.GameServer/Tests/ApiV3/AdminUserEditApiTests.cs +++ b/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? response = client.PatchData($"/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? response = client.PatchData($"/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] diff --git a/RefreshTests.GameServer/Tests/ApiV3/UserApiTests.cs b/RefreshTests.GameServer/Tests/ApiV3/UserApiTests.cs index 409ce93d2..5bd0be883 100644 --- a/RefreshTests.GameServer/Tests/ApiV3/UserApiTests.cs +++ b/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? response = context.Http.PostData("/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")]