Skip to content

Commit

Permalink
enable TLS by default; remove command-line option --mqtt-enable-tls
Browse files Browse the repository at this point in the history
  • Loading branch information
fphammerle committed Nov 4, 2023
1 parent 52764c2 commit 0843a25
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 70 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- declare compatibility with `python3.11`

### Changed
- TLS now enabled by default (disable via `--mqtt-disable-tls`)
- replaced [paho-mqtt](https://github.com/eclipse/paho.mqtt.python)
with its async wrapper [aiomqtt](https://github.com/sbtinstruments/aiomqtt)

### Removed
- command-line option `--mqtt-enable-tls` (TLS now enabled by default)
- compatibility with `python3.7`

## [3.3.1] - 2022-08-31
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $ pip3 install --user --upgrade switchbot-mqtt
## Usage

```sh
$ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS --mqtt-enable-tls
$ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS
# or
$ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS --mqtt-disable-tls
```
Expand Down
27 changes: 6 additions & 21 deletions switchbot_mqtt/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,9 @@ def _main() -> None:
argparser.add_argument(
"--mqtt-port",
type=int,
help=f"default {_MQTT_DEFAULT_PORT} ({_MQTT_DEFAULT_TLS_PORT} with --mqtt-enable-tls)",
)
mqtt_tls_argument_group = argparser.add_mutually_exclusive_group()
mqtt_tls_argument_group.add_argument(
"--mqtt-enable-tls",
action="store_true",
help="TLS will be enabled by default in the next major release",
)
mqtt_tls_argument_group.add_argument( # for upward compatibility
"--mqtt-disable-tls", action="store_true", help="Currently enabled by default"
help=f"default {_MQTT_DEFAULT_TLS_PORT} ({_MQTT_DEFAULT_PORT} with --mqtt-disable-tls)",
)
argparser.add_argument("--mqtt-disable-tls", action="store_true")
argparser.add_argument("--mqtt-username", type=str)
password_argument_group = argparser.add_mutually_exclusive_group()
password_argument_group.add_argument("--mqtt-password", type=str)
Expand Down Expand Up @@ -129,17 +121,10 @@ def _main() -> None:
_LOGGER.debug("args=%r", args)
if args.mqtt_port:
mqtt_port = args.mqtt_port
elif args.mqtt_enable_tls:
mqtt_port = _MQTT_DEFAULT_TLS_PORT
else:
elif args.mqtt_disable_tls:
mqtt_port = _MQTT_DEFAULT_PORT
if not args.mqtt_enable_tls and not args.mqtt_disable_tls:
warnings.warn(
"In switchbot-mqtt's next major release, TLS will be enabled by default"
" (--mqtt-enable-tls)."
" Please add --mqtt-disable-tls to your command for upward compatibility.",
UserWarning, # DeprecationWarning ignored by default
)
else:
mqtt_port = _MQTT_DEFAULT_TLS_PORT
if args.mqtt_password_path:
# .read_text() replaces \r\n with \n
mqtt_password = args.mqtt_password_path.read_bytes().decode()
Expand All @@ -159,7 +144,7 @@ def _main() -> None:
switchbot_mqtt._run( # pylint: disable=protected-access; internal
mqtt_host=args.mqtt_host,
mqtt_port=mqtt_port,
mqtt_disable_tls=not args.mqtt_enable_tls,
mqtt_disable_tls=args.mqtt_disable_tls,
mqtt_username=args.mqtt_username,
mqtt_password=mqtt_password,
mqtt_topic_prefix=args.mqtt_topic_prefix,
Expand Down
68 changes: 20 additions & 48 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,23 @@ def test_console_entry_point() -> None:
(
["", "--mqtt-host", "mqtt-broker.local"],
"mqtt-broker.local",
1883,
8883,
None,
None,
3,
),
(
["", "--mqtt-host", "mqtt-broker.local", "--mqtt-port", "8883"],
["", "--mqtt-host", "mqtt-broker.local", "--mqtt-port", "8884"],
"mqtt-broker.local",
8883,
8884,
None,
None,
3,
),
(
["", "--mqtt-host", "mqtt-broker.local", "--mqtt-username", "me"],
"mqtt-broker.local",
1883,
8883,
"me",
None,
3,
Expand All @@ -86,7 +86,7 @@ def test_console_entry_point() -> None:
"21",
],
"mqtt-broker.local",
1883,
8883,
"me",
"secret",
21,
Expand All @@ -103,12 +103,12 @@ def test__main(
) -> None:
with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
"sys.argv", argv
), pytest.warns(UserWarning, match=r"Please add --mqtt-disable-tls\b"):
):
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
mqtt_host=expected_mqtt_host,
mqtt_port=expected_mqtt_port,
mqtt_disable_tls=True,
mqtt_disable_tls=False,
mqtt_username=expected_username,
mqtt_password=expected_password,
mqtt_topic_prefix="homeassistant/",
Expand Down Expand Up @@ -152,8 +152,8 @@ def test__main_mqtt_password_file(
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
mqtt_host="localhost",
mqtt_port=1883,
mqtt_disable_tls=True,
mqtt_port=8883,
mqtt_disable_tls=False,
mqtt_username="me",
mqtt_password=expected_password,
mqtt_topic_prefix="homeassistant/",
Expand Down Expand Up @@ -215,8 +215,8 @@ def test__main_device_password_file(
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
mqtt_host="localhost",
mqtt_port=1883,
mqtt_disable_tls=True,
mqtt_port=8883,
mqtt_disable_tls=False,
mqtt_username=None,
mqtt_password=None,
mqtt_topic_prefix="homeassistant/",
Expand All @@ -228,8 +228,8 @@ def test__main_device_password_file(

_RUN_DEFAULT_KWARGS: typing.Dict[str, typing.Any] = {
"mqtt_host": "localhost",
"mqtt_port": 1883,
"mqtt_disable_tls": True,
"mqtt_port": 8883,
"mqtt_disable_tls": False,
"mqtt_username": None,
"mqtt_password": None,
"mqtt_topic_prefix": "homeassistant/",
Expand All @@ -239,10 +239,10 @@ def test__main_device_password_file(
}


def test__main_mqtt_disable_tls_implicit() -> None:
def test__main_mqtt_disable_tls() -> None:
with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
"sys.argv", ["", "--mqtt-host", "mqtt.local"]
), pytest.warns(UserWarning, match=r"Please add --mqtt-disable-tls\b"):
"sys.argv", ["", "--mqtt-host", "mqtt.local", "--mqtt-disable-tls"]
):
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
**{
Expand All @@ -254,50 +254,22 @@ def test__main_mqtt_disable_tls_implicit() -> None:
)


def test__main_mqtt_enable_tls() -> None:
with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
"sys.argv", ["", "--mqtt-host", "mqtt.local", "--mqtt-enable-tls"]
):
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
**{
**_RUN_DEFAULT_KWARGS,
"mqtt_host": "mqtt.local",
"mqtt_disable_tls": False,
"mqtt_port": 8883,
}
)


def test__main_mqtt_enable_tls_overwrite_port() -> None:
def test__main_mqtt_disable_tls_overwrite_port() -> None:
with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
"sys.argv",
["", "--mqtt-host", "mqtt.local", "--mqtt-port", "1883", "--mqtt-enable-tls"],
["", "--mqtt-host", "mqtt.local", "--mqtt-port", "1884", "--mqtt-disable-tls"],
):
switchbot_mqtt._cli._main()
run_mock.assert_called_once_with(
**{
**_RUN_DEFAULT_KWARGS,
"mqtt_host": "mqtt.local",
"mqtt_disable_tls": False,
"mqtt_port": 1883,
"mqtt_disable_tls": True,
"mqtt_port": 1884,
}
)


def test__main_mqtt_tls_collision(capsys: _pytest.capture.CaptureFixture) -> None:
with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
"sys.argv",
["", "--mqtt-host", "mqtt.local", "--mqtt-enable-tls", "--mqtt-disable-tls"],
), pytest.raises(SystemExit):
switchbot_mqtt._cli._main()
run_mock.assert_not_called()
assert (
"error: argument --mqtt-disable-tls: not allowed with argument --mqtt-enable-tls\n"
in capsys.readouterr()[1]
)


@pytest.mark.parametrize(
("additional_argv", "expected_topic_prefix"),
[([], "homeassistant/"), (["--mqtt-topic-prefix", ""], "")],
Expand Down

0 comments on commit 0843a25

Please sign in to comment.