Handling for MqttPassword / Passwords in general

[deveth0]

Hey,

when setting the mqtt password using the ansible role, it is detected as change each time. Also the password is published in plaintext inside the playbook output:

changed: [tasmota_balkon_chirp] => (item={'command': 'MqttPassword', 'value': 'blubblub'})

I'm not sure, if we can solve the first problem, is there any best practive if such configs should return a "changed" or "ok"?

22:06:34.603 CMD: mqttpassword
22:06:34.611 MQT: stat/tasmota_balkon_chirp/RESULT = {"MqttPassword":"****"}

For the second problem: It is also relevant for the following configs:

• Password
• WebPassword

I guess we could set the no_log flag for all commands that contain "password", what do you think?

[tobias-richter]

I am not using the role to set MqttPassword or other credentials. My approach would be to use the normal no_log pattern.

Item definition

- command: MqttPassword
  value: "LoremIpsum"
  no_log: True

Task:

- name: "Configure tasmota."
  tasmota:
    command: "{{ item.command }}"
    value: "{{ item.value }}"
    no_log: "{{ item.no_log | default(True) }}"
  with_items: "{{ tasmota_commands }}"

I did a quick test and it not working out of the box so there is something missing.

Regarding the changed / not changed output of the password I have not really an opinion. If you have the feeling that a not changed output is the correct output in this case you can implement a PR for this. Another solution would be to make the "changed" result overwritable via a task variable:

- name: "Configure tasmota."
  tasmota:
    command: "{{ item.command }}"
    value: "{{ item.value }}"
    changed: "{{ item.report_changed | default(omit) }}"
  with_items: "{{ tasmota_commands }}"

ℹ️ Please note that the command MqttPassword will restart the device. The Role does currently not handle restarts so this must also be implemented.

Feel free to implement a PR for this. Since the issue does not touch my usecases and I have currently not that much time I will not spend any effort in a solution :)

[tobias-richter]

@deveth0 my bad regarding the not working no_log parameter. See #15 with a working solution. Please contribute the missing no_log functionality for the debug logs.

[deveth0]

👍 thanks, I'll have a look into this. Any reason why the user has to configure no_log manually? I'd extend the logic so that all items with the name *Password are not logged by default, any objections?

[tobias-richter]

Any reason why the user has to configure no_log manually

This is default ansible behavior / concept. Another solution would be to migrate the tasmota role into a Ansible collection with special tasks for mqtt password etc. which can then handle this, but from effort perspective this is (for me) out of scope from the moment.

I do not know if there is an "automatic" way inside the action plugin to set the no_log functionality based on the parameters but you are free to try. Maybe you can pass it to the super ActionModule __init__ function.

I think from user perspective it would be confusing if some values will cause an automatic no_log behavior and some not. The manual setting of no_log is more familiar to the ansible community.

[deveth0]

ah ok, i thought about something like this, but you are the ansible expert ;)

no_log: "{{ item.command is regex('.*[pP]assword') or item.no_log | default(omit) }}"

[tobias-richter]

IMHO the user should be responsible vor hiding sensitive data in this case, but of course we should enable the user to do so. One of the main use-case for hiding sensitive data is in the log output of multi-user CI/CD systems where not all users have access to the Ansible vault. In your use-case I assume that you are executing the command and you know the password of your mqtt server :)

[deveth0]

This PR brings in the retry handling that is required for restarts: #16

I also added a basic no_log support to your branch. I'm not sure if we should simply hide all logs if the no_log is set or just those, that might contain the value. Imho that's enough and easier for debugging.

[tobias-richter]

16 and #15 are merged. Closing this issue. Feel free to open again if required.