core icon indicating copy to clipboard operation
core copied to clipboard

Bosch SHC can authenticate only 1 hub

Open Perrylicious opened this issue 3 years ago • 5 comments

The problem

If you have more than 1 Bosch SHC hub in your network, you can initially pair both of them and they're properly listed and bring up their sensors. After a HAOS restart (or whenever the TLS credentials are checked) the earlier of the 2 devices to be registered shows an authentication error. Their TLS credentials are written to the same filename in the config folder, with the newest registered hub overwriting the other devices credentials, locking them out.

What version of Home Assistant Core has the issue?

core-2022.12.7

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

bosch_shc

Link to integration documentation on our website

https://www.home-assistant.io/integrations/bosch_shc

Diagnostics information

home-assistant_bosch_shc_2022-12-20T23-16-38.877Z.log

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

I have 2 Bosch SHC hubs sitting on the same network, each with different devices (thermostats) connected (large space, not enough coverage with just one hub).

shc11b90b is on 192.168.1.13 shc11de30 is on 192.168.1.14

Registering the devices via the regular discovery->registration process (press button on device, log in with password) works and both hubs deliver entities and values to the frontend, but upon restarting HAOS the first device to be registered will ask to be re-authenticated.

Useability sidenote: during the re-authentication prompt, a modal pops up telling you that "The bosch_shc integration needs to re-authenticate your account". The first instinct to re-authenticate is to enter the password, but looking at the logs and the source, this first window expects the hostname or IP of the device. If you enter the password, it tries to use that as the host and failes due to no route (which presents as "unknown error" to the frontend but can be discerned through the logs). Inspection of the modal shows that the data name for the input field is "host" but that's not exposed to the end user.

Perrylicious avatar Dec 20 '22 23:12 Perrylicious

Hey there @tschamm, mind taking a look at this issue as it has been labeled with an integration (bosch_shc) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of bosch_shc can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign bosch_shc Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


bosch_shc documentation bosch_shc source (message by IssueLinks)

home-assistant[bot] avatar Dec 20 '22 23:12 home-assistant[bot]

I managed to work around this by forking your integration (amazing work btw, very easy to read and understand!) into a custom_components folder and changing a function in config_flow.py to prepend the TLS filenames with the host:

def create_credentials_and_validate(hass, host, user_input, zeroconf_instance):
    """Create and store credentials and validate session."""
    helper = SHCRegisterClient(host, user_input[CONF_PASSWORD])
    result = helper.register(host, "HomeAssistant")

    if result is not None:
        write_tls_asset(hass, host+CONF_SHC_CERT, result["cert"])
        write_tls_asset(hass, host+CONF_SHC_KEY, result["key"])

        session = SHCSession(
            host,
            hass.config.path(DOMAIN, host+CONF_SHC_CERT),
            hass.config.path(DOMAIN, host+CONF_SHC_KEY),
            True,
            zeroconf_instance,
        )
        session.authenticate()

    return result

Then I also changed the entry data (same file, lines 152-158) to include self.host:

                entry_data = {
                    CONF_SSL_CERTIFICATE: self.hass.config.path(DOMAIN, self.host+CONF_SHC_CERT),
                    CONF_SSL_KEY: self.hass.config.path(DOMAIN, self.host+CONF_SHC_KEY),
                    CONF_HOST: self.host,
                    CONF_TOKEN: result["token"],
                    CONF_HOSTNAME: result["token"].split(":", 1)[1],
                }

This fixes the problem because the files that are written are now host specific. There are probably a million ways to do this more elegantly, but I'm not a python programmer. :) The above hack has survived several restarts so far. I had to reset home assistant to factory defaults before re-registering the devices with the new TLS files, but that's a minor hurdle. Thank you very much for your amazing work and happy holidays!

Perrylicious avatar Dec 21 '22 12:12 Perrylicious

Will this code be implemented?

STB747400 avatar Mar 19 '23 10:03 STB747400

Already implemented in the custom component.

tschamm avatar Mar 19 '23 12:03 tschamm

The custom component described above is my somewhat hacky way of getting around the issue.

When I put that in, I was just starting with Home Assistant and wanted to get this edge case working. To actually get it working, after switching from this integration to the forked custom component, I had to reset Home Assistant to factory defaults to remove all previous configs/settings of Bosch from the system. This is because I couldn't figure out how to save the changed values (host+CONF_SHC_CERT and host+CONF_SHC_KEY) into the config for the existing integration. I couldn't even figure out where those values were stored to update them manually to allow for the host+ configuration. It would always pull the values without "host+" unless I registered all devices fresh from the custom component.

Now while you can do that, migrating people with an existing installation to make the component use the updated values allowing for more than one host to save CERT and KEY would likely be a more user friendly solution but my knowledge of Home Assistant, Python and Github pull request etiquette are too lacking to do this properly. :(

Plus you'd have to track changes to the main branch and merge them into your fork to stay up to date with development.

Perrylicious avatar Mar 19 '23 13:03 Perrylicious

I can see your point, as both versions (official integration as well as custom component) are currently diverging, switching from one to the other is not straight forward.

tschamm avatar Mar 19 '23 19:03 tschamm

I'd love to ditch the custom component if the above functionality (authenticate more than 1 hub) was somehow made available in the official integration; this was just a hacky way to get around this so I could set up HA over the holidays. I did not fork this into its own github project, I just copied the folder into custom_components locally and made the edits I needed to get my local setup working.

If putting this into the integration is out of scope I completely understand.

Perrylicious avatar Mar 19 '23 19:03 Perrylicious

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.