From 3f454743a93ff54e1d960f2978e4ece7a643d97f Mon Sep 17 00:00:00 2001 From: Basraah Date: Wed, 6 Sep 2017 03:12:27 +1000 Subject: [PATCH] Openfire group names fix (#859) * Force lowercase group names * Fix comparison of group names * Sanitise group name for broadcast message --- services/modules/openfire/manager.py | 23 ++++++++++++----------- services/modules/openfire/tests.py | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/services/modules/openfire/manager.py b/services/modules/openfire/manager.py index 7cf18ea7..9b8c82e6 100755 --- a/services/modules/openfire/manager.py +++ b/services/modules/openfire/manager.py @@ -61,7 +61,7 @@ class OpenfireManager: @staticmethod def _sanitize_groupname(name): - name = name.strip(' _') + name = name.strip(' _').lower() return re.sub('[^\w.-]', '', name) @staticmethod @@ -120,9 +120,10 @@ class OpenfireManager: logger.error("Unable to update openfire user %s password - user not found on server." % username) return "" - @staticmethod - def update_user_groups(username, groups): + @classmethod + def update_user_groups(cls, username, groups): logger.debug("Updating openfire user %s groups %s" % (username, groups)) + s_groups = list(map(cls._sanitize_groupname, groups)) # Sanitized group names api = ofUsers(settings.OPENFIRE_ADDRESS, settings.OPENFIRE_SECRET_KEY) response = api.get_user_groups(username) remote_groups = [] @@ -130,16 +131,15 @@ class OpenfireManager: remote_groups = response['groupname'] if isinstance(remote_groups, six.string_types): remote_groups = [remote_groups] + remote_groups = list(map(cls._sanitize_groupname, remote_groups)) logger.debug("Openfire user %s has groups %s" % (username, remote_groups)) add_groups = [] del_groups = [] - for g in groups: - g = OpenfireManager._sanitize_groupname(g) + for g in s_groups: if g not in remote_groups: add_groups.append(g) for g in remote_groups: - g = OpenfireManager._sanitize_groupname(g) - if g not in groups: + if g not in s_groups: del_groups.append(g) logger.info( "Updating openfire groups for user %s - adding %s, removing %s" % (username, add_groups, del_groups)) @@ -155,10 +155,11 @@ class OpenfireManager: api.delete_user_groups(username, groups) logger.info("Deleted groups %s from openfire user %s" % (groups, username)) - @staticmethod - def send_broadcast_message(group_name, broadcast_message): - logger.debug("Sending jabber ping to group %s with message %s" % (group_name, broadcast_message)) - to_address = group_name + '@' + settings.BROADCAST_SERVICE_NAME + '.' + settings.JABBER_URL + @classmethod + def send_broadcast_message(cls, group_name, broadcast_message): + s_group_name = cls._sanitize_groupname(group_name) + logger.debug("Sending jabber ping to group %s with message %s" % (s_group_name, broadcast_message)) + to_address = s_group_name + '@' + settings.BROADCAST_SERVICE_NAME + '.' + settings.JABBER_URL xmpp = PingBot(settings.BROADCAST_USER, settings.BROADCAST_USER_PASSWORD, to_address, broadcast_message) xmpp.register_plugin('xep_0030') # Service Discovery xmpp.register_plugin('xep_0199') # XMPP Ping diff --git a/services/modules/openfire/tests.py b/services/modules/openfire/tests.py index 2ce4a31b..b351bf6f 100644 --- a/services/modules/openfire/tests.py +++ b/services/modules/openfire/tests.py @@ -219,3 +219,28 @@ class OpenfireManagerTestCase(TestCase): result_username = self.manager._OpenfireManager__sanitize_username(test_username) self.assertEqual(result_username, 'My_Test\\20User\\22\\27\\26\\2f\\3a\\3c\\3e\\40name\\5c20name') + + def test__sanitize_groupname(self): + test_groupname = " My_Test Groupname" + + result_groupname = self.manager._sanitize_groupname(test_groupname) + + self.assertEqual(result_groupname, "my_testgroupname") + + @mock.patch(MODULE_PATH + '.manager.ofUsers') + def test_update_user_groups(self, api): + groups = ["AddGroup", "othergroup", "Guest Group"] + server_groups = ["othergroup", "Guest Group", "REMOVE group"] + username = "testuser" + api_instance = api.return_value + api_instance.get_user_groups.return_value = {'groupname': server_groups} + + self.manager.update_user_groups(username, groups) + + self.assertTrue(api_instance.add_user_groups.called) + args, kwargs = api_instance.add_user_groups.call_args + self.assertEqual(args[1], ["addgroup"]) + + self.assertTrue(api_instance.delete_user_groups.called) + args, kwargs = api_instance.delete_user_groups.call_args + self.assertEqual(args[1], ["removegroup"])