diff --git a/src/GameLogic/PlayerActions/Guild/GuildKickPlayerAction.cs b/src/GameLogic/PlayerActions/Guild/GuildKickPlayerAction.cs index dcbea66226..6ba4a0db17 100644 --- a/src/GameLogic/PlayerActions/Guild/GuildKickPlayerAction.cs +++ b/src/GameLogic/PlayerActions/Guild/GuildKickPlayerAction.cs @@ -41,10 +41,20 @@ public async ValueTask KickPlayerAsync(Player player, string nickname, string se return; } - if (player.Account!.SecurityCode != null && player.Account.SecurityCode != securityCode) + // Same fallback as DeleteCharacterAction: an account with no security code ever set (the + // default, "" - there is no in-game or web flow to configure one) is checked against the + // account password instead. Without this fallback, "" != null is always true, so the check + // below would always run against an empty string that no legitimate input can match, and a + // guild could never be kicked from - or disbanded, since a self-kick by the guild master is + // how disbanding works below. + var checkAsPassword = string.IsNullOrEmpty(player.Account!.SecurityCode); + var securityCodeIsWrong = checkAsPassword + ? !BCrypt.Net.BCrypt.Verify(securityCode, player.Account.PasswordHash) + : player.Account.SecurityCode != securityCode; + if (securityCodeIsWrong) { await player.ShowLocalizedBlueMessageAsync(nameof(PlayerMessage.WrongSecurityCode)).ConfigureAwait(false); - player.Logger.LogDebug("Wrong Security Code: [{0}] <> [{1}], Player: {2}", securityCode, player.Account.SecurityCode, player.SelectedCharacter?.Name); + player.Logger.LogDebug("Wrong Security Code, Player: {0}", player.SelectedCharacter?.Name); await player.InvokeViewPlugInAsync(p => p.GuildKickResultAsync(GuildKickSuccess.Failed)).ConfigureAwait(false); return; diff --git a/tests/MUnique.OpenMU.Tests/GuildActionTest.cs b/tests/MUnique.OpenMU.Tests/GuildActionTest.cs index 067e9d5955..0557b7efbb 100644 --- a/tests/MUnique.OpenMU.Tests/GuildActionTest.cs +++ b/tests/MUnique.OpenMU.Tests/GuildActionTest.cs @@ -622,6 +622,68 @@ public async ValueTask GuildRoleAssignHandlerSpacePaddedNameAsync() Assert.That(this._player.GuildStatus!.Position, Is.EqualTo(GuildPosition.AssistantMaster)); } + /// + /// Tests that the guild master can kick a member by entering the account password, + /// when the account has no security code. + /// + [Test] + public async ValueTask GuildKickWithPasswordWhenNoSecurityCodeIsSetAsync() + { + await this.PrepareRoleAssignScenarioAsync().ConfigureAwait(false); + this.SetupGuildMasterAccount(securityCode: string.Empty, password: "password"); + var guildId = this._player.GuildStatus!.GuildId; + + var action = new GuildKickPlayerAction(); + await action.KickPlayerAsync(this._guildMasterPlayer, this._player.SelectedCharacter!.Name, "password").ConfigureAwait(false); + + var members = await this.GuildServer.GetGuildListAsync(guildId).ConfigureAwait(false); + Assert.That(members.Select(m => m.PlayerName), Does.Not.Contain(this._player.SelectedCharacter!.Name)); + } + + /// + /// Tests that a kick with a wrong password is rejected, when the account has no security code. + /// + [Test] + public async ValueTask GuildKickWithWrongPasswordWhenNoSecurityCodeIsSetAsync() + { + await this.PrepareRoleAssignScenarioAsync().ConfigureAwait(false); + this.SetupGuildMasterAccount(securityCode: string.Empty, password: "password"); + var guildId = this._player.GuildStatus!.GuildId; + + var action = new GuildKickPlayerAction(); + await action.KickPlayerAsync(this._guildMasterPlayer, this._player.SelectedCharacter!.Name, "wrong").ConfigureAwait(false); + + var members = await this.GuildServer.GetGuildListAsync(guildId).ConfigureAwait(false); + Assert.That(members.Select(m => m.PlayerName), Does.Contain(this._player.SelectedCharacter!.Name)); + } + + /// + /// Tests that the security code is required instead of the password, when the account has one. + /// + [Test] + public async ValueTask GuildKickRequiresSecurityCodeWhenSetAsync() + { + await this.PrepareRoleAssignScenarioAsync().ConfigureAwait(false); + this.SetupGuildMasterAccount(securityCode: "1234567", password: "password"); + var guildId = this._player.GuildStatus!.GuildId; + var action = new GuildKickPlayerAction(); + + await action.KickPlayerAsync(this._guildMasterPlayer, this._player.SelectedCharacter!.Name, "password").ConfigureAwait(false); + var membersAfterPassword = await this.GuildServer.GetGuildListAsync(guildId).ConfigureAwait(false); + Assert.That(membersAfterPassword.Select(m => m.PlayerName), Does.Contain(this._player.SelectedCharacter!.Name)); + + await action.KickPlayerAsync(this._guildMasterPlayer, this._player.SelectedCharacter!.Name, "1234567").ConfigureAwait(false); + var membersAfterSecurityCode = await this.GuildServer.GetGuildListAsync(guildId).ConfigureAwait(false); + Assert.That(membersAfterSecurityCode.Select(m => m.PlayerName), Does.Not.Contain(this._player.SelectedCharacter!.Name)); + } + + private void SetupGuildMasterAccount(string securityCode, string password) + { + var account = this._guildMasterPlayer.Account!; + account.SecurityCode = securityCode; + account.PasswordHash = BCrypt.Net.BCrypt.HashPassword(password); + } + private IGameServerContext CreateGameServer() { var gameConfiguration = new GameConfiguration();