Open yveshanoulle opened 5 years ago
@kaikreuzer I'm not sure if you consider this a bug or a refactoring, yet the fact that it shows a password, seems a big flaw. It looks like the original coder is no longer maintaining it. I have other issues with the binding, yet this seems more important as it's a security issue
I would suggest just moving the credentials into a TRACE message, so that the user can enable viewing it if needed, but it never shows up by default:
} else {
logger.warn("Error occurred when sending UDP data to Anel device", e);
logger.trace("Command that was being sent: {}", cmd);
}
I would not even log the user name and password. as far as I know, some months ago al lot of passwords were removed from logging. Might be more interesting to send a message, wrong username + password, yet I have no idea if anel hut supports that. Yet I think for now, removing it would already make this much safer.
@9037568 I see the awaiting-feedback label, yet I have no idea who or what should give feedback.
I agree with @yveshanoulle. I'll try to find some time to create a PR the next days.
Expected Behavior
when there is an error, the password of teh user having the trouble, should not be in the log
Current Behavior
In https://github.com/openhab/openhab1-addons/blob/35f5a2973ddb1de98468ce1382d9ec1d334b3727/bundles/binding/org.openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelConnectorThread.java
this code add's the password to cmd and then gives out cmd when there is an exception
Possible Solution
something like
final String cmd = "IO_" + (newState ? "on" : "off") + ioNr ; final String up= user + password; final String cmdup= cmd+up; ...
connector.sendDatagram(cmdup.getBytes()); By changing cmd, all the places where cmd is used in teh logging, don't have to change..