search

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
72.64k stars 30.41k forks source link

Withings Integration does not work with non-443 port #101703

Closed Qhilm closed 1 year ago

Qhilm commented 1 year ago

The problem

I switched my external URL from 443 to an alternate port and am now getting the below error when reloading the integration in debug mode. It was working before and I was not seeing this python errors in debug log (I looked at debug log because of previous unrelated error).

What version of Home Assistant Core has the issue?

2023.10.1

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

Withings

Link to integration documentation on our website

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

Diagnostics information

2023-10-09 14:49:01.242 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/withings/__init__.py", line 173, in register_webhook
    await hass.data[DOMAIN][entry.entry_id].async_subscribe_webhooks(webhook_url)
  File "/usr/src/homeassistant/homeassistant/components/withings/coordinator.py", line 115, in async_subscribe_webhooks
    await self._client.async_notify_subscribe(webhook_url, notification)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 158, in async_notify_subscribe
    await self._do_retry(call_super)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 87, in _do_retry
    raise exception
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 76, in _do_retry
    return await func()
           ^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 154, in call_super
    await self._hass.async_add_executor_job(
  File "/usr/local/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 348, in notify_subscribe
    self.request(path=self.PATH_NOTIFY, params=params)
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 115, in request
    return response_body_or_raise(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/common.py", line 830, in response_body_or_raise
    raise InvalidParamsException(status=status)
withings_api.common.InvalidParamsException: Error code 293
[...]
2023-10-09 14:53:17.796 ERROR (MainThread) [homeassistant.components.fritzbox] Timeout fetching 21a1ea981671ce2d035e0dae8993e551 data
2023-10-09 14:53:45.049 DEBUG (MainThread) [homeassistant.components.withings] Updating withings measures
2023-10-09 14:53:45.049 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-09 14:53:45.338 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-09 14:53:45.646 DEBUG (MainThread) [homeassistant.components.withings] Finished fetching Withings data in 0.597 seconds (success: True)
2023-10-09 14:53:46.648 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-09 14:53:46.992 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-09 14:53:47.344 DEBUG (MainThread) [homeassistant.components.withings] Subscribing https://ha.domain.com:9999/api/webhook/53cf29554be69c7988670787ca62e63d11951e44e4fc9e03e792fa4634e6fb68 for 1 in 5.0 seconds
2023-10-09 14:53:52.346 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-09 14:53:52.588 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 1 of 3 (Error code 293)
2023-10-09 14:53:53.089 DEBUG (MainThread) [homeassistant.components.withings] Attempt 2 of 3
2023-10-09 14:53:53.379 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 2 of 3 (Error code 293)
2023-10-09 14:53:54.385 DEBUG (MainThread) [homeassistant.components.withings] Attempt 3 of 3
2023-10-09 14:53:54.649 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 3 of 3 (Error code 293)
2023-10-09 14:53:56.150 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/withings/__init__.py", line 173, in register_webhook
    await hass.data[DOMAIN][entry.entry_id].async_subscribe_webhooks(webhook_url)
  File "/usr/src/homeassistant/homeassistant/components/withings/coordinator.py", line 115, in async_subscribe_webhooks
    await self._client.async_notify_subscribe(webhook_url, notification)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 158, in async_notify_subscribe
    await self._do_retry(call_super)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 87, in _do_retry
    raise exception
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 76, in _do_retry
    return await func()
           ^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 154, in call_super
    await self._hass.async_add_executor_job(
  File "/usr/local/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 348, in notify_subscribe
    self.request(path=self.PATH_NOTIFY, params=params)
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 115, in request
    return response_body_or_raise(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/common.py", line 830, in response_body_or_raise
    raise InvalidParamsException(status=status)
withings_api.common.InvalidParamsException: Error code 293

Example YAML snippet

new configuration.yaml:

homeassistant:
  external_url: "https://ha.domain.com:9999"

old configuration.yaml:

homeassistant:
  external_url: "https://ha.domain.com"

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 year ago

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

Code owner commands Code owners of `withings` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign withings` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)

withings documentation withings source (message by IssueLinks)

joostlek commented 1 year ago

I know you are doing some advanced networking stuff, did you keep in mind that Withings will do a HEAD request to the url to check if its valid?

Qhilm commented 1 year ago

That might fail indeed with my current setup if the HEAD request is not sent to port 9999, but that shouldn't cause python "InvalidParamsException" error, right? It feels like the URL with :port at the end cannot be parsed. Which might very well be an error on withings side...

joostlek commented 1 year ago

as a matter of fact, Withings returns an error code and then also returns a message, but we can't see the message right now

Qhilm commented 1 year ago

I'm not sure I understood this:

If you have the time, maybe try setting external_url in configuration.yaml to https://your.url.com:1234 and see if you have the same error as well, that would be interesting.

joostlek commented 1 year ago

The only problem being that I don't have a public IP address since I am living in an apartment complex.

Qhilm commented 1 year ago

you could run cloudflared, like I do, that allows you to have a public IP, which is then tunneled back to your HA. Happy to help setting this up, ping me if you have questions.

joostlek commented 1 year ago

I was just checking the Withings docs, but I can't find any info on any port restrictions.

You also got 293 as error code when you were blocking outside traffic right? If that's the case, can weassume that 293 is the error code for that it can't validate the url?

Qhilm commented 1 year ago

I did not see this error in the past, hence I assumed it's related to the port added at the end of the external_url parameter. I will change back, block everything and see if I have a similar issue or not.

mkaatman commented 1 year ago

you could run cloudflared, like I do, that allows you to have a public IP, which is then tunneled back to your HA. Happy to help setting this up, ping me if you have questions.

I'm getting 293. I recently switched to cloudflare. I could use some assistance if you've got any docs.

I'm running haproxy on my side. It seems like that part is working and it's being redirected.

If I hit https://mydomain.com/redirect/oauth in a browser I get a 400. If I test it through withings dev portal it says site can't be reached.


Logger: homeassistant.components.withings
Source: components/withings/common.py:363
Integration: Withings (documentation, issues)
First occurred: September 29, 2023 at 7:52:13 AM (145 occurrences)
Last logged: 8:49:15 AM

Unexpected error fetching subscription_update_coordinator data: Error code 293
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 290, in _async_refresh
    self.data = await self._async_update_data()
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 246, in _async_update_data
    return await self.update_method()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/common.py", line 321, in async_subscribe_webhook
    return await self._do_retry(self._async_subscribe_webhook)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/common.py", line 317, in _do_retry
    raise exception
  File "/usr/src/homeassistant/homeassistant/components/withings/common.py", line 306, in _do_retry
    return await func()
           ^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/common.py", line 363, in _async_subscribe_webhook
    await self._hass.async_add_executor_job(
  File "/usr/local/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 348, in notify_subscribe
    self.request(path=self.PATH_NOTIFY, params=params)
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 115, in request
    return response_body_or_raise(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/common.py", line 830, in response_body_or_raise
    raise InvalidParamsException(status=status)
withings_api.common.InvalidParamsException: Error code 293
joostlek commented 1 year ago

@mkaatman that really sounds like Withings can't connect to your instance. Do you have any blocking or anything?

mkaatman commented 1 year ago

Let me see if I can find logs. It was working previously for a year or two.

Confirmed that haproxy is seeing the request and forwarding it on when I test from withings dev portal.

Oct 11 14:40:00 haproxy haproxy[25837]: 172.71.134.94:53148 [11/Oct/2023:14:40:00.527] http-in homeassistant_cluster/node1 0/0/0/2/2 400 160 - - --NN 4/4/0/0/0 0/0 "HEAD /redirect/oauth HTTP/1.1"

joostlek commented 1 year ago

But, to be clear, you are using the 443 port? If so, please open a separate issue

mkaatman commented 1 year ago

I've tried it with 80 and 443.

New issue created: https://github.com/home-assistant/core/issues/101826

joostlek commented 1 year ago

Please create a new issue

Qhilm commented 1 year ago

You also got 293 as error code when you were blocking outside traffic right? If that's the case, can weassume that 293 is the error code for that it can't validate the url?

Beforehand: all the data seems to reach my Home Assistant, I see updated temperature values, without having to reload the integration. Although I don't understand if it's the integration pulling the data from withings (I see log lines like "Finished fetching Withings data" which to me means its pulling the data, it's not really a webhook pushing the data – but maybe I'm misunderstanding something)

But the logs are full of the error 293. I disabled blocking in cloudflare WAF as you asked, I still get the same error (debug logs enabled before reloading the integration):

2023-10-12 08:36:18.981 DEBUG (MainThread) [homeassistant.components.withings] Updating withings measures
2023-10-12 08:36:18.981 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:36:19.246 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:36:19.553 DEBUG (MainThread) [homeassistant.components.withings] Finished fetching Withings data in 0.573 seconds (success: True)
2023-10-12 08:36:20.556 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:36:20.889 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:36:21.224 DEBUG (MainThread) [homeassistant.components.withings] Subscribing https://ha.domain.com:9999/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 1 in 5.0 seconds
2023-10-12 08:36:26.225 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:36:26.470 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 1 of 3 (Error code 293)
2023-10-12 08:36:26.972 DEBUG (MainThread) [homeassistant.components.withings] Attempt 2 of 3
2023-10-12 08:36:27.215 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 2 of 3 (Error code 293)
2023-10-12 08:36:28.216 DEBUG (MainThread) [homeassistant.components.withings] Attempt 3 of 3
2023-10-12 08:36:28.489 DEBUG (MainThread) [homeassistant.components.withings] Failed attempt 3 of 3 (Error code 293)
2023-10-12 08:36:29.991 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/withings/__init__.py", line 173, in register_webhook
    await hass.data[DOMAIN][entry.entry_id].async_subscribe_webhooks(webhook_url)
  File "/usr/src/homeassistant/homeassistant/components/withings/coordinator.py", line 115, in async_subscribe_webhooks
    await self._client.async_notify_subscribe(webhook_url, notification)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 158, in async_notify_subscribe
    await self._do_retry(call_super)
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 87, in _do_retry
    raise exception
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 76, in _do_retry
    return await func()
           ^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/withings/api.py", line 154, in call_super
    await self._hass.async_add_executor_job(
  File "/usr/local/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 348, in notify_subscribe
    self.request(path=self.PATH_NOTIFY, params=params)
  File "/usr/local/lib/python3.11/site-packages/withings_api/__init__.py", line 115, in request
    return response_body_or_raise(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/withings_api/common.py", line 830, in response_body_or_raise
    raise InvalidParamsException(status=status)
withings_api.common.InvalidParamsException: Error code 293

then I removed the port from the external_url parameter in configuration.yaml and the issue is gone from the logs (debug logs enabled before reloading the integration):

2023-10-12 08:40:38.643 WARNING (SyncWorker_0) [homeassistant.loader] We found a custom integration opnsense which has not been tested by Home Assistant. This component might cause stability problems, be sure to disable it if you experience issues with Home Assistant
2023-10-12 08:40:38.645 WARNING (SyncWorker_0) [homeassistant.loader] We found a custom integration hacs which has not been tested by Home Assistant. This component might cause stability problems, be sure to disable it if you experience issues with Home Assistant
2023-10-12 08:40:38.647 WARNING (SyncWorker_0) [homeassistant.loader] We found a custom integration dwd_weather which has not been tested by Home Assistant. This component might cause stability problems, be sure to disable it if you experience issues with Home Assistant
2023-10-12 08:41:06.053 WARNING (MainThread) [homeassistant.components.binary_sensor] Updating ping binary_sensor took longer than the scheduled update interval 0:00:05
2023-10-12 08:41:18.523 WARNING (MainThread) [homeassistant.components.device_tracker] Setup of device_tracker platform opnsense is taking over 10 seconds.
2023-10-12 08:41:50.882 WARNING (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: dwd_weather
2023-10-12 08:41:59.611 WARNING (MainThread) [homeassistant.helpers.script.trigger_update_coordinator] Trigger Update Coordinator: Running script requires passing in a context
2023-10-12 08:43:51.186 DEBUG (MainThread) [homeassistant.components.withings] Updating withings measures
2023-10-12 08:43:51.186 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:51.481 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:51.763 DEBUG (MainThread) [homeassistant.components.withings] Finished fetching Withings data in 0.577 seconds (success: True)
2023-10-12 08:43:52.765 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:53.098 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 1 in 1.0 seconds
2023-10-12 08:43:54.099 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:54.334 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 4 in 1.0 seconds
2023-10-12 08:43:55.337 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:55.567 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 16 in 1.0 seconds
2023-10-12 08:43:56.568 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:56.814 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 44 in 1.0 seconds
2023-10-12 08:43:57.815 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:58.110 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 50 in 1.0 seconds
2023-10-12 08:43:59.112 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:43:59.370 DEBUG (MainThread) [homeassistant.components.withings] Unsubscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 51 in 1.0 seconds
2023-10-12 08:44:00.371 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:44:00.659 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:44:00.983 DEBUG (MainThread) [homeassistant.components.withings] Subscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 1 in 5.0 seconds
2023-10-12 08:44:05.984 DEBUG (MainThread) [homeassistant.components.withings] Attempt 1 of 3
2023-10-12 08:44:06.595 DEBUG (MainThread) [homeassistant.components.withings] Subscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 4 in 5.0 seconds

then I thought of one more test: set external_url to https://ha.domain.com:443. It doesn't change the port number at all, but it's explicitly defined and if there's a parsing error, it should then suddenly trigger the issue again. But it does not trigger the issue. But this does not trigger the issue, hence it's not a parsing issue, it's really the value of the port number which is triggering the issue somehow.

The Withings callback URL only supports port 80 and 443 as stated in the Withings developer doc, but there is no documentation for the webhook destination URL. I will open a ticket with Withings to check this.

Finally, I have a question about something you mentioned before:

I don't have a public IP address

How does the integration works for you then, where are the webhooks sending the data to? Home Assistant Cloud? Just to understand your setup.

Qhilm commented 1 year ago

The more I read on the doc the more confused I get.

Withings docs say:

You will have to specify the endpoint you want Withings to send the notification request to. This endpoint is called callbackurl and must be listed in the Callback URI field of your application.

My callback URL is set to https://my.home-assistant.io/redirect/oauth

How can the Withings cloud send data to this URL? This is not the URL of my home assistant. If you open this URL in your browser, you will not see my home assistant GUI...

In the logs I see things like this:

2023-10-12 08:51:00.888 DEBUG (MainThread) [homeassistant.components.withings] Subscribing https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941 for 16 in 5.0 seconds

which I read as: "the integration has instructed withings cloud to send the data to URL https://ha.domain.com/api/webhook/12a0a84e515179979becf5e05369cb89ebe0e4bdced1abf41ab5e118fa73f941".

But this contradicts the Withings documentation I quoted above.

Do you have a simple description of the data flow from withings cloud to the home assistant by any chance?

joostlek commented 1 year ago

The withings docs is kinda wrong on that. When we startup the integration we send a request to withings with an URL and which topic to subscribe to. Withings will do a HEAD request to check it works.

After that withings will send POSTs with updates.

Qhilm commented 1 year ago

thanks, this is helpful. Is there a way to see the webhooks the withing clouds is sending the data to? And which data goes to which URL?

joostlek commented 1 year ago

If you can get the token from your system you can do the request manually. Although currently we are unsubscribing from everything when it starts registring the webhooks.

So everything in the debug log is what you are subscribing to.

Did that answer your question?

Qhilm commented 1 year ago

thanks.

I think then it's clear Withings only supports 80 and 443, that's it. It's just confusingly documented under the "callback URL".

So I think my feature request would be to be able to configure the destination URL manually in the integration, rather than the integration using whatever is configured as external_url. Of course the default should be to use external_url. I can whitelist port 443 specifically for the Withings IPs.

For reference, the WIthings IPs which must be whitelisted are listed using these 2 commands: dig +short TXT ipblock-notify.withings.net dig +short TXT ipblock-front.withings.net

I chose to whitelist the entire subnet from withings which is 89.30.121.0/24 according to whois/IANA, as I feel it will be more robust if they choose to add servers.

joostlek commented 1 year ago

I only don't get the point, why do you want to configure a different webhook url than your external_url?

Qhilm commented 1 year ago

I would use non-standard port for the HA app, as it protects against a lot of bots (they don't scan all ports). But since Withings can't deal with alternate port, I would allow specifically 89.30.121.0/24 to connect to port 443 and only this IP range.

But for this to work, I must be able to manually configure the URL the integration uses for webhooks so it does not use the alternate port but simply 443, as it breaks withings it seems (error 293)...

I know this alternate port thing seems paranoid but you should see what I see on the firewall logs... (To be fair, that was my mistake, I used letsencrypt to create a cert and didn't realise it would put the URL on the map for bots due to letsencrypt's transparency logs – that's another area I need to improve.)

joostlek commented 1 year ago

Right. I understand your problem but there's one other problem I will face with this.

This is a wider problem than just Withings. For example this whole code is copied from the Netatmo integration. Other integrations with webhooks might have this problem aswell then. So if we would want to support this, we would require the logic in the core to change to support a separate webhook_url or something along those lines. And that would indeed be a feature request and for that I have to forward it to the community forums.

But for this issue itself, currently we do a check if the webhook we register starts with https, which is fine, but yours start with https but does not run on :443. So the only thing I can do for you for now is make sure that we double check the external URL is without port before we send it off, making your setup fallback to polling as we can't create a webhook.

I hope you understand the reasoning :)

Qhilm commented 1 year ago

Sure, if there's a different place where I can raise this as a feature request (sorry for my ignorance regarding your code base, I didn't even check), please let me know, happy to place it there, as you are right, it's a generic feature request, not specific to this integration.

Last question:

making your setup fallback to polling as we can't create a webhook.

The integration falls back to polling in this case? I did not know this. How often does it poll?

joostlek commented 1 year ago

Haha good. Thanks for understanding.

It should poll every 10 minutes. Without webhooks, the bed sensors don't work tho.

I am not sure if your integration now works or not as it raises the error whilst setting up. I think I could change the code to specifically check for if there is a port defined explicitly. And if so, if it isn't 443, we should fail (which we already do, but now only when the url doesnt start with https)

mkaatman commented 1 year ago

I would use non-standard port for the HA app, as it protects against a lot of bots (they don't scan all ports). But since Withings can't deal with alternate port, I would allow specifically 89.30.121.0/24 to connect to port 443 and only this IP range.

But for this to work, I must be able to manually configure the URL the integration uses for webhooks so it does not use the alternate port but simply 443, as it breaks withings it seems (error 293)...

I know this alternate port thing seems paranoid but you should see what I see on the firewall logs... (To be fair, that was my mistake, I used letsencrypt to create a cert and didn't realise it would put the URL on the map for bots due to letsencrypt's transparency logs – that's another area I need to improve.)

I had some of the same concerns. If you want to setup haproxy and need any help with the acls let me know. Then you can at least add some fine tuning around what's open instead of having it all wide open.

Qhilm commented 1 year ago

@mkaatman , you installed HAproxy on your Home Assistant OS? I just run cloudflared, I felt it was more secure than opening a port on my home router, but YMMV, depends on the router you have and on your threat model.

mkaatman commented 1 year ago

@mkaatman , you installed HAproxy on your Home Assistant OS? I just run cloudflared, I felt it was more secure than opening a port on my home router, but YMMV, depends on the router you have and on your threat model.

I use both. Cloudflare covers the entire domain and all incoming requests go to haproxy which routes them to different LXCs depending on what the request is. I have haproxy running on its own LXC but I imagine it should be no issue to run wherever you're running homeassistant.

joostlek commented 1 year ago

May I kindly ask to move this conversation away from this issue :)

Qhilm commented 1 year ago

I see you have added a warning in #102026. Wouldn't it be possible to simply remove the port from webhook_url? I mean, if https:// is the required scheme anyway, we can remove the port number, it won't change anything. Maybe just update the warning saying "port is not 443, removing" for example.

joostlek commented 1 year ago

I mean the warning was already there, we now just check explicitly if the port is 443. There probably are some snowflake instances out there that have http running on 443 for some reason, now we don't have that problem

Qhilm commented 1 year ago

Sorry, I'm struggling a little bit to understand how I can make a code suggestion in github, hence making it here. If you can give me the high level steps on how to do a proper code suggestion (would it be a pull request in this case?), I'm willing to do it. Here the line:

webhook_url = URL(webhook_url).with_port(None)

This change is non-impacting for all other users and it fixes my specific issue: Withings sends data to port 443 even though my external_url points to 9999, which is exactly what I want.

[edit] I simplified to a single line, original proposition was incorrect, since line was already in the part of code where the webhook registration was being aborted.

joostlek commented 1 year ago

But like I said, this is a wider problem. I don't see this to be a fix specific to Withings. What if tomorrow you start using Netatmo or Traccar, then they also have to be changed to handle your use case. Instead this would have to be picked up at a higher level. We do a call to the webhook component to ask for a valid webhook url. They return an URL we use. If you want to use a separate webhook url, that would have to be handled in that part of the application and not at this level.

joostlek commented 1 year ago

The thing I fixed here is that if you don't expose HA at 443, we fall back to polling to make sure you are still able to receive data and not end up with an invalid integration.

Qhilm commented 1 year ago

But this is a limitation specific to Withings, they do not support any port different than 443 or 80 for webhooks in their cloud. I have no idea if it's the same for netatmo (for example), hence I would remove the port only for Withings at the moment, as I do not know if other applications (netatmo, etc.) all have the same limitation.

If one day Withings starts supporting other ports, this suggestion will continue to work, it will simply become supefluous and can then be removed.

joostlek commented 1 year ago

But I think it's wrong to check if the port is 443, and if it isn't we automatically try registering 443. Like it would work technically, but not in all cases. What if that port is mapped to something else? The HEAD request would pass, Withings would think you are correctly receiving webhooks on that url, thus will send webhook requests. Eventually when they send webhooks, they will fail. Your integration doesn't work. And in 20 days, Withings will remove that webhook again and the integration never got new information since they did not receive a webhook.

Automatically falling back to try to use 443 would be expecting someone has that set up. And I think that's a bad practice with a lot of unwanted side effects.

I mean you can always install the integration as a custom component and do your change. But I am not going to hold your hand when it breaks after an update. (and I won't be trying to intentionally break it, dont worry, since this works well for all other cases)

joostlek commented 1 year ago

And to add, if we do that, we have to expect something is running at 443. If it doesn't, they get the same error message as you and the integration becomes unusable since the integration can't detect if we're also running at 443.

Qhilm commented 1 year ago

I see, you want the fallback to polling to happen rather than attempting to fallback to port 443 for webhook, as you feel it's more reliable.

The crux is, the integration cannot know if POST to webhook is really working, hence the only way to decide if integration should fallback to polling is checking webhook scheme and port number. Automatically changing scheme and port number to something that Withings supports (https and 443 for example) does not guarantee it will be received on the specific HA instance. As you said, 443 could be mapped to something else, and in this case it would result in neither polling nor webhook notifications happening.

ok, then it's more complex, it would require to let the user actively choose, via some additional integration config.

I am really not sure it is not a fix for a higher level, because this choice is only required for services like withings which are unable to support non-443 ports. Any other service should use the standard external HA port, not matter if it's 443 or something else like 853.

joostlek commented 1 year ago

But I am going to be honest here, I am not going to change the integration and add configuration options for one network setup that doesn't work.

The reason I even removed the option to choose for webhooks or not has been removed as we can detect it. We should not bother users with what a webhook is, we should only communicate what that means for them. We can detect this.

For solving this externally, a webhook URL is usually public without authentication. I can imagine that someone might have their whole setup locked down except for their webhook URL. In that case it might make sense to have a predefined webhook url in their configuration.

Like, we already have 3 ways to receive data: home assistant cloud, an external url and if that all doesn't work you can just poll. This covers 99.9% of all use cases (not backed up by real numbers, but if this was a bigger deal this issue would've exploded by now).

And don't get me wrong, I can help you install it as a custom component so you can still enjoy your webhooks. But I won't add this configuration option in the integration for a single case.

Qhilm commented 12 months ago

I understand, no problem. How often does the polling happen?

I think the real solution is to request Withings to support alternate ports actually. All of the discussion above is about working around their limitation.

joostlek commented 12 months ago

Thanks for understanding :)

It should poll every 10 minutes. The only thing that you are missing without polling is the in bed sensor. Furthermore everything should work. If you want a different polling interval, there are ways to do that via an automation.

mkaatman commented 12 months ago

I understand, no problem. How often does the polling happen?

I think the real solution is to request Withings to support alternate ports actually. All of the discussion above is about working around their limitation.

Actually, let's ask them for mqtt support 😁

joostlek commented 12 months ago

Lmao, that would be nice

mkaatman commented 12 months ago

I asked for mqtt on LinkedIn. Maybe if a few others ask it could get it on their radar?