Repository: Yakifo/amqtt (language: python)

Issue:
unecessary constraint on password 
Hi all,

It seems to me that amqtt's broker is needlessly requiring a password when a username is provided. It happens in https://github.com/Yakifo/amqtt/blob/master/amqtt/mqtt/protocol/broker_handler.py#L164-L167

```python
        elif not connect.username_flag and connect.password_flag:
            connack = ConnackPacket.build(0, BAD_USERNAME_PASSWORD)  # [MQTT-3.1.2-22]
        elif connect.username_flag and not connect.password_flag:
            connack = ConnackPacket.build(0, BAD_USERNAME_PASSWORD)  # [MQTT-3.1.2-22]
```

As per the spec (http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc385349236 is hopefully the right ref)

> If the User Name Flag is set to 0, the Password Flag MUST be set to 0 [MQTT-3.1.2-22].

So while the first `elif` is perfectly in line with the spec (you can't have a password without a username), I can't find anything in the spec about that second `elif`. Commenting out those 2 lines allows my client to connect and work just fine.

FTR, my client code is built using `azure-iot-device` (which uses `paho` behind the scene) and I use amqtt's broker to create a fake Azure IoTHub for mocking purposes. When using X.509 client certificates, the Azure libs use the "device id" (also found in the certificate's CN) and pass that to `paho` as a username, but without any password. For now, I've worked around the issue by monkeypatching `paho` to send both a username and a fake password to please amqtt. But it really looks like a bug in amqtt AFAICT.

I wanted to create a patch for this but poetry is foreign to me.

Cheers


Discussion / hints:
Hi @RemiCardona 

thanks for your report. Nice use case!

I agree with your interpretation and think we should allow username + no password.

About poetry: Have you seen: https://github.com/Yakifo/amqtt/blob/master/contributing.md ? If so, do you have a suggestion how we can improve on it to make it easier to get started?

Fix patch:
diff --git a/amqtt/mqtt/protocol/broker_handler.py b/amqtt/mqtt/protocol/broker_handler.py
index a70ebd94..1efc7891 100644
--- a/amqtt/mqtt/protocol/broker_handler.py
+++ b/amqtt/mqtt/protocol/broker_handler.py
@@ -163,8 +163,6 @@ async def init_from_connect(
             )  # [MQTT-3.2.2-4] session_parent=0
         elif not connect.username_flag and connect.password_flag:
             connack = ConnackPacket.build(0, BAD_USERNAME_PASSWORD)  # [MQTT-3.1.2-22]
-        elif connect.username_flag and not connect.password_flag:
-            connack = ConnackPacket.build(0, BAD_USERNAME_PASSWORD)  # [MQTT-3.1.2-22]
         elif connect.username_flag and connect.username is None:
             error_msg = "Invalid username from %s" % (
                 format_client_message(address=remote_address, port=remote_port)


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index eb958653..cc1691db 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -111,6 +111,17 @@ async def test_connect_ws_secure():
     await broker.shutdown()
 
 
+@pytest.mark.asyncio
+async def test_connect_username_without_password():
+    broker = Broker(broker_config, plugin_namespace="amqtt.test.plugins")
+    await broker.start()
+    client = MQTTClient()
+    await client.connect("mqtt://alice@127.0.0.1/")
+    assert client.session is not None
+    await client.disconnect()
+    await broker.shutdown()
+
+
 @pytest.mark.asyncio
 async def test_ping():
     broker = Broker(broker_config, plugin_namespace="amqtt.test.plugins")