fix(guild): check the password on kick when no security code is set

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<IGuildKickResultPlugIn>(p => p.GuildKickResultAsync(GuildKickSuccess.Failed)).ConfigureAwait(false);
             return;

tests/MUnique.OpenMU.Tests/GuildActionTest.cs

@@ -622,6 +622,68 @@ public async ValueTask GuildRoleAssignHandlerSpacePaddedNameAsync()
         Assert.That(this._player.GuildStatus!.Position, Is.EqualTo(GuildPosition.AssistantMaster));
     }
 
+    /// <summary>
+    /// Tests that the guild master can kick a member by entering the account password,
+    /// when the account has no security code.
+    /// </summary>
+    [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));
+    }
+
+    /// <summary>
+    /// Tests that a kick with a wrong password is rejected, when the account has no security code.
+    /// </summary>
+    [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));
+    }
+
+    /// <summary>
+    /// Tests that the security code is required instead of the password, when the account has one.
+    /// </summary>
+    [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();