diff --git a/cockatrice/src/interface/widgets/server/game_selector.cpp b/cockatrice/src/interface/widgets/server/game_selector.cpp index a8bf54e91..659325987 100644 --- a/cockatrice/src/interface/widgets/server/game_selector.cpp +++ b/cockatrice/src/interface/widgets/server/game_selector.cpp @@ -369,7 +369,7 @@ void GameSelector::joinGame(const ServerInfo_Game &game, const bool asSpectator, return; } - bool overrideRestrictions = !tabSupervisor->getAdminLocked(); + bool overrideRestrictions = tabSupervisor->canOverrideGameRestrictions(); // Joining a full game without override privileges silently becomes a // spectator join, so ask first instead of surprising the player. @@ -462,7 +462,7 @@ void GameSelector::enableButtonsForIndex(const QModelIndex ¤t) } const ServerInfo_Game &game = gameListModel->getGame(current.data(Qt::UserRole).toInt()); - bool overrideRestrictions = !tabSupervisor->getAdminLocked(); + bool overrideRestrictions = tabSupervisor->canOverrideGameRestrictions(); spectateButton->setEnabled(game.spectators_allowed() || overrideRestrictions); joinButton->setEnabled(game.player_count() < game.max_players() || overrideRestrictions); diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp index 462aa420b..ccb687ff3 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp @@ -1451,6 +1451,11 @@ bool TabSupervisor::getAdminLocked() const return tabAdmin->getLocked(); } +bool TabSupervisor::canOverrideGameRestrictions() const +{ + return !getAdminLocked() || (userInfo->user_level() & ServerInfo_User::IsJudge); +} + void TabSupervisor::processNotifyUserEvent(const Event_NotifyUser &event) { diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.h b/cockatrice/src/interface/widgets/tabs/tab_supervisor.h index aec1d7418..adde7f971 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.h +++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.h @@ -171,6 +171,7 @@ public: [[nodiscard]] QList getGameInviteLinksForRoom(int roomId) const; void sendInviteToUser(const QString &userName, const QString &inviteText); [[nodiscard]] bool getAdminLocked() const; + [[nodiscard]] bool canOverrideGameRestrictions() const; void closeEvent(QCloseEvent *event) override; bool switchToGameTabIfAlreadyExists(const int gameId); static void actShowPopup(const QString &message); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp index 799b1e7ee..131ff1077 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp @@ -465,7 +465,7 @@ Response::ResponseCode Server_Game::checkJoin(ServerInfo_User *user, if (asJudge && !(user->user_level() & ServerInfo_User::IsJudge)) { return Response::RespUserLevelTooLow; } - if (!(overrideRestrictions && (user->user_level() & ServerInfo_User::IsModerator))) { + if (!(overrideRestrictions && (user->user_level() & (ServerInfo_User::IsModerator | ServerInfo_User::IsJudge)))) { if ((_password != password) && !(spectator && !spectatorsNeedPassword)) { return Response::RespWrongPassword; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index dfbdabb5e..7bb834d7e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -12,6 +12,7 @@ add_test(NAME server_card_counter_test COMMAND server_card_counter_test) add_test(NAME server_counter_test COMMAND server_counter_test) add_test(NAME server_rate_limiter_test COMMAND server_rate_limiter_test) add_test(NAME server_developer_role_test COMMAND server_developer_role_test) +add_test(NAME server_game_join_test COMMAND server_game_join_test) add_test(NAME warning_categories_test COMMAND warning_categories_test) add_test(NAME lag_monitor_test COMMAND lag_monitor_test) add_test(NAME latency_tracker_test COMMAND latency_tracker_test) @@ -34,6 +35,7 @@ add_executable(server_card_counter_test server_card_counter_test.cpp) add_executable(server_counter_test server_counter_test.cpp) add_executable(server_rate_limiter_test server_rate_limiter_test.cpp) add_executable(server_developer_role_test server_developer_role_test.cpp) +add_executable(server_game_join_test server_game_join_test.cpp) add_executable(warning_categories_test warning_categories_test.cpp) add_executable(lag_monitor_test ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_monitor.cpp lag_monitor_test.cpp) target_include_directories(lag_monitor_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src) @@ -87,6 +89,7 @@ if(NOT GTEST_FOUND) add_dependencies(server_counter_test gtest) add_dependencies(server_rate_limiter_test gtest) add_dependencies(server_developer_role_test gtest) + add_dependencies(server_game_join_test gtest) add_dependencies(warning_categories_test gtest) add_dependencies(lag_monitor_test gtest) add_dependencies(latency_tracker_test gtest) @@ -127,6 +130,10 @@ target_link_libraries( server_developer_role_test libcockatrice_network libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) +target_link_libraries( + server_game_join_test libcockatrice_network_server_remote libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) target_link_libraries( warning_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) diff --git a/tests/server_game_join_test.cpp b/tests/server_game_join_test.cpp new file mode 100644 index 000000000..c84e4e066 --- /dev/null +++ b/tests/server_game_join_test.cpp @@ -0,0 +1,174 @@ +/** @file server_game_join_test.cpp + * @brief Tests for the moderator/judge game-entry restriction override in Server_Game::checkJoin. + * @ingroup Tests + */ + +#include "game/server_game.h" +#include "server.h" +#include "server_database_interface.h" +#include "server_room.h" + +#include +#include +#include + +RNG_Abstract *rng = nullptr; // referenced by the server_remote library + +namespace +{ + +class MockDatabaseInterface : public Server_DatabaseInterface +{ +public: + AuthenticationResult checkUserPassword(Server_ProtocolHandler *, + const QString &, + const QString &, + const QString &, + QString &, + int &, + bool) override + { + return NotLoggedIn; + } + int getNextReplayId() override + { + return 1; + } + int getNextGameId() override + { + return 1; + } + int getActiveUserCount(QString) override + { + return 0; + } + ServerInfo_User getUserData(const QString &, bool) override + { + return ServerInfo_User(); + } +}; + +class FakeServer : public Server +{ +public: + FakeServer() + { + setDatabaseInterface(new MockDatabaseInterface()); + } +}; + +class GameJoinOverrideTest : public ::testing::Test +{ +protected: + FakeServer server; + Server_Room room{0, 0, "", "", "", "", false, "", {}, &server}; + ServerInfo_User creator; + ServerInfo_User plainUser; + ServerInfo_User unregisteredJudge; + ServerInfo_User moderator; + ServerInfo_User judge; + Server_Game *game = nullptr; + + void SetUp() override + { + creator.set_name("creator"); + creator.set_user_level(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered); + plainUser.set_name("plain-user"); + plainUser.set_user_level(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered); + unregisteredJudge.set_name("unregistered-judge"); + unregisteredJudge.set_user_level(ServerInfo_User::IsUser | ServerInfo_User::IsJudge); + moderator.set_name("moderator"); + moderator.set_user_level(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered | + ServerInfo_User::IsModerator); + judge.set_name("judge"); + judge.set_user_level(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered | ServerInfo_User::IsJudge); + } + + void TearDown() override + { + delete game; + } + + Server_Game *makeGame(bool passwordProtected, bool onlyRegistered, bool onlyBuddies, bool spectatorsAllowed) + { + GameConfig config{.creatorInfo = creator, + .gameId = 1, + .description = QString(), + .password = passwordProtected ? "secret" : QString(), + .maxPlayers = 2, + .gameTypes = QList(), + .onlyBuddies = onlyBuddies, + .onlyRegistered = onlyRegistered, + .spectatorsAllowed = spectatorsAllowed, + .spectatorsNeedPassword = true, + .spectatorsCanTalk = false, + .spectatorsSeeEverything = false, + .startingLifeTotal = 20, + .shareDecklistsOnLoad = false}; + return new Server_Game(config, &room); + } +}; + +TEST_F(GameJoinOverrideTest, StaffBypassPasswordRestriction) +{ + game = makeGame(true, false, false, true); + + // A plain user cannot override the password even with the override flag set. + EXPECT_EQ(game->checkJoin(&plainUser, "wrong", false, true, false), Response::RespWrongPassword); + // Moderators and judges may enter any game regardless of the password. + EXPECT_EQ(game->checkJoin(&moderator, "wrong", false, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, "wrong", false, true, false), Response::RespOk); + // Without the override flag judges are still subject to the password. + EXPECT_EQ(game->checkJoin(&judge, "wrong", false, false, true), Response::RespWrongPassword); + EXPECT_EQ(game->checkJoin(&judge, "secret", false, false, true), Response::RespOk); +} + +TEST_F(GameJoinOverrideTest, StaffBypassRegisteredOnlyRestriction) +{ + game = makeGame(false, true, false, true); + + // Without the override flag the only-registered restriction still applies. + EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), false, false, false), Response::RespUserLevelTooLow); + // An unregistered judge may enter when overriding restrictions. + EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), false, true, false), Response::RespOk); +} + +TEST_F(GameJoinOverrideTest, StaffBypassBuddiesOnlyRestriction) +{ + game = makeGame(false, false, true, true); + + // A plain user who is not on the creator's buddy list gets rejected. + EXPECT_EQ(game->checkJoin(&plainUser, QString(), false, true, false), Response::RespOnlyBuddies); + // Moderators and judges bypass the buddies-only restriction. + EXPECT_EQ(game->checkJoin(&moderator, QString(), false, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, QString(), false, true, false), Response::RespOk); +} + +TEST_F(GameJoinOverrideTest, StaffBypassSpectatorsNotAllowedRestriction) +{ + game = makeGame(false, false, false, false); + + // A plain user cannot spectate when the game disallows spectators. + EXPECT_EQ(game->checkJoin(&plainUser, QString(), true, false, false), Response::RespSpectatorsNotAllowed); + // Moderators and judges may spectate any game regardless of the password + // and the spectator restriction. + EXPECT_EQ(game->checkJoin(&moderator, "wrong", true, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, "wrong", true, true, false), Response::RespOk); +} + +TEST_F(GameJoinOverrideTest, JudgeOverrideDoesNotGrantJudgeJoinToPlainUser) +{ + game = makeGame(false, false, false, true); + + // joining with join_as_judge still requires the judge flag even when overriding. + EXPECT_EQ(game->checkJoin(&plainUser, QString(), false, true, true), Response::RespUserLevelTooLow); + EXPECT_EQ(game->checkJoin(&judge, QString(), false, true, true), Response::RespOk); +} + +} // namespace + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file