Skip to content

resource/mqtt: add optional username/password for MQTT authentication#1834

Merged
Emantor merged 1 commit intolabgrid-project:masterfrom
jacmet:feature/mqtt-optional-username-password
Mar 13, 2026
Merged

resource/mqtt: add optional username/password for MQTT authentication#1834
Emantor merged 1 commit intolabgrid-project:masterfrom
jacmet:feature/mqtt-optional-username-password

Conversation

@jacmet
Copy link
Copy Markdown
Contributor

@jacmet jacmet commented Mar 4, 2026

Description

Some setups do not allow unauthenticated MQTT access, so add optional username and password arguments to TasmotaPowerPort.

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested
  • Man pages have been regenerated

@Emantor
Copy link
Copy Markdown
Member

Emantor commented Mar 4, 2026

We can add this, however the username and password will be globally exposed for all users that can access the coordinator, so this does not add much security.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.4%. Comparing base (b666734) to head (022f552).
⚠️ Report is 8 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1834   +/-   ##
======================================
  Coverage    45.4%   45.4%           
======================================
  Files         177     177           
  Lines       13863   13869    +6     
======================================
+ Hits         6300    6306    +6     
  Misses       7563    7563           
Flag Coverage Δ
3.10 45.4% <100.0%> (+<0.1%) ⬆️
3.11 45.4% <100.0%> (+<0.1%) ⬆️
3.12 45.4% <100.0%> (+<0.1%) ⬆️
3.13 45.4% <100.0%> (+<0.1%) ⬆️
3.14 45.4% <100.0%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread labgrid/resource/mqtt.py Outdated
host = attr.ib(validator=attr.validators.instance_of(str))
avail_topic = attr.ib(validator=attr.validators.instance_of(str))
username = attr.ib(default="", validator=attr.validators.instance_of(str))
password = attr.ib(default="", validator=attr.validators.instance_of(str))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use None instead of an empty string here if no password is set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why the empty string would not work just as well? I chose the empty string to simplify the {host}:{username}:{password} logic

Comment thread labgrid/resource/mqtt.py Outdated
@jacmet
Copy link
Copy Markdown
Contributor Author

jacmet commented Mar 4, 2026

We can add this, however the username and password will be globally exposed for all users that can access the coordinator, so this does not add much security.

True, but that is the general situation with labgrid, right?

@jacmet
Copy link
Copy Markdown
Contributor Author

jacmet commented Mar 5, 2026

We can add this, however the username and password will be globally exposed for all users that can access the coordinator, so this does not add much security.

True, but that is the general situation with labgrid, right?

Even without a strong security guarantee, there may be use cases where unauthenticated MQTT is disabled, E.G.:

  • One broker used for multiple unrelated things and using authentication (together with ACLs) for isolation
  • Simply to stop alerts from silly corporate network scanners

I am personally in the 2nd group ;)

@jacmet jacmet force-pushed the feature/mqtt-optional-username-password branch from 70192bb to 7c0cfd5 Compare March 5, 2026 20:09
@jacmet
Copy link
Copy Markdown
Contributor Author

jacmet commented Mar 5, 2026

Changes since v1:

Some setups do not allow unauthenticated MQTT access, so add optional
username and password arguments to TasmotaPowerPort.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
@jacmet jacmet force-pushed the feature/mqtt-optional-username-password branch from 7c0cfd5 to 022f552 Compare March 5, 2026 20:13
@jacmet
Copy link
Copy Markdown
Contributor Author

jacmet commented Mar 5, 2026

Rebased on current master for consistency.

@jacmet jacmet requested a review from Emantor March 5, 2026 20:14
@jacmet
Copy link
Copy Markdown
Contributor Author

jacmet commented Mar 13, 2026

@Emantor great, anything else missing before this can be merged?

@Emantor Emantor merged commit 5e74a80 into labgrid-project:master Mar 13, 2026
10 checks passed
@Emantor
Copy link
Copy Markdown
Member

Emantor commented Mar 13, 2026

@Emantor great, anything else missing before this can be merged?

No, CI hadn't run when I approved. Thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants