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
73.51k stars 30.71k forks source link

Smartthings component base_url check doesn't consider proxy configurations #33980

Closed fcassirer closed 4 years ago

fcassirer commented 4 years ago

https://github.com/home-assistant/core/blob/bc26be3c11ea40ed8d423f35a0fcbaf7b9cf7cb4/homeassistant/components/smartthings/smartapp.py#L96

I will apologize in advance if I have this incorrect, but I ran into catch-22 configuration error with home assistant. In trying to install my konnected.io component, it requires that the internal configuration.yaml NOT specify an ssl setup. It recommends using the nginx add-on. (See https://www.home-assistant.io/integrations/konnected/) This however precludes using the Smartthings integration as it checks that the base_url has https:// as part of the base_url webhook entry. In my configuration I can make one or the other work, but not both. I can make external access work with an http: based internal network and https: on my external via nginx or setup the cert on home assistant directly and just redirect my external duckdns there (without nginx). I am sure there are dozens of other creative solutions to how to layer the network access with a number of them choosing to not have the homeassistant internal instance ssl protected.

In the former case (no https:/ssl config in configuration.yaml) I get: The 'base_url' of the 'http' integration must be configured and start with 'https://' Connection lost. Reconnecting… on startup. In the latter, konnected.io fails.

With this use-case in mind I would suggest that the url endpoint that I provide to smartthings shouldn't be solely derived from the base_url in configuration.yaml (api?), it would seem that there could/should be several layers such as an external_url for home-assistant as well as a component level base_url override. That would support the greatest choice of configurations.

Thanks for looking at this and/or setting me straight, either outcome works ;-)

-Fred

dshokouhi commented 4 years ago

Please do not delete the issue template, all requested information is important to help you out.

fcassirer commented 4 years ago

I didn't delete any template, I created the issue from a pulldown on the line of code I was investigating. Click any line of code in a repo you get a "..." To the left of the line number, click and you can create an issue.

fcassirer commented 4 years ago

The problem

https://github.com/home-assistant/core/blob/bc26be3c11ea40ed8d423f35a0fcbaf7b9cf7cb4/homeassistant/components/smartthings/smartapp.py#L96

I will apologize in advance if I have this incorrect, but I ran into catch-22 configuration error with home assistant. In trying to install my konnected.io component, it requires that the internal configuration.yaml NOT specify an ssl setup. It recommends using the nginx add-on. (See https://www.home-assistant.io/integrations/konnected/) This however precludes using the Smartthings integration as it checks that the base_url has https:// as part of the base_url webhook entry. In my configuration I can make one or the other work, but not both. I can make external access work with an http: based internal network and https: on my external via nginx or setup the cert on home assistant directly and just redirect my external duckdns there (without nginx). I am sure there are dozens of other creative solutions to how to layer the network access with a number of them choosing to not have the homeassistant internal instance ssl protected.

In the former case (no https:/ssl config in configuration.yaml) I get:

The 'base_url' of the 'http' integration must be configured and start with 'https://'
Connection lost. Reconnecting… 

on startup. In the latter, konnected.io fails.

With this use-case in mind I would suggest that the url endpoint that I provide to smartthings shouldn't be solely derived from the base_url in configuration.yaml (api?), it would seem that there could/should be several layers such as an external_url for home-assistant as well as a component level base_url override. That would support the greatest choice of configurations.

Thanks for looking at this and/or setting me straight, either outcome works ;-)

-Fred

Environment

Uncomment this if you are using SSL/TLS, running in Docker container, etc.

http:

use_x_forwarded_for: true

trusted_proxies:

- 172.30.32.0/23

- 127.0.0.1

- ::1

base_url: https://XXXXXXXXXXX.duckdns.org

ssl_certificate: /ssl/fullchain.pem

ssl_key: /ssl/privkey.pem

Text to speech

tts:

group: !include groups.yaml automation: !include automations.yaml script: !include scripts.yaml scene: !include scenes.yaml


## Traceback/Error logs
<!--
  If you come across any trace or error logs, please provide them.
-->

```txt
Log Details (WARNING)
Logger: homeassistant.components.smartthings
Source: components/smartthings/__init__.py:81
Integration: smartthings (documentation, issues)
First occurred: April 10, 2020, 6:12:45 PM (1 occurrences)
Last logged: April 10, 2020, 6:12:45 PM

The 'base_url' of the 'http' integration must be configured and start with 'https://'

Log Details (ERROR)
Logger: aiohttp.server
Source: /usr/local/lib/python3.7/site-packages/aiohttp/web_protocol.py:355
First occurred: April 10, 2020, 6:13:06 PM (17642 occurrences)
Last logged: 10:54:41 AM

Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/aiohttp/web_protocol.py", line 275, in data_received
    messages, upgraded, tail = self._request_parser.feed_data(data)
  File "aiohttp\_http_parser.pyx", line 523, in aiohttp._http_parser.HttpParser.feed_data
aiohttp.http_exceptions.BadStatusLine: invalid HTTP method

Additional information

I believe the aiohttp invalid HTTP method traceback (which is crushing my logs) is from the konnected.io component, but I cannot be sure. I have gone through all my configurations (via the UI as well as a

$ sudo docker exec -it homeassistant  /bin/bash 
$ find /config -type f |xargs grep https://

to find anything that might have an https:// url associated with it ...

The current configuration is using a non-ssl configuration.yaml with nginx add-on, external url:port is port forwarded from the nighthawk router to internal port 443

frenck commented 4 years ago

As of Home Assistant 0.110 this has been solved, since the internal and external URL can be set separately.