From 980ce845abbb02e0f694e668d400089005f45596 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 15:24:41 +1000
Subject: [PATCH 01/33] broker: Pass current action to topic filter.

This will allow a topic filter to act on both subscription requests and
publishing.
---
 amqtt/broker.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index fb25bf76..26ce33ab 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -703,7 +703,7 @@ async def authenticate(self, session: Session, listener):
         # If all plugins returned True, authentication is success
         return auth_result
 
-    async def topic_filtering(self, session: Session, topic):
+    async def topic_filtering(self, session: Session, topic, action: str):
         """
         This method call the topic_filtering method on registered plugins to check that the subscription is allowed.
         User is considered allowed if all plugins called return True.
@@ -713,7 +713,8 @@ async def topic_filtering(self, session: Session, topic):
          - None if topic filtering can't be achieved (then plugin result is then ignored)
         :param session:
         :param listener:
-        :param topic: Topic in which the client wants to subscribe
+        :param topic: Topic in which the client wants to subscribe / publish
+        :param action: What is being done with the topic?  subscribe or publish
         :return:
         """
         topic_plugins = None
@@ -724,6 +725,7 @@ async def topic_filtering(self, session: Session, topic):
             "topic_filtering",
             session=session,
             topic=topic,
+            action=action,
             filter_plugins=topic_plugins,
         )
         topic_result = True

From 49e39d55c355e95c22cb61ac0d5c9d3aab7ddbd9 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 15:32:41 +1000
Subject: [PATCH 02/33] broker: Pass action=subscribe when checking topics

---
 amqtt/broker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index 26ce33ab..11d0fdb6 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -775,7 +775,7 @@ async def add_subscription(self, subscription, session):
                         # [MQTT-4.7.1-3] + wildcard character must occupy entire level
                         return 0x80
             # Check if the client is authorised to connect to the topic
-            permitted = await self.topic_filtering(session, topic=a_filter)
+            permitted = await self.topic_filtering(session, topic=a_filter, action='subscribe')
             if not permitted:
                 return 0x80
             qos = subscription[1]

From 88886db6220d3e3136f432e01ed927e94e3be66b Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 15:37:18 +1000
Subject: [PATCH 03/33] broker: Check ACL on publish for permissions

---
 amqtt/broker.py | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index 11d0fdb6..d1e1e94c 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -620,21 +620,32 @@ async def client_connected(
                             % client_session.client_id
                         )
                         break
-                    await self.plugins_manager.fire_event(
-                        EVENT_BROKER_MESSAGE_RECEIVED,
-                        client_id=client_session.client_id,
-                        message=app_message,
-                    )
-                    await self._broadcast_message(
-                        client_session, app_message.topic, app_message.data
+
+                    # See if the user is allowed to publish to this topic.
+                    permitted = await self.topic_filtering(
+                            client_session, topic=app_message.topic, action='publish'
                     )
-                    if app_message.publish_packet.retain_flag:
-                        self.retain_message(
-                            client_session,
-                            app_message.topic,
-                            app_message.data,
-                            app_message.qos,
+                    if not permitted:
+                        self.logger.info(
+                                "%s forbidden TOPIC %s sent in PUBLISH message.",
+                                client_session.client_id, app_message.topic
+                        )
+                    else:
+                        await self.plugins_manager.fire_event(
+                            EVENT_BROKER_MESSAGE_RECEIVED,
+                            client_id=client_session.client_id,
+                            message=app_message,
+                        )
+                        await self._broadcast_message(
+                            client_session, app_message.topic, app_message.data
                         )
+                        if app_message.publish_packet.retain_flag:
+                            self.retain_message(
+                                client_session,
+                                app_message.topic,
+                                app_message.data,
+                                app_message.qos,
+                            )
                     wait_deliver = asyncio.Task(
                         handler.mqtt_deliver_next_message(), loop=self._loop
                     )

From 35dcccf575e4c627111ede9b946ecfee6e6fc10d Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 15:44:51 +1000
Subject: [PATCH 04/33] plugins.topic_checking: Check the `action` in
 `topic_acl`.

We maintain backward compatibility with older configurations by assuming
all `PUBLISH` actions are permitted if no ACL is present.  Otherwise, we
follow the same rules as for `SUBSCRIBE`, with the exception that we
read the ACL from the `publish-acl` property instead of `acl`.
---
 amqtt/plugins/topic_checking.py | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/amqtt/plugins/topic_checking.py b/amqtt/plugins/topic_checking.py
index b52a156f..4e870a3e 100644
--- a/amqtt/plugins/topic_checking.py
+++ b/amqtt/plugins/topic_checking.py
@@ -66,11 +66,24 @@ async def topic_filtering(self, *args, **kwargs):
         if filter_result:
             session = kwargs.get("session", None)
             req_topic = kwargs.get("topic", None)
+            action = kwargs.get("action", None)
+
+            # hbmqtt and older amqtt do not support publish filtering
+            if (action == "publish") and ("publish-acl" not in self.topic_config):
+                # maintain backward compatibility, assume permitted
+                return True
+
             if req_topic:
                 username = session.username
                 if username is None:
                     username = "anonymous"
-                allowed_topics = self.topic_config["acl"].get(username, None)
+
+                if action == "publish":
+                    acl = self.topic_config["publish-acl"]
+                elif action == "subscribe":
+                    acl = self.topic_config["acl"]
+
+                allowed_topics = acl.get(username, None)
                 if allowed_topics:
                     for allowed_topic in allowed_topics:
                         if self.topic_ac(req_topic, allowed_topic):

From 6ab32a1ed6bbc2e7796f27c3f25ab89785f66b0c Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 16:39:57 +1000
Subject: [PATCH 05/33] plugins.topic_checking: Initialise topic_config to None
 if not present.

If the user forgets to provide a `topic-check` key, we need to
initialise `topic_config` to some value or else `topic_filtering` will
fail with an `AttributeError`.
---
 amqtt/plugins/topic_checking.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/amqtt/plugins/topic_checking.py b/amqtt/plugins/topic_checking.py
index 4e870a3e..8624520b 100644
--- a/amqtt/plugins/topic_checking.py
+++ b/amqtt/plugins/topic_checking.py
@@ -7,6 +7,7 @@ def __init__(self, context):
             self.context.logger.warning(
                 "'topic-check' section not found in context configuration"
             )
+            self.topic_config = None
 
     def topic_filtering(self, *args, **kwargs):
         if not self.topic_config:

From 60f3bee2a1279b6b91d06274beebabe3dd0edff0 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 16:41:49 +1000
Subject: [PATCH 06/33] plugins.topic_checking tests: Test BaseTopicPlugin.

---
 tests/plugins/test_topic_checking.py | 82 ++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 tests/plugins/test_topic_checking.py

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
new file mode 100644
index 00000000..f2099f0e
--- /dev/null
+++ b/tests/plugins/test_topic_checking.py
@@ -0,0 +1,82 @@
+# Copyright (c) 2015 Nicolas JOUANIN
+#
+# See the file license.txt for copying permission.
+
+import pytest
+
+from amqtt.plugins.manager import BaseContext
+from amqtt.plugins.topic_checking import BaseTopicPlugin, TopicTabooPlugin, TopicAccessControlListPlugin
+
+
+class DummyLogger(object):
+    def __init__(self):
+        self.messages = []
+
+    def warning(self, *args, **kwargs):
+        self.messages.append((args, kwargs))
+
+
+@pytest.mark.asyncio
+async def test_base_no_config():
+    """
+    Check BaseTopicPlugin returns false if no topic-check is present.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {}
+
+    plugin = BaseTopicPlugin(context)
+    assert plugin.topic_filtering() is False
+
+    # Should have printed a couple of warnings
+    assert len(context.logger.messages) == 2
+    assert context.logger.messages[0] == (
+            ("'topic-check' section not found in context configuration",),
+            {}
+    )
+    assert context.logger.messages[1] == (
+            ("'auth' section not found in context configuration",),
+            {}
+    )
+
+
+@pytest.mark.asyncio
+async def test_base_empty_config():
+    """
+    Check BaseTopicPlugin returns false if topic-check is empty.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {}
+    }
+
+    plugin = BaseTopicPlugin(context)
+    assert plugin.topic_filtering() is False
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 1
+    assert context.logger.messages[0] == (
+            ("'auth' section not found in context configuration",),
+            {}
+    )
+
+
+@pytest.mark.asyncio
+async def test_base_enabled_config():
+    """
+    Check BaseTopicPlugin returns true if enabled.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    plugin = BaseTopicPlugin(context)
+    assert plugin.topic_filtering() is True
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0

From ae2780167163546b56a68594744589dfd46d3ee7 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:00:41 +1000
Subject: [PATCH 07/33] plugins.test_topic tests: Add tests for
 TopicTabooPlugin.

This is more of a "demo" plug-in, but let's test it anyway.
---
 tests/plugins/test_topic_checking.py | 144 ++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 3 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index f2099f0e..cb9e1006 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -6,6 +6,7 @@
 
 from amqtt.plugins.manager import BaseContext
 from amqtt.plugins.topic_checking import BaseTopicPlugin, TopicTabooPlugin, TopicAccessControlListPlugin
+from amqtt.session import Session
 
 
 class DummyLogger(object):
@@ -16,6 +17,9 @@ def warning(self, *args, **kwargs):
         self.messages.append((args, kwargs))
 
 
+# Base plug-in object
+
+
 @pytest.mark.asyncio
 async def test_base_no_config():
     """
@@ -26,7 +30,8 @@ async def test_base_no_config():
     context.config = {}
 
     plugin = BaseTopicPlugin(context)
-    assert plugin.topic_filtering() is False
+    authorised = plugin.topic_filtering()
+    assert authorised is False
 
     # Should have printed a couple of warnings
     assert len(context.logger.messages) == 2
@@ -52,7 +57,8 @@ async def test_base_empty_config():
     }
 
     plugin = BaseTopicPlugin(context)
-    assert plugin.topic_filtering() is False
+    authorised = plugin.topic_filtering()
+    assert authorised is False
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 1
@@ -76,7 +82,139 @@ async def test_base_enabled_config():
     }
 
     plugin = BaseTopicPlugin(context)
-    assert plugin.topic_filtering() is True
+    authorised = plugin.topic_filtering()
+    assert authorised is True
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+
+# Taboo plug-in
+
+@pytest.mark.asyncio
+async def test_taboo_empty_config():
+    """
+    Check TopicTabooPlugin returns false if topic-check absent.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {}
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering()
+    assert authorised is False
+
+    # Should have printed a couple of warnings
+    assert len(context.logger.messages) == 2
+    assert context.logger.messages[0] == (
+            ("'topic-check' section not found in context configuration",),
+            {}
+    )
+    assert context.logger.messages[1] == (
+            ("'auth' section not found in context configuration",),
+            {}
+    )
+
+@pytest.mark.asyncio
+async def test_taboo_not_taboo_topic():
+    """
+    Check TopicTabooPlugin returns true if topic not taboo
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    session = Session()
+    session.username = 'anybody'
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering(
+            session=session,
+            topic='not/prohibited'
+    )
+    assert authorised is True
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+@pytest.mark.asyncio
+async def test_taboo_anon_taboo_topic():
+    """
+    Check TopicTabooPlugin returns false if topic is taboo and session is anonymous.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    session = Session()
+    session.username = ''
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering(
+            session=session,
+            topic='prohibited'
+    )
+    assert authorised is False
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+@pytest.mark.asyncio
+async def test_taboo_notadmin_taboo_topic():
+    """
+    Check TopicTabooPlugin returns false if topic is taboo and user is not "admin".
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    session = Session()
+    session.username = 'notadmin'
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering(
+            session=session,
+            topic='prohibited'
+    )
+    assert authorised is False
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+@pytest.mark.asyncio
+async def test_taboo_admin_taboo_topic():
+    """
+    Check TopicTabooPlugin returns true if topic is taboo and user is "admin".
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    session = Session()
+    session.username = 'admin'
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering(
+            session=session,
+            topic='prohibited'
+    )
+    assert authorised is True
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0

From be8f4541599077e27d49866440d17d62a6930770 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:02:08 +1000
Subject: [PATCH 08/33] plugins.topic_checking tests: Throw test suite at
 `black`.

Clean up the coding style a bit.
---
 tests/plugins/test_topic_checking.py | 93 ++++++++++------------------
 1 file changed, 34 insertions(+), 59 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index cb9e1006..ede90d91 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -5,7 +5,11 @@
 import pytest
 
 from amqtt.plugins.manager import BaseContext
-from amqtt.plugins.topic_checking import BaseTopicPlugin, TopicTabooPlugin, TopicAccessControlListPlugin
+from amqtt.plugins.topic_checking import (
+    BaseTopicPlugin,
+    TopicTabooPlugin,
+    TopicAccessControlListPlugin,
+)
 from amqtt.session import Session
 
 
@@ -36,12 +40,12 @@ async def test_base_no_config():
     # Should have printed a couple of warnings
     assert len(context.logger.messages) == 2
     assert context.logger.messages[0] == (
-            ("'topic-check' section not found in context configuration",),
-            {}
+        ("'topic-check' section not found in context configuration",),
+        {},
     )
     assert context.logger.messages[1] == (
-            ("'auth' section not found in context configuration",),
-            {}
+        ("'auth' section not found in context configuration",),
+        {},
     )
 
 
@@ -52,9 +56,7 @@ async def test_base_empty_config():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {}
-    }
+    context.config = {"topic-check": {}}
 
     plugin = BaseTopicPlugin(context)
     authorised = plugin.topic_filtering()
@@ -63,8 +65,8 @@ async def test_base_empty_config():
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 1
     assert context.logger.messages[0] == (
-            ("'auth' section not found in context configuration",),
-            {}
+        ("'auth' section not found in context configuration",),
+        {},
     )
 
 
@@ -75,11 +77,7 @@ async def test_base_enabled_config():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     plugin = BaseTopicPlugin(context)
     authorised = plugin.topic_filtering()
@@ -91,6 +89,7 @@ async def test_base_enabled_config():
 
 # Taboo plug-in
 
+
 @pytest.mark.asyncio
 async def test_taboo_empty_config():
     """
@@ -107,14 +106,15 @@ async def test_taboo_empty_config():
     # Should have printed a couple of warnings
     assert len(context.logger.messages) == 2
     assert context.logger.messages[0] == (
-            ("'topic-check' section not found in context configuration",),
-            {}
+        ("'topic-check' section not found in context configuration",),
+        {},
     )
     assert context.logger.messages[1] == (
-            ("'auth' section not found in context configuration",),
-            {}
+        ("'auth' section not found in context configuration",),
+        {},
     )
 
+
 @pytest.mark.asyncio
 async def test_taboo_not_taboo_topic():
     """
@@ -122,25 +122,19 @@ async def test_taboo_not_taboo_topic():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     session = Session()
-    session.username = 'anybody'
+    session.username = "anybody"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(
-            session=session,
-            topic='not/prohibited'
-    )
+    authorised = await plugin.topic_filtering(session=session, topic="not/prohibited")
     assert authorised is True
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
 
+
 @pytest.mark.asyncio
 async def test_taboo_anon_taboo_topic():
     """
@@ -148,25 +142,19 @@ async def test_taboo_anon_taboo_topic():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     session = Session()
-    session.username = ''
+    session.username = ""
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(
-            session=session,
-            topic='prohibited'
-    )
+    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
     assert authorised is False
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
 
+
 @pytest.mark.asyncio
 async def test_taboo_notadmin_taboo_topic():
     """
@@ -174,25 +162,19 @@ async def test_taboo_notadmin_taboo_topic():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     session = Session()
-    session.username = 'notadmin'
+    session.username = "notadmin"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(
-            session=session,
-            topic='prohibited'
-    )
+    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
     assert authorised is False
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
 
+
 @pytest.mark.asyncio
 async def test_taboo_admin_taboo_topic():
     """
@@ -200,20 +182,13 @@ async def test_taboo_admin_taboo_topic():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     session = Session()
-    session.username = 'admin'
+    session.username = "admin"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(
-            session=session,
-            topic='prohibited'
-    )
+    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
     assert authorised is True
 
     # Should NOT have printed warnings

From c52a1047bcb687ef1a86193ea62d38ac05ac5541 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:24:13 +1000
Subject: [PATCH 09/33] plugins.topic_checking: Test topic_ac static method

---
 tests/plugins/test_topic_checking.py | 59 ++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index ede90d91..75829c84 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -193,3 +193,62 @@ async def test_taboo_admin_taboo_topic():
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
+
+
+def test_topic_ac_not_match():
+    """
+    Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match.
+    """
+    assert (
+        TopicAccessControlListPlugin.topic_ac("a/topic/to/match", "a/topic/to/notmatch")
+        is False
+    )
+
+
+def test_topic_ac_not_match_longer_acl():
+    """
+    Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match and ACL topic is longer.
+    """
+    assert (
+        TopicAccessControlListPlugin.topic_ac("topic", "topic/is/longer") is False
+    )
+
+
+def test_topic_ac_not_match_longer_rq():
+    """
+    Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match and RQ topic is longer.
+    """
+    assert (
+        TopicAccessControlListPlugin.topic_ac("topic/is/longer", "topic") is False
+    )
+
+
+def test_topic_ac_match_exact():
+    """
+    Test TopicAccessControlListPlugin.topic_ac returns true if topics match exactly.
+    """
+    assert TopicAccessControlListPlugin.topic_ac("exact/topic", "exact/topic") is True
+
+
+def test_topic_ac_match_hash():
+    """
+    Test TopicAccessControlListPlugin.topic_ac correctly handles '+' wildcard.
+    """
+    assert (
+        TopicAccessControlListPlugin.topic_ac(
+            "a/topic/anything/value", "a/topic/+/value"
+        )
+        is True
+    )
+
+
+def test_topic_ac_match_hash():
+    """
+    Test TopicAccessControlListPlugin.topic_ac correctly handles '#' wildcard.
+    """
+    assert (
+        TopicAccessControlListPlugin.topic_ac(
+            "topic/prefix/and/suffix", "topic/prefix/#"
+        )
+        is True
+    )

From c79ee4ceb802ef1af1c06cbe5d95ecbd982dcbd1 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:35:32 +1000
Subject: [PATCH 10/33] plugins.topic_checking: Return False if superclass
 refuses.

If `BaseTopicPlugin` returns `False`, we should propagate that failed
response in `TopicAccessControlListPlugin`.
---
 amqtt/plugins/topic_checking.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/amqtt/plugins/topic_checking.py b/amqtt/plugins/topic_checking.py
index 8624520b..08fa9b5b 100644
--- a/amqtt/plugins/topic_checking.py
+++ b/amqtt/plugins/topic_checking.py
@@ -94,3 +94,5 @@ async def topic_filtering(self, *args, **kwargs):
                     return False
             else:
                 return False
+        else:
+            return False

From 1bc361bc8d849a969739978765e2de16b3f5d137 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:42:06 +1000
Subject: [PATCH 11/33] plugins.topic_checking: Add remaining tests.

---
 tests/plugins/test_topic_checking.py | 212 +++++++++++++++++++++++++++
 1 file changed, 212 insertions(+)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 75829c84..e9aec431 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -195,6 +195,9 @@ async def test_taboo_admin_taboo_topic():
     assert len(context.logger.messages) == 0
 
 
+# TopicAccessControlListPlugin tests
+
+
 def test_topic_ac_not_match():
     """
     Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match.
@@ -252,3 +255,212 @@ def test_topic_ac_match_hash():
         )
         is True
     )
+
+
+@pytest.mark.asyncio
+async def test_taclp_empty_config():
+    """
+    Check TopicAccessControlListPlugin returns false if topic-check absent.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {}
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering()
+    assert authorised is False
+
+    # Should have printed a couple of warnings
+    assert len(context.logger.messages) == 2
+    assert context.logger.messages[0] == (
+        ("'topic-check' section not found in context configuration",),
+        {},
+    )
+    assert context.logger.messages[1] == (
+        ("'auth' section not found in context configuration",),
+        {},
+    )
+
+
+@pytest.mark.asyncio
+async def test_taclp_true_no_pub_acl():
+    """
+    Check TopicAccessControlListPlugin returns true if action=publish and no publish-acl given.
+    (This is for backward-compatibility with existing installations.)
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='publish', session=session, topic='a/topic')
+    assert authorised is True
+
+
+@pytest.mark.asyncio
+async def test_taclp_false_sub_no_topic():
+    """
+    Check TopicAccessControlListPlugin returns false user there is no topic.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'acl': {
+                    'anotheruser': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='')
+    assert authorised is False
+
+
+@pytest.mark.asyncio
+async def test_taclp_false_sub_unknown_user():
+    """
+    Check TopicAccessControlListPlugin returns false user is not listed in ACL.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'acl': {
+                    'anotheruser': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    assert authorised is False
+
+
+@pytest.mark.asyncio
+async def test_taclp_false_sub_no_permission():
+    """
+    Check TopicAccessControlListPlugin returns false if "acl" does not list allowed topic.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'acl': {
+                    'user': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='forbidden/topic')
+    assert authorised is False
+
+
+@pytest.mark.asyncio
+async def test_taclp_true_sub_permission():
+    """
+    Check TopicAccessControlListPlugin returns true if "acl" lists allowed topic.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'acl': {
+                    'user': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    assert authorised is True
+
+
+@pytest.mark.asyncio
+async def test_taclp_true_pub_permission():
+    """
+    Check TopicAccessControlListPlugin returns true if "publish-acl" lists allowed topic for publish action.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'publish-acl': {
+                    'user': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='publish', session=session, topic='allowed/topic')
+    assert authorised is True
+
+
+@pytest.mark.asyncio
+async def test_taclp_true_anon_sub_permission():
+    """
+    Check TopicAccessControlListPlugin handles anonymous users.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {
+            'topic-check': {
+                'enabled': True,
+                'acl': {
+                    'anonymous': [
+                        'allowed/topic',
+                        'another/allowed/topic/#'
+                    ]
+                }
+            }
+    }
+
+    session = Session()
+    session.username = None
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    assert authorised is True

From 3951ce0b442d7e280aa1ed2c90e0d985fd03aad9 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 17:43:02 +1000
Subject: [PATCH 12/33] plugin.topic_checking tests: Fix code formatting.

Yes, too lazy to hold down shift when hitting the `'` key.
---
 tests/plugins/test_topic_checking.py | 120 +++++++++++----------------
 1 file changed, 48 insertions(+), 72 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index e9aec431..e94c57b9 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -212,18 +212,14 @@ def test_topic_ac_not_match_longer_acl():
     """
     Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match and ACL topic is longer.
     """
-    assert (
-        TopicAccessControlListPlugin.topic_ac("topic", "topic/is/longer") is False
-    )
+    assert TopicAccessControlListPlugin.topic_ac("topic", "topic/is/longer") is False
 
 
 def test_topic_ac_not_match_longer_rq():
     """
     Test TopicAccessControlListPlugin.topic_ac returns false if topics do not match and RQ topic is longer.
     """
-    assert (
-        TopicAccessControlListPlugin.topic_ac("topic/is/longer", "topic") is False
-    )
+    assert TopicAccessControlListPlugin.topic_ac("topic/is/longer", "topic") is False
 
 
 def test_topic_ac_match_exact():
@@ -290,17 +286,15 @@ async def test_taclp_true_no_pub_acl():
     """
     context = BaseContext()
     context.logger = DummyLogger()
-    context.config = {
-            'topic-check': {
-                'enabled': True
-            }
-    }
+    context.config = {"topic-check": {"enabled": True}}
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='publish', session=session, topic='a/topic')
+    authorised = await plugin.topic_filtering(
+        action="publish", session=session, topic="a/topic"
+    )
     assert authorised is True
 
 
@@ -312,22 +306,19 @@ async def test_taclp_false_sub_no_topic():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'acl': {
-                    'anotheruser': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='')
+    authorised = await plugin.topic_filtering(
+        action="subscribe", session=session, topic=""
+    )
     assert authorised is False
 
 
@@ -339,22 +330,19 @@ async def test_taclp_false_sub_unknown_user():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'acl': {
-                    'anotheruser': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    authorised = await plugin.topic_filtering(
+        action="subscribe", session=session, topic="allowed/topic"
+    )
     assert authorised is False
 
 
@@ -366,22 +354,19 @@ async def test_taclp_false_sub_no_permission():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'acl': {
-                    'user': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='forbidden/topic')
+    authorised = await plugin.topic_filtering(
+        action="subscribe", session=session, topic="forbidden/topic"
+    )
     assert authorised is False
 
 
@@ -393,22 +378,19 @@ async def test_taclp_true_sub_permission():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'acl': {
-                    'user': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    authorised = await plugin.topic_filtering(
+        action="subscribe", session=session, topic="allowed/topic"
+    )
     assert authorised is True
 
 
@@ -420,22 +402,19 @@ async def test_taclp_true_pub_permission():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'publish-acl': {
-                    'user': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "publish-acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = "user"
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='publish', session=session, topic='allowed/topic')
+    authorised = await plugin.topic_filtering(
+        action="publish", session=session, topic="allowed/topic"
+    )
     assert authorised is True
 
 
@@ -447,20 +426,17 @@ async def test_taclp_true_anon_sub_permission():
     context = BaseContext()
     context.logger = DummyLogger()
     context.config = {
-            'topic-check': {
-                'enabled': True,
-                'acl': {
-                    'anonymous': [
-                        'allowed/topic',
-                        'another/allowed/topic/#'
-                    ]
-                }
-            }
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anonymous": ["allowed/topic", "another/allowed/topic/#"]},
+        }
     }
 
     session = Session()
     session.username = None
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(action='subscribe', session=session, topic='allowed/topic')
+    authorised = await plugin.topic_filtering(
+        action="subscribe", session=session, topic="allowed/topic"
+    )
     assert authorised is True

From 37407617b387913723ff68668caba78037c60ca9 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 18:48:48 +1000
Subject: [PATCH 13/33] test password file: Add some more users

---
 tests/plugins/passwd | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/plugins/passwd b/tests/plugins/passwd
index 07fda795..7749b33a 100644
--- a/tests/plugins/passwd
+++ b/tests/plugins/passwd
@@ -1,3 +1,7 @@
 # Test password file
 user:$6$1sSXVMBVKMF$uDStq59YfiuFxVF1Gi/.i/Li7Vwf5iTwg8LovLKmCvM5FsRNJM.OPWHhXwI2.4AscLZXSPQVxpIlX6laUl9570
-test_user:$6$.c9f9sAzs5YXX2de$GSdOi3iFwHJRCIJn1W63muDFQAL29yoFmU/TXcwDB42F2BZg3zcN5uKBM0.1PjwdMpWHRydbhXWSc3uWKSmKr.
\ No newline at end of file
+test_user:$6$.c9f9sAzs5YXX2de$GSdOi3iFwHJRCIJn1W63muDFQAL29yoFmU/TXcwDB42F2BZg3zcN5uKBM0.1PjwdMpWHRydbhXWSc3uWKSmKr.
+# Password for these is "test"
+user1:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1
+user2:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1
+user3:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1

From ea42422f783e7b01205462293b9982748d7325f4 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 18:49:10 +1000
Subject: [PATCH 14/33] test cases: Add fixture for ACL-enabled test server

This uses the real plug-ins to test the broker correctly responds to the
return values when publishing and subscribing.

The plug-ins themselves are tested elsewhere.
---
 tests/conftest.py | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/tests/conftest.py b/tests/conftest.py
index 5b58d47e..d1ff16da 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,4 +1,5 @@
 import unittest.mock
+import os.path
 
 import pytest
 
@@ -15,6 +16,39 @@
 }
 
 
+test_config_acl = {
+    "listeners": {
+        "default": {"type": "tcp", "bind": "127.0.0.1:1884", "max_connections": 10},
+    },
+    "sys_interval": 0,
+    "auth": {
+        "plugins": ["auth_file"],
+        "password-file": os.path.join(
+            os.path.dirname(os.path.realpath(__file__)),
+            'plugins',
+            'passwd'
+        )
+    },
+    "topic-check": {
+        "enabled": True,
+        "plugins": ["topic_acl"],
+        "acl": {
+            "user1": [
+                "public/#"
+            ],
+            "user2": [
+                "#"
+            ],
+        },
+        "publish-acl": {
+            "user1": [
+                "public/subtopic/#"
+            ]
+        },
+    }
+}
+
+
 @pytest.fixture(scope="function")
 def mock_plugin_manager():
     with unittest.mock.patch("amqtt.broker.PluginManager") as plugin_manager:
@@ -36,3 +70,17 @@ async def broker(mock_plugin_manager):
 
     if not broker.transitions.is_stopped():
         await broker.shutdown()
+
+
+@pytest.fixture(scope="function")
+async def acl_broker():
+    broker = amqtt.broker.Broker(test_config_acl, plugin_namespace="amqtt.broker.plugins")
+    await broker.start()
+    assert broker.transitions.is_started()
+    assert broker._sessions == {}
+    assert "default" in broker._servers
+
+    yield broker
+
+    if not broker.transitions.is_stopped():
+        await broker.shutdown()

From d9538930b5bc39f7e5bdc2b996bb1e13a27c5da7 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 6 Jun 2021 18:57:39 +1000
Subject: [PATCH 15/33] plugins.topic_checking: Clean up brackets.

As per review comments.
---
 amqtt/plugins/topic_checking.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/amqtt/plugins/topic_checking.py b/amqtt/plugins/topic_checking.py
index 08fa9b5b..e61a4b72 100644
--- a/amqtt/plugins/topic_checking.py
+++ b/amqtt/plugins/topic_checking.py
@@ -70,7 +70,7 @@ async def topic_filtering(self, *args, **kwargs):
             action = kwargs.get("action", None)
 
             # hbmqtt and older amqtt do not support publish filtering
-            if (action == "publish") and ("publish-acl" not in self.topic_config):
+            if action == "publish" and "publish-acl" not in self.topic_config:
                 # maintain backward compatibility, assume permitted
                 return True
 

From 6256048b8711d3e61edf01e2f084378d7102fad7 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Mon, 7 Jun 2021 08:20:43 +1000
Subject: [PATCH 16/33] plugins.topic_checking tests: Fix comment statement

---
 tests/plugins/test_topic_checking.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index e94c57b9..93bd7003 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -62,7 +62,7 @@ async def test_base_empty_config():
     authorised = plugin.topic_filtering()
     assert authorised is False
 
-    # Should NOT have printed warnings
+    # Should have printed just one warning
     assert len(context.logger.messages) == 1
     assert context.logger.messages[0] == (
         ("'auth' section not found in context configuration",),

From 47c407a619e364ad4a2280d788854424ef449679 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Mon, 7 Jun 2021 08:21:16 +1000
Subject: [PATCH 17/33] plugins.topic_checking tests: Drop copyright header

---
 tests/plugins/test_topic_checking.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 93bd7003..c051a16f 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -1,7 +1,3 @@
-# Copyright (c) 2015 Nicolas JOUANIN
-#
-# See the file license.txt for copying permission.
-
 import pytest
 
 from amqtt.plugins.manager import BaseContext

From f351627b879912487a1bafa869eb07e29a83f612 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Mon, 7 Jun 2021 08:22:05 +1000
Subject: [PATCH 18/33] broker tests: Add system test for publish authorisation

---
 tests/test_broker.py | 98 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tests/test_broker.py b/tests/test_broker.py
index f9689e5d..2b5ebe6e 100644
--- a/tests/test_broker.py
+++ b/tests/test_broker.py
@@ -269,6 +269,104 @@ async def test_client_publish(broker, mock_plugin_manager):
     )
 
 
+@pytest.mark.asyncio
+async def test_client_publish_acl_permitted(acl_broker):
+    sub_client = MQTTClient()
+    ret = await sub_client.connect("mqtt://user2:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret = await sub_client.subscribe(
+        [("public/subtopic/test", QOS_0)]
+    )
+    assert ret == [QOS_0]
+
+    pub_client = MQTTClient()
+    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)
+
+    message = await sub_client.deliver_message(timeout=1)
+    await pub_client.disconnect()
+    await sub_client.disconnect()
+
+    assert message is not None
+    assert message.topic == "public/subtopic/test"
+    assert message.data == b"data"
+    assert message.qos == QOS_0
+
+
+@pytest.mark.asyncio
+async def test_client_publish_acl_forbidden(acl_broker):
+    sub_client = MQTTClient()
+    ret = await sub_client.connect("mqtt://user2:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret = await sub_client.subscribe(
+        [("public/forbidden/test", QOS_0)]
+    )
+    assert ret == [QOS_0]
+
+    pub_client = MQTTClient()
+    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret_message = await pub_client.publish("public/forbidden/test", b"data", QOS_0)
+
+    try:
+        await sub_client.deliver_message(timeout=1)
+        assert False, 'Should not have worked'
+    except asyncio.exceptions.TimeoutError:
+        pass
+
+    await pub_client.disconnect()
+    await sub_client.disconnect()
+
+
+@pytest.mark.asyncio
+async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
+    sub_client1 = MQTTClient()
+    ret = await sub_client1.connect("mqtt://user2:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    sub_client2 = MQTTClient()
+    ret = await sub_client2.connect("mqtt://user3:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret = await sub_client1.subscribe(
+        [("public/subtopic/test", QOS_0)]
+    )
+    assert ret == [QOS_0]
+
+    ret = await sub_client2.subscribe(
+        [("public/subtopic/test", QOS_0)]
+    )
+    assert ret == [0x80]
+
+    pub_client = MQTTClient()
+    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    assert ret == 0
+
+    ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)
+
+    message = await sub_client1.deliver_message(timeout=1)
+
+    try:
+        await sub_client2.deliver_message(timeout=1)
+        assert False, 'Should not have worked'
+    except asyncio.exceptions.TimeoutError:
+        pass
+
+    await pub_client.disconnect()
+    await sub_client1.disconnect()
+    await sub_client2.disconnect()
+
+    assert message is not None
+    assert message.topic == "public/subtopic/test"
+    assert message.data == b"data"
+    assert message.qos == QOS_0
+
+
 @pytest.mark.asyncio
 async def test_client_publish_dup(broker, event_loop):
     conn_reader, conn_writer = await asyncio.open_connection(

From 0e0313474e26b140eda7810451a524de5686ac64 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Mon, 7 Jun 2021 08:25:19 +1000
Subject: [PATCH 19/33] plugins.topic_checking tests: Add tests for
 enabled=False

---
 tests/plugins/test_topic_checking.py | 56 ++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index c051a16f..1d8cac57 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -66,6 +66,23 @@ async def test_base_empty_config():
     )
 
 
+@pytest.mark.asyncio
+async def test_base_disabled_config():
+    """
+    Check BaseTopicPlugin returns true if disabled. (it doesn't actually check)
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {"topic-check": {"enabled": False}}
+
+    plugin = BaseTopicPlugin(context)
+    authorised = plugin.topic_filtering()
+    assert authorised is True
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+
 @pytest.mark.asyncio
 async def test_base_enabled_config():
     """
@@ -111,6 +128,26 @@ async def test_taboo_empty_config():
     )
 
 
+@pytest.mark.asyncio
+async def test_taboo_not_taboo_topic():
+    """
+    Check TopicTabooPlugin returns true if checking disabled.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {"topic-check": {"enabled": False}}
+
+    session = Session()
+    session.username = "anybody"
+
+    plugin = TopicTabooPlugin(context)
+    authorised = await plugin.topic_filtering(session=session, topic="not/prohibited")
+    assert authorised is True
+
+    # Should NOT have printed warnings
+    assert len(context.logger.messages) == 0
+
+
 @pytest.mark.asyncio
 async def test_taboo_not_taboo_topic():
     """
@@ -274,6 +311,25 @@ async def test_taclp_empty_config():
     )
 
 
+@pytest.mark.asyncio
+async def test_taclp_true_disabled():
+    """
+    Check TopicAccessControlListPlugin returns true if topic checking is disabled.
+    """
+    context = BaseContext()
+    context.logger = DummyLogger()
+    context.config = {"topic-check": {"enabled": False}}
+
+    session = Session()
+    session.username = "user"
+
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action="publish", session=session, topic="a/topic"
+    )
+    assert authorised is True
+
+
 @pytest.mark.asyncio
 async def test_taclp_true_no_pub_acl():
     """

From 2536da82a5c1ec88663a42b06023466f8e27ac08 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Mon, 7 Jun 2021 08:27:30 +1000
Subject: [PATCH 20/33] plugins.topic_checking tests: Clean up `assert`/`await`
 calls

---
 tests/plugins/test_topic_checking.py | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 1d8cac57..9bd2f693 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -113,8 +113,7 @@ async def test_taboo_empty_config():
     context.config = {}
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering()
-    assert authorised is False
+    assert (await plugin.topic_filtering()) is False
 
     # Should have printed a couple of warnings
     assert len(context.logger.messages) == 2
@@ -141,8 +140,9 @@ async def test_taboo_not_taboo_topic():
     session.username = "anybody"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(session=session, topic="not/prohibited")
-    assert authorised is True
+    assert (
+        await plugin.topic_filtering(session=session, topic="not/prohibited")
+    ) is True
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
@@ -161,8 +161,9 @@ async def test_taboo_not_taboo_topic():
     session.username = "anybody"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(session=session, topic="not/prohibited")
-    assert authorised is True
+    assert (
+        await plugin.topic_filtering(session=session, topic="not/prohibited")
+    ) is True
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
@@ -181,8 +182,7 @@ async def test_taboo_anon_taboo_topic():
     session.username = ""
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
-    assert authorised is False
+    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
@@ -201,8 +201,7 @@ async def test_taboo_notadmin_taboo_topic():
     session.username = "notadmin"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
-    assert authorised is False
+    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
@@ -221,8 +220,7 @@ async def test_taboo_admin_taboo_topic():
     session.username = "admin"
 
     plugin = TopicTabooPlugin(context)
-    authorised = await plugin.topic_filtering(session=session, topic="prohibited")
-    assert authorised is True
+    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is True
 
     # Should NOT have printed warnings
     assert len(context.logger.messages) == 0
@@ -296,8 +294,7 @@ async def test_taclp_empty_config():
     context.config = {}
 
     plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering()
-    assert authorised is False
+    assert (await plugin.topic_filtering()) is False
 
     # Should have printed a couple of warnings
     assert len(context.logger.messages) == 2

From 2077ecf1d998227de0120b85cf1fb1867c94bf75 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 20 Jun 2021 08:24:38 +1000
Subject: [PATCH 21/33] amqtt.broker: Use Enum for topic checking action

---
 amqtt/broker.py                 | 8 +++++++-
 amqtt/plugins/topic_checking.py | 9 ++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index d1e1e94c..6713d475 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -9,6 +9,7 @@
 import re
 from asyncio import CancelledError
 from collections import deque
+from enum import Enum
 
 from functools import partial
 from transitions import Machine, MachineError
@@ -43,6 +44,11 @@
 EVENT_BROKER_MESSAGE_RECEIVED = "broker_message_received"
 
 
+class Action(Enum):
+    subscribe = 'subscribe'
+    publish = 'publish'
+
+
 class BrokerException(Exception):
     pass
 
@@ -714,7 +720,7 @@ async def authenticate(self, session: Session, listener):
         # If all plugins returned True, authentication is success
         return auth_result
 
-    async def topic_filtering(self, session: Session, topic, action: str):
+    async def topic_filtering(self, session: Session, topic, action: Action):
         """
         This method call the topic_filtering method on registered plugins to check that the subscription is allowed.
         User is considered allowed if all plugins called return True.
diff --git a/amqtt/plugins/topic_checking.py b/amqtt/plugins/topic_checking.py
index e61a4b72..aa2179f1 100644
--- a/amqtt/plugins/topic_checking.py
+++ b/amqtt/plugins/topic_checking.py
@@ -1,3 +1,6 @@
+from ..broker import Action
+
+
 class BaseTopicPlugin:
     def __init__(self, context):
         self.context = context
@@ -70,7 +73,7 @@ async def topic_filtering(self, *args, **kwargs):
             action = kwargs.get("action", None)
 
             # hbmqtt and older amqtt do not support publish filtering
-            if action == "publish" and "publish-acl" not in self.topic_config:
+            if action == Action.publish and "publish-acl" not in self.topic_config:
                 # maintain backward compatibility, assume permitted
                 return True
 
@@ -79,9 +82,9 @@ async def topic_filtering(self, *args, **kwargs):
                 if username is None:
                     username = "anonymous"
 
-                if action == "publish":
+                if action == Action.publish:
                     acl = self.topic_config["publish-acl"]
-                elif action == "subscribe":
+                elif action == Action.subscribe:
                     acl = self.topic_config["acl"]
 
                 allowed_topics = acl.get(username, None)

From 47893612c34e67b1ac7dc14afc716fdd21ee33d0 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 15:24:43 +1000
Subject: [PATCH 22/33] plugins.topic_checking tests: Fix tests broken by Enum
 change.

---
 tests/plugins/test_topic_checking.py | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 9bd2f693..3dfcb44a 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -5,6 +5,7 @@
     BaseTopicPlugin,
     TopicTabooPlugin,
     TopicAccessControlListPlugin,
+    Action,
 )
 from amqtt.session import Session
 
@@ -322,7 +323,7 @@ async def test_taclp_true_disabled():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="publish", session=session, topic="a/topic"
+        action=Action.publish, session=session, topic="a/topic"
     )
     assert authorised is True
 
@@ -342,7 +343,7 @@ async def test_taclp_true_no_pub_acl():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="publish", session=session, topic="a/topic"
+        action=Action.publish, session=session, topic="a/topic"
     )
     assert authorised is True
 
@@ -366,7 +367,7 @@ async def test_taclp_false_sub_no_topic():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="subscribe", session=session, topic=""
+        action=Action.subscribe, session=session, topic=""
     )
     assert authorised is False
 
@@ -390,7 +391,7 @@ async def test_taclp_false_sub_unknown_user():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="subscribe", session=session, topic="allowed/topic"
+        action=Action.subscribe, session=session, topic="allowed/topic"
     )
     assert authorised is False
 
@@ -414,7 +415,7 @@ async def test_taclp_false_sub_no_permission():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="subscribe", session=session, topic="forbidden/topic"
+        action=Action.subscribe, session=session, topic="forbidden/topic"
     )
     assert authorised is False
 
@@ -438,7 +439,7 @@ async def test_taclp_true_sub_permission():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="subscribe", session=session, topic="allowed/topic"
+        action=Action.subscribe, session=session, topic="allowed/topic"
     )
     assert authorised is True
 
@@ -462,7 +463,7 @@ async def test_taclp_true_pub_permission():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="publish", session=session, topic="allowed/topic"
+        action=Action.publish, session=session, topic="allowed/topic"
     )
     assert authorised is True
 
@@ -486,6 +487,6 @@ async def test_taclp_true_anon_sub_permission():
 
     plugin = TopicAccessControlListPlugin(context)
     authorised = await plugin.topic_filtering(
-        action="subscribe", session=session, topic="allowed/topic"
+        action=Action.subscribe, session=session, topic="allowed/topic"
     )
     assert authorised is True

From c694765bcb697d43ff421f077621fa6d8947e2c3 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 15:01:42 +1000
Subject: [PATCH 23/33] poetry/pyproject: Pull in pytest-logdog

This is a `py.test` plug-in that allows us to test logging in test
cases.  Seems like an ideal candidate for a logger fixture.
---
 poetry.lock    | 17 ++++++++++++++++-
 pyproject.toml |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/poetry.lock b/poetry.lock
index 49dcd837..1a789525 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -429,6 +429,17 @@ pytest = ">=4.6"
 [package.extras]
 testing = ["fields", "hunter", "process-tests (==2.0.2)", "six", "pytest-xdist", "virtualenv"]
 
+[[package]]
+name = "pytest-logdog"
+version = "0.1.0"
+description = "Pytest plugin to test logging"
+category = "dev"
+optional = false
+python-versions = ">=3.7"
+
+[package.dependencies]
+pytest = ">=6.2.0"
+
 [[package]]
 name = "pyyaml"
 version = "5.4.1"
@@ -565,7 +576,7 @@ ci = ["coveralls"]
 [metadata]
 lock-version = "1.1"
 python-versions = "^3.7"
-content-hash = "8eea41d1ef5f01f58a973a749a766740aa529fc0b94a8fcf71c6e91babdd6613"
+content-hash = "2030ca9b6173701a0b94bd85c8c7b26f30ea9f7ad5be320a2b97c51c847afb62"
 
 [metadata.files]
 appdirs = [
@@ -800,6 +811,10 @@ pytest-cov = [
     {file = "pytest-cov-2.11.1.tar.gz", hash = "sha256:359952d9d39b9f822d9d29324483e7ba04a3a17dd7d05aa6beb7ea01e359e5f7"},
     {file = "pytest_cov-2.11.1-py2.py3-none-any.whl", hash = "sha256:bdb9fdb0b85a7cc825269a4c56b48ccaa5c7e365054b6038772c32ddcdc969da"},
 ]
+pytest-logdog = [
+    {file = "pytest-logdog-0.1.0.tar.gz", hash = "sha256:b84aca02b6b609bda8bfcd6d0207a428b146cd706d14c7095a3ba79429ab534b"},
+    {file = "pytest_logdog-0.1.0-py3-none-any.whl", hash = "sha256:4d5a4c46442ca7da73b1cf6c9ebea144958a0a6258ba19ad7bf877dec22400e8"},
+]
 pyyaml = [
     {file = "PyYAML-5.4.1-cp27-cp27m-macosx_10_9_x86_64.whl", hash = "sha256:3b2b1824fe7112845700f815ff6a489360226a5609b96ec2190a45e62a9fc922"},
     {file = "PyYAML-5.4.1-cp27-cp27m-win32.whl", hash = "sha256:129def1b7c1bf22faffd67b8f3724645203b79d8f4cc81f674654d9902cb4393"},
diff --git a/pyproject.toml b/pyproject.toml
index 244b6612..3564b571 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -42,6 +42,7 @@ pylint = "^2.7.2"
 black = "^20.8b1"
 flake8 = "^3.9.0"
 hypothesis = "^6.10.0"
+pytest-logdog = "^0.1.0"
 
 
 [tool.poetry.scripts]

From ef1a109aa1e470b1f3476b45ff6faea2adf95df6 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 15:26:26 +1000
Subject: [PATCH 24/33] test config: Enable `pytest_logdog` plug-in

---
 tests/conftest.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/conftest.py b/tests/conftest.py
index d1ff16da..bf0ca9f9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -5,6 +5,8 @@
 
 import amqtt.broker
 
+pytest_plugins = ["pytest_logdog"]
+
 test_config = {
     "listeners": {
         "default": {"type": "tcp", "bind": "127.0.0.1:1883", "max_connections": 10},

From de8575fea4b093f3e56f3f872d915c07b664bc58 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 15:56:34 +1000
Subject: [PATCH 25/33] plugins.topic_checking tests: Replace DummyLogger with
 `logdog` plug-in.

---
 tests/plugins/test_topic_checking.py | 517 ++++++++++++++-------------
 1 file changed, 263 insertions(+), 254 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 3dfcb44a..2e4f80fb 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -1,4 +1,5 @@
 import pytest
+import logging
 
 from amqtt.plugins.manager import BaseContext
 from amqtt.plugins.topic_checking import (
@@ -10,221 +11,225 @@
 from amqtt.session import Session
 
 
-class DummyLogger(object):
-    def __init__(self):
-        self.messages = []
-
-    def warning(self, *args, **kwargs):
-        self.messages.append((args, kwargs))
-
-
 # Base plug-in object
 
 
 @pytest.mark.asyncio
-async def test_base_no_config():
+async def test_base_no_config(logdog):
     """
     Check BaseTopicPlugin returns false if no topic-check is present.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {}
 
-    plugin = BaseTopicPlugin(context)
-    authorised = plugin.topic_filtering()
-    assert authorised is False
+        plugin = BaseTopicPlugin(context)
+        authorised = plugin.topic_filtering()
+        assert authorised is False
 
-    # Should have printed a couple of warnings
-    assert len(context.logger.messages) == 2
-    assert context.logger.messages[0] == (
-        ("'topic-check' section not found in context configuration",),
-        {},
-    )
-    assert context.logger.messages[1] == (
-        ("'auth' section not found in context configuration",),
-        {},
-    )
+        # Should have printed a couple of warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 2
+        assert log_records[0].levelno == logging.WARN
+        assert log_records[0].message == "'topic-check' section not found in context configuration"
+
+        assert log_records[1].levelno == logging.WARN
+        assert log_records[1].message == "'auth' section not found in context configuration"
+        assert pile.is_empty()
 
 
 @pytest.mark.asyncio
-async def test_base_empty_config():
+async def test_base_empty_config(logdog):
     """
     Check BaseTopicPlugin returns false if topic-check is empty.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {}}
 
-    plugin = BaseTopicPlugin(context)
-    authorised = plugin.topic_filtering()
-    assert authorised is False
+        plugin = BaseTopicPlugin(context)
+        authorised = plugin.topic_filtering()
+        assert authorised is False
 
-    # Should have printed just one warning
-    assert len(context.logger.messages) == 1
-    assert context.logger.messages[0] == (
-        ("'auth' section not found in context configuration",),
-        {},
-    )
+        # Should have printed just one warning
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 1
+        assert log_records[0].levelno == logging.WARN
+        assert log_records[0].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
-async def test_base_disabled_config():
+async def test_base_disabled_config(logdog):
     """
     Check BaseTopicPlugin returns true if disabled. (it doesn't actually check)
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": False}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": False}}
 
-    plugin = BaseTopicPlugin(context)
-    authorised = plugin.topic_filtering()
-    assert authorised is True
+        plugin = BaseTopicPlugin(context)
+        authorised = plugin.topic_filtering()
+        assert authorised is True
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
-async def test_base_enabled_config():
+async def test_base_enabled_config(logdog):
     """
     Check BaseTopicPlugin returns true if enabled.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    plugin = BaseTopicPlugin(context)
-    authorised = plugin.topic_filtering()
-    assert authorised is True
+        plugin = BaseTopicPlugin(context)
+        authorised = plugin.topic_filtering()
+        assert authorised is True
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 # Taboo plug-in
 
 
 @pytest.mark.asyncio
-async def test_taboo_empty_config():
+async def test_taboo_empty_config(logdog):
     """
     Check TopicTabooPlugin returns false if topic-check absent.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {}
 
-    plugin = TopicTabooPlugin(context)
-    assert (await plugin.topic_filtering()) is False
+        plugin = TopicTabooPlugin(context)
+        assert (await plugin.topic_filtering()) is False
 
-    # Should have printed a couple of warnings
-    assert len(context.logger.messages) == 2
-    assert context.logger.messages[0] == (
-        ("'topic-check' section not found in context configuration",),
-        {},
-    )
-    assert context.logger.messages[1] == (
-        ("'auth' section not found in context configuration",),
-        {},
-    )
+        # Should have printed a couple of warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 2
+        assert log_records[0].levelno == logging.WARN
+        assert log_records[0].message == "'topic-check' section not found in context configuration"
+        assert log_records[1].levelno == logging.WARN
+        assert log_records[1].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
-async def test_taboo_not_taboo_topic():
+async def test_taboo_not_taboo_topic(logdog):
     """
     Check TopicTabooPlugin returns true if checking disabled.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": False}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": False}}
 
-    session = Session()
-    session.username = "anybody"
+        session = Session()
+        session.username = "anybody"
 
-    plugin = TopicTabooPlugin(context)
-    assert (
-        await plugin.topic_filtering(session=session, topic="not/prohibited")
-    ) is True
+        plugin = TopicTabooPlugin(context)
+        assert (
+            await plugin.topic_filtering(session=session, topic="not/prohibited")
+        ) is True
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
-async def test_taboo_not_taboo_topic():
+async def test_taboo_not_taboo_topic(logdog):
     """
     Check TopicTabooPlugin returns true if topic not taboo
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    session = Session()
-    session.username = "anybody"
+        session = Session()
+        session.username = "anybody"
 
-    plugin = TopicTabooPlugin(context)
-    assert (
-        await plugin.topic_filtering(session=session, topic="not/prohibited")
-    ) is True
+        plugin = TopicTabooPlugin(context)
+        assert (
+            await plugin.topic_filtering(session=session, topic="not/prohibited")
+        ) is True
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
-async def test_taboo_anon_taboo_topic():
+async def test_taboo_anon_taboo_topic(logdog):
     """
     Check TopicTabooPlugin returns false if topic is taboo and session is anonymous.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    session = Session()
-    session.username = ""
+        session = Session()
+        session.username = ""
 
-    plugin = TopicTabooPlugin(context)
-    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
+        plugin = TopicTabooPlugin(context)
+        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
-async def test_taboo_notadmin_taboo_topic():
+async def test_taboo_notadmin_taboo_topic(logdog):
     """
     Check TopicTabooPlugin returns false if topic is taboo and user is not "admin".
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    session = Session()
-    session.username = "notadmin"
+        session = Session()
+        session.username = "notadmin"
 
-    plugin = TopicTabooPlugin(context)
-    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
+        plugin = TopicTabooPlugin(context)
+        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
-async def test_taboo_admin_taboo_topic():
+async def test_taboo_admin_taboo_topic(logdog):
     """
     Check TopicTabooPlugin returns true if topic is taboo and user is "admin".
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    session = Session()
-    session.username = "admin"
+        session = Session()
+        session.username = "admin"
 
-    plugin = TopicTabooPlugin(context)
-    assert (await plugin.topic_filtering(session=session, topic="prohibited")) is True
+        plugin = TopicTabooPlugin(context)
+        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is True
 
-    # Should NOT have printed warnings
-    assert len(context.logger.messages) == 0
+        # Should NOT have printed warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 0
 
 
 # TopicAccessControlListPlugin tests
@@ -286,207 +291,211 @@ def test_topic_ac_match_hash():
 
 
 @pytest.mark.asyncio
-async def test_taclp_empty_config():
+async def test_taclp_empty_config(logdog):
     """
     Check TopicAccessControlListPlugin returns false if topic-check absent.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {}
 
-    plugin = TopicAccessControlListPlugin(context)
-    assert (await plugin.topic_filtering()) is False
+        plugin = TopicAccessControlListPlugin(context)
+        assert (await plugin.topic_filtering()) is False
 
-    # Should have printed a couple of warnings
-    assert len(context.logger.messages) == 2
-    assert context.logger.messages[0] == (
-        ("'topic-check' section not found in context configuration",),
-        {},
-    )
-    assert context.logger.messages[1] == (
-        ("'auth' section not found in context configuration",),
-        {},
-    )
+        # Should have printed a couple of warnings
+        log_records = list(pile.drain(name='testlog'))
+        assert len(log_records) == 2
+        assert log_records[0].message == "'topic-check' section not found in context configuration"
+        assert log_records[1].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
-async def test_taclp_true_disabled():
+async def test_taclp_true_disabled(logdog):
     """
     Check TopicAccessControlListPlugin returns true if topic checking is disabled.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": False}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": False}}
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.publish, session=session, topic="a/topic"
-    )
-    assert authorised is True
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.publish, session=session, topic="a/topic"
+        )
+        assert authorised is True
 
 
 @pytest.mark.asyncio
-async def test_taclp_true_no_pub_acl():
+async def test_taclp_true_no_pub_acl(logdog):
     """
     Check TopicAccessControlListPlugin returns true if action=publish and no publish-acl given.
     (This is for backward-compatibility with existing installations.)
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {"topic-check": {"enabled": True}}
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {"topic-check": {"enabled": True}}
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.publish, session=session, topic="a/topic"
-    )
-    assert authorised is True
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.publish, session=session, topic="a/topic"
+        )
+        assert authorised is True
 
 
 @pytest.mark.asyncio
-async def test_taclp_false_sub_no_topic():
+async def test_taclp_false_sub_no_topic(logdog):
     """
     Check TopicAccessControlListPlugin returns false user there is no topic.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.subscribe, session=session, topic=""
-    )
-    assert authorised is False
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.subscribe, session=session, topic=""
+        )
+        assert authorised is False
 
 
 @pytest.mark.asyncio
-async def test_taclp_false_sub_unknown_user():
+async def test_taclp_false_sub_unknown_user(logdog):
     """
     Check TopicAccessControlListPlugin returns false user is not listed in ACL.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.subscribe, session=session, topic="allowed/topic"
-    )
-    assert authorised is False
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.subscribe, session=session, topic="allowed/topic"
+        )
+        assert authorised is False
 
 
 @pytest.mark.asyncio
-async def test_taclp_false_sub_no_permission():
+async def test_taclp_false_sub_no_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns false if "acl" does not list allowed topic.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.subscribe, session=session, topic="forbidden/topic"
-    )
-    assert authorised is False
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.subscribe, session=session, topic="forbidden/topic"
+        )
+        assert authorised is False
 
 
 @pytest.mark.asyncio
-async def test_taclp_true_sub_permission():
+async def test_taclp_true_sub_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns true if "acl" lists allowed topic.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.subscribe, session=session, topic="allowed/topic"
-    )
-    assert authorised is True
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.subscribe, session=session, topic="allowed/topic"
+        )
+        assert authorised is True
 
 
 @pytest.mark.asyncio
-async def test_taclp_true_pub_permission():
+async def test_taclp_true_pub_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns true if "publish-acl" lists allowed topic for publish action.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "publish-acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "publish-acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = "user"
+        session = Session()
+        session.username = "user"
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.publish, session=session, topic="allowed/topic"
-    )
-    assert authorised is True
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.publish, session=session, topic="allowed/topic"
+        )
+        assert authorised is True
 
 
 @pytest.mark.asyncio
-async def test_taclp_true_anon_sub_permission():
+async def test_taclp_true_anon_sub_permission(logdog):
     """
     Check TopicAccessControlListPlugin handles anonymous users.
     """
-    context = BaseContext()
-    context.logger = DummyLogger()
-    context.config = {
-        "topic-check": {
-            "enabled": True,
-            "acl": {"anonymous": ["allowed/topic", "another/allowed/topic/#"]},
+    with logdog() as pile:
+        context = BaseContext()
+        context.logger = logging.getLogger('testlog')
+        context.config = {
+            "topic-check": {
+                "enabled": True,
+                "acl": {"anonymous": ["allowed/topic", "another/allowed/topic/#"]},
+            }
         }
-    }
 
-    session = Session()
-    session.username = None
+        session = Session()
+        session.username = None
 
-    plugin = TopicAccessControlListPlugin(context)
-    authorised = await plugin.topic_filtering(
-        action=Action.subscribe, session=session, topic="allowed/topic"
-    )
-    assert authorised is True
+        plugin = TopicAccessControlListPlugin(context)
+        authorised = await plugin.topic_filtering(
+            action=Action.subscribe, session=session, topic="allowed/topic"
+        )
+        assert authorised is True

From cfe7ff5f82f0adc87e875eb2ae5cafa517a5ad00 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 16:08:53 +1000
Subject: [PATCH 26/33] plugins.topic_checking tests: Run code formatting tool.

---
 tests/plugins/test_topic_checking.py | 107 +++++++++++++++++----------
 1 file changed, 67 insertions(+), 40 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 2e4f80fb..a198c38a 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -21,7 +21,7 @@ async def test_base_no_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {}
 
         plugin = BaseTopicPlugin(context)
@@ -29,13 +29,19 @@ async def test_base_no_config(logdog):
         assert authorised is False
 
         # Should have printed a couple of warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 2
         assert log_records[0].levelno == logging.WARN
-        assert log_records[0].message == "'topic-check' section not found in context configuration"
+        assert (
+            log_records[0].message
+            == "'topic-check' section not found in context configuration"
+        )
 
         assert log_records[1].levelno == logging.WARN
-        assert log_records[1].message == "'auth' section not found in context configuration"
+        assert (
+            log_records[1].message
+            == "'auth' section not found in context configuration"
+        )
         assert pile.is_empty()
 
 
@@ -46,7 +52,7 @@ async def test_base_empty_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {}}
 
         plugin = BaseTopicPlugin(context)
@@ -54,10 +60,13 @@ async def test_base_empty_config(logdog):
         assert authorised is False
 
         # Should have printed just one warning
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 1
         assert log_records[0].levelno == logging.WARN
-        assert log_records[0].message == "'auth' section not found in context configuration"
+        assert (
+            log_records[0].message
+            == "'auth' section not found in context configuration"
+        )
 
 
 @pytest.mark.asyncio
@@ -67,7 +76,7 @@ async def test_base_disabled_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": False}}
 
         plugin = BaseTopicPlugin(context)
@@ -75,7 +84,7 @@ async def test_base_disabled_config(logdog):
         assert authorised is True
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -86,7 +95,7 @@ async def test_base_enabled_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         plugin = BaseTopicPlugin(context)
@@ -94,7 +103,7 @@ async def test_base_enabled_config(logdog):
         assert authorised is True
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -108,19 +117,25 @@ async def test_taboo_empty_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {}
 
         plugin = TopicTabooPlugin(context)
         assert (await plugin.topic_filtering()) is False
 
         # Should have printed a couple of warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 2
         assert log_records[0].levelno == logging.WARN
-        assert log_records[0].message == "'topic-check' section not found in context configuration"
+        assert (
+            log_records[0].message
+            == "'topic-check' section not found in context configuration"
+        )
         assert log_records[1].levelno == logging.WARN
-        assert log_records[1].message == "'auth' section not found in context configuration"
+        assert (
+            log_records[1].message
+            == "'auth' section not found in context configuration"
+        )
 
 
 @pytest.mark.asyncio
@@ -130,7 +145,7 @@ async def test_taboo_not_taboo_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": False}}
 
         session = Session()
@@ -142,7 +157,7 @@ async def test_taboo_not_taboo_topic(logdog):
         ) is True
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -153,7 +168,7 @@ async def test_taboo_not_taboo_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         session = Session()
@@ -165,7 +180,7 @@ async def test_taboo_not_taboo_topic(logdog):
         ) is True
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -176,17 +191,19 @@ async def test_taboo_anon_taboo_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         session = Session()
         session.username = ""
 
         plugin = TopicTabooPlugin(context)
-        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
+        assert (
+            await plugin.topic_filtering(session=session, topic="prohibited")
+        ) is False
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -197,17 +214,19 @@ async def test_taboo_notadmin_taboo_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         session = Session()
         session.username = "notadmin"
 
         plugin = TopicTabooPlugin(context)
-        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is False
+        assert (
+            await plugin.topic_filtering(session=session, topic="prohibited")
+        ) is False
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -218,17 +237,19 @@ async def test_taboo_admin_taboo_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         session = Session()
         session.username = "admin"
 
         plugin = TopicTabooPlugin(context)
-        assert (await plugin.topic_filtering(session=session, topic="prohibited")) is True
+        assert (
+            await plugin.topic_filtering(session=session, topic="prohibited")
+        ) is True
 
         # Should NOT have printed warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 0
 
 
@@ -297,17 +318,23 @@ async def test_taclp_empty_config(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {}
 
         plugin = TopicAccessControlListPlugin(context)
         assert (await plugin.topic_filtering()) is False
 
         # Should have printed a couple of warnings
-        log_records = list(pile.drain(name='testlog'))
+        log_records = list(pile.drain(name="testlog"))
         assert len(log_records) == 2
-        assert log_records[0].message == "'topic-check' section not found in context configuration"
-        assert log_records[1].message == "'auth' section not found in context configuration"
+        assert (
+            log_records[0].message
+            == "'topic-check' section not found in context configuration"
+        )
+        assert (
+            log_records[1].message
+            == "'auth' section not found in context configuration"
+        )
 
 
 @pytest.mark.asyncio
@@ -317,7 +344,7 @@ async def test_taclp_true_disabled(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": False}}
 
         session = Session()
@@ -338,7 +365,7 @@ async def test_taclp_true_no_pub_acl(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {"topic-check": {"enabled": True}}
 
         session = Session()
@@ -358,7 +385,7 @@ async def test_taclp_false_sub_no_topic(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,
@@ -383,7 +410,7 @@ async def test_taclp_false_sub_unknown_user(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,
@@ -408,7 +435,7 @@ async def test_taclp_false_sub_no_permission(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,
@@ -433,7 +460,7 @@ async def test_taclp_true_sub_permission(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,
@@ -458,7 +485,7 @@ async def test_taclp_true_pub_permission(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,
@@ -483,7 +510,7 @@ async def test_taclp_true_anon_sub_permission(logdog):
     """
     with logdog() as pile:
         context = BaseContext()
-        context.logger = logging.getLogger('testlog')
+        context.logger = logging.getLogger("testlog")
         context.config = {
             "topic-check": {
                 "enabled": True,

From 3c3ab5097b04df78540ab1bd29f7f6b693b191aa Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Sun, 4 Jul 2021 16:19:53 +1000
Subject: [PATCH 27/33] broker: Pass through enum values not strings.

---
 amqtt/broker.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index 6713d475..99878e9c 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -629,7 +629,7 @@ async def client_connected(
 
                     # See if the user is allowed to publish to this topic.
                     permitted = await self.topic_filtering(
-                            client_session, topic=app_message.topic, action='publish'
+                            client_session, topic=app_message.topic, action=Action.publish
                     )
                     if not permitted:
                         self.logger.info(
@@ -792,7 +792,7 @@ async def add_subscription(self, subscription, session):
                         # [MQTT-4.7.1-3] + wildcard character must occupy entire level
                         return 0x80
             # Check if the client is authorised to connect to the topic
-            permitted = await self.topic_filtering(session, topic=a_filter, action='subscribe')
+            permitted = await self.topic_filtering(session, topic=a_filter, action=Action.subscribe)
             if not permitted:
                 return 0x80
             qos = subscription[1]

From f3776058d5eb8fc01d597126d63f6b05b2cc516c Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 07:52:01 +1000
Subject: [PATCH 28/33] plugins.topic_checking: De-indent logging checks

As pointed out, the `logdog` docs do the checks outside of the context
manager scope, so we should do so as well.
---
 tests/plugins/test_topic_checking.py | 134 +++++++++++++--------------
 1 file changed, 67 insertions(+), 67 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index a198c38a..36192f11 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -28,21 +28,21 @@ async def test_base_no_config(logdog):
         authorised = plugin.topic_filtering()
         assert authorised is False
 
-        # Should have printed a couple of warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 2
-        assert log_records[0].levelno == logging.WARN
-        assert (
-            log_records[0].message
-            == "'topic-check' section not found in context configuration"
-        )
+    # Should have printed a couple of warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 2
+    assert log_records[0].levelno == logging.WARN
+    assert (
+        log_records[0].message
+        == "'topic-check' section not found in context configuration"
+    )
 
-        assert log_records[1].levelno == logging.WARN
-        assert (
-            log_records[1].message
-            == "'auth' section not found in context configuration"
-        )
-        assert pile.is_empty()
+    assert log_records[1].levelno == logging.WARN
+    assert (
+        log_records[1].message
+        == "'auth' section not found in context configuration"
+    )
+    assert pile.is_empty()
 
 
 @pytest.mark.asyncio
@@ -59,14 +59,14 @@ async def test_base_empty_config(logdog):
         authorised = plugin.topic_filtering()
         assert authorised is False
 
-        # Should have printed just one warning
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 1
-        assert log_records[0].levelno == logging.WARN
-        assert (
-            log_records[0].message
-            == "'auth' section not found in context configuration"
-        )
+    # Should have printed just one warning
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 1
+    assert log_records[0].levelno == logging.WARN
+    assert (
+        log_records[0].message
+        == "'auth' section not found in context configuration"
+    )
 
 
 @pytest.mark.asyncio
@@ -83,9 +83,9 @@ async def test_base_disabled_config(logdog):
         authorised = plugin.topic_filtering()
         assert authorised is True
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
@@ -102,9 +102,9 @@ async def test_base_enabled_config(logdog):
         authorised = plugin.topic_filtering()
         assert authorised is True
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 # Taboo plug-in
@@ -123,19 +123,19 @@ async def test_taboo_empty_config(logdog):
         plugin = TopicTabooPlugin(context)
         assert (await plugin.topic_filtering()) is False
 
-        # Should have printed a couple of warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 2
-        assert log_records[0].levelno == logging.WARN
-        assert (
-            log_records[0].message
-            == "'topic-check' section not found in context configuration"
-        )
-        assert log_records[1].levelno == logging.WARN
-        assert (
-            log_records[1].message
-            == "'auth' section not found in context configuration"
-        )
+    # Should have printed a couple of warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 2
+    assert log_records[0].levelno == logging.WARN
+    assert (
+        log_records[0].message
+        == "'topic-check' section not found in context configuration"
+    )
+    assert log_records[1].levelno == logging.WARN
+    assert (
+        log_records[1].message
+        == "'auth' section not found in context configuration"
+    )
 
 
 @pytest.mark.asyncio
@@ -156,9 +156,9 @@ async def test_taboo_not_taboo_topic(logdog):
             await plugin.topic_filtering(session=session, topic="not/prohibited")
         ) is True
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
@@ -179,9 +179,9 @@ async def test_taboo_not_taboo_topic(logdog):
             await plugin.topic_filtering(session=session, topic="not/prohibited")
         ) is True
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
@@ -202,9 +202,9 @@ async def test_taboo_anon_taboo_topic(logdog):
             await plugin.topic_filtering(session=session, topic="prohibited")
         ) is False
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
@@ -225,9 +225,9 @@ async def test_taboo_notadmin_taboo_topic(logdog):
             await plugin.topic_filtering(session=session, topic="prohibited")
         ) is False
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 @pytest.mark.asyncio
@@ -248,9 +248,9 @@ async def test_taboo_admin_taboo_topic(logdog):
             await plugin.topic_filtering(session=session, topic="prohibited")
         ) is True
 
-        # Should NOT have printed warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 0
+    # Should NOT have printed warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 0
 
 
 # TopicAccessControlListPlugin tests
@@ -324,17 +324,17 @@ async def test_taclp_empty_config(logdog):
         plugin = TopicAccessControlListPlugin(context)
         assert (await plugin.topic_filtering()) is False
 
-        # Should have printed a couple of warnings
-        log_records = list(pile.drain(name="testlog"))
-        assert len(log_records) == 2
-        assert (
-            log_records[0].message
-            == "'topic-check' section not found in context configuration"
-        )
-        assert (
-            log_records[1].message
-            == "'auth' section not found in context configuration"
-        )
+    # Should have printed a couple of warnings
+    log_records = list(pile.drain(name="testlog"))
+    assert len(log_records) == 2
+    assert (
+        log_records[0].message
+        == "'topic-check' section not found in context configuration"
+    )
+    assert (
+        log_records[1].message
+        == "'auth' section not found in context configuration"
+    )
 
 
 @pytest.mark.asyncio

From a1dee1f3d455784efeafe63b6de643bf6bd19e69 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 07:57:48 +1000
Subject: [PATCH 29/33] =?UTF-8?q?broker=20tests:=20Change=20passwords=20fo?=
 =?UTF-8?q?r=20user[1=E2=80=A63]?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As pointed out, it's possible to accidentally mix them up if the
passwords are the same.  Obviously, the passwords used for these tests
should _NOT_ be used in production.
---
 tests/plugins/passwd |  8 ++++----
 tests/test_broker.py | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/plugins/passwd b/tests/plugins/passwd
index 7749b33a..c2dfe648 100644
--- a/tests/plugins/passwd
+++ b/tests/plugins/passwd
@@ -1,7 +1,7 @@
 # Test password file
 user:$6$1sSXVMBVKMF$uDStq59YfiuFxVF1Gi/.i/Li7Vwf5iTwg8LovLKmCvM5FsRNJM.OPWHhXwI2.4AscLZXSPQVxpIlX6laUl9570
 test_user:$6$.c9f9sAzs5YXX2de$GSdOi3iFwHJRCIJn1W63muDFQAL29yoFmU/TXcwDB42F2BZg3zcN5uKBM0.1PjwdMpWHRydbhXWSc3uWKSmKr.
-# Password for these is "test"
-user1:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1
-user2:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1
-user3:$6$QeikCQ5JjR$AS0ibQtNpfkLZY7fhpLqDC6KkVvYhwKQd1CM.WGjVQIoWIKxxTJGkyzNx4rWvp7FaoMG9kENULRHEI907I0fx1
+# Password for these is "${USER}password"
+user1:$6$h.fV0zYsXI$8wKblqETpztRKcPD6OLWZc1mU4nW5yQ713R5ECs7EwJa7oas/yrhI2itUdhETI8BvmtfGy65ltAMap9gHkzdc1
+user2:$6$bUyF8v0mTo94$IJMa2BlCd6/mAM5N5s6heFB2ewC4j3CDkFb9mYIoarXg4OxiZVnLyh20IWHf6GY8i4sc5m2D9nWLrtbnIO5o8.
+user3:$6$YOjKg1kYEeGibb$HlVa2EvQbF1ssSQs.lFnS1NhoTBQLG5YF7h0z4komAEvNJw6m4gay81MRp.lt4PSbcVrimcuRbidR9cRZkfBb/
diff --git a/tests/test_broker.py b/tests/test_broker.py
index 2b5ebe6e..7012e378 100644
--- a/tests/test_broker.py
+++ b/tests/test_broker.py
@@ -272,7 +272,7 @@ async def test_client_publish(broker, mock_plugin_manager):
 @pytest.mark.asyncio
 async def test_client_publish_acl_permitted(acl_broker):
     sub_client = MQTTClient()
-    ret = await sub_client.connect("mqtt://user2:test@127.0.0.1:1884/")
+    ret = await sub_client.connect("mqtt://user2:user2password@127.0.0.1:1884/")
     assert ret == 0
 
     ret = await sub_client.subscribe(
@@ -281,7 +281,7 @@ async def test_client_publish_acl_permitted(acl_broker):
     assert ret == [QOS_0]
 
     pub_client = MQTTClient()
-    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
     ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)
@@ -299,7 +299,7 @@ async def test_client_publish_acl_permitted(acl_broker):
 @pytest.mark.asyncio
 async def test_client_publish_acl_forbidden(acl_broker):
     sub_client = MQTTClient()
-    ret = await sub_client.connect("mqtt://user2:test@127.0.0.1:1884/")
+    ret = await sub_client.connect("mqtt://user2:user2password@127.0.0.1:1884/")
     assert ret == 0
 
     ret = await sub_client.subscribe(
@@ -308,7 +308,7 @@ async def test_client_publish_acl_forbidden(acl_broker):
     assert ret == [QOS_0]
 
     pub_client = MQTTClient()
-    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
     ret_message = await pub_client.publish("public/forbidden/test", b"data", QOS_0)
@@ -326,11 +326,11 @@ async def test_client_publish_acl_forbidden(acl_broker):
 @pytest.mark.asyncio
 async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
     sub_client1 = MQTTClient()
-    ret = await sub_client1.connect("mqtt://user2:test@127.0.0.1:1884/")
+    ret = await sub_client1.connect("mqtt://user2:user2password@127.0.0.1:1884/")
     assert ret == 0
 
     sub_client2 = MQTTClient()
-    ret = await sub_client2.connect("mqtt://user3:test@127.0.0.1:1884/")
+    ret = await sub_client2.connect("mqtt://user3:user3password@127.0.0.1:1884/")
     assert ret == 0
 
     ret = await sub_client1.subscribe(
@@ -344,7 +344,7 @@ async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
     assert ret == [0x80]
 
     pub_client = MQTTClient()
-    ret = await pub_client.connect("mqtt://user1:test@127.0.0.1:1884/")
+    ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
     ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)

From 245702f5f0ea5dadcbaaed2b552024b60b79ba9a Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 10:23:24 +1000
Subject: [PATCH 30/33] Pass code through `black` formatter

---
 amqtt/broker.py                      | 15 +++++++++------
 tests/conftest.py                    | 26 +++++++++-----------------
 tests/plugins/test_topic_checking.py | 20 ++++----------------
 tests/test_broker.py                 | 20 ++++++--------------
 4 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/amqtt/broker.py b/amqtt/broker.py
index 99878e9c..0ea461e2 100644
--- a/amqtt/broker.py
+++ b/amqtt/broker.py
@@ -45,8 +45,8 @@
 
 
 class Action(Enum):
-    subscribe = 'subscribe'
-    publish = 'publish'
+    subscribe = "subscribe"
+    publish = "publish"
 
 
 class BrokerException(Exception):
@@ -629,12 +629,13 @@ async def client_connected(
 
                     # See if the user is allowed to publish to this topic.
                     permitted = await self.topic_filtering(
-                            client_session, topic=app_message.topic, action=Action.publish
+                        client_session, topic=app_message.topic, action=Action.publish
                     )
                     if not permitted:
                         self.logger.info(
-                                "%s forbidden TOPIC %s sent in PUBLISH message.",
-                                client_session.client_id, app_message.topic
+                            "%s forbidden TOPIC %s sent in PUBLISH message.",
+                            client_session.client_id,
+                            app_message.topic,
                         )
                     else:
                         await self.plugins_manager.fire_event(
@@ -792,7 +793,9 @@ async def add_subscription(self, subscription, session):
                         # [MQTT-4.7.1-3] + wildcard character must occupy entire level
                         return 0x80
             # Check if the client is authorised to connect to the topic
-            permitted = await self.topic_filtering(session, topic=a_filter, action=Action.subscribe)
+            permitted = await self.topic_filtering(
+                session, topic=a_filter, action=Action.subscribe
+            )
             if not permitted:
                 return 0x80
             qos = subscription[1]
diff --git a/tests/conftest.py b/tests/conftest.py
index bf0ca9f9..9aab33d9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -26,28 +26,18 @@
     "auth": {
         "plugins": ["auth_file"],
         "password-file": os.path.join(
-            os.path.dirname(os.path.realpath(__file__)),
-            'plugins',
-            'passwd'
-        )
+            os.path.dirname(os.path.realpath(__file__)), "plugins", "passwd"
+        ),
     },
     "topic-check": {
         "enabled": True,
         "plugins": ["topic_acl"],
         "acl": {
-            "user1": [
-                "public/#"
-            ],
-            "user2": [
-                "#"
-            ],
+            "user1": ["public/#"],
+            "user2": ["#"],
         },
-        "publish-acl": {
-            "user1": [
-                "public/subtopic/#"
-            ]
-        },
-    }
+        "publish-acl": {"user1": ["public/subtopic/#"]},
+    },
 }
 
 
@@ -76,7 +66,9 @@ async def broker(mock_plugin_manager):
 
 @pytest.fixture(scope="function")
 async def acl_broker():
-    broker = amqtt.broker.Broker(test_config_acl, plugin_namespace="amqtt.broker.plugins")
+    broker = amqtt.broker.Broker(
+        test_config_acl, plugin_namespace="amqtt.broker.plugins"
+    )
     await broker.start()
     assert broker.transitions.is_started()
     assert broker._sessions == {}
diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 36192f11..40558e8a 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -38,10 +38,7 @@ async def test_base_no_config(logdog):
     )
 
     assert log_records[1].levelno == logging.WARN
-    assert (
-        log_records[1].message
-        == "'auth' section not found in context configuration"
-    )
+    assert log_records[1].message == "'auth' section not found in context configuration"
     assert pile.is_empty()
 
 
@@ -63,10 +60,7 @@ async def test_base_empty_config(logdog):
     log_records = list(pile.drain(name="testlog"))
     assert len(log_records) == 1
     assert log_records[0].levelno == logging.WARN
-    assert (
-        log_records[0].message
-        == "'auth' section not found in context configuration"
-    )
+    assert log_records[0].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
@@ -132,10 +126,7 @@ async def test_taboo_empty_config(logdog):
         == "'topic-check' section not found in context configuration"
     )
     assert log_records[1].levelno == logging.WARN
-    assert (
-        log_records[1].message
-        == "'auth' section not found in context configuration"
-    )
+    assert log_records[1].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
@@ -331,10 +322,7 @@ async def test_taclp_empty_config(logdog):
         log_records[0].message
         == "'topic-check' section not found in context configuration"
     )
-    assert (
-        log_records[1].message
-        == "'auth' section not found in context configuration"
-    )
+    assert log_records[1].message == "'auth' section not found in context configuration"
 
 
 @pytest.mark.asyncio
diff --git a/tests/test_broker.py b/tests/test_broker.py
index 7012e378..de127d23 100644
--- a/tests/test_broker.py
+++ b/tests/test_broker.py
@@ -275,9 +275,7 @@ async def test_client_publish_acl_permitted(acl_broker):
     ret = await sub_client.connect("mqtt://user2:user2password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret = await sub_client.subscribe(
-        [("public/subtopic/test", QOS_0)]
-    )
+    ret = await sub_client.subscribe([("public/subtopic/test", QOS_0)])
     assert ret == [QOS_0]
 
     pub_client = MQTTClient()
@@ -302,9 +300,7 @@ async def test_client_publish_acl_forbidden(acl_broker):
     ret = await sub_client.connect("mqtt://user2:user2password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret = await sub_client.subscribe(
-        [("public/forbidden/test", QOS_0)]
-    )
+    ret = await sub_client.subscribe([("public/forbidden/test", QOS_0)])
     assert ret == [QOS_0]
 
     pub_client = MQTTClient()
@@ -315,7 +311,7 @@ async def test_client_publish_acl_forbidden(acl_broker):
 
     try:
         await sub_client.deliver_message(timeout=1)
-        assert False, 'Should not have worked'
+        assert False, "Should not have worked"
     except asyncio.exceptions.TimeoutError:
         pass
 
@@ -333,14 +329,10 @@ async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
     ret = await sub_client2.connect("mqtt://user3:user3password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret = await sub_client1.subscribe(
-        [("public/subtopic/test", QOS_0)]
-    )
+    ret = await sub_client1.subscribe([("public/subtopic/test", QOS_0)])
     assert ret == [QOS_0]
 
-    ret = await sub_client2.subscribe(
-        [("public/subtopic/test", QOS_0)]
-    )
+    ret = await sub_client2.subscribe([("public/subtopic/test", QOS_0)])
     assert ret == [0x80]
 
     pub_client = MQTTClient()
@@ -353,7 +345,7 @@ async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
 
     try:
         await sub_client2.deliver_message(timeout=1)
-        assert False, 'Should not have worked'
+        assert False, "Should not have worked"
     except asyncio.exceptions.TimeoutError:
         pass
 

From e4b1fc3cfc3195d9d4791aec90d445172e23221d Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 10:37:45 +1000
Subject: [PATCH 31/33] plugins.topic_checking tests: Fix issues identified by
 flake8

---
 tests/plugins/test_topic_checking.py | 220 +++++++++++++--------------
 1 file changed, 106 insertions(+), 114 deletions(-)

diff --git a/tests/plugins/test_topic_checking.py b/tests/plugins/test_topic_checking.py
index 40558e8a..ae4be1ed 100644
--- a/tests/plugins/test_topic_checking.py
+++ b/tests/plugins/test_topic_checking.py
@@ -130,7 +130,7 @@ async def test_taboo_empty_config(logdog):
 
 
 @pytest.mark.asyncio
-async def test_taboo_not_taboo_topic(logdog):
+async def test_taboo_disabled(logdog):
     """
     Check TopicTabooPlugin returns true if checking disabled.
     """
@@ -278,7 +278,7 @@ def test_topic_ac_match_exact():
     assert TopicAccessControlListPlugin.topic_ac("exact/topic", "exact/topic") is True
 
 
-def test_topic_ac_match_hash():
+def test_topic_ac_match_plus():
     """
     Test TopicAccessControlListPlugin.topic_ac correctly handles '+' wildcard.
     """
@@ -330,19 +330,18 @@ async def test_taclp_true_disabled(logdog):
     """
     Check TopicAccessControlListPlugin returns true if topic checking is disabled.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {"topic-check": {"enabled": False}}
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {"topic-check": {"enabled": False}}
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.publish, session=session, topic="a/topic"
-        )
-        assert authorised is True
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.publish, session=session, topic="a/topic"
+    )
+    assert authorised is True
 
 
 @pytest.mark.asyncio
@@ -351,19 +350,18 @@ async def test_taclp_true_no_pub_acl(logdog):
     Check TopicAccessControlListPlugin returns true if action=publish and no publish-acl given.
     (This is for backward-compatibility with existing installations.)
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {"topic-check": {"enabled": True}}
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {"topic-check": {"enabled": True}}
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.publish, session=session, topic="a/topic"
-        )
-        assert authorised is True
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.publish, session=session, topic="a/topic"
+    )
+    assert authorised is True
 
 
 @pytest.mark.asyncio
@@ -371,24 +369,23 @@ async def test_taclp_false_sub_no_topic(logdog):
     """
     Check TopicAccessControlListPlugin returns false user there is no topic.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.subscribe, session=session, topic=""
-        )
-        assert authorised is False
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.subscribe, session=session, topic=""
+    )
+    assert authorised is False
 
 
 @pytest.mark.asyncio
@@ -396,24 +393,23 @@ async def test_taclp_false_sub_unknown_user(logdog):
     """
     Check TopicAccessControlListPlugin returns false user is not listed in ACL.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anotheruser": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.subscribe, session=session, topic="allowed/topic"
-        )
-        assert authorised is False
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.subscribe, session=session, topic="allowed/topic"
+    )
+    assert authorised is False
 
 
 @pytest.mark.asyncio
@@ -421,24 +417,23 @@ async def test_taclp_false_sub_no_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns false if "acl" does not list allowed topic.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.subscribe, session=session, topic="forbidden/topic"
-        )
-        assert authorised is False
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.subscribe, session=session, topic="forbidden/topic"
+    )
+    assert authorised is False
 
 
 @pytest.mark.asyncio
@@ -446,24 +441,23 @@ async def test_taclp_true_sub_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns true if "acl" lists allowed topic.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.subscribe, session=session, topic="allowed/topic"
-        )
-        assert authorised is True
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.subscribe, session=session, topic="allowed/topic"
+    )
+    assert authorised is True
 
 
 @pytest.mark.asyncio
@@ -471,24 +465,23 @@ async def test_taclp_true_pub_permission(logdog):
     """
     Check TopicAccessControlListPlugin returns true if "publish-acl" lists allowed topic for publish action.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "publish-acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "publish-acl": {"user": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = "user"
+    session = Session()
+    session.username = "user"
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.publish, session=session, topic="allowed/topic"
-        )
-        assert authorised is True
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.publish, session=session, topic="allowed/topic"
+    )
+    assert authorised is True
 
 
 @pytest.mark.asyncio
@@ -496,21 +489,20 @@ async def test_taclp_true_anon_sub_permission(logdog):
     """
     Check TopicAccessControlListPlugin handles anonymous users.
     """
-    with logdog() as pile:
-        context = BaseContext()
-        context.logger = logging.getLogger("testlog")
-        context.config = {
-            "topic-check": {
-                "enabled": True,
-                "acl": {"anonymous": ["allowed/topic", "another/allowed/topic/#"]},
-            }
+    context = BaseContext()
+    context.logger = logging.getLogger("testlog")
+    context.config = {
+        "topic-check": {
+            "enabled": True,
+            "acl": {"anonymous": ["allowed/topic", "another/allowed/topic/#"]},
         }
+    }
 
-        session = Session()
-        session.username = None
+    session = Session()
+    session.username = None
 
-        plugin = TopicAccessControlListPlugin(context)
-        authorised = await plugin.topic_filtering(
-            action=Action.subscribe, session=session, topic="allowed/topic"
-        )
-        assert authorised is True
+    plugin = TopicAccessControlListPlugin(context)
+    authorised = await plugin.topic_filtering(
+        action=Action.subscribe, session=session, topic="allowed/topic"
+    )
+    assert authorised is True

From 57e88a8c08cfa6bc00981ee6811fd8dd529c9c8f Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 10:39:07 +1000
Subject: [PATCH 32/33] broker tests: Clean up flake8 warnings

---
 tests/test_broker.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_broker.py b/tests/test_broker.py
index de127d23..a1e8d872 100644
--- a/tests/test_broker.py
+++ b/tests/test_broker.py
@@ -282,7 +282,7 @@ async def test_client_publish_acl_permitted(acl_broker):
     ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)
+    await pub_client.publish("public/subtopic/test", b"data", QOS_0)
 
     message = await sub_client.deliver_message(timeout=1)
     await pub_client.disconnect()
@@ -307,7 +307,7 @@ async def test_client_publish_acl_forbidden(acl_broker):
     ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret_message = await pub_client.publish("public/forbidden/test", b"data", QOS_0)
+    await pub_client.publish("public/forbidden/test", b"data", QOS_0)
 
     try:
         await sub_client.deliver_message(timeout=1)
@@ -339,7 +339,7 @@ async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
     ret = await pub_client.connect("mqtt://user1:user1password@127.0.0.1:1884/")
     assert ret == 0
 
-    ret_message = await pub_client.publish("public/subtopic/test", b"data", QOS_0)
+    await pub_client.publish("public/subtopic/test", b"data", QOS_0)
 
     message = await sub_client1.deliver_message(timeout=1)
 

From a74abf7a8af207904a84240b17ebb6ce2b78aac7 Mon Sep 17 00:00:00 2001
From: Stuart Longland <me@vk4msl.id.au>
Date: Thu, 8 Jul 2021 17:18:26 +1000
Subject: [PATCH 33/33] broker tests: Fix reference to TimeoutError.

Seems Python 3.8+ moved it, and I took the reference from there.
---
 tests/test_broker.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_broker.py b/tests/test_broker.py
index a1e8d872..0d9d5b54 100644
--- a/tests/test_broker.py
+++ b/tests/test_broker.py
@@ -312,7 +312,7 @@ async def test_client_publish_acl_forbidden(acl_broker):
     try:
         await sub_client.deliver_message(timeout=1)
         assert False, "Should not have worked"
-    except asyncio.exceptions.TimeoutError:
+    except asyncio.TimeoutError:
         pass
 
     await pub_client.disconnect()
@@ -346,7 +346,7 @@ async def test_client_publish_acl_permitted_sub_forbidden(acl_broker):
     try:
         await sub_client2.deliver_message(timeout=1)
         assert False, "Should not have worked"
-    except asyncio.exceptions.TimeoutError:
+    except asyncio.TimeoutError:
         pass
 
     await pub_client.disconnect()