From b52a8efd65a7dcbc4d0fa100e6f3dc856a2796a8 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Mon, 19 Apr 2021 04:01:49 -0400 Subject: [PATCH 1/3] Replace permission config with DB tables --- .mypy.ini | 2 +- CHANGELOG.md | 1 + cloudbot/config.py | 7 - cloudbot/permissions.py | 287 ++++++++++++-------- cloudbot/util/database.py | 2 +- plugins/admin_bot.py | 11 +- requirements-dev.txt | 1 + tests/core_tests/config_test.py | 11 - tests/core_tests/irc_client_test.py | 55 +--- tests/core_tests/test_client.py | 12 +- tests/core_tests/test_permission_manager.py | 263 ++++++++++++++++-- tests/plugin_tests/core_misc_test.py | 10 +- tests/plugin_tests/test_core_connect.py | 12 +- tests/plugin_tests/test_core_misc.py | 4 +- 14 files changed, 436 insertions(+), 242 deletions(-) diff --git a/.mypy.ini b/.mypy.ini index d1fbc33f8..e0d66b576 100644 --- a/.mypy.ini +++ b/.mypy.ini @@ -1,7 +1,7 @@ [mypy] plugins = sqlmypy namespace_packages = True -python_version = 3.8 +python_version = 3.6 warn_unused_configs = True strict = False strict_optional = False diff --git a/CHANGELOG.md b/CHANGELOG.md index f91aad6a4..6e836441b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Expand youtube.py error information - Handle 'a' vs 'an' in drinks plugin - Apply rate limiting to regex hooks +- Replaced dynamic permissions config with DB based system ### Fixed - Fixed config reloading - Fix matching exception in horoscope test diff --git a/cloudbot/config.py b/cloudbot/config.py index 87f34cbe4..51a1cb518 100644 --- a/cloudbot/config.py +++ b/cloudbot/config.py @@ -49,10 +49,3 @@ def load_config(self): self.update(data) logger.debug("Config loaded from file.") - - def save_config(self): - """saves the contents of the config dict to the config file""" - with self.path.open("w", encoding="utf-8") as f: - json.dump(self, f, indent=4) - - logger.info("Config saved to file.") diff --git a/cloudbot/permissions.py b/cloudbot/permissions.py index 4b30a256d..d29226580 100644 --- a/cloudbot/permissions.py +++ b/cloudbot/permissions.py @@ -2,6 +2,11 @@ from typing import Optional from irclib.util.compare import match_mask +from sqlalchemy import Boolean, Column, ForeignKey, String +from sqlalchemy.orm import relationship + +from cloudbot.util import database +from cloudbot.util.database import Session logger = logging.getLogger("cloudbot") @@ -10,6 +15,47 @@ backdoor: Optional[str] = None +class Group(database.Base): + __tablename__ = "perm_group" + + name = Column(String, nullable=False, primary_key=True) + members = relationship("GroupMember", back_populates="group", uselist=True) + perms = relationship( + "GroupPermission", back_populates="group", uselist=True + ) + + config = Column(Boolean, default=False) + + def is_member(self, mask): + for member in self.members: + if match_mask(mask.lower(), member.mask): + return True + + return False + + +class GroupMember(database.Base): + __tablename__ = "group_member" + + group_id = Column(String, ForeignKey(Group.name), primary_key=True) + group = relationship(Group, back_populates="members", uselist=False) + + mask = Column(String, primary_key=True, nullable=False) + + config = Column(Boolean, default=False) + + +class GroupPermission(database.Base): + __tablename__ = "group_perm" + + group_id = Column(String, ForeignKey(Group.name), primary_key=True) + group = relationship(Group, back_populates="perms", uselist=False) + + name = Column(String, primary_key=True, nullable=False) + + config = Column(Boolean, default=False) + + class PermissionManager: def __init__(self, conn): logger.info( @@ -18,75 +64,72 @@ def __init__(self, conn): conn.name, ) - # stuff self.name = conn.name self.config = conn.config - self.group_perms = {} - self.group_users = {} - self.perm_users = {} + session = Session() + Group.__table__.create(session.bind, checkfirst=True) + GroupPermission.__table__.create(session.bind, checkfirst=True) + GroupMember.__table__.create(session.bind, checkfirst=True) self.reload() def reload(self): - self.group_perms = {} - self.group_users = {} - self.perm_users = {} - logger.info( - "[%s|permissions] Reloading permissions for %s.", - self.name, - self.name, - ) - groups = self.config.get("permissions", {}) - # work out the permissions and users each group has - for key, value in groups.items(): - if not key.islower(): - logger.warning( - "[%s|permissions] Warning! Non-lower-case group %r in " - "config. This will cause problems when setting " - "permissions using the bot's permissions commands", - self.name, - key, + session = Session() + + updated = [] + for group_id, data in self.config.get("permissions", {}).items(): + group = self.get_group(group_id) + if not group: + group = Group(name=group_id) + session.add(group) + + group.config = True + updated.append(group) + + for user in data["users"]: + member = session.get( + GroupMember, {"group_id": group_id, "mask": user} ) - key = key.lower() - self.group_perms[key] = [] - self.group_users[key] = [] - for permission in value["perms"]: - self.group_perms[key].append(permission.lower()) - for user in value["users"]: - self.group_users[key].append(user.lower()) - - for group, users in self.group_users.items(): - group_perms = self.group_perms[group] - for perm in group_perms: - if self.perm_users.get(perm) is None: - self.perm_users[perm] = [] - self.perm_users[perm].extend(users) - - logger.debug( - "[%s|permissions] Group permissions: %s", - self.name, - self.group_perms, - ) - logger.debug( - "[%s|permissions] Group users: %s", self.name, self.group_users - ) - logger.debug( - "[%s|permissions] Permission users: %s", self.name, self.perm_users - ) + if not member: + member = GroupMember(group_id=group_id, mask=user) + session.add(member) - def has_perm_mask(self, user_mask, perm, notice=True): - if backdoor: - if match_mask(user_mask.lower(), backdoor.lower()): - return True + member.config = True + updated.append(member) - if not perm.lower() in self.perm_users: - # no one has access - return False + for perm in data["perms"]: + binding = session.get( + GroupPermission, {"group_id": group_id, "name": perm} + ) + if not binding: + binding = GroupPermission(group_id=group_id, name=perm) + session.add(binding) + + binding.config = True + updated.append(binding) + + session.commit() + + for item in session.query(GroupMember).filter_by(config=True).all(): + if item not in updated: + session.delete(item) - allowed_users = self.perm_users[perm.lower()] + for item in session.query(GroupPermission).filter_by(config=True).all(): + if item not in updated: + session.delete(item) + + for item in session.query(Group).filter_by(config=True).all(): + if item not in updated: + session.delete(item) + + session.commit() + + def has_perm_mask(self, user_mask, perm, notice=True): + if backdoor and match_mask(user_mask.lower(), backdoor.lower()): + return True - for allowed_mask in allowed_users: + for allowed_mask in self.get_perm_users(perm): if match_mask(user_mask.lower(), allowed_mask): if notice: logger.info( @@ -95,101 +138,111 @@ def has_perm_mask(self, user_mask, perm, notice=True): user_mask, perm, ) + return True return False + def get_perm_users(self, perm): + session = Session() + member_masks = ( + session.query(GroupMember.mask) + .filter( + GroupMember.group_id.in_( + session.query(GroupPermission.group_id).filter( + GroupPermission.name == perm + ) + ) + ) + .all() + ) + + return [item[0] for item in member_masks] + def get_groups(self): - return set().union(self.group_perms.keys(), self.group_users.keys()) + return Session().query(Group).all() + + def get_group_permissions(self, name): + group = self.get_group(name) + if not group: + return [] - def get_group_permissions(self, group): - return self.group_perms.get(group.lower()) + return [perm.name for perm in group.perms] - def get_group_users(self, group): - return self.group_users.get(group.lower()) + def get_group_users(self, name): + group = self.get_group(name) + if not group: + return [] + + return [member.mask for member in group.members] def get_user_permissions(self, user_mask): - permissions = set() - for permission, users in self.perm_users.items(): - for mask_to_check in users: - if match_mask(user_mask.lower(), mask_to_check): - permissions.add(permission) - return permissions + return { + perm.name + for group in self.get_user_groups(user_mask) + for perm in group.perms + } def get_user_groups(self, user_mask): - groups = [] - for group, users in self.group_users.items(): - for mask_to_check in users: - if match_mask(user_mask.lower(), mask_to_check): - groups.append(group) - continue - return groups + return [ + group for group in self.get_groups() if group.is_member(user_mask) + ] + + def get_group(self, group_id): + return Session().get(Group, group_id) def group_exists(self, group): """ Checks whether a group exists """ - return group.lower() in self.group_perms + return self.get_group(group) is not None - def user_in_group(self, user_mask, group): + def user_in_group(self, user_mask, group_id): """ Checks whether a user is matched by any masks in a given group """ - users = self.group_users.get(group.lower()) - if not users: + group = self.get_group(group_id) + if not group: return False - for mask_to_check in users: - if match_mask(user_mask.lower(), mask_to_check): - return True - return False - def remove_group_user(self, group, user_mask): + return group.is_member(user_mask) + + def remove_group_user(self, group_id, user_mask): """ Removes all users that match user_mask from group. Returns a list of user masks removed from the group. - Use permission_manager.reload() to make this change take affect. - Use bot.config.save_config() to save this change to file. """ - masks_removed = [] + group = self.get_group(group_id) + if not group: + return [] - config_groups = self.config.get("permissions", {}) + masks_removed = [] - for mask_to_check in list(self.group_users[group.lower()]): + session = Session() + for member in group.members: + mask_to_check = member.mask if match_mask(user_mask.lower(), mask_to_check): masks_removed.append(mask_to_check) - # We're going to act like the group keys are all lowercase. - # The user has been warned (above) if they aren't. - # Okay, maybe a warning, but no support. - if group not in config_groups: - logger.warning( - "[%s|permissions] Can't remove user from group due to" - " upper-case group names!", - self.name, - ) - continue - config_group = config_groups.get(group) - config_users = config_group.get("users") - config_users.remove(mask_to_check) + session.delete(member) + + Session().commit() return masks_removed - def add_user_to_group(self, user_mask, group): + def add_user_to_group(self, user_mask, group_id): """ Adds user to group. Returns whether this actually did anything. - Use permission_manager.reload() to make this change take affect. - Use bot.config.save_config() to save this change to file. """ - if self.user_in_group(user_mask, group): + if self.user_in_group(user_mask, group_id): return False - # We're going to act like the group keys are all lowercase. - # The user has been warned (above) if they aren't. - groups = self.config.setdefault("permissions", {}) - if group in groups: - group_dict = groups.get(group) - users = group_dict["users"] - users.append(user_mask) - else: - # create the group - group_dict = {"users": [user_mask], "perms": []} - groups[group] = group_dict + + group = self.get_group(group_id) + session = Session() + if not group: + group = Group(name=group_id) + session.add(group) + + group.members.append(GroupMember(mask=user_mask)) + + session.commit() return True diff --git a/cloudbot/util/database.py b/cloudbot/util/database.py index df65645d4..9d137468b 100644 --- a/cloudbot/util/database.py +++ b/cloudbot/util/database.py @@ -16,7 +16,7 @@ def configure(bind: Engine = None) -> None: - metadata.bind = bind + metadata.bind = bind # type: ignore[assignment] close_all_sessions() Session.remove() Session.configure(bind=bind) diff --git a/plugins/admin_bot.py b/plugins/admin_bot.py index 17e2fb909..8fc6911ad 100644 --- a/plugins/admin_bot.py +++ b/plugins/admin_bot.py @@ -143,14 +143,8 @@ async def remove_permission_user(text, event, bot, conn, notice, reply): reply("No masks with elevated permissions matched {}".format(user)) return - changed = False for group in groups: - if remove_user_from_group(user, group, event): - changed = True - - if changed: - bot.config.save_config() - perm_manager.reload() + remove_user_from_group(user, group, event) @hook.command("adduser", permissions=["permissions_users"]) @@ -192,9 +186,6 @@ async def add_permissions_user(text, nick, conn, bot, notice, reply, admin_log): ) ) - bot.config.save_config() - permission_manager.reload() - @hook.command("stopthebot", permissions=["botcontrol"]) async def stop(text, bot): diff --git a/requirements-dev.txt b/requirements-dev.txt index 6814f2314..b2877d834 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -8,4 +8,5 @@ pytest-cov == 2.11.1 pytest-random-order == 1.0.4 responses == 0.13.2 sqlalchemy-stubs == 0.4 +pre-commit == 2.12.1 -r requirements.txt diff --git a/tests/core_tests/config_test.py b/tests/core_tests/config_test.py index 77d8d8b37..ed1110454 100644 --- a/tests/core_tests/config_test.py +++ b/tests/core_tests/config_test.py @@ -34,14 +34,3 @@ def test_missing_config( "Thank you for " "using CloudBot!\n" ) - - -def test_save(mock_bot_factory, tmp_path, event_loop): - config_file = tmp_path / "config.json" - config_file.write_text("{}", encoding="utf-8") - bot = mock_bot_factory(loop=event_loop) - config = Config(bot, filename=str(config_file)) - config["foo"] = "bar" - config.save_config() - - assert config_file.read_text(encoding="utf-8") == '{\n "foo": "bar"\n}' diff --git a/tests/core_tests/irc_client_test.py b/tests/core_tests/irc_client_test.py index f9deca469..0ee99acc3 100644 --- a/tests/core_tests/irc_client_test.py +++ b/tests/core_tests/irc_client_test.py @@ -26,18 +26,19 @@ def make_mock_conn(event_loop, *, name="testconn"): return conn -def test_send_not_connected(): +def test_send_not_connected(mock_db): bot = MagicMock() client = irc.IrcClient( bot, "irc", "foo", "bar", config={"connection": {"server": "server"}} ) + with pytest.raises(ValueError): client.send("foobar") assert bot.mock_calls == [("loop.create_future", (), {})] -def test_send_closed(event_loop): +def test_send_closed(event_loop, mock_db): bot = MagicMock(loop=event_loop) client = irc.IrcClient( bot, "irc", "foo", "bar", config={"connection": {"server": "server"}} @@ -458,7 +459,7 @@ async def make_client(self, event_loop) -> irc.IrcClient: return client @pytest.mark.asyncio() - async def test_exc(self, caplog_bot, event_loop): + async def test_exc(self, caplog_bot, event_loop, mock_db): client = await self.make_client(event_loop) runs = 0 @@ -479,14 +480,6 @@ async def connect(timeout): 20, "[testconn|permissions] Created permission manager for testconn.", ), - ( - "cloudbot", - 20, - "[testconn|permissions] Reloading permissions for testconn.", - ), - ("cloudbot", 10, "[testconn|permissions] Group permissions: {}"), - ("cloudbot", 10, "[testconn|permissions] Group users: {}"), - ("cloudbot", 10, "[testconn|permissions] Permission users: {}"), ( "cloudbot", 40, @@ -521,7 +514,7 @@ async def connect(timeout): assert client.bot.mock_calls == [] @pytest.mark.asyncio() - async def test_timeout_exc(self, caplog_bot, event_loop): + async def test_timeout_exc(self, caplog_bot, event_loop, mock_db): client = await self.make_client(event_loop) runs = 0 @@ -542,14 +535,6 @@ async def connect(timeout): 20, "[testconn|permissions] Created permission manager for testconn.", ), - ( - "cloudbot", - 20, - "[testconn|permissions] Reloading permissions for testconn.", - ), - ("cloudbot", 10, "[testconn|permissions] Group permissions: {}"), - ("cloudbot", 10, "[testconn|permissions] Group users: {}"), - ("cloudbot", 10, "[testconn|permissions] Permission users: {}"), ( "cloudbot", 40, @@ -579,7 +564,7 @@ async def connect(timeout): assert client.bot.mock_calls == [] @pytest.mark.asyncio() - async def test_other_exc(self, caplog_bot, event_loop): + async def test_other_exc(self, caplog_bot, event_loop, mock_db): client = await self.make_client(event_loop) client.connect = AsyncMock() # type: ignore @@ -594,19 +579,11 @@ async def test_other_exc(self, caplog_bot, event_loop): 20, "[testconn|permissions] Created permission manager for testconn.", ), - ( - "cloudbot", - 20, - "[testconn|permissions] Reloading permissions for testconn.", - ), - ("cloudbot", 10, "[testconn|permissions] Group permissions: {}"), - ("cloudbot", 10, "[testconn|permissions] Group users: {}"), - ("cloudbot", 10, "[testconn|permissions] Permission users: {}"), ] assert client.bot.mock_calls == [] @pytest.mark.asyncio() - async def test_one_connect(self, caplog_bot, event_loop): + async def test_one_connect(self, caplog_bot, event_loop, mock_db): client = await self.make_client(event_loop) async def _connect(timeout=5): @@ -625,19 +602,11 @@ async def _connect(timeout=5): 20, "[testconn|permissions] Created permission manager for testconn.", ), - ( - "cloudbot", - 20, - "[testconn|permissions] Reloading permissions for testconn.", - ), - ("cloudbot", 10, "[testconn|permissions] Group permissions: {}"), - ("cloudbot", 10, "[testconn|permissions] Group users: {}"), - ("cloudbot", 10, "[testconn|permissions] Permission users: {}"), ] assert client.bot.mock_calls == [] @pytest.mark.asyncio() - async def test_create_socket(self, caplog_bot, event_loop): + async def test_create_socket(self, caplog_bot, event_loop, mock_db): client = await self.make_client(event_loop) client.loop.create_connection = mock = MagicMock() fut: "Future[Tuple[None, None]]" = asyncio.Future(loop=client.loop) @@ -652,14 +621,6 @@ async def test_create_socket(self, caplog_bot, event_loop): 20, "[testconn|permissions] Created permission manager for testconn.", ), - ( - "cloudbot", - 20, - "[testconn|permissions] Reloading permissions for testconn.", - ), - ("cloudbot", 10, "[testconn|permissions] Group permissions: {}"), - ("cloudbot", 10, "[testconn|permissions] Group users: {}"), - ("cloudbot", 10, "[testconn|permissions] Permission users: {}"), ("cloudbot", 20, "[testconn] Connecting"), ] assert client.bot.mock_calls == [ diff --git a/tests/core_tests/test_client.py b/tests/core_tests/test_client.py index 892a8c298..b8ae4baf8 100644 --- a/tests/core_tests/test_client.py +++ b/tests/core_tests/test_client.py @@ -37,19 +37,19 @@ async def connect(self, timeout=None): raise ValueError("This is a test") -def test_reload(event_loop): +def test_reload(event_loop, mock_db): client = MockClient(Bot(event_loop), "foo", "foobot", channels=["#foo"]) client.permissions = MagicMock() client.reload() assert client.permissions.mock_calls == [call.reload()] -def test_client_no_config(event_loop): +def test_client_no_config(event_loop, mock_db): client = MockClient(Bot(event_loop), "foo", "foobot", channels=["#foo"]) assert client.config.get("a") is None -def test_client(event_loop): +def test_client(event_loop, mock_db): client = MockClient( Bot(event_loop), "foo", @@ -70,7 +70,7 @@ def test_client(event_loop): client.loop.run_until_complete(client.try_connect()) -def test_client_connect_exc(event_loop): +def test_client_connect_exc(event_loop, mock_db): with patch("random.randrange", return_value=1): client = FailingMockClient( Bot(event_loop), @@ -84,7 +84,7 @@ def test_client_connect_exc(event_loop): @pytest.mark.asyncio() -async def test_try_connect(event_loop): +async def test_try_connect(event_loop, mock_db): client = MockClient( Bot(event_loop), "foo", @@ -97,7 +97,7 @@ async def test_try_connect(event_loop): await client.try_connect() -def test_auto_reconnect(event_loop): +def test_auto_reconnect(event_loop, mock_db): client = MockClient( Bot(event_loop), "foo", diff --git a/tests/core_tests/test_permission_manager.py b/tests/core_tests/test_permission_manager.py index 040e4cc3f..d057c692a 100644 --- a/tests/core_tests/test_permission_manager.py +++ b/tests/core_tests/test_permission_manager.py @@ -1,5 +1,10 @@ from cloudbot import permissions -from cloudbot.permissions import PermissionManager +from cloudbot.permissions import ( + Group, + GroupMember, + GroupPermission, + PermissionManager, +) class MockConn: @@ -8,12 +13,21 @@ def __init__(self, name, config): self.config = config -def test_manager_load(): +def model_to_dict(obj): + cls = type(obj) + out = {} + for column in cls.__table__.c: + out[column.name] = getattr(obj, column.name) + + return out + + +def test_manager_load(mock_db): + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ manager = PermissionManager(MockConn("testconn", {})) - assert not manager.group_perms - assert not manager.group_users - assert not manager.perm_users assert not manager.get_groups() assert not manager.get_group_permissions("foobar") assert not manager.get_group_users("foobar") @@ -33,63 +47,254 @@ def test_manager_load(): other_user = "user1!b@hosaacom" permissions.backdoor = None + perm = "testperm" manager = PermissionManager( MockConn( "testconn", { "permissions": { - "admins": {"users": [user_mask], "perms": ["testperm"]} + "admins": {"users": [user_mask], "perms": [perm]} } }, ) ) assert manager.group_exists("admins") - assert manager.get_groups() == {"admins"} - assert manager.get_user_groups(user) == ["admins"] + assert [model_to_dict(item) for item in manager.get_groups()] == [ + {"name": "admins", "config": True} + ] + assert [model_to_dict(item) for item in manager.get_user_groups(user)] == [ + {"config": True, "name": "admins"} + ] assert not manager.get_user_groups(other_user) assert manager.get_group_users("admins") == [user_mask] - assert manager.get_group_permissions("admins") == ["testperm"] - assert manager.get_user_permissions(user) == {"testperm"} + assert manager.get_group_permissions("admins") == [perm] + assert manager.get_user_permissions(user) == {perm} assert not manager.get_user_permissions(other_user) - assert manager.has_perm_mask(user, "testperm") + assert mock_db.get_data(group_table) == [("admins", True)] + + assert mock_db.get_data(group_member_table) == [ + ("admins", "user!*@host??om", True) + ] + + assert mock_db.get_data(permission_table) == [("admins", perm, True)] + assert manager.get_perm_users(perm) == ["user!*@host??om"] assert manager.user_in_group(user, "admins") assert not manager.user_in_group(other_user, "admins") + assert manager.has_perm_mask(user, perm) assert manager.remove_group_user("admins", user) == [user_mask] - manager.reload() assert "admins" not in manager.get_user_groups(user) assert user_mask not in manager.get_group_users("admins") - assert "testperm" not in manager.get_user_permissions(user) - assert not manager.has_perm_mask(user, "testperm") + assert perm not in manager.get_user_permissions(user) + assert not manager.has_perm_mask(user, perm) assert not manager.user_in_group(user, "admins") -def test_mix_case_group(): +def test_add_user_to_group(mock_db): + manager = PermissionManager(MockConn("testconn", {})) + manager.add_user_to_group("*!*@host", "admins") + manager.add_user_to_group("*!*@mask", "admins") + assert manager.user_in_group("user!name@host", "admins") + assert manager.user_in_group("otheruser!name@mask", "admins") + manager.add_user_to_group("*!*@mask", "admins") + assert len(manager.get_group_users("admins")) == 2 + + +def test_db(mock_db): + session = mock_db.session() + + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ + + group_table.create(mock_db.engine) + group_member_table.create(mock_db.engine) + permission_table.create(mock_db.engine) + + perm = "bar" + group = Group( + name="foo", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing")], + ) + + group1 = Group( + name="foo1", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing1")], + ) + + session.add(group) + session.add(group1) + + session.commit() + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + res = ( + session.query(GroupMember.mask) + .filter( + GroupMember.group_id.in_( + session.query(GroupPermission.group_id).filter( + GroupPermission.name == perm + ) + ) + ) + .all() + ) + + assert res == [("thing",), ("thing1",)] + manager = PermissionManager( MockConn( "testconn", { "permissions": { - "Admins": {"users": ["*!*@host"], "perms": ["testperm"]} + "admins": {"users": ["a", "d"], "perms": ["foo"]}, + "foo": { + "users": ["b", "c", "thing"], + "perms": ["bar", "baz"], + }, } }, ) ) - assert manager.group_exists("admins") - manager.remove_group_user("admins", "user!name@host") - manager.reload() - assert manager.user_in_group("user!name@host", "admins") + assert mock_db.get_data(group_table) == [ + ("foo", True), + ("foo1", False), + ("admins", True), + ] + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", True), + ("foo1", "thing1", False), + ("admins", "a", True), + ("admins", "d", True), + ("foo", "b", True), + ("foo", "c", True), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", True), + ("foo1", "bar", False), + ("admins", "foo", True), + ("foo", "baz", True), + ] + + assert manager.has_perm_mask("a", "foo", notice=False) + assert not manager.has_perm_mask("b", "foo", notice=False) + assert not manager.has_perm_mask("b", "foo", notice=True) + + +def test_db_config_merge(mock_db): + session = mock_db.session() + + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ + + group_table.create(mock_db.engine) + group_member_table.create(mock_db.engine) + permission_table.create(mock_db.engine) + + perm = "bar" + group = Group( + name="foo", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing")], + ) + + group1 = Group( + name="foo1", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing1")], + ) + + session.add(group) + session.add(group1) + + session.commit() + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + conn = MockConn( + "testconn", + { + "permissions": { + "admins": {"users": ["a", "d"], "perms": ["foo"]}, + "foo": {"users": ["b", "c", "thing"], "perms": ["bar", "baz"]}, + } + }, + ) + + manager = PermissionManager(conn) + + assert mock_db.get_data(group_table) == [ + ("foo", True), + ("foo1", False), + ("admins", True), + ] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", True), + ("foo1", "thing1", False), + ("admins", "a", True), + ("admins", "d", True), + ("foo", "b", True), + ("foo", "c", True), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", True), + ("foo1", "bar", False), + ("admins", "foo", True), + ("foo", "baz", True), + ] + + perm_config = conn.config["permissions"] + perm_config["foo"]["perms"].pop() + perm_config["foo"]["perms"].append("test") + perm_config["foo"]["users"].pop() + perm_config.pop("admins") -def test_add_user_to_group(): - manager = PermissionManager(MockConn("testconn", {})) - manager.add_user_to_group("*!*@host", "admins") - manager.add_user_to_group("*!*@mask", "admins") - manager.reload() - assert manager.user_in_group("user!name@host", "admins") - assert manager.user_in_group("otheruser!name@mask", "admins") - manager.add_user_to_group("*!*@mask", "admins") manager.reload() - assert len(manager.get_group_users("admins")) == 2 + + assert mock_db.get_data(group_table) == [("foo", True), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo1", "thing1", False), + ("foo", "b", True), + ("foo", "c", True), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", True), + ("foo1", "bar", False), + ("foo", "test", True), + ] + + assert manager.remove_group_user("other", "test") == [] + + assert manager.remove_group_user("foo", "missing") == [] diff --git a/tests/plugin_tests/core_misc_test.py b/tests/plugin_tests/core_misc_test.py index 17ad9bae3..ee5abdcdb 100644 --- a/tests/plugin_tests/core_misc_test.py +++ b/tests/plugin_tests/core_misc_test.py @@ -8,8 +8,8 @@ from tests.util.mock_irc_client import MockIrcClient -def test_invite_join(mock_bot_factory, event_loop): - bot = mock_bot_factory(loop=event_loop) +def test_invite_join(mock_bot_factory, event_loop, mock_db): + bot = mock_bot_factory(loop=event_loop, db=mock_db) conn = MockIrcClient( bot, "fooconn", "foo", {"connection": {"server": "host.invalid"}} ) @@ -18,8 +18,8 @@ def test_invite_join(mock_bot_factory, event_loop): assert cast(MagicMock, conn.send).mock_calls == [call("JOIN #bar")] -def test_invite_join_disabled(mock_bot_factory, event_loop): - bot = mock_bot_factory(loop=event_loop) +def test_invite_join_disabled(mock_bot_factory, event_loop, mock_db): + bot = mock_bot_factory(loop=event_loop, db=mock_db) conn = MockIrcClient( bot, "fooconn", @@ -65,7 +65,7 @@ def test_invite_join_disabled(mock_bot_factory, event_loop): ({"mode": "+I"}, [call("MODE foobot +I")]), ], ) -async def test_on_connect(config, calls): +async def test_on_connect(config, calls, mock_db): bot = MagicMock() config = config.copy() config.setdefault("connection", {}).setdefault("server", "host.invalid") diff --git a/tests/plugin_tests/test_core_connect.py b/tests/plugin_tests/test_core_connect.py index 76845d7ce..0c7f221cb 100644 --- a/tests/plugin_tests/test_core_connect.py +++ b/tests/plugin_tests/test_core_connect.py @@ -14,8 +14,8 @@ async def connect(self, timeout=None): pass -def test_ssl_client(event_loop, mock_bot_factory): - bot = mock_bot_factory(loop=event_loop) +def test_ssl_client(event_loop, mock_bot_factory, mock_db): + bot = mock_bot_factory(loop=event_loop, db=mock_db) client = MockClient( bot, "mock", @@ -38,8 +38,8 @@ def test_ssl_client(event_loop, mock_bot_factory): assert client.ssl_context.verify_mode is ssl.CERT_REQUIRED -def test_ssl_client_no_verify(event_loop, mock_bot_factory): - bot = mock_bot_factory(loop=event_loop) +def test_ssl_client_no_verify(event_loop, mock_bot_factory, mock_db): + bot = mock_bot_factory(loop=event_loop, db=mock_db) client = MockClient( bot, "mock", @@ -64,8 +64,8 @@ def test_ssl_client_no_verify(event_loop, mock_bot_factory): @pytest.mark.asyncio() -async def test_core_connects(event_loop, mock_bot_factory): - bot = mock_bot_factory(loop=event_loop) +async def test_core_connects(event_loop, mock_bot_factory, mock_db): + bot = mock_bot_factory(loop=event_loop, db=mock_db) client = MockClient( bot, "mock", diff --git a/tests/plugin_tests/test_core_misc.py b/tests/plugin_tests/test_core_misc.py index 1fbf10ee6..03e842f86 100644 --- a/tests/plugin_tests/test_core_misc.py +++ b/tests/plugin_tests/test_core_misc.py @@ -15,9 +15,9 @@ def __init__(self, bot, *args, **kwargs): @pytest.mark.asyncio -async def test_do_joins(mock_bot_factory, event_loop): +async def test_do_joins(mock_bot_factory, event_loop, mock_db): client = MockClient( - mock_bot_factory(loop=event_loop), + mock_bot_factory(loop=event_loop, db=mock_db), "foo", "foobot", channels=[ From 5c50a9ef5164ed3a6352e42d2126644eb843d217 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Thu, 29 Apr 2021 06:47:55 -0400 Subject: [PATCH 2/3] Add more tests for db permissions --- plugins/admin_bot.py | 17 ++++--- tests/core_tests/config_test.py | 13 +++++ tests/plugin_tests/test_admin_bot.py | 73 ++++++++++++++++++++++++++++ tests/util/mock_conn.py | 1 + 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/plugins/admin_bot.py b/plugins/admin_bot.py index 8fc6911ad..33010d247 100644 --- a/plugins/admin_bot.py +++ b/plugins/admin_bot.py @@ -96,7 +96,7 @@ async def get_user_groups(event): return "User {} is in no permission groups".format(user) -def remove_user_from_group(user, group, event): +def remove_user_from_group(user, group: str, event): permission_manager = event.conn.permissions changed_masks = permission_manager.remove_group_user( group.lower(), user.lower() @@ -114,15 +114,15 @@ def remove_user_from_group(user, group, event): @hook.command("deluser", permissions=["permissions_users"]) -async def remove_permission_user(text, event, bot, conn, notice, reply): +async def remove_permission_user(text, event, conn): """ [group] - removes from [group], or from all groups if no group is specified""" split = text.split() if len(split) > 2: - notice("Too many arguments") + event.notice("Too many arguments") return if not split: - notice("Not enough arguments") + event.notice("Not enough arguments") return perm_manager = conn.permissions @@ -131,16 +131,17 @@ async def remove_permission_user(text, event, bot, conn, notice, reply): group = split[1] if group and not perm_manager.group_exists(group.lower()): - notice("Unknown group '{}'".format(group)) + event.notice("Unknown group '{}'".format(group)) return groups = [group] if perm_manager.user_in_group(user, group) else [] else: - group = None - groups = perm_manager.get_user_groups(user.lower()) + groups = [g.name for g in perm_manager.get_user_groups(user.lower())] if not groups: - reply("No masks with elevated permissions matched {}".format(user)) + event.reply( + "No masks with elevated permissions matched {}".format(user) + ) return for group in groups: diff --git a/tests/core_tests/config_test.py b/tests/core_tests/config_test.py index ed1110454..ff7d0ee47 100644 --- a/tests/core_tests/config_test.py +++ b/tests/core_tests/config_test.py @@ -34,3 +34,16 @@ def test_missing_config( "Thank you for " "using CloudBot!\n" ) + + +def test_loads( + tmp_path, + mock_sleep, + event_loop, + mock_bot_factory, +): + config_file = tmp_path / "config.json" + config_file.write_text('{"a":1}') + bot = mock_bot_factory(loop=event_loop) + conf = Config(bot, filename=str(config_file)) + assert dict(conf) == {"a": 1} diff --git a/tests/plugin_tests/test_admin_bot.py b/tests/plugin_tests/test_admin_bot.py index 421b0920c..fc0c50031 100644 --- a/tests/plugin_tests/test_admin_bot.py +++ b/tests/plugin_tests/test_admin_bot.py @@ -3,8 +3,15 @@ import pytest from cloudbot.event import CommandEvent +from cloudbot.permissions import ( + Group, + GroupMember, + GroupPermission, + PermissionManager, +) from cloudbot.util import async_util, func_utils from plugins import admin_bot +from tests.util.mock_conn import MockConn @pytest.mark.parametrize( @@ -91,3 +98,69 @@ def f(self, attr): call.admin_log('bar used ME to make me ACT "do thing" in #foo.'), call.conn.ctcp("#foo", "ACTION", "do thing"), ] + + +@pytest.mark.asyncio +async def test_remove_permission_user(mock_db): + conn = MockConn(nick="testconn") + + session = mock_db.session() + + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ + + group_table.create(mock_db.engine) + group_member_table.create(mock_db.engine) + permission_table.create(mock_db.engine) + + manager = PermissionManager(conn) + conn.permissions = manager + perm = "bar" + group = Group( + name="foo", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing")], + ) + + group1 = Group( + name="foo1", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing1")], + ) + + session.add(group) + session.add(group1) + + session.commit() + event = MagicMock() + event.conn = conn + event.nick = "foo" + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + await admin_bot.remove_permission_user("thing1", event, conn) + + assert event.mock_calls == [ + call.reply("Removed thing1 from foo1"), + call.admin_log("foo used deluser remove thing1 from foo1."), + ] + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [("foo", "thing", False)] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] diff --git a/tests/util/mock_conn.py b/tests/util/mock_conn.py index 46962c358..da135acb7 100644 --- a/tests/util/mock_conn.py +++ b/tests/util/mock_conn.py @@ -5,6 +5,7 @@ class MockConn: def __init__(self, *, nick=None, name=None): self.nick = nick or "TestBot" self.name = name or "testconn" + self.permissions = None self.config = {} self.history = {} self.reload = MagicMock() From 17c5f55d0ef73fe4a047fd33fca11ee74273a02a Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Thu, 29 Apr 2021 20:09:00 -0400 Subject: [PATCH 3/3] Add more tests for db perms --- tests/plugin_tests/test_admin_bot.py | 132 +++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/tests/plugin_tests/test_admin_bot.py b/tests/plugin_tests/test_admin_bot.py index fc0c50031..251fcd59c 100644 --- a/tests/plugin_tests/test_admin_bot.py +++ b/tests/plugin_tests/test_admin_bot.py @@ -164,3 +164,135 @@ async def test_remove_permission_user(mock_db): ("foo", "bar", False), ("foo1", "bar", False), ] + + +@pytest.mark.asyncio +async def test_remove_permission_user_too_many_args(mock_db): + conn = MockConn(nick="testconn") + + session = mock_db.session() + + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ + + group_table.create(mock_db.engine) + group_member_table.create(mock_db.engine) + permission_table.create(mock_db.engine) + + manager = PermissionManager(conn) + conn.permissions = manager + perm = "bar" + group = Group( + name="foo", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing")], + ) + + group1 = Group( + name="foo1", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing1")], + ) + + session.add(group) + session.add(group1) + + session.commit() + event = MagicMock() + event.conn = conn + event.nick = "foo" + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + await admin_bot.remove_permission_user("thing1 a b c", event, conn) + + assert event.mock_calls == [call.notice("Too many arguments")] + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + +@pytest.mark.asyncio +async def test_remove_permission_user_too_few_args(mock_db): + conn = MockConn(nick="testconn") + + session = mock_db.session() + + group_table = Group.__table__ + group_member_table = GroupMember.__table__ + permission_table = GroupPermission.__table__ + + group_table.create(mock_db.engine) + group_member_table.create(mock_db.engine) + permission_table.create(mock_db.engine) + + manager = PermissionManager(conn) + conn.permissions = manager + perm = "bar" + group = Group( + name="foo", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing")], + ) + + group1 = Group( + name="foo1", + perms=[GroupPermission(name=perm)], + members=[GroupMember(mask="thing1")], + ) + + session.add(group) + session.add(group1) + + session.commit() + event = MagicMock() + event.conn = conn + event.nick = "foo" + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ] + + await admin_bot.remove_permission_user("", event, conn) + + assert event.mock_calls == [call.notice("Not enough arguments")] + + assert mock_db.get_data(group_table) == [("foo", False), ("foo1", False)] + + assert mock_db.get_data(group_member_table) == [ + ("foo", "thing", False), + ("foo1", "thing1", False), + ] + + assert mock_db.get_data(permission_table) == [ + ("foo", "bar", False), + ("foo1", "bar", False), + ]